publish with an explicit release credential, and prove it before building - #138
Conversation
…ding This repo was next in line for the failure OpenIPC/firmware hit on 2026-08-29. Moving the nightly off `schedule:` onto a dispatch changes the identity the run executes as, and the default GITHUB_TOKEN's release-write goes with it: a scheduled run executes as the repository's scheduling identity, a dispatched one as whoever dispatched it, and for an app/OAuth-backed identity the releases API answers 403 "Resource not accessible by integration". It does so even though the job declares `contents: write` and the run log confirms the token was granted it, which is why nothing in the workflow file looks wrong. firmware's run 33273333359 built 102 boards over two hours before finding out. This repo builds 107 devices and publishes the same way -- `gh release create` plus a PATCH of refs/tags/latest -- so it would have failed identically the first time the train reached it. It never got that far only because the train correctly refused to advance past firmware's failure. The publish step takes RELEASE_TOKEN instead of the default token, and preflight proves that credential can create a release before the matrix is allowed to spend hours on it, by creating a draft release and deleting it: invisible, no tag, two API calls. Asserting the secret is merely non-empty would not have caught the firmware incident. The token that failed existed and was simply refused, so the probe has to attempt the write.
PR Summary by QodoValidate explicit release credentials before firmware builds
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1.
|
Review caught a real defect in the credential probe. It deleted its draft with a single fail-fast call, so an interrupted or cancelled job left the draft behind -- and because github.run_id is stable across re-runs, that leftover would collide with its own re-run and report a perfectly good credential as broken. A guard that fails wrongly is worse than no guard, because it blocks good runs and teaches people to ignore it. Three changes. The probe tag now carries run_attempt as well as run_id, so a re-run can never collide with its own leftovers. Deletion moved into a trap, so a cancelled job still cleans up. And a best-effort sweep removes probe drafts stranded by earlier runs, because cleanup.yml does not know this tag pattern and they would otherwise accumulate forever. The sweep is capped at the 10 most recent releases and swallows every error. That cap is not arbitrary: listing 100 releases in these repos returns HTTP 504 because each carries hundreds of assets, measured at ~2s for 10 and ~7s for 30. Housekeeping must never be the thing that fails a build. Verified end to end against the live repo with a real token: creates the draft, reports "release credential OK", the trap removes it, no tag is created and no published release is touched. Both failure paths still fail loudly -- unset token, and a credential the API refuses.
Review caught that the probe treated any one-shot POST failure as a bad
credential. GitHub answers 403 for secondary rate limiting as well as
for refusal -- the publish step retries six times for exactly that
reason -- and these repos return 504 on ordinary release listings often
enough that one was hit while writing this change. A blip would have
failed the nightly with "the token is wrong", sending whoever read it
off to reissue a credential that was never the problem.
The POST now retries three times with 5s and 10s backoff, and the error
says "after 3 attempts ... if this is not a transient API error". Three
rather than the publish step`s six because the value here is failing
fast: a genuine misconfiguration still surfaces in well under a minute
instead of after the whole matrix.
Testing the retry then exposed a second defect, in the sweep added by
the previous commit. gh writes its error body to STDOUT, so a failed
list feeds the cleanup loop lines of JSON instead of release ids, and
the log filled with `removing stranded probe draft {"status": "401"}`.
The deletes were harmless -- they simply failed -- but a step whose
entire purpose is to be believed must not narrate confident nonsense.
Only numeric ids are acted on now.
Verified against the live repo: happy path creates the draft, reports
OK, the trap removes it, nothing is left behind; a refused credential
retries three times and fails with the honest message; a failing list
produces zero bogus removal lines.
|
Both findings addressed. The transient-failure one was right, and sharper than it looks. 1. Probe misclassifies transient failures — fixed in
|
Companion to OpenIPC/firmware#2339. This repo was next in line for the same failure and hasn't hit it yet only because the release train correctly refused to advance past firmware's failure.
The failure being pre-empted
Moving the nightly off
schedule:onto a dispatch changes the identity the run executes as, and the defaultGITHUB_TOKEN's release-write goes with it. For an app/OAuth-backed identity the releases API answers:even though the job declares
contents: writeand the run log confirms the token was granted it — which is why nothing in the workflow file looks wrong. OpenIPC/firmware's run 33273333359 built 102 boards over two hours before finding out;latestnever moved and cameras stopped updating.This repo publishes exactly the same way —
gh release createplus a PATCH ofrefs/tags/latest— so it would have failed identically, after building all 107 devices.Changes
1.
publishusessecrets.RELEASE_TOKEN(Contents: read and write). Also makes publishing independent of who or what starts the run.2.
preflightproves the credential first — draft release created and deleted; invisible, no tag, two API calls. Checking the secret is merely non-empty would not have caught the firmware incident: that token existed and was simply refused, so the probe attempts the write.Skipped on
pull_request, which neither publishes nor has the secret.Before merging
RELEASE_TOKENmust exist as a repo secret here, or the next non-PR run fails fast in preflight — intended, but red rather than silently unpublished.Verification
.github/scripts/ci-matrix.py --self-testpasses (111 devices, 15 smoke, 39 cases).::error::and exit 1. Neither passes silently.