From 5a7a78690bd2823babc1def3eebcb05878ca01e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 14:26:29 +0000 Subject: [PATCH 1/2] Skip on-disk gem preview in dry takeovers A dry hosted-to-vendored takeover previews the vendored backend over the restored text, then again over the files on disk. For a gem, the Gemfile on disk still wraps the declaration in the hosted `source ... do` block, so the second preview, which now checks the Gemfile edit, could refuse it as indented while the real run vendors it. Stop after the restored-text preview for gems, as bun and sbt already do. Adds an e2e test that a top-level hosted gem previews a clean eject and then ejects for real. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/vendor.rs | 17 ++-- .../tests/e2e_redirect_gem_build.rs | 84 +++++++++++++++++++ 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 7effb167a..cdd7286f7 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -3516,11 +3516,18 @@ pub(crate) async fn vendor_records_reusing( // disk, so the sbt planner would refuse the // `vendor_sbt_hosted_conflict` the wet run (which // restores first) never sees. - if restore.reverted_files.iter().any(|f| { - f == "bun.lock" - || f == "bun.lockb" - || f == socket_patch_core::formats::sbt::owned_file::HOSTED_FILE - }) { + // + // And a gem: its hosted `source … do` block still wraps + // the declaration on disk, so the backend's Gemfile + // planner would refuse it as indented. The preview over + // the restored text above already asked the backend. + if candidate.starts_with("pkg:gem/") + || restore.reverted_files.iter().any(|f| { + f == "bun.lock" + || f == "bun.lockb" + || f == socket_patch_core::formats::sbt::owned_file::HOSTED_FILE + }) + { continue; } } else { diff --git a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs index 55bc5c17e..330f3cd17 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -2945,6 +2945,90 @@ async fn gem_hosted_group_block_pin_survives_a_refused_eject_of_a_converged_lock } } +/// The other side of #1761: a top-level hosted gem the vendored backend +/// can wire previews a clean `vendor --dry-run` eject (exit 0), and the wet +/// `vendor` then ejects it. The dry run checks the Gemfile/lock edits over +/// the restored text; the hosted `source … do` block that still wraps the +/// declaration on disk must not be read as the user's own indented +/// declaration. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 1.17); \ + the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"] +async fn gem_hosted_pin_previews_and_runs_a_clean_eject() { + let Some(fx) = redirect_scanned_project( + "hosted clean eject", + Spelling::Gemfile, + false, + true, + None, + Driver::ScanVex, + ) + .await + else { + return; + }; + // Converge the hosted pair, so the eject can restore its upstream entry. + let install = bundle(&fx.proj, &["install"]); + assert!( + install.status.success(), + "converging install:\n{}", + String::from_utf8_lossy(&install.stderr) + ); + mount_vendored_grant(&fx).await; + let proj = fx.proj.to_str().expect("utf8 tmp path"); + let api = fx._server.uri(); + for dry_run in [true, false] { + let before: Vec> = ["Gemfile", "Gemfile.lock"] + .iter() + .map(|f| std::fs::read(fx.proj.join(f)).unwrap()) + .collect(); + let mut argv = vec![ + "vendor", + "--json", + "--yes", + "--cwd", + proj, + "--api-url", + &api, + "--org", + ORG, + "--api-token", + "fake", + "--patch-server-url", + &api, + ]; + if dry_run { + argv.push("--dry-run"); + } + let label = format!("vendor eject (dry_run={dry_run})"); + let (code, stdout, stderr) = run_socket(&fx.proj, &argv); + assert_eq!(code, 0, "{label}:\nstdout:\n{stdout}\nstderr:\n{stderr}"); + assert!( + !stdout.contains("gemfile_declaration_not_editable"), + "{label}: the hosted block is not a user declaration:\n{stdout}" + ); + if dry_run { + assert!( + stdout.contains("eject_planned"), + "{label}: previews the eject:\n{stdout}" + ); + for (file, before) in ["Gemfile", "Gemfile.lock"].iter().zip(before) { + assert_eq!( + std::fs::read(fx.proj.join(file)).unwrap(), + before, + "{label}: a dry run writes nothing to {file}" + ); + } + } else { + let gemfile = std::fs::read_to_string(fx.proj.join("Gemfile")).unwrap(); + assert!( + gemfile.contains("path: \".socket/vendor/gem/"), + "{label}: the Gemfile now points at the vendored copy:\n{gemfile}" + ); + } + } +} + /// One refused `vendor` eject of the group-block fixture (#1761): dry or /// wet, the eject is refused whole with `gemfile_declaration_not_editable` /// and `eject_refused` (exit 1), and the hosted `Gemfile` / From dc55a1084989721825817fe202e5846a4ac39536 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 16:26:50 +0000 Subject: [PATCH 2/2] Skip the gem disk preview only after a restored-text preview The dry takeover skipped the on-disk backend preview for every gem, but takeover_dry_refusal returns None both when the restored text vendors cleanly and when it could not preview it at all (an uncaptured or binary reverted file). In the second case nothing checked the gem before the dry run reported success. Skip only when the restore rewrote a Gemfile/gems.rb (where the hosted source block lives) and its text was previewable, and match the ecosystem with Ecosystem::from_purl like the rest of the function. Co-Authored-By: Claude --- .../socket-patch-cli/src/commands/vendor.rs | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index cdd7286f7..fc6e611a3 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -269,12 +269,7 @@ async fn takeover_dry_refusal( pipenv_version: &tokio::sync::OnceCell>, installed_sites: &vendor::pypi::InstalledSiteListings, ) -> Option<(&'static str, String)> { - if restore.reverted_files.is_empty() - || !restore.reverted_files.iter().all(|f| { - socket_patch_core::utils::group_commit::captures(f) - && restore.staged_text.contains_key(f) - }) - { + if !restored_text_previewable(restore) { return None; } let probe = GroupCommit::begin(project_root); @@ -321,6 +316,18 @@ async fn takeover_dry_refusal( } } +/// Whether [`takeover_dry_refusal`] can stage `restore`'s text and preview +/// the backend over it: every reverted file is captured and has staged text. +fn restored_text_previewable( + restore: &socket_patch_core::patch::redirect::upstream::RestoreOutcome, +) -> bool { + !restore.reverted_files.is_empty() + && restore.reverted_files.iter().all(|f| { + socket_patch_core::utils::group_commit::captures(f) + && restore.staged_text.contains_key(f) + }) +} + /// Dispatch one purl to its ecosystem backend. `pkg_path` is the crawler's /// installed location (site-packages root for pypi, the package dir /// otherwise), when available. Returns `None` for purls with no vendor @@ -3517,11 +3524,21 @@ pub(crate) async fn vendor_records_reusing( // `vendor_sbt_hosted_conflict` the wet run (which // restores first) never sees. // - // And a gem: its hosted `source … do` block still wraps - // the declaration on disk, so the backend's Gemfile - // planner would refuse it as indented. The preview over - // the restored text above already asked the backend. - if candidate.starts_with("pkg:gem/") + // And a gem whose restore rewrote its Gemfile: the hosted + // `source … do` block still wraps the declaration on + // disk, so the backend's Gemfile planner would refuse it + // as indented. When the restored text was previewable, + // the preview above already asked the backend over it. + let gem_block_reverted = + matches!(Ecosystem::from_purl(candidate), Some(Ecosystem::Gem)) + && restored_text_previewable(&restore) + && restore.reverted_files.iter().any(|f| { + matches!( + Path::new(f).file_name().and_then(|n| n.to_str()), + Some("Gemfile" | "gems.rb") + ) + }); + if gem_block_reverted || restore.reverted_files.iter().any(|f| { f == "bun.lock" || f == "bun.lockb"