GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Keep Go unit tests off the host's Go env by mikolalysenko · Pull Request #2057 · SocketDev/socket-patch

Conversation

Mikola Lysenko (mikolalysenko) commented Nov 11, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

Problem

CI is healthy this run. The merge queue is empty, every merge_group run since 11-10 passed, main's latest push run (42975739856) and today's nightly (42967443938) are green, nothing is queued, and no run in the last two days needed a re-run. Every PR failure in that window traces to the PR's own change. So this run takes a priority-4 robustness target in the same class as #1919/#2054/#2055/#2056: unit tests that pass only because CI's environment is clean. Those PRs cover npm/yarn, python and ruby. This one covers Go, and touches none of their files.

utils::go_env resolves Go settings from the process env, then the go env -w file under HOME. The Go unit tests run production code that reads it. Reproduced on origin/main (c09cb0e) with cargo test -p socket-patch-core --lib:

env	before	after
clean	4 failed*	4 failed*
`GOWORK=off` (a common `go env -w` setting)	19 failed (15 Go tests)	4 failed*
`GOWORK=<any absolute file>`	107 failed (103 Go tests across `golang_local`, `vendor::golang`, `go_consumer_sync`, `go_mod_edit`, `go_sum_backup`, `vex::discover::golang`)	4 failed*
`GOFLAGS=-modfile=alt.mod`	99 of 717 Go tests failed	4 failed*
`HOME` with `.config/go/env` → `GOWORK=off`, `GOFLAGS=-modfile=alt.mod`	(not measured on main; same lookup path)	4 failed*

* The same 4 tests fail in every row, before and after: copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, and the two pypi_* wire-failure tests. They rely on permission errors, and my sandbox runs as root. CI doesn't run as root.

A Go developer with GOWORK=off or GOFLAGS set gets red tests on a clean checkout. The CI image could get the same result if it ever exports one of them.

Root cause

go_env is the single place that reads process env for Go settings (GOWORK, GOFLAGS, GOPATH, GOMODCACHE, GOBIN, plus GOENV/HOME/XDG_CONFIG_HOME/AppData to find the go env file). In unit tests it reads the developer's real environment.

Fix

go_env reads through process_env. In production builds that's std::env::var, unchanged. In #[cfg(test)] builds it's test_env::guarded_var, which returns a var only while an EnvGuard holds it. Host values, including the go env file reached through HOME, are invisible.
EnvGuard records the keys it holds in a mutex-protected multiset and releases them on drop. Nested guards work, and so does a panic partway through many.
Three registry_fetch goproxy tests hand-rolled set_var/restore. They now hold their keys with EnvGuard, which also drops about 35 lines of manual save/restore. Assertions are unchanged.
New self-test guarded_var_sees_only_vars_a_live_guard_holds.
I chose this over a before-main env scrub (ctor) on purpose: that would add two new crates to the lockfile, and it would mutate env before main.

No test is removed and no assertion is changed or weakened.

Proof

Before/after matrix above, full --lib suite (7,692 tests).
GOWORK=off loop over the Go, test_env and goproxy tests: 30 runs, 0 failures.
cargo clippy --locked -p socket-patch-core --all-features -- -D warnings (CI's command) is clean. --all-targets reports no lint in any touched file. The pre-existing --all-targets lints are elsewhere.
The integration tests that include test_env.rs with #[path] pass: crawler_npm_e2e 128, crawler_deno_e2e 13, upstream_restore_golden 62.

https://claude.ai/code/session_011f9aeSwyBVuHJgG8G9SKZ1

Generated by Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Keep Go unit tests off the host's Go env

740eb68

`utils::go_env` reads GOWORK, GOFLAGS, GOPATH and friends from the
process and from the `go env -w` file under HOME. The Go unit tests
call production code that goes through it, so a developer's ambient
Go config changes what every fixture resolves. On origin/main
`cargo test -p socket-patch-core --lib` fails 15 tests with
GOWORK=off (a common `go env -w` setting), 103 with GOWORK pointing at
any absolute file, and 99 with GOFLAGS=-modfile=alt.mod.

In unit-test builds `go_env` now reads only the vars a test holds
with an EnvGuard (`test_env::guarded_var`); everything else, the
go env file's HOME/XDG_CONFIG_HOME/AppData included, is invisible.
Production builds read the process env exactly as before. The three
registry_fetch tests that hand-rolled set_var/restore now hold their
keys with EnvGuard, so their values stay visible.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011f9aeSwyBVuHJgG8G9SKZ1

Mikola Lysenko (mikolalysenko) added the ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) label

Nov 11, 2026

Mikola Lysenko (mikolalysenko) commented Nov 11, 2026

Copy link

Copy Markdown

Collaborator Author

bugbot run

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot commented Nov 11, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

PR Summary

Low Risk
Test-only isolation and small refactors; production go_env still uses std::env::var unchanged.

Overview
Go unit tests were picking up the developer's real GOWORK, GOFLAGS, and go env -w settings via go_env, causing large flaky failures off clean CI.

EnvGuard now tracks which keys it holds in a global mutex-backed list and exposes guarded_var, which only reports a value while a live guard owns that key. In #[cfg(test)], go_env reads the process environment through guarded_var instead of std::env::var, so host env and paths into the user's Go config file are invisible unless a test explicitly sets them with EnvGuard.

A new test_env self-test covers nested guards and guarded_var visibility. Three registry_fetch goproxy tests drop hand-rolled save/restore in favor of EnvGuard; behavior is unchanged.

^{Reviewed by Cursor Bugbot for commit 740eb68. Configure here.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Nov 11, 2026

View reviewed changes

cursor Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

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 740eb68. Configure here.}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) mentioned this pull request

Nov 11, 2026

Skip permission-denial unit tests when run as root #2058

Open

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

Nov 11, 2026

Mikola Lysenko (mikolalysenko) commented Nov 11, 2026

Copy link

Copy Markdown

Collaborator Author

Ready for review at 740eb68.

CI: 50/50 checks done, none failed (31 success incl. ci-ok and clippy, 19 skipped by path filters). No main-wide failures.
Bugbot: reviewed 740eb68, no findings.
Look at: utils/go_env.rs — production go_env should still read the real environment; only the unit tests go through the injected test_env seam.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Nov 11, 2026

Copy link

Copy Markdown

Collaborator Author

Final review brief

What it does: In #[cfg(test)] builds, utils::go_env now sees only the env vars a test holds with an EnvGuard. A developer's GOWORK=off, GOFLAGS=-modfile=... or go env -w file (found through HOME) no longer changes what the Go fixtures resolve. Before this change those settings failed up to about 100 unit tests. EnvGuard records the keys it holds in a mutex-protected multiset, and three goproxy tests swap their hand-rolled save/restore for EnvGuard.

Risk: low. The production process_env is still std::env::var (#[cfg(not(test))]). The test-only side could only hide a var that a test sets with raw set_var without a guard. I grepped for that: the only raw GO* writes are in the goproxy tests, on keys an EnvGuard already holds. CI is green on the whole suite.

Look here

utils/go_env.rs:32: the cfg-split process_env (prod and test variants).
test_env.rs:10: the GUARDED multiset and guarded_var, plus the push at L52 and release at L74. The release uses rposition, so nested guards work.
vendor/registry_fetch.rs:1557: the nested _mixed_env guard drops before _env, so the restore order is correct.
test_env_tests.rs:53: the new self-test for nested and released guards.

Verified: I read the full diff (4 files, +71/−44). Locally, GOWORK=off GOFLAGS=-modfile=alt.mod cargo test -p socket-patch-core --lib -- go gives 1086 passed, 0 failed. The integration tests that #[path]-include test_env.rs keep #[allow(dead_code)], so the new guarded_var doesn't warn there. CHANGELOG.md is untouched. CI: ci-ok and clippy are green, Bugbot is clean on 740eb68b, and there are no review threads.

Changes I made: none.

Open questions: none. One minor point that isn't blocking: guarded_var compares keys case-sensitively, which is stricter than Windows env lookups. That can only hide a var, never leak one.

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

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) enabled auto-merge

November 11, 2026 14:57

Reviewers

cursor[bot]

cursor[bot] left review comments

At least 1 approving review is required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Mikola Lysenko (mikolalysenko) commented Nov 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root cause

Fix

Proof

Uh oh!

Mikola Lysenko (mikolalysenko) commented Nov 11, 2026

Uh oh!

cursor Bot commented Nov 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Nov 11, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Nov 11, 2026

Final review brief

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mikola Lysenko (mikolalysenko) commented Nov 11, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot commented Nov 11, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.