diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 69fb09df9..ad0d44ab5 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -717,7 +717,11 @@ worse, lets a warm cache silently serve unpatched bytes): whole-file wiring cannot tell a converged fragment from a drifted one, keep the artifact exactly while the live `composer.lock` / `pom.xml` / `nuget.config` still names its `.socket/vendor//` dir — a file that no longer references it is warned about and the - artifact removed), removes the artifacts, prunes the + artifact removed; in the npm family (npm, yarn classic and berry, pnpm, bun) a recorded lock entry + that no longer exists at all — the user removed the dependency — is not drift: it warns + `vendor_lock_entry_removed` and the artifact and entry are kept unless every wired file that exists + was read and none mentions the uuid in any spelling (an unreadable lock keeps them), so `rollback` / `remove` / `scan --prune` clean up + after `npm uninstall` / `yarn remove` / `pnpm remove` / `bun remove`), removes the artifacts, prunes the ledger, sweeps orphan uuid dirs, and (v5.0) prunes the now-empty `.socket/vendor//` and `.socket/vendor/` levels — `.socket/` itself is removed by the lock guard when nothing else is left. It works without a manifest: with no manifest and no ledger it is a clean exit-0 no-op. diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index 1b6b410fc..c0eb71045 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -492,6 +492,57 @@ async fn revert_round_trip() { assert_eq!(env["summary"]["removed"], 0); } +/// #665: `npm uninstall left-pad` after vendoring deletes the lock entry +/// the wiring recorded, so nothing resolves through the vendored artifact +/// any more. `rollback` used to report that as drift, keep the artifact +/// and the ledger entry, and exit 1 on every run; `vendor --revert` then +/// "succeeded" without cleaning anything up. Now the first rollback drops +/// the unreferenced artifact and ledger entry, and every later run is a +/// clean exit 0. +#[tokio::test] +async fn rollback_after_dependency_removed_cleans_up_and_converges() { + let fx = npm_fixture(); + assert_eq!(vendor_run(vendor_args(fx.root())).await, 0); + assert!(fx.tgz_path().exists(), "sanity: vendored"); + + // What `npm uninstall left-pad` leaves behind. + let mut lock = fx.lock_value(); + let packages = lock["packages"].as_object_mut().unwrap(); + packages.remove("node_modules/left-pad"); + packages[""]["dependencies"] = json!({}); + let mut uninstalled = serde_json::to_vec_pretty(&lock).unwrap(); + uninstalled.push(b'\n'); + std::fs::write(fx.lock_path(), &uninstalled).unwrap(); + std::fs::remove_dir_all(fx.root().join("node_modules/left-pad")).unwrap(); + + let cwd = fx.root().to_str().unwrap(); + let (code, stdout, stderr) = run_cli( + fx.root(), + &["rollback", "--json", "--yes", "--offline", "--cwd", cwd], + &[], + ); + assert_eq!(code, 0, "rollback must succeed:\n{stdout}\n{stderr}"); + assert!( + !stdout.contains("vendor_artifact_kept"), + "nothing is kept:\n{stdout}" + ); + assert!( + !fx.vendor_dir().exists(), + "the unreferenced artifact and ledger are cleaned up:\n{stdout}" + ); + assert_eq!(fx.lock_bytes(), uninstalled, "the user's lock is untouched"); + + let (code, stdout, stderr) = run_cli( + fx.root(), + &["rollback", "--json", "--yes", "--offline", "--cwd", cwd], + &[], + ); + assert_eq!(code, 0, "a second rollback is a no-op:\n{stdout}\n{stderr}"); + let (code, env) = vendor_cli(fx.root(), &["--revert"]); + assert_eq!(code, 0, "{env:#}"); + assert!(events(&env).is_empty(), "nothing left to revert: {env:#}"); +} + // ───────────────────────────────────────────────────────────────────── // 5. revert works without a manifest // ───────────────────────────────────────────────────────────────────── diff --git a/crates/socket-patch-cli/tests/scan_vendor_e2e.rs b/crates/socket-patch-cli/tests/scan_vendor_e2e.rs index 97a194e5a..2f0208055 100644 --- a/crates/socket-patch-cli/tests/scan_vendor_e2e.rs +++ b/crates/socket-patch-cli/tests/scan_vendor_e2e.rs @@ -1024,20 +1024,15 @@ async fn scan_vendor_resolves_percent_encoded_scoped_purl() { // ───────────────────── prune reconciles vendored state ───────────────────── /// After a dependency is removed and re-locked, `scan --prune` (without -/// `--vendor`) honors the drift-keep contract, then completes the reclaim -/// once the drift is undone: +/// `--vendor`) reclaims its vendored entry in one run (#665): /// -/// 1. The wired lock entry VANISHED (an uninstall is one drift flavor — -/// the live lock no longer matches anything the wiring recorded), so -/// the backend revert keeps the artifacts (`RevertOutcome:: -/// kept_artifact`) and the GC must keep the ledger entry too — pruning -/// it would let the orphan sweep destroy the kept artifacts (with the -/// recorded pre-vendor originals, the state a later `git checkout` of -/// the vendored lock still points at). -/// 2. Undoing the drift (restoring the pre-vendor registry lock — the -/// keep warning's documented remediation) converges every recorded -/// fragment, and the same prune then reverts fully: ledger entry -/// dropped, artifact dir removed, lock untouched. +/// 1. The wired lock entry VANISHED (`npm uninstall`). That is not drift: +/// nothing in the lock resolves through the artifact any more, so the +/// backend revert removes it and the GC drops the ledger entry, leaving +/// the user's re-locked lock byte-identical. (Before #665 the vanished +/// entry was drift-kept forever and the `scan --prune` remedy the +/// vendored rescan prints never converged.) +/// 2. A second prune is a no-op. /// /// Every vendored entry is ledger-owned (`detached`), and the lockfile- /// usage leg of the GC judges entries by the LIVE lock, so being detached @@ -1048,7 +1043,6 @@ async fn scan_prune_reverts_unused_vendored_entry() { mount_patch_api(&mock, UUID).await; let tmp = tempfile::tempdir().unwrap(); write_fixture(tmp.path()); - let original_lock = std::fs::read(tmp.path().join("package-lock.json")).unwrap(); // A second installed package so the later prune run's crawl is // non-empty (left-pad itself gets removed below). @@ -1108,44 +1102,18 @@ async fn scan_prune_reverts_unused_vendored_entry() { serde_json::from_str::(stdout.trim()).expect("valid JSON") }; - // 1. Drifted (vanished) lock entry: everything is KEPT — nothing may - // be reported reverted, and the artifacts must survive the sweep. - let v = run_prune(); - assert_eq!( - v["gc"]["revertedVendoredEntries"], - serde_json::json!([]), - "a drift-kept entry must not be reported reverted: {v}" - ); - let state: serde_json::Value = serde_json::from_str( - &std::fs::read_to_string(tmp.path().join(".socket/vendor/state.json")).unwrap(), - ) - .unwrap(); - assert!( - state["entries"][PURL].is_object(), - "ledger entry must be kept: {state}" - ); - assert!( - tmp.path() - .join(format!(".socket/vendor/npm/{UUID}")) - .exists(), - "kept artifacts must survive the orphan sweep" - ); - // The (already left-pad-free) lock stays exactly as the user re-locked - // it — the keep never edits a lock it refused to own. - assert_eq!( - std::fs::read(tmp.path().join("package-lock.json")).unwrap(), - lock_bytes - ); - - // 2. Undo the drift: restore the pre-vendor registry lock, so every - // recorded fragment is converged. The same prune now reclaims fully. - std::fs::write(tmp.path().join("package-lock.json"), &original_lock).unwrap(); + // 1. Vanished lock entry: reverted in one run. let v = run_prune(); assert_eq!( v["gc"]["revertedVendoredEntries"], serde_json::json!([PURL]), "gc must report the reverted entry: {v}" ); + assert_eq!( + v["gc"]["keptVendoredEntries"], + serde_json::json!([]), + "nothing resolves through the artifact, so nothing is kept: {v}" + ); // Ledger empty (an emptied state file is removed outright), artifact // gone. @@ -1166,11 +1134,22 @@ async fn scan_prune_reverts_unused_vendored_entry() { .exists(), "artifact dir removed" ); - // The converged revert restores nothing (the lock already equals every - // recorded original), so the restored lock survives byte-for-byte. + // The user's re-locked lock is left exactly as they wrote it. assert_eq!( std::fs::read(tmp.path().join("package-lock.json")).unwrap(), - original_lock + lock_bytes + ); + + // 2. Nothing left to reclaim. + let v = run_prune(); + assert_eq!( + v["gc"]["revertedVendoredEntries"], + serde_json::json!([]), + "{v}" + ); + assert_eq!( + std::fs::read(tmp.path().join("package-lock.json")).unwrap(), + lock_bytes ); } @@ -1288,38 +1267,43 @@ async fn scan_vendor_prune_reconciles_unwired_entry_on_an_empty_crawl() { assert_eq!(v["scannedPackages"], 0, "envelope={v}"); assert_eq!(unwired(&v), 1, "envelope={v}"); assert!(v.get("gc").is_none(), "no --prune, no GC: {v}"); + // The detail names the purl and the prune that reverts it. + let detail = v["warnings"] + .as_array() + .into_iter() + .flatten() + .find(|w| w["code"] == "vendor_ledger_entry_unwired") + .and_then(|w| w["detail"].as_str()) + .unwrap_or_else(|| panic!("envelope={v}")) + .to_string(); + assert!( + detail.contains(&format!("({PURL})")) && detail.contains("scan --prune"), + "{detail}" + ); let (code, stdout, stderr) = run_scan_vendor(tmp.path(), &mock.uri(), &["--prune"]); assert_eq!(code, 0, "stdout={stdout}; stderr={stderr}"); let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON"); assert_eq!(unwired(&v), 0, "the pruning run reconciles instead: {v}"); - // npm re-locked the entry away, so the wet revert drift-keeps it (see - // `scan_prune_reverts_unused_vendored_entry`): the point here is that - // the vendored GC ran at all on an empty crawl. + // The vendored GC ran on an empty crawl, and since npm re-locked the + // entry away (nothing resolves through the artifact) it reverts it + // (#665; see `scan_prune_reverts_unused_vendored_entry`). assert_eq!( - v["gc"]["keptVendoredEntries"], + v["gc"]["revertedVendoredEntries"], serde_json::json!([PURL]), "envelope={v}" ); + assert_eq!( + v["gc"]["keptVendoredEntries"], + serde_json::json!([]), + "envelope={v}" + ); - // The drift-kept entry is still unwired, so the next plain rescan warns - // again; its detail names the purl and the prune's `GC: kept` report - // instead of a lock edit that could unwire other vendored entries. + // Reconciled: the next plain rescan has nothing left to warn about. let (code, stdout, stderr) = run_scan_vendor(tmp.path(), &mock.uri(), &[]); assert_eq!(code, 0, "stdout={stdout}; stderr={stderr}"); let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON"); - let detail = v["warnings"] - .as_array() - .into_iter() - .flatten() - .find(|w| w["code"] == "vendor_ledger_entry_unwired") - .and_then(|w| w["detail"].as_str()) - .unwrap_or_else(|| panic!("envelope={v}")) - .to_string(); - assert!( - detail.contains(&format!("({PURL})")) && detail.contains("`GC: kept`"), - "{detail}" - ); + assert_eq!(unwired(&v), 0, "envelope={v}"); } /// Interactive (non-JSON) `scan --vendor` pre-verifies patch baselines: diff --git a/crates/socket-patch-core/src/vendor/bun_lock.rs b/crates/socket-patch-core/src/vendor/bun_lock.rs index 314429442..24a3dae22 100644 --- a/crates/socket-patch-core/src/vendor/bun_lock.rs +++ b/crates/socket-patch-core/src/vendor/bun_lock.rs @@ -912,6 +912,17 @@ pub(crate) async fn revert_bun_opts( // ran; the artifact dir stays behind (and the caller keeps the ledger // entry), so only the deletion is skipped. if !keep_artifact { + if super::npm_flavor::keep_artifact_while_lock_references_it( + &mut outcome, + project_root, + &[BUN_LOCK], + &entry.uuid, + &uuid_dir_rel, + ) + .await + { + return outcome; + } // The last npm-family entry leaves `.socket/vendor/npm/` (and // `.socket/vendor/`) empty: the shared helper prunes them so a // reverted project carries no vendor residue (non-recursive: @@ -1001,9 +1012,13 @@ fn revert_one_record( } return; } - warnings.push(drifted(format!( - "lock entry `{key}` no longer exists; nothing to restore" - ))); + // REMOVED, not drifted (#665): `bun remove` dropped the entry. The + // caller keeps the artifact only while the lock still resolves + // through it. + warnings.push(VendorWarning::new( + super::LOCK_ENTRY_REMOVED_CODE, + format!("lock entry `{key}` no longer exists; nothing to restore"), + )); } // ───────────────────────── vendor-specific classification ───────────────── @@ -3309,8 +3324,13 @@ mod tests { assert!(fx.root().join(fx.rel_tgz()).exists(), "artifact kept"); } + /// #665: `bun remove left-pad` deleted the vendored entry line, so + /// nothing in bun.lock resolves through the artifact any more. A + /// vanished entry is not drift: the revert succeeds and removes the + /// unreferenced artifact instead of keeping it (and the ledger entry) + /// forever. #[tokio::test] - async fn vanished_entry_key_drift_keeps() { + async fn vanished_entry_drops_the_unreferenced_artifact() { let fx = fixture_with(BN3_BEFORE_LOCK, "node_modules/left-pad").await; let (_, entry, _) = expect_done(fx.vendor(false).await); let entry = entry.unwrap(); @@ -3329,17 +3349,50 @@ mod tests { let outcome = revert_bun(&entry, fx.root(), false).await; assert!(outcome.success, "{:?}", outcome.error); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); assert!( outcome .warnings .iter() - .any(|w| w.code == "vendor_lock_entry_drifted" + .any(|w| w.code == "vendor_lock_entry_removed" && w.detail.contains("no longer exists; nothing to restore")), "{:?}", outcome.warnings ); - assert!(outcome.kept_artifact, "drift-skip keeps the artifact"); + assert!(!outcome.kept_artifact, "{:?}", outcome.warnings); assert_eq!(fx.read_lock().await, without_entry, "nothing rewritten"); + assert!( + !fx.root().join(fx.rel_tgz()).exists(), + "unreferenced artifact removed" + ); + } + + /// #665 guard: the recorded entry vanished but another entry line still + /// resolves through the artifact, so it is kept like a drift-skip. + #[tokio::test] + async fn vanished_entry_keeps_the_artifact_while_the_lock_references_it() { + let fx = fixture_with(BN3_BEFORE_LOCK, "node_modules/left-pad").await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + let new_line = entry.wiring[0] + .new + .as_ref() + .and_then(Value::as_str) + .unwrap(); + let live = fx.read_lock().await; + // Re-key the entry (`"left-pad"` → `"other/left-pad"`): the + // recorded key is gone, the tuple still points into our uuid dir. + let rekeyed_line = new_line.replacen("\"left-pad\"", "\"other/left-pad\"", 1); + assert_ne!(rekeyed_line, new_line, "the re-key must hit"); + let rekeyed = live.replace(new_line, &rekeyed_line); + tokio::fs::write(fx.root().join(BUN_LOCK), &rekeyed) + .await + .unwrap(); + + let outcome = revert_bun(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.kept_artifact, "{:?}", outcome.warnings); + assert_eq!(fx.read_lock().await, rekeyed, "nothing rewritten"); assert!(fx.root().join(fx.rel_tgz()).exists(), "artifact kept"); } diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index a2592e400..776e0d460 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -657,6 +657,10 @@ impl RevertOpts { } } +/// Warning code for a recorded lock entry that no longer exists at revert +/// time (the dependency was removed). See [`RevertOutcome::lock_entry_removed`]. +pub const LOCK_ENTRY_REMOVED_CODE: &str = "vendor_lock_entry_removed"; + /// The result of one backend `revert_*` call. #[derive(Debug)] pub struct RevertOutcome { @@ -695,9 +699,14 @@ impl RevertOutcome { } /// True when any wiring record was left alone during the restore — - /// every left-alone branch (ownership-gate drift, vanished block, - /// missing pre-vendor original, unknown kind/key, allowlist skip) - /// warns with the stable code `vendor_lock_entry_drifted`. + /// every left-alone branch (ownership-gate drift, missing pre-vendor + /// original, unknown kind/key, allowlist skip) warns with the stable + /// code `vendor_lock_entry_drifted`. + /// + /// A recorded lock entry that has VANISHED (the user removed the + /// dependency) is not drift: it warns `vendor_lock_entry_removed` + /// instead (see [`Self::lock_entry_removed`]), and the backend keeps + /// the artifact only while the lock still resolves through it. /// /// LIVENESS CONTRACT: backends must NOT emit that code for a record /// whose live state already equals its reverted state (the fragment @@ -714,6 +723,19 @@ impl RevertOutcome { .any(|w| w.code == "vendor_lock_entry_drifted") } + /// True when any recorded lock entry no longer existed at revert time + /// (`vendor_lock_entry_removed`): the user removed the dependency + /// (`yarn remove`, `npm uninstall`, ...), so there was nothing to + /// restore. Unlike drift, nothing of the user's is being protected, so + /// this alone must not keep the artifact forever (#665). The backend + /// keeps it only while a lockfile still mentions the uuid dir, see + /// `npm_flavor::keep_artifact_while_lock_references_it`. + pub fn lock_entry_removed(&self) -> bool { + self.warnings + .iter() + .any(|w| w.code == LOCK_ENTRY_REMOVED_CODE) + } + /// Mark the artifact dir as deliberately kept after a drift-skip and /// surface it honestly. Backends call this INSTEAD of removing the /// uuid dir when [`Self::drift_skipped`] is true: deleting it would be @@ -726,7 +748,7 @@ impl RevertOutcome { "vendor_artifact_kept", format!( "kept {uuid_dir_rel}: some recorded lock entries were left alone (see the \ - vendor_lock_entry_drifted warnings) and the vendored artifacts may still be \ + vendor_lock_entry_drifted / vendor_lock_entry_removed warnings) and the vendored artifacts may still be \ needed for a later restore; undo the drift (restore the vendored lock entries \ or re-vendor) and re-run `vendor --revert` to finish cleaning up" ), diff --git a/crates/socket-patch-core/src/vendor/npm_flavor.rs b/crates/socket-patch-core/src/vendor/npm_flavor.rs index 667b6ff51..39c9378b7 100644 --- a/crates/socket-patch-core/src/vendor/npm_flavor.rs +++ b/crates/socket-patch-core/src/vendor/npm_flavor.rs @@ -654,6 +654,88 @@ pub(super) async fn lock_text_mentions_uuid( any_readable.then_some(false) } +/// The keep gate for recorded lock entries that VANISHED during a revert +/// ([`RevertOutcome::lock_entry_removed`], #665). The user removed the +/// dependency, so there was nothing to restore; the artifact can go only +/// once [`uuid_proven_unreferenced`] shows no file in `names` resolves +/// through it any more. Anything short of that proof (the entry moved to a +/// key the wiring never recorded, a lock that cannot be read) keeps the +/// artifact exactly like a drift-skip, since it may be the only copy an +/// install needs. Returns true when the artifact was kept and the caller +/// must stop before deleting it. +pub(super) async fn keep_artifact_while_lock_references_it( + outcome: &mut RevertOutcome, + project_root: &Path, + names: &[&str], + uuid: &str, + uuid_dir_rel: &str, +) -> bool { + if !outcome.lock_entry_removed() { + return false; + } + if uuid_proven_unreferenced(project_root, names, uuid).await { + return false; + } + outcome.keep_artifact(uuid_dir_rel); + true +} + +/// True only when at least one file in `names` exists, every existing one +/// was read, and none of them references `uuid`. Stricter than +/// [`lock_text_mentions_uuid`] because `true` here deletes the artifact: +/// +/// - Only a missing file (`NotFound`) counts as absent. A lock that exists +/// but cannot be read (permissions, invalid UTF-8, not a regular file) +/// may be the one an install resolves through, so it fails closed. +/// - The match is on the uuid alone, ASCII case-insensitively, so any +/// spelling of the artifact path counts (`\/`-escaped JSON slashes, a +/// backslash separator, a different case on a case-insensitive disk). +/// - JSON files are also parsed and every key and string value checked, +/// which decodes `\u` escapes. A file that might hide the uuid behind an +/// escape the raw scan cannot see (a `\u`/`\x`/`\U` sequence in YAML, +/// yarn.lock or JSON that does not parse) fails closed. +async fn uuid_proven_unreferenced(project_root: &Path, names: &[&str], uuid: &str) -> bool { + let mut any_present = false; + for name in names { + match read_regular_to_string(&project_root.join(name)).await { + Ok(text) => { + if text_may_reference_uuid(name, &text, uuid) { + return false; + } + any_present = true; + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(_) => return false, + } + } + any_present +} + +fn text_may_reference_uuid(name: &str, text: &str, uuid: &str) -> bool { + let needle = uuid.to_ascii_lowercase(); + if text.to_ascii_lowercase().contains(&needle) { + return true; + } + if name.ends_with(".json") { + if let Ok(value) = serde_json::from_str::(text) { + return json_mentions(&value, &needle); + } + } + ["\\u", "\\x", "\\U"].iter().any(|esc| text.contains(esc)) +} + +fn json_mentions(value: &serde_json::Value, needle: &str) -> bool { + let hit = |s: &str| s.to_ascii_lowercase().contains(needle); + match value { + serde_json::Value::String(s) => hit(s), + serde_json::Value::Array(items) => items.iter().any(|v| json_mentions(v, needle)), + serde_json::Value::Object(map) => { + map.iter().any(|(k, v)| hit(k) || json_mentions(v, needle)) + } + _ => false, + } +} + /// Does this build have a backend for an npm entry's recorded flavor? /// `None` is a pre-flavor (package-lock) ledger. An unknown flavor was /// wired by a newer socket-patch, so health checks and rebuilds must not diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index b1911ba90..4beee71ff 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -780,6 +780,18 @@ pub async fn revert_npm_opts( return outcome; } + if super::npm_flavor::keep_artifact_while_lock_references_it( + &mut outcome, + project_root, + &[SHRINKWRAP, PACKAGE_LOCK], + &entry.uuid, + &uuid_dir_rel, + ) + .await + { + return outcome; + } + // FAIL-CLOSED (same brick class as the unwired guard above): the // restore only rewrites the lock files the wiring names, but the lock // npm actually installs from can still resolve through the artifact — @@ -1116,8 +1128,11 @@ fn revert_one_record( } }; let Some(live) = live else { + // REMOVED, not drifted (#665): `npm uninstall` dropped the entry. + // The caller keeps the artifact only while a lock still resolves + // through it. warnings.push(VendorWarning::new( - "vendor_lock_entry_drifted", + super::LOCK_ENTRY_REMOVED_CODE, format!("lock entry `{key}` no longer exists; nothing to restore"), )); return; @@ -3185,6 +3200,164 @@ mod tests { ); } + /// #665: the user dropped the patched dependency (`npm uninstall + /// left-pad`), so npm deleted every lock entry the wiring recorded and + /// nothing in the lock resolves through the artifact any more. That is + /// not a third-party re-resolution to protect: the revert must succeed, + /// remove the artifact (so the CLI drops the ledger entry) and say why, + /// instead of drift-keeping it forever. + #[tokio::test] + async fn revert_after_dependency_removed_drops_the_unreferenced_artifact() { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let mut live = fx.read_lock().await; + let packages = live["packages"].as_object_mut().unwrap(); + packages.remove("node_modules/left-pad"); + packages.remove("node_modules/foo/node_modules/left-pad"); + packages[""]["dependencies"] = json!({}); + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + let uninstalled = tokio::fs::read(fx.lock_path()).await.unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!(!outcome.kept_artifact, "{:?}", outcome.warnings); + assert!( + outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lock_entry_removed" + && w.detail.contains("node_modules/left-pad")), + "the vanished entry is still surfaced: {:?}", + outcome.warnings + ); + assert!( + !fx.root() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists(), + "nothing resolves through the artifact, so it is removed" + ); + assert_eq!( + tokio::fs::read(fx.lock_path()).await.unwrap(), + uninstalled, + "the user's post-uninstall lock is left byte-identical" + ); + } + + /// #665 guard: a recorded entry vanished but the lock still resolves + /// through the artifact under a key the wiring never recorded (npm + /// re-hoisted it). The artifact may be the only copy that install + /// needs, so it is kept, exactly like a drift-skip. + #[tokio::test] + async fn revert_keeps_artifact_when_a_vanished_entry_moved_to_an_unrecorded_key() { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let mut live = fx.read_lock().await; + let packages = live["packages"].as_object_mut().unwrap(); + let wired = packages.remove("node_modules/left-pad").unwrap(); + packages.remove("node_modules/foo/node_modules/left-pad"); + packages.insert("node_modules/bar/node_modules/left-pad".into(), wired); + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.kept_artifact, "{:?}", outcome.warnings); + assert!( + outcome + .warnings + .iter() + .any(|w| w.code == "vendor_artifact_kept"), + "{:?}", + outcome.warnings + ); + assert!( + fx.root() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists(), + "the lock still resolves through the artifact" + ); + } + + /// #665 guard, escaped spelling: the moved entry's resolution is + /// written with JSON-escaped slashes (`.socket\/vendor\/npm\/...`), + /// which parses to the same path npm installs from. A literal-path scan + /// misses it, so the gate matches the uuid itself. + #[tokio::test] + async fn revert_keeps_artifact_when_a_moved_entry_uses_escaped_slashes() { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let mut live = fx.read_lock().await; + let packages = live["packages"].as_object_mut().unwrap(); + let wired = packages.remove("node_modules/left-pad").unwrap(); + packages.remove("node_modules/foo/node_modules/left-pad"); + packages.insert("node_modules/bar/node_modules/left-pad".into(), wired); + let text = String::from_utf8(serialize_json(&live, " ").unwrap()) + .unwrap() + .replace(".socket/vendor/npm/", ".socket\\/vendor\\/npm\\/"); + assert!(!text.contains(".socket/vendor/npm/"), "{text}"); + tokio::fs::write(fx.lock_path(), text).await.unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.kept_artifact, "{:?}", outcome.warnings); + assert!( + fx.root().join(fx.expected_rel_tgz()).exists(), + "the escaped lock still resolves through the artifact" + ); + } + + /// #665 guard, unreadable alternate lock: package-lock.json no longer + /// has the entry, but an npm-shrinkwrap.json that exists and cannot be + /// read may be the lock an install resolves through. One readable, + /// clean lock is not proof of absence, so the artifact is kept. + #[tokio::test] + async fn revert_keeps_artifact_when_an_alternate_lock_is_unreadable() { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let mut live = fx.read_lock().await; + let packages = live["packages"].as_object_mut().unwrap(); + let wired = packages.remove("node_modules/left-pad").unwrap(); + packages.remove("node_modules/foo/node_modules/left-pad"); + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + // Invalid UTF-8 makes the shrinkwrap unreadable even as root, where + // a permission bit would not. + let mut shrinkwrap = b"\xff".to_vec(); + shrinkwrap.extend(serde_json::to_vec(&wired).unwrap()); + tokio::fs::write(fx.root().join(SHRINKWRAP), shrinkwrap) + .await + .unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.kept_artifact, "{:?}", outcome.warnings); + assert!( + fx.root().join(fx.expected_rel_tgz()).exists(), + "an unreadable lock is not proof the artifact is unused" + ); + + // Once the shrinkwrap is gone, the same revert reclaims it. + tokio::fs::remove_file(fx.root().join(SHRINKWRAP)) + .await + .unwrap(); + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(!outcome.kept_artifact, "{:?}", outcome.warnings); + assert!(!fx.root().join(fx.expected_rel_tgz()).exists()); + } + /// The WIRED revert (non-empty wiring) fails closed on an unparseable /// lock — editing JSON we cannot parse risks destroying it. (The /// unreadable-lock test below covers only the EMPTY-wiring guard.) diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock.rs b/crates/socket-patch-core/src/vendor/pnpm_lock.rs index 1b08ba52d..d35a8e7be 100644 --- a/crates/socket-patch-core/src/vendor/pnpm_lock.rs +++ b/crates/socket-patch-core/src/vendor/pnpm_lock.rs @@ -999,6 +999,17 @@ pub(super) async fn revert_pnpm_dialect( // ran; the artifact dir stays behind (and the caller keeps the ledger // entry), so only the deletion is skipped. if !keep_artifact { + if super::npm_flavor::keep_artifact_while_lock_references_it( + &mut outcome, + project_root, + &[PNPM_LOCK, PACKAGE_JSON, PNPM_WORKSPACE], + &entry.uuid, + &uuid_dir_rel, + ) + .await + { + return outcome; + } // The last npm-family entry leaves `.socket/vendor/npm/` (and // `.socket/vendor/`) empty: the shared helper prunes them so a // reverted project carries no vendor residue (non-recursive: @@ -3108,7 +3119,7 @@ fn revert_importer_dep( } break; } - warnings.push(drifted(format!( + warnings.push(removed(format!( "importer dep `{key}` no longer exists; nothing to restore" ))); } @@ -3187,7 +3198,7 @@ fn revert_block( j = block.end; } } - warnings.push(drifted(format!( + warnings.push(removed(format!( "{section} entry `{new_key}` no longer exists; nothing to restore" ))); } @@ -3251,7 +3262,7 @@ fn revert_snapshot_ref( } break; } - warnings.push(drifted(format!( + warnings.push(removed(format!( "snapshot ref `{key}` no longer exists; nothing to restore" ))); } @@ -3260,6 +3271,13 @@ pub(super) fn drifted(detail: impl Into) -> VendorWarning { VendorWarning::new("vendor_lock_entry_drifted", detail.into()) } +/// A recorded lock entry that no longer exists (`pnpm remove` dropped the +/// dependency). Not drift (#665): the revert keeps the artifact only while +/// a wired file still resolves through it. +pub(super) fn removed(detail: impl Into) -> VendorWarning { + VendorWarning::new(super::LOCK_ENTRY_REMOVED_CODE, detail.into()) +} + // ────────────────────────── surfaces commit + unwind ────────────────────── /// Write the override surfaces FIRST (package.json, then pnpm-workspace.yaml), @@ -6579,23 +6597,24 @@ snapshots: .await .unwrap(); + // #665: a vanished block is not drift. Once the other recorded + // fragments are restored nothing resolves through the artifact, so + // it is removed instead of kept forever. let outcome = revert_pnpm(&entry, fx.root(), false).await; assert!(outcome.success, "{:?}", outcome.error); assert_warning( &outcome.warnings, - "vendor_lock_entry_drifted", - "no longer exists; nothing to restore", - ); - assert_warning( - &outcome.warnings, - "vendor_lock_entry_drifted", + "vendor_lock_entry_removed", "packages entry `left-pad@file:", ); - assert!(outcome.kept_artifact); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!(!outcome.kept_artifact, "{:?}", outcome.warnings); + assert!(!uuid_dir(&fx).exists(), "unreferenced artifact removed"); } - /// Snapshot dep refs: the re-resolved (foreign value) arm and the - /// line-vanished arm both warn and keep. + /// Snapshot dep refs: the re-resolved (foreign value) arm warns and + /// keeps; the line-vanished arm warns `vendor_lock_entry_removed` and, + /// with nothing left resolving through the artifact, removes it (#665). #[tokio::test] async fn snapshot_ref_drift_and_vanished_arms_warn() { // Re-resolved behind our back. @@ -6636,13 +6655,16 @@ snapshots: assert!(outcome.success, "{:?}", outcome.error); assert_warning( &outcome.warnings, - "vendor_lock_entry_drifted", + "vendor_lock_entry_removed", "snapshot ref `consumer@file:consumer|left-pad` no longer exists; nothing to restore", ); - assert!(outcome.kept_artifact); + assert!(!outcome.kept_artifact, "{:?}", outcome.warnings); + assert!(!uuid_dir(&fx).exists(), "unreferenced artifact removed"); } - /// The whole importer dep entry deleted since vendoring: warn + keep. + /// The whole importer dep entry deleted since vendoring: warned as + /// removed, and the artifact goes once nothing resolves through it + /// (#665). #[tokio::test] async fn vanished_importer_dep_entry_warns_nothing_to_restore() { let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; @@ -6665,10 +6687,11 @@ snapshots: assert!(outcome.success, "{:?}", outcome.error); assert_warning( &outcome.warnings, - "vendor_lock_entry_drifted", + "vendor_lock_entry_removed", "importer dep `.|left-pad` no longer exists; nothing to restore", ); - assert!(outcome.kept_artifact); + assert!(!outcome.kept_artifact, "{:?}", outcome.warnings); + assert!(!uuid_dir(&fx).exists(), "unreferenced artifact removed"); } /// The user hand-restored the importer dep to its pre-vendor pair before @@ -7973,7 +7996,7 @@ snapshots: &mut warnings, ); assert!(!dirty); - assert_warning(&warnings, "vendor_lock_entry_drifted", "no longer exists"); + assert_warning(&warnings, "vendor_lock_entry_removed", "no longer exists"); } /// A record whose `original` lost its `specifier`/`version` fields @@ -8015,8 +8038,9 @@ snapshots: } /// A rekeyed block that vanished, where the recorded original ALSO - /// matches no live block, is drift ("no longer exists") — the converged - /// silent return applies only when the original block is live verbatim. + /// matches no live block, is warned as removed ("no longer exists") — + /// the converged silent return applies only when the original block is + /// live verbatim. #[test] fn packages_block_revert_with_no_live_or_original_match_warns_vanished() { let mut lines = @@ -8047,10 +8071,10 @@ snapshots: &mut warnings, ); assert!(!dirty); - assert_warning(&warnings, "vendor_lock_entry_drifted", "no longer exists"); + assert_warning(&warnings, "vendor_lock_entry_removed", "no longer exists"); // Same vanish with NO recorded original (a stripped/reconstructed - // record): the converged probe is skipped — still the same drift + // record): the converged probe is skipped — still the same removed // warning, never a silent pass. let rec = WiringRecord { original: None, @@ -8068,7 +8092,7 @@ snapshots: &mut warnings, ); assert!(!dirty); - assert_warning(&warnings, "vendor_lock_entry_drifted", "no longer exists"); + assert_warning(&warnings, "vendor_lock_entry_removed", "no longer exists"); } /// The unwind helper restores exactly what was written: with no diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs b/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs index ca815da75..21e68f6d1 100644 --- a/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs +++ b/crates/socket-patch-core/src/vendor/pnpm_lock_legacy.rs @@ -64,8 +64,8 @@ use crate::utils::fs::read_regular_to_string; use super::common::refused; use super::path::parse_vendor_path; use super::pnpm_lock::{ - dep_field_lines, drifted, lines_value, revert_overrides_line, value_lines, vendor_value_is_for, - EditCtx, PnpmDialect, KIND_LOCK_OVERRIDES, KIND_LOCK_PACKAGE, + dep_field_lines, drifted, lines_value, removed, revert_overrides_line, value_lines, + vendor_value_is_for, EditCtx, PnpmDialect, KIND_LOCK_OVERRIDES, KIND_LOCK_PACKAGE, }; use super::source::PackageSource; use super::state::{VendorEntry, WiringAction, WiringRecord}; @@ -986,7 +986,7 @@ fn revert_value_line( *dirty = true; return; } - warnings.push(drifted(format!( + warnings.push(removed(format!( "{section} entry `{dep}` no longer exists; nothing to restore" ))); } @@ -1069,7 +1069,7 @@ fn revert_root_dep_pair( *dirty = true; return; } - warnings.push(drifted(format!( + warnings.push(removed(format!( "root dep `{key}` no longer exists; nothing to restore" ))); } @@ -1156,7 +1156,7 @@ fn revert_package_block( j = block.end; } } - warnings.push(drifted(format!( + warnings.push(removed(format!( "packages entry `{new_key}` no longer exists; nothing to restore" ))); } @@ -1229,7 +1229,7 @@ fn revert_pkg_dep_ref( } break; } - warnings.push(drifted(format!( + warnings.push(removed(format!( "dep ref `{key}` no longer exists; nothing to restore" ))); } @@ -2782,6 +2782,47 @@ packages: ); } + /// Revert and assert the #665 removed-entry contract: success, no + /// drift, one `vendor_lock_entry_removed` warning containing `want`, + /// and the artifact kept exactly while the lock still resolves through + /// it (`still_referenced`). A removed artifact is put back afterwards + /// so the caller's next matrix case starts from the vendored state. + async fn assert_removed(fx: &Fixture, entry: &VendorEntry, want: &str, still_referenced: bool) { + let uuid_dir = fx.root().join(fx.rel_tgz()).parent().unwrap().to_path_buf(); + let mut saved = Vec::new(); + for file in std::fs::read_dir(&uuid_dir).unwrap() { + let path = file.unwrap().path(); + saved.push((path.clone(), std::fs::read(&path).unwrap())); + } + + let outcome = revert_pnpm_legacy(entry, fx.root(), false).await; + assert!(outcome.success, "{want}: {:?}", outcome.error); + assert!(!outcome.drift_skipped(), "{want}: {:?}", outcome.warnings); + assert!( + outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lock_entry_removed" && w.detail.contains(want)), + "expected a removed warning containing `{want}`: {:?}", + outcome.warnings + ); + assert_eq!( + outcome.kept_artifact, still_referenced, + "{want}: {:?}", + outcome.warnings + ); + assert_eq!( + fx.root().join(fx.rel_tgz()).exists(), + still_referenced, + "{want}: the artifact goes once nothing resolves through it" + ); + + std::fs::create_dir_all(&uuid_dir).unwrap(); + for (path, bytes) in saved { + std::fs::write(path, bytes).unwrap(); + } + } + /// A dry run previews and records NOTHING through the legacy entry /// point, and a missing patch-target file fails the vendor before any /// pack — both land in the `staged == None` early return with the @@ -3344,8 +3385,9 @@ packages: } /// v5.4 drift matrix: third-party re-resolutions are left alone with ONE - /// precise warning each, vanished fragments/sections warn, and every - /// case keeps the artifact. + /// precise warning each and keep the artifact; vanished sections warn + /// and keep; a vanished fragment warns as removed and the artifact goes + /// once nothing resolves through it (#665). #[tokio::test] async fn reresolved_and_vanished_fragments_drift_keep_v54() { let (fx, entry) = vendored(T7_BEFORE_LOCK).await; @@ -3381,11 +3423,11 @@ packages: tokio::fs::write(fx.root().join(PNPM_LOCK), &tampered) .await .unwrap(); - assert_drift_keep( + assert_removed( &fx, &entry, "specifiers entry `left-pad` no longer exists", - 1, + false, ) .await; rewire(&fx, &wired_pkg, &wired).await; @@ -3412,11 +3454,11 @@ packages: tokio::fs::write(fx.root().join(PNPM_LOCK), &tampered) .await .unwrap(); - assert_drift_keep( + assert_removed( &fx, &entry, "dep ref `file:consumer|left-pad` no longer exists", - 1, + false, ) .await; rewire(&fx, &wired_pkg, &wired).await; @@ -3430,7 +3472,7 @@ packages: .await .unwrap(); let want_block_gone = format!("packages entry `file:{rel}` no longer exists"); - assert_drift_keep(&fx, &entry, &want_block_gone, 1).await; + assert_removed(&fx, &entry, &want_block_gone, false).await; rewire(&fx, &wired_pkg, &wired).await; // (f) the whole specifiers section is gone. @@ -3482,11 +3524,13 @@ packages: tokio::fs::write(fx.root().join(PNPM_LOCK), &tampered) .await .unwrap(); - assert_drift_keep( + // The surviving `specifier:` line still names the artifact, so it + // is kept. + assert_removed( &fx, &entry, "root dep `dependencies|left-pad` no longer exists", - 1, + true, ) .await; rewire(&fx, &wired_pkg, &wired).await; @@ -4350,7 +4394,8 @@ packages: /// Our rekeyed block gone (user re-locked) AND the record's original /// stripped (corrupted ledger): with neither the live block nor a /// converged original to anchor on, the record warns `no longer exists` - /// and the artifact is kept. + /// (removed), and with nothing left resolving through the artifact it is + /// removed (#665). #[tokio::test] async fn missing_block_with_no_recorded_original_warns_no_longer_exists() { let (fx, mut entry) = vendored(T7_BEFORE_LOCK).await; @@ -4369,11 +4414,11 @@ packages: .find(|r| r.kind == KIND_LOCK_PACKAGE) .unwrap(); rec.original = None; - assert_drift_keep( + assert_removed( &fx, &entry, &format!("packages entry `file:{rel}` no longer exists"), - 1, + false, ) .await; } diff --git a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs index 5721d2a51..c5669f77f 100644 --- a/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_berry_lock.rs @@ -857,6 +857,18 @@ pub async fn revert_yarn_berry_opts( return outcome; } + if super::npm_flavor::keep_artifact_while_lock_references_it( + &mut outcome, + project_root, + &[YARN_LOCK, PACKAGE_JSON], + &entry.uuid, + &uuid_dir_rel, + ) + .await + { + return outcome; + } + // FAIL-CLOSED (same brick class as the unwired guard above, twin of // npm_lock's post-restore probe): the restore only rewrites the // fragments the wiring recorded, but yarn can still resolve through the diff --git a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs index 70602238b..fce6985e5 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -514,6 +514,18 @@ pub async fn revert_yarn_classic_opts( return outcome; } + if super::npm_flavor::keep_artifact_while_lock_references_it( + &mut outcome, + project_root, + &[YARN_LOCK], + &entry.uuid, + &uuid_dir_rel, + ) + .await + { + return outcome; + } + // FAIL-CLOSED (same brick class as the unwired guard above, twin of // npm_lock's and yarn_berry's post-restore probes): the restore only // rewrites the blocks the wiring recorded, but yarn can still resolve @@ -602,8 +614,11 @@ pub(super) fn revert_recorded_block( return false; } } + // REMOVED, not drifted (#665): the user dropped the dependency. + // The caller keeps the artifact only while the lock still + // resolves through it. warnings.push(VendorWarning::new( - "vendor_lock_entry_drifted", + super::LOCK_ENTRY_REMOVED_CODE, format!("{noun} `{key}` no longer exists; nothing to restore"), )); return false; @@ -1959,6 +1974,75 @@ left-pad@^1.3.0: ); } + /// #665: `yarn remove left-pad` deleted the vendored block, so nothing + /// in yarn.lock resolves through the artifact any more. A vanished + /// block is not a re-resolution to protect: the revert must succeed and + /// remove the artifact (letting rollback / `remove` / `scan --prune` + /// drop the ledger entry), instead of drift-keeping it forever. + #[tokio::test] + async fn revert_after_yarn_remove_drops_the_unreferenced_artifact() { + let fx = fixture_with_lock(Y2_BEFORE).await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + // What `yarn remove left-pad` leaves behind: the other dependency's + // block only. + let removed = "# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.\n\ + # yarn lockfile v1\n\n\n\ + is-number@7.0.0:\n version \"7.0.0\"\n \ + resolved \"https://registry.yarnpkg.com/is-number/-/is-number-7.0.0.tgz\"\n"; + tokio::fs::write(fx.lock_path(), removed).await.unwrap(); + + for _ in 0..2 { + let outcome = revert_yarn_classic(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!(!outcome.kept_artifact, "{:?}", outcome.warnings); + assert!( + !fx.root() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists(), + "nothing resolves through the artifact, so it is removed" + ); + assert_eq!( + fx.lock_text().await, + removed, + "the user's lock is untouched" + ); + } + let outcome = revert_yarn_classic(&entry, fx.root(), false).await; + assert!( + outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lock_entry_removed"), + "the vanished block is surfaced: {:?}", + outcome.warnings + ); + } + + /// #665 guard: the recorded block vanished, but another (hand-copied or + /// re-keyed) block still resolves through the artifact. Deleting it + /// would break that install, so the artifact is kept, as for drift. + #[tokio::test] + async fn revert_keeps_artifact_when_a_vanished_block_was_rekeyed() { + let fx = fixture_with_lock(Y2_BEFORE).await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let text = fx + .lock_text() + .await + .replace("left-pad@^1.3.0:", "left-pad@1.3.0:"); + tokio::fs::write(fx.lock_path(), &text).await.unwrap(); + + let outcome = revert_yarn_classic(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.kept_artifact, "{:?}", outcome.warnings); + assert!(fx.tgz_path().exists(), "the re-keyed block still needs it"); + assert_eq!(fx.lock_text().await, text); + } + #[tokio::test] async fn revert_allowlist_fails_closed_on_foreign_files() { let fx = fixture_with_lock(Y2_BEFORE).await; @@ -2523,7 +2607,8 @@ left-pad@^1.3.0: /// Direct matrix over [`revert_recorded_block`]'s degradation arms /// (shared with the berry backend): every poisoned-or-newer-ledger - /// record must degrade to a `vendor_lock_entry_drifted` warning and + /// record must degrade to a `vendor_lock_entry_drifted` warning (a + /// vanished block to `vendor_lock_entry_removed`) and /// leave the text byte-untouched, the converged-under-a-different-key /// arm must stay silent, and (control) a well-formed record restores. #[test] @@ -2587,7 +2672,9 @@ left-pad@^1.3.0: assert_drift_warning(&warnings, "unknown wiring kind `future_kind`"); // (c) Recorded key gone AND the original block not live anywhere: - // deleted-since-vendoring drift. + // the dependency was removed since vendoring. Not drift (#665): it + // warns `vendor_lock_entry_removed`, and the caller keeps the + // artifact only while the lock still resolves through it. let absent = vec!["absent@^9:".to_string(), " version \"9.0.0\"".to_string()]; let rec = wrec( Some("absent@^9"), @@ -2597,7 +2684,15 @@ left-pad@^1.3.0: let (changed, text, warnings) = run(Y2_AFTER, &rec); assert!(!changed); assert_eq!(text, Y2_AFTER); - assert_drift_warning(&warnings, "no longer exists; nothing to restore"); + assert_eq!(warnings.len(), 1, "{warnings:?}"); + assert_eq!(warnings[0].code, super::super::LOCK_ENTRY_REMOVED_CODE); + assert!( + warnings[0] + .detail + .contains("no longer exists; nothing to restore"), + "{}", + warnings[0].detail + ); // (d) ALREADY CONVERGED under a different key: the recorded key is // gone but the original block is live verbatim — a silent no-op so