Skip to content

fix(skills): keep the version-bump release skill maintainer-only (#4430) - #4575

Open
somil14 wants to merge 6 commits into
thedotmack:mainfrom
somil14:fix/4430-version-bump-maintainer-only
Open

somil14 wants to merge 6 commits into
thedotmack:mainfrom
somil14:fix/4430-version-bump-maintainer-only

Conversation

@somil14

@somil14 somil14 commented Oct 7, 2026

Copy link
Copy Markdown

Closes #4430

Problem

plugin/skills/version-bump/SKILL.md tells an agent to use the maintainer's npm token from ~/.npmrc and run npm publish. Everything in plugin/skills/ ships to users through two channels:

  • the npm tarball: "files": ["plugin/skills", ...]
  • the Claude marketplace plugin: "source": "./plugin"

So every install got a release workflow that only makes sense on the maintainer's machine. Removing it from files alone wouldn't help, because the marketplace bundle would still include it.

Changes (one commit per stage)

  1. 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-bump when run inside this repo (checked locally: it shows up as a project skill). .claude/ is already in .npmignore and 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: removed version-bump. tests/telemetry/skill-id.test.ts requires that list to match plugin/skills/, so maintainer invocations now count as other.
  2. docs(version-bump): scope guard. Before any step, the agent checks that git remote get-url origin is thedotmack/claude-mem and the package name is claude-mem, and stops if either doesn't match. The description now says the skill is maintainer-only.

  3. docs(version-bump): confirmation before npm publish. The agent still runs the publish. It first runs npm 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 ~/.npmrc himself (before, the agent rewrote the file with a placeholder).

  4. test: regression guard. tests/infrastructure/maintainer-skills-not-shipped.test.ts fails if:

    • version-bump shows up in any shipped skill root (plugin, cowork, cursor, grok-bot, openclaw)
    • package files or a marketplace source includes .claude/
    • any shipped SKILL.md mentions .npmrc
    • the scope check or the confirmation gate is removed

    It 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 pass
  • tests/infrastructure/plugin-distribution.test.ts: one test (#3424, expects dist/bug-report/index.js) fails on a fresh clone without a build, on main as well as on this branch. Not related to this change.

🤖 Generated with Claude Code

somil14 and others added 4 commits October 7, 2026 12:39
…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>
Copilot AI balanced review requested due to automatic review settings October 7, 2026 07:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Moves a release automation skill out of shipped packages.

No outstanding findings block merging this PR.

What we checked:

  • CI has npm available: CI installs Node and successfully uses npm before running the tests.

Summary

This PR keeps version-bump in the repository instead of shipping it to users. It adds repository checks and requires approval before publishing.

  • The latest change checks npm’s actual file list instead of treating files patterns as plain paths.
  • Both previous coverage findings are addressed: marketplace checks catch parent directories, and safeguard checks require the commands and approval instructions.
  • No new actionable issues were found.

Reviews (3) · Last reviewed commit: "test: check npm's real pack list for the..." · Reviewed by Greptile

Comment thread tests/infrastructure/maintainer-skills-not-shipped.test.ts Outdated
Comment thread tests/infrastructure/maintainer-skills-not-shipped.test.ts Outdated
…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>
@somil14

somil14 commented Oct 7, 2026

Copy link
Copy Markdown
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).

@greptile-apps

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

No deployments
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.

Security concern: version-bump skill instructs an AI agent to autonomously read npm credentials and publish without human review

2 participants