ci(build-stable): fix release-check manifest source#83
Conversation
…check check-release-needed.sh ran after 'Prepare stable branch' switched the working tree to upstream's stable/<version> branch, which does not contain BRIGHTFIRE_PATCHES.md. The script was reading from the filesystem ($PATCHES_FILE) and failing with 'Manifest not found'. Fix: read the manifest directly from origin/brightfire/ci via git show. BRIGHTFIRE_PATCHES.md only ever lives on brightfire/ci anyway. The release-check step no longer depends on working-tree contents. Also pipes git-show through sha256sum (instead of capturing into a shell var) so the trailing newline is preserved byte-for-byte. Verified locally that the resulting sha matches 'sha256sum BRIGHTFIRE_PATCHES.md' on the working-tree copy. Failure: https://github.com/brightfire/openclaw/actions/runs/26901255445/job/79354008660
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f1e81da0a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| set -euo pipefail | ||
|
|
||
| PATCHES_FILE="${PATCHES_FILE:-BRIGHTFIRE_PATCHES.md}" | ||
| MANIFEST_REF="${MANIFEST_REF:-origin/brightfire/ci}" |
There was a problem hiding this comment.
In the bf-build-stable workflow, the manifest that drives the build is copied and parsed near the start (.github/workflows/bf-build-stable.yml:70-86), but the workflow later refreshes origin/brightfire/* before this script runs (.github/workflows/bf-build-stable.yml:112). If brightfire/ci advances during the long build/test phase, this moving default reads the newer manifest instead of the manifest that produced the current stable tree, so the release gate can skip or write a fingerprint for patches that were not actually built. Pass a pinned ref/blob or the staged manifest hash from the parse step instead of rereading origin/brightfire/ci.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 9849462.
The race is real even with concurrency: cancel-in-progress: cancel signals aren't instant, and the workflow has an intervening git fetch origin '+refs/heads/brightfire/*:refs/remotes/origin/brightfire/*' step that advances origin/brightfire/ci mid-run. Worse, since the fingerprint writer also reads from the (re-read) manifest_sha256 output, a mid-build push would have made us publish a fingerprint claiming to represent a manifest the build never actually saw.
Fix: the "Fetch patch manifest" step now also stages a frozen copy to /tmp/BRIGHTFIRE_PATCHES.pinned.md and records its sha256 to GITHUB_ENV. check-release-needed.sh accepts PINNED_MANIFEST_PATH / PINNED_MANIFEST_SHA256 and prefers them; it falls back to the MANIFEST_REF git read (with a warning) for local invocations. Same sha now flows into both the gate comparison and the published fingerprint asset.
Verified all three branches manually (pinned + pre-sha, pinned + auto-sha, missing-pinned fallback). +70/-23 across the workflow and the script.
Codex flagged a real race: the workflow stages BRIGHTFIRE_PATCHES.md from origin/brightfire/ci at the top, but check-release-needed.sh re-reads the same moving ref ~30+ min later, after the build/test phase. The intervening 'Fetch all brightfire branches' step (and any concurrent push to brightfire/ci that lands before this run's cancel signal does) advances origin/brightfire/ci, so the gate compares AND fingerprints against a manifest that did NOT drive this build. Fix: stage a frozen copy of the manifest to /tmp at the top of the workflow, record its sha256 to GITHUB_ENV, and pipe both into check-release-needed.sh as PINNED_MANIFEST_PATH / PINNED_MANIFEST_SHA256. The script prefers the pinned source when present, with a documented git-ref fallback for local testing and a warning emitted if the pinned path is set but unreadable. Verified the three branches manually: pinned path + pre-computed sha, pinned path + auto-computed sha, and pinned path missing (warns + falls back to git ref). All emit consistent manifest_sha256 to GITHUB_OUTPUT. Net: 2 files, +70/-23.
1 participant
PR #80 broke build-stable:
check-release-needed.shran afterPrepare stable branchswitched the working tree to upstream'sstable/<version>branch (which does not contain BRIGHTFIRE_PATCHES.md). The script was reading from the filesystem and failing withManifest not found.Failing run: https://github.com/brightfire/openclaw/actions/runs/26901255445/job/79354008660
Fix (round 1: f24c…)
Read the manifest directly from
origin/brightfire/civiagit showinstead of the filesystem. BRIGHTFIRE_PATCHES.md only ever lives onbrightfire/cianyway. The release-check step no longer depends on working-tree contents.The script's
MANIFEST_REFenv var defaults toorigin/brightfire/ci; overridable for local testing.Fix (round 2: 9849…) — pin manifest source
Codex flagged a real race in round 1 (review):
origin/brightfire/ciis a moving ref. Between the manifest staging at the top of the workflow andcheck-release-needed.sh~30+ min later (after build+test), the interveninggit fetch origin '+refs/heads/brightfire/*:...'step advancesorigin/brightfire/ciif anything pushed to it during the build window.concurrency: cancel-in-progressdoesn't fully save us — cancels aren't instant, and the failure mode is silent: the gate AND the published fingerprint would both reflect a manifest that did not drive this build, hiding new patches behind a stale tag.Fix: stage a frozen copy to
/tmp/BRIGHTFIRE_PATCHES.pinned.md+ record its sha256 toGITHUB_ENVat the top of the workflow.check-release-needed.shacceptsPINNED_MANIFEST_PATH/PINNED_MANIFEST_SHA256and prefers them. Falls back toMANIFEST_REFgit read for local invocations (with a warning if the pinned path was set but unreadable).Sha consistency
Both fixes preserve byte-identical sha against
sha256sum BRIGHTFIRE_PATCHES.mdtaken inwrite-build-fingerprint.sh.git showis piped throughsha256sumdirectly (no$(...)newline stripping); pinned path usessha256sumon the staged file.Net diff
2 files, +70/-23.