From 2030b3cccaa957071552e253b0082609f62b7d61 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 04:23:12 +0000 Subject: [PATCH 1/3] Start fix for #1766 Assisted-by: Claude Code:claude-opus-5-5 From 92e6d2f6f1f3e99724eb6c4030fcb0792486f828 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 04:51:48 +0000 Subject: [PATCH 2/3] Warn on stale go.work vendor in workspace members After rollback or remove in a go.work member other than the workspace root, socket-patch stayed silent about the vendor/modules.txt line that `go work vendor` had recorded for the patch. Every workspace build then failed with "inconsistent vendoring" while both commands exited 0. `go work vendor` writes a member's replace relative to the go.work directory (./a/.socket/go-patches/...), and the stale check only treated root-relative .socket paths as socket-owned. The audit now resolves a recorded path against the go.work directory and treats it as ours when it lies under any member's .socket copy directories. A user's replace onto another checkout (../sibling/.socket/...) stays user-owned. Adds unit tests (go-patches and vendored copies, nested and out-of-tree members) and a real-go e2e covering rollback and remove. Fixes #1766 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_golang_build.rs | 107 +++++++++++++++ .../src/vendor/go_consumer_sync.rs | 125 +++++++++++++++--- 2 files changed, 215 insertions(+), 17 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index ad8515f65..f3ff71442 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -500,6 +500,113 @@ fn committed_vendor_dir_needs_go_mod_vendor_after_apply_and_rollback() { chmod_writable(tmp.path()); } +/// #1766: `go work vendor` records a non-root member's replace relative to +/// the workspace root (`./consumer/.socket/go-patches/…`). After rollback +/// or remove in that member every workspace build fails "inconsistent +/// vendoring", so both must warn and `apply --check` must report it. +#[test] +fn go_work_vendor_member_needs_go_work_vendor_after_rollback_and_remove() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(go-work-vendor)") { + return; + } + if golang_e2e_matrix::go_minor() < 22 { + eprintln!("SKIP: `go work vendor` needs Go 1.22+"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let (consumer, modcache, proxy_url) = stage(tmp.path()); + let ws = tmp.path(); + std::fs::write(ws.join("go.work"), "go 1.22\n\nuse ./consumer\n").unwrap(); + let cs = consumer.to_str().unwrap(); + let mc = modcache.to_str().unwrap(); + let mut goenv = go_env(mc, &proxy_url); + goenv.retain(|(k, _)| *k != "GOFLAGS"); + goenv.push(("GOFLAGS", "")); + let vendor = |what: &str| { + let out = go(ws, &["work", "vendor"], &goenv); + assert!( + out.status.success(), + "go work vendor ({what}): {}", + String::from_utf8_lossy(&out.stderr) + ); + }; + let warns = |args: &[&str], what: &str| { + let (code, so, se) = run_socket(&consumer, args, &modcache); + assert_eq!(code, 0, "{what} failed.\n{so}\n{se}"); + assert!( + warnings_of(&so).contains(&"go_vendor_modules_txt_out_of_sync".to_string()), + "{what} must name the vendor/modules.txt regeneration:\n{so}" + ); + }; + let apply = ["apply", "--offline", "--json", "--cwd", cs]; + vendor("before apply"); + + write_patch(&consumer); + warns(&apply, "apply"); + vendor("after apply"); + let run = go(ws, &["run", "./consumer"], &goenv); + assert!( + String::from_utf8_lossy(&run.stdout).contains("OUT: PATCHED"), + "{}{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + let modules_txt = std::fs::read_to_string(ws.join("vendor/modules.txt")).unwrap(); + assert!( + modules_txt.contains("=> ./consumer/.socket/go-patches/"), + "the fixture records the member replace relative to go.work:\n{modules_txt}" + ); + + for (what, args) in [ + ( + "rollback", + vec!["rollback", "--offline", "--json", "--cwd", cs], + ), + ( + "remove", + vec!["remove", UPURL, "--offline", "--json", "--cwd", cs], + ), + ] { + warns(&args, what); + let broken = go(ws, &["build", "./consumer"], &goenv); + assert!( + !broken.status.success() + && String::from_utf8_lossy(&broken.stderr).contains("inconsistent vendoring"), + "{what}: the fixture reproduces the broken build: {}", + String::from_utf8_lossy(&broken.stderr) + ); + let (code, _so, se) = run_socket( + &consumer, + &["apply", "--check", "--ecosystems", "golang", "--cwd", cs], + &modcache, + ); + assert_eq!(code, 1, "{what}: stale vendor/modules.txt is drift:\n{se}"); + assert!( + se.contains("no longer has") && se.contains("go work vendor"), + "{what}: {se}" + ); + + // The step the warning names fixes the build. + vendor(what); + let run = go(ws, &["run", "./consumer"], &goenv); + assert!( + String::from_utf8_lossy(&run.stdout).contains("OUT: PRISTINE"), + "{what}: {}{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + if what == "rollback" { + // Re-apply and re-vendor so `remove` starts from a patched, + // in-sync workspace (rollback dropped the manifest entry). + write_patch(&consumer); + warns(&apply, "re-apply"); + vendor("after re-apply"); + } + } + + chmod_writable(tmp.path()); +} + /// #618: a patch whose go.mod raises a requirement (the common security-fix /// shape) leaves the default `-mod=readonly` build failing with "updates /// to go.mod needed". apply names `go mod tidy`, `apply --check` reports diff --git a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs index b51638281..796ce4225 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -343,19 +343,6 @@ async fn load_workspace(project_root: &Path) -> Option { }) } -/// The replaces a workspace `modules.txt` records: `go work vendor` writes -/// every `use` member's `go.mod` replaces plus the `go.work` ones. Returns -/// `(go.work replaces, every member's replaces)`. -async fn workspace_replaces(project_root: &Path) -> (Vec, Vec) { - match load_workspace(project_root).await { - Some(ws) => { - let members = ws.members.into_iter().flat_map(|m| m.replaces).collect(); - (ws.replaces, members) - } - None => (Vec::new(), Vec::new()), - } -} - /// Whether the `go.work` in effect for `project_root` keeps a project /// `go.mod` replace of `module@version` => `ours` (a replace as parsed /// from, or about to be written to, that go.mod) from taking effect. @@ -551,11 +538,19 @@ async fn vendor_modules_txt_issues( // In a workspace, a `go.work` replace of the same module overrides the // member's, and a recorded replacement may come from any member. - let (work_replaces, member_replaces) = if workspace { - workspace_replaces(project_root).await + // `go work vendor` writes every `use` member's `go.mod` replaces plus + // the `go.work` ones. + let ws = if workspace { + load_workspace(project_root).await } else { - (Vec::new(), Vec::new()) + None }; + let work_replaces = ws.as_ref().map(|w| w.replaces.clone()).unwrap_or_default(); + let member_replaces: Vec = ws + .iter() + .flat_map(|w| &w.members) + .flat_map(|m| m.replaces.iter().cloned()) + .collect(); let overridden_by_work = |entry: &ReplaceEntry| { work_replaces.iter().any(|w| { w.module == entry.module && (w.version.is_none() || w.version == entry.version) @@ -577,7 +572,10 @@ async fn vendor_modules_txt_issues( }); } } - for rec in recorded.iter().filter(|r| r.socket_owned()) { + for rec in recorded + .iter() + .filter(|r| r.socket_owned() || ws.as_ref().is_some_and(|w| member_owned(w, r))) + { if !replaces .iter() .chain(&member_replaces) @@ -595,6 +593,26 @@ async fn vendor_modules_txt_issues( issues } +/// Whether a workspace `modules.txt` replacement is a socket copy of some +/// `use` member. `go work vendor` rewrites a member's directory replace +/// relative to the go.work directory (`./a/.socket/go-patches/…`), which +/// [`go_mod_edit::detect_owner`] alone, anchored at one go.mod's directory, +/// reads as user-authored. Joined lexically, as go joins them. +fn member_owned(ws: &Workspace, recorded: &ReplaceEntry) -> bool { + let Some(path) = recorded.path.as_deref() else { + return false; + }; + let work_dir = ws.work_file.parent().unwrap_or(Path::new("")); + let abs = PathBuf::from(lexical_target(work_dir, recorded)); + let cwd = std::env::current_dir().unwrap_or_default(); + !Path::new(&path.replace('\\', "/")).is_absolute() + && ws.members.iter().any(|m| { + let dir = crate::utils::relpath::normalize_lexically_keeping_escapes(&cwd.join(&m.dir)); + abs.strip_prefix(&dir) + .is_ok_and(|rel| go_mod_edit::detect_owner(&rel.to_string_lossy()).is_some()) + }) +} + /// Whether a `modules.txt` replacement records the `go.mod` one. go writes /// the target verbatim from `go.mod`; a workspace vendor directory may /// record a module's directory replacement relative to the workspace root, @@ -834,6 +852,79 @@ mod tests { assert_eq!(codes, vec![WORKSPACE_CODE]); } + /// #1766: `go work vendor` records a non-root member's replace relative + /// to the workspace root (`./a/.socket/…`). After a rollback or remove + /// in that member, the recorded line is stale and the workspace build + /// fails "inconsistent vendoring", so the audit must say so, for either + /// socket backend and a member nested or outside the go.work directory. + #[tokio::test] + async fn workspace_member_rollback_leaves_modules_txt_stale() { + let vendored = ".socket/vendor/golang/11111111-1111-4111-8111-111111111111/\ + example.com/upstream@v1.0.0"; + for (use_dir, copy) in [ + ("a", COPY), + ("a", vendored), + ("sub/a", COPY), + ("../a", COPY), + ] { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path().join("ws"); + let member = ws.join(use_dir); + // How go spells a path relative to the go.work directory. + let rel = if use_dir.starts_with("../") { + use_dir.to_string() + } else { + format!("./{use_dir}") + }; + consumer(&member, ""); + write(&ws, "go.work", &format!("go 1.22\n\nuse {rel}\n")); + write( + &ws, + "vendor/modules.txt", + &format!( + "# example.com/upstream v1.0.0 => {rel}/{copy}\n\ + ## explicit; go 1.21\nexample.com/upstream\n" + ), + ); + let issues = audit(&member, &HashMap::new()).await; + assert!( + matches!( + &issues[..], + [GoSyncIssue::VendorModulesTxt { stale: true, command: "go work vendor", module, .. }] + if module == "example.com/upstream" + ), + "{use_dir} {copy}: {issues:?}" + ); + + // Applied again: the recorded line matches the member's replace. + consumer( + &member, + &format!("\nreplace example.com/upstream v1.0.0 => ./{copy}\n"), + ); + assert_eq!( + audit(&member, &HashMap::new()).await, + vec![], + "{use_dir} {copy}" + ); + } + + // A user's replace onto another checkout's copy is not ours, even + // when the path has a `.socket/go-patches/` segment. + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path().join("ws"); + consumer(&ws.join("a"), ""); + write(&ws, "go.work", "go 1.22\n\nuse ./a\n"); + write( + &ws, + "vendor/modules.txt", + &format!( + "# example.com/upstream v1.0.0 => ../sibling/{COPY}\n\ + ## explicit; go 1.21\nexample.com/upstream\n" + ), + ); + assert_eq!(audit(&ws.join("a"), &HashMap::new()).await, vec![]); + } + fn codes(issues: &[GoSyncIssue]) -> Vec<&'static str> { issues.iter().map(GoSyncIssue::code).collect() } From 9376571b92497b7fc5f543cab9fa2d6c7d707677 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 05:08:10 +0000 Subject: [PATCH 3/3] Match modules.txt lines by resolved target Review follow-up. The stale check compared a recorded workspace line with go.mod replaces by path suffix, so a stale ./a/.socket/... line still matched member b's ./.socket/... replace after a rollback in a and an apply in b. Both commands then stayed silent while the build failed "inconsistent vendoring". Each replace is now resolved against its own file's directory and compared with the recorded line resolved against the go.work (or go.mod) directory, as go does. A member whose go.mod cannot be read no longer makes its recorded lines look stale from other members. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/go_consumer_sync.rs | 167 +++++++++++------- 1 file changed, 103 insertions(+), 64 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs index 796ce4225..15c0f5ae7 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -318,6 +318,9 @@ struct Member { /// directory (go resolves the member's relative replaces against it). dir: PathBuf, replaces: Vec, + /// Whether the member's go.mod could be read (else `replaces` is empty + /// because it is unknown, not because there are none). + readable: bool, } async fn load_workspace(project_root: &Path) -> Option { @@ -328,13 +331,18 @@ async fn load_workspace(project_root: &Path) -> Option { let mut members = Vec::new(); for dir in go_mod_edit::parse_use_dirs(&text) { let dir = work_dir.join(dir); - let replaces = match read_regular_to_string(&dir.join("go.mod")).await { - Ok(go_mod) => { - go_mod_edit::parse_replace_entries(&go_mod_edit::normalize_for_read(&go_mod)) - } - Err(_) => Vec::new(), + let (replaces, readable) = match read_regular_to_string(&dir.join("go.mod")).await { + Ok(go_mod) => ( + go_mod_edit::parse_replace_entries(&go_mod_edit::normalize_for_read(&go_mod)), + true, + ), + Err(_) => (Vec::new(), false), }; - members.push(Member { dir, replaces }); + members.push(Member { + dir, + replaces, + readable, + }); } Some(Workspace { replaces: go_mod_edit::parse_replace_entries(&text), @@ -537,20 +545,53 @@ async fn vendor_modules_txt_issues( .collect(); // In a workspace, a `go.work` replace of the same module overrides the - // member's, and a recorded replacement may come from any member. + // member's, and a recorded replacement may come from any member: // `go work vendor` writes every `use` member's `go.mod` replaces plus - // the `go.work` ones. + // the `go.work` ones, each resolved against its own file's directory + // and recorded relative to the go.work directory. Outside a workspace + // `go mod vendor` records the go.mod's replaces verbatim. let ws = if workspace { load_workspace(project_root).await } else { None }; - let work_replaces = ws.as_ref().map(|w| w.replaces.clone()).unwrap_or_default(); - let member_replaces: Vec = ws + // modules.txt sits in `/vendor/`. + let work_dir = path.parent().and_then(Path::parent).unwrap_or(project_root); + let project = same_dir_key(project_root); + let project_dir = ws .iter() .flat_map(|w| &w.members) - .flat_map(|m| m.replaces.iter().cloned()) + .find(|m| same_dir_key(&m.dir) == project) + .map_or(project_root, |m| m.dir.as_path()); + let work_replaces: &[ReplaceEntry] = ws.as_ref().map_or(&[], |w| &w.replaces); + // Every replace modules.txt may record, with the directory go resolves + // it against. + let sources: Vec<(&Path, &ReplaceEntry)> = replaces + .iter() + .map(|e| (project_dir, e)) + .chain( + ws.iter() + .flat_map(|w| &w.members) + .flat_map(|m| m.replaces.iter().map(|e| (m.dir.as_path(), e))), + ) + .chain(work_replaces.iter().map(|e| (work_dir, e))) + .collect(); + // Members whose socket copies a recorded line may point into; one whose + // go.mod can't be read is left out, since its replaces are unknown. + let member_dirs: Vec = ws + .iter() + .flat_map(|w| &w.members) + .filter(|m| m.readable) + .map(|m| { + let abs = std::env::current_dir().unwrap_or_default().join(&m.dir); + crate::utils::relpath::normalize_lexically_keeping_escapes(&abs) + }) .collect(); + let records = |dir: &Path, entry: &ReplaceEntry, rec: &ReplaceEntry| { + entry.module == rec.module + && entry.version == rec.version + && lexical_target(dir, entry) == lexical_target(work_dir, rec) + }; let overridden_by_work = |entry: &ReplaceEntry| { work_replaces.iter().any(|w| { w.module == entry.module && (w.version.is_none() || w.version == entry.version) @@ -559,11 +600,7 @@ async fn vendor_modules_txt_issues( let mut issues = Vec::new(); for entry in replaces.iter().filter(|e| e.socket_owned()) { - if !overridden_by_work(entry) - && !recorded - .iter() - .any(|r| same_replacement(entry, r, workspace)) - { + if !overridden_by_work(entry) && !recorded.iter().any(|r| records(project_dir, entry, r)) { issues.push(GoSyncIssue::VendorModulesTxt { module: entry.module.clone(), modules_txt: display.clone(), @@ -574,14 +611,9 @@ async fn vendor_modules_txt_issues( } for rec in recorded .iter() - .filter(|r| r.socket_owned() || ws.as_ref().is_some_and(|w| member_owned(w, r))) + .filter(|r| r.socket_owned() || member_owned(&member_dirs, work_dir, r)) { - if !replaces - .iter() - .chain(&member_replaces) - .chain(&work_replaces) - .any(|e| same_replacement(e, rec, workspace)) - { + if !sources.iter().any(|(dir, e)| records(dir, e, rec)) { issues.push(GoSyncIssue::VendorModulesTxt { module: rec.module.clone(), modules_txt: display.clone(), @@ -593,48 +625,21 @@ async fn vendor_modules_txt_issues( issues } -/// Whether a workspace `modules.txt` replacement is a socket copy of some -/// `use` member. `go work vendor` rewrites a member's directory replace -/// relative to the go.work directory (`./a/.socket/go-patches/…`), which -/// [`go_mod_edit::detect_owner`] alone, anchored at one go.mod's directory, -/// reads as user-authored. Joined lexically, as go joins them. -fn member_owned(ws: &Workspace, recorded: &ReplaceEntry) -> bool { - let Some(path) = recorded.path.as_deref() else { +/// Whether a workspace `modules.txt` replacement points into a `use` +/// member's socket copies. `go work vendor` records a member's directory +/// replace relative to the go.work directory (`./a/.socket/go-patches/…`), +/// which [`go_mod_edit::detect_owner`] alone, anchored at one go.mod's +/// directory, reads as user-authored. +fn member_owned(member_dirs: &[PathBuf], work_dir: &Path, recorded: &ReplaceEntry) -> bool { + if recorded.path.is_none() { return false; - }; - let work_dir = ws.work_file.parent().unwrap_or(Path::new("")); - let abs = PathBuf::from(lexical_target(work_dir, recorded)); - let cwd = std::env::current_dir().unwrap_or_default(); - !Path::new(&path.replace('\\', "/")).is_absolute() - && ws.members.iter().any(|m| { - let dir = crate::utils::relpath::normalize_lexically_keeping_escapes(&cwd.join(&m.dir)); - abs.strip_prefix(&dir) - .is_ok_and(|rel| go_mod_edit::detect_owner(&rel.to_string_lossy()).is_some()) - }) -} - -/// Whether a `modules.txt` replacement records the `go.mod` one. go writes -/// the target verbatim from `go.mod`; a workspace vendor directory may -/// record a module's directory replacement relative to the workspace root, -/// so there the recorded path only has to end with the module's. -fn same_replacement(go_mod: &ReplaceEntry, recorded: &ReplaceEntry, workspace: bool) -> bool { - let norm = |p: &str| { - let p = p.replace('\\', "/"); - p.strip_prefix("./").unwrap_or(&p).to_string() - }; - go_mod.module == recorded.module - && go_mod.version == recorded.version - && match (&go_mod.path, &recorded.path) { - (Some(a), Some(b)) => { - let (a, b) = (norm(a), norm(b)); - a == b || (workspace && b.ends_with(&format!("/{a}"))) - } - (None, None) => { - go_mod.rhs_module == recorded.rhs_module - && go_mod.rhs_version == recorded.rhs_version - } - _ => false, - } + } + let target = PathBuf::from(lexical_target(work_dir, recorded)); + member_dirs.iter().any(|dir| { + target + .strip_prefix(dir) + .is_ok_and(|rel| go_mod_edit::detect_owner(&rel.to_string_lossy()).is_some()) + }) } async fn requirement_issues( @@ -908,6 +913,40 @@ mod tests { ); } + // Rolled back in `a`, applied in `b`, not re-vendored: `b`'s replace + // has the same spelling as `a`'s recorded one but another target, + // so the line is stale and `b`'s is not recorded. + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path().join("ws"); + consumer(&ws.join("a"), ""); + consumer(&ws.join("b"), REPLACE); + write(&ws, "go.work", "go 1.22\n\nuse (\n\t./a\n\t./b\n)\n"); + write( + &ws, + "vendor/modules.txt", + &format!( + "# example.com/upstream v1.0.0 => ./a/{COPY}\n\ + ## explicit; go 1.21\nexample.com/upstream\n" + ), + ); + let mut stale: Vec = audit(&ws.join("b"), &HashMap::new()) + .await + .iter() + .filter_map(|i| match i { + GoSyncIssue::VendorModulesTxt { stale, .. } => Some(*stale), + _ => None, + }) + .collect(); + stale.sort(); + assert_eq!(stale, vec![false, true]); + + // A member whose go.mod can't be read: its recorded line is not + // judged stale from another member, since its replaces are unknown. + std::fs::remove_file(ws.join("a/go.mod")).unwrap(); + std::fs::create_dir(ws.join("a/go.mod")).unwrap(); + consumer(&ws.join("b"), ""); + assert_eq!(audit(&ws.join("b"), &HashMap::new()).await, vec![]); + // A user's replace onto another checkout's copy is not ours, even // when the path has a `.socket/go-patches/` segment. let dir = tempfile::tempdir().unwrap();