fix(git-transport): support path: "." / "" so skills at the repo root are fully fetched#1711
Open
besdar wants to merge 1 commit into
Open
fix(git-transport): support path: "." / "" so skills at the repo root are fully fetched#1711besdar wants to merge 1 commit into
path: "." / "" so skills at the repo root are fully fetched#1711besdar wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
rulesync installwithtransport: "git"only fetches root-level files when the configuredpathpoints at the repository root ("",".", or"./"). For example, given:{ "source": "https://github.com/feature-sliced/skills", "path": ".", "skills": ["feature-sliced-design"], "transport": "git" }…only
README.mdends up in.rulesync/skills/.curated/feature-sliced-design/, while the actual skill (feature-sliced-design/SKILL.md+ thereferences/folder) is silently dropped. No error is reported, which makes the failure mode quite confusing —installhappily says “Fetched 1 skill(s)”.Root cause
fetchSkillFilesinsrc/lib/git-client.tsruns:git sparse-checkoutin cone mode (the default since git 2.27) treats./""/./as “top-level files only — exclude every subdirectory”. So aftercheckout, the working tree contains only the files directly at the repo root, and the subsequentwalkDirectory(<tmpDir>/.)only finds those root files.The downstream
groupRemoteFilesBySkillRootfallback atsrc/cli/commands/install/install-rulesync.ts(the “rootLevelFiles → singleSkillName” branch) then groups every root-level file under the requested skill name. That’s why the README is written as if it were the body of the skill.This makes it impossible to install via
transport: "git"from any repository that stores skills at the root (<repo>/<skill-name>/SKILL.mdwithout askills/container directory).feature-sliced/skillsis one such repository — the github transport works fine because it uses the RESTlistDirectoryAPI which has no equivalent quirk.Fix
In
fetchSkillFiles, treat"",".", and"./"as “the whole repo”:git -C <tmpDir> sparse-checkout disableinstead ofsparse-checkout set -- ., so the full working tree is restored.<tmpDir>itself as the skills root passed towalkDirectory, instead of<tmpDir>/..For any other
pathvalue the behavior is unchanged.Repro
Before the fix:
$ cat rulesync.jsonc { "sources": [ { "source": "https://github.com/feature-sliced/skills", "path": ".", "skills": ["feature-sliced-design"], "transport": "git" } ] } $ rulesync install Installing skills from 1 source(s)... Fetched 1 skill(s) from https://github.com/feature-sliced/skills: feature-sliced-design Installed 1 skill(s) from 1 source(s). $ ls .rulesync/skills/.curated/feature-sliced-design README.md # ← only this file, no SKILL.md, no references/After the fix (same config):
Changes
src/lib/git-client.ts— branch onskillsPath ∈ {"", ".", "./"}: callsparse-checkout disableand use the clone root directly as the skills root.src/lib/git-client.test.ts— added two test cases:it.each(["", ".", "./"])("disables sparse-checkout when skillsPath is %j (repo root)", …)— assertssparse-checkout disableis invoked andsparse-checkout setis not."uses the clone directory itself as the skills root when skillsPath is the repo root"— asserts files are read from<tmpDir>directly.Notes
<repo>/SKILL.md(a single root-level skill, no per-skill subdirectory) already exists ingroupRemoteFilesBySkillRootand continues to work; this PR only makes the data reach that fallback when usingtransport: "git".transport: "github", the default) is unaffected — it never had this issue because it lists directories via the REST API.