Skip to content

Fix vendored rescans failing on unwired ledger entries (#541) - #543

Open
Mikola Lysenko (mikolalysenko) wants to merge 8 commits into
mainfrom
agent/fix-vendored-supplement-unwired
Open

Mikola Lysenko (mikolalysenko) wants to merge 8 commits into
mainfrom
agent/fix-vendored-supplement-unwired

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #541

Summary

After a vendored dependency was upgraded or uninstalled, every scan --mode vendored exited 1 (partial_failure / vendor_lock_entry_not_found) and told the user to run vlt install, which they already had. The documented reconcile, scan --prune, failed too: it exited 1 in the same run where it reverted the stale entry. Scheduled vendored scans stayed red after any routine upgrade.

With this change, a vendored rescan skips such an entry, prints a vendor_ledger_entry_unwired warning that points at scan --prune, and exits 0. --prune reverts the entry and exits 0. That holds even when the removed dependency was the project's last one.

Root cause

vendored_ledger_supplement (scan/discovery.rs) puts back into discovery every vendor-ledger entry that has no crawled counterpart. This exists for the fresh-clone case, where the committed artifact is the dependency. The function never checks whether the lock still wires the artifact. Once the dependency has left the lock, the vendor step tries to re-vendor a package the lock no longer has, and it does this before the prune GC runs. The bug is in shared code: npm package-lock projects hit it the same way.

Fix

  • Filter at the shared boundary. The supplement now asks the lockfile in-use probe that the prune GC already reverts by (vendor::dispatch_in_use_one). An entry is supplemented only when the probe says it's in use or can't decide (None, fail-safe: no probe for the ecosystem, or no readable lock). Entries proven unwired come back in LedgerSupplement::unwired. get uses the same supplement and gets the same behavior.
  • Warn, then reconcile. A run that won't prune reports the skipped entries in the run-level vendor_ledger_entry_unwired warning, naming the purls and pointing at scan --prune. Any non-hosted --prune run reverts them in its GC with no warning.
  • Empty crawl. If the removed dependency was the last one, the crawl finds nothing. Until now the GC was skipped entirely there, because pruning the manifest against an empty crawl is destructive. --prune now still runs the vendored half of the GC (gc::run_vendor_only_gc, under the apply lock) whenever unwired entries exist. That half decides from the lockfile and manifest, not the crawl. The manifest prune is still skipped.
  • CLI_CONTRACT.md documents the supplement rule, the warning code and the empty-crawl GC.
  • Drift-kept entries (Bugbot, three rounds): npm can re-lock an uninstalled entry away, and then the prune GC drift-keeps it by design, so the warning comes back. The warning only recommends scan --prune, which reverts just the entries proven unwired. For a drift-kept entry it points at the prune's own GC: kept line. Both lock-edit remedies I tried were unsafe: one doesn't converge, and the other would unwire every vendored entry. Reclaiming such entries is the npm prune follow-up noted on Vendored vlt scan exits 1 after the patched dependency is upgraded or uninstalled, and even scan --prune exits 1 while it reverts the stale entry #541.

Tests (red on main, green here)

Issue scenario Test main this PR
bumped dep, vlt (real vlt): rescan exit 0 + warning, --prune reverts + exit 0, next run clean e2e_vendor_vlt_build::vlt_pinned_matrix_vendored_dependency_bumped_rescan FAIL (rescan: exit 1) PASS
vlt uninstall, same assertions e2e_vendor_vlt_build::vlt_pinned_matrix_vendored_dependency_uninstalled_rescan FAIL (rescan: exit 1) PASS
npm package-lock uninstall: rescan exit 0 + warning scan_vendor_e2e::scan_vendor_skips_ledger_entry_the_lock_no_longer_wires FAIL PASS
last dependency removed: rescan warns, --prune runs the vendored GC on an empty crawl, a drift-kept entry's repeat warning points at GC: kept scan_vendor_e2e::scan_vendor_prune_reconciles_unwired_entry_on_an_empty_crawl FAIL PASS
supplement unit: bumped/uninstalled skipped; wired and lock-less entries kept discovery::tests::ledger_supplement_skips_entries_the_lock_no_longer_wires, ..._keeps_wired_and_undecidable_entries FAIL with the filter disabled PASS

Test-harness notes:

  • The real-vlt mock patch API now answers only for the purls a batch request asks about, as the production API does. Before, it offered the 1.3.0 patch to a project bumped to 1.2.0.
  • On vlt 0.0.0-30 … 0.0.0-32, a dependency removed from package.json stays in the lock: vlt uninstall keeps a file: spec declared, and vlt install keeps the removed edge and node. socket-patch correctly keeps the entry there. I tested every supported release from 0.0.0-19 through rc.13, and those three are the only ones affected. The uninstall leg skips them under a new removed-dependency-stays-locked rule in docs/testing/vlt-compatibility.md, and the manifest is regenerated with check-vlt-legs.py --derive.

Local verification

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • e2e_vendor_vlt_build through check-vlt-legs: clean on vlt 0.0.0-30, 0.0.0-32, rc.14 and 1.2.0 (34/34). The new legs also pass on 0.0.0-19 … 0.0.0-29, rc.1/5/9/13, rc.32 and 1.0.4.
  • On vlt 1.2.0: e2e_redirect_vlt_build 38/38, mode_migration_vlt 14/14, plus e2e_safety_vlt and e2e_vlt passing. scan_vendor_e2e passes 33/33.
  • cargo test --workspace --all-features --no-fail-fast: 208 binaries pass. 12 tests in 4 binaries fail identically on origin/main, because the sandbox runs as root and their read-only-directory setups don't block writes. Those are covgap_commands_vendor (3), in_process_redirect (3), repair (2) and the socket-patch-core lib (4); this PR doesn't touch core.
  • The *_production vlt legs can't run in this sandbox (the CLI's TLS roots reject the proxy's CA), so CI covers them.
  • cargo fmt --all -- --check reports drift in 127 files on origin/main itself, and CI doesn't run fmt. I left the repo's formatting as is.

CI notes

  • The PDM native (...) jobs fail one random cell per run against the live patch API, and the cell changes every time. This is the same flake PR Fix lock-only requirements.txt discovery (#412, #523) #530 cleared on a rerun; details are in the PR comments. I re-ran the failed jobs once.
  • No wrapper (npm/, pypi/, gem/) changes are needed: they only dispatch to the binary.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VEgj84zEsgqYoVSqrt7CqJ


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A vendored scan re-added every vendor-ledger entry to discovery, even
after the dependency was upgraded or uninstalled. The vendor step then
tried to re-vendor a package the lockfile no longer has, so every
rescan failed with exit 1, including the --prune run that reverts the
stale entry (#541).

Ledger entries are now filtered through the same lockfile in-use
check the prune GC uses. Entries the lock no longer wires are skipped
with a vendor_ledger_entry_unwired warning that points at --prune.
When the crawl finds nothing, a --prune run still does the lock-based
vendored half of GC, so the last removed dependency can be reverted.

Assisted-by: Claude Code:claude-opus-5-5
Covers #541 with real vlt: after the vendored dependency is bumped or
uninstalled, a plain vendored rescan exits 0 with a warning, --prune
reverts the entry and exits 0, and the next run is clean.

The harness's mock patch API now answers only for the purls a batch
request asks about, as the production API does, so a bumped release
is not offered the old release's patch.

Assisted-by: Claude Code:claude-opus-5-5
Adds the npm package-lock regression test for #541 and documents the
new vendor_ledger_entry_unwired warning and the empty-crawl vendored
GC in CLI_CONTRACT.md.

Assisted-by: Claude Code:claude-opus-5-5
On vlt 0.0.0-30 to 0.0.0-32 a dependency removed from package.json
stays in the lock: vlt uninstall keeps its file: spec declared, and
vlt install keeps the removed edge and node. socket-patch correctly
keeps the vendored entry there, so the uninstall leg now skips those
releases with a documented compatibility rule.

The leg keeps a bystander dependency and re-locks by hand if vlt
leaves the spec declared. A new mock npm test covers --prune
reconciling an unwired entry when the crawl finds no packages.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] On 27991ff, install-proof (*, 0.0.0-30) and e2e_vendor_vlt_build (0.0.0-32) failed in the new dependency_uninstalled_rescan leg. The cause is in the test, not the fix. On vlt 0.0.0-30 … 0.0.0-32, removing a dependency from package.json doesn't take it out of the lock: vlt uninstall keeps its file: spec declared, and vlt install keeps the removed edge and node. So the vendored node is still wired, and socket-patch correctly keeps the entry.

I ran every supported release from 0.0.0-19 through rc.13 locally, and only 0.0.0-30, 0.0.0-31 and 0.0.0-32 behave this way. 5d6bfb6 adds a removed-dependency-stays-locked rule to docs/testing/vlt-compatibility.md and regenerates the manifest. With it, check-vlt-legs is clean on 0.0.0-30, 0.0.0-32, rc.14 and 1.2.0.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] The PDM native (...) failures aren't caused by this PR:

  • 27991ff: 2.10.4 marker-excluded vendored FAIL rescanIdempotent
  • 5d6bfb6: 2.7.4 crlf hosted FAIL refusalCodeReported
  • 5d6bfb6: 2.29.2 static-urls hosted FAIL rescanIdempotent

Each failing job has exactly one FAIL out of 36–46 cells, and a different cell fails on every run. The 2.10.4 cell that failed on 27991ff passes on 5d6bfb6, and the two jobs that fail on 5d6bfb6 passed on 27991ff. Each failing cell ran 20–34s against the live patch API, while its sibling cells took about 5s. PR #530 hit the same pattern (2.9.3 extras vendored FAIL appliedExactlyOne) and went green on a rerun of the same commit.

The code path rules this PR out as well. The new filter only drops entries that the lockfile in-use probe proves unwired, and that probe exists only for npm and cargo; it answers "keep" for every PyPI entry. Hosted runs have no vendor ledger at all. I'll re-run the failed jobs once when this run finishes.


Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 2, 2026 09:13
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-cli/src/commands/scan/render.rs
An npm entry whose lock entries were re-resolved since vendoring is
drift-kept by scan --prune, so the vendor_ledger_entry_unwired warning
came back on the next rescan while still suggesting only --prune. The
warning now also names the way out of that case: restore the vendored
dependency and run vendor --revert.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-cli/src/commands/scan/render.rs Outdated
The unwired-entry warning suggested restoring the dependency and
running vendor --revert. That does not clear a drift-kept npm entry:
reinstalling writes a fresh registry entry that still counts as drift,
and vendor --revert reverts every ledger entry anyway.

The warning now gives the remedy the drift-keep contract supports:
restore the lockfile from before vendoring, run scan --prune again,
then re-run the package manager's install. The purls move into
parentheses so they no longer read as command arguments. The npm test
follows those steps and checks that the warning is gone afterwards.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-cli/src/commands/scan/render.rs
The previous wording told users to restore the pre-vendor lockfile
and prune again. That lock has no .socket/vendor paths, so the prune
would revert every vendored entry, not just the stale ones.

The warning now names only scan --prune. A prune that drift-keeps an
entry already explains it on its own GC: kept line, so the warning
points there instead of suggesting a lock edit whose reach it cannot
bound.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] On 11934b9, native (ubuntu-latest, 1.7.1) in the Poetry workflow failed two hosted cells: crlf hosted FAIL appliedExactlyOne and direct hosted FAIL rescanIdempotent. Every vendored and agent cell passed. This isn't caused by the PR. The same Poetry job passed on beb2118, eacc11b and 5d6bfb6, and 11934b9 changes only the vendor_ledger_entry_unwired warning text and one npm test. Hosted mode also has no vendor ledger, so none of this PR's code runs in those cells. It's the same live-patch-API flake as the PDM cells above. I'll re-run the failed job once when the run finishes.


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 11934b9. Configure here.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vendored vlt scan exits 1 after the patched dependency is upgraded or uninstalled, and even scan --prune exits 1 while it reverts the stale entry

2 participants