From 004ad9c44b5f68eaadf54d8efa65d6fa5c26975e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 14:28:03 +0000 Subject: [PATCH 1/2] Start fix for #786 Assisted-by: Claude Code:claude-opus-5-5 From 580fff7847e386f5b65f18ec2e53d78ad1c02541 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 14:37:02 +0000 Subject: [PATCH 2/2] Stop re-vendoring removed requirements.txt pins A vendored requirements.txt entry stayed "in use" forever, because the in-use probe behind the ledger supplement and `scan --prune` had no pypi arm. After a user removed a vendored pin, the next vendored scan re-added the package as a "(transitive)" line. After a bump, every scan failed with pypi_requirement_not_pinned. `--prune` never reverted the entry. The requirements flavor now asks its requirements tree (the root file plus in-root -r includes) whether any requirement line still installs the vendored wheel. A removed or bumped pin now gets the vendor_ledger_entry_unwired warning, and `--prune` reverts it with exit 0. An unreadable tree still keeps the entry. Fixes #786 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/vendor.rs | 9 +- .../tests/scan_vendor_requirements_unwired.rs | 223 ++++++++++++++++++ crates/socket-patch-core/src/vendor/pypi.rs | 16 ++ .../src/vendor/pypi_requirements.rs | 139 +++++++++++ 4 files changed, 384 insertions(+), 3 deletions(-) create mode 100644 crates/socket-patch-cli/tests/scan_vendor_requirements_unwired.rs diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 59be95b85..13c25e178 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -255,8 +255,9 @@ pub(crate) async fn dispatch_revert_one_opts( /// Is this vendored entry still consumed by its project's lockfile /// dependency graph? `None` = cannot determine — callers must keep the -/// entry (fail-safe): ecosystems other than npm and cargo have no in-use -/// probe yet, and a missing/unreadable lockfile proves nothing. +/// entry (fail-safe): ecosystems other than npm, cargo and pypi (whose +/// probe covers the requirements flavor only) have no in-use probe yet, +/// and a missing/unreadable lockfile proves nothing. pub(crate) async fn dispatch_in_use_one( entry: &VendorEntry, project_root: &Path, @@ -267,6 +268,7 @@ pub(crate) async fn dispatch_in_use_one( // at this entry's copy = in use; a registry source (crates.io // re-resolve or a hosted takeover) or a missing entry = reclaimable. "cargo" => vendor::cargo::vendored_entry_in_use(entry, project_root).await, + "pypi" => vendor::pypi::vendored_entry_in_use(entry, project_root).await, _ => None, } } @@ -5369,7 +5371,8 @@ mod revert_dispatch_tests { } /// [`dispatch_in_use_one`]'s fail-safe arm: every ecosystem without an - /// in-use probe (everything but npm/cargo) reports `None` — "cannot + /// in-use probe (everything but npm/cargo), and a pypi entry of a flavor + /// without one (here the pre-flavor `None`), reports `None` — "cannot /// determine" — which all callers must treat as KEEP. #[tokio::test] async fn in_use_probe_is_none_for_unprobed_ecosystems() { diff --git a/crates/socket-patch-cli/tests/scan_vendor_requirements_unwired.rs b/crates/socket-patch-cli/tests/scan_vendor_requirements_unwired.rs new file mode 100644 index 000000000..6f4645676 --- /dev/null +++ b/crates/socket-patch-cli/tests/scan_vendor_requirements_unwired.rs @@ -0,0 +1,223 @@ +//! #786: a vendored requirements.txt entry whose pin the user removed or +//! bumped is unwired. A vendored rescan must not rediscover it from the +//! vendor ledger — before, it re-added a removed package as a +//! `(transitive)` line (exit 0) or refused a bumped one with +//! `pypi_requirement_not_pinned` (exit 1, on every run) — and must warn +//! `vendor_ledger_entry_unwired` instead; `--prune` reverts the entry and +//! exits 0. +//! +//! Driven through the built binary against a mock patch API that has no +//! patches, so the only way the stale entry can reach the vendor step is +//! the ledger supplement. The ledger is seeded in the exact shape the +//! requirements backend records (a rewritten `requirements_line`). The +//! package name is a fixture no interpreter on the machine has installed +//! (a crawled install would bypass the supplement). + +use std::path::Path; +use std::process::Command; + +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +const ORG_SLUG: &str = "test-org"; +const UUID: &str = "9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f"; +const PURL: &str = "pkg:pypi/sp-fixture-six@1.16.0"; +const WHEEL: &str = "sp_fixture_six-1.16.0-py3-none-any.whl"; + +fn vendor_line() -> String { + format!("./.socket/vendor/pypi/{UUID}/{WHEEL} # socket-patch vendor: sp-fixture-six==1.16.0") +} + +/// A project vendored by `scan --mode vendored` from `sp-fixture-six==1.16.0` on +/// line 1 of requirements.txt: the committed wheel, and the ledger entry +/// recording the rewritten pin. +fn seed_vendored(root: &Path) { + let dir = root.join(format!(".socket/vendor/pypi/{UUID}")); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join(WHEEL), b"not a real wheel").unwrap(); + let state = serde_json::json!({ + "version": 1, + "entries": { + PURL: { + "ecosystem": "pypi", + "basePurl": PURL, + "uuid": UUID, + "artifact": { + "path": format!(".socket/vendor/pypi/{UUID}/{WHEEL}"), + "sha256": "", + }, + "wiring": [{ + "file": "requirements.txt", + "kind": "requirements_line", + "action": "rewritten", + "key": "requirements.txt:1", + "original": ["sp-fixture-six==1.16.0"], + "new": vendor_line(), + }], + "flavor": "requirements", + "detached": true, + } + } + }); + std::fs::write( + root.join(".socket/vendor/state.json"), + serde_json::to_vec_pretty(&state).unwrap(), + ) + .unwrap(); +} + +async fn empty_patch_api() -> MockServer { + let mock = MockServer::start().await; + Mock::given(method("POST")) + .and(path(format!("/v0/orgs/{ORG_SLUG}/patches/batch"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "packages": [], + "canAccessPaidPatches": false, + }))) + .mount(&mock) + .await; + mock +} + +fn run_scan_vendored(root: &Path, mock_uri: &str, extra: &[&str]) -> (i32, serde_json::Value) { + let mut argv = vec![ + "scan", + "--mode", + "vendored", + "--json", + "--yes", + "--api-url", + mock_uri, + "--api-token", + "fake-token", + "--org", + ORG_SLUG, + "--vendor-url", + mock_uri, + "--patch-server-url", + mock_uri, + ]; + argv.extend_from_slice(extra); + let out = Command::new(env!("CARGO_BIN_EXE_socket-patch")) + .args(&argv) + .current_dir(root) + .env("SOCKET_TELEMETRY_DISABLED", "1") + .env_remove("VIRTUAL_ENV") + .env_remove("CONDA_PREFIX") + .output() + .expect("run socket-patch"); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + let v = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("invalid JSON ({e}): stdout={stdout}; stderr={stderr}")); + (out.status.code().unwrap_or(-1), v) +} + +fn unwired_warnings(v: &serde_json::Value) -> usize { + v["warnings"] + .as_array() + .into_iter() + .flatten() + .filter(|w| w["code"] == "vendor_ledger_entry_unwired") + .count() +} + +/// Every purl the scan sent to the batch endpoint. +async fn batch_purls(mock: &MockServer) -> Vec { + let mut purls = Vec::new(); + for req in mock.received_requests().await.unwrap_or_default() { + if !req.url.path().ends_with("/patches/batch") { + continue; + } + let body: serde_json::Value = serde_json::from_slice(&req.body).unwrap_or_default(); + for c in body["components"] + .as_array() + .or_else(|| body["purls"].as_array()) + .cloned() + .unwrap_or_default() + { + if let Some(p) = c["purl"].as_str().or_else(|| c.as_str()) { + purls.push(p.to_string()); + } + } + } + purls +} + +/// The user's edit of the vendored line, run through a plain rescan and +/// then `--prune`. +async fn assert_unwired_and_pruned(edited: &str) { + let mock = empty_patch_api().await; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + seed_vendored(root); + std::fs::write(root.join("requirements.txt"), edited).unwrap(); + + let (code, v) = run_scan_vendored(root, &mock.uri(), &[]); + assert_eq!(code, 0, "edited={edited:?}: {v}"); + assert_eq!(unwired_warnings(&v), 1, "edited={edited:?}: {v}"); + assert!( + !batch_purls(&mock).await.iter().any(|p| p == PURL), + "edited={edited:?}: the unwired ledger entry must not be rediscovered: {v}" + ); + assert_eq!( + std::fs::read_to_string(root.join("requirements.txt")).unwrap(), + edited, + "a plain rescan must not touch requirements.txt" + ); + + let (code, v) = run_scan_vendored(root, &mock.uri(), &["--prune"]); + assert_eq!(code, 0, "edited={edited:?}: {v}"); + assert_eq!( + v["gc"]["revertedVendoredEntries"], + serde_json::json!([PURL]), + "edited={edited:?}: {v}" + ); + assert_eq!( + std::fs::read_to_string(root.join("requirements.txt")).unwrap(), + edited, + "the user's own pin must survive the prune" + ); + assert!( + !root.join(format!(".socket/vendor/pypi/{UUID}")).exists(), + "edited={edited:?}: the dead uuid dir is reclaimed" + ); + let state = std::fs::read_to_string(root.join(".socket/vendor/state.json")).unwrap_or_default(); + assert!(!state.contains(PURL), "edited={edited:?}: {state}"); +} + +/// #786 case A: the user deleted the vendored line. +#[tokio::test] +async fn removed_vendored_pin_is_unwired_and_pruned() { + assert_unwired_and_pruned("idna==3.7\n").await; +} + +/// #786 case B: the user bumped the pin to another release. +#[tokio::test] +async fn bumped_vendored_pin_is_unwired_and_pruned() { + assert_unwired_and_pruned("sp-fixture-six==1.17.0\nidna==3.7\n").await; +} + +/// The fresh-clone case the ledger supplement exists for: the vendored +/// line is still there and nothing is installed, so the entry stays +/// discoverable and no unwired warning is raised. +#[tokio::test] +async fn wired_vendored_pin_stays_discoverable() { + let mock = empty_patch_api().await; + let tmp = tempfile::tempdir().unwrap(); + seed_vendored(tmp.path()); + let wired = format!("{}\nidna==3.7\n", vendor_line()); + std::fs::write(tmp.path().join("requirements.txt"), &wired).unwrap(); + + let (code, v) = run_scan_vendored(tmp.path(), &mock.uri(), &[]); + assert_eq!(code, 0, "{v}"); + assert_eq!(unwired_warnings(&v), 0, "{v}"); + assert!( + batch_purls(&mock).await.iter().any(|p| p == PURL), + "the wired entry stays discoverable: {v}" + ); + assert_eq!( + std::fs::read_to_string(tmp.path().join("requirements.txt")).unwrap(), + wired + ); +} diff --git a/crates/socket-patch-core/src/vendor/pypi.rs b/crates/socket-patch-core/src/vendor/pypi.rs index 74883c23e..ad3df6179 100644 --- a/crates/socket-patch-core/src/vendor/pypi.rs +++ b/crates/socket-patch-core/src/vendor/pypi.rs @@ -1357,6 +1357,22 @@ pub async fn revert_pypi(entry: &VendorEntry, project_root: &Path, dry_run: bool revert_pypi_opts(entry, project_root, RevertOpts::new(dry_run)).await } +/// Is this pypi-vendored entry still consumed by its project? The prune GC +/// and the vendored discovery supplement ask this; `None` keeps the entry. +/// +/// Only the `requirements` flavor has a probe: its requirements tree is +/// the lock pip installs from, so a pin the user removed or bumped there +/// proves the entry unused. The other flavors report `None` (cannot +/// determine), as before. +pub async fn vendored_entry_in_use(entry: &VendorEntry, project_root: &Path) -> Option { + match entry.flavor.as_deref() { + Some("requirements") => { + super::pypi_requirements::requirements_entry_in_use(project_root, &entry.uuid).await + } + _ => None, + } +} + /// Fail-closed twin of [`super::npm_lock::guard_unwired_textual_revert`] /// for the Python backends. A ledger entry with NO wiring records cannot /// restore any project file — that is the shape `socket-patch repair` diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 70e771ec5..96ecb45b6 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -682,6 +682,37 @@ pub async fn requirements_include_names(root: &Path) -> std::io::Result/`? +/// The requirements tree IS this flavor's lock, so the answer is whether +/// any requirement line (its code, not its comment) reached from the root +/// `requirements.txt` through in-root `-r` includes still names the uuid +/// dir. `Some(false)` when the tree was read and none does — the user +/// removed the pin, or bumped it to another release; `None` when no file +/// of the tree could be read, or a reached include exists but cannot be +/// read (cannot prove the absence of a reference: callers keep the entry). +pub(super) async fn requirements_entry_in_use(root: &Path, uuid: &str) -> Option { + let needle = format!(".socket/vendor/pypi/{uuid}/"); + let names = requirements_include_names(root).await.ok()?; + let mut any_readable = false; + for name in &names { + match read_regular_to_string(&root.join(name)).await { + Ok(content) => { + any_readable = true; + if logical_lines(&content) + .iter() + .any(|ll| split_comment(&ll.text).0.contains(&needle)) + { + return Some(true); + } + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(_) => return None, + } + } + any_readable.then_some(false) +} + /// A root-relative requirements path that stays inside the project root /// (not `../…`, not absolute) — the only files the planner may edit. pub(crate) fn is_in_root_rel(rel: &str) -> bool { @@ -2245,4 +2276,112 @@ mod tests { "the link target is untouched" ); } + + // ── in-use probe (#786) ─────────────────────────────────────────────── + + const PROBE_UUID: &str = "9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f"; + + fn probe_vendor_line(transitive: bool) -> String { + vendor_line( + &format!(".socket/vendor/pypi/{PROBE_UUID}/six-1.16.0-py2.py3-none-any.whl"), + None, + "six", + "1.16.0", + &None, + transitive, + ) + } + + /// The wired shapes: the rewritten pin in the root file, the appended + /// `(transitive)` line, and a pin rewritten inside an `-r` include all + /// keep the entry in use. + #[tokio::test] + async fn in_use_probe_sees_wired_vendor_lines() { + for (root_txt, include) in [ + (format!("{}\nidna==3.7\n", probe_vendor_line(false)), None), + (format!("idna==3.7\n{}\n", probe_vendor_line(true)), None), + ( + "-r requirements/base.txt\nidna==3.7\n".to_string(), + Some(format!("{}\n", probe_vendor_line(false))), + ), + ] { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + tokio::fs::write(root.join("requirements.txt"), &root_txt) + .await + .unwrap(); + if let Some(include) = &include { + tokio::fs::create_dir(root.join("requirements")) + .await + .unwrap(); + tokio::fs::write(root.join("requirements/base.txt"), include) + .await + .unwrap(); + } + assert_eq!( + requirements_entry_in_use(root, PROBE_UUID).await, + Some(true), + "root={root_txt:?} include={include:?}" + ); + } + } + + /// #786: the user removed the vendored pin (case A), bumped it to + /// another release (case B), or commented it out (pip ignores the + /// line). The requirements tree was read and nothing pip installs + /// names the uuid dir any more, so the entry is unused. + #[tokio::test] + async fn in_use_probe_reports_removed_or_bumped_pins_unused() { + for root_txt in [ + "idna==3.7\n".to_string(), + "six==1.17.0\nidna==3.7\n".to_string(), + format!("# {}\nidna==3.7\n", probe_vendor_line(false)), + ] { + let tmp = tempfile::tempdir().unwrap(); + tokio::fs::write(tmp.path().join("requirements.txt"), &root_txt) + .await + .unwrap(); + assert_eq!( + requirements_entry_in_use(tmp.path(), PROBE_UUID).await, + Some(false), + "{root_txt:?}" + ); + } + // A line for ANOTHER uuid (a superseding patch) does not keep this + // one in use either. + let tmp = tempfile::tempdir().unwrap(); + tokio::fs::write( + tmp.path().join("requirements.txt"), + probe_vendor_line(false).replace(PROBE_UUID, "1a2b3c4d-5e6f-4a1b-8c2d-9e0f1a2b3c4d"), + ) + .await + .unwrap(); + assert_eq!( + requirements_entry_in_use(tmp.path(), PROBE_UUID).await, + Some(false) + ); + } + + /// Nothing proves the entry unused when the tree cannot be read: no + /// `requirements.txt` at all, or a reached include that exists but is + /// unreadable (a FIFO). Callers keep the entry. + #[cfg(unix)] + #[tokio::test] + async fn in_use_probe_is_undeterminable_without_a_readable_tree() { + let tmp = tempfile::tempdir().unwrap(); + assert_eq!( + requirements_entry_in_use(tmp.path(), PROBE_UUID).await, + None + ); + + let tmp = tempfile::tempdir().unwrap(); + tokio::fs::write(tmp.path().join("requirements.txt"), "-r base.txt\n") + .await + .unwrap(); + mkfifo(&tmp.path().join("base.txt")); + assert_eq!( + requirements_entry_in_use(tmp.path(), PROBE_UUID).await, + None + ); + } }