Skip to content

fix(skill): mark installed skills internal#279

Merged
kunchenguid merged 2 commits into
mainfrom
fm/nm-init-internal-t2
Jun 11, 2026
Merged

fix(skill): mark installed skills internal#279
kunchenguid merged 2 commits into
mainfrom
fm/nm-init-internal-t2

Conversation

@kunchenguid

Copy link
Copy Markdown
Owner

Intent

Fix no-mistakes so the skill files that 'no-mistakes init' vendors into target repos (.claude/skills/no-mistakes/SKILL.md and .agents/skills/no-mistakes/SKILL.md) are marked internal via 'metadata.internal: true' in the YAML frontmatter, so end-user skill discovery tools like 'npx skills add /' never surface the vendored copy alongside a repo's real public skills. Downstream repos (lavish-axi, chrome-devtools-axi, gh-axi) were patched by hand with this flag, but init refresh overwrites vendored files from its template and would strip it, so the fix belongs at the source. Deliberate decisions: (1) the rendering was split in internal/skill into Markdown() - the canonical public document, byte-identical to before, still used for skills/no-mistakes/SKILL.md which must REMAIN discoverable via npx skills add kunchenguid/no-mistakes - and a new InstalledMarkdown() that only adds the metadata.internal block; Install() now writes InstalledMarkdown() on both fresh init and refresh. (2) cmd/genskill was deliberately extended to also generate and drift-check this repo's own vendored copy at .agents/skills/no-mistakes/SKILL.md (.claude/skills is a symlink to it), so make skill / make lint keep the vendored copy in lockstep and discovery on this repo surfaces exactly one no-mistakes skill; the committed .agents/skills/no-mistakes/SKILL.md change is that regenerated output. (3) Tests updated accordingly: internal/skill Install tests now compare against InstalledMarkdown(), a new TestInstalledMarkdownMarksInternal guards that installed copies carry the marker, that the public rendering does not, and that the two renderings differ only by the marker; the e2e assertSkillInstalled helper now requires the marker. Verified end to end: fresh init and refresh-over-old-unflagged-copy in scratch /tmp repos both write the flag, npx skills add on the scratch repo finds no skills, and on this repo finds exactly the one public skill. go test -race ./..., make e2e, and make lint all pass.

What Changed

  • Marked no-mistakes init vendored skill files as internal while keeping the public skills/no-mistakes/SKILL.md discoverable.
  • Split public and installed skill rendering so fresh installs and refreshes write metadata.internal: true to both .claude and .agents skill copies.
  • Updated generated skill docs, drift checks, and tests to require the internal marker for installed copies.

Risk Assessment

✅ Low: The change is narrow, preserves the public skill rendering while marking only installed vendored copies internal, and updates generator and install assertions consistently.

Testing

Focused skill and init e2e tests passed, a manual CLI transcript demonstrated fresh init, refresh-over-stale-copy, target-repo discovery suppression, and public repo discovery, then the full Go race suite and full e2e suite passed; an initial evidence run exposed a setup-only Unix socket path length issue, so the manual check was rerun with a shorter approved temp NM_HOME and transient artifacts were cleaned.

Evidence: End-to-end init and skills discovery transcript

Source: End-to-end init and skills discovery transcript

Shows `no-mistakes init` installing `metadata.internal: true` in both vendored skill locations for fresh and refreshed target repos, `skills add --list` finding no skills in those target repos, and finding exactly one public `no-mistakes` skill in this repo.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • go test -race ./internal/skill -run 'TestInstalledMarkdownMarksInternal|TestInstallWritesBothPaths|TestInstallIsIdempotent|TestInstallOverwritesStaleContent' -count=1
  • go test -race ./internal/e2e -tags e2e -run 'TestInit' -count=1
  • go build -o .no-mistakes/evidence/fm/nm-init-internal-t2/no-mistakes-test-bin ./cmd/no-mistakes
  • Manual end-to-end check: ran no-mistakes init in a fresh git repo with an origin remote, inspected .claude/skills/no-mistakes/SKILL.md and .agents/skills/no-mistakes/SKILL.md, then ran npx --yes skills add <fresh-target-repo> --list --full-depth.
  • Manual end-to-end check: seeded a target repo with a stale unmarked .claude/skills/no-mistakes/SKILL.md, ran no-mistakes init, inspected refreshed .claude and .agents frontmatter, then ran npx --yes skills add <refresh-target-repo> --list --full-depth.
  • Manual public-discovery check: inspected skills/no-mistakes/SKILL.md frontmatter and ran npx --yes skills add . --list --full-depth.
  • go test -race ./...
  • make e2e
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

no-mistakes init vendors the agent skill into target repos
(.claude/skills and .agents/skills). Skill discovery tools like
`npx skills add <owner>/<repo>` surfaced those vendored copies
alongside the repo's own public skills, and init refreshes would
strip any downstream-added flag.

Split the skill rendering: Markdown() stays the canonical public
document (skills/no-mistakes/SKILL.md, still discoverable), while
the new InstalledMarkdown() adds `metadata.internal: true` to the
frontmatter and is what Install() writes, on fresh init and refresh
alike. genskill now also generates and drift-checks this repo's own
vendored copy in .agents/skills, so discovery on this repo surfaces
exactly one no-mistakes skill.
@kunchenguid kunchenguid merged commit d7878f7 into main Jun 11, 2026
9 checks passed
@kunchenguid kunchenguid deleted the fm/nm-init-internal-t2 branch June 11, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant