Repository navigation
test(ci): derive the release-verify fixture's signed set from the updater table - #5425
Conversation
…ater table dev went red at the union of #5405 and #5391: lane F made the deb a second Linux updater target, so the verifier's derived expected set gained OpenCodex-<version>-linux-amd64.deb.sig, while the test's hand-written oracle still described the earlier world where only the AppImage was signed. Each branch was green alone; the merge was not. The fix is derivation, not list-keeping. The signed set and the manifest platform list in the fixture now come straight from platformFiles — the table that decides which bundles carry the updater key — and the produced payload list comes from the shared standalone target module and the bundle table. A future updater target changes both sides of the assertion by itself. The derivation test keeps its concrete payload anchors (a renamed or dropped bundle should still fail for a human to review) and asserts the rule instead of the roster: a bundle's signature is expected exactly when the updater table names it. Only the two test oracles changed; the verification ordering (checksums, signatures and the manifest all precede publication) is untouched.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe release workflow tests now derive expected archives, signatures, and manifest platforms from producer tables instead of fixed fixtures. The test setup imports the tables and helpers used to build these expectations. ChangesRelease asset test contracts
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![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: 5164db952b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `OpenCodex-${VERSION}-windows-x64.msi`, | ||
| `OpenCodex-${VERSION}-linux-x86_64.AppImage`, | ||
| `OpenCodex-${VERSION}-linux-amd64.deb`, | ||
| ...standaloneTargets.map(target => standaloneArchiveName(VERSION, target)), |
There was a problem hiding this comment.
The package-standalone archive step in .github/workflows/release.yml:150-169 constructs .zip versus .tar.gz names independently from $RUNNER_OS; it does not call standaloneArchiveName. Using that helper here means both the fixture and expectedReleaseAssets now obtain standalone names from the same code, so an incorrect helper change—especially one affecting the currently unanchored Darwin x64 or Linux arm64 target—will leave this test green while the release workflow produces different files and verification fails after packaging. Keep an independent assertion against the workflow's naming convention, or make the workflow itself consume the shared helper before deriving the fixture from it.
AGENTS.md reference: AGENTS.md:L284-L287
Useful? React with 👍 / 👎.
1 participant
Summary
dev is red at the union of #5405 and #5391. Lane F made the deb a second Linux updater target (
linux-x86_64-deb), so the pre-publication verifier's derived expected set correctly gainedOpenCodex-<version>-linux-amd64.deb.sig— while the test's hand-written oracle still described the earlier world where only the AppImage was signed. Each branch was green alone; the merge was not. This is the union defect class AGENTS.md warns about: two branches each restated a set, and the merged truth outgrew one of the copies.The fix is derivation, not list-keeping:
signedset now comes straight fromplatformFiles— the table that decides which bundles carry the updater key — and its produced-payload list comes from the shared standalone target module and the bundle table. A future updater target or bundle changes both sides of the assertion by itself..sigis expected exactly when the updater table names it.platformFileskeys rather than a copied list — it would have failed next for the same reason.Only the two test oracles changed. The verification ordering from #5405 (checksums, signatures and the manifest all verified before publication) is untouched, and no production code changes.
Security review: this PR touches the release-verification test surface only; the release automation itself is unchanged. No permissions, secrets, or publication ordering are modified. Noted explicitly because the file sits beside release automation that requires it.
Verification
Local execution checks: NOT RUN (lane policy — no local
bun test,bun run test,bun run test:changed,bun run typecheck, builds, installs, orocxexecution; hosted CI at the exact head SHA is the gate and is reported separately).Static verification performed instead:
+ "OpenCodex-2.61.0-linux-amd64.deb.sig"in the verifier's derived set against the oracle — the oracle now derives that entry fromplatformFiles, which contains"linux-x86_64-deb": "linux-amd64.deb"since feat(desktop): dual Linux updater targets and the installed-artifact release gate #5391.expectedReleaseAssetswithrequireSignaturesrequires the same set from the same table; the fixture writes every signature the verifier will demand, so the full-flow test passes the expected-set, checksum, signature, manifest parse-back, and receipt stages. The manifest platform assertion compares againstObject.keys(platformFiles).sort(), which is whatparseBackManifestenforces — the five current platforms includinglinux-x86_64-deb.platformFileswould dropdeb.sigfrom both sides consistently, and a future sixth updater platform extends both sides together; conversely a verifier that stopped requiring a signature the table names would fail the rule assertion in the derivation test.git merge-tree --write-tree origin/dev HEADclean.Checklist
Summary by CodeRabbit