Repository navigation
fix(action)!: honour dry-run, Node 24, and caller tokens; refresh action.md - #752
goosewobbler wants to merge 4 commits into
Conversation
…on.md Fixes #738 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Standing release PR: #730 · v0.42.1 queued (4 packages) · open 8h 49m · ✅ ready to merge Release Preview — v0.42.0 → v0.42.1
These changes will be added to the release PR (#730) when merged: Changelogmonorepo 0.42.0 → 0.42.1
After merge — predicted release
Updated automatically by ReleaseKit |
| it('should reject dry-run for standing-pr-publish', () => { | ||
| expect(() => assertDryRunSupported('standing-pr-publish', 'true')).toThrow(/dry-run is not supported/); | ||
| }); |
There was a problem hiding this comment.
Dry-run safeguards lack test coverage
The new test calls assertDryRunSupported directly, but never checks its connection to parseInputs or the backfill fix. Add tests that reject unsupported modes through parseInputs and confirm that dry-run: true removes --apply even when apply: true. Otherwise, either safeguard can disappear while this test stays green.
There was a problem hiding this comment.
Fixed in 5dbf783: the guard is now tested through parseInputs for each rejected mode (and gate, which is read-only, is accepted), and a backfill test checks dry-run: true drops --apply even with apply: true.
| } | ||
|
|
||
| const token = process.env.NPM_TOKEN ?? process.env.NODE_AUTH_TOKEN; | ||
| const token = process.env.NPM_TOKEN || process.env.NODE_AUTH_TOKEN; |
There was a problem hiding this comment.
Empty-token fallback lacks test coverage
The existing token tests use a nonempty NPM_TOKEN, so they pass with both ?? and || and do not protect this fix. Add a regression test with an empty NPM_TOKEN and a valid NODE_AUTH_TOKEN. Check both the generated .npmrc and the returned NODE_AUTH_TOKEN so this authentication failure cannot return unnoticed.
There was a problem hiding this comment.
Fixed in 5dbf783: added a test with an empty NPM_TOKEN and a set NODE_AUTH_TOKEN, checking both the returned env and the generated .npmrc. It fails with ??.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@greptile review |
… mode Finishes the docs items #738 listed and the PR hadn't covered: - the versioning section is folded into the pinning note, which now says vX.Y.Z tags move and to pin the commit one points to; - inputs stable, scope and ssh-key, `bump: prerelease`, and the gate and standing-PR outputs are documented, as is that release outputs need json; - permissions for gate and the standing-PR modes; - the examples drop the ignored `env: GITHUB_TOKEN`, the preview trigger includes label events, and the dry-run example passes the markdown through env instead of interpolating PR text into shell; - README no longer promises rollout guidance. Gate is read-only, so dry-run is now accepted there instead of failing. Tests drive the dry-run guard through parseInputs, cover backfill dropping --apply on a dry run, and cover an empty NPM_TOKEN falling back to NODE_AUTH_TOKEN. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The action's own setup-node replaces any Node version an earlier step set up, so its default is what the publish runs on. Node 22 bundles npm 10, and npm trusted publishing needs npm 11.5.1 or later, which ships with Node 24. This repo's own release job, the examples and ci-setup already use 24. Callers can still pass node-version: '22'. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes #738
Breaking: the action fails when
dry-run: trueis set withstanding-pr-update,standing-pr-publishorrefresh-after-release, which used to ignore it and run for real.node-versiondefaults to24(was20), whose npm supports OIDC trusted publishing; passnode-version: '22'to stay on 22. Pass a PAT or App token asgithub-token;GITHUB_TOKENin the step'senvis ignored, as before.Made with Cursor


Confidence Score: 5/5
The PR appears safe to merge, with non-blocking documentation and test gaps remaining.Summary
The PR updates the action's runtime, credentials, dry-run safeguards, and documentation.
Reviews (2) · Last reviewed commit: "docs(action): clarify skip-checkout skip..." · Reviewed by Greptile