Skip to content

Keep CLI test children off the host's Go env - #2063

Merged
Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
ci-janitor/go-env-scrub
Nov 13, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
ci-janitor/go-env-scrub

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

CI was healthy on this run. The merge queue was empty, and all 28 merge_group runs since 11-12 18:56 UTC passed. Main's latest push run (43324491043) is green. Nothing was queued or in progress, and no PR run in the last 3 days needed a re-run because of a flake. Last night's Windows nightly failure (scan_hosted_paths_run_once_per_project_directory) was already fixed by #2061. So this run takes a priority-4 robustness target, in the same class as #2054, #2055, #2056 and #2057: tests that pass only because CI's environment is clean.

#2057 kept the core unit tests off the host's Go env. The CLI integration tests still pass the parent's GOWORK, GOFLAGS, GOMODCACHE, GOPATH and go env -w file on to every socket-patch and go child they spawn (#1981, items 1–2). On origin/main (de81f1b), with GOFLAGS=-modfile=/nonexistent/alt.mod GOWORK=/nonexistent/go.work exported:

suite before after
e2e_golang_build 21 of 21 failed (go mod download in stage fails on the ambient GOWORK) 21/21 pass
e2e_vendor_golang_build 11 of 13 failed (vendor refuses: "GOFLAGS sets -modfile=…, which go refuses in workspace mode") 13/13 pass
e2e_golang 3/3 pass 3/3 pass
spawn_env_hygiene n/a 21/21 pass (new self-test included)

With a clean env, every suite this PR touches passes before and after: e2e_golang, e2e_golang_build, e2e_golang_hosted_build, e2e_vendor_golang_build, e2e_hosted_production, e2e_vendored_production and spawn_env_hygiene.

Root cause

hermetic.rs has a scrub (Extra) for every other ecosystem whose config the crawler reads from the environment (venv, Bundler, Composer, PDM, Poetry), but none for Go. The go helpers in the Go suites set only some keys, and pin GOWORK in none of them.

Fix

  • Add hermetic::Extra::Go. It removes GOFLAGS, GOWORK, GOPATH and GOMODCACHE, then sets GOENV=off so the same keys can't come back from the go env -w file.
  • Add it to CRAWLER_EXTRAS, which covers the vex e2e harness, the hosted and vendored production suites, and the gem suites.
  • Apply it in common::run_bin_with_env, right after jvm_env::isolate_cli, so it reaches every run/run_with_env child.
  • Apply it in the go() / run_socket spawn helpers of e2e_golang_build, e2e_golang_hosted_build and e2e_vendor_golang_build, before cache_env::isolate (scrub first, as the ordering rule in hermetic.rs says).
  • Tests that need a value still set it after the scrub, as they already do (GOMODCACHE, GOFLAGS, GOENV pins). The existing -modfile/GOWORK tests set theirs explicitly and still pass.
  • New self-test crawler_inputs_sweep_go_module_cache_and_flags.

On CI this changes nothing: no workflow exports these variables and runners have no go env -w file. It protects developer machines, and CI if a runner image ever starts setting them.

Not in this PR (left for #1981)

Where tests run

No test was removed or moved. Only spawn env changed.

Validation

  • rustfmt --check on the touched files: clean.
  • cargo clippy -p socket-patch-cli --all-targets -- -D warnings fails locally only on errors that already exist on main (prebuilt_common/mod.rs, repair.rs, common/mod.rs:27 duplicate-module lint). None are in lines this PR changes. CI's clippy job doesn't use --all-targets.

Generated by Claude Code


Generated by Claude Code

A developer or runner that exports GOWORK, GOFLAGS=-modfile=...,
GOMODCACHE or GOPATH (or sets them with `go env -w`) leaks them into
every socket-patch and `go` child the CLI tests spawn. With
GOWORK=<file> exported, every `go mod download` in the Go suites'
staging fails; with GOFLAGS=-modfile=... Go vendor refuses to write
its replace. e2e_golang_build failed 21/21 and e2e_vendor_golang_build
11/13 that way on a clean checkout.

Add hermetic::Extra::Go (remove GOFLAGS, GOWORK, GOPATH, GOMODCACHE;
set GOENV=off) and apply it to CRAWLER_EXTRAS, to every child of
common::run_bin_with_env, and to the go/socket-patch spawn helpers
of e2e_golang_build, e2e_golang_hosted_build and
e2e_vendor_golang_build. Tests that need a value still set it after
the scrub. A spawn_env_hygiene self-test pins the scrub.

Part of #1981 (items 1-2).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011f9aeR7CFpkqZDdd9hfYmM
@mikolalysenko Mikola Lysenko (mikolalysenko) added the ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) label Nov 12, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor

cursor Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Changes are confined to CLI test harness environment setup; no production or crawler runtime behavior is modified.

Overview
Adds hermetic::Extra::Go so CLI integration tests stop inheriting the developer shell’s GOFLAGS, GOWORK, GOPATH, GOMODCACHE, and the go env -w file (GOENV=off).

Go joins CRAWLER_EXTRAS (hosted/vendored production and vex harnesses), run_bin_with_env, and the go() helpers in the golang e2e suites—scrub first, then fixture env wins. A new spawn_env_hygiene test asserts the sweep under hostile ambient values.

Reviewed by Cursor Bugbot for commit 5b42c27. Configure here.

@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 5b42c27. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

Ready for review (burn-down agent).

  • Head: 5b42c27
  • CI: 50/50 checks settled green (31 success, 19 skipped); no main-wide failures
  • Bugbot: reviewed 5b42c27 — no findings
  • Mergeable, no CHANGELOG changes. Test-harness-only change (crates/socket-patch-cli/tests/**); reviewers may want to check the ordering of the Go scrub vs. cache_env::isolate in the three Go spawn helpers.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. Test-only change. It adds a Go scrub (hermetic::Extra::Go: removes GOFLAGS, GOWORK, GOPATH, GOMODCACHE and sets GOENV=off) and applies it to every socket-patch and go child that the CLI integration tests spawn. Before this, a developer with GOWORK or GOFLAGS=-modfile=… exported saw most of the Go e2e suites fail. CI exports none of these, so CI behavior doesn't change.

Risk: low. Only files under crates/socket-patch-cli/tests/ change, and no production code does. Every caller applies the scrub before its own pins (cache_env::isolate, GOMODCACHE, run_with_env env, VexRun::envs), so explicit test values still win.

Look here

  • hermetic.rs:277, :287, :377: the new variant, adding it to CRAWLER_EXTRAS, and the scrub itself
  • common/mod.rs:113: applied to every run/run_with_env child, before caller env
  • e2e_golang_build.rs:93, [e2e_vendor_golang_build.rs:67](https://github.com/SocketDev/socket-patch/blob/5b42c27d833f021474f07945b695786343fac4ef/crates/socket-patch-cli/tests/e2e_vendor_golang_build.rs#L67): go/run_socket helpers, scrubbed before cache_env::isolate
  • spawn_env_hygiene.rs:713: new self-test (fails without the change)

Verified

  • Read the full diff (9 files, +69/−10). Checked ordering at every scrub_crawler_inputs caller (vex harness, hosted and vendored production, gem suites): nothing sets a GO* value before the scrub. The golang_production legs pin their own full Go env, so the scrub doesn't touch them.
  • rustfmt --check on the touched files: clean.
  • cargo test -p socket-patch-cli --test spawn_env_hygiene: 21/21 pass.
  • With GOFLAGS=-modfile=/nonexistent/alt.mod GOWORK=/nonexistent/go.work exported: e2e_golang_build 21/21 and e2e_golang 3/3 pass.
  • CI on the head: 50/50 settled (31 success, 19 skipped), ci-ok and clippy green. Cursor Bugbot: success, no open threads. CHANGELOG.md untouched.

Changes I made: none.

Open questions: none blocking. Nit: the doc comment changed at e2e_hosted_production.rs:308 runs past 100 columns. As the description says, the in-process Go discovery tests (e2e_golang_workspace_build and 2 in e2e_golang_hosted_build) are still exposed to a hostile env and are left for #1981.

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


Generated by Claude Code

Merged via the queue into main with commit 9bac91f Nov 13, 2026
50 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the ci-janitor/go-env-scrub branch November 13, 2026 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants