Repository navigation
release.yml: correct the stage-2FA rationale and report skipped packages honestly in the Summary #368
Description
Two Minor findings raised on the final head of #365 (3985835a). Both are text-only,
both live in .github/workflows/release.yml, and neither blocks the merge — but both
mislead an operator who is reading the workflow during a partial-stage recovery, which
is exactly when it matters. Deliberately deferred so a fifth round would not dismiss two
fresh approvals; the publish path cannot execute until the six Trusted Publisher
registrations exist, so this lands before the workflow ever runs for real.
1. The 2FA justification is wrong for list / view (found by tps-sherlock)
The loop comment says the stage subcommands "(list/view/reject/approve) need interactive
proof-of-presence". Per npm-stage(1), list and view require no 2FA — only
approve and reject do.
The conclusion is still correct (CI cannot inspect the staging area) but the stated reason
is not. The real reason is the separate rule that short-lived tokens issued through a trust
relationship cannot run npm stage subcommands at all, regardless of 2FA.
Worth fixing because an operator who believes list needs proof-of-presence may not try it
from a machine where it would have worked.
2. The Summary overstates what happened when packages are skipped (found by CodeRabbit)
When npm stage publish fails and the npm view "$nm@$VERSION" branch catches it, the loop
skips that package as already published. The job Summary still says "Six packages staged"
and "Nothing is live yet".
Both statements are false on that path: fewer than six were staged, and the skipped ones are
already live. Track skipped packages separately from newly staged ones and report the two
counts distinctly.
Fix
One commit, .github/workflows/release.yml only:
- reword the CI-cannot-inspect comment to cite the trusted-token restriction, not 2FA
- accumulate
STAGEDandSKIPPEDlists in the loop, and have the Summary print both,
with "Nothing is live yet" emitted only whenSKIPPEDis empty
Refs #365
Activity
tps-flint commentedon Sep 17, 2026
3. The failure text asserts the wrong cause — observed on a real run
First real run of the OIDC path: run 35229055942,
tag
v0.6.0on1cec216e. Fired deliberately before the Trusted Publisher registrations exist,to exercise everything up to the npm boundary.
Everything behaved: preflight, smoke, and all four binary builds green;
publish-packagesfailed;github-releaseskipped; verified by status code that no release exists (404) and none of thesix packages are on npm. The containment design is correct.
But the failure text is wrong about why it failed. npm said:
and the workflow answered:
It was not already staged. Nothing had ever been staged. The error path assumes a single cause —
stage collision — and states it as the likely one, so the whole recovery block sends an operator to
npm stage listto hunt for a pending stage that does not exist. The actual fix is unrelated: registerthe trusted publisher, or fix the OIDC claim.
This is the same defect class as #2 in this issue: the message is confident about a state it never checked.
Fix: branch on the npm error code before printing a cause.
ENEEDAUTH/E401/E403→ OIDC did not authenticate. Point at the Trusted Publisherregistration for this exact package, the
id-token: writepermission, and the npm version floor —not at the staging area.
E409/ already-exists → the existing already-staged recovery block, which is right for that case.The general rule: a remedy is only useful if the diagnosis is right, and a confidently wrong diagnosis
costs more than no diagnosis. Worth stating the unknown case as unknown.
tps-flint commentedon Sep 17, 2026
Closed by #369 (merged
55b6bce8). Five rounds, and worth recording what they were, because thecode was correct at round one and every round was about what the failure path TELLS AN OPERATOR.
list/view; the Summaryclaiming "six staged / nothing live" on a run that skipped packages; and the failure text
asserting ALREADY STAGED for every failure when the first real run was
ENEEDAUTHand nothinghad ever been staged.
stage — Trusted Publisher config is per-package — and exited without saying so; and the promote
guidance was hidden in the one run where promote is the only remaining step.
unclassified, disclosednothing after a partial stage. Fixed structurally:
disclose_pending()called from all threeterminal branches, so a fourth branch added later cannot forget it.
stages but withheld the commands, because
stage_recoverywas still branch-specific — adirective with no way to carry it out. Also caught a sentence the R3 refactor had silently
falsified: "The staging recovery block deliberately does not run", sitting directly beneath a
disclosure telling the operator to clear stages.
npm stage list @tpsdev-ai/clifilters to one package. We stage six, soour own recovery procedure would have shown at most one stage and hidden the other five —
possibly the blocking one. Verified against npm 11.19.0's own usage (
npm stage list [<package-spec>], "List all staged package versions") before acting on it.The generalisable lesson is in
~/ops/CONTROL-DESIGN-CHECKLIST.md: in a failure handler, separatethe DIAGNOSIS (legitimately withheld when unestablished) from the STATE and the PROCEDURE (never
withheld — both are true regardless of cause). The catch-all branch is where this leaks, because
it is the branch no test can enumerate by definition.