Repository navigation
Fix cargo agent mode patching overridden crates (#506) - #1715
Mikola Lysenko (mikolalysenko) merged 5 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
Regression tests for #506: apply and vex must ignore a registry-cache copy of a crate that Cargo.lock takes from a [patch.crates-io] path (in Cargo.toml or .cargo/config.toml) or a git source, since cargo never builds that copy. A registry-sourced control still patches and attests. Assisted-by: Claude Code:claude-opus-5-5
Agent-mode apply and vex looked a crate up in the shared $CARGO_HOME/registry/src by name and version alone. When Cargo.lock takes the crate from a git dependency, apply patched a registry copy cargo never builds and reported success. When the crate came from a [patch] path override, vex hash-verified that unused copy and attested not_affected for code the build does not contain. Both lookups now keep only crates the lock takes from a registry, the same scope the scan crawl already uses. Rollback and scan --prune still see the copy, so a patch an earlier apply left there can be restored. The cargo not-installed message now names this case besides the cargo fetch remedy. Fixes #506 Assisted-by: Claude Code:claude-opus-5-5
Address review of the #506 fix: test a plain path dependency and a [patch] override that --allow-patch-conflicts lets apply try, since the lock-source filter alone decides those. Share one lock-scope helper between the scan crawl and the new lookup so the two can't drift, and say in the docs which code each override shape gets. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
|
Ready for review (burn-down agent)
Slack announcement wasn't sent in this run; a later run will retry. Generated by Claude Code |
Final review briefWhat it does: In a local Cargo project, agent-mode Risk: Low. The filter reuses the lock scope Look here:
Verified:
Changes I made: none. Open questions: none blocking. One note: when Auto-merge is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
A [replace] override leaves the crate's Cargo.lock entry with its registry source and only adds a replace key naming the copy cargo builds. The lock-scope check counted that entry as registry-sourced, so vex still attested the unused registry copy and apply with --allow-patch-conflicts still patched it. The lock reader now reads the replace key, and replaced entries are out of scope. Reported in review of #1715. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 ed601ac. Configure here.
|
[final reviewer] Disabled auto-merge. This PR was armed only on the strength of the final-review brief, and the head has gained non-merge commits since that brief (review fixes pushed today). I'll write a fresh brief once those pushes settle and CI and Bugbot are green on the new head, then re-arm. The Generated by Claude Code |
|
Burn-down agent: Ready for review at
Generated by Claude Code |
Final review briefWhat it does: Risk: low to medium.
Look here
Verified
Changes I made: none. Open questions (none blocking; follow-up material)
Auto-merge is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #506
Summary
In a Cargo project, agent mode and
vexno longer patch or attest acrate's copy in the shared
$CARGO_HOME/registry/srcwhen the project'sCargo.locktakes that crate from a git, path or[patch]source. Cargonever builds that copy. Before this change, agent mode could patch it and
vexcould emit a falsenot_affectedfor code the build doesn'tcontain.
Root cause
Apply (
find_all_packages_for_purls) and vex(
find_manifest_package_copies_reusing) find a Cargo crate by looking up<name>-<version>in every registry index dir(
CargoCrawler::find_by_purls), by name and version alone. They neverasked whether the lock takes that crate from a registry. The scan crawl
was already scoped to the lock's registry-sourced entries (
lock_scope,#1662), but these two lookups were not.
What
maindid with each shape, from the red runs:mainmain[patch.crates-io]path inCargo.tomluser_patch_conflict).cargo/config.tomluser_patch_conflict)[patch]+--allow-patch-conflictspathdependency[replace](lock keeps registrysource+replacekey)--allow-patch-conflictsFix
CargoCrawler::locked_registry_crates(options)returns theregistry-sourced
(name, version)s of a local project's lock, orNonewhen the crawl isn't scoped to a lock (global run,cargo vendordirectory source, no readable lock). A registry entry carrying a
replacekey (a[replace]override) is out of scope too: the lockreader now reads that key (review finding, ed601ac). It shares one helper
(
registry_lock_scope) withcrawl_all, so scan, apply and vex agree.retain_locked_cargo_copiesdrops Cargo copies outside that scope.Only the apply and vex lookups call it. Rollback and
scan --prunestillfind the copy, because an earlier apply may have patched it (Agent-mode cargo
scan --sync/--prunedrops the manifest entry of a crate the lock no longer resolves but leaves its shared registry-cache copy patched, sorollbackcan't restore it (regression from #1205) #1278).package_not_installeddetail and human hint now also namethe path/git/
[patch]case, besides thecargo fetchremedy.CLI_CONTRACT.mdanddocs/ecosystems.mdare updated to match.Tests (red → green)
apply::cargo_lock_source::crate_overridden_away_from_the_registry_is_not_patched[patch]+--allow-patch-conflictsapply::cargo_lock_source::allowed_patch_conflict_still_leaves_the_unused_registry_copy[replace](review)Replaceshape; corereplaced_registry_entry_is_out_of_scopee2e_vex::verify_mode_does_not_attest_cargo_crate_overridden_away_from_registryregistry_sourced_crate_is_patched_in_the_registry_cache,verify_mode_attests_registry_sourced_cargo_crateCommands run locally:
cargo fmt --all -- --check: cleancargo clippy --workspace --all-features -- -D warnings: cleancargo test --workspace --all-features --no-fail-fast: 14,911 passed, 11failed. All 11 are environmental and in code this PR doesn't touch. The
container runs as root, so the chmod-based write-failure tests in
in_process_redirect,repairand corevendor::pypi_*/vlt_heal/copy_treecan't fail their writes. The twoe2e_bun_lockbfailures come from a blocked network blob fetch and thelocal bun 1.4.4. CI runs these on its own runners.
e2e_safety_cargo_build -- --ignored10/10 pass.e2e_redirect_cargo_build,e2e_redirect_cargo_shapes,e2e_vendor_cargo_buildandmode_migration_cargohave no ignored tests;their regular tests passed in the workspace run.
npm/,pypi/andgem/only dispatchto the binary.
Review notes (
/code-review high), not fixed here<cwd>/Cargo.lockonly, sofrom a member directory with no lock of its own the filter doesn't apply.
This is the same limitation
crawl_allalready has. Walking up to anancestor lock risks scoping a nested non-member project to an unrelated
lock, so it needs its own change. Follow-up.
get's installed-version narrowing still uses the unscoped rollbacklookup, so
getcan record a patch thatapplythen reports as notinstalled. Nothing gets patched or attested wrongly. Follow-up.
calm skip. This is deliberate: the build stays unpatched, which matches
vendored mode refusing the same project. The detail now says why.
.cargo/configpaths = [...]overrides leave no trace inCargo.lock, so this lock-based scope can't see them. Plainapplyalready skips them through the user-patch detector, but
vexdoesn'tconsult it. Follow-up.
Cargo.lockonce perapply/vex run. That's minor, and only for runs with Cargo purls.
https://claude.ai/code/session_01HAS6MmhgCPZmdDxkfHZmMG
Generated by Claude Code