Skip to content

Fix Go replace-target patches reported applied (#1823) - #1824

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-go-replace-target-selection
Oct 29, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-go-replace-target-selection

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #1823

Summary

Go projects often swap a dependency for a fork with a replace like replace other.com/fork => example.com/upstream v1.0.0. When a patch targets example.com/upstream@v1.0.0 in that setup, socket-patch reported it applied (apply exit 0, apply --check "in sync", vex not_affected), but go build still linked the unpatched code. This PR makes every Go path refuse the patch or flag it instead.

Root cause

go_build_list::select treats a go.sum zip line for module@version as proof that the build compiles that module. go records a module-to-module replace target's go.sum lines under the target's own path, but the build list holds only the replaced module (other.com/fork). go doesn't chain replaces, so a Socket replace of example.com/upstream does nothing. select still answered Selected for go ≥ 1.17, and for go < 1.17 it read the zip as the built version. All of these callers share that answer: agent apply / apply --check, vendor, scan --mode hosted, CLI vex and VEX lockfile discovery.

Fix

  • select now takes the root go.mod text. It ignores a zip line at the version that an in-effect module-to-module replace targets, and reports a new Selection::ReplaceTarget(via). A replace counts as in effect when go.mod requires the replaced module, any LHS version matches the required version, and no versioned replace shadows a version-less one. go mod tidy keeps unused replaces, so stale ones are skipped. Ignoring the zip is sound because go refuses to build when one module@version is used under two module paths, so that zip line can't come from the target being built in its own right.
  • Callers:
    • agent apply and vendor refuse the patch (selection_refusal)
    • apply --check reports a new Drift::ReplaceTargetOnly
    • CLI vex leaves the patch out (go_version_not_selected)
    • the hosted rewriter refuses with redirect_golang_not_in_module_graph and removes a stale hosted pin it wrote earlier
    • VEX discovery reports a hosted pin of such a module as inert
  • docs/ecosystems.md (Go section) explains the new refusal.

Tests (red → green)

Issue item Test
#1823 apply refuses, apply --check exits 1, vex omits; versioned and version-less fork replace; go 1.21 and go 1.16 e2e_golang_build::module_built_only_as_a_replace_target_is_not_patched (real go toolchain, hermetic file GOPROXY)
selection logic, every replace form / go directive go_build_list::tests::replace_target_zip_is_not_the_module_built, replace_target_at_another_version_keeps_its_own_evidence, inactive_replace_target_keeps_the_zip_as_evidence
agent drift + refusal golang_local::tests::test_replace_target_only_module_is_refused_and_flagged
hosted rewriter redirect::tests::golang_replace_target_only_module_skips
VEX discovery of a hosted pin vex::discover::golang::tests::pin_of_a_module_built_only_as_a_replace_target_is_inert

Red: with the src/ changes stashed, the e2e fails at apply --check must flag the inert replace: left: 0 right: 1. Green: it passes with the fix (go 1.24.7).

Local: cargo fmt --all -- --check ✅, cargo clippy --workspace --all-features -- -D warnings ✅, cargo test --workspace --all-features 15,253 passed / 11 failed. The 11 failures are environmental and outside the Go code: the sandbox runs as root, which defeats the read-only-permission tests (copy_tree, vlt_heal, pypi wire-failure, repair lock, redirect write-failure); it has no route to patches-api; and the bun lockb fixtures depend on the local setup. CI is the reference for those. Every Go suite passed (e2e_golang_build, e2e_golang_workspace_build, e2e_golang_hosted_*, e2e_vendor_golang_build, core Go unit tests).

/code-review high: findings not fixed

  • VEX discovery ignores proven (the socket module's zip line) for a replace target. Not fixed on purpose. With the in-effect check, an active fork replace means the upstream isn't in the build list, so go never applies the Socket replace. The hosted rewriter writes both sum lines and go mod tidy keeps them, so a socket-module zip line there is stale and proves nothing.
  • Workspace mode doesn't report ReplaceTarget. GoWorkspace::selection walks the module graph and already reports such a module as NotInGraph, as the issue confirms. An incomplete graph gives Unknown, which apply --check and vex already flag as UnprovenSelection. Out of scope here.
  • Three different codes across modes (vendor_go_version_not_selected, redirect_golang_not_in_module_graph, go_version_not_selected). These reuse each mode's existing code for "the build won't use this replace", so no new contract codes are added.
  • select gets both the go.mod text and a required value derived from it; per-call parsing of replaces. required stays separate because VEX discovery passes the maximum across workspace files. The extra parse is trivial next to file IO.

Known limitation / follow-up

  • In a go < 1.17 module, if go.mod doesn't list the forked module (it is required only transitively), the fork replace isn't recognized as in effect, and the previous, optimistic answer stands. Modules with go ≥ 1.17 list every module the build uses, so they aren't affected.

https://claude.ai/code/session_011f9aePVb5rbxqLrbfAWspU


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
Add an end-to-end regression for #1823: when go.mod replaces a fork
with the patched module (`replace other.com/fork => upstream v1.0.0`),
go records the upstream's go.sum lines but never applies a replace of
the upstream itself. apply, apply --check and vex all reported the
patch as in effect while go linked the unpatched code.

Assisted-by: Claude Code:claude-opus-5-5
A module the build uses only as the target of another module's
replace (`replace other.com/fork => example.com/upstream v1.0.0`) has
its go.sum zip line under its own path, so the build-list check took
it as built. go doesn't chain replaces, so a Socket replace of it is
silently ignored: apply reported success, apply --check passed and
vex attested not_affected while the build stayed unpatched.

The shared build-list check now ignores a zip line that a
module-to-module replace in go.mod explains (go refuses a build that
uses one module@version under two paths, so it can't be the module's
own) and reports the module as a replace target. apply and vendor
refuse it, apply --check flags it, vex leaves it out, the hosted
rewriter refuses it (redirect_golang_not_in_module_graph) and VEX
discovery treats an existing hosted pin of it as inert.

Fixes #1823

Assisted-by: Claude Code:claude-opus-5-5
go mod tidy keeps unused replace directives. A fork replace of a
module go.mod doesn't require, one pinned to a version other than the
required one, or a version-less one that a versioned replace of the
same module shadows doesn't change what go builds, so it must not
discount the patched module's own go.sum line. Also stop quoting a
version-less directive in the refusal messages, which misquoted a
versioned replace, and cover the agent-mode drift and refusal with a
unit test.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 29, 2026 11:14
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 29, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at 4b7a58ffb417e050c1d2d69c7b3695770af235c2.

  • CI: 82/82 settled on this head (32 success, 50 skipped by path filters); no failures.
  • Bugbot: reviewed this head (Cursor Bugbot check success), no findings; no open review threads.
  • Reviewer focus: go_build_list::select now ignores a go.sum zip line for the target of an in-effect module-to-module replace and returns Selection::ReplaceTarget; check the "in effect" rules (required LHS, version match, versioned replace shadowing a version-less one) and the per-mode refusal codes listed in the description.
  • Slack announcement not sent this run (Slack send tool unavailable); the next run will retry.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. When go.mod swaps a dependency for a fork with replace other.com/fork => example.com/upstream v1.0.0, go.sum holds the upstream's zip line, and socket-patch read that as "the build compiles upstream@v1.0.0", so a patch of upstream was reported applied while go build linked unpatched code (go doesn't chain replaces). Now go_build_list::select ignores that zip line when an in-effect module-to-module replace targets the exact version and returns a new Selection::ReplaceTarget(via); agent apply/vendor refuse, apply --check flags drift, vex omits it, and the hosted rewriter refuses and drops a stale hosted pin.

Risk: low–medium. Changes the shared Go selection logic used by apply, vendor, scan, vex and VEX discovery, but only for the narrow case of an exact-version replace target, and the new answer is always the safer one (refuse/flag rather than claim patched).

Look here

  • go_build_list.rs:146 replaced_module: the "in effect" rules (replaced module is required; versioned LHS must equal the required version; a versioned replace shadows a version-less one).
  • go_build_list.rs:95 select: zip filtering, and ReplaceTarget only when no other evidence remains (an explicit require of the module still wins).
  • redirect/mod.rs:9638: hosted rewriter now treats a replace target like a version mismatch, including stale-pin removal.
  • vex/discover/golang.rs:474: VEX discovery marks such a pin inert before the proven check.
  • golang_local.rs:709:`` apply/vendor refusal text.

Verified

  • Read the full diff (8 files, +498/−35); the PR description matches the code; CHANGELOG.md untouched; no debug leftovers.
  • Ran locally on 4b7a58f: cargo test -p socket-patch-core --lib go_build_list (10 passed) and the new replace_target* tests (7 passed). Tests cover go 1.21 and 1.16, versioned/version-less/block replaces, inactive replaces, and the e2e with a real go toolchain (red without the fix per the description).
  • CI 82/82 settled on this head (32 success, 50 path-skipped), ci-ok and clippy green; Bugbot reviewed this head with no findings; no open threads.

Changes I made: none.

Open questions (non-blocking)

  • replaced_module only consults the root go.mod's require list. In a go < 1.17 module where the forked module is required only transitively (not listed in the root go.mod), the replace isn't recognised as in effect and the old (optimistic) answer stands. With go ≥ 1.17 every dependency is listed, so this only affects old go directives; fine to leave as a follow-up.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

3 participants