GeezerVison

BugBot review

Generated by Claude Code

Labeled Ready for review by the burn-down agent.

• Head: bad5ac25b37969fbb56747cb7ed625d781db3790
• CI: all 293 check runs on this head completed success/skipped/neutral; no merge conflict with main.
• Bugbot: reviewed this head, no findings; no unresolved review threads.
• CHANGELOG.md untouched.

Generated by Claude Code

Final review brief (bad5ac25b)

What it does: 40 socket-patch-cli test files drop their private binary() and git_sha256 copies and use the shared ones from tests/common (more of #824). The shared_helper_copies ratchet loses the matching 40 entries. The two pnpm build suites keep their SOCKET_PATCH_PNPM_E2E_SOCKET_BIN override, now as socket_bin().

Risk: low. Test-only and mechanical: 41 files, +142/−401, no production code. Every removed copy matched the shared helper. git_sha256 was always SHA256("blob <len>\0" + content) or compute_git_sha256_from_bytes, and binary() was always CARGO_BIN_EXE_socket-patch. The compiler checks every call site, and no assertion was removed.

Look here:

• tests/common/mod.rs:46 binary and :126 git_sha256: the shared helpers. The module is #[allow(dead_code)] (:17).
• e2e_vendor_pnpm_build.rs:106: socket_bin() keeps the env override, and no bare binary() call is left in either pnpm suite.
• cli/shared_helper_copies.rs:16:`` the ratchet list.
• e2e_gem.rs:21-32: the wiring pattern; see the note below.

Verified: Compared every removed helper line by line with the shared one. cargo test -p socket-patch-cli --all-features --no-run builds all 29 touched targets with no warnings. Ran 10 of them, all passing: cli (incl. the ratchet), rollback, scan, e2e_vex_lockfile, e2e_vendor_pnpm_build, vendor_jvm_cli, cli_scan_silent, covgap_commands_scan_mod, mode_migration_cargo, e2e_gem. CHANGELOG.md untouched. CI 293/293 success/skipped (ci-ok, clippy green), Bugbot success, no review threads, mergeable.

Changes I made: none.

Open questions / non-blocking note: 9 standalone files now pull in common/mod.rs (which already has pub mod cache_env/hermetic) and also keep their own #[path = "common/cache_env.rs"]/hermetic.rs mods: e2e_gem, e2e_npm, e2e_pypi, e2e_redirect_{gem,gradle}build, e2e_scan, e2e_vendor{composer,pnpm,pypi}_build. That adds clippy::duplicate_mod hits, and 6 targets that were clean now fail under local cargo clippy --all-targets. CI's clippy runs without --all-targets (ci.yml:81), and main already has 312 such errors, so nothing gates on this. A later #824 pass can switch those files to use common::{cache_env, hermetic};. e2e_gem.rs also keeps a private git_sha256_file, which is outside this PR's scope.

Auto-merge is armed, so approving sends it straight to the merge queue.

Generated by Claude Code

[agent] Addressed the final-review note in 189dddb: the 9 standalone suites (e2e_gem, e2e_npm, e2e_pypi, e2e_redirect_{gem,gradle}build, e2e_scan, e2e_vendor{composer,pnpm,pypi}_build) now take use common::{cache_env, hermetic} instead of re-loading those files through their own #[path] mods, so the cache_env.rs/hermetic.rs duplicate_mod hits are gone. All 9 targets pass (cargo test -p socket-patch-cli --all-features).

One duplicate_mod hit of the same kind remains under --all-targets: common/jvm_env.rs is loaded by both common/mod.rs and the shared prebuilt_common/mod.rs in the four suites that include both modules. e2e_vex_lockfile/main.rs on main already has this pattern. Fixing it means changing prebuilt_common, whose includers don't all declare mod common, so it stays for a later #824 pass. e2e_gem.rs's private git_sha256_file is also left for that pass.

Generated by Claude Code

BugBot review

Generated by Claude Code

[final reviewer] Disarmed auto-merge: the head moved from bad5ac25b (my brief) to 189dddb, a non-merge commit that addresses the brief note (shared cache_env/hermetic helpers in 9 standalone suites). ci-ok hasn't finished on the new head yet. I'll re-review the delta and re-arm once CI is green and the brief is updated.

Generated by Claude Code