From 66fb68c2ee958c38198d56c8d694724199bfc8e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 20:22:20 +0000 Subject: [PATCH 1/4] Start fix for #1084 Assisted-by: Claude Code:claude-opus-5-5 From 6a3f2fe987fc362463138097dbce0a58056c02f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 20:30:33 +0000 Subject: [PATCH 2/4] Test rollback of an orphaned superseded copy Regression tests for #1084. On Bun's isolated linker a hosted reinstall links the package to a new store entry, but the old node_modules/.bun/@ entry still holds the agent patch. Rollback and remove of the superseded agent record must restore that orphan before dropping the record, and must fail and keep the record when the orphan cannot be restored. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/in_process_rollback_hosted.rs | 127 +++++++++++++++++- 1 file changed, 125 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs index 5091adb59..84539f1be 100644 --- a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs +++ b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs @@ -1144,8 +1144,9 @@ async fn a_git_pattern_hosted_pin_is_refused_not_restored_to_the_registry() { assert!( envelope["hosted"]["failed"][0]["error"] .as_str() - .is_some_and(|e| e.contains("installs from git") - && e.contains("`git checkout -- yarn.lock`")), + .is_some_and( + |e| e.contains("installs from git") && e.contains("`git checkout -- yarn.lock`") + ), "{envelope}" ); assert_eq!( @@ -1568,3 +1569,125 @@ async fn remove_unhosts_a_package_whose_agent_record_is_superseded() { "the superseded record is reported:\n{envelope:#}" ); } + +/// Re-shape the superseded fixture as Bun's isolated linker leaves it +/// after `bun install` of the hosted pin (#1084): `node_modules/` +/// links to the new store entry holding B's bytes, and the old +/// `node_modules/.bun/@` entry Bun never prunes still holds +/// agent record A's patched bytes. Returns the orphan's `index.js`. +#[cfg(unix)] +fn orphan_bun_store_copy(root: &Path) -> std::path::PathBuf { + let nm = root.join("node_modules"); + let top = nm.join(NAME); + let store = nm.join(".bun"); + let live = store + .join(format!("{NAME}@http+++patch.test+bbbbbbbb")) + .join("node_modules") + .join(NAME); + let orphan = store + .join(format!("{NAME}@{VERSION}")) + .join("node_modules") + .join(NAME); + for (dir, index) in [(&live, B_PATCHED_INDEX), (&orphan, A_PATCHED_INDEX)] { + std::fs::create_dir_all(dir).unwrap(); + std::fs::copy(top.join("package.json"), dir.join("package.json")).unwrap(); + std::fs::write(dir.join("index.js"), index).unwrap(); + } + std::fs::remove_dir_all(&top).unwrap(); + std::os::unix::fs::symlink(&live, &top).unwrap(); + orphan.join("index.js") +} + +/// #1084: the live copy holds B's bytes, but an orphaned Bun store copy +/// still holds A's. Rollback restores that copy before dropping record A, +/// so the next `bun install` cannot relink agent patch A unrecorded. +#[cfg(unix)] +#[tokio::test] +#[serial] +async fn rollback_restores_an_orphaned_store_copy_of_a_superseded_record() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + let pristine = write_superseded_agent_fixture(tmp.path(), &server, B_PATCHED_INDEX).await; + let orphan = orphan_bun_store_copy(tmp.path()); + + let (code, envelope) = run_rollback_subprocess_online(tmp.path(), &server, &[]); + assert_eq!(code, 0, "{envelope:#}"); + assert_eq!( + std::fs::read(&orphan).unwrap(), + ORIGINAL_INDEX, + "the orphaned store copy at A's patched bytes is restored:\n{envelope:#}" + ); + let live = tmp.path().join("node_modules").join(NAME).join("index.js"); + assert_eq!( + std::fs::read(live).unwrap(), + B_PATCHED_INDEX, + "B's live copy is still the reinstall's to replace" + ); + assert!( + warning_codes(&envelope).contains(&"rollback_record_superseded".to_string()), + "{envelope:#}" + ); + let restored = std::fs::read_to_string(tmp.path().join("package-lock.json")).unwrap(); + assert_eq!(restored, pristine); + assert!(manifest_patch_keys(tmp.path()).is_empty()); +} + +/// #1084: `remove ` restores the orphaned copy the same way. +#[cfg(unix)] +#[tokio::test] +#[serial] +async fn remove_restores_an_orphaned_store_copy_of_a_superseded_record() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + let pristine = write_superseded_agent_fixture(tmp.path(), &server, B_PATCHED_INDEX).await; + let orphan = orphan_bun_store_copy(tmp.path()); + + let (code, envelope) = run_remove_subprocess_online(tmp.path(), &server, PURL); + assert_eq!(code, 0, "{envelope:#}"); + assert_eq!( + std::fs::read(&orphan).unwrap(), + ORIGINAL_INDEX, + "the orphaned store copy at A's patched bytes is restored:\n{envelope:#}" + ); + let restored = std::fs::read_to_string(tmp.path().join("package-lock.json")).unwrap(); + assert_eq!(restored, pristine); + assert!(manifest_patch_keys(tmp.path()).is_empty()); +} + +/// #1084: a store copy that holds A's bytes but cannot be restored (its +/// before-blob is gone) fails the run and keeps record A, instead of +/// dropping the only data that could restore it. +#[cfg(unix)] +#[tokio::test] +#[serial] +async fn an_unrestorable_orphaned_store_copy_keeps_the_superseded_record() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + write_superseded_agent_fixture(tmp.path(), &server, B_PATCHED_INDEX).await; + let orphan = orphan_bun_store_copy(tmp.path()); + std::fs::write(&orphan, b"module.exports = 'patched by A'; // and edited\n").unwrap(); + // A second file of the copy still at A's patched bytes keeps it A's. + let before = socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes(ORIGINAL_INDEX); + let after = socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes(A_PATCHED_INDEX); + let manifest_path = tmp.path().join(".socket/manifest.json"); + let mut manifest: Value = + serde_json::from_str(&std::fs::read_to_string(&manifest_path).unwrap()).unwrap(); + manifest["patches"][PURL]["files"]["package/lib.js"] = + serde_json::json!({ "beforeHash": before, "afterHash": after }); + std::fs::write(&manifest_path, manifest.to_string()).unwrap(); + std::fs::write(orphan.with_file_name("lib.js"), A_PATCHED_INDEX).unwrap(); + // B's live copy never carried A's lib.js change. + let live = tmp.path().join("node_modules").join(NAME).join("lib.js"); + std::fs::write(live, ORIGINAL_INDEX).unwrap(); + + let (code, envelope) = run_rollback_subprocess_online(tmp.path(), &server, &[]); + assert_eq!( + code, 1, + "a copy still holding A's bytes that cannot be restored fails:\n{envelope:#}" + ); + assert_eq!( + manifest_patch_keys(tmp.path()), + vec![PURL.to_string()], + "record A and its blobs stay for a later rollback" + ); +} From 8b19551b641b4e120fc8f90223f659868d784ca8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 20:30:33 +0000 Subject: [PATCH 3/4] Restore store copies a superseded record patched When a hosted pin superseded an agent-mode patch, rollback and remove left the installed copy to the lockfile restore and dropped the agent record and its blobs. A copy is only checked for its other store copies after the main copy rolls back, so on Bun's isolated linker the orphaned store entry that still held the agent patch was never restored. The next `bun install` linked it again, with no record left to roll it back. Rollback now restores every store copy that still holds the record's patched bytes before dropping the record. If one of those copies can't be restored, the run fails (exit 1) and keeps the record, as it did before the supersede handling was added. Fixes #1084 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/rollback.rs | 25 ++++++- .../socket-patch-core/src/patch/rollback.rs | 69 +++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 950bf5af9..88e42e14b 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -2659,10 +2659,29 @@ pub(crate) async fn rollback_patches_inner( continue; } let files = target.files.as_ref().unwrap_or(&patch.files); + let mut result = result; if let Some(warning) = superseded_record_skip(target, &result, files, superseded).await { - warnings.push(warning); - superseded_left.push(purl.clone()); - continue; + // The superseded primary never reached its store copies; one + // still at this record's patched bytes (Bun's orphaned + // isolated-store entry, #1084) is restored here, or fails the + // run and keeps the record, before the record is dropped. + match socket_patch_core::patch::rollback::rollback_store_copies_holding_patch( + purl, + pkg_path, + files, + &blobs_path, + common.dry_run, + ) + .await + { + Some(copies) if !copies.success => result = copies, + restored => { + warnings.push(warning); + superseded_left.push(purl.clone()); + results.extend(restored); + continue; + } + } } if !result.success { has_errors = true; diff --git a/crates/socket-patch-core/src/patch/rollback.rs b/crates/socket-patch-core/src/patch/rollback.rs index fc9cd3354..271084c78 100644 --- a/crates/socket-patch-core/src/patch/rollback.rs +++ b/crates/socket-patch-core/src/patch/rollback.rs @@ -371,6 +371,75 @@ pub async fn rollback_package_patch( .await } +/// Roll back the other pnpm/vlt/Bun store copies of `pkg_path` that still +/// hold this record's patched bytes, when the primary copy was left to a +/// superseding hosted pin (#1084). +/// +/// [`rollback_package_patch`] visits a package's store copies only after +/// the primary succeeds. A superseded primary fails (it holds the hosted +/// patch's bytes), so its copies are never reached. On Bun's isolated +/// linker the store entry the hosted reinstall orphaned is such a copy: +/// it still holds the agent patch, and the next `bun install` can link it +/// again. Each copy holding any of the record's patched bytes (or a file +/// that can't be checked) is rolled back with the single-copy engine and +/// folded into the returned result, which fails if any copy fails. Copies +/// holding neither side are left, as the primary is. `None` when no copy +/// holds the record's patched bytes (or `package_key` is not npm). +pub async fn rollback_store_copies_holding_patch( + package_key: &str, + pkg_path: &Path, + files: &HashMap, + blobs_path: &Path, + dry_run: bool, +) -> Option { + if !package_key.starts_with("pkg:npm/") { + return None; + } + let mut folded: Option = None; + for copy in crate::crawlers::npm_crawler::find_store_peer_variant_copies(pkg_path).await { + if !holds_patched_bytes(©, files).await { + continue; + } + let copy_result = + rollback_package_patch_at(package_key, ©, files, blobs_path, dry_run).await; + let result = folded.get_or_insert_with(|| RollbackResult { + package_key: package_key.to_string(), + package_path: pkg_path.display().to_string(), + success: true, + files_verified: Vec::new(), + files_rolled_back: Vec::new(), + error: None, + sidecar: None, + }); + crate::patch::store_copies::fold(result, ©, copy_result); + } + folded +} + +/// Whether any of `files` under `dir` is at the record's patched bytes, or +/// cannot be checked (an unsafe key, a read error other than "not found"), +/// which may hide them. +async fn holds_patched_bytes(dir: &Path, files: &HashMap) -> bool { + for (file, info) in files { + let rel = normalize_file_path(file); + if !crate::patch::apply::is_safe_relative_subpath(rel) { + return true; + } + // FIFO-safe: a FIFO or device at the leaf is refused, not opened. + match crate::utils::fs::read_regular_to_bytes(&dir.join(rel)).await { + Ok(bytes) => { + if crate::hash::git_sha256::compute_git_sha256_from_bytes(&bytes) == info.after_hash + { + return true; + } + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(_) => return true, + } + } + false +} + impl crate::patch::store_copies::CopyFold for RollbackResult { const VERB: &'static str = "roll back"; From 6a8703b0fe936b2ed12b0abc0f910c239f3f1fcd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 15:34:34 +0000 Subject: [PATCH 4/4] Leave store copies the superseding patch owns A store copy was restored for a superseded record when any one of its files held the record's patched bytes. A copy of the superseding hosted patch that shares a file with the record (same patched lib.js, its own index.js) passed that check, then failed verification on the other file. The run exited 1 and kept the record on every later rollback, where it used to drop the record cleanly. A copy is now restored only when every file is at the record's patched or original bytes and at least one is patched or can't be checked. A file at any other bytes marks the copy as not the record's, the same rule that leaves the primary to the lockfile restore. The fail-closed test now makes the orphan unrestorable by removing its before-blob, since an edited file no longer counts as the record's copy. Assisted-by: Claude Code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014GeZsEc88AKZsFkBs86Ruy --- .../tests/in_process_rollback_hosted.rs | 53 ++++++++++++++----- .../socket-patch-core/src/patch/rollback.rs | 46 ++++++++++------ 2 files changed, 70 insertions(+), 29 deletions(-) diff --git a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs index c32076f73..c1fa514dd 100644 --- a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs +++ b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs @@ -1664,9 +1664,35 @@ async fn an_unrestorable_orphaned_store_copy_keeps_the_superseded_record() { let server = MockServer::start().await; let tmp = tempfile::tempdir().unwrap(); write_superseded_agent_fixture(tmp.path(), &server, B_PATCHED_INDEX).await; - let orphan = orphan_bun_store_copy(tmp.path()); - std::fs::write(&orphan, b"module.exports = 'patched by A'; // and edited\n").unwrap(); - // A second file of the copy still at A's patched bytes keeps it A's. + orphan_bun_store_copy(tmp.path()); + let before = socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes(ORIGINAL_INDEX); + std::fs::remove_file(tmp.path().join(".socket/blobs").join(before)).unwrap(); + + let (code, envelope) = run_rollback_subprocess_online(tmp.path(), &server, &[]); + assert_eq!( + code, 1, + "a copy still holding A's bytes that cannot be restored fails:\n{envelope:#}" + ); + assert_eq!( + manifest_patch_keys(tmp.path()), + vec![PURL.to_string()], + "record A stays for a later rollback" + ); +} + +/// #1084 review: a store copy of the superseding patch B that shares a +/// file with record A (B's `lib.js` is A's patched `lib.js`, its +/// `index.js` is B's own) is not A's copy. It is left, as the primary is, +/// and the run drops record A as before instead of failing on it forever. +#[cfg(unix)] +#[tokio::test] +#[serial] +async fn a_superseding_store_copy_sharing_a_file_with_the_record_is_left() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + write_superseded_agent_fixture(tmp.path(), &server, B_PATCHED_INDEX).await; + let copy = orphan_bun_store_copy(tmp.path()); + // Record A patched lib.js too; B carries the same patched lib.js. let before = socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes(ORIGINAL_INDEX); let after = socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes(A_PATCHED_INDEX); let manifest_path = tmp.path().join(".socket/manifest.json"); @@ -1675,21 +1701,24 @@ async fn an_unrestorable_orphaned_store_copy_keeps_the_superseded_record() { manifest["patches"][PURL]["files"]["package/lib.js"] = serde_json::json!({ "beforeHash": before, "afterHash": after }); std::fs::write(&manifest_path, manifest.to_string()).unwrap(); - std::fs::write(orphan.with_file_name("lib.js"), A_PATCHED_INDEX).unwrap(); - // B's live copy never carried A's lib.js change. + std::fs::write(©, B_PATCHED_INDEX).unwrap(); + std::fs::write(copy.with_file_name("lib.js"), A_PATCHED_INDEX).unwrap(); let live = tmp.path().join("node_modules").join(NAME).join("lib.js"); - std::fs::write(live, ORIGINAL_INDEX).unwrap(); + std::fs::write(live, A_PATCHED_INDEX).unwrap(); let (code, envelope) = run_rollback_subprocess_online(tmp.path(), &server, &[]); - assert_eq!( - code, 1, - "a copy still holding A's bytes that cannot be restored fails:\n{envelope:#}" + assert_eq!(code, 0, "{envelope:#}"); + assert!( + warning_codes(&envelope).contains(&"rollback_record_superseded".to_string()), + "{envelope:#}" ); + assert_eq!(std::fs::read(©).unwrap(), B_PATCHED_INDEX); assert_eq!( - manifest_patch_keys(tmp.path()), - vec![PURL.to_string()], - "record A and its blobs stay for a later rollback" + std::fs::read(copy.with_file_name("lib.js")).unwrap(), + A_PATCHED_INDEX, + "B's copy is the reinstall's to replace" ); + assert!(manifest_patch_keys(tmp.path()).is_empty()); } /// B07: a remove/rollback identifier that names the superseded record's diff --git a/crates/socket-patch-core/src/patch/rollback.rs b/crates/socket-patch-core/src/patch/rollback.rs index 8d9d556b7..0c7177d49 100644 --- a/crates/socket-patch-core/src/patch/rollback.rs +++ b/crates/socket-patch-core/src/patch/rollback.rs @@ -380,11 +380,12 @@ pub async fn rollback_package_patch( /// patch's bytes), so its copies are never reached. On Bun's isolated /// linker the store entry the hosted reinstall orphaned is such a copy: /// it still holds the agent patch, and the next `bun install` can link it -/// again. Each copy holding any of the record's patched bytes (or a file -/// that can't be checked) is rolled back with the single-copy engine and -/// folded into the returned result, which fails if any copy fails. Copies -/// holding neither side are left, as the primary is. `None` when no copy -/// holds the record's patched bytes (or `package_key` is not npm). +/// again. Each copy [`holds_this_patch`] is rolled back with the +/// single-copy engine and folded into the returned result, which fails if +/// any copy fails. A copy with any file at bytes the record never wrote +/// (the superseding patch's own copies, which can share a file with this +/// record) is left, as the primary is. `None` when no copy holds this +/// record's patch (or `package_key` is not npm). pub async fn rollback_store_copies_holding_patch( package_key: &str, pkg_path: &Path, @@ -397,7 +398,7 @@ pub async fn rollback_store_copies_holding_patch( } let mut folded: Option = None; for copy in crate::crawlers::npm_crawler::find_store_peer_variant_copies(pkg_path).await { - if !holds_patched_bytes(©, files).await { + if !holds_this_patch(©, files).await { continue; } let copy_result = @@ -416,28 +417,39 @@ pub async fn rollback_store_copies_holding_patch( folded } -/// Whether any of `files` under `dir` is at the record's patched bytes, or -/// cannot be checked (an unsafe key, a read error other than "not found"), -/// which may hide them. -async fn holds_patched_bytes(dir: &Path, files: &HashMap) -> bool { +/// Whether `dir` is a copy this record patched: every file is at the +/// record's patched or original bytes (a new file may be absent), and at +/// least one is at the patched bytes or cannot be checked (an unsafe key, +/// a read error other than "not found"), which may hide them. A file at +/// any other bytes, or a missing original file, marks a copy the record +/// does not own, just as it marks the primary superseded. +async fn holds_this_patch(dir: &Path, files: &HashMap) -> bool { + let mut patched = false; for (file, info) in files { let rel = normalize_file_path(file); if !crate::patch::apply::is_safe_relative_subpath(rel) { - return true; + patched = true; + continue; } // FIFO-safe: a FIFO or device at the leaf is refused, not opened. match crate::utils::fs::read_regular_to_bytes(&dir.join(rel)).await { Ok(bytes) => { - if crate::hash::git_sha256::compute_git_sha256_from_bytes(&bytes) == info.after_hash - { - return true; + let hash = crate::hash::git_sha256::compute_git_sha256_from_bytes(&bytes); + if hash == info.after_hash { + patched = true; + } else if hash != info.before_hash { + return false; + } + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + if !info.before_hash.is_empty() { + return false; } } - Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} - Err(_) => return true, + Err(_) => patched = true, } } - false + patched } impl crate::patch::store_copies::CopyFold for RollbackResult {