From 28208553d74ce633bbd6bfaf8ef0adaf3fd3b212 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 07:23:52 +0000 Subject: [PATCH 1/5] Start fix for #1688 Assisted-by: Claude Code:claude-opus-5-5 From 8a9750de25598319cee4ac91ee690a4f7c834032 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 07:45:22 +0000 Subject: [PATCH 2/5] Apply superseding gem patches on Bundler < 2.2 Bundler 1.17-2.1 lock every rubygems source into one merged GEM section, so the Socket remote an earlier redirect added is tied to the gem only through the Gemfile's `source ... do` block. A superseding patch (or a rotated grant) moved that block to the new URL but left the lock's remote on the old one. Lockfile discovery then saw two different patches and the scan skipped the new one as redirect_unattributable on every run, exiting 0 with the project stuck on the older patch. The hosted gem rewriter now moves that merged section's Socket remote to the new URL in place, alongside the Gemfile block, when the lock has exactly the shape a converged earlier redirect leaves. Every other lock shape is left as before. Fixes #1688 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/gem/hosted.rs | 172 ++++++++++++++++++ crates/socket-patch-core/src/hosted/engine.rs | 110 +++++++++++ .../src/patch/redirect/mod.rs | 25 ++- 3 files changed, 301 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-core/src/formats/gem/hosted.rs b/crates/socket-patch-core/src/formats/gem/hosted.rs index 6a12e2517..684d4f0ba 100644 --- a/crates/socket-patch-core/src/formats/gem/hosted.rs +++ b/crates/socket-patch-core/src/formats/gem/hosted.rs @@ -296,6 +296,84 @@ pub(crate) fn converge_gem_lock_source( true } +/// Keep a merged multi-remote `GEM` section (bundler ≤ 2.1, and locks that +/// started there) on the patch the Gemfile now names (#1688). Such a +/// section lists every rubygems remote, so the Socket remote a previous +/// run's prescribed `bundle install` added is attributed to a gem only +/// through the Gemfile's `source … do` block (`vex::discover::gem`, "Merged +/// sections"). When a superseding patch or rotated grant moves that block to +/// a new URL, the section's Socket remote must move with it, or discovery +/// sees the block and the lock name different patches and the scan refuses +/// the new pin as unattributable on every run. +/// +/// Refreshes only the shape a converged earlier redirect leaves: the dep's +/// spec listed once, in a merged section, which has exactly one remote +/// matching the dep's token-wildcard index pattern, and a source-pinned +/// (`!`) DEPENDENCIES entry. The remote keeps its position (bundler orders a +/// merged section's remotes by the Gemfile's declarations, which the +/// in-place Gemfile refresh keeps). Returns true when the lock is converged +/// on `index_url` (already, or via the recorded +/// `redirect_gemfile_lock_source_url` edit); false, touching nothing, for +/// any other shape (a first redirect into a merged lock stays mixed until +/// the next unfrozen install). +pub(crate) fn refresh_merged_gem_lock_remote( + lk: &mut String, + dep: &DepOverride, + index_url: &str, + lock_name: &str, + lock_changed: &mut bool, + result: &mut RewriteResult, +) -> bool { + let model = super::parse(lk); + if !model.problems.is_empty() || !model.pinned.contains(dep.name.as_str()) { + return false; + } + let spec_raw = format!("{} ({})", dep.name, dep.version); + let holders: Vec<&Section<'_>> = model + .gem_sections() + .filter(|s| s.specs.iter().any(|spec| spec.raw == spec_raw)) + .collect(); + let [sec] = holders.as_slice() else { + return false; + }; + if sec.remotes.len() < 2 || sec.specs.iter().filter(|s| s.raw == spec_raw).count() != 1 { + return false; + } + let socket_remote_re = Regex::new(&format!("^{}$", gem_index_url_pattern(dep, index_url))) + .expect("anchored index-url pattern from the escaped URL is valid"); + let ours: Vec<(usize, &str)> = sec + .remotes + .iter() + .zip(&sec.remote_line_nos) + .filter(|(remote, _)| socket_remote_re.is_match(remote)) + .map(|(remote, line_no)| (line_no - 1, *remote)) + .collect(); + let [(remote_idx, remote_url)] = ours.as_slice() else { + return false; + }; + if *remote_url == index_url { + return true; + } + let (remote_idx, remote_url) = (*remote_idx, remote_url.to_string()); + let mut lines: Vec<&str> = lk.split_inclusive('\n').collect(); + let ending = &lines[remote_idx][gem_lock_line_content(lines[remote_idx]).len()..]; + let refreshed = format!(" remote: {index_url}{ending}"); + lines[remote_idx] = &refreshed; + let text = lines.concat(); + drop(model); + *lk = text; + *lock_changed = true; + result.edits.push(FileEdit { + path: lock_name.into(), + kind: "redirect_gemfile_lock_source_url".into(), + action: "rewritten".into(), + key: Some(dep.name.clone()), + original: Some(Value::String(remote_url)), + new: Some(Value::String(index_url.to_string())), + }); + true +} + /// The byte span (line content, ending excluded) of the `CHECKSUMS` entry /// for exactly `name (version)` — the platform-less spec the hosted planner /// pins — read with the shared entry grammar @@ -581,4 +659,98 @@ DEPENDENCIES "GEM\n remote: https://patch.test/gem/tok/uuid/\n specs:\n racc (1.7.3)\n\nGEM\n remote: https://rubygems.org/\n specs:\n\nDEPENDENCIES\n racc (= 1.7.3)!" ); } + + /// `(converged, lock after, edit kinds)` for [`refresh_merged_gem_lock_remote`]. + fn refresh(lock: &str, dep: &DepOverride) -> (bool, String, Vec) { + let mut lk = lock.to_string(); + let mut changed = false; + let mut result = RewriteResult::default(); + let ok = refresh_merged_gem_lock_remote( + &mut lk, + dep, + INDEX_URL, + "Gemfile.lock", + &mut changed, + &mut result, + ); + assert_eq!(changed, lk != lock, "lock_changed must report a change"); + let edits = result + .edits + .iter() + .map(|e| format!("{}:{}", e.kind, e.action)) + .collect(); + (ok, lk, edits) + } + + /// A bundler ≤ 2.1 merged section an earlier redirect converged: + /// `remotes` listed in order, `rails` source-pinned when `bang`. + fn merged(remotes: &[&str], bang: &str) -> String { + let remotes: String = remotes.iter().map(|r| format!(" remote: {r}\n")).collect(); + format!( + "GEM\n{remotes} specs:\n racc (1.7.3)\n rails (7.0.0)\n racc\n\n\ + PLATFORMS\n ruby\n\nDEPENDENCIES\n rails (= 7.0.0){bang}\n\nBUNDLED WITH\n 2.1.4\n" + ) + } + + /// #1688: the merged section's one Socket remote for the dep moves to + /// the new index URL in place (any earlier patch uuid or grant token), + /// CRLF kept; a lock already on the URL is converged with no edit. + #[test] + fn merged_section_socket_remote_follows_the_new_index_url() { + let upstream = "https://rubygems.org/"; + for old in [ + "https://patch.test/gem/tok/older-uuid/", + "https://patch.test/gem/old-tok/uuid/", + ] { + for eol in ["\n", "\r\n"] { + let lock = merged(&[upstream, old], "!").replace('\n', eol); + let (ok, out, edits) = refresh(&lock, &dep("rails", "7.0.0")); + assert!(ok, "{old}"); + assert_eq!(out, lock.replace(old, INDEX_URL), "{old} {eol:?}"); + assert_eq!(edits, vec!["redirect_gemfile_lock_source_url:rewritten"]); + } + } + let current = merged(&[upstream, INDEX_URL], "!"); + assert_eq!( + refresh(¤t, &dep("rails", "7.0.0")), + (true, current.clone(), vec![]) + ); + } + + /// Every other shape is left alone (false, no edit): a merged section + /// with no Socket remote (a first redirect), a gem not source-pinned, a + /// second Socket remote, a single-remote section, a spec listed twice, + /// or a gem the lock does not list. + #[test] + fn merged_section_refresh_refuses_other_shapes() { + let upstream = "https://rubygems.org/"; + let old = "https://patch.test/gem/tok/older-uuid/"; + let duplicated = format!( + "{}GEM\n remote: https://gems.mirror.example/\n specs:\n rails (7.0.0)\n", + merged(&[upstream, old], "!").replace("\nPLATFORMS", "\nGEM_SPLIT") + ) + .replace("GEM_SPLIT", "PLATFORMS"); + let cases = [ + ( + "no Socket remote", + merged(&[upstream, "https://gems.mirror.example/"], "!"), + ), + ("not source-pinned", merged(&[upstream, old], "")), + ( + "two Socket remotes", + merged(&[upstream, old, "https://patch.test/gem/tok/third/"], "!"), + ), + ("single remote", merged(&[old], "!")), + ("spec in two sections", duplicated), + ]; + for (label, lock) in &cases { + let (ok, out, edits) = refresh(lock, &dep("rails", "7.0.0")); + assert!(!ok, "{label}"); + assert_eq!(&out, lock, "{label}"); + assert!(edits.is_empty(), "{label}: {edits:?}"); + } + let lock = merged(&[upstream, old], "!"); + let (ok, out, _) = refresh(&lock, &dep("rails", "7.1.0")); + assert!(!ok && out == lock, "a version the lock does not list"); + } } diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 0fb484b36..0c70f2490 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -5440,6 +5440,116 @@ snapshots: } } + /// #1688: bundler ≤ 2.1 writes ONE merged `GEM` section listing every + /// remote, so after an earlier redirect + `bundle install` the lock's + /// Socket remote names the earlier patch. A superseding patch (or a + /// rotated grant) moves the Gemfile's `source … do` block to the new + /// URL; the lock's remote must move with it, or discovery reads the + /// pair as naming two patches and the run skips the new one as + /// `redirect_unattributable` on every scan. + #[tokio::test] + async fn merged_section_supersede_moves_the_lock_remote_with_the_gemfile() { + const OLD_UUID: &str = "10000000-1a2b-4a1b-8c2d-3e4f5a6b7c8d"; + const OLD_TOKEN: &str = "22222222-2222-4222-8222-222222222222"; + let new_url = + format!("https://patch.test/patch-registry/gem/{FIXTURE_TOKEN}/{FIXTURE_UUID}/"); + for (label, old_url) in [ + ( + "superseding patch", + format!("https://patch.test/patch-registry/gem/{FIXTURE_TOKEN}/{OLD_UUID}/"), + ), + ( + "rotated grant", + format!("https://patch.test/patch-registry/gem/{OLD_TOKEN}/{FIXTURE_UUID}/"), + ), + ] { + for eol in ["\n", "\r\n"] { + let gemfile = format!( + "source \"https://rubygems.org\"\n\nsource \"{old_url}\" do\n gem \"rails\", \"7.0.0\"\nend\n" + ) + .replace('\n', eol); + let lock = format!( + "GEM\n remote: https://rubygems.org/\n remote: {old_url}\n specs:\n \ + rails (7.0.0)\n\nPLATFORMS\n ruby\n\nDEPENDENCIES\n rails (= 7.0.0)!\n\n\ + BUNDLED WITH\n 2.1.4\n" + ) + .replace('\n', eol); + let mut p = MemoryProject::new(); + p.insert_text("Gemfile", gemfile.as_str()); + p.insert_text("Gemfile.lock", lock.as_str()); + let outer = OuterAllowRemote::default; + let options = RewriteOptions { + dry_run: false, + targets_pipenv_lock: false, + pipenv_major: None, + pipenv_unknown_detail: String::new(), + trust_lockfile_config: true, + npm_allow_remote_config: true, + npm_outer: &outer, + yarn_classic_outer: &OuterYarnMirror::default, + cargo_outer: &no_cargo_outer, + pnpm_env: &no_env, + blocking: false, + takeover_uuids: Default::default(), + patch_server_origins: vec!["https://patch.test".to_string()], + prior_discovery: None, + }; + let mut candidate = gem_candidate(); + if let Some(ov) = candidate.dep.registry_override.as_mut() { + ov.index_url = new_url.clone(); + } + let candidates = vec![candidate]; + let view = ProjectView::Memory(&p); + let read = read_candidate_files(&view, &BTreeSet::new(), &candidates).await; + let done = rewrite( + &view, + read, + &candidates, + BTreeMap::new(), + &BTreeSet::new(), + options, + ) + .await; + let ctx = format!("{label} ({eol:?})"); + assert!( + done.unattributed.is_empty(), + "{ctx}: {:?}", + done.unattributed + ); + assert_eq!( + done.confirmed, + vec![("pkg:gem/rails@7.0.0".to_string(), FIXTURE_UUID.to_string())], + "{ctx}: {:?}", + done.rewrite.warnings + ); + assert_eq!( + done.rewrite.files.get("Gemfile").map(String::as_str), + Some(gemfile.replace(&old_url, &new_url).as_str()), + "{ctx}" + ); + assert_eq!( + done.rewrite.files.get("Gemfile.lock").map(String::as_str), + Some(lock.replace(&old_url, &new_url).as_str()), + "{ctx}: the merged section's Socket remote moves in place" + ); + assert!( + done.rewrite.edits.iter().any(|e| { + e.path == "Gemfile.lock" + && e.kind == "redirect_gemfile_lock_source_url" + && e.original == Some(serde_json::Value::String(old_url.clone())) + }), + "{ctx}: {:?}", + done.rewrite.edits + ); + assert!( + !warning_codes(&done).contains(&"redirect_gem_frozen_install"), + "{ctx}: the refreshed pair stays converged: {:?}", + done.rewrite.warnings + ); + } + } + } + /// A twin whose other spelling this run can't read (a symlink, an /// unreadable or a non-UTF-8 file) is still a twin: bundler's /// `File.file?` sees it, so neither pair is wired (Bugbot on #768). diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 83e5c6561..7fdf3da73 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -63,7 +63,9 @@ use crate::formats::cargo::source_replacement::{crates_io_source, ConfigLayer, C use crate::formats::cargo::CargoLock; use crate::formats::composer::hosted::rewrite_composer_lock; use crate::formats::gem::gemfile; -use crate::formats::gem::hosted::{checksum_entry_span, converge_gem_lock_source}; +use crate::formats::gem::hosted::{ + checksum_entry_span, converge_gem_lock_source, refresh_merged_gem_lock_remote, +}; use crate::formats::gem::{lock_lists_direct_dependency, locked_specs as gem_locked_specs}; #[cfg(test)] use crate::formats::pnpm::hosted::pnpm_unrewritten_instances; @@ -7721,17 +7723,28 @@ fn rewrite_gem( // remote and refuses the pair outright (unfrozen: exit 37 // "mismatched checksums"; frozen: exit 16). A pre-CHECKSUMS lock // has no sha to converge around, so it keeps the - // mixed-but-installable state + the frozen-install caveat. - if !checksums_era - || !converge_gem_lock_source( + // mixed-but-installable state + the frozen-install caveat — + // unless an earlier redirect already converged it into a merged + // multi-remote section, whose Socket remote follows the Gemfile + // block to the new URL (#1688). + let converged = (checksums_era + && converge_gem_lock_source( lk, dep, &ov.index_url, lock_name, &mut lock_changed, result, - ) - { + )) + || refresh_merged_gem_lock_remote( + lk, + dep, + &ov.index_url, + lock_name, + &mut lock_changed, + result, + ); + if !converged { mixed_state = true; } } From e00abe11f1414cd87524ea4736a43a3ef9c5c459 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 07:49:46 +0000 Subject: [PATCH 3/5] Test gem supersede end to end on Bundler < 2.2 The unconverged-supersede capstone assumed every bundler leaves the lock on the old patch after a superseding scan. On bundler < 2.2 the merged GEM section now moves with the Gemfile block, so assert that the new patch lands there (frozen install accepts the pair) and that rollback and remove still unwind it. Refs #1688 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_gem_build.rs | 38 ++++++++++++++++--- 1 file changed, 32 insertions(+), 6 deletions(-) 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 3d1e01b83..64d7229ba 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -2278,6 +2278,9 @@ async fn gem_hosted_superseding_patch_keeps_gem_sections_in_bundler_order() { /// `rollback` / `remove` in that window must undo the new block along with /// the lock, or the pair stays inconsistent (frozen installs fail) and the /// next unfrozen install re-patches. +/// On bundler < 2.2 (one merged GEM section) the re-scan moves the lock's +/// Socket remote along with the block (#1688), so there is no such window; +/// the test proves the superseding patch lands there and unwinds. #[tokio::test(flavor = "multi_thread")] #[ignore = "host capstone: shells out to a real ruby/gem/bundler; \ the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"] @@ -2339,15 +2342,38 @@ async fn unwind_after_unconverged_supersede(command: &str) { assert_eq!(code, 0, "{command}: gen 3 scan: {out}\n{err}"); let gemfile_path = fx.proj.join(fx.gemfile_name); let lock_path = fx.proj.join(fx.lock_name); - // The unconverged window this test is about: Gemfile on gen 3, lock on - // gen 2. let gemfile = std::fs::read_to_string(&gemfile_path).unwrap(); let lock = std::fs::read_to_string(&lock_path).unwrap(); assert!(gemfile.contains(GEN3), "{command}: Gemfile:\n{gemfile}"); - assert!( - lock.contains(GEN2) && !lock.contains(GEN3), - "{command}: lock:\n{lock}" - ); + if fx.bundler.at_least(2, 2) { + // The unconverged window this test is about: Gemfile on gen 3, lock + // on gen 2. + assert!( + lock.contains(GEN2) && !lock.contains(GEN3), + "{command}: lock:\n{lock}" + ); + } else { + // #1688: bundler < 2.2 locks every remote in one merged GEM section, + // whose Socket remote moves to gen 3 with the Gemfile block (the + // pair stays converged, so a frozen install accepts it). The + // unwind below then undoes the converged gen 3 pair. + assert!( + lock.contains(GEN3) && !lock.contains(GEN2), + "{command}: lock:\n{lock}" + ); + let frozen = bundle_env(&fx.proj, &["install"], &[("BUNDLE_FROZEN", "true")]); + assert!( + frozen.status.success(), + "{command}: frozen install of the gen 3 pair:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&frozen.stdout), + String::from_utf8_lossy(&frozen.stderr) + ); + assert_eq!( + std::fs::read_to_string(&lock_path).unwrap(), + lock, + "{command}: the frozen install leaves the gen 3 lock as is" + ); + } let upstream = format!("{api}/upstream"); let cwd = fx.proj.to_str().expect("utf8 tmp path"); From 11efaeb0e56b4207e88528d91a2ca5a6a837e091 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 07:59:21 +0000 Subject: [PATCH 4/5] Only move a merged gem remote the gem owns Review follow-up. The merged-section refresh matched any Socket remote in the section, so a first redirect of one gem could take over another redirected gem's remote. It now moves only the remote that equals this gem's own existing Gemfile source block URL. The frozen-install caveat is now dropped only for locks bundler < 2.2 wrote, the era the e2e proves frozen-installable; a later bundler that kept a merged lock still gets the caveat. The remote line rewrite is shared with the CHECKSUMS-era refresh. Refs #1688 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/gem/hosted.rs | 218 +++++++++++------- .../src/patch/redirect/mod.rs | 49 ++-- 2 files changed, 173 insertions(+), 94 deletions(-) diff --git a/crates/socket-patch-core/src/formats/gem/hosted.rs b/crates/socket-patch-core/src/formats/gem/hosted.rs index 684d4f0ba..224d69c1e 100644 --- a/crates/socket-patch-core/src/formats/gem/hosted.rs +++ b/crates/socket-patch-core/src/formats/gem/hosted.rs @@ -195,17 +195,9 @@ pub(crate) fn converge_gem_lock_source( // Already ours. Rotated grant or superseding patch: refresh the // remote. if remote_url != index_url { - let ending = - lines[remote_idx][gem_lock_line_content(&lines[remote_idx]).len()..].to_string(); - lines[remote_idx] = Cow::Owned(format!(" remote: {index_url}{ending}")); - result.edits.push(FileEdit { - path: lock_name.into(), - kind: "redirect_gemfile_lock_source_url".into(), - action: "rewritten".into(), - key: Some(dep.name.clone()), - original: Some(Value::String(remote_url)), - new: Some(Value::String(index_url.to_string())), - }); + refresh_remote_line( + &mut lines, remote_idx, remote_url, index_url, dep, lock_name, result, + ); changed = true; } // The refreshed URL (a superseding patch uuid, a rotated grant) can @@ -301,24 +293,24 @@ pub(crate) fn converge_gem_lock_source( /// section lists every rubygems remote, so the Socket remote a previous /// run's prescribed `bundle install` added is attributed to a gem only /// through the Gemfile's `source … do` block (`vex::discover::gem`, "Merged -/// sections"). When a superseding patch or rotated grant moves that block to -/// a new URL, the section's Socket remote must move with it, or discovery -/// sees the block and the lock name different patches and the scan refuses -/// the new pin as unattributable on every run. +/// sections"). When a superseding patch or rotated grant moves that block +/// from `prior_url` to `index_url`, the section's remote must move with it, +/// or discovery sees the block and the lock name different patches and the +/// scan refuses the new pin as unattributable on every run. /// /// Refreshes only the shape a converged earlier redirect leaves: the dep's -/// spec listed once, in a merged section, which has exactly one remote -/// matching the dep's token-wildcard index pattern, and a source-pinned -/// (`!`) DEPENDENCIES entry. The remote keeps its position (bundler orders a -/// merged section's remotes by the Gemfile's declarations, which the -/// in-place Gemfile refresh keeps). Returns true when the lock is converged -/// on `index_url` (already, or via the recorded -/// `redirect_gemfile_lock_source_url` edit); false, touching nothing, for -/// any other shape (a first redirect into a merged lock stays mixed until -/// the next unfrozen install). +/// spec listed once, in a merged section, which lists `prior_url` (the dep's +/// own block URL, so another gem's Socket remote is never taken) exactly +/// once, and a source-pinned (`!`) DEPENDENCIES entry. The remote keeps its +/// position (bundler orders a merged section's remotes by the Gemfile's +/// declarations, which the in-place Gemfile refresh keeps). Returns true +/// when the section's remote reads `index_url` (already, or via the +/// recorded `redirect_gemfile_lock_source_url` edit); false, touching +/// nothing, for any other shape. pub(crate) fn refresh_merged_gem_lock_remote( lk: &mut String, dep: &DepOverride, + prior_url: &str, index_url: &str, lock_name: &str, lock_changed: &mut bool, @@ -339,30 +331,45 @@ pub(crate) fn refresh_merged_gem_lock_remote( if sec.remotes.len() < 2 || sec.specs.iter().filter(|s| s.raw == spec_raw).count() != 1 { return false; } - let socket_remote_re = Regex::new(&format!("^{}$", gem_index_url_pattern(dep, index_url))) - .expect("anchored index-url pattern from the escaped URL is valid"); let ours: Vec<(usize, &str)> = sec .remotes .iter() .zip(&sec.remote_line_nos) - .filter(|(remote, _)| socket_remote_re.is_match(remote)) + .filter(|(remote, _)| super::same_remote(remote, prior_url)) .map(|(remote, line_no)| (line_no - 1, *remote)) .collect(); let [(remote_idx, remote_url)] = ours.as_slice() else { return false; }; - if *remote_url == index_url { - return true; + if *remote_url != index_url { + let (remote_idx, remote_url) = (*remote_idx, remote_url.to_string()); + let mut lines: Vec> = lk.split_inclusive('\n').map(Cow::Borrowed).collect(); + refresh_remote_line( + &mut lines, remote_idx, remote_url, index_url, dep, lock_name, result, + ); + let text = lines.concat(); + drop(lines); + drop(model); + *lk = text; + *lock_changed = true; } - let (remote_idx, remote_url) = (*remote_idx, remote_url.to_string()); - let mut lines: Vec<&str> = lk.split_inclusive('\n').collect(); - let ending = &lines[remote_idx][gem_lock_line_content(lines[remote_idx]).len()..]; - let refreshed = format!(" remote: {index_url}{ending}"); - lines[remote_idx] = &refreshed; - let text = lines.concat(); - drop(model); - *lk = text; - *lock_changed = true; + true +} + +/// Point the `remote:` line `idx` (reading `remote_url`) at `index_url`, +/// keeping its line ending, and record the `redirect_gemfile_lock_source_url` +/// edit a revert reads. +fn refresh_remote_line( + lines: &mut [Cow<'_, str>], + idx: usize, + remote_url: String, + index_url: &str, + dep: &DepOverride, + lock_name: &str, + result: &mut RewriteResult, +) { + let ending = lines[idx][gem_lock_line_content(&lines[idx]).len()..].to_string(); + lines[idx] = Cow::Owned(format!(" remote: {index_url}{ending}")); result.edits.push(FileEdit { path: lock_name.into(), kind: "redirect_gemfile_lock_source_url".into(), @@ -371,7 +378,26 @@ pub(crate) fn refresh_merged_gem_lock_remote( original: Some(Value::String(remote_url)), new: Some(Value::String(index_url.to_string())), }); - true +} + +/// Whether the lock's `BUNDLED WITH` names a bundler older than 2.2 — the +/// era that writes one merged `GEM` section, where a refreshed merged +/// section is proven frozen-installable. False when it names none. +pub(crate) fn bundled_before_2_2(lock: &str) -> bool { + let mut lines = lock.lines().map(|l| l.trim_end_matches('\r')); + let Some(version) = lines + .by_ref() + .find(|l| *l == "BUNDLED WITH") + .and_then(|_| lines.next()) + .map(str::trim) + else { + return false; + }; + let mut parts = version.split('.').map(|p| p.parse::()); + match (parts.next(), parts.next()) { + (Some(Ok(major)), Some(Ok(minor))) => (major, minor) < (2, 2), + _ => false, + } } /// The byte span (line content, ending excluded) of the `CHECKSUMS` entry @@ -660,14 +686,16 @@ DEPENDENCIES ); } - /// `(converged, lock after, edit kinds)` for [`refresh_merged_gem_lock_remote`]. - fn refresh(lock: &str, dep: &DepOverride) -> (bool, String, Vec) { + /// `(refreshed, lock after, edit kinds)` for + /// [`refresh_merged_gem_lock_remote`] from the dep's block URL `prior`. + fn refresh(lock: &str, dep: &DepOverride, prior: &str) -> (bool, String, Vec) { let mut lk = lock.to_string(); let mut changed = false; let mut result = RewriteResult::default(); let ok = refresh_merged_gem_lock_remote( &mut lk, dep, + prior, INDEX_URL, "Gemfile.lock", &mut changed, @@ -683,74 +711,108 @@ DEPENDENCIES } /// A bundler ≤ 2.1 merged section an earlier redirect converged: - /// `remotes` listed in order, `rails` source-pinned when `bang`. - fn merged(remotes: &[&str], bang: &str) -> String { + /// `remotes` listed in order, `rails` source-pinned when `bang`, and + /// `extra` sections before PLATFORMS. + fn merged(remotes: &[&str], bang: &str, extra: &str) -> String { let remotes: String = remotes.iter().map(|r| format!(" remote: {r}\n")).collect(); format!( - "GEM\n{remotes} specs:\n racc (1.7.3)\n rails (7.0.0)\n racc\n\n\ + "GEM\n{remotes} specs:\n racc (1.7.3)\n rails (7.0.0)\n racc\n\n{extra}\ PLATFORMS\n ruby\n\nDEPENDENCIES\n rails (= 7.0.0){bang}\n\nBUNDLED WITH\n 2.1.4\n" ) } - /// #1688: the merged section's one Socket remote for the dep moves to - /// the new index URL in place (any earlier patch uuid or grant token), - /// CRLF kept; a lock already on the URL is converged with no edit. + const UPSTREAM: &str = "https://rubygems.org/"; + const OLD: &str = "https://patch.test/gem/tok/older-uuid/"; + + /// #1688: the merged section's remote for the dep's own block moves to + /// the new index URL in place (an earlier patch uuid or grant token), + /// CRLF kept; a lock already on the URL is refreshed with no edit. #[test] fn merged_section_socket_remote_follows_the_new_index_url() { - let upstream = "https://rubygems.org/"; - for old in [ - "https://patch.test/gem/tok/older-uuid/", - "https://patch.test/gem/old-tok/uuid/", - ] { + for old in [OLD, "https://patch.test/gem/old-tok/uuid/"] { for eol in ["\n", "\r\n"] { - let lock = merged(&[upstream, old], "!").replace('\n', eol); - let (ok, out, edits) = refresh(&lock, &dep("rails", "7.0.0")); + let lock = merged(&[UPSTREAM, old], "!", "").replace('\n', eol); + let (ok, out, edits) = refresh(&lock, &dep("rails", "7.0.0"), old); assert!(ok, "{old}"); assert_eq!(out, lock.replace(old, INDEX_URL), "{old} {eol:?}"); assert_eq!(edits, vec!["redirect_gemfile_lock_source_url:rewritten"]); } } - let current = merged(&[upstream, INDEX_URL], "!"); + let current = merged(&[UPSTREAM, INDEX_URL], "!", ""); assert_eq!( - refresh(¤t, &dep("rails", "7.0.0")), + refresh(¤t, &dep("rails", "7.0.0"), INDEX_URL), (true, current.clone(), vec![]) ); } - /// Every other shape is left alone (false, no edit): a merged section - /// with no Socket remote (a first redirect), a gem not source-pinned, a - /// second Socket remote, a single-remote section, a spec listed twice, - /// or a gem the lock does not list. + /// Every other shape is left alone (false, no edit): a remote that is + /// not the dep's own block URL (another gem's Socket remote, or none), + /// a gem not source-pinned, a single-remote section, a spec listed in + /// two sections, or a version the lock does not list. #[test] fn merged_section_refresh_refuses_other_shapes() { - let upstream = "https://rubygems.org/"; - let old = "https://patch.test/gem/tok/older-uuid/"; - let duplicated = format!( - "{}GEM\n remote: https://gems.mirror.example/\n specs:\n rails (7.0.0)\n", - merged(&[upstream, old], "!").replace("\nPLATFORMS", "\nGEM_SPLIT") - ) - .replace("GEM_SPLIT", "PLATFORMS"); + let other_gem = "https://patch.test/gem/tok/other-gem-uuid/"; + let second = "GEM\n remote: https://gems.mirror.example/\n specs:\n rails (7.0.0)\n\n"; + let rails = dep("rails", "7.0.0"); let cases = [ + ( + "another gem's Socket remote", + merged(&[UPSTREAM, other_gem], "!", ""), + &rails, + ), ( "no Socket remote", - merged(&[upstream, "https://gems.mirror.example/"], "!"), + merged(&[UPSTREAM, "https://gems.mirror.example/"], "!", ""), + &rails, + ), + ( + "not source-pinned", + merged(&[UPSTREAM, OLD], "", ""), + &rails, ), - ("not source-pinned", merged(&[upstream, old], "")), + ("single remote", merged(&[OLD], "!", ""), &rails), ( - "two Socket remotes", - merged(&[upstream, old, "https://patch.test/gem/tok/third/"], "!"), + "spec in two sections", + merged(&[UPSTREAM, OLD], "!", second), + &rails, + ), + ( + "unlocked version", + merged(&[UPSTREAM, OLD], "!", ""), + &dep("rails", "7.1.0"), ), - ("single remote", merged(&[old], "!")), - ("spec in two sections", duplicated), ]; - for (label, lock) in &cases { - let (ok, out, edits) = refresh(lock, &dep("rails", "7.0.0")); + for (label, lock, dep) in &cases { + assert!( + super::super::parse(lock).problems.is_empty(), + "{label}: the fixture must be a valid lock" + ); + let (ok, out, edits) = refresh(lock, dep, OLD); assert!(!ok, "{label}"); assert_eq!(&out, lock, "{label}"); assert!(edits.is_empty(), "{label}: {edits:?}"); } - let lock = merged(&[upstream, old], "!"); - let (ok, out, _) = refresh(&lock, &dep("rails", "7.1.0")); - assert!(!ok && out == lock, "a version the lock does not list"); + } + + /// Only a lock bundler < 2.2 wrote counts as the merged era. + #[test] + fn bundled_before_2_2_reads_the_bundled_with_version() { + for (version, want) in [ + ("1.17.3", true), + ("2.1.4", true), + ("2.2.0", false), + ("2.5.23", false), + ("4.0.18", false), + ("garbage", false), + ] { + let lock = format!("DEPENDENCIES\n rails\n\nBUNDLED WITH\n {version}\n"); + assert_eq!(bundled_before_2_2(&lock), want, "{version}"); + assert_eq!( + bundled_before_2_2(&lock.replace('\n', "\r\n")), + want, + "{version} CRLF" + ); + } + assert!(!bundled_before_2_2("DEPENDENCIES\n rails\n")); } } diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 7fdf3da73..09e08c214 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -64,7 +64,8 @@ use crate::formats::cargo::CargoLock; use crate::formats::composer::hosted::rewrite_composer_lock; use crate::formats::gem::gemfile; use crate::formats::gem::hosted::{ - checksum_entry_span, converge_gem_lock_source, refresh_merged_gem_lock_remote, + bundled_before_2_2, checksum_entry_span, converge_gem_lock_source, + refresh_merged_gem_lock_remote, }; use crate::formats::gem::{lock_lists_direct_dependency, locked_specs as gem_locked_specs}; #[cfg(test)] @@ -7394,6 +7395,10 @@ fn rewrite_gem( // Whether THIS dep's Gemfile source redirect is in place (just // written or already present) — the lock pin below is gated on it. let mut source_placed = false; + // The URL of the Socket `source … do` block an earlier run left for + // THIS dep (before any in-place refresh below): the remote a merged + // lock section attributes to it (#1688). + let mut prior_block_url: Option = None; if let Some(gf) = gemfile.as_mut() { // Looser "declared at all?" probe: matches every `gem` call that // names the gem anywhere in its arguments, in any form. It gates @@ -7453,6 +7458,7 @@ fn rewrite_gem( let url = m .get(1) .expect("block_re always captures group 1 (index URL)"); + prior_block_url = Some(url.as_str().to_string()); if url.as_str() == ov.index_url { source_placed = true; } else { @@ -7723,11 +7729,17 @@ fn rewrite_gem( // remote and refuses the pair outright (unfrozen: exit 37 // "mismatched checksums"; frozen: exit 16). A pre-CHECKSUMS lock // has no sha to converge around, so it keeps the - // mixed-but-installable state + the frozen-install caveat — - // unless an earlier redirect already converged it into a merged - // multi-remote section, whose Socket remote follows the Gemfile - // block to the new URL (#1688). - let converged = (checksums_era + // mixed-but-installable state + the frozen-install caveat. + // + // A lock an earlier redirect converged into a merged + // multi-remote section (bundler ≤ 2.1, or a lock that started + // there) instead has its Socket remote follow this dep's Gemfile + // block to the new URL, or discovery reads the pair as naming + // two patches and every scan skips the new one (#1688). Only a + // lock bundler < 2.2 wrote is proven frozen-installable after + // that refresh (e2e_redirect_gem_build); a later bundler keeping + // a merged lock keeps the caveat. + let converged = checksums_era && converge_gem_lock_source( lk, dep, @@ -7735,16 +7747,20 @@ fn rewrite_gem( lock_name, &mut lock_changed, result, - )) - || refresh_merged_gem_lock_remote( - lk, - dep, - &ov.index_url, - lock_name, - &mut lock_changed, - result, ); - if !converged { + let merged_refreshed = !converged + && prior_block_url.as_deref().is_some_and(|prior| { + refresh_merged_gem_lock_remote( + lk, + dep, + prior, + &ov.index_url, + lock_name, + &mut lock_changed, + result, + ) + }); + if !converged && !(merged_refreshed && bundled_before_2_2(lk)) { mixed_state = true; } } @@ -7754,7 +7770,8 @@ fn rewrite_gem( // section still records the upstream source, so `bundle install` with // `frozen`/`--deployment` set rejects the Gemfile's new source block. // Mirror of the CLI's pnpm trust-lockfile warning. A converged pair (the - // CHECKSUMS-era path) is frozen-installable as written — no caveat. + // CHECKSUMS-era path, or a bundler < 2.2 merged section refreshed in + // place) is frozen-installable as written — no caveat. if (gemfile_changed || lock_changed) && mixed_state { result.warnings.push(RewriteWarning { code: "redirect_gem_frozen_install".into(), From f26a4a0fc5bd6f701f94056e1e1caf75f471cd91 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 08:07:31 +0000 Subject: [PATCH 5/5] Simplify the gem frozen-install check Clippy flagged the negated boolean; name the condition instead. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/patch/redirect/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 09e08c214..99765c02d 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -7760,7 +7760,8 @@ fn rewrite_gem( result, ) }); - if !converged && !(merged_refreshed && bundled_before_2_2(lk)) { + let frozen_installable = converged || (merged_refreshed && bundled_before_2_2(lk)); + if !frozen_installable { mixed_state = true; } }