fix(scan): warn when a mode takeover leaves the other ledger stale - #164
Merged
Mikola Lysenko (mikolalysenko) merged 2 commits intoAug 13, 2026
Merged
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Wrong takeover warning direction
- Fixed by checking that packages were actually confirmed/vendored in the current run before emitting takeover warnings, preventing false warnings on dry-runs and no-op scans where the lockfile still points to the other mode.
Or push these changes by commenting:
@cursor push b56c9b00d2
Preview (b56c9b00d2)
diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs
--- a/crates/socket-patch-cli/src/commands/scan/hosted.rs
+++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs
@@ -472,13 +472,28 @@
// stale. Detect + warn (JSON `warnings[]` and stderr) WITHOUT deleting the
// other mode's ledger; full reconciliation is deferred (see PR Scope).
// Read after the ledger write above so a non-dry-run reflects this run.
+ // Only warn about PURLs that THIS RUN confirmed redirected (landed in the
+ // lockfile) — a dry-run or no-op scan that redirected zero packages must
+ // not warn, even when both ledgers exist (the lockfile still points at the
+ // vendored artifacts, not the hosted server).
let mut takeover_warnings: Vec<serde_json::Value> = Vec::new();
- let superseded = super::overlapping_ledger_purls(&args.common.cwd).await;
- if !superseded.is_empty() {
- takeover_warnings.push(serde_json::json!({
- "code": super::REDIRECT_SUPERSEDES_VENDORED,
- "detail": super::mode_takeover_detail(&superseded, /*current_is_hosted=*/ true),
- }));
+ if !confirmed.is_empty() {
+ use socket_patch_core::utils::purl::{normalize_purl, strip_purl_qualifiers};
+ let confirmed_purls: std::collections::HashSet<String> = confirmed
+ .iter()
+ .map(|(purl, _)| normalize_purl(strip_purl_qualifiers(purl)).into_owned())
+ .collect();
+ let all_overlapping = super::overlapping_ledger_purls(&args.common.cwd).await;
+ let superseded: Vec<String> = all_overlapping
+ .into_iter()
+ .filter(|purl| confirmed_purls.contains(purl))
+ .collect();
+ if !superseded.is_empty() {
+ takeover_warnings.push(serde_json::json!({
+ "code": super::REDIRECT_SUPERSEDES_VENDORED,
+ "detail": super::mode_takeover_detail(&superseded, /*current_is_hosted=*/ true),
+ }));
+ }
}
// Emit an OpenVEX attestation when `--vex` was requested. The redirected
diff --git a/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs b/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs
--- a/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs
+++ b/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs
@@ -61,8 +61,32 @@
/// at the envelope level (JSON `warnings[]` and stderr), mirroring
/// [`note_classic_migration_risk`]; the stale ledger is NOT deleted here
/// (reconciliation is deferred — see the redirect twin in `hosted.rs`).
+/// Only warns about PURLs that THIS RUN actually vendored (have Applied or
+/// Rebuilt events) — a no-op vendor run or dry-run that vendored zero packages
+/// must not warn, even when both ledgers exist.
async fn note_vendor_supersedes_redirect(env: &mut Envelope, cwd: &Path, common: &GlobalArgs) {
- let superseded = super::overlapping_ledger_purls(cwd).await;
+ use crate::json_envelope::PatchAction;
+ use socket_patch_core::utils::purl::{normalize_purl, strip_purl_qualifiers};
+
+ // Extract PURLs that were actually vendored in this run (Applied or Rebuilt).
+ let vendored_purls: std::collections::HashSet<String> = env
+ .events
+ .iter()
+ .filter(|e| matches!(e.action, PatchAction::Applied | PatchAction::Rebuilt))
+ .filter_map(|e| e.purl.as_ref())
+ .map(|purl| normalize_purl(strip_purl_qualifiers(purl)).into_owned())
+ .collect();
+
+ if vendored_purls.is_empty() {
+ return;
+ }
+
+ let all_overlapping = super::overlapping_ledger_purls(cwd).await;
+ let superseded: Vec<String> = all_overlapping
+ .into_iter()
+ .filter(|purl| vendored_purls.contains(purl))
+ .collect();
+
if superseded.is_empty() {
return;
}You can send follow-ups to the cloud agent here.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 44df961. Configure here.
Mikola Lysenko (mikolalysenko)
enabled auto-merge (squash)
August 12, 2026 23:48
Mikola Lysenko (mikolalysenko)
force-pushed
the
fix/mode-takeover-ledger-warn
branch
2 times, most recently
from
August 13, 2026 18:44
02709ef to
dbe7f94
Compare
Wenxin Jiang (Wenxin-Jiang)
approved these changes
Aug 13, 2026
Switching a project's patch mode rewired the lockfile to the new mode but left the displaced mode's ledger on disk asserting wiring that is no longer live (hosted: .socket/vendor/redirect-state.json; vendored: .socket/vendor/state.json + orphaned tarballs). Both scans reported `warnings: []`, so anything auditing a ledger as "what is live" (including `vex`) was misled. Detect the overlap (PURLs claimed by BOTH ledgers) and emit a takeover warning in each flow — `redirect_supersedes_vendored` from the hosted flow, `vendor_supersedes_redirect` from the vendored flow — surfaced in both the JSON `warnings[]` and stderr, naming the displaced package(s) and the stale ledger/orphaned artifacts to clean up. Neither mode silently mutates or deletes the other's ledger; reconciliation is deferred. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The overlap between the hosted and vendored ledgers only proves both name the same package(s) — not which mode won. Each flow assumed the running command displaced the other, so a hosted dry-run/no-op (or a vendored run that did not rewire the overlap) emitted the OPPOSITE `*_supersedes_*` warning and pointed cleanup at the ledger matching the LIVE lockfile — it could tell the user to delete the ledger for what is actually installed. Decide direction from the actual current lockfile wiring instead. `classify_overlap_takeover` reads the scan inventory (a `patch.socket.dev` `resolved` ⇒ hosted is live) and the vendored ledger's wired lockfile(s) (a live `.socket/vendor/<eco>/<uuid>` marker ⇒ vendored is live), then buckets each overlapping PURL by the mode the lock actually proves. The hosted flow warns only for the hosted-live subset, the vendored flow only for the vendored-live subset, and a PURL the lock proves neither way stays silent. Remediation now always points at the ledger that does NOT match the live lockfile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
force-pushed
the
fix/mode-takeover-ledger-warn
branch
from
August 13, 2026 21:59
dbe7f94 to
7f51a0d
Compare
Mikola Lysenko (mikolalysenko)
deleted the
fix/mode-takeover-ledger-warn
branch
August 13, 2026 21:59
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.


Problem
Switching a project's patch mode leaves the DISPLACED mode's ledger
stale, with no warning or reconciliation.
.socket/vendor/redirect-state.json..socket/vendor/state.json(+ committed tarballsunder
.socket/vendor/<eco>/).Running
scan --mode vendoredon a project already redirected by hosted (andvice-versa) rewires the lockfile to the new mode, but the OLD mode's ledger
stays on disk asserting wiring that is no longer live — and, vendored→hosted,
the orphaned vendored tarball is left behind. Both scans report
warnings: []. Anything auditing a ledger as "what is live" (includingvex)is misled.
Confirmed both directions against real prod (minimist@1.2.2 npm): after
hosted→vendored,
package-lock.jsonresolves minimist tofile:.socket/vendor/npm/.../minimist-1.2.2.tgzwhileredirect-state.jsonstill says
mode: hosted,edits[0].action: rewritten,new.resolved: https://patch.socket.dev/...; mirror for vendored→hosted. No warning eitherway. (Kills sweep finding: stale-ledger-on-mode-takeover, P2; reduces the
downstream vex-prefers-stale-vendor-ledger issue.)
Fix
Detect the cross-mode overlap and warn in each flow — no silent mutation of
the other mode's ledger.
crates/socket-patch-cli/src/commands/scan/mod.rs:421— newoverlapping_ledger_purls(cwd)loads BOTH ledgers and returns the PURLsclaimed by each (canonicalized, mirroring
vendored_ledger_supplement).A non-empty result means one mode took the lockfile over from the other for
those package(s) and exactly one ledger is stale per PURL. Empty when either
ledger is missing/empty or the two describe disjoint packages (no false
positives). Plus
mode_takeover_detail(...)and the two warning codesREDIRECT_SUPERSEDES_VENDORED/VENDOR_SUPERSEDES_REDIRECT.crates/socket-patch-cli/src/commands/scan/hosted.rs:469—run_redirectdetects a still-live vendored ledger after writing its own ledger and adds a
redirect_supersedes_vendoredwarning to the JSONwarnings[](line 524)and stderr (line 587).
crates/socket-patch-cli/src/commands/scan/vendor_flow.rs:56— newnote_vendor_supersedes_redirect(...)(mirrorsnote_classic_migration_risk)pushes a
vendor_supersedes_redirectRunWarningonto the envelope +stderr; called at both finalize points of
run_scan_vendor_step(lines 139 and 178).
The warning names the displaced package(s) and the stale ledger / orphaned
artifacts to clean up.
Test
crates/socket-patch-cli/src/commands/scan/mod.rs(tests module), hermetic,no network:
overlapping_ledgers_flag_the_taken_over_package— both ledgers claimminimist ⇒ detection returns exactly that PURL (the takeover warning fires).
single_ledger_present_flags_nothing— a first-time redirect (only oneledger, or none) displaces nothing ⇒ empty (guards against warning on the
FIRST scan of a fresh project).
disjoint_ledgers_are_not_a_takeover— one package redirected, a differentone vendored ⇒ empty (no false positive on a legitimate split).
takeover_detail_names_direction_package_and_remediation— each direction'sdetail names the PURL, the correct stale ledger file, and distinct codes.
cargo test -p socket-patch-cli --lib 'commands::scan::tests'→ 7 passed.cargo build -p socket-patch-cliandcargo clippy -p socket-patch-cliclean.Scope
ledger superseded, or deleting the now-orphaned vendored tarball) is
deliberately deferred — the fix instruction called for warning as sufficient,
and neither mode should silently mutate/delete the other's ledger. The
warning text points the user at
vendor --revert/ removingredirect-state.json/ deleting orphaned.socket/vendor/<eco>/.so it fires on the SECOND scan (the takeover) and stays silent on the first,
matching the real prod repro in both directions.
--dry-rundoes not writeredirect-state.json, so a would-betakeover during a pristine dry run is not flagged; the real (non-dry-run)
takeover is. Not addressed here.
🤖 Generated with Claude Code
Note
Medium Risk
Changes audit-facing warnings and CLI JSON contracts for scan modes; behavior is advisory only (no ledger mutation), but misleading VEX/audit reads were the reported production issue.
Overview
When hosted and vendored patch modes both have ledgers claiming the same package, the lockfile only reflects the current mode — the other ledger (and vendored tarballs after hosted takeover) can stay on disk with no signal. This PR detects overlapping PURLs between
redirect-state.jsonandstate.json(canonicalized like existing vendor supplement logic) and warns without deleting or rewriting the displaced ledger.Hosted
run_redirectaddsredirect_supersedes_vendoredto JSONwarnings[]and stderr after its ledger write. Vendored scan flow addsnote_vendor_supersedes_redirect(envelopewarnings[]+ stderr), aligned withnote_classic_migration_risk, at both finalize paths inrun_scan_vendor_step. Warning text names packages, which file is stale, and manual cleanup (vendor --revert, remove redirect ledger, etc.).Hermetic tests cover overlap detection, single/disjoint ledgers (no false positives), and direction-specific detail strings.
Reviewed by Cursor Bugbot for commit 44df961. Configure here.