fix(release): verify all crate tarballs before the first upload - #5893
Conversation
Cargo publication of v0.9.12 stopped at codewhale-tui after 18 crates: its 16 include_str! paths reached outside the package root, so the verified tarball could not compile. Move the existing plugin into the TUI package and update every source, test, CI, documentation, and ignore reference. Keep one canonical bundle, with all 16 embedded runtime files byte-identical to the v0.9.12 release payload. Validation: - Original cargo publish --locked -p codewhale-tui: FAIL, 16 missing-file errors; Cargo refused the upload. - Corrected cargo package --locked --allow-dirty -p codewhale-tui: PASS, including compilation of the unpacked library and binary. - npm test in crates/tui/plugins/computer-use: 31 passed; 0 failed. - Embedded payload comparison: 16/16 original files identical in source and in the verified Cargo tarball. - cargo fmt --all -- --check: PASS. - Coordination npm test && npm run check:web gate: unavailable here; this Rust repository defines neither root script (both report Missing script). No broad-suite or hosted-CI pass is claimed. This is a local packaging repair based on dcd4c20. It does not move the published v0.9.12 tag or alter the already-published release binaries.
Fixes #5892. Cargo packages the release together through its temporary registry so unpublished workspace dependencies can be verified before upload. Preserve tag/assets guards and resumable publication. Validation: full 21-crate dry-run passed; 3/3 real-Cargo regression tests passed, and both broken-package cases reject the previous implementation; publication-order suite and bash syntax passed. Repository npm test/check:web scripts are unavailable; no full workspace test claim. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
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. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1f5d07a0-68c9-4d3d-9f5a-954c3ae7a649) |
There was a problem hiding this comment.
🟢 Approval recommended
The release gate change is narrowly scoped and backed by a new offline regression test, with only a minor test robustness nit noted.
Pull request overview
This PR strengthens the crates.io release gate by verifying all workspace crate tarballs up front (including crates with unpublished workspace dependencies) so missing packaged assets or tarball build failures stop the release before any upload.
Changes:
- Run a single
cargo packageacross all release crates before starting the per-crate publish loop; dry-run exits immediately after verification. - Remove the previous special-casing that only listed files (or skipped true dry-run) for crates with workspace dependencies.
- Add an offline regression test that exercises the “no upload before tarballs verify” boundary and wire it into CI.
File summaries
| File | Description |
|---|---|
| scripts/release/publish-crates.sh | Packages/verifies all release crates in one Cargo invocation before any publish; dry-run now validates dependent tarballs too. |
| scripts/release/publish-crates.test.py | New offline test fixture that uses real Cargo packaging to ensure missing embedded assets fail before any upload is attempted. |
| .github/workflows/ci.yml | Runs the new Python release-helper contract test in CI. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| self.env = { | ||
| **os.environ, | ||
| "TEST_REAL_CARGO": shutil.which("cargo"), | ||
| "TEST_UPLOADS": str(self.uploads), | ||
| "PATH": str(bin_dir) + os.pathsep + os.environ["PATH"], | ||
| "CARGO_NET_OFFLINE": "true", | ||
| "CARGO_TARGET_DIR": str(self.root / "target"), | ||
| } |
![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: faaf378c75
ℹ️ 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".
| return 1 | ||
| } | ||
| echo "Verifying all ${#packages[@]} release package tarballs before any upload..." | ||
| cargo package "${package_args[@]}" |
There was a problem hiding this comment.
When a release operator uses Cargo 1.88 or 1.89, which are compatible with the workspace's declared rust-version = "1.88", this multi-package command does not provide unpublished packages through a temporary registry: the new regression test fails with no matching package named codewhale-preflight-base found before verifying the dependent tarball. I reproduced all three failures with 1.88/1.89 and verified they pass starting with 1.90; either retain a compatible preflight or explicitly declare and enforce Cargo >=1.90 before invoking this command.
Useful? React with 👍 / 👎.
| # Package the complete release together. Cargo resolves unpublished workspace | ||
| # dependencies through a temporary local registry, then builds each unpacked | ||
| # tarball. A file inventory alone cannot detect missing embedded assets. |
There was a problem hiding this comment.
After this change, docs/RELEASE_RUNBOOK.md:102-107 still tells release operators that dependency-free crates receive cargo publish --dry-run while crates with workspace dependencies receive only a packaging preflight. The implementation now verifies every unpacked tarball in one invocation, so the authoritative runbook referenced by the release checklist describes the opposite behavior and gives operators inaccurate expectations when interpreting or troubleshooting this gate.
Useful? React with 👍 / 👎.
…913-20260905 Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_702aaa26-f818-44d3-90ab-bdf1edb9f97b) |
|
Claude finished @Hmbown's task in 2m 31s —— View job Review complete ✅
SummaryThe actual root cause — the The new Findings
No issues found with the provider/model routing invariant (EPIC #2608) — this PR doesn't touch model/provider selection code. |
![codewhale-agent[bot]](https://avatars.githubusercontent.com/u/101357273?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codewhale review
The PR relocates the computer-use plugin into crates/tui and rewrites release publishing to run one cargo package verification across all release crates before any upload, replacing per-crate dry-run logic. It adds offline fixture tests that validate the tarball upload boundary.
Findings
- [WARNING] Publish preflight loses crates.io-specific dry-run for dependency-free crates (
scripts/release/publish-crates.sh:55)
Removing cargo publish --dry-run for crates without workspace dependencies means package-level cargo package verifies tarball contents and compilation, but it does not perform all crates.io validation that cargo publish --dry-run does (such as registry metadata checks or publish-time rules). A package could pass the new preflight and then fail when its turn arrives after earlier crates have already uploaded. - [WARNING] Preflight verifies already-published crates and can block resuming a partial release (
scripts/release/publish-crates.sh:55)
In publish mode, the full cargo package pass runs before the per-package crate_version_exists skip. If a previous partial release left a crate already on crates.io whose source no longer packages cleanly, the preflight fails and prevents the remaining unpublished crates from being uploaded, breaking the resumable publication loop. - [INFO] New offline tests do not exercise the previous bad-publisher regression or the resumable skip path (
scripts/release/publish-crates.test.py)
The Python test's stub cargo has a publish --dry-run branch, but the new script never invokes that mode, so that branch is unreachable in the test. The tests also do not run the old per-crate publisher or test that already-published crates are skipped before preflight/upload, leaving the regression and resumability behavior unverified.
Assessment
The approach is sound and directly addresses the missing-asset failure by making tarball verification the gate before uploads. However, it drops some crates.io dry-run coverage and may impair resumability by packaging already-published crates; tests should be extended to cover those scenarios before relying on this for a release.
Advisory review by Codewhale (codewhale review --pr 5893 --post, head cf62b8eb7aa21c65a1b3a75be6a24318534e8fa2). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| for package in "${packages[@]}"; do | ||
| package_args+=(-p "${package}") | ||
| done | ||
|
|
There was a problem hiding this comment.
[WARNING] Publish preflight loses crates.io-specific dry-run for dependency-free crates
Removing cargo publish --dry-run for crates without workspace dependencies means package-level cargo package verifies tarball contents and compilation, but it does not perform all crates.io validation that cargo publish --dry-run does (such as registry metadata checks or publish-time rules). A package could pass the new preflight and then fail when its turn arrives after earlier crates have already uploaded.
| for package in "${packages[@]}"; do | ||
| package_args+=(-p "${package}") | ||
| done | ||
|
|
There was a problem hiding this comment.
[WARNING] Preflight verifies already-published crates and can block resuming a partial release
In publish mode, the full cargo package pass runs before the per-package crate_version_exists skip. If a previous partial release left a crate already on crates.io whose source no longer packages cleanly, the preflight fails and prevents the remaining unpublished crates from being uploaded, breaking the resumable publication loop.
Use one Cargo publication dry run for all 21 crates before uploading, with an explicit Cargo 1.90+ release-tool requirement and crates.io destination. Update the runbook and test resumability without weakening full artifact verification. Validation: 5/5 offline tarball fixtures passed; real 21-crate publication dry run passed with no upload; real two-crate unpublished dependency dry run passed; publication order, bash syntax, and git diff --check passed. Runtime root has no npm test/check:web scripts. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_25f9cc2e-b284-4013-acb8-45ef0981fb90) |
|
Claude finished @Hmbown's task in 3m 36s —— View job Review complete ✅
Note on scopeHEAD is now
Remaining findings
No correctness bugs, no provider/model routing concerns (this PR doesn't touch model/provider selection code), and the Cargo-version regex/arithmetic gate at |
|
Review follow-up is pushed at daea481. Release tooling now explicitly requires Cargo 1.90+, checks that before metadata or uploads, and documents the runtime MSRV separately. The complete gate uses Cargo publication dry run across all 21 crates, retaining publication metadata checks and explicitly targeting crates.io. The runbook matches the implementation. The real 21-crate dry run passed with no upload. Five offline tarball fixtures pass, including old-toolchain rejection and resuming after published versions. Full artifact verification is intentionally retained during resume; already-published source is not exempted from the release gate. Missing Cargo now has a clear test assertion. |
There was a problem hiding this comment.
Codewhale review
PR adds a complete cargo publish dry-run preflight for all release crates and moves the computer-use plugin under crates/tui so its assets are packaged. The release-script change is sound, but the new test fixture substitutes cargo package for cargo publish --dry-run and may miss publish-specific behavior, and the move may package dev-only plugin files without explicit exclusions.
Findings
- [WARNING] Test fixture forwards publish-only flags to cargo package (
scripts/release/publish-crates.test.py:71)
In the fake cargo shim,cargo publish --dry-runis translated to"$TEST_REAL_CARGO" package "${args[@]}"after removing only--dry-run. The remaining args include--registry crates-io, which is a publish/registry option and is not guaranteed to be accepted bycargo packageon all Cargo 1.90+ toolchains. This can make the offline fixtures fail for the wrong reason or hide differences from the real multi-packagecargo publish --dry-runpath the release script uses. - [WARNING] Moved plugin files may now be packaged in the crates/tui tarball
Movingplugins/computer-useundercrates/tui/plugins/computer-useputs the entire bundle under thecrates/tuipackage root. Unlesscrates/tui/Cargo.tomlalready excludes them (not shown in this diff), Cargo will now include development-only files such aspackage.json,tests/,scripts/smoke.mjs, andREADME.mdin the publishedcrates/tuicrate. The embedded runtime tree deliberately omits these files; the published tarball should exclude them explicitly. - [INFO] No successful publish path coverage in the new fixture (
scripts/release/publish-crates.test.py:72)
The fake cargo shim aborts every real (non-dry-run)cargo publishwithexit 98, and all successful scenarios either exit after dry-run or skip already-published versions. There is no fixture asserting that, after preflight passes and a version is absent, publish mode actually invokescargo publish --locked --registry crates-ioin order and then waits. The realcargo publish --dry-runpath is also never executed; onlycargo packageis used.
Assessment
The release preflight approach is a meaningful improvement and should prevent the 0.9.12 tarball-asset failure. Before merge, the fixture should avoid forwarding unsupported publish-only flags to cargo package and the crate packaging implications of the plugin move should be verified with explicit exclusions if needed.
Advisory review by Codewhale (codewhale review --pr 5893 --post, head daea4810eaaf5c55533fa280cb6c5b7aab786efe). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| ' shift\n if [[ " $* " == *" --dry-run "* ]]; then\n' | ||
| ' args=()\n for arg in "$@"; do\n' | ||
| ' [[ "$arg" == --dry-run ]] || args+=("$arg")\n done\n' | ||
| ' exec "$TEST_REAL_CARGO" package "${args[@]}"\n fi\n' |
There was a problem hiding this comment.
[WARNING] Test fixture forwards publish-only flags to cargo package
In the fake cargo shim, cargo publish --dry-run is translated to "$TEST_REAL_CARGO" package "${args[@]}" after removing only --dry-run. The remaining args include --registry crates-io, which is a publish/registry option and is not guaranteed to be accepted by cargo package on all Cargo 1.90+ toolchains. This can make the offline fixtures fail for the wrong reason or hide differences from the real multi-package cargo publish --dry-run path the release script uses.
| ' args=()\n for arg in "$@"; do\n' | ||
| ' [[ "$arg" == --dry-run ]] || args+=("$arg")\n done\n' | ||
| ' exec "$TEST_REAL_CARGO" package "${args[@]}"\n fi\n' | ||
| ' echo attempted >> "$TEST_UPLOADS"\n exit 98\nfi\n' |
There was a problem hiding this comment.
[INFO] No successful publish path coverage in the new fixture
The fake cargo shim aborts every real (non-dry-run) cargo publish with exit 98, and all successful scenarios either exit after dry-run or skip already-published versions. There is no fixture asserting that, after preflight passes and a version is absent, publish mode actually invokes cargo publish --locked --registry crates-io in order and then waits. The real cargo publish --dry-run path is also never executed; only cargo package is used.
The 0.9.12 publish sequence reached crates.io before discovering that the dependent TUI tarball omitted embedded assets. Verify the complete release before its first upload so a workspace build cannot hide a broken published crate.
Both modes now run one Cargo publication dry run across all 21 release crates, resolving unpublished dependencies through Cargo’s temporary local registry and building every unpacked tarball. The script requires Cargo 1.90+ for this release-tool capability, explicitly targets crates.io, and documents the separate runtime MSRV. Publish mode retains the release checkout/assets guards and skips already-published versions after full verification.
Validation:
--dry-run.npm testorcheck:webscripts.Fixes #5892.
Note
Medium Risk
Changes the crates.io release gate and toolchain requirement for maintainers; incorrect preflight behavior could block or delay releases, though it reduces risk of shipping broken crate tarballs.
Overview
Release publishing now runs a single multi-crate
cargo publish --dry-run --locked --registry crates-ioacross all release packages before any real upload, so broken tarballs (e.g. missing embedded assets in a dependent crate) fail during preflight instead of after partial uploads. The script requires Cargo 1.90+ for that workflow (documented separately from runtime MSRV inRELEASE_RUNBOOK.md); publish mode still enforces release checkout/assets guards, performs full verification first, then uploads in order while skipping versions already on crates.io.Adds
scripts/release/publish-crates.test.pyand wires it into CI’s release-helper contract step, with offline fixtures for old Cargo rejection, missing tarball assets blocking upload, unpublished workspace deps, and resume/skip behavior.Bundled computer-use plugin path references are updated from
plugins/computer-usetocrates/tui/plugins/computer-use(embed paths inbuiltin.rs, manifest test, Extensions UI,.gitignore, README, and CInpm testdirectory).Reviewed by Cursor Bugbot for commit daea481. Bugbot is set up for automated code reviews on this repo. Configure here.