From c81742911256490b55d945fc07df0409a312104b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 15:22:41 +0000 Subject: [PATCH 1/5] Start fix for #1285, #890 Assisted-by: Claude Code:claude-opus-5-5 From d2f847c28c36529da7ee3a5248647ec377f161d7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 15:37:57 +0000 Subject: [PATCH 2/5] Revert each vendored uv script pair on its own A vendored PyPI entry can wire several PEP 723 scripts (a.py and b.py, each with its .py.lock) or several PEP 751 locks at once. The revert judged the whole entry together, so a change to one script stranded all of them: - After `uv remove --script b.py six`, a.py still named the vendored wheel, so b.py's removal counted as drift. vendor --revert left a.py wired, and remove and rollback exited 1 (#1285). - Deleting b.py, b.py.lock or pylock.dev.toml failed every unwind with "cannot read ./b.py.lock", and the hosted takeover pointed at the same failing command (#890). The revert now decides per pair: a pair that no longer names the wheel is removed, not drift, and a wired file that no longer exists warns vendor_lockfile_missing and is skipped while the remaining files are restored. Other read errors still fail closed, and a hand edit inside a still-wired pair is still drift. Fixes #1285 Fixes #890 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 6 +- .../tests/mode_migration_pypi.rs | 176 +++++++++++ .../socket-patch-core/src/vendor/pypi_lock.rs | 273 +++++++++++++++++- 3 files changed, 443 insertions(+), 12 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 1a0c56081..e8b5450df 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -892,7 +892,11 @@ worse, lets a warm cache silently serve unpatched bytes): script locks the same holds after `uv remove` of the package, and after `uv remove` of the parent of a TRANSITIVE vendored package, whose `[tool.uv]` override + source and `[manifest]` records uv leaves in place: those are socket-patch's own records, which the revert restores, so only a - reference outside them counts as drift, #1287), removes the artifacts, prunes the + reference outside them counts as drift, #1287; with several PEP 723 scripts or PEP 751 locks + in one entry this is judged per pair — a script with its `.py.lock`, or one `pylock*.toml` — so + `uv remove --script` of one script still restores the others, #1285, and a wired script, script + lock or PEP 751 lock the user deleted warns `vendor_lockfile_missing` naming it while the files + that still exist are restored, #890), 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/mode_migration_pypi.rs b/crates/socket-patch-cli/tests/mode_migration_pypi.rs index 448ba51af..e82fdcf19 100644 --- a/crates/socket-patch-cli/tests/mode_migration_pypi.rs +++ b/crates/socket-patch-cli/tests/mode_migration_pypi.rs @@ -1031,6 +1031,182 @@ async fn assert_script_lock_unwinds(stage: StageFn, remove: fn(&Path)) { } } +/// Two uv PEP 723 scripts, `a.py` and `b.py`, each with its `.py.lock`, +/// vendored into one ledger entry; returns its wiring files. +fn stage_two_script_locks(root: &Path) -> &'static [&'static str] { + stage_script_lock(root); + for file in ["job.py", "job.py.lock"] { + let text = std::fs::read_to_string(root.join(file)).unwrap(); + std::fs::remove_file(root.join(file)).unwrap(); + for script in ["a", "b"] { + std::fs::write(root.join(file.replacen("job", script, 1)), &text).unwrap(); + } + } + &["a.py", "a.py.lock", "b.py", "b.py.lock"] +} + +/// Two PEP 751 locks, `pylock.toml` and `pylock.dev.toml`, both locking +/// six; returns its wiring files. +fn stage_two_pylocks(root: &Path) -> &'static [&'static str] { + let lock = format!( + "lock-version = \"1.0\"\ncreated-by = \"uv\"\nrequires-python = \">=3.9\"\n\n[[packages]]\nname = \"six\"\nversion = \"1.16.0\"\nindex = \"https://pypi.org/simple\"\nwheels = [{{ url = \"https://files.pythonhosted.org/six-1.16.0-py2.py3-none-any.whl\", hashes = {{ sha256 = \"{WHEEL_SHA}\" }} }}]\n" + ); + for file in ["pylock.toml", "pylock.dev.toml"] { + std::fs::write(root.join(file), &lock).unwrap(); + } + &["pylock.toml", "pylock.dev.toml"] +} + +/// `uv remove --script b.py six` on [`stage_two_script_locks`]. +fn uv_remove_six_from_b(root: &Path) { + std::fs::write( + root.join("b.py"), + "# /// script\n# requires-python = \">=3.9\"\n# dependencies = []\n# ///\nimport six\n", + ) + .unwrap(); + std::fs::write( + root.join("b.py.lock"), + "version = 1\nrevision = 3\nrequires-python = \">=3.9\"\n", + ) + .unwrap(); +} + +/// #1285 / #890: one vendored entry wires several scripts (or PEP 751 +/// locks), and the user changes just one of them: `uv remove --script +/// b.py six` (#1285), or deletes `b.py` and/or `b.py.lock`, or +/// `pylock.dev.toml` (#890). Every unwind must still restore the files the +/// user left alone and retire the entry. Before the fix #1285's shape was +/// drift-kept as a whole (`vendor_lock_entry_drifted`, a.py stayed wired; +/// `remove` / `rollback` exit 1) and #890's failed every unwind with +/// "cannot read ./b.py.lock". +#[tokio::test] +async fn multi_script_unwinds_after_one_script_changes() { + type Change = (&'static str, StageFn, fn(&Path), &'static [&'static str]); + let changes: [Change; 5] = [ + ( + "uv remove b", + stage_two_script_locks, + uv_remove_six_from_b, + &[], + ), + ( + "rm b.py b.py.lock", + stage_two_script_locks, + |root| { + std::fs::remove_file(root.join("b.py")).unwrap(); + std::fs::remove_file(root.join("b.py.lock")).unwrap(); + }, + &["b.py", "b.py.lock"], + ), + ( + "rm b.py.lock", + stage_two_script_locks, + |root| std::fs::remove_file(root.join("b.py.lock")).unwrap(), + &["b.py.lock"], + ), + ( + "rm b.py", + stage_two_script_locks, + |root| std::fs::remove_file(root.join("b.py")).unwrap(), + &["b.py"], + ), + ( + "rm pylock.dev.toml", + stage_two_pylocks, + |root| std::fs::remove_file(root.join("pylock.dev.toml")).unwrap(), + &["pylock.dev.toml"], + ), + ]; + let server = MockServer::start().await; + let hosted_url = mount_hosted_api(&server, true).await; + let uri = server.uri(); + for (label, stage, change, deleted) in changes { + for unwind in [ + vec!["vendor", "--revert"], + vec!["remove", PURL, "--yes", "--offline"], + vec!["rollback", "--yes", "--offline"], + hosted_scan_args(&uri), + ] { + let (_tmp, root) = project(); + let files = stage(&root); + let originals: Vec = files + .iter() + .map(|f| std::fs::read_to_string(root.join(f)).unwrap()) + .collect(); + vendor_project(&root, files); + change(&root); + let changed: Vec> = files + .iter() + .map(|f| std::fs::read_to_string(root.join(f)).ok()) + .collect(); + + let (code, env) = run_cli(&root, &unwind, &[]); + assert_eq!(code, 0, "{label} / {unwind:?}: {env:#}"); + let rendered = env.to_string(); + let hosted = unwind.contains(&"hosted"); + if hosted && deleted == ["b.py"] { + // Hosted mode refuses an orphan `.py.lock` whatever the + // vendored state, so the takeover keeps the project vendored + // (untouched) and names the hosted refusal, not a revert + // failure. + assert!( + rendered.contains("redirect_uv_script_missing") + && rendered.contains("redirect_takeover_kept_vendored") + && !rendered.contains("redirect_vendored_revert_failed"), + "{label} / {unwind:?}: {env:#}" + ); + continue; + } + assert!( + !rendered.contains("vendor_lock_entry_drifted") + && !rendered.contains("vendor_artifact_kept") + && !rendered.contains("cannot read"), + "{label} / {unwind:?}: {env:#}" + ); + // The takeover reports only `redirect_takeover_reverted_vendored` + // for a clean revert; the unwinds name each missing file. + for file in deleted.iter().filter(|_| !hosted) { + assert!( + rendered.contains(&format!("{file} no longer exists")), + "{label} / {unwind:?}: the missing {file} is named: {env:#}" + ); + } + if hosted { + assert!( + rendered.contains("redirect_takeover_reverted_vendored"), + "{label} / {unwind:?}: {env:#}" + ); + } + assert!( + !root.join(format!(".socket/vendor/pypi/{UUID}")).exists(), + "{label} / {unwind:?}: the vendored wheel is reclaimed" + ); + for ((f, original), changed) in files.iter().zip(&originals).zip(&changed) { + let after = std::fs::read_to_string(root.join(f)).ok(); + let Some(after) = after else { + assert!(changed.is_none(), "{label} / {unwind:?}: {f} is gone"); + continue; + }; + assert!( + !after.contains(".socket/vendor/"), + "{label} / {unwind:?}: {f} is unwired:\n{after}" + ); + let untouched = changed.as_deref().is_some_and(|c| c.contains(UUID)); + if hosted && untouched && after.contains(&hosted_url) { + // The takeover re-pins what it restored, hosted. + continue; + } + let expected = if untouched { + original.as_str() + } else { + changed.as_deref().unwrap() + }; + assert_eq!(after, expected, "{label} / {unwind:?}: {f}"); + } + } + } +} + /// #742 / #650 / #1136: a vendored uv project, uv script lock, Hatch /// project and Poetry project (LF and CRLF) pick up a superseding patch. The manifest moves `six` from patch A to patch B /// (different patched bytes); the next `vendor` must wire B's wheel, remove diff --git a/crates/socket-patch-core/src/vendor/pypi_lock.rs b/crates/socket-patch-core/src/vendor/pypi_lock.rs index e32972dec..216a1d9c9 100644 --- a/crates/socket-patch-core/src/vendor/pypi_lock.rs +++ b/crates/socket-patch-core/src/vendor/pypi_lock.rs @@ -704,6 +704,22 @@ pub(crate) fn restore_document( )) } +/// [`read_file`], with a file that does not exist read as `None`. +async fn read_existing(path: &Path) -> Result, Failure> { + match tokio::fs::symlink_metadata(path).await { + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None), + _ => read_file(path).await.map(Some), + } +} + +/// The pair a wired file belongs to: a `.py.lock` goes with its script, and +/// every other file (a script, a PEP 751 lock) names its own pair. +fn pair_of(file: &str) -> &str { + file.strip_suffix(".lock") + .filter(|script| script.ends_with(".py")) + .unwrap_or(file) +} + fn allowed_file(file: &str, kind: &str) -> bool { Path::new(file).file_name().and_then(|name| name.to_str()) == Some(file) && ((kind == KIND && is_python_lock_name(file) && file != "uv.lock") @@ -797,13 +813,19 @@ pub(super) async fn revert_python_locks( let mut edits = Vec::new(); // REMOVED, not drift (#1214, the script-lock twin of #1140): `uv remove // --script` drops the dependency together with every fragment that - // routed through the wheel. When no wired file names this entry's uuid - // any more, a record whose written text carried it has nothing left to + // routed through the wheel. When no file of a record's pair (a script + // and its `.py.lock`, or one PEP 751 lock) names this entry's uuid any + // more, a record whose written text 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. + // Decided per pair (#1285): one entry can wire several scripts, and a + // script that still routes through the wheel must not turn another + // script's removal into drift. A wired file that no longer exists has + // nothing to restore either (#890): it warns `vendor_lockfile_missing` + // instead of failing every other pair's unwind on the read. Probed once, + // before any record is reverted, so only the user's own edits count. let uuid_lower = entry.uuid.to_ascii_lowercase(); - let mut unreferenced = true; + let mut referenced = BTreeSet::new(); + let mut missing = BTreeSet::new(); for file in entry .wiring .iter() @@ -811,12 +833,14 @@ pub(super) async fn revert_python_locks( .map(|record| record.file.as_str()) .collect::>() { - match read_file(&root.join(file)).await { - Ok(live) if live.to_ascii_lowercase().contains(&uuid_lower) => { - unreferenced = false; - break; + match read_existing(&root.join(file)).await { + Ok(Some(live)) if live.to_ascii_lowercase().contains(&uuid_lower) => { + referenced.insert(pair_of(file)); + } + Ok(Some(_)) => {} + Ok(None) => { + missing.insert(file); } - Ok(_) => {} Err((_, error)) => return RevertOutcome::failed(error), } } @@ -860,7 +884,20 @@ pub(super) async fn revert_python_locks( )); continue; }; - if unreferenced && new.to_ascii_lowercase().contains(&uuid_lower) { + if missing.contains(record.file.as_str()) { + warnings.push(VendorWarning::new( + "vendor_lockfile_missing", + format!( + "{} no longer exists, so its wiring cannot be restored; the rest of the \ + entry is reverted", + record.file + ), + )); + continue; + } + if !referenced.contains(pair_of(&record.file)) + && new.to_ascii_lowercase().contains(&uuid_lower) + { warnings.push(VendorWarning::new( super::LOCK_ENTRY_REMOVED_CODE, format!( @@ -1957,4 +1994,218 @@ mod tests { "drift writes nothing" ); } + + const SCRIPT_LOCK: &str = "version = 1\nrevision = 3\nrequires-python = \">=3.9\"\n\n[manifest]\nrequirements = [{name = \"one\", specifier = \"==1\"}]\n\n[[package]]\nname = \"one\"\nversion = \"1\"\nsource = {registry = \"https://pypi.org/simple\"}\n"; + const SCRIPT: &str = "# /// script\n# requires-python = \">=3.9\"\n# dependencies = [\"one==1\", \"attrs>=20\"]\n# ///\nimport one\n"; + + /// Vendor `one==1` into every file of `files` at once (PEP 723 scripts + /// with their `.py.lock`s, or PEP 751 locks): one ledger entry whose + /// wiring spans several independent pairs. + async fn vendor_files(root: &Path, files: &[(&str, &str)]) -> VendorEntry { + for (name, text) in files { + tokio::fs::write(root.join(name), text).await.unwrap(); + } + let project = load_python_locks(root, "one", "1", UUID).await.unwrap(); + let wheel = + ".socket/vendor/pypi/11111111-1111-4111-8111-111111111111/one-1-py3-none-any.whl"; + let records = wire_python_locks(&project, root, "one", "1", wheel, &"a".repeat(64)) + .await + .unwrap(); + for (name, _) in files { + let wired = std::fs::read_to_string(root.join(name)).unwrap(); + assert!(wired.contains(UUID), "{name} is wired:\n{wired}"); + } + serde_json::from_value(serde_json::json!({ + "ecosystem": "pypi", + "basePurl": "pkg:pypi/one@1", + "uuid": UUID, + "artifact": { "path": wheel, "sha256": "a".repeat(64) }, + "wiring": serde_json::to_value(&records).unwrap(), + "flavor": "python-lock", + })) + .unwrap() + } + + /// Two vendored PEP 723 scripts, `a.py` and `b.py`, sharing one entry. + async fn vendor_two_scripts(root: &Path) -> VendorEntry { + vendor_files( + root, + &[ + ("a.py", SCRIPT), + ("a.py.lock", SCRIPT_LOCK), + ("b.py", SCRIPT), + ("b.py.lock", SCRIPT_LOCK), + ], + ) + .await + } + + fn assert_a_restored(root: &Path) { + assert_eq!(std::fs::read_to_string(root.join("a.py")).unwrap(), SCRIPT); + assert_eq!( + std::fs::read_to_string(root.join("a.py.lock")).unwrap(), + SCRIPT_LOCK + ); + } + + /// #1285: `uv remove --script b.py one` drops the dependency from ONE of + /// two vendored scripts. b.py's pair no longer names the uuid, so its + /// records are removed, not drift, although a.py still names it; a.py, + /// which the user never touched, is restored, and the revert finishes. + #[tokio::test] + async fn uv_remove_from_one_of_two_scripts_reverts_the_other() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let entry = vendor_two_scripts(root).await; + let script = "# /// script\n# requires-python = \">=3.9\"\n# dependencies = [\"attrs>=20\"]\n# ///\nimport one\n"; + let lock = "version = 1\nrevision = 3\nrequires-python = \">=3.9\"\n\n[manifest]\nrequirements = [{name = \"attrs\", specifier = \">=20\"}]\n"; + tokio::fs::write(root.join("b.py"), script).await.unwrap(); + write_pylock(root, "b.py.lock", lock).await; + + let dry = revert_python_locks(&entry, root, true).await; + assert!(dry.success && !dry.drift_skipped(), "{dry:?}"); + let outcome = revert_python_locks(&entry, root, false).await; + assert!(outcome.success, "{outcome:?}"); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!(outcome.lock_entry_removed(), "{:?}", outcome.warnings); + assert!( + outcome + .warnings + .iter() + .all(|w| w.detail.starts_with("b.py")), + "only b.py's pair is removed: {:?}", + outcome.warnings + ); + assert_a_restored(root); + assert_eq!(std::fs::read_to_string(root.join("b.py")).unwrap(), script); + assert_eq!( + std::fs::read_to_string(root.join("b.py.lock")).unwrap(), + lock + ); + } + + /// #1285's control: a hand edit inside the pair that still routes + /// through the wheel stays drift, even with another pair removed. + #[tokio::test] + async fn drift_in_a_still_wired_pair_keeps_the_entry() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let entry = vendor_two_scripts(root).await; + // b.py's dependency is gone from the lock by hand, but b.py still + // routes through the wheel. + write_pylock( + root, + "b.py.lock", + "version = 1\nrevision = 3\nrequires-python = \">=3.9\"\n", + ) + .await; + let outcome = revert_python_locks(&entry, root, false).await; + assert!(outcome.success, "{outcome:?}"); + assert!(outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!( + std::fs::read_to_string(root.join("a.py")) + .unwrap() + .contains(UUID), + "drift writes nothing" + ); + } + + /// #890: the user deletes one of two vendored scripts (both files, only + /// its `.py.lock`, or only the script). A record whose file is gone has + /// nothing to restore: it warns `vendor_lockfile_missing` naming the + /// file, and the files that still exist are restored instead of the + /// whole unwind failing on the read. + #[tokio::test] + async fn deleted_script_or_lock_does_not_block_the_other_scripts() { + for deleted in [&["b.py", "b.py.lock"][..], &["b.py.lock"], &["b.py"]] { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let entry = vendor_two_scripts(root).await; + for file in deleted { + tokio::fs::remove_file(root.join(file)).await.unwrap(); + } + for dry_run in [true, false] { + let outcome = revert_python_locks(&entry, root, dry_run).await; + assert!(outcome.success, "{deleted:?}: {outcome:?}"); + assert!( + !outcome.drift_skipped(), + "{deleted:?}: {:?}", + outcome.warnings + ); + for file in deleted { + assert!( + outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lockfile_missing" + && w.detail.starts_with(&format!("{file} "))), + "{deleted:?}: {file} is named: {:?}", + outcome.warnings + ); + } + } + assert_a_restored(root); + for (file, upstream) in [("b.py", SCRIPT), ("b.py.lock", SCRIPT_LOCK)] { + if !deleted.contains(&file) { + assert_eq!( + std::fs::read_to_string(root.join(file)).unwrap(), + upstream, + "{deleted:?}: the surviving half is restored" + ); + } else { + assert!(!root.join(file).exists(), "{deleted:?}: not recreated"); + } + } + } + } + + /// #890's PEP 751 lane: `pylock.toml` and `pylock.dev.toml` vendored, + /// then `pylock.dev.toml` deleted. + #[tokio::test] + async fn deleted_pylock_does_not_block_the_other_pylock() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let entry = vendor_files(root, &[("pylock.toml", LOCK), ("pylock.dev.toml", LOCK)]).await; + tokio::fs::remove_file(root.join("pylock.dev.toml")) + .await + .unwrap(); + let outcome = revert_python_locks(&entry, root, false).await; + assert!(outcome.success, "{outcome:?}"); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!( + outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lockfile_missing" + && w.detail.starts_with("pylock.dev.toml ")), + "{:?}", + outcome.warnings + ); + assert_eq!( + std::fs::read_to_string(root.join("pylock.toml")).unwrap(), + LOCK + ); + assert!(!root.join("pylock.dev.toml").exists()); + } + + /// Only a MISSING file is skipped: any other read failure (here a + /// directory where the lock was) still fails the revert closed. + #[tokio::test] + async fn unreadable_wired_file_still_fails_closed() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let entry = vendor_two_scripts(root).await; + tokio::fs::remove_file(root.join("b.py.lock")) + .await + .unwrap(); + tokio::fs::create_dir(root.join("b.py.lock")).await.unwrap(); + let outcome = revert_python_locks(&entry, root, false).await; + assert!(!outcome.success, "{outcome:?}"); + assert!( + std::fs::read_to_string(root.join("a.py")) + .unwrap() + .contains(UUID), + "nothing is written" + ); + } } From 194f0c310c4804de50de2eaaabb842461b57284e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 15:53:40 +0000 Subject: [PATCH 3/5] Read each wired Python file once on revert Review follow-ups to the per-pair revert: - Read every wired file once and judge every record against that text, so a file deleted mid-revert cannot fail it with "cannot read". - Warn vendor_lockfile_missing once per file, in wording that holds on a dry run or a drift-kept revert. - Pair a .py.lock with its script through script_of_lock. - The hosted takeover now passes the missing-file advisory through, so scan --mode hosted names a deleted script too. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/scan/hosted/takeover.rs | 7 +- .../tests/mode_migration_pypi.rs | 4 +- .../socket-patch-core/src/vendor/pypi_lock.rs | 71 ++++++++++--------- 3 files changed, 43 insertions(+), 39 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs b/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs index a5c975a17..26db94db1 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs @@ -58,7 +58,7 @@ struct Staged { uuid: String, entry: VendorEntry, /// The revert's own advisories that outlive the takeover (vlt's - /// reinstall notice). + /// reinstall notice, a deleted wiring file). advisories: Vec, } @@ -278,7 +278,10 @@ impl Takeover { let advisories = outcome .warnings .iter() - .filter(|w| w.code == socket_patch_core::vendor::vlt_lock::REINSTALL_REQUIRED) + .filter(|w| { + w.code == socket_patch_core::vendor::vlt_lock::REINSTALL_REQUIRED + || w.code == "vendor_lockfile_missing" + }) .map(|w| serde_json::json!({ "code": w.code, "detail": w.detail })) .collect(); self.staged.push(Staged { diff --git a/crates/socket-patch-cli/tests/mode_migration_pypi.rs b/crates/socket-patch-cli/tests/mode_migration_pypi.rs index e82fdcf19..6a2657c77 100644 --- a/crates/socket-patch-cli/tests/mode_migration_pypi.rs +++ b/crates/socket-patch-cli/tests/mode_migration_pypi.rs @@ -1163,9 +1163,7 @@ async fn multi_script_unwinds_after_one_script_changes() { && !rendered.contains("cannot read"), "{label} / {unwind:?}: {env:#}" ); - // The takeover reports only `redirect_takeover_reverted_vendored` - // for a clean revert; the unwinds name each missing file. - for file in deleted.iter().filter(|_| !hosted) { + for file in deleted { assert!( rendered.contains(&format!("{file} no longer exists")), "{label} / {unwind:?}: the missing {file} is named: {env:#}" diff --git a/crates/socket-patch-core/src/vendor/pypi_lock.rs b/crates/socket-patch-core/src/vendor/pypi_lock.rs index 216a1d9c9..a1c566c65 100644 --- a/crates/socket-patch-core/src/vendor/pypi_lock.rs +++ b/crates/socket-patch-core/src/vendor/pypi_lock.rs @@ -715,9 +715,7 @@ async fn read_existing(path: &Path) -> Result, Failure> { /// The pair a wired file belongs to: a `.py.lock` goes with its script, and /// every other file (a script, a PEP 751 lock) names its own pair. fn pair_of(file: &str) -> &str { - file.strip_suffix(".lock") - .filter(|script| script.ends_with(".py")) - .unwrap_or(file) + script_of_lock(file).unwrap_or(file) } fn allowed_file(file: &str, kind: &str) -> bool { @@ -821,29 +819,36 @@ pub(super) async fn revert_python_locks( // script that still routes through the wheel must not turn another // script's removal into drift. A wired file that no longer exists has // nothing to restore either (#890): it warns `vendor_lockfile_missing` - // instead of failing every other pair's unwind on the read. Probed once, - // before any record is reverted, so only the user's own edits count. + // instead of failing every other pair's unwind on the read. Every wired + // file is read once, before any record is reverted, so only the user's + // own edits count and the records below judge the same text. let uuid_lower = entry.uuid.to_ascii_lowercase(); - let mut referenced = BTreeSet::new(); - let mut missing = BTreeSet::new(); + let mut live_texts = std::collections::BTreeMap::new(); for file in entry .wiring .iter() .filter(|record| allowed_file(&record.file, &record.kind)) .map(|record| record.file.as_str()) - .collect::>() { + if live_texts.contains_key(file) { + continue; + } match read_existing(&root.join(file)).await { - Ok(Some(live)) if live.to_ascii_lowercase().contains(&uuid_lower) => { - referenced.insert(pair_of(file)); - } - Ok(Some(_)) => {} - Ok(None) => { - missing.insert(file); + Ok(live) => { + live_texts.insert(file, live); } Err((_, error)) => return RevertOutcome::failed(error), } } + let referenced: BTreeSet<&str> = live_texts + .iter() + .filter(|(_, live)| { + live.as_ref() + .is_some_and(|live| live.to_ascii_lowercase().contains(&uuid_lower)) + }) + .map(|(file, _)| pair_of(file)) + .collect(); + let mut missing_warned = BTreeSet::new(); // The vendored package's PEP 503 name, and each script lock's live // script (read once, before any record is reverted). let package_name = entry @@ -853,12 +858,12 @@ pub(super) async fn revert_python_locks( .map(|(name, _)| canonicalize_pypi_name(name)); let mut script_texts = std::collections::BTreeMap::new(); for record in entry.wiring.iter().filter(|r| r.kind == KIND) { - if let Some(script) = record - .file - .strip_suffix(".lock") - .filter(|s| s.ends_with(".py")) - { - if let Ok(text) = read_file(&root.join(script)).await { + if let Some(script) = script_of_lock(&record.file) { + let text = match live_texts.get(script) { + Some(live) => live.clone(), + None => read_file(&root.join(script)).await.ok(), + }; + if let Some(text) = text { script_texts.insert(record.file.as_str(), text); } } @@ -884,17 +889,18 @@ pub(super) async fn revert_python_locks( )); continue; }; - if missing.contains(record.file.as_str()) { - warnings.push(VendorWarning::new( - "vendor_lockfile_missing", - format!( - "{} no longer exists, so its wiring cannot be restored; the rest of the \ - entry is reverted", - record.file - ), - )); + let Some(Some(live)) = live_texts.get(record.file.as_str()) else { + if missing_warned.insert(record.file.as_str()) { + warnings.push(VendorWarning::new( + "vendor_lockfile_missing", + format!( + "{} no longer exists, so it has no wiring left to restore", + record.file + ), + )); + } continue; - } + }; if !referenced.contains(pair_of(&record.file)) && new.to_ascii_lowercase().contains(&uuid_lower) { @@ -908,10 +914,7 @@ pub(super) async fn revert_python_locks( )); continue; } - let live = match read_file(&root.join(&record.file)).await { - Ok(live) => live, - Err((_, error)) => return RevertOutcome::failed(error), - }; + let live = live.clone(); // #1287: the vendored package's own unit is gone from the lock along // with every dependent (`uv remove` of the parent of a transitive // package), while the overrides it was wired through survive. That From e7fae29921a2cbd00629ee80a4f77f980cdb3921 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 16:29:57 +0000 Subject: [PATCH 4/5] Judge a missing script lock through the overlay The hosted takeover reverts inside a group commit, whose in-memory overlay can hold a removal the disk does not show yet. The missing check looked at the disk while the read went through the overlay, so a file the run had removed still failed the revert with "cannot read". The check now uses the overlay-aware read and treats its NotFound as missing. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/pypi_lock.rs | 59 +++++++++++++++---- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/pypi_lock.rs b/crates/socket-patch-core/src/vendor/pypi_lock.rs index a1c566c65..e2f0baaf6 100644 --- a/crates/socket-patch-core/src/vendor/pypi_lock.rs +++ b/crates/socket-patch-core/src/vendor/pypi_lock.rs @@ -42,16 +42,20 @@ pub(super) struct PythonLocks { /// vendoring of the whole project. Whether a linked file may be WRITTEN is a separate question, /// answered by [`refuse_symlinked`] right before the writers run. async fn read_file(path: &Path) -> Result { - read_regular_to_string(path).await.map_err(|error| { - ( - "pypi_lock_read_failed", - if error.kind() == std::io::ErrorKind::InvalidInput { - error.to_string() - } else { - format!("cannot read {}: {error}", path.display()) - }, - ) - }) + read_regular_to_string(path) + .await + .map_err(|error| read_failure(path, error)) +} + +fn read_failure(path: &Path, error: std::io::Error) -> Failure { + ( + "pypi_lock_read_failed", + if error.kind() == std::io::ErrorKind::InvalidInput { + error.to_string() + } else { + format!("cannot read {}: {error}", path.display()) + }, + ) } /// The run's PEP 751 / script-lock parses. Both readers below would @@ -704,11 +708,14 @@ pub(crate) fn restore_document( )) } -/// [`read_file`], with a file that does not exist read as `None`. +/// [`read_file`], with a file that does not exist read as `None`. Judged by +/// the same overlay-aware read, so a file an open `GroupCommit` removed is +/// missing too. async fn read_existing(path: &Path) -> Result, Failure> { - match tokio::fs::symlink_metadata(path).await { + match read_regular_to_string(path).await { + Ok(text) => Ok(Some(text)), Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None), - _ => read_file(path).await.map(Some), + Err(error) => Err(read_failure(path, error)), } } @@ -2191,6 +2198,32 @@ mod tests { assert!(!root.join("pylock.dev.toml").exists()); } + /// The hosted takeover reverts inside a `GroupCommit`, whose overlay is + /// what the revert must judge: a wired file the run already removed + /// there (still on disk) is missing, not a failed read. + #[tokio::test] + async fn missing_is_judged_through_the_group_commit_overlay() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let entry = vendor_two_scripts(root).await; + let group = crate::utils::group_commit::GroupCommit::begin(root); + crate::utils::fs::remove_file(&root.join("b.py.lock")) + .await + .unwrap(); + assert!(root.join("b.py.lock").exists(), "captured, not unlinked"); + let outcome = revert_python_locks(&entry, root, true).await; + drop(group); + assert!(outcome.success, "{outcome:?}"); + assert!( + outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lockfile_missing" && w.detail.starts_with("b.py.lock ")), + "{:?}", + outcome.warnings + ); + } + /// Only a MISSING file is skipped: any other read failure (here a /// directory where the lock was) still fails the revert closed. #[tokio::test] From b8bd9e64405cccdffc1532864477d80bbc3795ad Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 19:18:37 +0000 Subject: [PATCH 5/5] Keep the wheel a renamed vendored script uses The revert now skips a wired script that no longer exists. When the user renamed it (`git mv job.py tool.py`) rather than deleting it, tool.py still installs from the vendored wheel through its own [tool.uv.sources] path. But the residual-reference guard never read root *.py files, so the revert deleted that wheel and broke `uv run tool.py`. The guard now also reads root *.py files. uv resolves a script's source paths from the script's own directory, so only a root script can still reach .socket/vendor/. A renamed script now keeps the wheel with vendor_revert_residual_reference, naming the file. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 3 +- .../tests/mode_migration_pypi.rs | 37 +++++++++++++++++++ crates/socket-patch-core/src/vendor/pypi.rs | 6 ++- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index e8b5450df..5a73c5d59 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -896,7 +896,8 @@ worse, lets a warm cache silently serve unpatched bytes): in one entry this is judged per pair — a script with its `.py.lock`, or one `pylock*.toml` — so `uv remove --script` of one script still restores the others, #1285, and a wired script, script lock or PEP 751 lock the user deleted warns `vendor_lockfile_missing` naming it while the files - that still exist are restored, #890), removes the artifacts, prunes the + that still exist are restored, #890, while a root `*.py` that still names the wheel (a renamed + script) keeps it, `vendor_revert_residual_reference`), 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/mode_migration_pypi.rs b/crates/socket-patch-cli/tests/mode_migration_pypi.rs index 6a2657c77..ad5cbcb0b 100644 --- a/crates/socket-patch-cli/tests/mode_migration_pypi.rs +++ b/crates/socket-patch-cli/tests/mode_migration_pypi.rs @@ -1205,6 +1205,43 @@ async fn multi_script_unwinds_after_one_script_changes() { } } +/// A vendored PEP 723 script the user renamed (`git mv job.py tool.py`) +/// still installs from the vendored wheel through its own +/// `[tool.uv.sources]`. The revert skips the missing `job.py` record +/// (`vendor_lockfile_missing`, #890), but must keep the wheel `tool.py` +/// still names instead of deleting it out from under `uv run tool.py`. +#[tokio::test] +async fn renamed_vendored_script_keeps_its_wheel() { + for unwind in [ + vec!["vendor", "--revert"], + vec!["remove", PURL, "--yes", "--offline"], + vec!["rollback", "--yes", "--offline"], + ] { + let (_tmp, root) = project(); + let files = stage_script_lock(&root); + vendor_project(&root, files); + std::fs::rename(root.join("job.py"), root.join("tool.py")).unwrap(); + let renamed = std::fs::read_to_string(root.join("tool.py")).unwrap(); + assert!(renamed.contains(UUID), "{renamed}"); + + let (_, env) = run_cli(&root, &unwind, &[]); + let rendered = env.to_string(); + assert!( + rendered.contains("tool.py still resolves through it"), + "{unwind:?}: the renamed script is named: {env:#}" + ); + assert!( + root.join(format!(".socket/vendor/pypi/{UUID}")).exists(), + "{unwind:?}: the wheel tool.py installs from is kept" + ); + assert_eq!( + std::fs::read_to_string(root.join("tool.py")).unwrap(), + renamed, + "{unwind:?}: the user's script is left alone" + ); + } +} + /// #742 / #650 / #1136: a vendored uv project, uv script lock, Hatch /// project and Poetry project (LF and CRLF) pick up a superseding patch. The manifest moves `six` from patch A to patch B /// (different patched bytes); the next `vendor` must wire B's wheel, remove diff --git a/crates/socket-patch-core/src/vendor/pypi.rs b/crates/socket-patch-core/src/vendor/pypi.rs index 7ec7a2f89..d73deda8e 100644 --- a/crates/socket-patch-core/src/vendor/pypi.rs +++ b/crates/socket-patch-core/src/vendor/pypi.rs @@ -2293,7 +2293,11 @@ async fn pypi_reference_clause(project_root: &Path, uuid: &str, skip: &[&str]) - continue; }; let is_lock = crate::utils::python_lock::is_python_lock_name(&name); - if !is_lock && !is_export_name(&name) { + // A root `*.py` too: a PEP 723 script's `[tool.uv.sources]` path + // resolves from the script's own directory, so a vendored script + // the user renamed (`git mv job.py tool.py`) still installs from + // the wheel although no lock pairs with its new name. + if !is_lock && !is_export_name(&name) && !name.ends_with(".py") { continue; } // lstat only: a regular file or ANY symlink is probed (the read