Repository navigation
feat(cli): ship the skills — build copies them, plur init installs them (#1190) - #1191
Merged
Merged
Conversation
…em (#1190) skills/plur-create-engrams/ has been maintained and version-stamped by release.sh on every release while shipping to nobody. Two gaps, each invisible alone: - packages/cli/package.json declares files: ["dist"], and skills/ lives at the REPO root. files[] is package-relative, so no manifest entry could reach it — adding "skills" there ships nothing at all. - plur init had no skill-installation leg. Its one `Skill` reference is a PreToolUse matcher that fires WHEN a skill is invoked, which reads as coverage at a glance and is a different thing. So `npm install -g @plur-ai/cli` delivered no engram-authoring guidance, and the per-release version bump made it look shipped. Engram quality degraded accordingly: the guidance on what earns a place in memory, and how to write a statement, rationale and boundary that still hold months later, was not present while engrams were being written. Both halves: - scripts/copy-skills.mjs copies the tree into dist/skills after tsup (the first tsup config sets clean: true, so a copy that runs first is wiped). files: ["dist"] already covers it, so no manifest change. The whole tree travels, including references/, which SKILL.md tells the agent to read before serialising. - installSkills() writes to skills/ beside the settings.json init is already writing, so it follows init's existing scope decision — --global to ~/.claude/skills, project mode to ./.claude/skills. No new flag. Idempotent; contained like the harness legs so an unwritable directory cannot abort the hooks and MCP registration; and it reports an overwrite of a locally-changed skill rather than clobbering in silence. Verified: npm pack --dry-run lists all 8 files; init lands them in a sandbox, reports "already current" on re-run, and names a locally-changed skill it overwrote. test/init-skills.test.ts holds both halves and fails if either is removed (confirmed by removing dist/skills: 3 tests fail). cli suite 64 files / 700 passed. CHANGELOG: 0.20.0 declared, headed by this fix. Provenance is demoted to a section and marked experimental and off by default — record generation defaults to `never`, the flags are opt-in, and the profile is 0.9 draft and OPTIONAL — with its two non-dormant behaviour changes called out (export now requires a licence; credentials in attribution/rationale/source are refused at write time even locally). Closes #1190 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S4or5TZEfYoreFm7GLJwRJ
…weet release.sh builds the tweet from the section's first four bullet LINES (grep '^- ' | head -4) and its first non-bullet paragraph. The section led with the two bullets describing what was BROKEN, so the generated tweet read "✅ plur init had no skill-installation leg" — the defect, marked as a feature — and each bullet was truncated at its hard wrap into a fragment. At 463 chars it also exceeded the 270-char budget, which Step 3.5 aborts on. Lead with what shipped instead, in four short bullets that survive the line-based extraction. Tweet is now 262/270 and reads correctly; the detail moved into the prose below, which is the better shape for release notes anyway. All 59 user-facing PRs since v0.19.4 stay declared — verified against the Step 3.6 gate logic. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S4or5TZEfYoreFm7GLJwRJ
… the tests
Three defects found reviewing the previous commit.
1. The mtime heuristic was wrong in the commonest path, and untestable in CI.
installSkills decided "local edit" vs "newer shipped version" by comparing
mtimes. npm normalises EVERY mtime in a published tarball to 1985-10-26
(verified: `tar -tvf` on npm pack output shows `26 okt. 1985`), so on any
real install the bundled copy is older than anything on disk and every
ordinary upgrade would have reported "overwrote locally-changed" — a false
alarm in the single most common path. CI could not catch it: CI builds
locally, where mtimes are real. The distinction is not decidable from what
we have, so the message now says what happened, "replaced ... (any local
edits overwritten)", and does not claim which it was.
2. A test asserted containment without causing a failure. "does not abort the
rest of init when the skills leg fails" only ran the happy path and checked
MCP was registered — it would pass with the containment removed. It now puts
a FILE where the skills directory belongs, so mkdirSync throws, and asserts
the leg reports FAILED while hooks and MCP still land. Verified by hand:
`Skills: FAILED (EEXIST ...)` with mcpServers.plur and hooks intact.
3. Double label on failure. containLeg already prefixes the label, and the
caller printed `Skills: ${status}` on top of it, giving "Skills: Skills:
FAILED". The status string now carries its own label and is printed bare,
matching the Cursor/Codex/Antigravity convention.
Also renamed the byte-vs-mtime test to say what it actually guards: it is a
contract test against a future timestamp fast path, not a regression test for
(1) — the byte check short-circuits first, so it would pass under the old code
too. Naming it for the bug would have overclaimed.
cli suite: 64 files / 701 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S4or5TZEfYoreFm7GLJwRJ
GitHub appends the PR number to a squash-merge subject, so this PR lands on main as "... (#1190) (#1191)" — the repo's own history shows the shape, e.g. "fix(core): throw RangeError on unparseable evaluation instant (#1166) (#1176)". The manifest gate parses EVERY number in a squash subject, not just the last, so #1191 counts as a shipped user-facing PR the moment this merges and must be declared or Step 3.6 aborts the release. Declaring it here, before the merge, because the CHANGELOG it would need to appear in is inside this PR. Simulated post-merge: 61 PR numbers shipped, 61 declared. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S4or5TZEfYoreFm7GLJwRJ
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.
The problem
skills/plur-create-engrams/has been maintained and version-stamped byrelease.shon every release while shipping to nobody. Two gaps, eachinvisible on its own:
packages/cli/package.jsondeclaresfiles: ["dist"], andskills/lives atthe repo root.
files[]is package-relative, so no manifest entry couldever reach it — adding
"skills"there ships nothing at all.plur inithad no skill-installation leg. Its singleSkillreference isa
PreToolUsematcher that fires when a skill is invoked — a differentthing, and one that reads as coverage at a glance.
So
npm install -g @plur-ai/clidelivered no engram-authoring guidance, whilethe per-release version bump made it look shipped. Engram quality degraded
accordingly: the guidance on what earns a place in memory, and how to write a
statement, rationale and boundary that still hold months later, was not present
while engrams were being written.
The fix
Packaging —
scripts/copy-skills.mjscopies the tree intodist/skillsafter tsup. It must run after, because the first tsup config sets
clean: trueand would wipe a copy made earlier.
files: ["dist"]already coversdist/, sono manifest change is needed. The whole tree travels, including
references/, whichSKILL.mdinstructs the agent to read before serialising —the part that actually carries the format.
Install —
installSkills()writes toskills/beside thesettings.jsoninit is already writing, so it inherits init's existing scope decision:
--global→~/.claude/skills/, project mode →./.claude/skills/. No newflag, one place decides scope. Idempotent; contained via
containLeglike theharness legs, so an unwritable directory cannot abort the hooks and MCP
registration that are the point of
plur init; and it reports overwriting alocally-changed skill rather than clobbering in silence.
Verification
npm pack --dry-runlists all 8 files underdist/skills/plur init: lands the full tree; re-run reportsalready current;after a local edit reports
overwrote locally-changed plur-memorytest/init-skills.test.ts(7 tests) holds both halves — confirmed they failif the fix is removed: hiding
dist/skillsfails 3@plur-ai/cli: 64 files / 700 passed / 0 failedCHANGELOG
Declares
## 0.20.0, headed by this fix. Provenance is demoted from headline toa section and marked experimental and off by default — record generation
defaults to
never, the flags are opt-in,plur_provenanceis behindplur_admin, and the profile is 0.9 draft and OPTIONAL. Its two non-dormantbehaviour changes are called out:
plur packs exportnow refuses without alicence, and credentials in
attribution/rationale/sourceare refused atwrite time even at local scope.
All 59 user-facing PRs since
v0.19.4are declared, so the release manifestgate (Step 3.6) passes.
Closes #1190
🤖 Generated with Claude Code
https://claude.ai/code/session_01S4or5TZEfYoreFm7GLJwRJ