From fe87eaf1858e836b0e569ab54f4b1e4971c29708 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 08:24:10 +0000 Subject: [PATCH 1/6] Start fix for #1626 Assisted-by: Claude Code:claude-opus-5-5 From ed663e438c4523d94cc8594942e14315005b2e0d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 08:46:00 +0000 Subject: [PATCH 2/6] Undo a superseding patch's Gemfile block on unwind On a Gemfile.lock without a CHECKSUMS section, a hosted re-scan that picks up a newer patch for the same gem moves only the Gemfile's source block to the new patch; the lock keeps the old one until the next bundle install. A rollback or remove in that window restored the lock but left the new block, reported success, and the next bundle install put the patched gem back. The restore now also undoes a Socket source block for the same gem name and version under another patch uuid when the lock pins the gem, so the Gemfile and lock come back consistent. Ambiguous shapes (several such blocks, or the uuid named elsewhere) are still refused. Fixes #1626 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/upstream/gem.rs | 186 +++++++++++++++--- 1 file changed, 163 insertions(+), 23 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs index 67d078cf2..40b99a00d 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs @@ -442,42 +442,83 @@ fn find_block( rel: &str, ctx: &Ctx<'_>, ) -> Result, String> { + let hits: Vec = socket_blocks(text, gem, ctx) + .into_iter() + .filter(|(_, uuid)| uuid == gem.uuid) + .map(|(b, _)| b) + .collect(); + let rest_mentions = |b: Option<&Block>| mentions_outside(text, b, gem.uuid); + match hits.as_slice() { + [] if rest_mentions(None) => Err(format!( + "{rel} wires it in a shape other than the source block socket-patch writes" + )), + [] => Ok(None), + [_] if rest_mentions(hits.first()) => Err(format!( + "{rel} names the patch outside the source block socket-patch writes" + )), + [_] => Ok(hits.into_iter().next()), + _ => Err(format!("{rel} declares it in several Socket source blocks")), + } +} + +/// Every rewriter-shaped `source "" do` block for `gem`'s +/// name and version, with the hosted patch uuid its index names. +fn socket_blocks(text: &str, gem: &Gem<'_>, ctx: &Ctx<'_>) -> Vec<(Block, String)> { let re = Regex::new(&format!( r#"(?m)^source "([^"\r\n]*)" do\r?\n gem "{}", "{}"(?:, ([^\r\n]*))?\r?\nend(\r?\n|\z)"#, regex::escape(&gem.name), regex::escape(&gem.version) )) .expect("source-block regex from escaped coordinates is valid"); - let hits: Vec = re - .captures_iter(text) - .filter(|c| ctx.hosted_uuid(&c[1]).as_deref() == Some(gem.uuid)) - .map(|c| { + re.captures_iter(text) + .filter_map(|c| { + let uuid = ctx.hosted_uuid(&c[1])?; let m = c.get(0).expect("group 0 is the whole match"); - Block { + let block = Block { start: m.start(), end: m.end(), opts: c.get(2).map(|o| o.as_str().to_string()), eol: !c[3].is_empty(), - } + }; + Some((block, uuid)) }) + .collect() +} + +/// Whether `text` names `uuid` anywhere but inside `block`. +fn mentions_outside(text: &str, block: Option<&Block>, uuid: &str) -> bool { + match block { + Some(b) => text[..b.start].contains(uuid) || text[b.end..].contains(uuid), + None => text.contains(uuid), + } +} + +/// The block a superseding hosted re-scan left for `gem` (#1626): on a lock +/// with no `CHECKSUMS` section that scan moves only the manifest's block to +/// the new patch's uuid, so until a `bundle install` the lock pins `gem`'s +/// uuid while the manifest's block names another patch of the same name +/// and version. Restoring the lock alone would leave that block behind, +/// and the next unfrozen install would re-patch. Returns the block and the +/// superseding patch's uuid. +fn superseded_block( + text: &str, + gem: &Gem<'_>, + rel: &str, + ctx: &Ctx<'_>, +) -> Result, String> { + let hits: Vec<(Block, String)> = socket_blocks(text, gem, ctx) + .into_iter() + .filter(|(_, uuid)| uuid != gem.uuid) .collect(); - let rest_mentions = |b: Option<&Block>| { - let rest = match b { - Some(b) => format!("{}{}", &text[..b.start], &text[b.end..]), - None => text.to_string(), - }; - rest.contains(gem.uuid) - }; match hits.as_slice() { - [] if rest_mentions(None) => Err(format!( - "{rel} wires it in a shape other than the source block socket-patch writes" - )), [] => Ok(None), - [_] if rest_mentions(hits.first()) => Err(format!( - "{rel} names the patch outside the source block socket-patch writes" + [(b, uuid)] if mentions_outside(text, Some(b), uuid) => Err(format!( + "{rel} names the superseding patch {uuid} outside the source block socket-patch writes" )), [_] => Ok(hits.into_iter().next()), - _ => Err(format!("{rel} declares it in several Socket source blocks")), + _ => Err(format!( + "{rel} declares it in several superseding Socket source blocks" + )), } } @@ -1228,10 +1269,20 @@ pub(crate) async fn restore( let mut manifest = manifest_raw.clone(); let globals = manifest.as_deref().map(global_sources).unwrap_or_default(); let handled_before = result.handled.clone(); - for pin in pins { + // Pins the lock wires go first, so one whose superseding patch's + // manifest block is also a pin here undoes that block itself + // (#1626), and the block's own pin then has nothing left to do. + let mut ordered: Vec<&&HostedPin> = pins.iter().collect(); + ordered.sort_by_key(|p| !lock.as_deref().is_some_and(|l| l.contains(p.uuid.as_str()))); + let mut undone: Vec = Vec::new(); + for pin in ordered { if result.refused.contains_key(&pin.uuid) { continue; } + if undone.contains(&pin.uuid) { + result.handled.insert(pin.uuid.clone()); + continue; + } let Some((name, version)) = pin.name_version() else { result.refuse(&pin.uuid, format!("{} is not a gem purl", pin.purl)); continue; @@ -1241,12 +1292,14 @@ pub(crate) async fn restore( name, version, }; + let mut superseded = None; match restore_one( &gem, lock.as_deref(), manifest.as_deref(), manifest_rel, &globals, + &mut superseded, ctx, ) .await @@ -1263,16 +1316,19 @@ pub(crate) async fn restore( lock = next_lock; manifest = next_manifest; result.handled.insert(pin.uuid.clone()); + undone.extend(superseded); } Err(why) => result.refuse(&pin.uuid, why), } } // The pins this pair restored, each judged once against its own - // restored lock. + // restored lock (a superseding pin shares its archive with the pin + // that undid it). for pin in pins { if !result.handled.contains(&pin.uuid) || handled_before.contains(&pin.uuid) || result.refused.contains_key(&pin.uuid) + || undone.contains(&pin.uuid) { continue; } @@ -1373,16 +1429,18 @@ async fn stale_cache_warning( } /// Restore one gem in a lock + manifest pair: `Ok(None)` when neither wires -/// it, else the next `(lock, manifest)` texts. +/// it, else the next `(lock, manifest)` texts. A superseding patch's block +/// undone with it ([`superseded_block`]) leaves its uuid in `superseded`. async fn restore_one( gem: &Gem<'_>, lock: Option<&str>, manifest: Option<&str>, manifest_rel: &str, globals: &[String], + superseded: &mut Option, ctx: &Ctx<'_>, ) -> Result, Option)>, String> { - let block = match manifest { + let mut block = match manifest { Some(text) => find_block(text, gem, manifest_rel, ctx)?, None => None, }; @@ -1391,6 +1449,16 @@ async fn restore_one( Some(_) => wiring(&gem_sections(&lines), gem.uuid, ctx)?.is_some(), None => false, }; + if wired && block.is_none() { + if let Some((b, uuid)) = manifest + .map(|text| superseded_block(text, gem, manifest_rel, ctx)) + .transpose()? + .flatten() + { + block = Some(b); + *superseded = Some(uuid); + } + } if !wired && block.is_none() { return Ok(None); } @@ -1876,6 +1944,7 @@ mod tests { Some(hosted_manifest), "Gemfile", &global_sources(hosted_manifest), + &mut None, &ctx, ) .await @@ -1978,6 +2047,7 @@ mod tests { Some(hosted_manifest), "Gemfile", &global_sources(hosted_manifest), + &mut None, &ctx, ) .await @@ -2558,6 +2628,76 @@ mod tests { assert_eq!(stale_cache(&r.warnings).len(), 1, "{:?}", r.warnings); } + /// #1626: a superseding hosted re-scan on a lock with no CHECKSUMS + /// moves only the Gemfile block to the new uuid; the lock still pins + /// the old one until a `bundle install`. Restoring the lock's pin must + /// undo that block too, or the next unfrozen install re-patches. + #[tokio::test] + async fn restore_undoes_a_superseding_patch_block() { + const NEW: &str = "88888888-8888-8888-8888-888888888888"; + let client = super::super::UpstreamClient::new(true); + let ctx = ctx_with(&client); + let upstream = hosted_pair(false, &"a".repeat(64)); + let run = |pins: Vec, gemfile: String| { + let lock = upstream["Gemfile.lock"].clone(); + let ctx = &ctx; + async move { + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("Gemfile"), &gemfile).unwrap(); + std::fs::write(tmp.path().join("Gemfile.lock"), &lock).unwrap(); + let mut view = View::new(tmp.path()); + let refs: Vec<&HostedPin> = pins.iter().collect(); + let files = vec!["Gemfile".to_string(), "Gemfile.lock".to_string()]; + let r = restore(&mut view, &refs, &files, ctx).await; + let changed = view.changed(); + ( + r, + changed.get("Gemfile").cloned().flatten(), + changed.get("Gemfile.lock").cloned().flatten(), + ) + } + }; + let pin = |uuid: &str, files: &[&str]| HostedPin { + purl: "pkg:gem/rails@7.0.0".into(), + uuid: uuid.into(), + files: files.iter().map(|f| f.to_string()).collect(), + }; + let superseded = upstream["Gemfile"].replace(UUID, NEW); + assert!(superseded.contains(NEW), "{superseded}"); + let want = "source \"https://rubygems.org\"\n\ngem \"rails\", \"7.0.0\"\n"; + + // Only the lock's (old) pin is discovered. + let (r, gemfile, lock) = run(vec![pin(UUID, &["Gemfile.lock"])], superseded.clone()).await; + assert!(r.refused.is_empty(), "{:?}", r.refused); + assert!(r.handled.contains(UUID)); + assert_eq!(gemfile.as_deref(), Some(want)); + let lock = lock.expect("the lock is restored"); + assert!( + !lock.contains(IDX) && !lock.contains("patch-registry"), + "{lock}" + ); + + // Both pins in one run: each is handled, neither refused. + let (r, gemfile, _) = run( + vec![pin(UUID, &["Gemfile.lock"]), pin(NEW, &["Gemfile"])], + superseded.clone(), + ) + .await; + assert!(r.refused.is_empty(), "{:?}", r.refused); + assert!(r.handled.contains(UUID) && r.handled.contains(NEW)); + assert_eq!(gemfile.as_deref(), Some(want)); + + // Two superseding blocks for the same gem: not ours to pick. + let other = upstream["Gemfile"].replace(UUID, "99999999-9999-9999-9999-999999999999"); + let block = &other[other.find("source \"https://patch").unwrap()..]; + let (r, _, _) = run( + vec![pin(UUID, &["Gemfile.lock"])], + format!("{superseded}{block}"), + ) + .await; + assert!(r.refused.contains_key(UUID), "{:?}", r.handled); + } + #[test] fn global_sources_skip_blocks() { let m = format!("source 'https://rubygems.org'\nsource(\"https://b.example\")\nsource \"{IDX}\" do\nend\n"); From 387f83d0c290f3d510dbed29e78dc145769e9084 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 08:50:24 +0000 Subject: [PATCH 3/6] Test gem unwinds after an unconverged supersede Adds the end-to-end repro from the issue: hosted scan, converge, superseding re-scan that leaves the Gemfile on the new patch and the lock on the old one, then rollback and remove. Both must leave no Socket wiring in either file, and a frozen bundle install must accept the restored pair. Documents the case in the gem unwind contract. Refs #1626 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../tests/e2e_redirect_gem_build.rs | 125 ++++++++++++++++++ 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 9b37242d4..69c4480c3 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1069,7 +1069,7 @@ v5.0 replaces v4's per-purl reverts and whole-ledger reverse replay (`revert_rem * **cargo** — `Cargo.lock` back on crates.io (source + the sparse index's checksum, `SOCKET_CRATES_INDEX`). When the lock already holds a crates.io block for the same `name@version` (a later dependent locked its own copy, #679), the Socket block merges into it instead: it is dropped (with a v1 lock's `[metadata]` line), and dependents' references take the spelling cargo writes, so no duplicate block is left that cargo cannot parse (#863), and no index lookup is needed; every `Cargo.toml` declaration loses its `registry = "socket-patch-"` pin (the shorthand the rewriter produced collapses back); every `[registries.socket-patch-]` block no manifest or lock still references leaves the project cargo config — including a superseded patch generation's block an earlier re-pin left behind (#864). A declaration it cannot unpin refuses. * **golang** — the hosted `replace` and the socket module's go.sum lines go; the upstream module's two go.sum lines come back, hashed from the module proxy (`SOCKET_GOPROXY`, else `GOPROXY` / `GONOPROXY` / `GOPRIVATE` as go reads them) and cross-checked against the checksum database (`SOCKET_GOSUMDB_URL`, else `sum.golang.org` unless `GOSUMDB=off` / `GONOSUMDB` / `GOPRIVATE` say go would not ask it). A `replace` the user had before the hosted run is not recorded anywhere, so the restore lands on the plain upstream module. * **pypi** — `Pipfile.lock`, `requirements.txt` (+ in-root `-r` includes), Hatch PEP 508 direct references (`pyproject.toml` / `hatch.toml`), `poetry.lock`, `pdm.lock`, `uv.lock`, PEP 723 script locks and PEP 751 `pylock*.toml` (+ the paired `pyproject.toml` / script metadata): hashes re-derived from PyPI's JSON API (`SOCKET_PYPI_JSON_API`). A restored `requirements.txt` line gets `--hash` options only when the file is in pip's hash-checking mode. The mode is read off the file's other requirement lines (an `-e` / `--editable` line means unhashed). When every requirement is a hosted pin, it is read off the hosted line itself (`--hash` vs a `#sha256=` url fragment) (#410). Refused: a `static_urls` `pdm.lock` whose project installs from an index other than PyPI (a `[[tool.pdm.source]]` in the sibling `pyproject.toml`, a project `pypi.url` (`.pdm.toml`, then `pdm.toml`), or, with neither, another package's file URL in the lock off PyPI's file host), since restoring PyPI's file URLs would make `pdm sync` bypass that index (#413); a `pdm.lock` without `cross_platform`, or a uv / script / pylock lock, whose release has a wheel that is not pure Python 3 (which files the lock keeps is not re-derivable); a uv lock whose options filter files (`exclude-newer`, `no-binary`, `no-build`, or an `exclude-newer-package` cutoff for the restored package; one of `false` exempts it from the global cutoff here and in pylocks, as in uv) (#1604); a pylock from `uv pip compile` whose header command line (`--no-build`, `--only-binary`, `--no-binary`, `--exclude-newer`, `--exclude-newer-package`) or uv settings (the nearest `uv.toml`, else a pyproject's `[tool.uv]` / `[tool.uv.pip]`, then the user's and the system's `uv.toml` unless `UV_NO_CONFIG` is set, merged as uv does with `[pip]` over the top level; none under `--no-config`, and any `--config-file` refuses; `UV_CONFIG_FILE` or `UV_EXCLUDE_NEWER` set in the restoring environment also refuses, since the header does not record uv's environment) filter the restored package's files, or whose header records no `uv pip compile` command line (`--no-header`, `UV_CUSTOM_COMPILE_COMMAND`), since uv 0.9.16 records only the files those filters let through (#1613); a uv lock whose other registry packages name no registry, several, or one other than PyPI's simple index; a pylock whose other registry packages show neither an `index` nor (as `uv pip compile` writes them) only PyPI files with none, which restores the entry without an `index` too; uv 0.2 `[[distribution]]` locks. Restored artifact fields keep the spelling the lock's other entries show, including the `upload_time` that uv 0.6.15–0.6.17 write. A restored pylock entry's `upload-time`s are whole seconds, as uv writes them, unless the lock's other entries show fractions. Its artifacts come back in the TOML spelling the other entries use: uv's inline `wheels = [{ … }]`, or the standard tables `pip lock` writes (`[[packages.wheels]]` with a `[packages.wheels.hashes]` sub-table, `[packages.sdist]`). A `pip lock` file (`created-by = "pip"`) records only the artifact pip selected, so the entry is restored with only the release's wheel (its sdist when it has none), and a release with several wheels is refused. A transitive `override-dependencies` entry hosted mode added is removed (`upstream_uv_override_removed`). Hosted mode keeps no ledger, so it marks the entry it adds with a `# socket-patch hosted: …` comment line above it and removes only a marked entry; a user's own `==` override (which the hosted rewrite reuses rather than adding a second one) is kept, and the lock's `[manifest] overrides` record of it gets its specifier back. - * **gem** — `Gemfile.lock` / `gems.locked` + `Gemfile` / `gems.rb`: the spec moves back into the upstream `GEM` section (or the Socket remote leaves a merged section), the `source "" do … end` block is undone, the `CHECKSUMS` entry is re-pinned from the rubygems.org compact index (`SOCKET_RUBYGEMS_URL`) and the `DEPENDENCIES` pin loses its `!`, unless the declaration comes back inside the user's own `source "…" do` block (Bundler writes the `!` for any dependency declared in one, rubygems.org included), where the `!` stays. The declaration's original constraint is not recorded, so it comes back as the exact pin `gem "", ""`. A transitive gem (one the manifest never declared) gets an appended block with a blank line before it; the restore removes that block, its blank line and the `DEPENDENCIES` entry, so the pair comes back byte for byte. An appended block with no blank line before it (written by a release before this one) can't be told apart from an in-place rewrite, so it still comes back as the exact pin. Refused: an ambiguous upstream section, an upstream remote other than rubygems.org. The manifest pair can't make a committed bundler cache upstream: a `-.gem` left in Bundler's cache dir that isn't the upstream archive is named by `upstream_gem_stale_cache`, never deleted. + * **gem** — `Gemfile.lock` / `gems.locked` + `Gemfile` / `gems.rb`: the spec moves back into the upstream `GEM` section (or the Socket remote leaves a merged section), the `source "" do … end` block is undone (also when a superseding hosted re-scan on a lock with no `CHECKSUMS` section has moved that block to the newer patch of the same name and version while the lock still pins the older one; several such blocks are refused), the `CHECKSUMS` entry is re-pinned from the rubygems.org compact index (`SOCKET_RUBYGEMS_URL`) and the `DEPENDENCIES` pin loses its `!`, unless the declaration comes back inside the user's own `source "…" do` block (Bundler writes the `!` for any dependency declared in one, rubygems.org included), where the `!` stays. The declaration's original constraint is not recorded, so it comes back as the exact pin `gem "", ""`. A transitive gem (one the manifest never declared) gets an appended block with a blank line before it; the restore removes that block, its blank line and the `DEPENDENCIES` entry, so the pair comes back byte for byte. An appended block with no blank line before it (written by a release before this one) can't be told apart from an in-place rewrite, so it still comes back as the exact pin. Refused: an ambiguous upstream section, an upstream remote other than rubygems.org. The manifest pair can't make a committed bundler cache upstream: a `-.gem` left in Bundler's cache dir that isn't the upstream archive is named by `upstream_gem_stale_cache`, never deleted. * **composer** — `composer.lock`: `dist` and the deleted `source` block from packagist's composer v2 metadata (`SOCKET_PACKAGIST_URL`). Refused unless the entry is packagist-sourced and packagist still serves the lock's `dist.reference` for the version. * **maven** — `pom.xml` (the `-socket.` version suffix, the added `` / `` entry) and the `.mvn/maven.config` / `.mvn/checksums/checksums.sha256` lines hosted mode writes: **no network**, so it restores under `--offline` too. `.mvn` files holding anything else keep the resolver lines (`maven_trusted_checksums_left`). * **nuget** — `nuget.config` loses the `socket-patch-` source and its exact-id mapping; every lock entry of the id at the pinned version, in every lock the root config governs (member projects' and `packages..lock.json` included), gets nuget.org's `contentHash` back (`SOCKET_NUGET_URL`). Refused when the restored config would not resolve the id from nuget.org alone. A config hosted mode created from scratch is kept (`nuget_default_config_left`). 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 fc0802015..c7beec38b 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -2272,6 +2272,131 @@ async fn gem_hosted_superseding_patch_keeps_gem_sections_in_bundler_order() { ); } +/// #1626: on a lock with no CHECKSUMS section, a superseding hosted +/// re-scan moves only the Gemfile's `source … do` block to the new patch; +/// the lock keeps pinning the old one until a `bundle install`. A +/// `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. +#[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"] +async fn gem_hosted_unwind_after_unconverged_supersede_undoes_the_new_block() { + for command in ["rollback", "remove"] { + unwind_after_unconverged_supersede(command).await; + } +} + +async fn unwind_after_unconverged_supersede(command: &str) { + let Some(fx) = redirect_scanned_project( + &format!("unconverged-supersede-{command}"), + Spelling::Gemfile, + false, + true, + None, + Driver::ScanVex, + ) + .await + else { + return; + }; + const GEN2: &str = "10000000-1a2b-4a1b-8c2d-3e4f5a6b7c8d"; + const GEN3: &str = "9a9a9a9a-1a2b-4a1b-8c2d-3e4f5a6b7c8d"; + let stage = fx.tmp.path().join("generation-stage"); + let vuln = |uuid: &'static str, lib: String| GenerationGem { + name: DEP, + uuid, + deps: vec![format!("{TRANSITIVE}:>= 0")], + lib_file: "vuln_gem.rb", + orig: orig_lib(), + gem: build_gem( + &stage.join(uuid), + DEP, + DEP_VERSION, + "vuln_gem.rb", + &lib, + &[TRANSITIVE], + ), + patched: lib, + }; + let api = fx._server.uri(); + mount_patch_generation(&fx._server, 2, &[vuln(GEN2, patched_lib())]).await; + let (code, out, err) = run_hosted_scan(&fx.proj, &api); + assert_eq!(code, 0, "{command}: gen 2 scan: {out}\n{err}"); + let install = bundle(&fx.proj, &["install"]); + assert!( + install.status.success(), + "{command}: converge gen 2:\n{}", + String::from_utf8_lossy(&install.stderr) + ); + mount_patch_generation( + &fx._server, + 1, + &[vuln(GEN3, patched_lib().replace("PATCHED", "PATCHED-GEN3"))], + ) + .await; + let (code, out, err) = run_hosted_scan(&fx.proj, &api); + 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}" + ); + + let upstream = format!("{api}/upstream"); + let cwd = fx.proj.to_str().expect("utf8 tmp path"); + let mut argv = vec![command, PURL, "--json", "--cwd", cwd]; + if command == "remove" { + argv.push("--yes"); + } + argv.extend([ + "--api-url", + &api, + "--org", + ORG, + "--api-token", + "fake", + "--patch-server-url", + &api, + ]); + let (code, stdout, stderr) = + run_socket_env(&fx.proj, &argv, &[("SOCKET_RUBYGEMS_URL", &upstream)]); + assert_eq!(code, 0, "{command}: {stdout}\nstderr:\n{stderr}"); + let gemfile = std::fs::read_to_string(&gemfile_path).unwrap(); + let lock = std::fs::read_to_string(&lock_path).unwrap(); + assert!( + !gemfile.contains("patch-registry"), + "{command}: the Gemfile is still wired to a Socket patch:\n{gemfile}" + ); + assert!( + !lock.contains("patch-registry"), + "{command}: the lock is still wired to a Socket patch:\n{lock}" + ); + assert!( + gemfile.contains(&format!("gem \"{DEP}\"")), + "{command}: the Gemfile declares the gem again:\n{gemfile}" + ); + // The pair is consistent: a frozen install accepts it as is. + let frozen = bundle_env(&fx.proj, &["install"], &[("BUNDLE_FROZEN", "true")]); + assert!( + frozen.status.success(), + "{command}: frozen install after the unwind:\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 restored lock as is" + ); +} + /// Bundler's modern `gems.rb`/`gems.locked` spelling, end to end: the /// candidate list must read the pair, the rewriter must key its edits to it, /// and the real bundler must install the patched gem from the redirected From b3caa5415f818736d883244e0d05d16fced9ebb3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 09:30:29 +0000 Subject: [PATCH 4/6] Refuse the superseding gem pin restored alone When the Gemfile names the newer patch but the lock still pins the older one, restoring only the newer patch's pin undid its block and left the lock on the older patch, so the pair stayed inconsistent. That pin is now refused with a message naming the older patch, while the older patch's pin restores both files. Pins the lock really wires (not ones whose uuid just appears in it) go first, so either order of the two pins gives the same result. Refs #1626 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/upstream/gem.rs | 93 ++++++++++++++----- 1 file changed, 70 insertions(+), 23 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs index 40b99a00d..f954ce2a9 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs @@ -1272,8 +1272,14 @@ pub(crate) async fn restore( // Pins the lock wires go first, so one whose superseding patch's // manifest block is also a pin here undoes that block itself // (#1626), and the block's own pin then has nothing left to do. + let lock_wires = |uuid: &str| { + lock.as_deref().is_some_and(|l| { + let lines: Vec<&str> = l.split('\n').collect(); + matches!(wiring(&gem_sections(&lines), uuid, ctx), Ok(Some(_))) + }) + }; let mut ordered: Vec<&&HostedPin> = pins.iter().collect(); - ordered.sort_by_key(|p| !lock.as_deref().is_some_and(|l| l.contains(p.uuid.as_str()))); + ordered.sort_by_key(|p| !lock_wires(&p.uuid)); let mut undone: Vec = Vec::new(); for pin in ordered { if result.refused.contains_key(&pin.uuid) { @@ -1292,20 +1298,18 @@ pub(crate) async fn restore( name, version, }; - let mut superseded = None; match restore_one( &gem, lock.as_deref(), manifest.as_deref(), manifest_rel, &globals, - &mut superseded, ctx, ) .await { Ok(None) => {} - Ok(Some((next_lock, next_manifest))) => { + Ok(Some((next_lock, next_manifest, superseded))) => { if endings == Some(LineEndings::Mixed) && next_lock != lock { result.refuse( &pin.uuid, @@ -1429,17 +1433,16 @@ async fn stale_cache_warning( } /// Restore one gem in a lock + manifest pair: `Ok(None)` when neither wires -/// it, else the next `(lock, manifest)` texts. A superseding patch's block -/// undone with it ([`superseded_block`]) leaves its uuid in `superseded`. +/// it, else the next `(lock, manifest)` texts and the uuid of a superseding +/// patch whose block was undone with it ([`superseded_block`]). async fn restore_one( gem: &Gem<'_>, lock: Option<&str>, manifest: Option<&str>, manifest_rel: &str, globals: &[String], - superseded: &mut Option, ctx: &Ctx<'_>, -) -> Result, Option)>, String> { +) -> Result, String> { let mut block = match manifest { Some(text) => find_block(text, gem, manifest_rel, ctx)?, None => None, @@ -1449,6 +1452,7 @@ async fn restore_one( Some(_) => wiring(&gem_sections(&lines), gem.uuid, ctx)?.is_some(), None => false, }; + let mut superseded = None; if wired && block.is_none() { if let Some((b, uuid)) = manifest .map(|text| superseded_block(text, gem, manifest_rel, ctx)) @@ -1456,12 +1460,25 @@ async fn restore_one( .flatten() { block = Some(b); - *superseded = Some(uuid); + superseded = Some(uuid); } } if !wired && block.is_none() { return Ok(None); } + // The other side of #1626: the manifest's block is this (superseding) + // patch's, but the lock still pins the gem to an earlier Socket patch. + // Undoing the block alone would leave the lock on that patch, so only + // the earlier patch's pin (which undoes both) can restore the pair. + if !wired { + if let Some(earlier) = earlier_lock_pin(&lines, gem, ctx) { + return Err(format!( + "the lock still pins it to the earlier Socket patch {earlier} (a superseding \ + re-scan moved only {manifest_rel}); restore that patch's pin, or run \ + `bundle install` first so the lock follows {manifest_rel}" + )); + } + } let decl = if wired { let transitive = block.as_ref().is_some_and(|b| { b.opts.is_none() @@ -1518,7 +1535,29 @@ async fn restore_one( (Some(text), Some(b)) => Some(restore_manifest(text, b, gem, &decl)), _ => manifest.map(str::to_string), }; - Ok(Some((next_lock, next_manifest))) + Ok(Some((next_lock, next_manifest, superseded))) +} + +/// What [`restore_one`] gives back: the next lock and manifest texts, and +/// the superseding patch whose block it undid. +type RestoredPair = (Option, Option, Option); + +/// Another Socket patch's uuid whose lock section locks `gem`'s name and +/// version (the lock side of an unconverged superseding re-scan). +fn earlier_lock_pin(lines: &[&str], gem: &Gem<'_>, ctx: &Ctx<'_>) -> Option { + gem_sections(lines).iter().find_map(|s| { + if !s + .entries + .iter() + .any(|e| e.name == gem.name && e.version == gem.version) + { + return None; + } + s.remotes + .iter() + .filter_map(|(_, r)| ctx.hosted_uuid(r)) + .find(|uuid| uuid != gem.uuid) + }) } #[cfg(test)] @@ -1938,13 +1977,12 @@ mod tests { ); let (hosted_manifest, hosted_lock) = (&r.files["Gemfile"], &r.files["Gemfile.lock"]); assert!(hosted_lock.contains(" rails (= 7.0.0)!"), "{hosted_lock}"); - let (next_lock, next_manifest) = restore_one( + let (next_lock, next_manifest, _) = restore_one( &gem(), Some(hosted_lock), Some(hosted_manifest), "Gemfile", &global_sources(hosted_manifest), - &mut None, &ctx, ) .await @@ -2041,14 +2079,13 @@ mod tests { ); let (hosted_manifest, hosted_lock) = (&r.files["Gemfile"], &r.files["Gemfile.lock"]); assert!(hosted_lock.contains(IDX), "{hosted_lock}"); - let (next_lock, next_manifest) = restore_one( + let (next_lock, next_manifest, _) = restore_one( &gem(), Some(hosted_lock), Some(hosted_manifest), "Gemfile", &global_sources(hosted_manifest), - &mut None, - &ctx, + &ctx, ) .await .unwrap() @@ -2677,15 +2714,25 @@ mod tests { "{lock}" ); - // Both pins in one run: each is handled, neither refused. - let (r, gemfile, _) = run( + // Both pins in one run, in either order: the lock's pin undoes the + // block, and neither is refused. + for pins in [ vec![pin(UUID, &["Gemfile.lock"]), pin(NEW, &["Gemfile"])], - superseded.clone(), - ) - .await; - assert!(r.refused.is_empty(), "{:?}", r.refused); - assert!(r.handled.contains(UUID) && r.handled.contains(NEW)); - assert_eq!(gemfile.as_deref(), Some(want)); + vec![pin(NEW, &["Gemfile"]), pin(UUID, &["Gemfile.lock"])], + ] { + let (r, gemfile, lock) = run(pins, superseded.clone()).await; + assert!(r.refused.is_empty(), "{:?}", r.refused); + assert!(r.handled.contains(UUID) && r.handled.contains(NEW)); + assert_eq!(gemfile.as_deref(), Some(want)); + assert!(!lock.unwrap().contains("patch-registry")); + } + + // The superseding pin alone can't restore the pair: undoing its + // block would leave the lock on the earlier patch. + let (r, gemfile, lock) = run(vec![pin(NEW, &["Gemfile"])], superseded.clone()).await; + let why = r.refused.get(NEW).expect("refused"); + assert!(why.contains(UUID), "{why}"); + assert_eq!((gemfile, lock), (None, None)); // Two superseding blocks for the same gem: not ours to pick. let other = upstream["Gemfile"].replace(UUID, "99999999-9999-9999-9999-999999999999"); From 423f48666a2665429a645f4a9370d033fe838fc6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 09:58:11 +0000 Subject: [PATCH 5/6] Only own lock sections name an earlier gem patch On a bundler < 2.2 lock, one merged GEM section lists the specs of every remote together, so another gem's Socket remote there says nothing about which patch serves this gem. The earlier-patch check read it as one and refused a valid restore of this gem. It now looks only at sections with a single remote. Refs #1626 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/patch/redirect/upstream/gem.rs | 40 ++++++++++++++++--- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs index f954ce2a9..07353c23d 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs @@ -1542,10 +1542,16 @@ async fn restore_one( /// the superseding patch whose block it undid. type RestoredPair = (Option, Option, Option); -/// Another Socket patch's uuid whose lock section locks `gem`'s name and -/// version (the lock side of an unconverged superseding re-scan). +/// Another Socket patch's uuid whose own lock section locks `gem`'s name +/// and version (the lock side of an unconverged superseding re-scan). Only +/// a single-remote section says which patch serves a spec: a merged +/// section (bundler < 2.2) lists every remote's specs together, so another +/// gem's Socket remote there says nothing about this gem. fn earlier_lock_pin(lines: &[&str], gem: &Gem<'_>, ctx: &Ctx<'_>) -> Option { gem_sections(lines).iter().find_map(|s| { + let [(_, remote)] = s.remotes.as_slice() else { + return None; + }; if !s .entries .iter() @@ -1553,10 +1559,7 @@ fn earlier_lock_pin(lines: &[&str], gem: &Gem<'_>, ctx: &Ctx<'_>) -> Option = own.split('\n').collect(); + assert_eq!( + earlier_lock_pin(&lines, &gem(), &ctx).as_deref(), + Some("99999999-9999-9999-9999-999999999999") + ); + let merged = format!( + "GEM\n remote: https://rubygems.org/\n remote: {OTHER}\n specs:\n puma \ + (6.0.0)\n rails (7.0.0)\n\nDEPENDENCIES\n puma!\n rails (= 7.0.0)\n" + ); + let lines: Vec<&str> = merged.split('\n').collect(); + assert_eq!(earlier_lock_pin(&lines, &gem(), &ctx), None); + } + #[test] fn global_sources_skip_blocks() { let m = format!("source 'https://rubygems.org'\nsource(\"https://b.example\")\nsource \"{IDX}\" do\nend\n"); From da3c35398f1dcd6445505e075fb8155d04f0f7a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 11:58:39 +0000 Subject: [PATCH 6/6] Fix misindented restore_one argument in a gem test rustfmt skips this test function (its long string literals defeat it), so the extra indentation the #1626 change introduced on `&ctx,` was never corrected by `cargo fmt --check`. Align it with the other arguments. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/socket-patch-core/src/patch/redirect/upstream/gem.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs index 07353c23d..fc0d2f30b 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/gem.rs @@ -2088,7 +2088,7 @@ mod tests { Some(hosted_manifest), "Gemfile", &global_sources(hosted_manifest), - &ctx, + &ctx, ) .await .unwrap()