Generate portable CLI release checksums - #2973
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: found issues before merge. Reviewed August 16, 2026, 4:46 AM ET / 08:46 UTC. ClawSweeper reviewWhat this changesThe PR adds a portable checksum helper, verifies basename-only CLI checksum sidecars before upload, and adds a relocation regression test. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readinessKeep this PR open: it correctly repairs future CLI checksum generation, but it declares the report closed while the currently released v0.50.0 sidecars remain unusable. Likely related people: steipete (high confidence). Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s release workflow packages standalone CLI archives for multiple platforms and uploads checksum sidecars alongside them. The helper creates and verifies those sidecars before release assets are published. flowchart LR
A[CLI archive] --> B[Release package job]
B --> C[Checksum helper]
C --> D[Basename-only sidecar]
D --> E[Checksum verification]
E --> F[Uploaded release assets]
Decision needed
Why: The patch is proven for future release jobs, but replacing already-published release assets requires release-owner authority and must use checksums for the exact published archives. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Backfill basename-only sidecars for the exact v0.50.0 CLI archives, then merge the future-release guard and close the report; otherwise remove the closing declaration and leave the report open until a corrected release ships. Do we have a high-confidence way to reproduce the issue? Yes; current main passes an absolute temporary archive path to the checksum tool, and the linked report provides a concrete download-and-verify path for v0.50.0. Is this the best way to solve the issue? No; the helper is the narrowest future-release repair, but this PR does not remediate the already-published v0.50.0 sidecars it says are fixed. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 966bf4a78ee7. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
There was a problem hiding this comment.
Pull request overview
This PR fixes non-portable CLI release checksum sidecars by generating and validating .sha256 files from within the asset output directory, ensuring the recorded filename is the asset basename and that the digest validates before publishing. It also adds a portable regression test and wires it into the existing portable lint checks.
Changes:
- Add
Scripts/generate_release_checksum.shto generate checksum sidecars with basename-only filenames and verify them via-c. - Update
.github/workflows/release-cli.ymlto use the shared checksum generator during packaging. - Add
Scripts/test_release_checksum.shand run it as part oflint-linuxportable checks viaScripts/lint.sh.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
Scripts/test_release_checksum.sh |
Adds a regression test that moves the asset + sidecar and verifies -c still succeeds from a different directory. |
Scripts/lint.sh |
Runs the new checksum regression test as part of the portable checks suite. |
Scripts/generate_release_checksum.sh |
Implements portable checksum sidecar generation + filename/digest validation. |
.github/workflows/release-cli.yml |
Switches CLI packaging to call the new checksum generator script to avoid embedding runner-local paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
-cCloses #2971
Testing
./Scripts/test_release_checksum.sh— passes./Scripts/lint.sh lint-linux— portable checks (including the new regression test), JavaScript formatting/linting, and TypeScript checks pass; the final SwiftLint invocation is blocked because the downloaded binary requires newer glibc/libstdc++ than the b2 host providesmake check— blocked on Linux because the existing app-localization check requires macOSplutilmake test— blocked before discovery because Swift is not installed on the b2 hostReal release-workflow proof
workflow_dispatchonshockbladenull:agent/fix-cli-checksum-paths9f7d27b230e4b7d7f650d6e1ce8a7424cfeb184ebuild-climatrix jobs passed: Linux glibc x86_64/aarch64, Linux musl x86_64/aarch64, and macOS x86_64/arm64.Packagestep generated and validated its sidecar; both macOS package jobs also reported: OK.sha256sum -cfrom the downloaded artifact directory. All six passed.