Skip to content

Fix Go stale go.sum line after a downgrade (#1862) - #1893

Open
Mikola Lysenko (mikolalysenko) wants to merge 10 commits into
mainfrom
agent/fix-go-unpruned-stale-zip
Open

Mikola Lysenko (mikolalysenko) wants to merge 10 commits into
mainfrom
agent/fix-go-unpruned-stale-zip

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #1862

Stacked on #1843. This branch starts from agent/fix-go-unpruned-replace-target (plus a merge of main), because the fix reuses the module-mode ModuleGraph reader #1843 adds. Until #1843 merges, the diff includes its commits; review the commits after Start fix for #1862.

Problem

In a go 1.16-or-older module, go get M@<lower> lowers go.mod's require but keeps the old version's go.sum lines until go mod tidy. go_build_list::select took the highest go.sum zip line as the version the build selects, so after a downgrade:

  1. Below the patched version: apply wrote an unused replace and reported applied, apply --check passed, and vex attested not_affected, while the build linked the unpatched module.
  2. To the patched version: apply refused a patch for the version the build really uses.

Root cause

A go.sum zip line above go.mod's own require is either a dependency's raise or a line a downgrade left behind. go.mod and go.sum alone can't tell those apart, and the unpruned branch of select treated every such line as a raise.

Fix

  • go_build_list: when go.mod requires a module below one of its go.sum zip lines, ask the module graph read from the module cache (Fix Go replace target missed in go 1.16 graphs (#1840) #1843's ModuleGraph, i.e. MVS over every go.mod the build reads). needs_module_graph / may_need_module_graph now take go.sum and also trigger on this shape.
  • No complete graph (the cache lacks a needed go.mod): new Selection::Unproven, which fails closed.
    • apply refuses and writes nothing.
    • apply --check reports a new UnprovenStaleSum drift.
    • vex leaves the patch out.
    • The hosted rewriter writes no pin.
    • VEX discovery won't attest a hosted ref.
    • All of them share one explanation (UNPROVEN_REASON) that names go mod download.
  • VEX discovery: a Socket module's own go.sum zip line no longer outranks a complete module graph that shows the build picks another version, because that line can be just as stale.
  • Patch below require: when the patched version is below go.mod's require (so it is never selected), the reported version is the require, or the graph's answer, instead of a possibly stale go.sum line.

Trade-off (deliberate)

Without a module cache, a go 1.16 module that a dependency legitimately raises past go.mod's require is now Unproven, not Selected/Other:

  • On a fresh clone that never ran go mod download, apply --check flags it.
  • The in-memory hosted engine, which has no cache, won't pin it.

Before this change the same files were read as proof, and that is exactly what produced the false not_affected attestation in #1862. Any machine that has built the project has the cache and gets the exact MVS answer.

Tests (red → green)

Issue case Test Without fix With fix
#1862 case 1 (unit) go_build_list::tests::stale_zip_line_above_require_does_not_prove_the_patched_version FAILED ok
#1862 case 2 (unit) go_build_list::tests::stale_zip_line_above_require_does_not_refuse_the_required_version FAILED ok
#1862 case 1 (real go 1.24.7) e2e_golang_build::downgrade_below_the_patch_leaves_its_replace_unattested: apply refuses; an earlier replace fails apply --check with and without the cache; vex omits it FAILED ok
#1862 case 2 (real go) e2e_golang_build::downgrade_to_the_patch_applies_and_checks_in_sync: apply patches, build prints PATCHED, check passes FAILED ok
hosted VEX stale socket line vex::discover::golang::tests::module_graph_outranks_a_stale_socket_zip_line FAILED ok

Also added module_graph_confirms_a_dependency_raise (the graph tells a raise from a stale line). unpruned_require_is_a_lower_bound now expects Unproven for the no-graph raise (the trade-off above).

Commands run locally:

  • cargo fmt --all -- --check: clean.
  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • SOCKET_PATCH_GO_E2E_REQUIRED=1 cargo test -p socket-patch-cli --all-features --test e2e_golang_build: 35/35 pass.
  • cargo test --workspace --all-features: 8043 pass. 11 fail, all permission-denial tests that can't fail when run as root, which this container runs as (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal/pypi_* write-failure tests, in_process_redirect write failures, repair unremovable lock, e2e_bun_lockb x2). None touch Go code; CI runs non-root.

No npm/pypi/gem wrapper changes: this is all in the Rust core.

/code-review high

Fixed: the discovery stale socket zip line, the stale version reported for a patch below require, the "go.sum shows" wording when the graph answered, the duplicated message, and the double go.sum parse.

Not fixed:

  • No-cache Unproven for legitimate raises: deliberate (see Trade-off).
  • One graph read per patched purl in project_selection, and may_need_module_graph triggering on any raised module: both are offline go.mod reads from the cache, and the per-call read is Fix Go replace target missed in go 1.16 graphs (#1840) #1843's design. A per-project memo is a follow-up if it shows up in benches.

Follow-up (not in this PR)

Hosted-mode VEX discovery when the downgrade leaves no zip line above require but a stale Socket module zip line, and there is no graph: proven still trusts that socket line.

Taken ahead of the open p1 issues because those are all refactors that change no behavior, while this one is a false VEX not_affected attestation.

https://claude.ai/code/session_011f9aeSFJwUPaqWXN5AA5kk


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
In a go 1.16 module, when a dependency imports a fork that the root
go.mod replaces with the patched upstream module, apply reported the
patch applied, apply --check said in sync and vex attested
not_affected, while go build linked the unpatched code.

The root go.mod of an unpruned graph does not list modules that only
a dependency requires, but its replace still applies to them. The
replace-target check only trusted a fork listed in the root require,
so it missed this shape. Now, in an unpruned graph, such a replace
counts as in effect unless go.mod requires the patched module itself.

Fixes #1840

Assisted-by: Claude Code:claude-opus-5-5
The first fix guessed that a fork replace in a go 1.16 module is in
effect whenever go.mod doesn't list the fork. That refused working
patches when go mod tidy kept a replace nothing uses, and the hosted
rewriter then removed a working Socket replace. It also missed a fork
a dependency raises to the replaced version.

Now, when an unpruned go.mod has a module-to-module replace, the
module graph is read from the module cache, the same walk go.work
projects use, and the fork's selected version decides. apply,
apply --check, the hosted rewriter and VEX discovery all use it. When
the cache lacks go.mod files, the safe answer stays: the patch is not
reported built.

Refs #1840

Assisted-by: Claude Code:claude-opus-5-5
Reading the go 1.16 module graph went around the project view, which
ended read recording for every project: VEX discovery asked for the
raw disk root even with no Go files, so discovery could no longer be
reused between passes. Now the graph is read only when go.mod needs
it, and the root go.mod and any directory replacement's go.mod are
declared as reads, so a change to them still invalidates the cache.

Refs #1840

Assisted-by: Claude Code:claude-opus-5-5
Without every go.mod of a go 1.16 module graph in the module cache,
a fork replace is only suspected of bypassing the Socket pin. That
guess now refuses a new pin but no longer removes an existing one
that may be working. A partial graph is no longer trusted as proof
that the fork is at a lower version. Socket's own hosted replaces no
longer trigger a graph walk.

Refs #1840

Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
In a go 1.16-or-older module, `go get M@<lower>` lowers go.mod's
require but keeps the old version's go.sum lines until `go mod tidy`.
socket-patch took the highest go.sum zip line as the version the build
selects, so after a downgrade apply wrote an unused replace that
apply --check and vex then reported as patched, or refused a patch for
the version the build really uses (#1862).

When go.mod requires a module below one of its go.sum zip lines, read
the module graph from the module cache and use the version MVS picks.
When the cache can't answer, fail closed: apply refuses with a remedy,
apply --check reports drift, and vex and the hosted rewriter don't
attest or pin the patch.

Assisted-by: Claude Code:claude-opus-5-5
End-to-end with a real go toolchain, both shapes from #1862: a
downgrade below the patched version (apply refuses, an earlier replace
fails apply --check with and without the module cache, and vex leaves
it out), and a downgrade to the patched version (apply patches it and
the build links the patch).

Assisted-by: Claude Code:claude-opus-5-5
Follow-ups from review of the #1862 fix:

- VEX discovery let a Socket module's own go.sum zip line vouch for a
  hosted replace even when a full module graph showed the build picks
  another version. That line can be just as stale, so the graph now
  wins.
- A patch below go.mod's require reported a leftover go.sum version as
  the one the build selects; it now reports go.mod's require, or the
  graph's answer when the graph is read.
- The hosted warning says "the module graph selects" when that is
  where the answer came from.
- The "unproven" explanation is written once and shared by apply,
  apply --check, vex and the hosted rewriter.

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

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 9577376. Configure here.

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

Labels

None yet

Projects

None yet

2 participants