Skip to content

Add release-gated npm publish job for the single v2 package - #1647

Merged
cliffhall merged 6 commits into
v2/mainfrom
1640-v2-publish-job
Jul 11, 2026
Merged

Add release-gated npm publish job for the single v2 package#1647
cliffhall merged 6 commits into
v2/mainfrom
1640-v2-publish-job

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #1640

Wires the v2 single-package publish/release job into .github/workflows/main.yml (parent tracking: #1636).

What's added

  • release: types: [published] trigger + a release-gated publish job:
    1. npm run pack:verify — the pre-publish gate (builds, npm packs the real tarball, installs it into a clean throwaway consumer, drives the installed mcp-inspector bin end to end).
    2. npm publish --access public --provenance — a single npm publish (v2 isn't an npm workspace, so no v1-style publish-all/--workspaces), with a signed provenance attestation via GitHub OIDC (id-token: write, environment: release, NPM_TOKEN). The v1 --provenance TODO is resolved now the repo is public.
  • The existing build job is unchanged; publish is gated on github.event_name == 'release' and needs: build, so normal push/PR CI is unaffected.
  • Release flow documented in the root README: single npm version at the root → tag → GitHub Release triggers publish. No per-client version sync / check-version (only the root package.json carries a version).

Deferred (split out)

Notes

  • No source/test changes — workflow + docs only, so the build job runs as before. The publish path itself can't be exercised without a real release (publishing is irreversible); pack:verify in the job is the automated pre-publish safety net.
  • Targets v2/main (non-default branch), so Closes #1640 is a cross-reference — close Wire the v2 single-package publish/release job into main.yml (at the v2→main cutover) #1640 + move its board card to Done manually on merge, per AGENTS.md.

🤖 Generated with Claude Code

…age (#1640)

Wire the v2 publish pipeline into main.yml. On a published GitHub release, a new
`publish` job runs `npm run pack:verify` (the pre-publish gate — builds, packs
the real tarball, installs it into a clean consumer, and drives the bin) then
`npm publish --access public --provenance`. This is a single `npm publish` (v2
is not an npm workspace, so no v1-style publish-all/--workspaces), with a signed
provenance attestation via GitHub OIDC (id-token: write, environment: release,
NPM_TOKEN). Add the `release: types: [published]` trigger; the existing `build`
job is unchanged and the publish job is gated on the release event.

Document the single-version release/tag flow in the root README (one `npm
version` at the root; no per-client sync / check-version since only the root
package.json carries a version).

Docker/GHCR is deferred: v2 has no Dockerfile (the v1 client/server/cli layout
is gone), and a correct image needs a net-new, docker-build-tested Dockerfile.
Split into #1646.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Jul 11, 2026
cliffhall and others added 2 commits July 10, 2026 23:47
The clients/web README was the default "React + TypeScript + Vite" template
(plus a proxy note). Replace it with a web-client-specific guide: the src/
(browser) vs server/ (Node) split and what each server/ file does, the build
pipeline (tsc -b → vite build → build:runner, producing dist/ + build/), the
component layers (elements/groups/screens/views) and theme system, the three
Vitest projects (unit happy-dom / integration node / storybook Chromium) and
the ≥90% coverage gate, Storybook (dev + play-function browser tests), and the
auth-token flow. Preserve the existing HTTP-proxy section. Points at the root
README/AGENTS.md for the repo-wide picture rather than duplicating it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
@cliffhall

Copy link
Copy Markdown
Member Author

Also bundled into this PR (per request): replaced clients/web/README.md — which was the stock "React + TypeScript + Vite" template — with a tailored web-client README covering the src/ (browser) vs server/ (Node) split, the build pipeline (tsc -bvite buildbuild:runnerdist/ + build/), the component layers + theme system, the three Vitest projects (unit/integration/storybook) and the ≥90% coverage gate, Storybook browser play-tests, and the auth-token flow. Preserved the existing HTTP-proxy note; links to the root README/AGENTS.md instead of duplicating the repo-wide overview.

@cliffhall

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 1m 44s —— View job


PR Review — Release-gated npm publish job

  • Read the workflow changes (.github/workflows/main.yml)
  • Read the README changes
  • Read the web README changes
  • Cross-check against repo conventions & packaging invariants

Overall this is a clean, well-designed publish job. Release-gated, needs: build so the whole CI gate must pass first, pack:verify as a real pre-publish smoke, and provenance wired correctly. The docs are accurate — I spot-checked the web README's factual claims (server file list, build:client/build:runner scripts, component-layer counts, the three Vitest projects) and they match the repo. Verified package.json has no private: true and carries a repository field, so --provenance will work. Nothing blocking; a few notes below.

👍 What's right

  • Auth token wiring is correct. setup-node with registry-url writes an .npmrc that reads NODE_AUTH_TOKEN, and the publish step sets NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} — the correct pairing (a common mistake is exporting NPM_TOKEN directly, which the generated .npmrc wouldn't pick up).
  • Provenance prerequisites met: id-token: write + contents: read, public repo, environment: release for secret scoping/approval. Node 22.x ships npm ≥10, which supports --provenance.
  • if: github.event_name == 'release' + needs: build keeps push/PR CI unaffected and forces a green build/coverage/smoke/storybook gate before publish.

Suggestions (non-blocking)

1. (medium) No guard that the release tag matches the published version.
npm publish ships whatever version is in the root package.json (currently 2.0.0), regardless of the release's tag name. If a maintainer drafts a release against a tag that doesn't match package.json (e.g. forgot to run npm version, or cut the release from the wrong commit), the job will happily publish a version that doesn't correspond to the release — and publishing is irreversible. A cheap assertion before publish closes this:

- name: Assert release tag matches package version
  run: |
    PKG=$(node -p "require('./package.json').version")
    TAG="${{ github.event.release.tag_name }}"
    # strip a leading v (npm version tags as vX.Y.Z)
    [ "${TAG#v}" = "$PKG" ] || {
      echo "Release tag '$TAG' != package.json version '$PKG'"; exit 1; }

This is exactly the kind of check the removed v1 check-version covered in spirit (tag ↔ artifact), even though the per-client version-sync concern is gone. Fix this →

2. (low) The build runs three times on the release path. build job (via validate/coverage) → pack:verify (builds + packs) → npm publish's prepack (npm run build). All safe and each is arguably defensible (clean-tree rebuild before packing), but worth a one-line comment noting the redundancy is intentional, since a future reader may try to "optimize" prepack away and break the tarball. Not worth changing behavior.

3. (low) No concurrency guard on the job. Two releases published in quick succession could run overlapping publishes. In practice releases are rare and the second npm publish of an already-taken version just fails, so this is minor — a concurrency: { group: publish-npm, cancel-in-progress: false } would make it tidy.

4. (nit) smoke:tui self-skip. The step comment notes smoke:tui self-skips on CI (no TTY). Good that it's called out — just confirming that's the intended behavior on the release runner too (it is; pack:verify doesn't fail on the skip).

Note: I can't edit files under .github/workflows/ (GitHub App workflow-permission limitation), so the suggestions above are for you to apply if you agree.
· 1640-v2-publish-job

Address PR #1647 review:

- Assert the release tag matches the root package.json version before publishing
  (fail fast, before install/pack:verify) — publishing is irreversible, so a
  release drafted without `npm version` or cut from the wrong commit shouldn't
  silently ship a mismatched version. Tolerates a leading `v`.
- Add a `concurrency: { group: publish-npm, cancel-in-progress: false }` guard so
  two releases in quick succession can't run overlapping publishes.
- Note in-comment that the triple build (build job → pack:verify → prepack) is
  intentional, so a future reader doesn't "optimize" prepack away and break the
  tarball.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
@cliffhall

Copy link
Copy Markdown
Member Author

Thanks for the review! Applied all three non-blocking suggestions:

1. Tag ↔ version guard — added an Assert release tag matches package version step before install/pack:verify (fail fast), comparing github.event.release.tag_name (minus a leading v) to the root package.json version. Exactly your snippet — the tag↔artifact half of what v1's check-version covered in spirit. Since publishing is irreversible, this is worth having.

2. Intentional triple build — added an in-comment note on the publish step that the build-job → pack:verify → prepack redundancy is intentional (each a clean-tree rebuild; prepack is what populates the tarball), so nobody optimizes it away.

3. Concurrency guard — added concurrency: { group: publish-npm, cancel-in-progress: false } so overlapping publishes can't race.

4. smoke:tui self-skip — confirmed intended on the release runner too; pack:verify doesn't fail on the skip.

(No worries on the workflow-file edit limitation — applied on my end.) YAML re-validated. Re-requesting review.

@claude review

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 1m 8s —— View job


Re-review — hardening applied

Went through the updated publish job. All three non-blocking suggestions landed cleanly, and the reasoning is captured in comments so a future reader won't undo them.

  • Read the updated publish job in .github/workflows/main.yml
  • Verify the tag↔version guard, triple-build comment, and concurrency guard
  • Re-check packaging invariants & conventions

✅ Verified

  1. Tag ↔ version guard (lines 113–126) — placed before npm install / pack:verify, so it fails fast on the cheap step. Strips a leading v (${TAG#v}) to tolerate npm version's vX.Y.Z tags. Confirmed the root package.json is 2.0.0, so a release tagged v2.0.0 or 2.0.0 passes and anything else blocks. 👍
  2. Triple-build note (lines 137–142) — clear comment that build-job → pack:verifyprepack is intentional and that prepack is what populates the tarball. Exactly the guardrail against a well-meaning "optimize this away" edit.
  3. Concurrency guard (lines 94–96) — group: publish-npm, cancel-in-progress: false. Serializes publishes without ever killing an in-flight one. 👍
  4. smoke:tui self-skip — confirmed; pack:verify doesn't fail on the CI skip.

Also re-checked the provenance prerequisites still hold: id-token: write + contents: read, environment: release, public repo, and package.json carries a repository field with no private: true — so --provenance will mint the attestation. YAML re-validated (parses clean).

One new nit (low, optional)

The guard interpolates the release tag straight into the shell script body:

run: |
  PKG="$(node -p "require('./package.json').version")"
  TAG="${{ github.event.release.tag_name }}"

Templating ${{ github.event.release.tag_name }} directly into a run: block is the pattern actionlint/CodeQL flag as a script-injection surface — a tag name containing shell metacharacters would be interpolated into the script. Risk here is genuinely low (only maintainers cut releases, and the tag would have to be crafted), but the zero-cost hardening is to pass it via env: so it arrives as a plain shell variable instead of being spliced into the script text:

- name: Assert release tag matches package version
  env:
    TAG: ${{ github.event.release.tag_name }}
  run: |
    PKG="$(node -p "require('./package.json').version")"
    if [ "${TAG#v}" != "$PKG" ]; then
      echo "Release tag '$TAG' does not match package.json version '$PKG'"
      exit 1
    fi

Purely defensive and doesn't change behavior — take it or leave it. Apply →

Reminder that I can't edit files under .github/workflows/ (GitHub App workflow-permission limitation), so this is for you to apply if you agree.

Verdict: LGTM. The three fixes are correct and the job is in good shape to merge; the injection nit is optional hardening, not a blocker.
· 1640-v2-publish-job

cliffhall and others added 2 commits July 11, 2026 00:15
Re-review nit: interpolating `${{ github.event.release.tag_name }}` directly
into the `run:` block is a script-injection surface (a crafted tag with shell
metacharacters would splice into the script). Pass it through `env: TAG` so it
arrives as a plain shell variable instead. Purely defensive; no behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
@cliffhall

Copy link
Copy Markdown
Member Author

Applied the injection-hardening nit — the release tag now arrives via env: TAG rather than being interpolated into the run: script body, so a crafted tag can't splice into the shell. Zero behavior change. (Good catch — it's the actionlint/CodeQL-flagged pattern even if the risk is maintainer-only here.)

@claude review

@claude

claude Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 26s —— View job


Re-review — injection hardening applied

  • Read the updated publish job in .github/workflows/main.yml
  • Verify the env: TAG script-injection fix
  • Re-confirm tag↔version guard, concurrency, triple-build note, provenance prereqs

✅ Verified

Injection hardening (the requested change) is correct. The Assert release tag matches package version step now passes the tag through env: TAG: ${{ github.event.release.tag_name }} and references "$TAG" / "${TAG#v}" in the run: body — so the tag arrives as a plain shell variable and is never spliced into the script text. This is exactly the actionlint/CodeQL-recommended fix; behavior is unchanged (a v2.0.0 or 2.0.0 tag still passes against the root package.json 2.0.0, anything else blocks). 👍

Everything from the prior rounds still holds:

  1. Tag↔version guard — placed before the heavy npm install / pack:verify, fails fast, tolerates the leading v.
  2. Concurrency guardgroup: publish-npm, cancel-in-progress: false serializes publishes without killing an in-flight one.
  3. Triple-build note — the comment makes the build-job → pack:verifyprepack redundancy intentional and explains prepack is what populates the tarball.
  4. Provenance prereqs — re-checked: id-token: write + contents: read, environment: release, public repo, root package.json has a repository field and no private: true. --provenance will mint the attestation.
  5. Auth pairingsetup-node registry-url + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} is the correct combination.

Verdict: LGTM ✅

The injection nit is resolved, no behavior change, and no new issues introduced. The publish job is in good shape to merge. Reminder that per AGENTS.md, since this targets v2/main (non-default), you'll need to manually close #1640 and move its board card to Done on merge.
· 1640-v2-publish-job

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

1 participant