ci(npm): a GitHub Release can no longer publish to npm on its own - #44
Merged
Merged
Conversation
Publishing the v3.8.54 Release started this workflow through its own
`release: [released]` trigger and went for `npm publish` of `omniroute` — a
package this fork does not own; upstream publishes it. The run was cancelled by
hand with the publish job already queued. Nothing reached the registry
(`npm view omniroute@3.8.54` → 404), but only because someone was watching.
Both brakes that should have held were pointing the wrong way:
1. The resolve step skips only when the version is ALREADY on npm. This fork
runs ahead of upstream (3.8.54 here, 3.8.50 there), so every release resolves
skip=false and proceeds. For a fork that brake is backwards.
2. electron-release.yml has carried `vars.ENABLE_NPM_PUBLISH == 'true'` since
v3.8.51, with a comment describing this exact risk — but it guards the
`workflow_call` path INTO this workflow. A Release published by hand enters
through `release:` and never meets it. The guard was on the caller instead of
on the thing being guarded.
So the gate moves here, as a `gate` job that every publishing job needs and
honours: publish, stage-npm and both opencode-plugin jobs. A release publishes
to npm only when the repository variable ENABLE_NPM_PUBLISH is "true".
`workflow_dispatch` is deliberately left ungated — it is already a deliberate
human action, and it is the emergency route if the variable is ever wrong.
`workflow_call` is left alone too, because its only caller already gates it.
tests/unit/npm-publish-release-gate.test.ts fails if a publishing job stops
depending on the gate, or if a new one is added without it — the way the hole
appeared the first time. It also asserts a `needs:` is never mistaken for a
guard: a `needs:` only orders a job, it does not stop it, so the test requires
the `if:` as well.
Verification (isolated DATA_DIR/HOME/USERPROFILE/APPDATA):
red-first, against v3.8.54's workflow: 2 of 3 fail
✖ the gate job exists and decides on the ENABLE_NPM_PUBLISH repository variable
✖ every publishing job depends on the gate and honours its answer
after the fix: 3 of 3 pass
workflow suites (4 files): 18 pass / 0 fail
check:workflows exit 0
eslint --max-warnings=0 exit 0
tsc -p tsconfig.typecheck-core.json exit 0
mutation coverage: the test imports no mutated module, so no tap.testFiles
entry is required (verified with testImportsModule against all 31 entries)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Item 6 read as though `BRANCH_LOCK_TOKEN` were locking release branches and only `main` were exposed. The secret is not set, so `lock-released-branch` fails on every release and no release branch is locked automatically. v3.8.54 shipped with its branch still writable; it was locked by hand afterwards. The root cause is owner-only: `GITHUB_TOKEN` cannot be granted the `Administration` scope, so the lock needs a PAT or fine-grained token stored as `BRANCH_LOCK_TOKEN`. Until that exists, the manual `gh api` call is the procedure, so it is written down instead of rediscovered — including the confirmation step, because a silent failure here is exactly what caused the v3.8.3 incident the workflow was built to prevent. Verification (isolated env): check-doc-links exit 0 check-fabricated-docs exit 0 check-docs-frontmatter exit 0 check-docs-sync exit 0 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 19, 2026
LMPrado-DZ23
added a commit
that referenced
this pull request
Sep 20, 2026
`github.event_name` inside a reusable workflow is the CALLER's event. The
gate read:
if [ "$EVENT_NAME" = "workflow_dispatch" ] || [ "$ENABLED" = "true" ]
So dispatching electron-release.yml — or any future caller — arrived here
as `workflow_dispatch` and took the "a human asked for this" exemption,
reaching a provenance-signed `npm publish` of `omniroute`, a package name
upstream owns, with NPM_TOKEN in scope and ENABLE_NPM_PUBLISH never set.
Nothing was reachable in practice: electron-release.yml's calling job
carries `vars.ENABLE_NPM_PUBLISH == 'true'` in its own `if:`. That is
exactly the problem. The whole point of the gate job is to be the last
line, and a last line whose correctness rests on an `if:` in another
file is not one. #44 exists because this fork already published upstream's
package once by accident.
`github.job_workflow_ref` is set ONLY when this file runs as a reusable
workflow, so it is what separates "a human dispatched THIS" from "a human
dispatched something that calls this". The exemption now requires it to
be empty.
The refusal also says which case it refused. A message that only says
"not authorised for a workflow_dispatch trigger" when the event WAS a
dispatch reads like a bug, and the next person removes the check.
6/6 tests/unit/npm-publish-release-gate.test.ts
the existing case for the old shape is rewritten, not deleted: it now
requires the dispatch arm to be paired with the reusable-call test
a new case fails on an unqualified `workflow_dispatch` test
YAML parses; prettier clean
Co-authored-by: zodyp <zodyprado@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Publishing the v3.8.54 Release started
npm-publish.ymlthrough its ownrelease: [released]trigger and went fornpm publishofomniroute— a package this fork does not own; upstream publishes it. The run was cancelled by hand with the publish job already queued. Nothing reached the registry (npm view omniroute@3.8.54→404), but only because someone happened to be watching the board.Why nothing stopped it
Two brakes existed. Both were pointing the wrong way.
1. The skip-if-already-published check. It skips only when the version is already on npm. This fork runs ahead of upstream — 3.8.54 here, 3.8.50 there — so every release resolves
skip=falseand proceeds. For a fork, that brake is backwards: it is loosest exactly when the risk is highest.2.
electron-release.yml'svars.ENABLE_NPM_PUBLISH == 'true'guard. It is correct, and its comment describes this precise risk. But it guards theworkflow_callpath into this workflow. A Release published by hand — or by any other workflow — enters throughrelease:and never meets it. The guard was on the caller instead of on the thing being guarded.The change
A
gatejob that decides once, and that every publishing jobneeds:and honours in itsif:—publish,stage-npm,publish-opencode-pluginandpublish-opencode-plugin-v2. A release publishes to npm only when repository variableENABLE_NPM_PUBLISHistrue.Deliberately left ungated:
workflow_dispatch— already an explicit human action, and the emergency route if the variable is ever wrong.workflow_call— its only caller already gates it, and that guard is greppable right next to this one.The regression test
tests/unit/npm-publish-release-gate.test.tsfails if a publishing job stops depending on the gate, or if a new one is added without it — which is exactly how the hole appeared the first time. It also refuses to accept a bareneeds:as a guard:needs:only orders a job, it does not stop it, so the test requires theif:too.Verification
Run with isolated
DATA_DIR/HOME/USERPROFILE/APPDATA.Red-first, against the workflow as it stood at
v3.8.54:After the fix: 3 / 3 pass.
check:workflowseslint --max-warnings=0tsc -p tsconfig.typecheck-core.jsontap.testFilesentry needed — the test imports no mutated module, verified withtestImportsModuleagainst all 31 entries🤖 Generated with Claude Code