diff --git a/crates/socket-patch-cli/tests/e2e_bun_lockb.rs b/crates/socket-patch-cli/tests/e2e_bun_lockb.rs index 60f369c1e..3e6d41a25 100644 --- a/crates/socket-patch-cli/tests/e2e_bun_lockb.rs +++ b/crates/socket-patch-cli/tests/e2e_bun_lockb.rs @@ -997,6 +997,47 @@ async fn native_binary_scan_vendored() { fixture.frozen("reverted", &fixture.original, "minimist"); } +/// #1132: `bun remove minimist` after a vendored scan leaves neither the +/// vendored nor the pre-vendor record in bun.lockb. `vendor --revert` must +/// retire the entry and delete its tarball; reading the removal as drift +/// kept both forever and left `vendor --check` red. Named outside the +/// `native_binary_` prefix: `scripts/backtest-bun-lockb.py` pins that set. +#[tokio::test(flavor = "multi_thread")] +#[serial_test::serial] +async fn binary_vendored_revert_after_bun_remove() { + let Some(fixture) = Fixture::new("direct") else { + return; + }; + let server = MockServer::start().await; + mock_api(&server, &fixture, "minimist").await; + let result = scan(&fixture.project, &server, "vendored", &[]); + assert_eq!( + result["vendor"]["summary"]["applied"], 1, + "scan vendored: {result}" + ); + let mut remove = command(&fixture.reader, &fixture.project); + remove + .args(["remove", "minimist", "--ignore-scripts"]) + .env( + "BUN_INSTALL_CACHE_DIR", + fixture.temp.path().join("remove-cache"), + ) + .env("BUN_INSTALL", fixture.temp.path().join("remove-home")); + require_success(remove.output().unwrap(), "bun remove minimist"); + assert!(!fixture.project.join("bun.lock").exists()); + + let result = cli(&fixture.project, &["vendor", "--revert"]); + assert_eq!(result["status"], "success", "vendor revert: {result}"); + assert!( + !result.to_string().contains("vendor_lock_entry_drifted"), + "a removed dependency is not drift: {result}" + ); + assert!( + !fixture.project.join(".socket/vendor").exists(), + "the vendored tarball must be deleted: {result}" + ); +} + #[tokio::test(flavor = "multi_thread")] #[serial_test::serial] async fn native_binary_alias_and_transitive() { diff --git a/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs index ac9f700fa..d99991947 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs @@ -954,6 +954,15 @@ fn uv_vendor_revert_sub_table_sources() { /// the pair (#806, #821). Before the fix the pyproject side reverted alone /// and `uv sync --locked` failed while revert reported success. fn uv_relock_then_revert(tag: &str, pyproject: &str, relock: &[&str]) { + uv_relock_then_unwind(tag, pyproject, relock, true); +} + +/// The shared body of [`uv_relock_then_revert`]. `keeps_vendored` is false +/// for a relock that drops the dependency altogether (`uv remove six`, +/// #1140): uv deletes every fragment that routed through the wheel, so the +/// revert has nothing to restore, and must still converge instead of +/// drift-keeping the entry (which kept `vendor --check` red forever). +fn uv_relock_then_unwind(tag: &str, pyproject: &str, relock: &[&str], keeps_vendored: bool) { let Some((uv, python)) = capstone_uv(tag) else { return; }; @@ -1000,9 +1009,11 @@ fn uv_relock_then_revert(tag: &str, pyproject: &str, relock: &[&str]) { return; } let relocked = std::fs::read_to_string(proj.join("uv.lock")).unwrap(); - assert!( + assert_eq!( relocked.contains(".socket/vendor/pypi/"), - "the relock must keep six vendored: {relocked}" + keeps_vendored, + "the relock must {} six vendored: {relocked}", + if keeps_vendored { "keep" } else { "drop" } ); let (code, stdout, stderr) = run_socket( @@ -1040,6 +1051,21 @@ fn uv_relock_then_revert(tag: &str, pyproject: &str, relock: &[&str]) { ); } +/// #1140: `uv remove six` after vendoring drops the dependency, its +/// `[tool.uv.sources]` line and every uv.lock fragment that pointed at the +/// wheel. `vendor --revert` must retire the entry and delete the wheel +/// instead of keeping both as drift. +#[test] +#[serial_test::serial] +fn uv_vendor_revert_after_uv_remove() { + uv_relock_then_unwind( + "uv-remove", + "[project]\nname = \"vendor-capstone\"\nversion = \"0.1.0\"\nrequires-python = \">=3.9\"\ndependencies = [\"six==1.16.0\", \"attrs>=20\"]\n", + &["remove", "-q", "six"], + false, + ); +} + /// #821: six in a PEP 735 dev group, then `uv add --dev zipp` rewrites the /// whole `requires-dev` group line. #[test] diff --git a/crates/socket-patch-core/src/vendor/bun_binary.rs b/crates/socket-patch-core/src/vendor/bun_binary.rs index 406dabc53..4674b3b80 100644 --- a/crates/socket-patch-core/src/vendor/bun_binary.rs +++ b/crates/socket-patch-core/src/vendor/bun_binary.rs @@ -451,6 +451,17 @@ pub(crate) async fn revert(entry: &VendorEntry, root: &Path, opts: RevertOpts) - } } } + // REMOVED, not drift (#1132): `bun remove ` (or an upgrade off the + // patched version) leaves neither snapshot in the lock. When no package + // resolves through this uuid dir any more, there is nothing to restore + // and nothing an install needs the artifact for. Probed once, before any + // record is restored; an unreadable package table fails closed (drift). + let uuid_lower = entry.uuid.to_ascii_lowercase(); + let unreferenced = lock.packages().is_ok_and(|packages| { + !packages + .iter() + .any(|p| p.resolution.to_ascii_lowercase().contains(&uuid_lower)) + }); let mut mirrors_to_remove = Vec::new(); for rec in entry.wiring.iter().rev() { if rec.kind == MIRROR_KIND { @@ -500,6 +511,7 @@ pub(crate) async fn revert(entry: &VendorEntry, root: &Path, opts: RevertOpts) - } continue; } + // `Ok(true)`: the record left the lock (see `unreferenced`). let restore = (|| { if rec.file != LOCK || rec.kind != KIND { return Err("unexpected binary wiring file or kind".to_string()); @@ -522,15 +534,25 @@ pub(crate) async fn revert(entry: &VendorEntry, root: &Path, opts: RevertOpts) - // original never makes this record appear already reverted. let id = match lock.find_snapshot_id(id, new)? { Some(id) => id, - None if lock.find_snapshot_id(id, original)?.is_some() => return Ok(()), + None if lock.find_snapshot_id(id, original)?.is_some() => return Ok(false), + None if unreferenced => return Ok(true), None => return Err("binary package resolution has drifted".into()), }; - lock.restore(id, original) + lock.restore(id, original).map(|()| false) })(); - if let Err(e) = restore { - outcome + match restore { + Ok(true) => outcome.warnings.push(VendorWarning::new( + super::LOCK_ENTRY_REMOVED_CODE, + format!( + "{LOCK} no longer resolves {} through {dir} (the dependency was removed or \ + re-resolved); nothing to restore", + entry.base_purl + ), + )), + Ok(false) => {} + Err(e) => outcome .warnings - .push(VendorWarning::new("vendor_lock_entry_drifted", e)); + .push(VendorWarning::new("vendor_lock_entry_drifted", e)), } } if outcome.drift_skipped() { @@ -1014,6 +1036,58 @@ mod rebuild_tests { assert_eq!(ts::request_count(&server).await, 0); } + /// #1132: once `bun remove minimist` (or `bun add minimist@1.2.8`) + /// moves the vendored record off its vendored resolution, neither the + /// rewritten nor the pre-vendor snapshot is in bun.lockb and nothing + /// resolves through the uuid dir. There is nothing to restore, so the + /// revert warns `vendor_lock_entry_removed` and finishes; reading it as + /// drift kept the tarball and ledger entry forever and looped + /// `vendor --check` → `scan --prune`. + #[tokio::test] + async fn revert_after_package_left_the_lock_is_not_drift() { + use base64::Engine as _; + let fx = flip_fixture().await; + let root = fx.root(); + let VendorOutcome::Done { + result, + entry: Some(entry), + .. + } = flip_run(&fx, None).await + else { + panic!("vendoring must wire the binary lock"); + }; + assert!(result.success, "{result:?}"); + let binary = entry.wiring.iter().find(|r| r.kind == KIND).unwrap(); + let id: usize = binary.key.as_ref().unwrap().parse().unwrap(); + let mut lock = BunLockb::parse(&std::fs::read(root.join(LOCK)).unwrap()).unwrap(); + let integrity = format!( + "sha512-{}", + base64::engine::general_purpose::STANDARD.encode([7u8; 64]) + ); + lock.set_registry_package( + id, + "1.2.8", + "https://registry.npmjs.org/minimist/-/minimist-1.2.8.tgz", + &integrity, + ) + .unwrap(); + let relocked = lock.bytes(); + assert!(!lock + .packages() + .unwrap() + .iter() + .any(|p| p.resolution.contains(UUID))); + std::fs::write(root.join(LOCK), &relocked).unwrap(); + + let outcome = revert(&entry, root, RevertOpts::new(false)).await; + assert!(outcome.success, "{outcome:?}"); + assert!(!outcome.drift_skipped(), "{outcome:?}"); + assert!(outcome.lock_entry_removed(), "{outcome:?}"); + assert!(!outcome.kept_artifact, "{outcome:?}"); + assert_eq!(std::fs::read(root.join(LOCK)).unwrap(), relocked); + assert!(!root.join(&entry.artifact.path).exists()); + } + /// With the canonical tarball GONE, an outage switches the same UUID /// from a prebuilt archive to a locally packed one. Different archive /// bytes must advance the integrity snapshot without losing the pristine diff --git a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs index cbb38c31c..729ca4f7c 100644 --- a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs +++ b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs @@ -597,6 +597,29 @@ pub(super) async fn revert_pipenv( warnings.push(drifted()); continue; } + // A relock dropped the entry (`pipenv uninstall `, a Pipfile + // edit + `pipenv lock`), or, on Pipenv 2022/2023, the whole category + // it emptied (#1142): the vendored reference is gone with it, so + // retire the record rather than keep the orphan forever. Only while + // no other entry still routes through this uuid dir — that one + // would need the artifact, so it stays drift. + let relocked_away = |lock: &Value| { + if rec.action == WiringAction::Rewritten + && rec.original.is_some() + && !to_canonical_json(lock).contains(&entry.uuid) + { + VendorWarning::new( + "vendor_lock_entry_relocked", + format!("{LOCK_FILE} entry for {:?} was removed by a relock; the vendored reference is already gone, so the record is retired", rec.key), + ) + } else { + drifted() + } + }; + if lock.get(section).is_none() { + warnings.push(relocked_away(&lock)); + continue; + } let Some(map) = lock.get_mut(section).and_then(Value::as_object_mut) else { warnings.push(drifted()); continue; @@ -615,17 +638,7 @@ pub(super) async fn revert_pipenv( continue; }; let Some(live) = map.get(name) else { - if rec.action == WiringAction::Rewritten && rec.original.is_some() { - // A relock dropped the entry (`pipenv uninstall `, a Pipfile - // edit + `pipenv lock`): the vendored reference is gone with - // it — retire the record rather than keep the orphan forever. - warnings.push(VendorWarning::new( - "vendor_lock_entry_relocked", - format!("{LOCK_FILE} entry for {:?} was removed by a relock; the vendored reference is already gone, so the record is retired", rec.key), - )); - } else { - warnings.push(drifted()); - } + warnings.push(relocked_away(&lock)); continue; }; // Still OUR reference (`file`/`path` string identical to what we @@ -1801,7 +1814,7 @@ mod tests { /// successful revert with exactly one drift warning and the lock bytes /// UNCHANGED (changed=false must skip re-serialization entirely). #[tokio::test] - async fn revert_drift_skips_missing_section_entry_new_and_missing_original() { + async fn revert_retires_missing_section_entry_and_drift_skips_new_and_missing_original() { use serde_json::json; let mut no_develop: Value = serde_json::from_str(LOCK_DIRECT_REGISTRY).unwrap(); no_develop.as_object_mut().unwrap().remove("develop"); @@ -1856,9 +1869,9 @@ mod tests { let outcome = revert_pipenv(&entry_for(vec![record], meta), tmp.path(), false).await; assert!(outcome.success, "{label}: {:?}", outcome.error); assert_eq!(outcome.warnings.len(), 1, "{label}: {:?}", outcome.warnings); - // An entry a relock REMOVED retires the record (the reference is - // gone with it); every other mismatch is drift. - let expected = if label.contains("entry removed") { + // An entry or category a relock REMOVED retires the record (the + // reference is gone with it, #1142); every other mismatch is drift. + let expected = if label.contains("entry removed") || label.contains("section deleted") { "vendor_lock_entry_relocked" } else { "vendor_lock_entry_drifted" @@ -1877,6 +1890,61 @@ mod tests { } } + /// #1142: `pipenv uninstall six --categories docs` on Pipenv 2022/2023 + /// drops the emptied `docs` key from Pipfile.lock altogether (2026 keeps + /// `"docs": {}`). The vendored reference left with the category, so the + /// record retires exactly like an entry a relock removed — reading it as + /// drift kept the wheel and ledger entry forever and looped + /// `vendor --check` → `scan --prune`. A lock that still names the uuid + /// anywhere (the entry moved to another category by hand) stays drift. + #[tokio::test] + async fn revert_retires_record_whose_category_a_relock_dropped() { + let registry_six = + serde_json::from_str::(LOCK_DIRECT_REGISTRY).unwrap()["default"]["six"].clone(); + let vendored_six = + serde_json::from_str::(LOCK_DIRECT_VENDORED).unwrap()["default"]["six"].clone(); + let record = WiringRecord { + file: LOCK_FILE.to_string(), + kind: KIND_LOCK_ENTRY.to_string(), + action: WiringAction::Rewritten, + key: Some("docs:six".to_string()), + original: Some(registry_six), + new: Some(vendored_six.clone()), + }; + let meta = || PipenvMeta { + sections: vec!["docs".into()], + }; + + // The lock after the uninstall: no `docs` key, and nothing else + // mentions the vendored wheel. + let mut relocked: Value = serde_json::from_str(LOCK_DIRECT_REGISTRY).unwrap(); + relocked["default"].as_object_mut().unwrap().remove("six"); + let relocked_text = to_canonical_json(&relocked); + let tmp = write_lock(&relocked_text).await; + let outcome = + revert_pipenv(&entry_for(vec![record.clone()], meta()), tmp.path(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); + assert_eq!(outcome.warnings.len(), 1, "{:?}", outcome.warnings); + assert_eq!(outcome.warnings[0].code, "vendor_lock_entry_relocked"); + assert!(outcome.warnings[0].detail.contains("docs:six")); + assert_eq!(read_lock(tmp.path()).await, relocked_text); + + // The same record while another category still routes through the + // wheel: deleting it would break that install, so it is drift. + let mut moved = relocked.clone(); + moved["default"] + .as_object_mut() + .unwrap() + .insert("six".into(), vendored_six); + let moved_text = to_canonical_json(&moved); + let tmp = write_lock(&moved_text).await; + let outcome = revert_pipenv(&entry_for(vec![record], meta()), tmp.path(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.drift_skipped(), "{:?}", outcome.warnings); + assert_eq!(read_lock(tmp.path()).await, moved_text); + } + /// The Added arm removes a lock entry — a destructive edit driven by /// tamper-able state.json, gated ONLY by the live==rec.new deep-equality /// check. wire_pipenv never emits Added (forward-compat/defensive), so diff --git a/crates/socket-patch-core/src/vendor/pypi_uv.rs b/crates/socket-patch-core/src/vendor/pypi_uv.rs index bdf092c4a..01ec886ed 100644 --- a/crates/socket-patch-core/src/vendor/pypi_uv.rs +++ b/crates/socket-patch-core/src/vendor/pypi_uv.rs @@ -823,6 +823,33 @@ pub(super) async fn revert_uv(entry: &VendorEntry, root: &Path, dry_run: bool) - // ALREADY-CONVERGED probes below key on it (see the LIVENESS CONTRACT // on `RevertOutcome::drift_skipped`). let needle = format!(".socket/vendor/pypi/{}", entry.uuid); + // REMOVED, not drift (#1140): `uv remove ` drops the dependency + // together with every fragment that routed through the wheel. When + // neither file names this entry's uuid any more, a record whose written + // fragment carried it has nothing left to restore; it warns + // `vendor_lock_entry_removed` so the revert converges. Probed once, + // before any record is reverted, so only the user's own edits count. + let uuid_lower = entry.uuid.to_ascii_lowercase(); + let unreferenced = ![&pyproject_text, &lock_text] + .iter() + .any(|text| text.to_ascii_lowercase().contains(&uuid_lower)); + let removed = |rec: &WiringRecord| { + let carried_uuid = rec + .new + .as_ref() + .and_then(serde_json::Value::as_str) + .is_some_and(|new| new.to_ascii_lowercase().contains(&uuid_lower)); + (unreferenced && carried_uuid).then(|| { + VendorWarning::new( + super::LOCK_ENTRY_REMOVED_CODE, + format!( + "{} entry for {:?} no longer exists and nothing references {needle} any \ + more (the dependency was removed); nothing to restore", + rec.kind, rec.key + ), + ) + }) + }; for rec in entry.wiring.iter().rev() { let new_text = rec.new.as_ref().and_then(serde_json::Value::as_str); @@ -836,6 +863,10 @@ pub(super) async fn revert_uv(entry: &VendorEntry, root: &Path, dry_run: bool) - match respell_original(orig, &rec.kind, key, &pyproject_text) { Ok(text) => Some(text), Err(reason) => { + if let Some(w) = removed(rec) { + warnings.push(w); + continue; + } warnings.push(VendorWarning::new( "vendor_lock_entry_drifted", format!( @@ -870,7 +901,9 @@ pub(super) async fn revert_uv(entry: &VendorEntry, root: &Path, dry_run: bool) - { ArrayRevert::Reverted(t) => lock_text = t, ArrayRevert::Converged => {} - ArrayRevert::Drift => warnings.push(drifted("uv.lock")), + ArrayRevert::Drift => { + warnings.push(removed(rec).unwrap_or_else(|| drifted("uv.lock"))) + } } } "uv_lock_package" | "uv_lock_requires_dist" => { @@ -895,7 +928,7 @@ pub(super) async fn revert_uv(entry: &VendorEntry, root: &Path, dry_run: bool) - if original_text.is_some_and(|orig| haystack.contains(orig)) { continue; } - warnings.push(drifted("uv.lock")); + warnings.push(removed(rec).unwrap_or_else(|| drifted("uv.lock"))); } } } @@ -943,7 +976,9 @@ pub(super) async fn revert_uv(entry: &VendorEntry, root: &Path, dry_run: bool) - ) { ArrayRevert::Reverted(t) => lock_text = t, ArrayRevert::Converged => {} - ArrayRevert::Drift => warnings.push(drifted("uv.lock")), + ArrayRevert::Drift => { + warnings.push(removed(rec).unwrap_or_else(|| drifted("uv.lock"))) + } }, }, "uv_sources_entry" => { @@ -2869,6 +2904,65 @@ wheels = [ assert_eq!(lock, DIRECT_REGISTRY_LOCK); } + /// #1140: `uv remove six` after vendoring drops the dependency, its + /// `[tool.uv.sources]` line and every uv.lock fragment that routed + /// through the wheel. Nothing is left to restore, so the revert must not + /// read the vanished package unit and requires-dist element (whose + /// declaration is gone too) as drift: that kept the wheel and ledger + /// entry forever and looped `vendor --check` → `scan --prune`. + #[tokio::test] + async fn revert_after_uv_remove_is_not_drift() { + const REMOVED_PYPROJECT: &str = "[project]\nname = \"proj\"\nversion = \"0.1.0\"\n\ + requires-python = \">=3.10\"\ndependencies = []\n"; + const REMOVED_LOCK: &str = "version = 1\nrevision = 3\nrequires-python = \">=3.10\"\n\n\ + [[package]]\nname = \"proj\"\nversion = \"0.1.0\"\nsource = { virtual = \".\" }\n"; + let tmp = write_pair(DIRECT_REGISTRY_PYPROJECT, DIRECT_REGISTRY_LOCK).await; + let p = load_uv_project(tmp.path()).await.unwrap(); + let (wiring, meta, _) = wire_uv( + &p, + tmp.path(), + "six", + "1.16.0", + REL_WHEEL, + WHEEL_NAME, + WHEEL_SHA, + "9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f", + ) + .await + .unwrap(); + let entry = entry_for(wiring, meta); + + tokio::fs::write(tmp.path().join("pyproject.toml"), REMOVED_PYPROJECT) + .await + .unwrap(); + tokio::fs::write(tmp.path().join("uv.lock"), REMOVED_LOCK) + .await + .unwrap(); + let outcome = revert_uv(&entry, tmp.path(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!(outcome.lock_entry_removed(), "{:?}", outcome.warnings); + let (pyproject, lock) = read_pair(tmp.path()).await; + assert_eq!(pyproject, REMOVED_PYPROJECT); + assert_eq!(lock, REMOVED_LOCK); + + // A lock that still routes through the uuid dir (a hand-edited + // package unit) is genuine drift and keeps everything. + let edited = DIRECT_PATH_LOCK.replace("version = \"1.16.0\"", "version = \"1.16.1\""); + tokio::fs::write(tmp.path().join("pyproject.toml"), REMOVED_PYPROJECT) + .await + .unwrap(); + tokio::fs::write(tmp.path().join("uv.lock"), &edited) + .await + .unwrap(); + let outcome = revert_uv(&entry, tmp.path(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!(!outcome.lock_entry_removed(), "{:?}", outcome.warnings); + let (_, lock) = read_pair(tmp.path()).await; + assert_eq!(lock, edited); + } + #[tokio::test] async fn revert_override_restores_originals_byte_identically() { let tmp = write_pair(TRANSITIVE_REGISTRY_PYPROJECT, TRANSITIVE_REGISTRY_LOCK).await;