fix(cli): skills check fails closed, and the installer installs skills itself - #784
Open
matt82198 wants to merge 4 commits into
Open
fix(cli): skills check fails closed, and the installer installs skills itself#784matt82198 wants to merge 4 commits into
matt82198 wants to merge 4 commits into
Conversation
… path doctor.js treated a missing ~/.claude/skills/power|buildsystem as a WARN and returned passed:true, so a green doctor could still mean the orchestrator had no skills to invoke. Claude Code only scans ~/.claude/skills/ (or a project's .claude/skills/) -- the scaffolded ./skills/ directory is never discovered, so an uncopied skill is an unusable one. The check now fails closed and the hint names the actual source directory (only when it exists) plus the restart step. The install text was also backwards: bin/cli.js told users to copy FROM ~/.claude/skills/ TO the project, which is the opposite of what makes skills discoverable. All four next-steps blocks and the README now give the correct direction. Also repairs a broken README code fence that swallowed the following section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scaffolding printed instructions for installing skills and then relied on the adopter to run them. Since Claude Code only discovers skills under ~/.claude/skills/, anyone who skipped that step got a fleet whose orchestrator could not be invoked -- and, until the preceding commit, a doctor that called it green. The installer now does the install itself. installSkills() copies each scaffolded skills/*/ into the skills home and is idempotent: a skill whose SKILL.md matches byte-for-byte is reported as already-installed, and one that DIFFERS is preserved and warned about rather than silently overwritten -- adopters who customize a skill do not lose it to a re-scaffold. --force overwrites, --no-skills opts out, and a symlinked skills home is refused (same guard as the pre-push hook install). AESOP_SKILLS_HOME redirects the target so tests never touch the real ~/.claude. installDependencies() stays behind --install-deps so scaffolding remains offline-safe; it runs npm install and pip install -r requirements.txt in the target and degrades to a warning rather than aborting the scaffold. requirements.txt / requirements-dev.txt now ship with the scaffold (they had no way to reach a target before) and are added to the re-scaffold allowlist, which otherwise reads its own output as unexpected files and refuses to re-run. package-lock.json version was stale at 0.7.0 against package.json's 0.7.2. Tests: tests/cli-skills-install.test.mjs (7 cases). Full Node suite 322/322. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ck-fail-closed # Conflicts: # README.md # bin/CLAUDE.md # tools/CLAUDE.md
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
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.
Why
doctor.jstreated a missing~/.claude/skills/power|buildsystemas a WARN and returnedpassed: true, so a green doctor could still mean the orchestrator had no skills to invoke. Claude Code only scans~/.claude/skills/(or a project's.claude/skills/) — a skill left in the scaffolded./skills/is undiscoverable.The install text was also backwards:
bin/cli.jstold users to copy from~/.claude/skills/to the project, the opposite of what makes skills discoverable.Then the deeper point: the installer was printing instructions and hoping. It now does the install.
What changed
Doctor — skills check fails closed; the hint names the actual source directory (only when it exists) plus the restart step.
Scaffolder —
installSkills()copies eachskills/*/into the skills home:SKILL.mdmatches byte-for-byte is reported as already-installed.--forceoverwrites.--no-skillsopts out; a symlinked skills home is refused (same guard as the pre-push hook install).AESOP_SKILLS_HOMEredirects the target so tests never touch the real~/.claude.Dependencies —
installDependencies()sits behind--install-depsso scaffolding stays offline-safe.requirements.txt/requirements-dev.txtnow ship with the scaffold; they previously had no way to reach a target at all. Both are added to the re-scaffold allowlist, which otherwise reads its own output as unexpected files and refuses to re-run.Also repairs a broken README code fence that swallowed the following section, and syncs
package-lock.json(stale at 0.7.0 against package.json's 0.7.2).Tests
tests/cli-skills-install.test.mjs— 7 cases: skills land in the skills home,--no-skillsopts out, re-scaffold is idempotent, a modified skill survives without--forceand is replaced with it, dependency manifests ship, and the real~/.claudeis provably untouched.Full Node suite 322/322.