GeezerVison is showing github.com — Fix cargo cold-cache apply exiting 0 (#616) by mikolalysenko · Pull Request #1310 · SocketDev/socket-patch
Conversation
LLM Description written by Claude Code:claude-opus-5-5
Fixes #616
Summary
socket-patch apply on a cold or pruned Cargo cache exits 1 again, as it did in 4.x. The error names cargo fetch as the remedy. Before this change it exited 0 with status: "success", and the next cargo build --locked compiled the unpatched crate.
Root cause
#555 (the #403 fix) made apply treat any manifest purl that the project lock resolves, but that isn't on disk, as a calm package_not_installed "lockfile-only" skip. For npm that means the package was deliberately left out (a platform-gated optional dependency, or --omit=dev). Cargo never leaves a locked crate out on purpose: cargo fetch unpacks every Cargo.lock entry into registry/src, target-gated ones included (checked locally with a cfg(windows) dependency on macOS). So a locked crate that is missing just hasn't been fetched yet, and the next build downloads or re-extracts it unpatched.
Fix
lockfile_resolved (crates/socket-patch-cli/src/commands/apply.rs) no longer counts pkg:cargo/ purls as lockfile-only (lock_resolution_is_calm). An unfetched crate is now an ordinary unresolved purl: an all-miss run exits 1 / partialFailure, and a partial miss prints the usual warning, as in 4.x.
- Cargo misses carry an actionable remedy in three places: the
package_not_installed event detail, the human error block and the human warning (run \cargo fetch` first …`).
- No target-gated special case is needed, because
cargo fetch unpacks every locked crate, so it always fixes the miss.
- The npm calm skips are unchanged, and their tests still pass.
- Docs: the CLI_CONTRACT
package_not_installed row, and docs/ecosystems.md "Cargo: shared registry cache", which now says to run cargo fetch before apply.
Per-issue checklist
Red → green
Commands run
cargo fmt --all -- --check: clean for the changed files. The only diff is pre-existing on main, in socket-patch-core/src/patch/redirect/upstream/mod.rs.
cargo clippy --workspace --all-features -- -D warnings: clean.
cargo test -p socket-patch-cli --lib apply: 64 passed.
cargo test -p socket-patch-cli --test apply --test in_process_cargo_apply --test e2e_safety_cargo_build --test e2e_vex_lockfile --test scan_vendor_e2e: all green.
cargo test -p socket-patch-cli --no-fail-fast (full suite): 207 binaries green. The only 2 failures are local-environment only, in e2e_vendor_cargo_build old-toolchain legs (Bad CPU type in executable: an x86 rustup 1.41 toolchain on Apple silicon without Rosetta). They are unrelated to this change.
🤖 Generated with Claude Code
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Since #555, apply treated a manifest crate that Cargo.lock resolves
but that is not unpacked in $CARGO_HOME/registry/src as a calm
lockfile-only skip: exit 0, status success. On a fresh CI runner or
a pruned cache that crate was simply not fetched yet, so
'socket-patch apply && cargo build --locked' went green and shipped
the unpatched crate (#616).
Cargo crates are no longer lockfile-only: cargo fetch unpacks every
locked crate, target-gated ones included, so a missing one is a real
miss. An all-miss run exits 1 again, as in 4.x, and the JSON detail,
the human error and the warning tell the user to run cargo fetch
first. npm's platform-gated and --omit=dev skips are unchanged.
Fixes #616
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9f84ebb. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[final reviewer] I disarmed auto-merge at 9f84ebbb because ci-ok is red on this head. Only hosted-e2e and e2e (ubuntu-latest, e2e_safety_pnpm) fail, which is the main-wide [email protected] failure (#1293), not this PR. #1302 fixes it and is in the merge queue now. Once main carries it, merge main in here (no other commits needed) and I'll re-arm auto-merge after CI goes green. The approval still covers this head.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Ready for review at bc0c6e5a8d9c.
- CI: required checks
ci-ok and clippy green; 7 check suites succeeded. 1 superseded workflow run(s) show as cancelled; the required gates passed on this head.
- Mergeable against
main, no CHANGELOG.md change.
- Bugbot reviewed this head; no unresolved review threads.
Labeled Ready for review by the burn-down agent. Slack announcement pending (connector unavailable this run).
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|