Repository navigation
fix: keep skill sync inside the package and correct install docs - #13
Merged
Merged
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.
Defects
Shared sync followed links
A throwaway package whose
skills/ship/scripts/pr-state.mjswas a symlink or a hard link to a file outside the skill:bun scripts/metadata.tsexited 0 and replaced that file with the shared script. Replacingshared/scripts/page-audit.mjswith a symlink to the textSMUGGLEDcopied that text intoskills/discoverability/scripts/page-audit.mjs.validatePackagereported no findings when the symlink target's bytes matched the skill copies. A manifest key../catalog.jsonwas read, then rejected only after the read. A hard-linkedagents/openai.yamlwas on the same write path. Running metadata also rewrote every picker file with a trailing space on nested keys (interface:), so a second run was not idempotent.Shared sources and copies must now be regular single-link files inside
shared/and the skill. Escaping manifest paths are rejected before they are read. Picker files get the same link check. A failed check writes nothing. Trailing spaces from the YAML emitter are stripped sobun run metadataleaves a clean tree.oneshot invocation locks were unchecked
Removing
disable-model-invocation: truefromskills/oneshot/SKILL.md, or settingallow_implicit_invocation: truein its picker metadata, leftvalidatePackagewith no oneshot findings. The package check now requires both locks.Install docs disagreed with the CLI
The README said
--scope globalinstalls skills everywhere and that an edited folder is left alone.docs/installation.mdsaid the same, and its--dry-runcomment said the command shows what would change. Global scope writes.claude/skillsand.agents/skillsunder the home directory. A differing selected folder returns "Nothing changed" and writes nothing, including under--dry-run.--force --dry-runreportsreplaceand still writes nothing.listaccepts only--json. The CLI help omitted the stop-on-difference rule. The README, installation doc, and help text now say this.Frontmatter that is missing, misnamed, blank, too long, or invalid YAML was already rejected and is now tested. Install tests also lock a file blocking the destination, a symlinked package skill tree, a symlinked skills directory, read-only
listand--version, and an executable bit that drifted.Verification
bun test— 77 pass, 0 failbun run check— package check passed, 54 pass, packed CLI smoke passednode skills/deslop/scripts/slop-scan.mjs README.md docs/installation.md --fail-on high— 0 highNo publishing.