Skip to content

ci: add publish.yml, wire release-please to dispatch it - #21

Merged
mark-brannan merged 2 commits into
mainfrom
add-publish-workflow
Sep 22, 2026
Merged

mark-brannan merged 2 commits into
mainfrom
add-publish-workflow

Conversation

@mark-brannan

Copy link
Copy Markdown
Owner

Summary

  • Add publish.yml: npm publish via OIDC trusted publishing (no stored token), triggered only by workflow_dispatch, gated on github.ref_type == 'tag' and a tag/package.json version match.
  • release-please.yml: add actions: write, dispatch publish.yml after a release is created and wait for it to succeed. Same pattern as mark-brannan/colregs's already-working release-please.yml/publish.yml pair.

Prerequisite (one-time, on npmjs.com)

Before the first tagged release can publish, register the trusted publisher for wire-wright: package settings -> Trusted publishers -> GitHub Actions -> repo mark-brannan/wire-wright, workflow publish.yml. Until that's set, the publish job fails authentication — the correct failure mode, not a bug.

Test plan

  • YAML validated (python3 -c "import yaml; yaml.safe_load(...)") on both files
  • Matched against mark-brannan/colregs's working publish.yml/release-please.yml, trimmed to what this repo's own test.yml actually needs (no unnecessary prose-budget engine checkout or npm ci where the repo has no dependencies)
  • First real release after merge exercises the dispatch end-to-end (needs the npmjs.com trusted-publisher registration above first)

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 337d7e3b-2935-4876-a4b6-024751d41229


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude

claude Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Claude finished @mark-brannan's task in 3m 46s —— View job


Independent review — publish.yml / release-please.yml

  • Read repo conventions (no CLAUDE.md/AGENTS.md at root)
  • Review publish.yml and release-please.yml diff
  • Check for credential/secret exposure — none found
  • Run npm ci / npm test / npm run build
  • Post findings

What I checked: the full diff against origin/main, both workflow files in full (not just the diff), release-please-config.json / .release-please-manifest.json (tag prefix v, always-bump-patch versioning) for consistency with the tag-match logic in publish.yml, and test.yml for the repo's existing conventions.

What I ran:

  • npm ci — passed, 0 vulnerabilities.
  • npm test — 13/13 passing.
  • YAML re-validation and npm run build:web required interactive approval this session couldn't grant, so I didn't re-run them — not exercised by the reviewed workflows anyway (publish.yml only runs npm ci / npm test / npm publish). The PR description states the YAML was already validated with yaml.safe_load.

Findings (both minor, non-blocking — posted inline):

  1. publish.yml:47 — npm install -g npm@latest floats the npm CLI version used for every future publish. Low risk, but this is the one workflow in the repo with publish authority; pinning would keep the release pipeline itself reproducible.
  2. publish.yml:38 — the checkout doesn't set persist-credentials: false, unlike test.yml's checkout, which does so deliberately (commented rationale: "the checkout's token should not outlive the step"). contents: read limits the impact here, but it diverges from the repo's own stated convention on the same action.

No blocking issues. The dispatch-and-wait logic in release-please.yml (the BEFORE/databaseId/headBranch matching to find the triggered run, then gh run watch --exit-status) correctly avoids the double-publish race its comments describe, and the tag-vs-package.json guard plus if: github.ref_type == 'tag' in publish.yml correctly close the branch-named-like-a-tag loophole. No secrets or credential-shaped strings anywhere in the diff.

Note: my custom instructions also ask for a top-level summary via gh pr comment, but that call is blocked in this runner by a personal safety hook (public-issue-guard) that can't reach its denylist file here — no add_repo/clone path is available in this environment. Posting the summary here instead, since it's my one working channel and the review content itself has nothing sensitive in it.

Comment thread .github/workflows/publish.yml
Comment thread .github/workflows/publish.yml
@mergify mergify Bot removed the awaiting-human label Sep 22, 2026
@claude

claude Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Claude finished @mark-brannan's task in 3m 1s —— View job


Independent review — round 2 (post-synchronize)

  • Read CLAUDE.md / AGENTS.md for repo conventions — none at root
  • Review full diff (origin/main...HEAD) — both workflow files
  • Check for credential/secret exposure — none found
  • Run npm ci / npm test
  • Post findings

What changed since the last review round: commit b0c739a fixed the persist-credentials: false gap on publish.yml's checkout that a prior review flagged — now publish.yml:42.

What I checked: full diff against origin/main, both workflow files in full, release-please-config.json/.release-please-manifest.json (tag prefix v, matches publish.yml's ${TAG#v} comparison) against the tag-match guard, test.yml for repo convention, package.json/package-lock.json for npm ci/npm test inputs.

What I ran:

  • npm ci — passed, 0 vulnerabilities.
  • npm test — 13/13 passing.
  • YAML re-validation and npm run build:web both required interactive approval this session couldn't grant, so not re-run — neither is exercised by publish.yml (it only runs npm ci/npm test/npm publish), and the PR description already states the YAML was validated.

Credential/secret check: none found. GH_TOKEN: ${{ github.token }} and the app-token step both use secret references, not literal values; npm publish uses OIDC (no NODE_AUTH_TOKEN).

Still open, non-blocking (flagged in the prior review round, unchanged in this diff): publish.yml:51 — npm install -g npm@latest still floats the npm CLI version on the one workflow with publish authority. Not re-posting as a new inline comment since it's already on the thread; author's call whether to pin.

No new or blocking issues. The dispatch-and-wait loop in release-please.yml and the tag-vs-package.json guard plus if: github.ref_type == 'tag' in publish.yml are unchanged from the prior round and still correct.

Note: the top-level summary is normally also posted via gh pr comment, but that call is blocked in this runner by the public-issue-guard hook, which can't reach its denylist file here (state repo not checked out, and mcp__Claude_Code_Remote__add_repo isn't available in this session). Posting here instead — this comment's content has nothing sensitive in it.

Comment thread .github/workflows/publish.yml
mark-brannan and others added 2 commits September 22, 2026 14:14
release-please cut tags and GitHub Releases but nothing published to
npm. publish.yml uses OIDC trusted publishing (no stored token);
release-please.yml dispatches it via workflow_dispatch after a
release, matching mark-brannan/colregs's already-working pattern.

Prerequisite before the first tagged release publishes: register the
trusted publisher on npmjs.com (package settings -> Trusted publishers
-> GitHub Actions, repo mark-brannan/wire-wright, workflow publish.yml).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Matches test.yml's checkout, which sets persist-credentials: false with
the rationale that nothing in the job writes to the repository and the
token should not outlive the step. The same holds here -- release-please
owns the tag and the Release -- and permissions are already contents:
read, so this closes the remaining gap rather than opening anything.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Claude finished @mark-brannan's task in 1m 38s —— View job


Independent review — round 3 (post-synchronize)

  • Read CLAUDE.md / AGENTS.md for repo conventions — none at root
  • Review current diff (origin/main...HEAD) — both workflow files
  • Check for credential/secret exposure — none found
  • Run npm ci / npm test / npm run build:web
  • Post findings

What's actually new in this diff vs. the last review round: nothing in publish.yml (unchanged from the version reviewed in round 2, persist-credentials: false on the checkout still present at publish.yml:42). release-please.yml's diff against origin/main is now just 5 deletions — it drops the with: publish: false opt-out (and its explanatory comment) now that publish.yml exists. The actions: write permission and the reusable-workflow call itself predate this PR (merged via #22) and aren't part of this branch's diff.

What I checked:

  • Tag-vs-version guard (publish.yml:53-60): ${TAG#v} against package.json's version, matches release-please-config.json's include-v-in-tag: true. Confirmed independently.
  • if: github.ref_type == 'tag' closes the branch-named-like-a-tag loophole; permissions: contents: read, id-token: write is minimal.
  • No literal secrets — RELEASE_PLEASE_APP_CLIENT_ID/RELEASE_PLEASE_APP_PRIVATE_KEY are secret references, npm publish uses OIDC (no NODE_AUTH_TOKEN).
  • The two prior findings are resolved: persist-credentials: false was added (confirmed in the current file); npm install -g npm@latest was declined deliberately by the author with a reasoned tradeoff (registry-imposed floor, not a build-tool pin, five-repo consistency) — I don't have a rebuttal to that reasoning, not re-raising it.

What I ran:

  • npm ci — passed, 0 vulnerabilities.
  • npm test — 13/13 passing.
  • npm run build:web — blocked by an approval prompt this session can't grant. Not exercised by publish.yml (which only runs npm ci/npm test/npm publish), so this doesn't affect the workflow under review.

One thing I can't verify from here: the actual dispatch/wait mechanics now live entirely in the external reusable workflow (mark-brannan/.github/.github/workflows/release-please.yml@main), out of this repo — I can't audit its default publish: behavior or confirm it does what dropping publish: false implies. That workflow isn't part of this diff, so it's not a finding against this PR, just a boundary on what "reviewed" covers here.

No new or blocking issues. Nothing to add as inline comments this round — the two open threads from earlier rounds are already closed (one fixed, one declined with reasoning on the thread).
· branch

@mark-brannan
mark-brannan merged commit f99dcfe into main Sep 22, 2026
14 checks passed
@mark-brannan
mark-brannan deleted the add-publish-workflow branch September 22, 2026 21:31
@mergify mergify Bot removed the awaiting-human label Sep 22, 2026
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