Skip to content

Commit 6f0ea2c

Browse files
Fix uv script-lock revert per pair (#1285, #890) (#1391)
* Start fix for #1285, #890 Assisted-by: Claude Code:claude-opus-5-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 * 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 * 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 * 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e74a845 commit 6f0ea2c

5 files changed

Lines changed: 546 additions & 36 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,12 @@ worse, lets a warm cache silently serve unpatched bytes):
897897
script locks the same holds after `uv remove` of the package, and after `uv remove` of the parent
898898
of a TRANSITIVE vendored package, whose `[tool.uv]` override + source and `[manifest]` records
899899
uv leaves in place: those are socket-patch's own records, which the revert restores, so only a
900-
reference outside them counts as drift, #1287), removes the artifacts, prunes the
900+
reference outside them counts as drift, #1287; with several PEP 723 scripts or PEP 751 locks
901+
in one entry this is judged per pair — a script with its `.py.lock`, or one `pylock*.toml` — so
902+
`uv remove --script` of one script still restores the others, #1285, and a wired script, script
903+
lock or PEP 751 lock the user deleted warns `vendor_lockfile_missing` naming it while the files
904+
that still exist are restored, #890, while a root `*.py` that still names the wheel (a renamed
905+
script) keeps it, `vendor_revert_residual_reference`), removes the artifacts, prunes the
901906
ledger, sweeps orphan uuid dirs, and (v5.0) prunes the now-empty `.socket/vendor/<eco>/` and
902907
`.socket/vendor/` levels — `.socket/` itself is removed by the lock guard when nothing else is
903908
left. It works without a manifest: with no manifest and no ledger it is a clean exit-0 no-op.

‎crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ struct Staged {
5858
uuid: String,
5959
entry: VendorEntry,
6060
/// The revert's own advisories that outlive the takeover (vlt's
61-
/// reinstall notice).
61+
/// reinstall notice, a deleted wiring file).
6262
advisories: Vec<serde_json::Value>,
6363
}
6464

@@ -278,7 +278,10 @@ impl Takeover {
278278
let advisories = outcome
279279
.warnings
280280
.iter()
281-
.filter(|w| w.code == socket_patch_core::vendor::vlt_lock::REINSTALL_REQUIRED)
281+
.filter(|w| {
282+
w.code == socket_patch_core::vendor::vlt_lock::REINSTALL_REQUIRED
283+
|| w.code == "vendor_lockfile_missing"
284+
})
282285
.map(|w| serde_json::json!({ "code": w.code, "detail": w.detail }))
283286
.collect();
284287
self.staged.push(Staged {

‎crates/socket-patch-cli/tests/mode_migration_pypi.rs‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,217 @@ async fn assert_script_lock_unwinds(stage: StageFn, remove: fn(&Path)) {
10311031
}
10321032
}
10331033

1034+
/// Two uv PEP 723 scripts, `a.py` and `b.py`, each with its `.py.lock`,
1035+
/// vendored into one ledger entry; returns its wiring files.
1036+
fn stage_two_script_locks(root: &Path) -> &'static [&'static str] {
1037+
stage_script_lock(root);
1038+
for file in ["job.py", "job.py.lock"] {
1039+
let text = std::fs::read_to_string(root.join(file)).unwrap();
1040+
std::fs::remove_file(root.join(file)).unwrap();
1041+
for script in ["a", "b"] {
1042+
std::fs::write(root.join(file.replacen("job", script, 1)), &text).unwrap();
1043+
}
1044+
}
1045+
&["a.py", "a.py.lock", "b.py", "b.py.lock"]
1046+
}
1047+
1048+
/// Two PEP 751 locks, `pylock.toml` and `pylock.dev.toml`, both locking
1049+
/// six; returns its wiring files.
1050+
fn stage_two_pylocks(root: &Path) -> &'static [&'static str] {
1051+
let lock = format!(
1052+
"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"
1053+
);
1054+
for file in ["pylock.toml", "pylock.dev.toml"] {
1055+
std::fs::write(root.join(file), &lock).unwrap();
1056+
}
1057+
&["pylock.toml", "pylock.dev.toml"]
1058+
}
1059+
1060+
/// `uv remove --script b.py six` on [`stage_two_script_locks`].
1061+
fn uv_remove_six_from_b(root: &Path) {
1062+
std::fs::write(
1063+
root.join("b.py"),
1064+
"# /// script\n# requires-python = \">=3.9\"\n# dependencies = []\n# ///\nimport six\n",
1065+
)
1066+
.unwrap();
1067+
std::fs::write(
1068+
root.join("b.py.lock"),
1069+
"version = 1\nrevision = 3\nrequires-python = \">=3.9\"\n",
1070+
)
1071+
.unwrap();
1072+
}
1073+
1074+
/// #1285 / #890: one vendored entry wires several scripts (or PEP 751
1075+
/// locks), and the user changes just one of them: `uv remove --script
1076+
/// b.py six` (#1285), or deletes `b.py` and/or `b.py.lock`, or
1077+
/// `pylock.dev.toml` (#890). Every unwind must still restore the files the
1078+
/// user left alone and retire the entry. Before the fix #1285's shape was
1079+
/// drift-kept as a whole (`vendor_lock_entry_drifted`, a.py stayed wired;
1080+
/// `remove` / `rollback` exit 1) and #890's failed every unwind with
1081+
/// "cannot read ./b.py.lock".
1082+
#[tokio::test]
1083+
async fn multi_script_unwinds_after_one_script_changes() {
1084+
type Change = (&'static str, StageFn, fn(&Path), &'static [&'static str]);
1085+
let changes: [Change; 5] = [
1086+
(
1087+
"uv remove b",
1088+
stage_two_script_locks,
1089+
uv_remove_six_from_b,
1090+
&[],
1091+
),
1092+
(
1093+
"rm b.py b.py.lock",
1094+
stage_two_script_locks,
1095+
|root| {
1096+
std::fs::remove_file(root.join("b.py")).unwrap();
1097+
std::fs::remove_file(root.join("b.py.lock")).unwrap();
1098+
},
1099+
&["b.py", "b.py.lock"],
1100+
),
1101+
(
1102+
"rm b.py.lock",
1103+
stage_two_script_locks,
1104+
|root| std::fs::remove_file(root.join("b.py.lock")).unwrap(),
1105+
&["b.py.lock"],
1106+
),
1107+
(
1108+
"rm b.py",
1109+
stage_two_script_locks,
1110+
|root| std::fs::remove_file(root.join("b.py")).unwrap(),
1111+
&["b.py"],
1112+
),
1113+
(
1114+
"rm pylock.dev.toml",
1115+
stage_two_pylocks,
1116+
|root| std::fs::remove_file(root.join("pylock.dev.toml")).unwrap(),
1117+
&["pylock.dev.toml"],
1118+
),
1119+
];
1120+
let server = MockServer::start().await;
1121+
let hosted_url = mount_hosted_api(&server, true).await;
1122+
let uri = server.uri();
1123+
for (label, stage, change, deleted) in changes {
1124+
for unwind in [
1125+
vec!["vendor", "--revert"],
1126+
vec!["remove", PURL, "--yes", "--offline"],
1127+
vec!["rollback", "--yes", "--offline"],
1128+
hosted_scan_args(&uri),
1129+
] {
1130+
let (_tmp, root) = project();
1131+
let files = stage(&root);
1132+
let originals: Vec<String> = files
1133+
.iter()
1134+
.map(|f| std::fs::read_to_string(root.join(f)).unwrap())
1135+
.collect();
1136+
vendor_project(&root, files);
1137+
change(&root);
1138+
let changed: Vec<Option<String>> = files
1139+
.iter()
1140+
.map(|f| std::fs::read_to_string(root.join(f)).ok())
1141+
.collect();
1142+
1143+
let (code, env) = run_cli(&root, &unwind, &[]);
1144+
assert_eq!(code, 0, "{label} / {unwind:?}: {env:#}");
1145+
let rendered = env.to_string();
1146+
let hosted = unwind.contains(&"hosted");
1147+
if hosted && deleted == ["b.py"] {
1148+
// Hosted mode refuses an orphan `.py.lock` whatever the
1149+
// vendored state, so the takeover keeps the project vendored
1150+
// (untouched) and names the hosted refusal, not a revert
1151+
// failure.
1152+
assert!(
1153+
rendered.contains("redirect_uv_script_missing")
1154+
&& rendered.contains("redirect_takeover_kept_vendored")
1155+
&& !rendered.contains("redirect_vendored_revert_failed"),
1156+
"{label} / {unwind:?}: {env:#}"
1157+
);
1158+
continue;
1159+
}
1160+
assert!(
1161+
!rendered.contains("vendor_lock_entry_drifted")
1162+
&& !rendered.contains("vendor_artifact_kept")
1163+
&& !rendered.contains("cannot read"),
1164+
"{label} / {unwind:?}: {env:#}"
1165+
);
1166+
for file in deleted {
1167+
assert!(
1168+
rendered.contains(&format!("{file} no longer exists")),
1169+
"{label} / {unwind:?}: the missing {file} is named: {env:#}"
1170+
);
1171+
}
1172+
if hosted {
1173+
assert!(
1174+
rendered.contains("redirect_takeover_reverted_vendored"),
1175+
"{label} / {unwind:?}: {env:#}"
1176+
);
1177+
}
1178+
assert!(
1179+
!root.join(format!(".socket/vendor/pypi/{UUID}")).exists(),
1180+
"{label} / {unwind:?}: the vendored wheel is reclaimed"
1181+
);
1182+
for ((f, original), changed) in files.iter().zip(&originals).zip(&changed) {
1183+
let after = std::fs::read_to_string(root.join(f)).ok();
1184+
let Some(after) = after else {
1185+
assert!(changed.is_none(), "{label} / {unwind:?}: {f} is gone");
1186+
continue;
1187+
};
1188+
assert!(
1189+
!after.contains(".socket/vendor/"),
1190+
"{label} / {unwind:?}: {f} is unwired:\n{after}"
1191+
);
1192+
let untouched = changed.as_deref().is_some_and(|c| c.contains(UUID));
1193+
if hosted && untouched && after.contains(&hosted_url) {
1194+
// The takeover re-pins what it restored, hosted.
1195+
continue;
1196+
}
1197+
let expected = if untouched {
1198+
original.as_str()
1199+
} else {
1200+
changed.as_deref().unwrap()
1201+
};
1202+
assert_eq!(after, expected, "{label} / {unwind:?}: {f}");
1203+
}
1204+
}
1205+
}
1206+
}
1207+
1208+
/// A vendored PEP 723 script the user renamed (`git mv job.py tool.py`)
1209+
/// still installs from the vendored wheel through its own
1210+
/// `[tool.uv.sources]`. The revert skips the missing `job.py` record
1211+
/// (`vendor_lockfile_missing`, #890), but must keep the wheel `tool.py`
1212+
/// still names instead of deleting it out from under `uv run tool.py`.
1213+
#[tokio::test]
1214+
async fn renamed_vendored_script_keeps_its_wheel() {
1215+
for unwind in [
1216+
vec!["vendor", "--revert"],
1217+
vec!["remove", PURL, "--yes", "--offline"],
1218+
vec!["rollback", "--yes", "--offline"],
1219+
] {
1220+
let (_tmp, root) = project();
1221+
let files = stage_script_lock(&root);
1222+
vendor_project(&root, files);
1223+
std::fs::rename(root.join("job.py"), root.join("tool.py")).unwrap();
1224+
let renamed = std::fs::read_to_string(root.join("tool.py")).unwrap();
1225+
assert!(renamed.contains(UUID), "{renamed}");
1226+
1227+
let (_, env) = run_cli(&root, &unwind, &[]);
1228+
let rendered = env.to_string();
1229+
assert!(
1230+
rendered.contains("tool.py still resolves through it"),
1231+
"{unwind:?}: the renamed script is named: {env:#}"
1232+
);
1233+
assert!(
1234+
root.join(format!(".socket/vendor/pypi/{UUID}")).exists(),
1235+
"{unwind:?}: the wheel tool.py installs from is kept"
1236+
);
1237+
assert_eq!(
1238+
std::fs::read_to_string(root.join("tool.py")).unwrap(),
1239+
renamed,
1240+
"{unwind:?}: the user's script is left alone"
1241+
);
1242+
}
1243+
}
1244+
10341245
/// #742 / #650 / #1136: a vendored uv project, uv script lock, Hatch
10351246
/// project and Poetry project (LF and CRLF) pick up a superseding patch. The manifest moves `six` from patch A to patch B
10361247
/// (different patched bytes); the next `vendor` must wire B's wheel, remove

‎crates/socket-patch-core/src/vendor/pypi.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2365,7 +2365,11 @@ async fn pypi_reference_clause(project_root: &Path, uuid: &str, skip: &[&str]) -
23652365
continue;
23662366
};
23672367
let is_lock = crate::utils::python_lock::is_python_lock_name(&name);
2368-
if !is_lock && !is_export_name(&name) {
2368+
// A root `*.py` too: a PEP 723 script's `[tool.uv.sources]` path
2369+
// resolves from the script's own directory, so a vendored script
2370+
// the user renamed (`git mv job.py tool.py`) still installs from
2371+
// the wheel although no lock pairs with its new name.
2372+
if !is_lock && !is_export_name(&name) && !name.ends_with(".py") {
23692373
continue;
23702374
}
23712375
// lstat only: a regular file or ANY symlink is probed (the read

0 commit comments

Comments
 (0)