From dcbf217cdf5ee8a4d983f184f4629bee15be2798 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 13 Aug 2026 15:29:24 -0700 Subject: [PATCH 1/2] fix(vendor): mirror the pnpm override into pnpm-workspace.yaml for pnpm >= 11 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pnpm 11 stopped reading `overrides` from package.json's `pnpm` field — it moved to `pnpm-workspace.yaml` (https://pnpm.io/settings). The pnpm vendor backend wrote only `package.json` `pnpm.overrides`, so pnpm 11 ignored it and a frozen install of the committable artifact refused with `ERR_PNPM_LOCKFILE_CONFIG_MISMATCH` — the lock carried an `overrides:` section but pnpm resolved none from config. The vendored tarball and lock were correct (they even pass pnpm 11's tarball-URL supply-chain policy); only the override LOCATION was wrong. Vendoring now mirrors the same versioned `@` → `file:` selector into `pnpm-workspace.yaml` alongside the existing package.json override, so whichever surface the installed pnpm reads matches the lock's `overrides:` section: - no workspace file → create one with a root-only `packages: ['.']` list (pnpm 9 refuses a workspace file whose `packages` field is missing/empty; `.` is the sole importer already and cannot glob a stray `packages/` subtree into the workspace the way `packages/*` would) plus the `overrides:` block; - existing file without `overrides:` → append the section (packages untouched); - existing `overrides:` section → insert our key (or take over a user's exact pin), fail-closed on a conflicting same-name override or an inline mapping. package.json `pnpm.overrides` is kept for pnpm 9/10. Verified across real pnpm 9.15.9 / 10.34.5 / 11.21.0: a cold `pnpm install --frozen-lockfile --offline` from only the committable files installs the vendored (patched) bytes with no config mismatch. `vendor --revert` deletes a file it created (when still the bare scaffold) or splices its override back out of one it edited; the three surfaces are committed override-first / lock-last so a lock-write failure never leaves a desynced override behind. Co-Authored-By: Claude Opus 4.8 --- .../tests/e2e_vendor_pnpm_build.rs | 42 +- .../tests/e2e_vendored_production.rs | 105 +-- .../socket-patch-core/src/vendor/pnpm_lock.rs | 636 ++++++++++++++++-- crates/socket-patch-core/src/vendor/state.rs | 39 +- docs/testing/vendored-production-e2e.md | 28 +- 5 files changed, 709 insertions(+), 141 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs index a3b65e24..2d7e1635 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs @@ -8,9 +8,11 @@ //! bytes (a marker comment prepended to `index.js`). //! 3. `socket-patch vendor --json --offline` — assert the deterministic //! tarball lands at `.socket/vendor/npm//…`, the root package.json -//! gains `pnpm.overrides`, and pnpm-lock.yaml carries the file: -//! resolution (spike P1: importer specifier+version rewritten, packages -//! entry rekeyed with the recomputed integrity). +//! gains `pnpm.overrides`, a `pnpm-workspace.yaml` is created carrying +//! the same `overrides:` (where pnpm >= 11 reads them) plus a root-only +//! `packages:` list, and pnpm-lock.yaml carries the file: resolution +//! (spike P1: importer specifier+version rewritten, packages entry +//! rekeyed with the recomputed integrity). //! 4. **Fresh-checkout proof**: copy ONLY the committable files //! (package.json + pnpm-lock.yaml + .socket/) to a new dir, an EMPTY //! `--store-dir`, and run the spike's strictest invocation @@ -368,6 +370,24 @@ fn run_pnpm_capstone(pm: &str) { ), "the inherited registry integrity must NOT survive the rewrite:\n{lock_after}" ); + + // pnpm >= 11 reads `overrides` only from pnpm-workspace.yaml, so vendoring + // mirrors the same versioned selector there. When the project had none + // (this fixture), it is CREATED with a root-only `packages:` list — pnpm 9 + // refuses a workspace file whose `packages` field is missing/empty, and + // `.` cannot glob a stray subtree into the workspace the way `packages/*` + // could. That makes the committable set install on pnpm 9/10/11 alike. + let ws_path = proj.join("pnpm-workspace.yaml"); + let ws_after = + std::fs::read_to_string(&ws_path).expect("vendoring must create pnpm-workspace.yaml"); + assert!( + ws_after.contains(&format!("{DEP}@{DEP_VERSION}: file:{tgz_rel}")), + "pnpm-workspace.yaml `overrides:` must point at the vendored tarball; got:\n{ws_after}" + ); + assert!( + ws_after.contains("packages:") && ws_after.contains("- '.'"), + "created pnpm-workspace.yaml must carry a root-only packages list; got:\n{ws_after}" + ); eprintln!("VENDOR OK ({pm})"); // 4. FRESH-CHECKOUT PROOF: committable files only, EMPTY store, @@ -376,6 +396,7 @@ fn run_pnpm_capstone(pm: &str) { std::fs::create_dir_all(&fresh).unwrap(); std::fs::copy(&pkg_path, fresh.join("package.json")).unwrap(); std::fs::copy(&lock_path, fresh.join("pnpm-lock.yaml")).unwrap(); + std::fs::copy(&ws_path, fresh.join("pnpm-workspace.yaml")).unwrap(); copy_dir_recursive(&proj.join(".socket"), &fresh.join(".socket")); let fresh_store = tmp.path().join("fresh-pnpm-store"); @@ -410,9 +431,10 @@ fn run_pnpm_capstone(pm: &str) { ); eprintln!("FRESH INSTALL OK ({pm})"); - // 5. Idempotency: a re-run exits 0 and leaves BOTH files byte-stable. + // 5. Idempotency: a re-run exits 0 and leaves ALL THREE files byte-stable. let lock_wired = std::fs::read(&lock_path).unwrap(); let pkg_wired = std::fs::read(&pkg_path).unwrap(); + let ws_wired = std::fs::read(&ws_path).unwrap(); let (code, stdout, stderr) = run_socket( &proj, &[ @@ -439,8 +461,14 @@ fn run_pnpm_capstone(pm: &str) { pkg_wired, "re-vendor must leave package.json byte-identical" ); + assert_eq!( + std::fs::read(&ws_path).unwrap(), + ws_wired, + "re-vendor must leave pnpm-workspace.yaml byte-identical" + ); - // 6. REVERT PROOF: package.json AND pnpm-lock.yaml restored byte-for-byte. + // 6. REVERT PROOF: package.json AND pnpm-lock.yaml restored byte-for-byte, + // and the pnpm-workspace.yaml vendoring created is deleted. let (code, stdout, stderr) = run_socket( &proj, &[ @@ -473,5 +501,9 @@ fn run_pnpm_capstone(pm: &str) { !proj.join(".socket/vendor").exists(), ".socket/vendor must be fully removed after revert" ); + assert!( + !ws_path.exists(), + "revert must delete the pnpm-workspace.yaml vendoring created" + ); eprintln!("REVERT OK ({pm})"); } diff --git a/crates/socket-patch-cli/tests/e2e_vendored_production.rs b/crates/socket-patch-cli/tests/e2e_vendored_production.rs index 41d9b043..ede6958e 100644 --- a/crates/socket-patch-cli/tests/e2e_vendored_production.rs +++ b/crates/socket-patch-cli/tests/e2e_vendored_production.rs @@ -514,23 +514,6 @@ fn minimist_entry(proj: &Path) -> PathBuf { proj.join("node_modules").join(NPM_NAME).join("index.js") } -/// Mirror the `pnpm.overrides` the CLI wrote into `package.json` over to a -/// `pnpm-workspace.yaml` `overrides:` block — the location pnpm >= 11 actually -/// reads (see [`pnpm_vendored_install_proof`]). Reads back exactly what the CLI -/// produced rather than hardcoding a value, so it exercises the real wiring. -fn write_pnpm_workspace_overrides(proj: &Path, leg: &str) { - let pkg: serde_json::Value = - serde_json::from_slice(&std::fs::read(proj.join("package.json")).unwrap()).unwrap(); - let overrides = pkg["pnpm"]["overrides"].as_object().unwrap_or_else(|| { - panic!("{leg}: package.json carries no `pnpm.overrides` to mirror into pnpm-workspace.yaml") - }); - let mut yaml = String::from("overrides:\n"); - for (k, v) in overrides { - yaml.push_str(&format!(" '{k}': '{}'\n", v.as_str().unwrap_or_default())); - } - std::fs::write(proj.join("pnpm-workspace.yaml"), yaml).unwrap(); -} - /// Locate `site-packages` inside a venv, across platforms and Python minors. fn site_packages(venv: &Path) -> Option { if cfg!(windows) { @@ -836,15 +819,28 @@ fn pnpm_vendored_install_proof() { lock.contains(&tgz_rel), "{LEG}: pnpm-lock.yaml was not rewired to the vendored tarball:\n{lock}" ); + // pnpm >= 11 reads `overrides` only from pnpm-workspace.yaml, so the CLI + // creates/updates it too. Assert it landed and points at the tarball. + let ws_path = proj.join("pnpm-workspace.yaml"); + let ws = read(&ws_path); + assert!( + ws.contains(&tgz_rel), + "{LEG}: pnpm-workspace.yaml `overrides:` was not wired to the vendored tarball:\n{ws}" + ); let lock_wired = std::fs::read(proj.join("pnpm-lock.yaml")).unwrap(); let pkg_wired = std::fs::read(proj.join("package.json")).unwrap(); + let ws_wired = std::fs::read(&ws_path).unwrap(); - // DELIVERY PROOF: committable files only (pnpm also edits package.json — - // pnpm.overrides), empty store, frozen offline install. + // DELIVERY PROOF: committable files only (pnpm edits package.json's + // `pnpm.overrides` AND creates pnpm-workspace.yaml), empty store, frozen + // offline install. This must now succeed directly on pnpm >= 11 — the + // pnpm-workspace.yaml override is exactly what closes the old + // ERR_PNPM_LOCKFILE_CONFIG_MISMATCH gap. let fresh = tmp.path().join("fresh"); std::fs::create_dir_all(&fresh).unwrap(); std::fs::copy(proj.join("package.json"), fresh.join("package.json")).unwrap(); std::fs::copy(proj.join("pnpm-lock.yaml"), fresh.join("pnpm-lock.yaml")).unwrap(); + std::fs::copy(&ws_path, fresh.join("pnpm-workspace.yaml")).unwrap(); copy_dir_recursive(&proj.join(".socket"), &fresh.join(".socket")); let fresh_store = tmp.path().join("fresh-pnpm-store").display().to_string(); @@ -862,55 +858,19 @@ fn pnpm_vendored_install_proof() { ]; let ci = tool(&fresh, "pnpm", &install_args, &fresh_env); let entry = minimist_entry(&fresh); - if ok(&ci) { - // pnpm <= 10: the package.json `pnpm.overrides` the CLI wrote is honored. - assert_patched(&entry, PATCH_MARKER, LEG); - assert_ne!( - std::fs::read(&entry).unwrap(), - pristine, - "{LEG}: the reinstalled bytes equal the PRISTINE registry bytes" - ); - } else { - // KNOWN CLI GAP (pnpm >= 11): pnpm no longer reads `overrides` from - // package.json's `pnpm` field — it moved to `pnpm-workspace.yaml` - // (https://pnpm.io/settings). The CLI still writes package.json - // `pnpm.overrides`, so pnpm 11 ignores it and the frozen install - // refuses with a lockfile/config mismatch even though the vendored - // tarball and lock are correct (the supply-chain policy passes). This - // is a real socket-patch compatibility gap, not a test bug: the pnpm - // vendor rewriter should also emit a `pnpm-workspace.yaml` `overrides` - // block on pnpm >= 11. Until it does, this leg reproduces the documented - // workaround (mirror the override into pnpm-workspace.yaml) to prove the - // vendored artifact IS installable, and fails loudly if the failure is - // anything OTHER than that known gap. - let detail = dump(&ci); - assert!( - detail.contains("ERR_PNPM_LOCKFILE_CONFIG_MISMATCH") - || detail.contains("no longer read by pnpm"), - "{LEG}: `pnpm install --frozen-lockfile --offline` failed for an UNEXPECTED reason \ - (not the known pnpm 11 overrides-field-moved gap). This is a new regression:\n{detail}" - ); - println!( - "KNOWN CLI GAP {LEG}: pnpm >= 11 ignores package.json `pnpm.overrides` (moved to \ - pnpm-workspace.yaml), so the CLI's vendored wiring does not take effect on a frozen \ - install. Retrying with the documented pnpm-workspace.yaml workaround. socket-patch \ - should emit that file for pnpm >= 11 during `scan --mode vendored`." - ); - write_pnpm_workspace_overrides(&fresh, LEG); - let retry = tool(&fresh, "pnpm", &install_args, &fresh_env); - assert!( - ok(&retry), - "{LEG}: even with the pnpm-workspace.yaml overrides workaround the vendored tarball \ - did not install — the artifact itself is not installable:\n{}", - dump(&retry) - ); - assert_patched(&entry, PATCH_MARKER, LEG); - assert_ne!( - std::fs::read(&entry).unwrap(), - pristine, - "{LEG}: the reinstalled bytes equal the PRISTINE registry bytes" - ); - } + assert!( + ok(&ci), + "{LEG}: `pnpm install --frozen-lockfile --offline` must install the vendored tarball \ + from the committable files (no ERR_PNPM_LOCKFILE_CONFIG_MISMATCH — the \ + pnpm-workspace.yaml override is what makes pnpm >= 11 honor it):\n{}", + dump(&ci) + ); + assert_patched(&entry, PATCH_MARKER, LEG); + assert_ne!( + std::fs::read(&entry).unwrap(), + pristine, + "{LEG}: the reinstalled bytes equal the PRISTINE registry bytes" + ); let env2 = scan_vendored(&proj, &[]); assert_eq!( @@ -928,6 +888,11 @@ fn pnpm_vendored_install_proof() { pkg_wired, "{LEG}: re-run must leave package.json byte-identical" ); + assert_eq!( + std::fs::read(&ws_path).unwrap(), + ws_wired, + "{LEG}: re-run must leave pnpm-workspace.yaml byte-identical" + ); assert_eq!(vendor_revert(&proj, LEG), 1, "{LEG}: one entry reverted"); assert_eq!( @@ -944,6 +909,10 @@ fn pnpm_vendored_install_proof() { !proj.join(".socket/vendor").exists(), "{LEG}: .socket/vendor must be gone after revert" ); + assert!( + !ws_path.exists(), + "{LEG}: revert must delete the pnpm-workspace.yaml vendoring created" + ); } #[test] diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock.rs b/crates/socket-patch-core/src/vendor/pnpm_lock.rs index 7339a69b..b27ffa41 100644 --- a/crates/socket-patch-core/src/vendor/pnpm_lock.rs +++ b/crates/socket-patch-core/src/vendor/pnpm_lock.rs @@ -1,14 +1,25 @@ -//! pnpm vendor backend: paired `package.json` + `pnpm-lock.yaml` surgery. +//! pnpm vendor backend: `package.json` + `pnpm-workspace.yaml` + +//! `pnpm-lock.yaml` surgery. //! -//! pnpm resolves overrides from the ROOT package.json (`pnpm.overrides`) and -//! cross-checks them against the lockfile's own `overrides:` section, so a -//! lock-only edit is unsound: `--frozen-lockfile` fails with -//! `ERR_PNPM_LOCKFILE_CONFIG_MISMATCH` and a plain `pnpm install` silently -//! strips the section and reinstalls the unpatched registry bytes (spike P3, -//! `spikes/PHASE0-V2-FINDINGS.txt`). Vendoring therefore writes the PAIR: a -//! versioned `pnpm.overrides` selector (`@` — only that exact -//! version moves, spike P6) pointing at the vendored tarball, plus the four -//! lock fragments pnpm itself would emit. The surgery is a faithful port of +//! pnpm cross-checks the overrides it reads from config against the +//! lockfile's own `overrides:` section, so a lock-only edit is unsound: +//! `--frozen-lockfile` fails with `ERR_PNPM_LOCKFILE_CONFIG_MISMATCH` and a +//! plain `pnpm install` silently strips the section and reinstalls the +//! unpatched registry bytes (spike P3, `spikes/PHASE0-V2-FINDINGS.txt`). +//! +//! WHERE pnpm reads overrides moved between majors: pnpm <= 10 reads +//! package.json `pnpm.overrides`; pnpm >= 11 no longer reads the package.json +//! `pnpm` field at all and reads `overrides:` from `pnpm-workspace.yaml` +//! instead (https://pnpm.io/settings). Vendoring writes the override to BOTH +//! surfaces (identical `@` → `file:` value) so the committable +//! artifact installs cleanly on pnpm 9/10/11 without a lockfile/config +//! mismatch: the versioned selector (`@` — only that exact +//! version moves, spike P6) points at the vendored tarball. When the project +//! has no `pnpm-workspace.yaml`, one is created carrying a root-only +//! `packages:` list (pnpm 9 refuses a workspace file with no `packages` +//! field) plus the `overrides:` block; `vendor --revert` deletes it again. +//! The lock still gets the four fragments pnpm itself would emit. The surgery +//! is a faithful port of //! `spikes/pnpm/edit_lock.py`, whose output was verified byte-identical to //! pnpm's own lock on BOTH supported majors (9.15.9 / 10.34.1 — they emit //! byte-identical `lockfileVersion: '9.0'` locks; fixtures in `spikes/pnpm/`): @@ -54,6 +65,14 @@ use super::{RevertOutcome, VendorOutcome, VendorWarning}; const PACKAGE_JSON: &str = "package.json"; const PNPM_LOCK: &str = "pnpm-lock.yaml"; +const PNPM_WORKSPACE: &str = "pnpm-workspace.yaml"; + +/// The root-only workspace member list written into a freshly created +/// `pnpm-workspace.yaml`. pnpm 9 refuses a workspace file whose `packages` +/// field is missing or empty; `.` (the root, already the sole importer) is a +/// no-op that cannot accidentally glob a stray `packages/` subtree into a +/// workspace the way `packages/*` would. +const WS_SCAFFOLD_PACKAGES: [&str; 2] = ["packages:", " - '.'"]; /// The only lockfileVersion the surgery has byte-exact fixtures for (both /// pnpm 9 and 10 emit it). @@ -61,6 +80,7 @@ const SUPPORTED_LOCK_VERSION: &str = "9.0"; /// Wiring kinds (the `WiringRecord.kind` discriminators this backend owns). const KIND_PKG_OVERRIDE: &str = "pnpm_pkg_override"; +const KIND_WS_OVERRIDE: &str = "pnpm_ws_override"; const KIND_LOCK_OVERRIDES: &str = "pnpm_lock_overrides"; const KIND_LOCK_IMPORTER_DEP: &str = "pnpm_lock_importer_dep"; const KIND_LOCK_PACKAGE: &str = "pnpm_lock_package"; @@ -71,7 +91,7 @@ const KIND_LOCK_SNAPSHOT_REF: &str = "pnpm_lock_snapshot_ref"; /// a poisoned state.json must not be able to point the rewrite at an /// arbitrary project file. Records naming anything else are skipped with a /// warning (fail-closed). -const REVERT_ALLOWLIST: [&str; 2] = [PNPM_LOCK, PACKAGE_JSON]; +const REVERT_ALLOWLIST: [&str; 3] = [PNPM_LOCK, PACKAGE_JSON, PNPM_WORKSPACE]; /// Vendor one installed npm package into a pnpm project (see the module doc /// for the wiring shape). Same contract as `npm_lock::vendor_npm`: @@ -139,6 +159,11 @@ pub async fn vendor_pnpm( return refused("vendor_lockfile_version_unsupported", detail); } let mut lines = split_lines(&lock_text); + // `pnpm-workspace.yaml` is optional (single-package projects have none); + // its `overrides:` is where pnpm >= 11 reads them. + let ws_text: Option = tokio::fs::read_to_string(project_root.join(PNPM_WORKSPACE)) + .await + .ok(); // ── 3. Pre-flight refusals (override conflicts, entry present) ─────── // A user-authored exact-version pin equal to `version` is TAKEN OVER @@ -152,6 +177,11 @@ pub async fn vendor_pnpm( if let Err(detail) = check_lock_override(&lines, name, version, &effective_key) { return refused("vendor_override_conflict", detail); } + if let Err(detail) = + check_workspace_override(ws_text.as_deref(), name, version, &effective_key) + { + return refused("vendor_override_conflict", detail); + } if !lock_has_target_package(&lines, name, version) { return refused( "vendor_lock_entry_not_found", @@ -232,7 +262,16 @@ pub async fn vendor_pnpm( } } - if !pkg_changed && !lock_changed { + // The pnpm >= 11 override surface. Mirrors the package.json override + // key-for-key so whichever surface the installed pnpm reads matches the + // lock's `overrides:` section. + let ws_edit = match apply_workspace_override(ws_text.as_deref(), &effective_key, &spec, &mut wiring) + { + Ok(edit) => edit, + Err(e) => return done_failure(purl, format!("{PNPM_WORKSPACE} surgery failed: {e}")), + }; + + if !pkg_changed && !lock_changed && ws_edit.new_text.is_none() { // Everything already carries this uuid + the packed integrity: the // project is in sync. The tarball re-pack above was byte-identical // by determinism; synthesize AlreadyPatched and record nothing (the @@ -244,17 +283,21 @@ pub async fn vendor_pnpm( ); } - // ── 6. Commit: package.json FIRST, lock second, unwind on failure ──── + // ── 6. Commit: package.json + pnpm-workspace.yaml FIRST, lock second, + // unwind the override surfaces on a lock failure (P3 desync safety). let pkg_indent = detect_indent(&String::from_utf8_lossy(&pkg_bytes)); let new_pkg_bytes = match serialize_json(&pkg, &pkg_indent) { Ok(bytes) => bytes, Err(e) => return done_failure(purl, format!("cannot serialize {PACKAGE_JSON}: {e}")), }; let lock_out = lines.join("\n"); - if let Err(e) = commit_pair( + if let Err(e) = commit_surfaces( project_root, pkg_changed.then_some(new_pkg_bytes.as_slice()), &pkg_bytes, + ws_edit.new_text.as_deref().map(str::as_bytes), + ws_text.as_deref().map(str::as_bytes), + ws_edit.created_file, lock_changed.then_some(lock_out.as_bytes()), ) .await @@ -291,6 +334,8 @@ pub async fn vendor_pnpm( pnpm: Some(PnpmMeta { created_overrides_table, created_pnpm_table, + created_workspace_file: ws_edit.created_file, + created_workspace_overrides: ws_edit.created_overrides, }), poetry: None, pdm: None, @@ -502,12 +547,158 @@ pub async fn revert_pnpm(entry: &VendorEntry, project_root: &Path, dry_run: bool } } + // pnpm-workspace.yaml override surface (pnpm >= 11): delete a file we + // created, or splice our override back out of one we edited. + if let Some(rec) = entry + .wiring + .iter() + .find(|r| r.file == PNPM_WORKSPACE && r.kind == KIND_WS_OVERRIDE) + { + let (created_file, created_overrides) = match &entry.pnpm { + Some(meta) => (meta.created_workspace_file, meta.created_workspace_overrides), + None => (false, false), + }; + if let Err(e) = revert_workspace( + project_root, + rec, + created_file, + created_overrides, + &entry.uuid, + &mut outcome.warnings, + ) + .await + { + return RevertOutcome::failed(e); + } + } + if let Err(e) = remove_tree(&project_root.join(&uuid_dir_rel)).await { return RevertOutcome::failed(format!("cannot remove {uuid_dir_rel}: {e}")); } outcome } +/// Undo the pnpm-workspace.yaml override: delete a file we created (when it +/// still holds only the vendoring scaffold), or splice our override key back +/// out of a file we edited (restoring the taken-over value, and dropping an +/// `overrides:` section we created once it empties). Drift ⇒ warning, left +/// alone. `Err` is a genuine write failure. +async fn revert_workspace( + project_root: &Path, + rec: &WiringRecord, + created_file: bool, + created_overrides: bool, + entry_uuid: &str, + warnings: &mut Vec, +) -> Result<(), String> { + let path = project_root.join(PNPM_WORKSPACE); + let text = match tokio::fs::read_to_string(&path).await { + Ok(t) => t, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + warnings.push(drifted(format!( + "{PNPM_WORKSPACE} is missing; the pnpm >= 11 override cannot be removed" + ))); + return Ok(()); + } + Err(e) => return Err(format!("cannot read {PNPM_WORKSPACE}: {e}")), + }; + + // Fast path: a file we created that is still byte-identical to the + // scaffold we wrote → delete it (byte-restore to "no file"). + if created_file { + let scaffold = match (rec.key.as_deref(), rec.new.as_ref().and_then(Value::as_str)) { + (Some(key), Some(spec)) => Some(ws_scaffold_text(key, spec)), + _ => None, + }; + if scaffold.as_deref() == Some(text.as_str()) { + return tokio::fs::remove_file(&path) + .await + .map_err(|e| format!("cannot remove {PNPM_WORKSPACE}: {e}")); + } + // Drifted since vendoring: keep the user's file, remove only our key. + } + + let mut lines = split_lines(&text); + let mut dirty = false; + revert_ws_record(&mut lines, rec, entry_uuid, &mut dirty, warnings); + if dirty && created_overrides { + remove_empty_ws_overrides_section(&mut lines); + } + if dirty { + atomic_write_bytes_preserving_mode(&path, lines.join("\n").as_bytes()) + .await + .map_err(|e| format!("cannot write {PNPM_WORKSPACE}: {e}"))?; + } + Ok(()) +} + +/// Remove our override key from the pnpm-workspace.yaml `overrides:` section +/// (or restore a taken-over value in place). Fail-closed on drift. +fn revert_ws_record( + lines: &mut Vec, + rec: &WiringRecord, + entry_uuid: &str, + dirty: &mut bool, + warnings: &mut Vec, +) { + let Some(key) = rec.key.as_deref() else { + warnings.push(drifted(format!( + "wiring record in {PNPM_WORKSPACE} has no key; left alone" + ))); + return; + }; + let Some((start, end, indent)) = ws_overrides_section(lines) else { + warnings.push(drifted(format!( + "{PNPM_WORKSPACE} overrides section is gone; `{key}` not removed" + ))); + return; + }; + for i in (start + 1)..end { + let Some((k, repr, rest)) = parse_key_line(&lines[i], indent) else { + continue; + }; + if k != key { + continue; + } + let ours = Some(rest.as_str()) == rec.new.as_ref().and_then(Value::as_str) + || parse_vendor_path(&rest).is_some_and(|p| p.eco == "npm" && p.uuid == entry_uuid); + if !ours { + warnings.push(drifted(format!( + "{PNPM_WORKSPACE} override `{key}` was changed since vendoring ({rest}); left alone" + ))); + return; + } + match rec.original.as_ref().and_then(Value::as_str) { + Some(orig) => { + lines[i] = format!("{}{}: {orig}", " ".repeat(indent), yaml_key_like(key, &repr)); + } + None => { + lines.remove(i); + } + } + *dirty = true; + return; + } + warnings.push(drifted(format!( + "{PNPM_WORKSPACE} override `{key}` no longer exists; nothing to remove" + ))); +} + +/// Drop an `overrides:` section header we created once its last entry is +/// gone (the append added no blank separator, so removing the header alone +/// restores the file's original trailing bytes). +fn remove_empty_ws_overrides_section(lines: &mut Vec) { + let Some((start, end, indent)) = ws_overrides_section(lines) else { + return; + }; + let still_has_entry = lines[start + 1..end] + .iter() + .any(|l| parse_key_line(l, indent).is_some()); + if !still_has_entry { + lines.remove(start); + } +} + // ───────────────────────────── edit context ────────────────────────────── struct EditCtx<'a> { @@ -910,6 +1101,198 @@ fn apply_pkg_override( Ok((true, created_pnpm_table, created_overrides_table)) } +// ─────────────────────── pnpm-workspace.yaml override ───────────────────── +// pnpm >= 11 reads `overrides:` only from pnpm-workspace.yaml, so the same +// `@` → `file:` mapping is mirrored here. Edits are line +// splices (never a YAML library) so untouched lines stay byte-identical and +// revert restores the file byte-for-byte (or deletes a file we created). + +/// The bytes of a freshly created pnpm-workspace.yaml: a root-only +/// `packages:` list (pnpm 9 refuses a workspace file with no `packages` +/// field) plus the single `overrides:` entry. Kept in one place so the +/// create edit and the revert equality-check cannot drift apart. +fn ws_scaffold_text(key: &str, spec: &str) -> String { + format!( + "{}\n{}\noverrides:\n {}: {spec}\n", + WS_SCAFFOLD_PACKAGES[0], + WS_SCAFFOLD_PACKAGES[1], + yaml_key(key), + ) +} + +/// The outcome of applying the override to pnpm-workspace.yaml. +struct WorkspaceEdit { + /// New file content to write (`None` ⇒ already in sync, nothing to do). + new_text: Option, + /// We created pnpm-workspace.yaml from scratch (revert deletes it). + created_file: bool, + /// We created the `overrides:` section in a pre-existing file (revert + /// removes just that section once emptied). + created_overrides: bool, +} + +/// Locate the top-level `overrides:` block and the indent its entries use +/// (pnpm's canonical is 2 spaces; a hand-authored file may differ). `None` +/// when there is no block-style `overrides:` section. +fn ws_overrides_section(lines: &[String]) -> Option<(usize, usize, usize)> { + let (start, end) = section_bounds(lines, "overrides")?; + let indent = lines[start + 1..end] + .iter() + .find(|l| !l.trim().is_empty()) + .map(|l| indent_of(l)) + .filter(|&n| n >= 1) + .unwrap_or(2); + Some((start, end, indent)) +} + +/// Pre-flight mirror check for pnpm-workspace.yaml (analogous to +/// [`check_lock_override`]): a block-style `overrides:` section may only +/// carry a same-name key equal to `effective_key` with an ownable value. +/// A flow-style/inline `overrides:` mapping is refused (the line surgery +/// cannot splice into it). A missing file/section is fine. +fn check_workspace_override( + ws_text: Option<&str>, + name: &str, + version: &str, + effective_key: &str, +) -> Result<(), String> { + let Some(text) = ws_text else { + return Ok(()); + }; + let lines = split_lines(text); + if lines + .iter() + .any(|l| l.starts_with("overrides:") && l.trim_end() != "overrides:") + { + return Err(format!( + "{PNPM_WORKSPACE} has an inline `overrides:` mapping the pair surgery cannot \ + edit — rewrite it as a block mapping (`overrides:` then indented entries) \ + and re-run" + )); + } + let Some((start, end, indent)) = ws_overrides_section(&lines) else { + return Ok(()); + }; + for line in &lines[start + 1..end] { + let Some((key, _repr, rest)) = parse_key_line(line, indent) else { + continue; + }; + if override_key_name(&key) != name { + continue; + } + // A sibling version's vendored override coexists — skip it. + if is_vendor_value(&rest) && !vendor_value_is_for(&rest, name, version) { + continue; + } + if key != effective_key { + return Err(format!( + "{PNPM_WORKSPACE} carries an override key `{key}` for `{name}` that does not \ + match `{effective_key}` — remove it (or vendor --revert) first" + )); + } + if !(is_vendor_value(&rest) || rest == version) { + return Err(format!( + "{PNPM_WORKSPACE} already carries an override for `{key}` ({rest}); vendoring \ + would fight it — remove the override (or vendor --revert) first" + )); + } + } + Ok(()) +} + +/// Add/refresh the `effective_key` → `file:` override in +/// pnpm-workspace.yaml (creating the file, or the section, when absent). +fn apply_workspace_override( + ws_text: Option<&str>, + our_key: &str, + spec: &str, + wiring: &mut Vec, +) -> Result { + let Some(text) = ws_text else { + // No workspace file: write the root-only scaffold + our override. + wiring.push(ws_record(our_key, spec, WiringAction::Added, None)); + return Ok(WorkspaceEdit { + new_text: Some(ws_scaffold_text(our_key, spec)), + created_file: true, + created_overrides: false, + }); + }; + let mut lines = split_lines(text); + + if let Some((start, end, indent)) = ws_overrides_section(&lines) { + let pad = " ".repeat(indent); + // Immutable scan: our line (if present) + the append anchor. + let mut ours = None; + let mut last_entry = start; + for (i, line) in lines.iter().enumerate().take(end).skip(start + 1) { + if let Some((key, repr, rest)) = parse_key_line(line, indent) { + last_entry = i; + if key == our_key { + ours = Some((i, repr, rest)); + break; + } + } + } + if let Some((i, repr, rest)) = ours { + if rest == spec { + return Ok(WorkspaceEdit { + new_text: None, + created_file: false, + created_overrides: false, + }); + } + // Ours (stale uuid, no original) or the user's exact-version pin + // being TAKEN OVER (recorded, live quoting preserved). + let original = (!is_vendor_value(&rest)).then(|| rest.clone()); + lines[i] = format!("{pad}{}: {spec}", yaml_key_like(our_key, &repr)); + wiring.push(ws_record(our_key, spec, WiringAction::Rewritten, original)); + } else { + lines.insert(last_entry + 1, format!("{pad}{}: {spec}", yaml_key(our_key))); + wiring.push(ws_record(our_key, spec, WiringAction::Added, None)); + } + return Ok(WorkspaceEdit { + new_text: Some(lines.join("\n")), + created_file: false, + created_overrides: false, + }); + } + + // File exists without an `overrides:` section: append one after the last + // non-empty line (no blank separator, so revert removes exactly two + // lines and the file's trailing bytes stay put). + let anchor = lines + .iter() + .rposition(|l| !l.trim().is_empty()) + .map(|i| i + 1) + .unwrap_or(lines.len()); + lines.splice( + anchor..anchor, + ["overrides:".to_string(), format!(" {}: {spec}", yaml_key(our_key))], + ); + wiring.push(ws_record(our_key, spec, WiringAction::Added, None)); + Ok(WorkspaceEdit { + new_text: Some(lines.join("\n")), + created_file: false, + created_overrides: true, + }) +} + +fn ws_record( + key: &str, + spec: &str, + action: WiringAction, + original: Option, +) -> WiringRecord { + WiringRecord { + file: PNPM_WORKSPACE.to_string(), + kind: KIND_WS_OVERRIDE.to_string(), + action, + key: Some(key.to_string()), + original: original.map(Value::String), + new: Some(Value::String(spec.to_string())), + } +} + // ───────────────────────────── lock edits ───────────────────────────────── /// Edit 1: the `overrides:` section — insert it before `importers:` when @@ -1653,15 +2036,19 @@ fn drifted(detail: impl Into) -> VendorWarning { VendorWarning::new("vendor_lock_entry_drifted", detail.into()) } -// ─────────────────────────── pair commit + unwind ───────────────────────── +// ────────────────────────── surfaces commit + unwind ────────────────────── -/// Write the pair: package.json FIRST, lock second; a lock failure restores -/// the original package.json bytes so the P3 desync (override without lock -/// entry or vice versa) is never left on disk. -async fn commit_pair( +/// Write the override surfaces FIRST (package.json, then pnpm-workspace.yaml), +/// the lock LAST; a lock failure unwinds both override surfaces so the P3 +/// desync (an override with no matching lock entry, which pnpm silently +/// unpatches or rejects as a config mismatch) is never left on disk. +async fn commit_surfaces( project_root: &Path, new_pkg: Option<&[u8]>, original_pkg: &[u8], + new_ws: Option<&[u8]>, + original_ws: Option<&[u8]>, + ws_created: bool, new_lock: Option<&[u8]>, ) -> Result<(), String> { if let Some(bytes) = new_pkg { @@ -1669,28 +2056,65 @@ async fn commit_pair( .await .map_err(|e| format!("cannot write {PACKAGE_JSON}: {e}"))?; } + if let Some(bytes) = new_ws { + if let Err(e) = + atomic_write_bytes_preserving_mode(&project_root.join(PNPM_WORKSPACE), bytes).await + { + unwind_override_surfaces(project_root, new_pkg, original_pkg, false, None, false).await; + return Err(format!( + "cannot write {PNPM_WORKSPACE}: {e} ({PACKAGE_JSON} restored to its original bytes)" + )); + } + } if let Some(bytes) = new_lock { if let Err(e) = atomic_write_bytes_preserving_mode(&project_root.join(PNPM_LOCK), bytes).await { - if new_pkg.is_some() { - // Unwind (best effort): a failure here leaves the desync pair - // anyway, but the lock write failing usually means the - // restore fails identically loudly. - let _ = atomic_write_bytes_preserving_mode( - &project_root.join(PACKAGE_JSON), - original_pkg, - ) - .await; - } + // Best effort: a lock write failing usually means the restores + // fail identically loudly, but we still try so the override + // surfaces do not outlive the lock they depend on. + unwind_override_surfaces( + project_root, + new_pkg, + original_pkg, + new_ws.is_some(), + original_ws, + ws_created, + ) + .await; return Err(format!( - "cannot write {PNPM_LOCK}: {e} ({PACKAGE_JSON} restored to its original bytes)" + "cannot write {PNPM_LOCK}: {e} (override surfaces restored to their original state)" )); } } Ok(()) } +/// Best-effort restore of the already-written override surfaces after a +/// downstream write failure: package.json back to its original bytes; a +/// created pnpm-workspace.yaml deleted, an edited one rewritten. +async fn unwind_override_surfaces( + project_root: &Path, + new_pkg: Option<&[u8]>, + original_pkg: &[u8], + ws_written: bool, + original_ws: Option<&[u8]>, + ws_created: bool, +) { + if new_pkg.is_some() { + let _ = atomic_write_bytes_preserving_mode(&project_root.join(PACKAGE_JSON), original_pkg) + .await; + } + if ws_written { + let ws_path = project_root.join(PNPM_WORKSPACE); + if ws_created { + let _ = tokio::fs::remove_file(&ws_path).await; + } else if let Some(orig) = original_ws { + let _ = atomic_write_bytes_preserving_mode(&ws_path, orig).await; + } + } +} + // ─────────────────────── yaml-ish line-block helpers ────────────────────── // pnpm-lock.yaml is machine-emitted with a fixed 2/4/6/8-space shape; these // helpers splice line blocks and never interpret YAML generically. @@ -2210,7 +2634,9 @@ snapshots: entry.pnpm, Some(PnpmMeta { created_overrides_table: true, - created_pnpm_table: true + created_pnpm_table: true, + created_workspace_file: true, + ..Default::default() }) ); assert_eq!(entry.artifact.path, fx.rel_tgz()); @@ -2224,6 +2650,7 @@ snapshots: KIND_LOCK_PACKAGE, KIND_LOCK_SNAPSHOT, KIND_LOCK_SNAPSHOT_REF, + KIND_WS_OVERRIDE, ], "{:?}", entry.wiring @@ -2323,8 +2750,8 @@ snapshots: assert_eq!( entry.pnpm, Some(PnpmMeta { - created_overrides_table: false, - created_pnpm_table: false + created_workspace_file: true, + ..Default::default() }) ); // Our entry extends the existing overrides section, theirs intact. @@ -2432,8 +2859,8 @@ snapshots: assert_eq!( entry.pnpm, Some(PnpmMeta { - created_overrides_table: false, - created_pnpm_table: false + created_workspace_file: true, + ..Default::default() }) ); @@ -2587,7 +3014,8 @@ snapshots: entry.pnpm, Some(PnpmMeta { created_overrides_table: true, - created_pnpm_table: false + created_workspace_file: true, + ..Default::default() }) ); @@ -2612,20 +3040,24 @@ snapshots: } #[tokio::test] - async fn commit_pair_unwinds_package_json_on_lock_write_failure() { + async fn commit_surfaces_unwinds_override_surfaces_on_lock_write_failure() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); tokio::fs::write(root.join(PACKAGE_JSON), P1_BEFORE_PKG) .await .unwrap(); // A directory where the lock should be makes the atomic rename fail - // AFTER package.json was already written. + // AFTER package.json and a freshly created pnpm-workspace.yaml were + // written. tokio::fs::create_dir(root.join(PNPM_LOCK)).await.unwrap(); - let err = commit_pair( + let err = commit_surfaces( root, Some(P1_AFTER_PKG.as_bytes()), P1_BEFORE_PKG.as_bytes(), + Some(b"overrides:\n x: y\n"), + None, + true, // ws created from scratch → unwind deletes it Some(b"lock bytes"), ) .await @@ -2638,6 +3070,10 @@ snapshots: P1_BEFORE_PKG, "package.json restored byte-for-byte after the lock failure" ); + assert!( + !root.join(PNPM_WORKSPACE).exists(), + "the created pnpm-workspace.yaml is deleted on unwind" + ); } #[tokio::test] @@ -3334,4 +3770,126 @@ snapshots: assert_eq!(yaml_key_like("k", "'orig'"), "'k'"); assert_eq!(yaml_key_like("k", "orig"), "k"); } + + // ── pnpm-workspace.yaml override surface (pnpm >= 11) ───────────────── + + async fn write_ws(fx: &Fixture, body: &str) { + tokio::fs::write(fx.root().join(PNPM_WORKSPACE), body) + .await + .unwrap(); + } + async fn ws_exists(fx: &Fixture) -> bool { + fx.root().join(PNPM_WORKSPACE).exists() + } + + /// No workspace file: vendor creates the root-only scaffold + our + /// override; the lock's `overrides:` value equals it (map parity that + /// pnpm >= 11 hard-checks); revert deletes the file again. + #[tokio::test] + async fn workspace_file_is_created_with_root_scaffold_and_revert_deletes_it() { + let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + assert!(!ws_exists(&fx).await, "fixture starts with no workspace file"); + + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + let spec = format!("file:{}", fx.rel_tgz()); + assert_eq!( + fx.read(PNPM_WORKSPACE).await, + ws_scaffold_text("left-pad@1.3.0", &spec), + "created workspace carries `packages: ['.']` + the override" + ); + // The three surfaces agree on the same key → value (no config mismatch). + assert!(fx.read(PNPM_WORKSPACE).await.contains(&format!( + "overrides:\n left-pad@1.3.0: {spec}" + ))); + assert!(fx + .read(PNPM_LOCK) + .await + .contains(&format!("overrides:\n left-pad@1.3.0: {spec}"))); + assert!(entry.pnpm.as_ref().unwrap().created_workspace_file); + assert!(!entry.pnpm.as_ref().unwrap().created_workspace_overrides); + + let outcome = revert_pnpm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!( + !ws_exists(&fx).await, + "revert deletes the workspace file it created" + ); + } + + /// Existing workspace file WITHOUT an `overrides:` section: vendor + /// appends one, leaving the `packages:` list untouched; revert restores + /// the file byte-for-byte. + #[tokio::test] + async fn workspace_overrides_section_is_appended_and_revert_restores_bytes() { + let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + let original = "packages:\n - 'packages/*'\n"; + write_ws(&fx, original).await; + + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + let spec = format!("file:{}", fx.rel_tgz()); + assert_eq!( + fx.read(PNPM_WORKSPACE).await, + format!("{original}overrides:\n left-pad@1.3.0: {spec}\n"), + "override block appended after the existing packages list" + ); + assert!(!entry.pnpm.as_ref().unwrap().created_workspace_file); + assert!(entry.pnpm.as_ref().unwrap().created_workspace_overrides); + + let outcome = revert_pnpm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert_eq!( + fx.read(PNPM_WORKSPACE).await, + original, + "revert removes the appended section byte-for-byte" + ); + } + + /// Existing workspace file WITH an `overrides:` section: vendor inserts + /// our key beside the user's, leaving theirs intact; revert removes only + /// our key. + #[tokio::test] + async fn workspace_override_inserted_beside_existing_and_revert_removes_only_ours() { + let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + let original = "packages:\n - 'packages/*'\noverrides:\n other-pkg: 2.0.0\n"; + write_ws(&fx, original).await; + + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + let spec = format!("file:{}", fx.rel_tgz()); + assert_eq!( + fx.read(PNPM_WORKSPACE).await, + format!("packages:\n - 'packages/*'\noverrides:\n other-pkg: 2.0.0\n left-pad@1.3.0: {spec}\n"), + ); + assert!(!entry.pnpm.as_ref().unwrap().created_workspace_file); + assert!(!entry.pnpm.as_ref().unwrap().created_workspace_overrides); + + let outcome = revert_pnpm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert_eq!( + fx.read(PNPM_WORKSPACE).await, + original, + "revert leaves the user's override intact and drops only ours" + ); + } + + /// A flow-style/inline `overrides:` mapping the line surgery cannot + /// splice into is refused before any write. + #[tokio::test] + async fn inline_workspace_overrides_mapping_is_refused() { + let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + write_ws(&fx, "overrides: {other-pkg: 2.0.0}\n").await; + let detail = expect_refused(fx.vendor(false).await, "vendor_override_conflict"); + assert!(detail.contains("inline"), "{detail}"); + } + + /// A conflicting same-name override already in the workspace file is a + /// fail-closed refusal. + #[tokio::test] + async fn workspace_conflicting_same_name_override_is_refused() { + let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + write_ws(&fx, "overrides:\n left-pad: 1.4.0\n").await; + expect_refused(fx.vendor(false).await, "vendor_override_conflict"); + } } diff --git a/crates/socket-patch-core/src/vendor/state.rs b/crates/socket-patch-core/src/vendor/state.rs index 2333079e..ed220024 100644 --- a/crates/socket-patch-core/src/vendor/state.rs +++ b/crates/socket-patch-core/src/vendor/state.rs @@ -137,15 +137,24 @@ pub struct UvMeta { /// npm/pnpm bookkeeping: which `pnpm-workspace.yaml`/`package.json` tables /// the wiring had to create (revert then removes the emptied tables too). -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct PnpmMeta { - /// Vendor created the `overrides` table itself. + /// Vendor created the package.json `pnpm.overrides` table itself. #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub created_overrides_table: bool, - /// Vendor created the enclosing `pnpm` table itself. + /// Vendor created the enclosing package.json `pnpm` table itself. #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub created_pnpm_table: bool, + /// Vendor created the `pnpm-workspace.yaml` file itself (pnpm >= 11 reads + /// `overrides` only from there); revert deletes it when it still holds + /// only the vendoring scaffold. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub created_workspace_file: bool, + /// Vendor created the `overrides:` section in a pre-existing + /// `pnpm-workspace.yaml`; revert removes just that section once emptied. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub created_workspace_overrides: bool, } /// pypi/poetry bookkeeping. @@ -551,7 +560,8 @@ mod tests { entry.flavor = Some("pnpm".into()); entry.pnpm = Some(PnpmMeta { created_overrides_table: true, - created_pnpm_table: false, + created_workspace_file: true, + ..Default::default() }); entry.poetry = Some(PoetryMeta { dep_class: "direct".into(), @@ -578,6 +588,7 @@ mod tests { // camelCase keys on the wire. for key in [ "\"createdOverridesTable\"", + "\"createdWorkspaceFile\"", "\"depClass\"", "\"lockVersion\"", "\"strategy\"", @@ -585,20 +596,20 @@ mod tests { ] { assert!(text.contains(key), "{key} missing: {text}"); } - // Skip-empty inner fields: the false bool and any empty vec vanish. + // Skip-empty inner fields: the false bools and any empty vec vanish. assert!( !text.contains("createdPnpmTable"), "false bool omitted: {text}" ); + assert!( + !text.contains("createdWorkspaceOverrides"), + "false bool omitted: {text}" + ); } #[test] fn v2_meta_empty_inner_fields_do_not_serialize() { - let pnpm = serde_json::to_string(&PnpmMeta { - created_overrides_table: false, - created_pnpm_table: false, - }) - .unwrap(); + let pnpm = serde_json::to_string(&PnpmMeta::default()).unwrap(); assert_eq!(pnpm, "{}", "all-default PnpmMeta serializes empty"); let pipenv = serde_json::to_string(&PipenvMeta { @@ -617,13 +628,7 @@ mod tests { // And the omitted spellings deserialize back to the defaults. let back: PnpmMeta = serde_json::from_str("{}").unwrap(); - assert_eq!( - back, - PnpmMeta { - created_overrides_table: false, - created_pnpm_table: false - } - ); + assert_eq!(back, PnpmMeta::default()); let back: PipenvMeta = serde_json::from_str("{}").unwrap(); assert!(back.sections.is_empty()); } diff --git a/docs/testing/vendored-production-e2e.md b/docs/testing/vendored-production-e2e.md index 5210cd39..47e1a2d0 100644 --- a/docs/testing/vendored-production-e2e.md +++ b/docs/testing/vendored-production-e2e.md @@ -61,7 +61,7 @@ hosted suite). | Package manager | Fixture | Delivery install (cold, offline, committable-only) | Status | |-----------------|---------|-----------------------------------------------------|--------| | npm | minimist@1.2.2 | `npm ci` | ✅ full | -| pnpm | minimist@1.2.2 | `pnpm install --frozen-lockfile --offline` | ⚠️ pnpm 11 gap (below), delivery proven via workaround | +| pnpm | minimist@1.2.2 | `pnpm install --frozen-lockfile --offline` | ✅ full | | yarn classic | minimist@1.2.2 | `yarn install --frozen-lockfile --offline` | ✅ full | | yarn berry (node-modules) | minimist@1.2.2 | `yarn install --immutable --check-cache` | ✅ full | | bun (text lockfile) | minimist@1.2.2 | `bun install --frozen-lockfile` | ✅ full | @@ -85,19 +85,23 @@ directory. Both were found against real production + real toolchains; neither is a test bug. -### 1. `pnpm` >= 11 — vendored `overrides` land in the wrong file (CLI) +### 1. `pnpm` >= 11 — vendored `overrides` land in the wrong file (CLI) — FIXED pnpm 11 stopped reading `overrides` from `package.json`'s `pnpm` field — it -moved to `pnpm-workspace.yaml` (https://pnpm.io/settings). The CLI still writes -`package.json` `pnpm.overrides`, so pnpm 11 ignores it and a frozen install -refuses with `ERR_PNPM_LOCKFILE_CONFIG_MISMATCH`, even though the vendored -tarball and lock are correct (the lockfile passes pnpm's supply-chain policy). - -**Fix belongs in the CLI**: the pnpm vendor rewriter should also emit a -`pnpm-workspace.yaml` `overrides` block on pnpm >= 11. The leg reproduces that -documented workaround (mirroring the override the CLI put in `package.json` into -`pnpm-workspace.yaml`) to prove the vendored artifact IS installable, and fails -loudly if the frozen install fails for any *other* reason. +moved to `pnpm-workspace.yaml` (https://pnpm.io/settings). The CLI used to write +only `package.json` `pnpm.overrides`, so pnpm 11 ignored it and a frozen install +refused with `ERR_PNPM_LOCKFILE_CONFIG_MISMATCH`, even though the vendored +tarball and lock were correct (the lockfile passes pnpm's supply-chain policy). + +**Fixed** (`fix/pnpm11-overrides-location`): the pnpm vendor backend now +mirrors the same versioned `@` → `file:` override into +`pnpm-workspace.yaml` (creating the file with a root-only `packages: ['.']` +list — pnpm 9 refuses a workspace file with no `packages` field — when the +project has none), alongside the existing `package.json` `pnpm.overrides` for +pnpm 9/10. The committable set installs cleanly on pnpm 9/10/11 with no config +mismatch, and `vendor --revert` deletes a file it created (or splices its +override back out of one it edited). This leg now asserts the frozen install +succeeds directly, with no workaround. ### 2. `gem` — vendoring the platform-qualified purl is unsupported (CLI) From 1af04dabfe46bf04d1ffb6cc061e1c6b7b79bfc2 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 14 Aug 2026 07:48:12 -0700 Subject: [PATCH 2/2] fix(vendor): preserve prior revert originals when re-vendoring adds a surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pnpm >= 11 support (#174) mirrors the tarball override into `pnpm-workspace.yaml` in addition to `package.json` + `pnpm-lock.yaml`. On the UPGRADE path — a project vendored by the pre-workspace code, then re-vendored under the new code — `package.json` and `pnpm-lock.yaml` already carry the override, so `vendor_pnpm` rewrites neither and its fresh `VendorEntry.wiring` names ONLY the newly created `pnpm-workspace.yaml` surface. The CLI vendor flow re-invokes the backend on every installed package (there is no ledger short-circuit; the `already_vendored` classification only happens AFTER the backend runs and returns `AlreadyPatched`). So `persist_vendor_entry` would replace the prior ledger entry wholesale with the workspace-only entry, dropping the `package.json` + `pnpm-lock.yaml` pre-vendor originals the FIRST vendoring recorded. `vendor --revert` could then restore only the workspace file, leaving the override wired into the other two surfaces forever. Fix: extract the re-vendor reconciliation into `vendor::carry_forward_wiring` and extend it to (1) union the prior entry's wiring records for surfaces THIS run left in sync, and (2) OR-merge the pnpm "created this table/file/section" bookkeeping — both scoped to a same-uuid re-vendor so a new-uuid re-vendor still rewires every surface fresh. The existing original-fill and go-takeover carry-forward are unchanged. Revert now byte-restores all three surfaces on the upgrade path. Regression test `revendor_upgrade_adds_workspace_and_revert_restores_all_three_surfaces` stages a pre-workspace vendored project, re-vendors (adding only the workspace mirror), reconciles, then reverts and asserts package.json + pnpm-lock.yaml + pnpm-workspace.yaml are all byte-restored to their pre-vendor originals. Co-Authored-By: Claude Opus 4.8 --- .../socket-patch-cli/src/commands/vendor.rs | 37 +++------ crates/socket-patch-core/src/vendor/mod.rs | 5 +- .../socket-patch-core/src/vendor/pnpm_lock.rs | 70 +++++++++++++++++ crates/socket-patch-core/src/vendor/state.rs | 75 +++++++++++++++++++ 4 files changed, 160 insertions(+), 27 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index e95638ad..d82f59f0 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -520,34 +520,19 @@ pub(crate) async fn persist_vendor_entry( let candidate = candidate.to_string(); entry.detached = detached; entry.record = detached.then(|| record.clone()); - // A re-vendor run re-derives the entry from current - // disk state, where the takeover already happened — - // preserve the prior flag or the revert-time - // "takeover_not_restored" hint is lost. + // A re-vendor run re-derives the entry from current disk state, where + // the takeover / earlier wiring already happened. Reconcile the fresh + // entry with the one it replaces so `--revert` still knows how to undo + // every surface any earlier vendoring touched: carry forward the true + // pre-vendor originals (a re-vendor records `original: None` for its own + // stale `.socket/vendor/` pointer), the wiring records for surfaces this + // run left in sync (e.g. package.json + pnpm-lock.yaml when only the new + // pnpm-workspace.yaml override was added on a pnpm >= 11 upgrade), the + // pnpm created-surface bookkeeping, and the takeover flag. See + // [`vendor::carry_forward_wiring`]. let prev = state.entries.get(&candidate).cloned(); if let Some(prev) = &prev { - entry.took_over_go_patches = entry.took_over_go_patches || prev.took_over_go_patches; - // A re-vendor (new patch uuid) rewrites our own - // stale wiring, so the backend records - // `original: None` (it must never record a - // dangling `.socket/vendor/` pointer as the - // pre-vendor fragment). The TRUE pre-vendor - // original lives in the entry being replaced — - // carry it forward by wiring identity, or a - // later `--revert` can only shrug - // (`vendor_lock_entry_drifted`) instead of - // restoring the registry fragment. - for rec in &mut entry.wiring { - if rec.action == vendor::state::WiringAction::Rewritten && rec.original.is_none() { - if let Some(prev_rec) = prev - .wiring - .iter() - .find(|p| p.file == rec.file && p.kind == rec.kind && p.key == rec.key) - { - rec.original = prev_rec.original.clone(); - } - } - } + vendor::carry_forward_wiring(prev, &mut entry); } let new_uuid = entry.uuid.clone(); state.entries.insert(candidate.clone(), entry); diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index 74b382b8..aa9aece3 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -81,7 +81,10 @@ mod yarn_classic_lock; mod yarn_layering_tests; pub use path::{ecosystem_dir_for_purl, parse_vendor_path}; -pub use state::{load_state, lookup_entry, save_state, VendorEntry, VendorState, VENDOR_STATE_REL}; +pub use state::{ + carry_forward_wiring, load_state, lookup_entry, save_state, VendorEntry, VendorState, + VENDOR_STATE_REL, +}; pub use verify::{check_vendored_artifact, file_sha256_hex, ArtifactHealth}; use std::collections::{HashMap, HashSet}; diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock.rs b/crates/socket-patch-core/src/vendor/pnpm_lock.rs index b27ffa41..c833da56 100644 --- a/crates/socket-patch-core/src/vendor/pnpm_lock.rs +++ b/crates/socket-patch-core/src/vendor/pnpm_lock.rs @@ -3892,4 +3892,74 @@ snapshots: write_ws(&fx, "overrides:\n left-pad: 1.4.0\n").await; expect_refused(fx.vendor(false).await, "vendor_override_conflict"); } + + /// The pnpm >= 11 UPGRADE path: a project vendored by the pre-workspace + /// code (package.json + pnpm-lock.yaml already overridden, NO + /// pnpm-workspace.yaml) is re-vendored under the current code, which adds + /// only the workspace mirror. The re-vendor's fresh entry names ONLY the + /// workspace surface, so replacing the ledger entry wholesale would lose + /// the package.json + lock originals the first vendoring recorded and + /// `--revert` could restore only the workspace file. `carry_forward_wiring` + /// (which `persist_vendor_entry` runs on every re-vendor) reconciles the + /// two so revert byte-restores ALL THREE surfaces. + #[tokio::test] + async fn revendor_upgrade_adds_workspace_and_revert_restores_all_three_surfaces() { + let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await; + + // 1. First vendoring, then downgrade the recorded state to what the + // pre-workspace code would have left: no pnpm-workspace.yaml, and a + // ledger entry carrying only the package.json + lock wiring. + let (_, prev, _) = expect_done(fx.vendor(false).await); + let mut prev = prev.unwrap(); + tokio::fs::remove_file(fx.root().join(PNPM_WORKSPACE)) + .await + .unwrap(); + prev.wiring.retain(|r| r.file != PNPM_WORKSPACE); + if let Some(meta) = prev.pnpm.as_mut() { + meta.created_workspace_file = false; + meta.created_workspace_overrides = false; + } + // Pre-workspace code created the pnpm/overrides tables (the fixture's + // package.json had no `pnpm` field), and left both surfaces wired. + let pnpm_meta = prev.pnpm.clone().unwrap(); + assert!(pnpm_meta.created_pnpm_table && pnpm_meta.created_overrides_table); + assert!(prev.wiring.iter().any(|r| r.file == PACKAGE_JSON)); + assert!(prev.wiring.iter().any(|r| r.file == PNPM_LOCK)); + assert!(!ws_exists(&fx).await, "downgraded state has no workspace file"); + + // 2. Re-vendor under the current code: package.json + lock are already + // in sync, so ONLY the workspace mirror is written and the fresh + // entry names ONLY that surface (the bug's precondition). + let (_, revendored, _) = expect_done(fx.vendor(false).await); + let mut merged = revendored.unwrap(); + assert!(ws_exists(&fx).await, "re-vendor added the workspace file"); + assert!( + merged.wiring.iter().all(|r| r.file == PNPM_WORKSPACE), + "the fresh re-vendor entry names only the workspace surface: {:?}", + merged.wiring + ); + + // 3. Reconcile with the entry being replaced (as persist_vendor_entry + // does), then revert. + super::super::state::carry_forward_wiring(&prev, &mut merged); + let outcome = revert_pnpm(&merged, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings); + + // All three surfaces byte-restored to their pre-vendor originals. + assert_eq!( + fx.read(PACKAGE_JSON).await, + P1_BEFORE_PKG, + "package.json byte-restored" + ); + assert_eq!(fx.read(PNPM_LOCK).await, P1_BEFORE_LOCK, "lock byte-restored"); + assert!( + !ws_exists(&fx).await, + "the workspace file the re-vendor created is deleted" + ); + assert!(!fx + .root() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists()); + } } diff --git a/crates/socket-patch-core/src/vendor/state.rs b/crates/socket-patch-core/src/vendor/state.rs index ed220024..4c1296d4 100644 --- a/crates/socket-patch-core/src/vendor/state.rs +++ b/crates/socket-patch-core/src/vendor/state.rs @@ -273,6 +273,81 @@ impl Default for VendorState { } } +/// Carry a re-vendor's ledger entry forward from the one it replaces so a +/// later `--revert` can still undo every surface an *earlier* vendoring of +/// the same package touched. +/// +/// A backend rebuilds `entry.wiring` from only the surfaces it changed THIS +/// run. When a re-vendor adds a NEW surface while the others are already in +/// sync — e.g. a project vendored before pnpm >= 11 support, whose +/// `package.json` + `pnpm-lock.yaml` already carry the override, gaining the +/// `pnpm-workspace.yaml` mirror on re-vendor — the fresh entry names ONLY the +/// new surface. Replacing the prior ledger entry wholesale would then drop +/// the pre-vendor originals the FIRST vendoring recorded for the untouched +/// surfaces, and revert could no longer restore them (it would undo only the +/// newly added surface). This reconciles the two: +/// +/// * fills a `Rewritten` record's missing `original` from the prior entry — +/// a re-vendor rewrites its OWN stale `.socket/vendor/` pointer and so +/// records `original: None` (it must never record a vendored pointer as +/// the pre-vendor fragment); the true original lives in the entry being +/// replaced (matched by file+kind+key); +/// * carries forward any prior wiring record for a surface THIS run did not +/// re-touch (union by file+kind+key), so revert still restores it; +/// * OR-merges the pnpm "created this table/file/section" bookkeeping so a +/// create recorded by the first vendoring is not lost when a re-vendor +/// finds the surface already present (revert byte-restores an emptied +/// table/file only when it knows vendor created it); +/// * preserves the go-patch-takeover flag. +/// +/// The union + meta merge are scoped to a re-vendor of the SAME patch +/// generation (`prev.uuid == entry.uuid`): a new-uuid re-vendor rewires every +/// surface fresh under the new uuid, so the prior uuid's records name nothing +/// the new entry left behind and carrying them forward would only dangle. +/// The original-fill and takeover flag are safe (identity-matched) either way +/// and run unconditionally. +pub fn carry_forward_wiring(prev: &VendorEntry, entry: &mut VendorEntry) { + entry.took_over_go_patches = entry.took_over_go_patches || prev.took_over_go_patches; + + for rec in &mut entry.wiring { + if rec.action == WiringAction::Rewritten && rec.original.is_none() { + if let Some(prev_rec) = prev + .wiring + .iter() + .find(|p| p.file == rec.file && p.kind == rec.kind && p.key == rec.key) + { + rec.original = prev_rec.original.clone(); + } + } + } + + if prev.uuid != entry.uuid { + return; + } + + for prev_rec in &prev.wiring { + let present = entry + .wiring + .iter() + .any(|r| r.file == prev_rec.file && r.kind == prev_rec.kind && r.key == prev_rec.key); + if !present { + entry.wiring.push(prev_rec.clone()); + } + } + + if let Some(prev_meta) = prev.pnpm.as_ref() { + match entry.pnpm.as_mut() { + Some(meta) => { + meta.created_overrides_table |= prev_meta.created_overrides_table; + meta.created_pnpm_table |= prev_meta.created_pnpm_table; + meta.created_workspace_file |= prev_meta.created_workspace_file; + meta.created_workspace_overrides |= prev_meta.created_workspace_overrides; + } + None => entry.pnpm = Some(prev_meta.clone()), + } + } +} + /// The ledger entry addressable as `purl`: the exact map key first, then /// any entry whose resolved `base_purl` equals it (a qualified manifest /// key resolves to the entry recorded under the base PURL).