Uh oh!
There was an error while loading. Please reload this page.
skill install: handle a repository that is a collection of skills - #437
Merged
Conversation
`skill install` assumed a repository is one skill: it clones the source into `<skills-dir>/<name>` and stops. Claude and Kimi discover skills by scanning exactly one level deep, so a repository whose skills live in subdirectories lands every one of them a level too deep, where nothing will ever find them. `git clone` still exits 0, so moshcode reports the install succeeded. The user gets a green summary and zero usable skills, with nothing to suggest otherwise. - `skillCollection(dir)` reports what a clone actually contains: `single` (a SKILL.md at the root), `collection` (subdirectories holding one), or `empty` - `settleSkillClone(dir)` resolves a clone into the shape engines scan. A single skill is left alone; a collection has each skill moved up beside its siblings and the wrapper removed, since the wrapper holds the repository's README, tooling and CI, none of which is a skill; an empty clone is removed rather than left as a directory that can never resolve - claude and kimi actions carry the clone target as `settle`, so the runner resolves them after a successful clone. Gemini installs natively and is never settled - a clone containing no SKILL.md anywhere is now reported as failed with that reason, instead of counting as installed - results carry `kind`, `skills` and `kept`, so the summary can say what landed A skill whose name is already taken is left alone and reported in `kept`. This runs inside the user's real skills directory, so a name collision must never silently replace a skill they already had. `settle` is injectable alongside `run`, matching how the suite already stubs subprocesses. Three existing tests stubbed `run` without it, so the real settle correctly found an empty directory where a stubbed clone never landed; they now stub both. Verified end to end against a real 13-skill collection: 0 discoverable before, 13 after, wrapper removed. Full suite green (2171 pass, 0 fail). Extends prd/0003. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7KVQWbU17PR2mCUn8jjpH
ThreatCrush Security Scan0 finding(s) in the 6 file(s) this pull request changes. Nothing in the files this pull request changes. 59 pre-existing finding(s) elsewhere in the repository — **HIGH/CRITICAL**: 5 | **MEDIUM**: 48 | **LOW**: 6Not introduced by this pull request. The full set is in the Security tab.
…and 39 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The bug
skill installassumes a repository is one skill. It clones the source into<skills-dir>/<name>and stops.Claude and Kimi discover skills by scanning exactly one level deep. So a repository whose skills live in subdirectories — a collection — lands every one of them a level too deep, where nothing will ever find them.
git clonestill exits 0, so moshcode reports the install succeeded. The user gets a green summary and zero usable skills, with nothing to suggest otherwise. That silence is the part worth fixing: a loud failure here would have been harmless.Verified against a real 13-skill collection before the change:
The change
skillCollection(dir)reports what a clone actually contains:single(aSKILL.mdat the root),collection(subdirectories holding one), orempty.settleSkillClone(dir)resolves a clone into the shape engines scan:singleis left exactly as cloned — the existing behaviour, unchanged.collectionhas each skill moved up beside its siblings and the wrapper removed. The wrapper holds the repository's README, tooling and CI, none of which is a skill.emptyis removed rather than left as a directory that can never resolve.claudeandkimiactions carry their clone target assettle, and the runner resolves it after a successful clone. Gemini installs natively and is never settled.SKILL.mdanywhere is now reported as failed, with that reason, instead of counting as installed.kind,skillsandkept, so the summary can say what actually landed.After:
Safety
A skill whose name is already taken is left alone and reported in
kept. This runs inside the user's real skills directory, so a name collision must never silently replace a skill they already had. There's a test pinning exactly that.On the test changes
settleis injectable alongsiderun, matching how the suite already stubs subprocesses.Three existing tests stubbed
runwithout it. With a stubbed clone nothing ever lands, so the real settle correctly found an empty directory and reported failure — the new strictness working as intended, on a fixture that was never meant to exercise it. Those tests now stub both. TheskillInstallActiondeep-equals were updated to assert the newsettlefield rather than ignore it.Verification
test/skill-install-collections.test.mjs, 12 tests covering shape detection, settling, collision safety, and that a failed clone is never settled.Extends prd/0003, which set out the fan-out but assumed one repository is one skill.
🤖 Generated with Claude Code
https://claude.ai/code/session_01B7KVQWbU17PR2mCUn8jjpH