From 6e52befac5f839ad9cd43a7f18ba167799cb4406 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 22:22:28 +0000 Subject: [PATCH 1/4] Start fix for #1751 Assisted-by: Claude Code:claude-opus-5-5 From 3d994be43fa79a3feeb3b515dbca9b8f0bc016c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 22:35:10 +0000 Subject: [PATCH 2/4] Check other go.work members before Go patching In a go.work workspace Go builds the highest version any member requires. The version check behind apply, apply --check, vendor and vex read only the current module's go.mod, so when another member required a newer version, apply wrote a replace the build ignored, apply --check said "in sync" and vex attested not_affected for a module the workspace still built unpatched. The check now reads every `use` member's go.mod and go.sum and treats a higher version anywhere in the workspace as the one built. apply and vendor refuse, apply --check fails and vex leaves the patch out (go_version_not_selected). Fixes #1751. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_golang_build.rs | 94 ++++++++++++ .../src/vendor/go_build_list.rs | 139 ++++++++++++++++-- .../src/vendor/go_consumer_sync.rs | 15 ++ 3 files changed, 233 insertions(+), 15 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..aff3ef6d8 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -983,6 +983,100 @@ fn go_work_member_replaces_must_not_conflict() { chmod_writable(tmp.path()); } +/// #1751: a go.work build list spans every `use` member's requirements, so +/// when member b requires a newer upstream than a, the workspace builds a +/// with b's version and a replace pinned to a's is inert. apply in a must +/// refuse, and a replace written before the go.work appeared fails +/// `apply --check` and is left out of VEX (`go_version_not_selected`). +#[test] +fn go_work_member_requiring_a_newer_version_makes_the_replace_inert() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(go-work-newer-member)") { + return; + } + const NEWER: &str = "v1.1.0"; + let tmp = tempfile::tempdir().unwrap(); + let (_consumer, modcache, proxy_url) = stage(tmp.path()); + let mc = modcache.to_str().unwrap(); + publish( + tmp.path(), + UMOD, + NEWER, + &[ + ("go.mod", &format!("module {UMOD}\n\ngo 1.21\n")), + ( + "lib.go", + "package upstream\n\nfunc Greeting() string { return \"PRISTINE-v1.1.0\" }\n", + ), + ], + ); + let ws = tmp.path().join("ws"); + let (a, b) = (ws.join("a"), ws.join("b")); + consumer_module(&a, "example.com/a", mc, &proxy_url); + consumer_module(&b, "example.com/b", mc, &proxy_url); + let go_mod = std::fs::read_to_string(b.join("go.mod")).unwrap(); + std::fs::write( + b.join("go.mod"), + go_mod.replace(&format!("{UMOD} {UVER}"), &format!("{UMOD} {NEWER}")), + ) + .unwrap(); + let mut tidy_env = go_env(mc, &proxy_url); + tidy_env.push(("GOWORK", "off")); + let tidy = go(&b, &["mod", "tidy"], &tidy_env); + assert!( + tidy.status.success(), + "go mod tidy: {}", + String::from_utf8_lossy(&tidy.stderr) + ); + write_patch(&a); + let go_work = "go 1.21\n\nuse (\n\t./a\n\t./b\n)\n"; + std::fs::write(ws.join("go.work"), go_work).unwrap(); + let env = workspace_env(mc, &proxy_url); + assert!(go_out(&a, &env).contains("OUT: PRISTINE-v1.1.0")); + + let (code, so, se) = apply_json(&a, &modcache); + assert_ne!(code, 0, "apply must refuse an inert replace:\n{so}\n{se}"); + assert!( + so.contains(&format!("the build selects {UMOD} {NEWER}")), + "{so}" + ); + assert!(replace_lines(&a.join("go.mod")).is_empty()); + assert!(go_out(&a, &env).contains("OUT: PRISTINE-v1.1.0")); + + // Applied before the go.work existed: the replace is inert. + std::fs::remove_file(ws.join("go.work")).unwrap(); + let (code, so, se) = apply_json(&a, &modcache); + assert_eq!(code, 0, "apply without go.work:\n{so}\n{se}"); + assert!(go_out(&a, &tidy_env).contains("OUT: PATCHED")); + std::fs::write(ws.join("go.work"), go_work).unwrap(); + assert!(go_out(&a, &env).contains("OUT: PRISTINE-v1.1.0")); + let (code, so, se) = check(&a, &modcache); + assert_eq!( + code, 1, + "apply --check must flag the inert replace:\n{so}\n{se}" + ); + let r = VexRun { + offline: true, + product: Some("pkg:golang/example.com/a@v0.0.1".into()), + ..VexRun::default() + } + .env("GOMODCACHE", &modcache) + .env("GOFLAGS", ""); + let out = run_vex(&vex_e2e_common::binary(), &a, &r); + assert_absent(out.doc.as_ref(), UPURL); + vex_e2e_common::assert_not_attested(&out.envelope, UPURL, "go_version_not_selected"); + + // Positive control: once b is back on a's version, the replace patches + // the whole workspace and nothing is flagged. + std::fs::write(b.join("go.mod"), &go_mod).unwrap(); + let tidy = go(&b, &["mod", "tidy"], &tidy_env); + assert!(tidy.status.success()); + assert!(go_out(&b, &env).contains("OUT: PATCHED")); + let (code, so, se) = check(&a, &modcache); + assert_eq!(code, 0, "{so}\n{se}"); + + chmod_writable(tmp.path()); +} + // ── global mode: tools already built with `go install` (#422) ── const TOOL: &str = "example.com/tool"; diff --git a/crates/socket-patch-core/src/vendor/go_build_list.rs b/crates/socket-patch-core/src/vendor/go_build_list.rs index 45a8f881e..4aee4a4f3 100644 --- a/crates/socket-patch-core/src/vendor/go_build_list.rs +++ b/crates/socket-patch-core/src/vendor/go_build_list.rs @@ -106,26 +106,64 @@ pub fn select(pruned: bool, required: Option<&str>, zips: &[&str], version: &str } /// [`selection`] for a project directory: reads `/go.mod` and -/// `go.sum` (`None` when there is no readable go.mod). In workspace mode the -/// build list also takes other members' requirements, so a module absent -/// from this go.mod may still be built: [`Selection::NotInGraph`] becomes -/// [`Selection::Unknown`] there. +/// `go.sum` (`None` when there is no readable go.mod). +/// +/// In workspace mode the build list also takes every `use` member's +/// requirements, and MVS selects the highest version any of them asks for +/// (#1751). So another member's higher version is the one built, and a +/// module absent from this go.mod may still be built: +/// [`Selection::NotInGraph`] becomes [`Selection::Unknown`] there unless a +/// member proves the version. pub async fn project_selection( project_root: &Path, module: &str, version: &str, ) -> Option { - let go_mod = read_regular_to_string(&project_root.join("go.mod")) - .await - .ok()?; - let go_sum = read_regular_to_string(&project_root.join("go.sum")) - .await - .ok(); - let sel = selection(&go_mod, go_sum.as_deref(), module, version); - if sel == Selection::NotInGraph && super::go_consumer_sync::workspace_in_effect(project_root) { - return Some(Selection::Unknown); - } - Some(sel) + let own = dir_selection(project_root, module, version).await?; + let Some(members) = super::go_consumer_sync::workspace_member_dirs(project_root).await else { + if own == Selection::NotInGraph + && super::go_consumer_sync::workspace_in_effect(project_root) + { + return Some(Selection::Unknown); + } + return Some(own); + }; + let mut highest = evidence(&own, version); + for dir in members { + let Some(member) = dir_selection(&dir, module, version).await else { + continue; + }; + if let Some(v) = evidence(&member, version) { + if highest + .as_deref() + .is_none_or(|h| go_semver_cmp(&v, h) == Ordering::Greater) + { + highest = Some(v); + } + } + } + Some(match highest { + Some(h) if go_semver_cmp(&h, version) == Ordering::Greater => Selection::Other(h), + Some(h) if h == version => Selection::Selected, + _ if own == Selection::NotInGraph => Selection::Unknown, + _ => own, + }) +} + +/// [`selection`] from `/go.mod` and `go.sum`, ignoring any workspace. +async fn dir_selection(dir: &Path, module: &str, version: &str) -> Option { + let go_mod = read_regular_to_string(&dir.join("go.mod")).await.ok()?; + let go_sum = read_regular_to_string(&dir.join("go.sum")).await.ok(); + Some(selection(&go_mod, go_sum.as_deref(), module, version)) +} + +/// The version a module's own go.mod and go.sum prove it builds, if any. +fn evidence(sel: &Selection, version: &str) -> Option { + match sel { + Selection::Selected => Some(version.to_string()), + Selection::Other(v) => Some(v.clone()), + Selection::NotInGraph | Selection::Unknown => None, + } } #[cfg(test)] @@ -226,6 +264,77 @@ mod tests { assert!(!prunes_module_graph("module c\ngodebug default=go1.21\n")); } + fn module(dir: &Path, name: &str, version: &str) { + std::fs::create_dir_all(dir).unwrap(); + let m = format!("module example.com/{name}\n\ngo 1.21\n\nrequire {UP} {version}\n"); + std::fs::write(dir.join("go.mod"), m).unwrap(); + let s = sum(&[ + &format!("{UP} {version}"), + &format!("{UP} {version}/go.mod"), + ]); + std::fs::write(dir.join("go.sum"), s).unwrap(); + } + + /// #1751: a go.work build list takes every `use` member's requirements, + /// so another member's higher version is the one built, and a replace + /// pinned to this member's lower version is inert. + #[tokio::test] + async fn workspace_member_requiring_a_higher_version_wins() { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path(); + let (a, b) = (ws.join("a"), ws.join("b")); + module(&a, "a", "v1.0.0"); + module(&b, "b", "v1.1.0"); + assert_eq!( + project_selection(&a, UP, "v1.0.0").await, + Some(Selection::Selected), + "outside a workspace only a's go.mod counts" + ); + std::fs::write(ws.join("go.work"), "go 1.21\n\nuse (\n\t./a\n\t./b\n)\n").unwrap(); + assert_eq!( + project_selection(&a, UP, "v1.0.0").await, + Some(Selection::Other("v1.1.0".into())) + ); + // b's own version is the workspace's. + assert_eq!( + project_selection(&b, UP, "v1.1.0").await, + Some(Selection::Selected) + ); + // A member that doesn't require the module is built at the version + // another member selects. + std::fs::write(a.join("go.mod"), "module example.com/a\n\ngo 1.21\n").unwrap(); + assert_eq!( + project_selection(&a, UP, "v1.0.0").await, + Some(Selection::Other("v1.1.0".into())) + ); + assert_eq!( + project_selection(&a, UP, "v1.1.0").await, + Some(Selection::Selected) + ); + } + + /// A member requiring a lower version doesn't move the workspace off + /// this member's higher one, and a member go.work doesn't `use` counts + /// for nothing. + #[tokio::test] + async fn workspace_lower_or_unused_members_do_not_change_the_selection() { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path(); + let (a, b, c) = (ws.join("a"), ws.join("b"), ws.join("c")); + module(&a, "a", "v1.1.0"); + module(&b, "b", "v1.0.0"); + module(&c, "c", "v1.2.0"); + std::fs::write(ws.join("go.work"), "go 1.21\n\nuse ./a\nuse ./b\n").unwrap(); + assert_eq!( + project_selection(&a, UP, "v1.1.0").await, + Some(Selection::Selected) + ); + assert_eq!( + project_selection(&b, UP, "v1.0.0").await, + Some(Selection::Other("v1.1.0".into())) + ); + } + #[test] fn incompatible_versions_compare_exactly() { let m = format!("module c\n\ngo 1.21\n\nrequire {UP} v2.0.0+incompatible\n"); 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..10d46d8d8 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -305,6 +305,21 @@ fn workspace_file(project_root: &Path) -> Option { .find(|file| file.is_file()) } +/// The `use` member directories of the `go.work` in effect for +/// `project_root` (joined to go.work's directory), or `None` when no +/// workspace is in effect or its go.work can't be read. +pub(crate) async fn workspace_member_dirs(project_root: &Path) -> Option> { + let work_file = workspace_file(project_root)?; + let text = read_regular_to_string(&work_file).await.ok()?; + let work_dir = work_file.parent().unwrap_or(Path::new("")); + Some( + go_mod_edit::parse_use_dirs(&go_mod_edit::normalize_for_read(&text)) + .into_iter() + .map(|dir| work_dir.join(dir)) + .collect(), + ) +} + /// The `go.work` in effect, read once: its own replaces and each `use` /// member's go.mod replaces. struct Workspace { From 10f5e26313eac26007240024efda750a3a4e61b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 22:41:24 +0000 Subject: [PATCH 3/4] Weigh lower workspace versions in the Go check When every go.work member proves a version below the patched one and none is undecided, the workspace builds the highest of them, so the refusal now names that version instead of this module's own or "unknown". Reading go.work's members is shared with the conflict check, and the Go workspace docs say which commands read the other members. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/go_build_list.rs | 56 ++++++++++++++++--- .../src/vendor/go_consumer_sync.rs | 37 +++++++----- docs/ecosystems.md | 6 +- 3 files changed, 76 insertions(+), 23 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/go_build_list.rs b/crates/socket-patch-core/src/vendor/go_build_list.rs index 4aee4a4f3..fce0b9f43 100644 --- a/crates/socket-patch-core/src/vendor/go_build_list.rs +++ b/crates/socket-patch-core/src/vendor/go_build_list.rs @@ -110,8 +110,8 @@ pub fn select(pruned: bool, required: Option<&str>, zips: &[&str], version: &str /// /// In workspace mode the build list also takes every `use` member's /// requirements, and MVS selects the highest version any of them asks for -/// (#1751). So another member's higher version is the one built, and a -/// module absent from this go.mod may still be built: +/// (#1751). So a version another member proves is weighed with this one's, +/// and a module absent from this go.mod may still be built: /// [`Selection::NotInGraph`] becomes [`Selection::Unknown`] there unless a /// member proves the version. pub async fn project_selection( @@ -121,18 +121,17 @@ pub async fn project_selection( ) -> Option { let own = dir_selection(project_root, module, version).await?; let Some(members) = super::go_consumer_sync::workspace_member_dirs(project_root).await else { - if own == Selection::NotInGraph - && super::go_consumer_sync::workspace_in_effect(project_root) - { - return Some(Selection::Unknown); - } return Some(own); }; let mut highest = evidence(&own, version); + // A member whose files can't tell may build a higher version than the + // highest one proven, so then only a proof at or above ours decides. + let mut unknown = own == Selection::Unknown; for dir in members { let Some(member) = dir_selection(&dir, module, version).await else { continue; }; + unknown |= member == Selection::Unknown; if let Some(v) = evidence(&member, version) { if highest .as_deref() @@ -143,8 +142,10 @@ pub async fn project_selection( } } Some(match highest { - Some(h) if go_semver_cmp(&h, version) == Ordering::Greater => Selection::Other(h), Some(h) if h == version => Selection::Selected, + Some(h) if !unknown || go_semver_cmp(&h, version) == Ordering::Greater => { + Selection::Other(h) + } _ if own == Selection::NotInGraph => Selection::Unknown, _ => own, }) @@ -313,6 +314,45 @@ mod tests { ); } + /// A version below the patched one counts when it is the highest any + /// member proves, unless a member's files can't tell. + #[tokio::test] + async fn workspace_lower_versions_decide_only_without_unknown_members() { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path(); + let (a, b, c) = (ws.join("a"), ws.join("b"), ws.join("c")); + module(&a, "a", "v1.0.0"); + module(&b, "b", "v1.0.5"); + std::fs::create_dir_all(&c).unwrap(); + std::fs::write(c.join("go.mod"), "module example.com/c\n\ngo 1.21\n").unwrap(); + std::fs::write( + ws.join("go.work"), + "go 1.21\n\nuse (\n\t./a\n\t./b\n\t./c\n)\n", + ) + .unwrap(); + for project in [&a, &c] { + assert_eq!( + project_selection(project, UP, "v1.1.0").await, + Some(Selection::Other("v1.0.5".into())) + ); + } + // An old-style member with only a lower bound may build anything. + std::fs::write( + b.join("go.mod"), + format!("module example.com/b\n\ngo 1.16\n\nrequire {UP} v1.0.5\n"), + ) + .unwrap(); + std::fs::write(b.join("go.sum"), "").unwrap(); + assert_eq!( + project_selection(&c, UP, "v1.1.0").await, + Some(Selection::Unknown) + ); + assert_eq!( + project_selection(&a, UP, "v1.1.0").await, + Some(Selection::Other("v1.0.0".into())) + ); + } + /// A member requiring a lower version doesn't move the workspace off /// this member's higher one, and a member go.work doesn't `use` counts /// for nothing. 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 10d46d8d8..c7978c2ae 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -306,18 +306,30 @@ fn workspace_file(project_root: &Path) -> Option { } /// The `use` member directories of the `go.work` in effect for -/// `project_root` (joined to go.work's directory), or `None` when no -/// workspace is in effect or its go.work can't be read. +/// `project_root`: `None` when no workspace is in effect, empty when its +/// go.work can't be read. pub(crate) async fn workspace_member_dirs(project_root: &Path) -> Option> { let work_file = workspace_file(project_root)?; - let text = read_regular_to_string(&work_file).await.ok()?; + Some(match read_work_file(&work_file).await { + Some(text) => use_dirs(&work_file, &text), + None => Vec::new(), + }) +} + +/// `work_file`'s text, normalized for parsing. +async fn read_work_file(work_file: &Path) -> Option { + let text = read_regular_to_string(work_file).await.ok()?; + Some(go_mod_edit::normalize_for_read(&text)) +} + +/// The `use` directories of go.work `text`, joined to `work_file`'s +/// directory. +fn use_dirs(work_file: &Path, text: &str) -> Vec { let work_dir = work_file.parent().unwrap_or(Path::new("")); - Some( - go_mod_edit::parse_use_dirs(&go_mod_edit::normalize_for_read(&text)) - .into_iter() - .map(|dir| work_dir.join(dir)) - .collect(), - ) + go_mod_edit::parse_use_dirs(text) + .into_iter() + .map(|dir| work_dir.join(dir)) + .collect() } /// The `go.work` in effect, read once: its own replaces and each `use` @@ -337,12 +349,9 @@ struct Member { async fn load_workspace(project_root: &Path) -> Option { let work_file = workspace_file(project_root)?; - let text = read_regular_to_string(&work_file).await.ok()?; - let text = go_mod_edit::normalize_for_read(&text); - let work_dir = work_file.parent().unwrap_or(Path::new("")).to_path_buf(); + let text = read_work_file(&work_file).await?; let mut members = Vec::new(); - for dir in go_mod_edit::parse_use_dirs(&text) { - let dir = work_dir.join(dir); + for dir in use_dirs(&work_file, &text) { 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)) diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 62b0b8556..b840a0058 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -1513,7 +1513,11 @@ and `go.sum`: the `require` line in a go 1.17+ module, or, in an older module (whose `require` lists only lower bounds and leaves out transitive modules), the highest of the required version and the versions `go.sum` has a module zip line for. `apply` and `scan --mode hosted` refuse to write a replace for -any other version. After an upgrade such as `go get M@v1.0.1`, `apply --check` +any other version. In a workspace the build takes the highest version any +`use` member selects, so `apply`, `vendor`, `apply --check` and `vex` read +every member's `go.mod` and `go.sum` too: a member that requires a newer +version makes a replace pinned to the older one inert. `scan --mode hosted` +still reads only the module it runs in (#1753). After an upgrade such as `go get M@v1.0.1`, `apply --check` fails and `vex` leaves the patch out (`go_version_not_selected`) until you get a patch for the new version. From ac7071b5ab761cc8e6897d6fb89be1c52f1f3ff6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 22:46:00 +0000 Subject: [PATCH 4/4] Drop the unused Go workspace_in_effect helper The workspace member list now answers whether a go.work is in effect, so the separate check has no callers left. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vendor/go_consumer_sync.rs | 7 ------- 1 file changed, 7 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 c7978c2ae..b2b7e3c37 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -280,13 +280,6 @@ fn modules_txt_location(project_root: &Path) -> (PathBuf, &'static str) { ) } -/// Whether the go command builds `project_root` in workspace mode (a -/// `go.work` from `GOWORK` or at or above the project), so its build list -/// spans every member's requirements, not only the project go.mod's. -pub(crate) fn workspace_in_effect(project_root: &Path) -> bool { - workspace_file(project_root).is_some() -} - /// The `go.work` go would use for `project_root`: `GOWORK` (resolved as go /// resolves it: the environment, else the `go env -w` file) when it names a /// file (`off` disables workspaces), else the nearest `go.work` at or above