Skip to content

CI: hermetic spawns in Go test suites (#823) - #1495

Merged
Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
agent/fix-ci-hermetic-golang
Oct 13, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
agent/fix-ci-hermetic-golang

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Refs #823 (slice 11; the issue stays open for the remaining PENDING_RAW_SPAWNS files)

What

The four Go suites still started socket-patch with a bare Command::new and their own partial SOCKET_* handling, which had drifted from tests/common/hermetic.rs:

  • e2e_golang.rs: two spawn sites seeded and removed a hand-picked list of SOCKET_* vars. Any other ambient SOCKET_* (e.g. SOCKET_DRY_RUN, SOCKET_MANIFEST_PATH) reached the child, and nothing forced the update notifier or the config layer off.
  • e2e_golang_build.rs: swept SOCKET_* but didn't force SOCKET_NO_CONFIG / SOCKET_NO_UPDATE_CHECK, so a developer's real socket login could reach the child.
  • e2e_golang_workspace_build.rs: private sweep plus hand-set opt-outs, no update-check opt-out.
  • vendor/e2e_golang_redirect.rs: removed only six named vars.

All four now spawn through the shared builder: e2e_golang through one golang_command helper (also scrubs VIRTUAL_ENV: its scans are unscoped, so an activated venv added packages to the exact-PURL batch), e2e_golang_build and the vendor redirect suite through common::run_with_env, and the workspace suite through hermetic::command + Extra::Venv (it needs prebuilt_common::prepare_command). The files leave PENDING_RAW_SPAWNS (26 -> 22), so the spawn_env_hygiene ratchet keeps them hermetic.

hermetic::command now also removes the SOCKET_CLI_* peer aliases (SOCKET_CLI_API_TOKEN & co.) by name, like the flag-bound vars. The binary promotes a peer token onto SOCKET_API_TOKEN, so one set in-process after the command was built put the child on the authenticated path (found by /code-review high; this suite used to remove SOCKET_CLI_API_TOKEN by hand).

Test-only change; no product code touched.

CI cost

No workflow change. Lean PR run: same job count and job-minutes before and after (these suites already run in the same jobs).

Test evidence

  • cargo fmt --all -- --check: clean.
  • cargo clippy on the five touched test targets: same output as origin/main (the pre-existing duplicate_mod / type_complexity lints in shared test modules; CI's cargo clippy --workspace doesn't build tests).
  • cargo test -p socket-patch-cli --all-features --test e2e_golang --test e2e_golang_build --test e2e_golang_workspace_build --test vendor --test spawn_env_hygiene -- --include-ignored with go 1.24.7, run with a hostile ambient env (VIRTUAL_ENV pointing at a venv holding requests-2.31.0, SOCKET_DRY_RUN=true, SOCKET_ECOSYSTEMS=npm, SOCKET_CLI_API_TOKEN=x): 23 + 26 + 16 + 95 + 15 passed, 0 failed.
  • Red to green: command_drops_flag_bound_vars_set_after_it_is_built (extended with the four peer aliases) fails without the hermetic.rs change (SOCKET_CLI_API_TOKEN set after build leaked) and passes with it. The ratchet's stale-entry check fails if any of the four files is left on PENDING_RAW_SPAWNS.
  • The full cargo test --workspace didn't fit the session's disk; CI runs it.

/code-review high

Fixed: venv leak in e2e_golang, duplicated env pin across its two spawn sites, private runner copies in e2e_golang_build and the vendor redirect suite (now common::run_with_env), peer-alias removal by name, and the redundant SOCKET_DRY_RUN set_var guard.
Not fixed: sharing the prebuilt-aware runner between e2e_golang_workspace_build and e2e_vendor_golang_build. That touches a file outside this slice and is a refactor, not a hermeticity gap.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FtgsyfBXmzpdFpQSBz4awp


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
The four Go suites started socket-patch with a bare Command::new and
their own SOCKET_* handling, which had drifted from the shared
tests/common/hermetic.rs: e2e_golang and the vendor redirect suite
removed only a hand-picked list of vars, so any other ambient
SOCKET_* (dry run, manifest path) reached the child, and none forced
the update notifier off. A developer's shell could silently change
what these tests exercise.

They now start from hermetic::command and keep only their Go env
(GOMODCACHE, GOPATH, GOFLAGS, plus VIRTUAL_ENV via Extra::Venv). The
files leave PENDING_RAW_SPAWNS (26 -> 22) so the ratchet keeps them
hermetic.

Refs #823

Assisted-by: Claude Code:claude-opus-5-5
From /code-review high on the Go hermetic migration:

- hermetic::command now removes the SOCKET_CLI_* peer aliases by
  name, like the flag-bound vars. The binary promotes a peer token
  onto SOCKET_API_TOKEN, so one set after the command was built put
  the child on the authenticated path.
- e2e_golang's scans are unscoped, so an activated VIRTUAL_ENV added
  the venv's packages to the batch and broke the exact-PURL checks.
  Both spawn sites now share one builder that scrubs it.
- e2e_golang_build and the vendor redirect suite use the shared
  common::run_with_env runner instead of private copies, and the
  redundant SOCKET_DRY_RUN set_var guard is gone (the builder seeds
  and scrubs it itself).

Refs #823

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 13, 2026 18:34
@mikolalysenko Mikola Lysenko (mikolalysenko) added the ci-perf CI / merge-queue performance finding (profiler routine) label Oct 13, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@mikolalysenko Mikola Lysenko (mikolalysenko) removed the ci-perf CI / merge-queue performance finding (profiler routine) label Oct 13, 2026

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

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 13, 2026
The NuGet and PyPI e2e suites started socket-patch with a bare
Command::new and their own partial SOCKET_* scrubs, so a developer's
ambient SOCKET_* settings, real `socket login` config or activated
venv could change what the tests exercised. They now spawn through
the shared hermetic builder, scrub VIRTUAL_ENV, and leave the
spawn-hygiene ratchet's pending list (26 -> 22 with #1495's slice).

Refs #823

Assisted-by: Claude Code:claude-opus-5-5
Merged via the queue into main with commit 7626582 Oct 13, 2026
86 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-ci-hermetic-golang branch October 13, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants