Repository navigation
Conversation
…edotmack#4430) plugin/skills/ ships to every user twice: in the npm tarball ("files": ["plugin/skills"]) and in the Claude marketplace plugin ("source": "./plugin"). The version-bump skill tells an agent to use the maintainer's npm token and run `npm publish`, so it was reaching every downstream install. Move it to the project-local .claude/skills/version-bump/. Claude Code still loads it as /version-bump inside this repo, but .claude/ is in .npmignore and outside the ./plugin bundle, so neither channel ships it. - .gitignore: narrow `.claude/skills/` to `.claude/skills/*` and track only version-bump - telemetry: drop `version-bump` from FIRST_PARTY_SKILL_IDS, which is pinned to plugin/skills/ by tests/telemetry/skill-id.test.ts Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…thedotmack#4430) Add a scope section that makes the agent verify the git remote and the package name before any step, and stop if either does not match. The description now says the skill is maintainer-only and does not apply to other repositories or packages, so its wording cannot be generalized to someone else's ~/.npmrc or publish rights. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hedotmack#4430) Keep the agent-run publish, but add one human check at the only step that reaches downstream users: - run `npm publish --dry-run` first and post the version, tag, and tarball summary in chat - wait for an explicit yes for this version before the real publish; a standing "release it" instruction does not count - stop if `npm whoami` is not thedotmack - never read, print, copy, or edit ~/.npmrc; npm uses it directly. On an expired token, open the npm token page and let the maintainer update ~/.npmrc himself instead of the agent rewriting it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fail if version-bump reappears in any shipped skill root (plugin/, cowork/, cursor, grok-bot, openclaw), if package.json "files" or a marketplace source picks up .claude/, or if any shipped SKILL.md references ~/.npmrc. Also pin the scope check and publish confirmation gate in the maintainer skill. Fails on 71ddd11 (3 of 4), passes with the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
…ds (thedotmack#4430) Address review feedback on the shipping guard: - Check every plugin source in all three marketplace manifests (.claude-plugin, .agents/plugins, .cursor-plugin), including the object form {source: "local", path}, by resolving it and failing when the directory contains .claude/skills/version-bump. A source of "." or "./" now fails; before, only sources starting with ".claude" did. - Apply the same containment check to package.json "files". - Assert the actual safeguard instructions, not just headings: the remote and package-name commands, "If either check fails, stop", the explicit-yes wait, the standing-instruction rule, and the npm whoami stop. Checked by mutation: a source of "." or "./", a "files" entry of .claude/skills, and deleting each safeguard line all fail the test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
|
I pushed e5ee69f to cover both Greptile findings. No CI checks have run on this PR yet; I think workflows from forks need a maintainer's approval. @thedotmack, could you approve the workflow run when you get a chance? Locally, the new test and the related telemetry and skill tests pass (23 tests). |
This comment has been minimized.
This comment has been minimized.
…#4430) The npm "files" guard resolved each entry as a literal path, so a glob such as ".claude/skills/*" slipped past it even though npm packs the version-bump skill under that pattern (.npmignore does not veto it). Ask npm instead: run `npm pack --dry-run --json --ignore-scripts` on the repo and fail if any .claude/ path is selected. Add regression cases that pack a throwaway fixture (repo .npmignore + the skill) with ".claude", ".claude/skills", ".claude/skills/*", ".claude/**" and the skill dir, and confirm each is caught. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
Closes #4430
Problem
plugin/skills/version-bump/SKILL.mdtells an agent to use the maintainer's npm token from~/.npmrcand runnpm publish. Everything inplugin/skills/ships to users through two channels:"files": ["plugin/skills", ...]"source": "./plugin"So every install got a release workflow that only makes sense on the maintainer's machine. Removing it from
filesalone wouldn't help, because the marketplace bundle would still include it.Changes (one commit per stage)
fix(skills): move the skill out of the shipped bundle.plugin/skills/version-bump/→.claude/skills/version-bump/. Claude Code still loads it as/version-bumpwhen run inside this repo (checked locally: it shows up as a project skill)..claude/is already in.npmignoreand outside./plugin, so neither channel ships it..gitignore:.claude/skills/→.claude/skills/*plus!.claude/skills/version-bump/, so only this skill is tracked.FIRST_PARTY_SKILL_IDS: removedversion-bump.tests/telemetry/skill-id.test.tsrequires that list to matchplugin/skills/, so maintainer invocations now count asother.docs(version-bump): scope guard. Before any step, the agent checks thatgit remote get-url originisthedotmack/claude-memand the package name isclaude-mem, and stops if either doesn't match. The description now says the skill is maintainer-only.docs(version-bump): confirmation beforenpm publish. The agent still runs the publish. It first runsnpm publish --dry-run, posts the version, tag and tarball summary, and waits for an explicit yes for that version. It never reads, prints, copies or edits~/.npmrc. When the token has expired, it opens the npm token page and the maintainer updates~/.npmrchimself (before, the agent rewrote the file with a placeholder).test: regression guard.tests/infrastructure/maintainer-skills-not-shipped.test.tsfails if:version-bumpshows up in any shipped skill root (plugin, cowork, cursor, grok-bot, openclaw)filesor a marketplace source includes.claude/SKILL.mdmentions.npmrcIt fails on
main(3 of 4 tests) and passes on this branch.Commits 2 and 3 change the workflow itself. If you want to keep fully unattended publishing, drop commit 3; commits 1 and 4 fix the distribution problem on their own.
Testing
bun test tests/infrastructure/maintainer-skills-not-shipped.test.ts tests/telemetry/skill-id.test.ts tests/utils/skill-docs-placement.test.ts: all passtests/infrastructure/plugin-distribution.test.ts: one test (#3424, expectsdist/bug-report/index.js) fails on a fresh clone without a build, onmainas well as on this branch. Not related to this change.🤖 Generated with Claude Code