From 8bd29b90d84e5622fa3f59c73120fa3573fa5ce1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 13:30:27 +0000 Subject: [PATCH 1/4] Start fix for #1782 Assisted-by: Claude Code:claude-opus-5-5 From cc2927349da934bc9a6fe899a9aacce522a2dd90 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 13:44:36 +0000 Subject: [PATCH 2/4] Keep an interrupted vendor eject hosted A plain `vendor` over hosted pins restored each pin's registry entry straight to disk (lock rewritten, hosted .npmrc removed) before the vendored apply opened its journaled group commit. A Ctrl-C, CI cancel or kill in that window left the lock pointing at the unpatched registry tarball with no journal to replay, and a re-run said "No manifest found, nothing to vendor" (exit 0). The eject now opens the group commit before the restore and hands it to the vendored apply, so the restore lands in the same journaled commit as the vendored wiring: killed before the journal, the project stays hosted and a re-run completes the eject; killed after it, the next locked command rolls forward. Fixes #1782 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/vendor.rs | 43 ++++++++++++--- crates/socket-patch-cli/tests/vendor_eject.rs | 53 +++++++++++++++++++ 2 files changed, 89 insertions(+), 7 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index bd85b661f..f7b27387c 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -2026,6 +2026,17 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { socket_patch_core::patch::redirect::vlt_heal::lock_targets(&lock, &origins, &pin_purls) }) .unwrap_or_default(); + // The restore runs inside the group commit the vendored apply commits + // (handed to it through the capture), so the registry entries it puts + // back land in the same journaled commit as the vendored wiring. An + // eject killed before that commit (SIGKILL, Ctrl-C, a CI cancel) leaves + // the project hosted, exactly as found; one killed after it rolls + // forward on the next locked command (#1782). + let mut capture = EjectCapture { + group: (!socket_patch_core::utils::failpoint::switched_off("group_commit")) + .then(|| GroupCommit::begin(&common.cwd)), + ..EjectCapture::default() + }; let restore = socket_patch_core::patch::redirect::upstream::restore_upstream( &common.cwd, &pins, @@ -2042,14 +2053,12 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { .map(|(_, why)| why.to_string()) .next() .or_else(|| restore.flush_error.clone()); - // Every file the vendored apply's group commit wrote, with its bytes - // from before (the rollback's scope beyond the restore's own files), - // and the flavors it wired for the close printed below. - let mut capture = EjectCapture::default(); // The vendored apply's events start here: a rollback retracts them. let events_start = env.events.len(); let mut exit: i32; if let Some(why) = restore_failure { + // Discard the captured restore before the rollback below writes. + drop(capture.group.take()); env.mark_error(EnvelopeError::new("redirect_revert_failed", why.clone())); if !common.json { eprintln!("Error: {}", crate::ui::sentence_case(&why)); @@ -2081,6 +2090,17 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { ) .await; exit = i32::from(applied); + // A vendored apply that returned before its commit leaves the + // group, and with it the captured restore, uncommitted: discard + // it, so the rollback below writes to disk, and undo the eject. + if capture.group.take().is_some() && exit == 0 { + let detail = "the vendored apply did not commit, so the eject is undone"; + if !common.json { + eprintln!("Error: {detail}"); + } + env.mark_error(EnvelopeError::new("eject_incomplete", detail)); + exit = 1; + } // Backstop: every ejected pin must now be vendored. A package the // backend skipped (a vendored gate condition the pre-pass did not // see) would otherwise leave the project neither hosted nor @@ -3146,9 +3166,15 @@ pub(crate) async fn vendor_records_reusing( // over. The replaced uuid dirs of re-vendored packages are swept only // after the commit, since until then the committed wiring still names // them. Dry runs write nothing and capture nothing. - let group = (!common.dry_run - && !socket_patch_core::utils::failpoint::switched_off("group_commit")) - .then(|| GroupCommit::begin(&common.cwd)); + // An eject hands over the group its upstream restore already wrote + // into, so the restore commits with this run's wiring. + let handed = eject + .as_deref_mut() + .and_then(|capture| capture.group.take()); + let group = handed.or_else(|| { + (!common.dry_run && !socket_patch_core::utils::failpoint::switched_off("group_commit")) + .then(|| GroupCommit::begin(&common.cwd)) + }); let mut stale_artifacts: Vec = Vec::new(); // The source of a superseded install (see [`superseded_install`]). let uninstalled = common.cwd.join(".socket/vendor/.uninstalled"); @@ -4153,8 +4179,11 @@ async fn remove_new_vendor_dirs( /// printing its own close: every project file its group commit wrote (with /// the bytes from before, for the rollback) and the lockfile flavors it /// wired (for the "Next steps:" the eject prints only when it completes). +/// The eject hands its open group commit IN through `group`, holding the +/// upstream restore; the vendored apply takes it as its own. #[derive(Default)] pub(crate) struct EjectCapture { + pub(crate) group: Option, pub(crate) committed: Vec, pub(crate) wired_flavors: HashSet, } diff --git a/crates/socket-patch-cli/tests/vendor_eject.rs b/crates/socket-patch-cli/tests/vendor_eject.rs index 77510068d..95c0cc676 100644 --- a/crates/socket-patch-cli/tests/vendor_eject.rs +++ b/crates/socket-patch-cli/tests/vendor_eject.rs @@ -75,6 +75,11 @@ impl Project { /// Run the binary against the mocks in the hermetic `SOCKET_*` /// environment. fn run(&self, args: &[&str]) -> (i32, String, String) { + self.run_with_env(args, &[]) + } + + /// [`Self::run`] with extra environment variables (a failpoint). + fn run_with_env(&self, args: &[&str], extra: &[(&str, &str)]) -> (i32, String, String) { let mut cmd = hermetic::binary_command(); cmd.args(args) .arg("--cwd") @@ -87,6 +92,9 @@ impl Project { .env("SOCKET_NPM_REGISTRY", &uri) .env("SOCKET_PATCH_SERVER_URL", &uri) .env("SOCKET_VENDOR_SOURCE", "service"); + for (key, value) in extra { + cmd.env(key, value); + } let out = cmd.output().expect("spawn socket-patch"); ( out.status.code().unwrap_or(-1), @@ -270,6 +278,51 @@ async fn eject_vendors_hosted_pins_and_revert_returns_to_upstream() { assert!(!p.redirect_state().exists()); } +/// #1782: an eject that dies before its group commit (a SIGKILL, a Ctrl-C, +/// a CI cancel) leaves the project hosted, exactly as found: the upstream +/// restore is captured with the vendored wiring and lands only with the +/// commit's journal. A re-run then sees the hosted pin and completes the +/// eject, instead of answering `noManifest` over an unpatched lock. +#[tokio::test] +async fn eject_killed_before_its_commit_stays_hosted_and_reruns() { + let p = hosted_project(true).await; + mock_registry(&p).await; + mock_view(&p).await; + let before = p.lock_bytes(); + + let (code, stdout, stderr) = p.run_with_env( + &["vendor", "--json"], + &[("SOCKET_PATCH_FAILPOINT", "vendor_group_commit")], + ); + assert_eq!( + code, 86, + "the failpoint kills the eject\n{stdout}\n{stderr}" + ); + assert_eq!( + p.lock_bytes(), + before, + "nothing reached the lock before the commit: it still pins the hosted tarball" + ); + assert!(!p + .root() + .join(socket_patch_core::utils::group_commit::COMMIT_JOURNAL_REL) + .exists()); + assert!(!p.root().join(".socket/vendor/state.json").exists()); + + let (code, env) = p.run_json(&["vendor"]); + assert_eq!(code, 0, "the re-run ejects: {env:#}"); + assert_eq!(find_event(&env, "applied", None)["purl"], PURL, "{env:#}"); + let lock = p.lock(); + let resolved = lock["packages"]["node_modules/left-pad"]["resolved"] + .as_str() + .unwrap_or_default() + .to_string(); + assert!( + resolved.contains(&format!(".socket/vendor/npm/{UUID}/left-pad-1.3.0.tgz")), + "{lock:#}" + ); +} + /// Hosted pins remain the input when an agent manifest is present. The /// manifest does not list this patch and nothing is installed locally. #[tokio::test] From fa35cf3741367a1d9658b3a357e8f933b3082e1f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 14:42:15 +0000 Subject: [PATCH 3/4] Harden the journaled eject restore Follow-ups from review of the eject crash fix: - Keep the old, unjournaled restore for a Gradle pin: its hosted index and settings script are written straight to disk, so a half-captured restore killed mid-run would leave settings.gradle applying a deleted script. Same when --manifest-path roots the vendored apply somewhere other than --cwd. - When a failed commit keeps its journal for the next command to finish, don't put the hosted files back first: they would match the journal's old bytes and be replayed over anyway. - A re-run of `vendor` replays a pending commit journal before it reads the lockfiles, so it no longer tries to eject a hosted pin the replay is about to replace. - The vendored apply ignores a handed group in a dry run. Refs #1782 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/vendor.rs | 60 ++++++++++++++++--- crates/socket-patch-cli/tests/vendor_eject.rs | 44 ++++++++++++++ 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index f7b27387c..05e5039f9 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -958,6 +958,24 @@ pub async fn run(args: VendorArgs) -> i32 { // The raw lockfile inventory does not require VEX eligibility or an // installed copy of a patch. Revert needs only the vendor ledger. if !args.revert { + // A commit journal a killed eject left is replayed (by taking the + // apply lock) before the lockfiles are read: the pins must come + // off the committed state, or a re-run would eject again from the + // hosted lock the replay is about to replace (#1782). + if tokio::fs::symlink_metadata( + args.common + .cwd + .join(socket_patch_core::utils::group_commit::COMMIT_JOURNAL_REL), + ) + .await + .is_ok() + { + let timeout = Duration::from_secs(args.common.lock_timeout.unwrap_or(0)); + if let Err(e) = crate::commands::lock_cli::acquire_with_status(&socket_dir, timeout) { + let (code, message) = crate::commands::lock_cli::lock_failure(&e, timeout); + return emit_eject_refusal(&args.common, code, &message); + } + } let inventory = crate::commands::hosted_inventory(&args.common, &args.common.cwd).await; // Contested hosted wiring: the patch set cannot be read off the // lockfiles, and a "nothing to vendor" answer would hide it. With @@ -2031,10 +2049,18 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { // back land in the same journaled commit as the vendored wiring. An // eject killed before that commit (SIGKILL, Ctrl-C, a CI cancel) leaves // the project hosted, exactly as found; one killed after it rolls - // forward on the next locked command (#1782). + // forward on the next locked command (#1782). Only when the overlay + // captures every file the restore writes (a Gradle pin's hosted index + // and settings script under `.socket/gradle/` go straight to disk, and + // a half-captured restore would leave settings.gradle applying a + // deleted script) and the vendored apply runs at this same root. + let journaled_restore = !socket_patch_core::utils::failpoint::switched_off("group_commit") + && common.project_root() == common.cwd + && touched + .iter() + .all(|rel| socket_patch_core::utils::group_commit::captures(rel)); let mut capture = EjectCapture { - group: (!socket_patch_core::utils::failpoint::switched_off("group_commit")) - .then(|| GroupCommit::begin(&common.cwd)), + group: journaled_restore.then(|| GroupCommit::begin(&common.cwd)), ..EjectCapture::default() }; let restore = socket_patch_core::patch::redirect::upstream::restore_upstream( @@ -2131,9 +2157,28 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { } } let committed = &capture.committed; + // A commit that failed part-way and could not put its files back keeps + // its journal, and the next locked command rolls it forward: putting + // the hosted files back now would match the journal's recorded old + // bytes and be replayed over, so the rollback is left to that replay + // (the error the vendored apply recorded already says so). + let commit_pending = exit != 0 + && tokio::fs::symlink_metadata( + common + .cwd + .join(socket_patch_core::utils::group_commit::COMMIT_JOURNAL_REL), + ) + .await + .is_ok(); if exit != 0 { - match snapshot.restore(&restore.reverted_files, committed).await { - Ok(()) => { + let rollback = if commit_pending { + None + } else { + Some(snapshot.restore(&restore.reverted_files, committed).await) + }; + match rollback { + None => {} + Some(Ok(())) => { let detail = "the eject did not complete, so every file it touched was \ restored: the project is still hosted, exactly as before"; // Nothing the vendored apply did survives the rollback: a @@ -2152,7 +2197,7 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { detail: detail.to_string(), }); } - Err(e) => { + Some(Err(e)) => { let detail = format!( "the eject did not complete and restoring the pre-eject files failed ({e}); \ restore them from version control (`git checkout -- {}`)", @@ -3170,7 +3215,8 @@ pub(crate) async fn vendor_records_reusing( // into, so the restore commits with this run's wiring. let handed = eject .as_deref_mut() - .and_then(|capture| capture.group.take()); + .and_then(|capture| capture.group.take()) + .filter(|_| !common.dry_run); let group = handed.or_else(|| { (!common.dry_run && !socket_patch_core::utils::failpoint::switched_off("group_commit")) .then(|| GroupCommit::begin(&common.cwd)) diff --git a/crates/socket-patch-cli/tests/vendor_eject.rs b/crates/socket-patch-cli/tests/vendor_eject.rs index 95c0cc676..ade21f42a 100644 --- a/crates/socket-patch-cli/tests/vendor_eject.rs +++ b/crates/socket-patch-cli/tests/vendor_eject.rs @@ -323,6 +323,50 @@ async fn eject_killed_before_its_commit_stays_hosted_and_reruns() { ); } +/// #1782, the other half: an eject killed once its commit journal is +/// durable is finished by the next command that takes the apply lock, so +/// the restore and the vendored wiring land together (never a lock back +/// on the registry with no vendored entry). +#[tokio::test] +async fn eject_killed_after_its_journal_rolls_forward() { + let p = hosted_project(true).await; + mock_registry(&p).await; + mock_view(&p).await; + + let (code, stdout, stderr) = p.run_with_env( + &["vendor", "--json"], + &[("SOCKET_PATCH_FAILPOINT", "group_commit_journal")], + ); + assert_eq!( + code, 86, + "the failpoint kills the eject\n{stdout}\n{stderr}" + ); + assert!(p + .root() + .join(socket_patch_core::utils::group_commit::COMMIT_JOURNAL_REL) + .exists()); + + let (code, env) = p.run_json(&["vendor"]); + assert_eq!(code, 0, "{env:#}"); + assert!(!p + .root() + .join(socket_patch_core::utils::group_commit::COMMIT_JOURNAL_REL) + .exists()); + let lock = p.lock(); + let resolved = lock["packages"]["node_modules/left-pad"]["resolved"] + .as_str() + .unwrap_or_default() + .to_string(); + assert!( + resolved.contains(&format!(".socket/vendor/npm/{UUID}/left-pad-1.3.0.tgz")), + "the replay wired the vendored artifact: {lock:#}" + ); + let state: Value = + serde_json::from_slice(&std::fs::read(p.root().join(".socket/vendor/state.json")).unwrap()) + .unwrap(); + assert!(state["entries"].get(PURL).is_some(), "{state:#}"); +} + /// Hosted pins remain the input when an agent manifest is present. The /// manifest does not list this patch and nothing is installed locally. #[tokio::test] From 1ff7c0cb0d36064c7e94cc94335df930205c8af7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 21:23:27 +0000 Subject: [PATCH 4/4] Never commit a partly failed vendor eject An eject's group commit also holds every pin's upstream restore. When one package failed to vendor, the vendored apply still committed the group, journaling that package on the plain registry entry. Only the eject's later rollback undid it, so a kill before that rollback (or a commit left pending) let the next command roll the package forward, unpatched and untracked. The vendored apply now drops a handed-in eject group unless every pin is vendored in the captured ledger. Nothing reaches the lockfiles, and the downloaded artifacts are removed, so the project stays hosted. Refs #1782 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/vendor.rs | 29 +++++++++++++++++++ .../tests/vendor_eject_fresh_checkout.rs | 18 ++++++++++++ 2 files changed, 47 insertions(+) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 05e5039f9..7b49eb090 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -3217,6 +3217,7 @@ pub(crate) async fn vendor_records_reusing( .as_deref_mut() .and_then(|capture| capture.group.take()) .filter(|_| !common.dry_run); + let eject_group = handed.is_some(); let group = handed.or_else(|| { (!common.dry_run && !socket_patch_core::utils::failpoint::switched_off("group_commit")) .then(|| GroupCommit::begin(&common.cwd)) @@ -3898,6 +3899,34 @@ pub(crate) async fn vendor_records_reusing( // others failed — a failed package's backend already put back what it // had touched, in the captured state — so a completed run ends exactly // where committing after every package would have left it. + // + // Not for an eject: its group also holds every pin's upstream restore, + // so committing past a failed or skipped package would land that + // package on the plain registry entry, unpatched, and a crash (or a + // commit left pending) before the eject's rollback would make it stick. + // The eject is all-or-nothing, so it drops the group unless every pin + // is vendored in the captured ledger; nothing of it reached the disk + // but the artifacts, which are removed here (#1782). + let group = match group { + Some(group) if eject_group => { + let ledger = load_state(&common.cwd).await.ok(); + let complete = !has_errors + && records.keys().all(|purl| { + ledger + .as_ref() + .is_some_and(|l| lookup_entry(&l.entries, purl).is_some()) + }); + if complete { + Some(group) + } else { + drop(group); + has_errors = true; + let _ = remove_new_vendor_dirs(common, vendor_dirs_before.as_ref()).await; + None + } + } + group => group, + }; let mut commit_failed = false; if let Some(group) = group { socket_patch_core::utils::failpoint::hit("vendor_group_commit"); diff --git a/crates/socket-patch-cli/tests/vendor_eject_fresh_checkout.rs b/crates/socket-patch-cli/tests/vendor_eject_fresh_checkout.rs index 28e0d3244..d80bcd8b4 100644 --- a/crates/socket-patch-cli/tests/vendor_eject_fresh_checkout.rs +++ b/crates/socket-patch-cli/tests/vendor_eject_fresh_checkout.rs @@ -723,4 +723,22 @@ async fn rolled_back_eject_reports_nothing_vendored() { !stdout.contains("Vendored") && !stdout.contains("Next steps"), "nothing was vendored: {stdout}\n{stderr}" ); + + // #1782 review: the eject's group also holds ms's upstream restore, so + // it must never be committed past ms's failure. A commit would journal + // ms on the plain registry entry, and a kill at that point (here the + // journal failpoint) would make the next locked command roll it + // forward, unpatched. + let root = fresh("killed"); + let (code, env) = run_json_with( + &root, + &server, + &["vendor"], + &[("SOCKET_PATCH_FAILPOINT", "group_commit_journal".to_string())], + ); + assert_eq!(code, 1, "the failed eject never reaches a commit: {env:#}"); + assert!(!root + .join(socket_patch_core::utils::group_commit::COMMIT_JOURNAL_REL) + .exists()); + still_hosted(&root); }