Skip to content

Commit b0a11c0

Browse files
Fix vendored revert leaving created scaffold behind (#636, #670) (#672)
* Start fix for #636, #670 Assisted-by: Claude Code:claude-opus-5-5 * Fix revert leaving vendored pnpm/uv scaffolding When two or more packages were vendored into a pnpm or uv project, only the first ledger entry remembered that vendoring created the shared pnpm.overrides / pnpm table, pnpm-workspace.yaml or [tool.uv.sources] table. A full vendor --revert, rollback or remove then left that scaffolding behind unless the first package happened to be reverted last, leaving the checkout dirty. The ledger now shares those "created by vendoring" flags across every entry that wires the same files, when it is written and when it is read, so whichever entry empties the scaffold removes it. Ledgers written by earlier releases are repaired on read. Scaffolding that still holds anything else, such as user keys, is kept as before. Fixes #636, #670 Assisted-by: Claude Code:claude-opus-5-5 * Test pnpm two-package revert through the CLI Vendors two packages into a pnpm 9.0-lock project with no pnpm table and no workspace file, then runs vendor --revert end to end, checking package.json, the lock and pnpm-workspace.yaml all come back exact (#636). Assisted-by: Claude Code:claude-opus-5-5 * Update pnpm legacy-ledger golden to clean revert The pnpm legacy-ledger fixture recorded what the earlier binary left after reverting two vendored packages: an empty pnpm.overrides in package.json and the scaffolded pnpm-workspace.yaml (#636). Reverting that same ledger now removes both, so the golden is the restored package.json (still re-indented, as before) and no workspace file. Assisted-by: Claude Code:claude-opus-5-5 * Assert pnpm group-commit revert drops scaffold The group-commit round-trip test exempted pnpm from the byte-exact check because a two-package revert left the emptied pnpm.overrides and the scaffolded pnpm-workspace.yaml behind (#636). Now that both are removed, say why pnpm is still exempt (the fixture's minified package.json comes back re-indented) and assert the scaffold is gone. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9706ff1 commit b0a11c0

7 files changed

Lines changed: 686 additions & 16 deletions

File tree

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

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1297,6 +1297,134 @@ fn pnpm8_lock_v60_hermetic_splice_idempotency_and_revert() {
12971297
run_legacy_hermetic(PNPM8_LOCK, PNPM8_AFTER_TEMPLATE, "6.0");
12981298
}
12991299

1300+
/// #636: two packages vendored into a project with no `pnpm` table and no
1301+
/// pnpm-workspace.yaml, then `vendor --revert` (purl order: is-number, the
1302+
/// entry that created both, goes first). The lock was already byte-exact;
1303+
/// package.json kept `"pnpm": { "overrides": {} }` and the scaffolded
1304+
/// workspace file stayed behind. Both must now come back byte-identical.
1305+
#[test]
1306+
fn pnpm9_two_packages_vendor_revert_removes_created_scaffold() {
1307+
const LOCK: &str = "lockfileVersion: '9.0'
1308+
1309+
settings:
1310+
autoInstallPeers: true
1311+
excludeLinksFromLockfile: false
1312+
1313+
importers:
1314+
1315+
.:
1316+
dependencies:
1317+
is-number:
1318+
specifier: 7.0.0
1319+
version: 7.0.0
1320+
left-pad:
1321+
specifier: 1.3.0
1322+
version: 1.3.0
1323+
1324+
packages:
1325+
1326+
is-number@7.0.0:
1327+
resolution: {integrity: sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==}
1328+
engines: {node: '>=0.12.0'}
1329+
1330+
left-pad@1.3.0:
1331+
resolution: {integrity: sha512-XI5MPzVNApjAyhQzphX8BkmKsKUxD4LdyK24iZeQGinBN9yTQT3bFlCBy/aVx2HrNcqQGsdot8ghrjyrvMCoEA==}
1332+
deprecated: use String.prototype.padStart()
1333+
1334+
snapshots:
1335+
1336+
is-number@7.0.0: {}
1337+
1338+
left-pad@1.3.0: {}
1339+
";
1340+
let tmp = tempfile::tempdir().unwrap();
1341+
let proj = tmp.path().join("proj");
1342+
let orig = b"module.exports = function (x) { return x; };\n".to_vec();
1343+
let patched: Vec<u8> = [MARKER.as_bytes(), orig.as_slice()].concat();
1344+
let mut patches = serde_json::Map::new();
1345+
for (name, version, uuid) in [
1346+
("left-pad", "1.3.0", UUID),
1347+
("is-number", "7.0.0", "2b3c4d5e-6f70-4a1b-8c2d-0123456789ac"),
1348+
] {
1349+
let dir = proj.join("node_modules").join(name);
1350+
std::fs::create_dir_all(&dir).unwrap();
1351+
std::fs::write(
1352+
dir.join("package.json"),
1353+
format!("{{\"name\":\"{name}\",\"version\":\"{version}\"}}\n"),
1354+
)
1355+
.unwrap();
1356+
std::fs::write(dir.join("index.js"), &orig).unwrap();
1357+
patches.insert(
1358+
format!("pkg:npm/{name}@{version}"),
1359+
serde_json::json!({
1360+
"uuid": uuid,
1361+
"exportedAt": "2026-01-01T00:00:00Z",
1362+
"files": { "package/index.js": {
1363+
"beforeHash": git_sha256(&orig),
1364+
"afterHash": git_sha256(&patched),
1365+
}},
1366+
"vulnerabilities": {},
1367+
"description": "two-package scaffold patch",
1368+
"license": "MIT",
1369+
"tier": "free",
1370+
}),
1371+
);
1372+
}
1373+
let socket = proj.join(".socket");
1374+
std::fs::create_dir_all(socket.join("blobs")).unwrap();
1375+
std::fs::write(
1376+
socket.join("manifest.json"),
1377+
serde_json::to_string_pretty(&serde_json::json!({ "patches": patches })).unwrap(),
1378+
)
1379+
.unwrap();
1380+
std::fs::write(socket.join("blobs").join(git_sha256(&patched)), &patched).unwrap();
1381+
1382+
let pkg_before = "{\n \"name\": \"fx\",\n \"version\": \"1.0.0\",\n \"private\": true,\n \
1383+
\"dependencies\": {\n \"left-pad\": \"1.3.0\",\n \
1384+
\"is-number\": \"7.0.0\"\n }\n}\n";
1385+
std::fs::write(proj.join("package.json"), pkg_before).unwrap();
1386+
std::fs::write(proj.join("pnpm-lock.yaml"), LOCK).unwrap();
1387+
let cwd = proj.to_str().unwrap();
1388+
1389+
let (code, stdout, stderr) =
1390+
run_socket(&proj, &["vendor", "--json", "--offline", "--cwd", cwd]);
1391+
assert_eq!(code, 0, "vendor.\nstdout:\n{stdout}\nstderr:\n{stderr}");
1392+
let env = parse_envelope(&stdout);
1393+
assert_eq!(
1394+
env["summary"]["applied"], 2,
1395+
"both packages vendored: {env}"
1396+
);
1397+
assert!(
1398+
proj.join("pnpm-workspace.yaml").is_file(),
1399+
"a 9.0 lock mirrors overrides into a scaffolded workspace file"
1400+
);
1401+
1402+
let (code, stdout, stderr) = run_socket(
1403+
&proj,
1404+
&["vendor", "--revert", "--json", "--offline", "--cwd", cwd],
1405+
);
1406+
assert_eq!(code, 0, "revert.\nstdout:\n{stdout}\nstderr:\n{stderr}");
1407+
let renv = parse_envelope(&stdout);
1408+
assert_eq!(
1409+
renv["summary"]["removed"], 2,
1410+
"both entries reverted: {renv}"
1411+
);
1412+
assert_eq!(
1413+
std::fs::read_to_string(proj.join("package.json")).unwrap(),
1414+
pkg_before,
1415+
"no empty pnpm.overrides left behind"
1416+
);
1417+
assert_eq!(
1418+
std::fs::read_to_string(proj.join("pnpm-lock.yaml")).unwrap(),
1419+
LOCK
1420+
);
1421+
assert!(
1422+
!proj.join("pnpm-workspace.yaml").exists(),
1423+
"the scaffolded pnpm-workspace.yaml must be deleted"
1424+
);
1425+
assert!(!proj.join(".socket/vendor").exists());
1426+
}
1427+
13001428
/// The tarball's SRI (`sha512-<base64>`), the integrity spelling pnpm locks
13011429
/// record.
13021430
fn tarball_integrity(tgz: &Path) -> String {

‎crates/socket-patch-cli/tests/fixtures/legacy-ledgers/pnpm/reverted/package.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,5 @@
55
"dependencies": {
66
"alpha": "1.0.0",
77
"beta": "1.0.0"
8-
},
9-
"pnpm": {
10-
"overrides": {}
118
}
129
}

‎crates/socket-patch-cli/tests/fixtures/legacy-ledgers/pnpm/reverted/pnpm-workspace.yaml‎

Lines changed: 0 additions & 3 deletions
This file was deleted.

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,22 @@ fn group_commit_ends_where_per_package_commits_end_for_every_ecosystem() {
113113
masked_tree(&oracle.root),
114114
"{eco}: same tree after --revert"
115115
);
116-
// Two ecosystems keep scaffolding their revert does not remove when
117-
// TWO packages were vendored (the emptied pnpm override tables and
118-
// workspace file; the catch-all `<packageSourceMapping>` nuget adds
119-
// to a config that had none). That predates the group commit — the
120-
// oracle leaves the same bytes, asserted just above — so only the
121-
// others are held to a byte-exact round trip here.
116+
// Two ecosystems do not round-trip byte for byte when TWO packages
117+
// were vendored: pnpm rewrites the fixture's minified package.json
118+
// with an indent, and nuget keeps the catch-all
119+
// `<packageSourceMapping>` it adds to a config that had none. That
120+
// predates the group commit — the oracle leaves the same bytes,
121+
// asserted just above — so only the others are held to a
122+
// byte-exact round trip here. pnpm's created scaffold itself is
123+
// gone (#636): no emptied `pnpm.overrides`, no workspace file.
124+
if *eco == "pnpm" {
125+
assert!(
126+
!grouped.root.join("pnpm-workspace.yaml").exists(),
127+
"pnpm: the scaffolded workspace file is removed"
128+
);
129+
let pkg = std::fs::read_to_string(grouped.root.join("package.json")).unwrap();
130+
assert!(!pkg.contains("\"pnpm\""), "pnpm: {pkg}");
131+
}
122132
if !["pnpm", "nuget"].contains(eco) {
123133
let after = masked_tree(&grouped.root);
124134
assert!(

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

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8860,4 +8860,196 @@ snapshots:
88608860
}
88618861
}
88628862
}
8863+
8864+
// ── two packages sharing created scaffolding (#636) ──────────────────
8865+
8866+
const TWO_PKG: &str = r#"{
8867+
"name": "fx",
8868+
"version": "1.0.0",
8869+
"private": true,
8870+
"dependencies": {
8871+
"left-pad": "1.3.0",
8872+
"is-number": "7.0.0"
8873+
}
8874+
}
8875+
"#;
8876+
8877+
const TWO_LOCK: &str = "lockfileVersion: '9.0'
8878+
8879+
settings:
8880+
autoInstallPeers: true
8881+
excludeLinksFromLockfile: false
8882+
8883+
importers:
8884+
8885+
.:
8886+
dependencies:
8887+
is-number:
8888+
specifier: 7.0.0
8889+
version: 7.0.0
8890+
left-pad:
8891+
specifier: 1.3.0
8892+
version: 1.3.0
8893+
8894+
packages:
8895+
8896+
is-number@7.0.0:
8897+
resolution: {integrity: sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==}
8898+
engines: {node: '>=0.12.0'}
8899+
8900+
left-pad@1.3.0:
8901+
resolution: {integrity: sha512-XI5MPzVNApjAyhQzphX8BkmKsKUxD4LdyK24iZeQGinBN9yTQT3bFlCBy/aVx2HrNcqQGsdot8ghrjyrvMCoEA==}
8902+
deprecated: use String.prototype.padStart()
8903+
8904+
snapshots:
8905+
8906+
is-number@7.0.0: {}
8907+
8908+
left-pad@1.3.0: {}
8909+
";
8910+
8911+
const IS_NUMBER_UUID: &str = "4d5e6f70-8a9b-4c0d-9e1f-2a3b4c5d6e7f";
8912+
const IS_NUMBER: &str = "pkg:npm/is-number@7.0.0";
8913+
const LEFT_PAD: &str = "pkg:npm/left-pad@1.3.0";
8914+
8915+
/// Vendor is-number then left-pad (is-number creates every scaffold),
8916+
/// persisting the ledger after each the way the vendor loop does.
8917+
async fn vendor_two(pkg_json: &str, workspace: Option<&str>) -> Fixture {
8918+
let fx = fixture_with(pkg_json, TWO_LOCK).await;
8919+
if let Some(ws) = workspace {
8920+
tokio::fs::write(fx.root().join(PNPM_WORKSPACE), ws)
8921+
.await
8922+
.unwrap();
8923+
}
8924+
let is_number = fx.root().join("node_modules/is-number");
8925+
tokio::fs::create_dir_all(&is_number).await.unwrap();
8926+
tokio::fs::write(
8927+
is_number.join("package.json"),
8928+
br#"{"name":"is-number","version":"7.0.0"}"#,
8929+
)
8930+
.await
8931+
.unwrap();
8932+
tokio::fs::write(is_number.join("index.js"), ORIG_INDEX)
8933+
.await
8934+
.unwrap();
8935+
let mut is_number_record = fx.record.clone();
8936+
is_number_record.uuid = IS_NUMBER_UUID.to_string();
8937+
8938+
let blobs = fx.root().join(".socket/blobs");
8939+
let sources = PatchSources::blobs_only(&blobs);
8940+
let mut state = crate::vendor::state::VendorState::new();
8941+
for (purl, dir, record) in [
8942+
(IS_NUMBER, is_number.clone(), &is_number_record),
8943+
(LEFT_PAD, fx.installed(), &fx.record),
8944+
] {
8945+
let outcome = crate::vendor::test_support::vendor_pnpm(
8946+
purl,
8947+
&dir,
8948+
fx.root(),
8949+
record,
8950+
&sources,
8951+
"2026-06-09T00:00:00Z",
8952+
false,
8953+
false,
8954+
None,
8955+
)
8956+
.await;
8957+
let (result, entry, _) = expect_done(outcome);
8958+
assert!(result.success, "{purl}: {:?}", result.error);
8959+
state.entries.insert(purl.to_string(), entry.unwrap());
8960+
crate::vendor::state::save_state(fx.root(), &state)
8961+
.await
8962+
.unwrap();
8963+
}
8964+
fx
8965+
}
8966+
8967+
/// Revert `order` one entry at a time from a freshly loaded ledger,
8968+
/// saving after each removal (`vendor --revert`, `rollback` and
8969+
/// successive `remove` runs all persist per entry).
8970+
async fn revert_in_order(fx: &Fixture, order: [&str; 2]) {
8971+
for key in order {
8972+
let mut state = crate::vendor::state::load_state(fx.root()).await.unwrap();
8973+
let entry = state.entries.get(key).cloned().unwrap();
8974+
let outcome = revert_pnpm(&entry, fx.root(), false).await;
8975+
assert!(outcome.success, "{key}: {:?}", outcome.error);
8976+
assert!(outcome.warnings.is_empty(), "{key}: {:?}", outcome.warnings);
8977+
state.entries.remove(key);
8978+
crate::vendor::state::save_state(fx.root(), &state)
8979+
.await
8980+
.unwrap();
8981+
}
8982+
}
8983+
8984+
/// #636: the creator (is-number, first in purl order) reverted first
8985+
/// must not leave `"pnpm": { "overrides": {} }` or the scaffolded
8986+
/// pnpm-workspace.yaml behind once left-pad empties them.
8987+
#[tokio::test]
8988+
async fn revert_two_packages_creator_first_removes_created_scaffold() {
8989+
let fx = vendor_two(TWO_PKG, None).await;
8990+
assert!(fx.root().join(PNPM_WORKSPACE).exists());
8991+
8992+
revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await;
8993+
assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG);
8994+
assert_eq!(fx.read(PNPM_LOCK).await, TWO_LOCK);
8995+
assert!(
8996+
!fx.root().join(PNPM_WORKSPACE).exists(),
8997+
"scaffolded workspace file left behind"
8998+
);
8999+
}
9000+
9001+
/// #636, `remove` in the other order: clean before the fix too.
9002+
#[tokio::test]
9003+
async fn revert_two_packages_creator_last_removes_created_scaffold() {
9004+
let fx = vendor_two(TWO_PKG, None).await;
9005+
9006+
revert_in_order(&fx, [LEFT_PAD, IS_NUMBER]).await;
9007+
assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG);
9008+
assert!(!fx.root().join(PNPM_WORKSPACE).exists());
9009+
}
9010+
9011+
/// The `overrides:` section vendoring added to the user's own
9012+
/// pnpm-workspace.yaml goes too, in creator-first order; the file and
9013+
/// the user's keys stay.
9014+
#[tokio::test]
9015+
async fn revert_two_packages_removes_created_workspace_overrides() {
9016+
let ws = "packages:\n - '.'\n";
9017+
let fx = vendor_two(TWO_PKG, Some(ws)).await;
9018+
9019+
revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await;
9020+
assert_eq!(fx.read(PNPM_WORKSPACE).await, ws);
9021+
assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG);
9022+
}
9023+
9024+
/// A ledger written before the flags were shared (only the creator
9025+
/// flagged) is repaired on load, so it unwinds cleanly too.
9026+
#[tokio::test]
9027+
async fn revert_two_packages_repairs_a_creator_only_ledger() {
9028+
let fx = vendor_two(TWO_PKG, None).await;
9029+
let path = fx.root().join(".socket/vendor/state.json");
9030+
let mut ledger: serde_json::Value =
9031+
serde_json::from_slice(&tokio::fs::read(&path).await.unwrap()).unwrap();
9032+
ledger["entries"][LEFT_PAD]["pnpm"] = serde_json::json!({});
9033+
tokio::fs::write(&path, serde_json::to_vec_pretty(&ledger).unwrap())
9034+
.await
9035+
.unwrap();
9036+
9037+
revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await;
9038+
assert_eq!(fx.read(PACKAGE_JSON).await, TWO_PKG);
9039+
assert!(!fx.root().join(PNPM_WORKSPACE).exists());
9040+
}
9041+
9042+
/// A user's own `pnpm` table keeps its keys; only the overrides table
9043+
/// vendoring created goes.
9044+
#[tokio::test]
9045+
async fn revert_two_packages_keeps_a_user_pnpm_table() {
9046+
let pkg = TWO_PKG.replace(
9047+
" }\n}\n",
9048+
" },\n \"pnpm\": {\n \"onlyBuiltDependencies\": []\n }\n}\n",
9049+
);
9050+
let fx = vendor_two(&pkg, None).await;
9051+
9052+
revert_in_order(&fx, [IS_NUMBER, LEFT_PAD]).await;
9053+
assert_eq!(fx.read(PACKAGE_JSON).await, pkg);
9054+
}
88639055
}

0 commit comments

Comments
 (0)