Repository navigation
Fix Go patches ignoring GOFLAGS=-modfile (#1799) - #1800
Conversation
Assisted-by: Claude Code:claude-opus-5-5
With -modfile in GOFLAGS (set in the environment or with go env -w), go builds from that file and ignores go.mod's replaces. socket-patch never read GOFLAGS, so apply wrote a go.mod replace and reported the patch applied, apply --check called it in sync, and vex attested not_affected while the build linked the unpatched module. GOFLAGS is now read the way go reads it. A -modfile naming any file other than the project's go.mod (or any -modfile in workspace mode, which go refuses) is treated like a go.work that overrides the replace: apply, vendor and scan --mode hosted write nothing, a replace already written fails apply --check and vendor --check, and vex attests none of the project's Go patches. Fixes #1799. Assisted-by: Claude Code:claude-opus-5-5
Review fixes for the -modfile gate: - A GOFLAGS that go refuses (a non-flag field, a bare -modfile, an unterminated quote) is reported as such, and a quoted field may run straight into the next one, as in go's own parser. - VEX discovery only looks at the project root when GOFLAGS names -modfile, so hosted passes keep their read-tracking, and it skips in-memory projects, as the hosted engine does. - A relative GOWORK is go's own refusal, not workspace mode, so it no longer hides behind a -modfile message. - apply --check names go.work, not go.mod, for a go.work replace. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
The VEX -modfile check took the project root through a raw read whenever GOFLAGS named -modfile, even -modfile=go.mod, which voided a tracked snapshot's recording for every hosted writing pass. The check now declares the files it reads (the go.work candidates, go.mod and the named file), so the recording survives. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 801d50f. Configure here.
|
Burn-down agent: labeled Ready for review at
Generated by Claude Code |
Final review briefWhat it does. When Risk: low. The PR only adds refusals and never adds writes. Every gate does nothing unless Look here
Verified
Changes I made. None. Notes (non-blocking).
Auto-merge (squash) is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1799
Summary
If
GOFLAGScontains-modfile=<file>, go builds from<file>and its.sumand ignores the replaces ingo.mod. This works whetherGOFLAGSis set in the environment or withgo env -w. Before this change, socket-patch still wrote its replace intogo.mod,applyreported the patch applied,apply --checksaid it was in sync, andvexattestednot_affected. The build linked the unpatched module. Now socket-patch refuses to write the replace and refuses to attest it, the same way it already handles ago.workthat overrides the replace.Root cause
socket-patch never read
GOFLAGS. Every Go writer (agentapply,vendor,scan --mode hosted) editedgo.mod/go.sum, and both--checkand VEX discovery read only those files.Fix
utils::go_env::goflags_modfilereads-modfilefromGOFLAGSthe way go'sInitGOFLAGSdoes:quoted.Splitsplits them;-flagor--flag;-modfilewins, and-modfile=means go's default.-modfile, or an unterminated quote.go_workspace::modfile_setting/modfile_conflictreturn a newWorkspaceConflict::ModFilein these cases:-modfilenames anything other than the project'sgo.mod. A relative path is taken from the project directory.go.workis in effect, where go refuses any-modfile.GOFLAGSitself is one go refuses.go.workoverride. No new codes:go_consumer_sync::replace_refusal: agentapplyandvendorwrite nothing.go_consumer_sync::audit/workspace_audit:apply --checkandvendor --checkfail withgo_workspace_replace_ineffective, andvexomits the patch withgo_workspace_override. This covers go.mod replaces and root go.work replaces. The message namesgo.workfor the latter.patched_ref_invalid, as it does for a foreignGOWORK. The probe runs only whenGOFLAGSsets-modfile(or is one go refuses). It declares the files it reads throughdisk_root_reading, so a tracked hosted pass keeps its read recording.HostedGoWorkspace.modfile. The rewriter then refuses withredirect_golang_workspace_conflict.npm/,pypi/andgem/only dispatch to the binary.Tests (red → green)
applyrefuse and write nothing; a replace written earlier fails--checkand is omitted fromvexwith-modfilefrom the env or aGOENVfile;-modfile=go.modstays in synce2e_golang_build::goflags_modfile_keeps_the_go_mod_replace_from_the_buildapplyexits 0)--, refused settingsgo_env::goflags_modfile_reads_the_last_modfile./) vs other file; workspace mode; relative GOWORKgo_workspace::goflags_modfile_conflicts_unless_it_names_the_own_go_moddiscover::golang::goflags_modfile_naming_another_go_mod_attests_nothing-modfile(Bugbot)discover::golang::goflags_modfile_probe_keeps_a_snapshot_recordingredirect::golang_goflags_modfile_refuses_the_pinPer-issue checklist:
apply(and its dry run),apply --check,vex(env andgo env -w), all in the e2e test above. The vendored and hosted writers go through the same gate, covered byreplace_refusalandgolang_goflags_modfile_refuses_the_pin.Local runs (0b99e93, then re-run at 801d50f):
cargo fmt --all -- --checkandcargo clippy --workspace --all-features -- -D warnings: clean.cargo test -p socket-patch-core --all-features --lib: 6928 passed. 4 failed, all chmod-based write-failure tests that can't fail as root in this container:copy_tree::relax_loop…,vlt_heal::an_unremovable…,pypi_poetry::wire_write_failure…andpypi_requirements::wire_failure…. None of them touch Go.hosted::engine::tests::a_writing_pass_hands_back_the_discovery_of_its_writespasses.socket-patch-coreintegration suites: 32 suites, 656 passed.SOCKET_PATCH_GO_E2E_REQUIRED=1 --include-ignored(go 1.24.7):e2e_golang_build30,e2e_golang_workspace_build28,e2e_golang_hosted_build25,e2e_golang_hosted_state30,e2e_vendor_golang_build44,e2e_golang23. All pass.e2e_vex,e2e_vex_redirect,e2e_vex_vendor,hosted_memory_engine,hosted_memory_parity,in_process_remote_ecosystems_apply,in_process_rollback_all_ecosystems,in_process_vendor,cli_apply_silentandecosystem_dispatch_e2e: all pass.cargo test --workspacelocally because the sandbox disk filled while linking every CLI test binary. CI runs it./code-review high
Fixed in 0b99e93:
disk_root()read.quoted.Split.GOFLAGSfields weren't treated as a setting go refuses.go.modfor ago.workreplace.GOWORKwas counted as workspace mode.Not changed:
go_workspace::load/Loaded.Loadeddescribes the go.work in effect, and-modfilematters in module mode too. Mapping it toLoaded::Invalid/Nonewould change everyresolve() != GoWork::None"is this a workspace" check (selection gates,modules.txtlocation). I kept the rule in one function (modfile_setting_with) and called it at the existing fail-closed boundaries.audit, andGOFLAGSre-read per module. Each is one small file read per module, only on--check/apply runs. I didn't think threading a resolved setting through every signature was worth it.Bugbot
-modfile=go.modstill took the raw root read). Fixed in 801d50f, with a regression test.https://claude.ai/code/session_011f9aePoQ9gtsKv1zP8Rxsg
Generated by Claude Code