Skip to content

CI: hermetic spawns in VEX tests (#823) - #1454

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

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

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Refs #823 (slice 7 of the tracking issue; #823 stays open for the remaining files).

Problem

The VEX test suites spawned the built socket-patch binary with a bare Command::new and their own SOCKET_* sweep, and the sweeps differed from file to file:

  • e2e_vex.rs, e2e_vex_redirect.rs, e2e_embedded_vex.rs and five e2e_vex_lockfile cells kept an ambient SOCKET_NO_CONFIG (so SOCKET_NO_CONFIG=0 in a dev shell reached the child), and the first three never forced telemetry off.
  • Only bun.rs forced SOCKET_NO_UPDATE_CHECK.
  • e2e_vex_build/hatch.rs's two vendor --revert spawns scrubbed nothing at all.
  • The shared vex_e2e_common::run_vex helper had its own copy of the sweep.

Change (test-only)

  • e2e_vex_lockfile/{bun,cargo,golang,maven,npm,nuget,pnpm,poetry,uv,yarn}.rs, e2e_vex_build/{deno,hatch,poetry}.rs, e2e_vex.rs, e2e_vex_redirect.rs and e2e_embedded_vex.rs now spawn through tests/common/hermetic.rs, and they leave PENDING_RAW_SPAWNS (92 → 76 on main).
  • vex_e2e_common::run_vex uses hermetic::command(bin). vex_e2e_common loads hermetic.rs as pub mod hermetic (with #[allow(clippy::duplicate_mod)], as the other shared modules do), so e2e_vex_build, which has no common, reaches it there.
  • The VIRTUAL_ENV removals go through hermetic::Extra::Venv. Opt-out env sets that the builder already forces are gone.
  • SOCKET_NO_API_TOKEN=1 stays where a file set it (caller env lands after the scrub).

Not changed here, on purpose

CI cost

No workflow change. A lean PR run keeps the same ~28 Linux jobs and the same job-minutes, before and after.

Evidence

  • cargo test -p socket-patch-cli --all-features --no-run: every test target builds (all vex_e2e_common users included), with no rustc warnings.
  • spawn_env_hygiene 15/15, including no_new_bare_binary_spawns. e2e_vex_lockfile 345/345, e2e_vex 40/40, e2e_vex_redirect 52/52, e2e_embedded_vex 34/34. e2e_vex_build compiles (its 16 tests are toolchain-gated #[ignore]).
  • Hostile shell (SOCKET_DRY_RUN=true SOCKET_JSON=true SOCKET_SILENT=true SOCKET_API_TOKEN=hostile SOCKET_API_URL/SOCKET_PROXY_URL=http://127.0.0.1:1 SOCKET_NO_CONFIG=0 SOCKET_NO_UPDATE_CHECK=0 SOCKET_ORG_SLUG=hostile SOCKET_ECOSYSTEMS=cargo SOCKET_OFFLINE=true SOCKET_VEX_PRODUCT=hostile SOCKET_VEX_NO_VERIFY=true VIRTUAL_ENV=/nonexistent) over e2e_vex_lockfile, e2e_vex, e2e_vex_redirect and e2e_embedded_vex: all pass. I didn't run main under the same shell for comparison.
  • cargo fmt --all -- --check is clean. cargo clippy on these test targets reports only warnings that main already has (prebuilt_common, oracle.rs duplicate_mod, e2e_vex_build/poetry.rs:485). CI's clippy job doesn't lint test targets.

/code-review high

7 findings, no correctness bugs. Fixed in 68732af: the new duplicate_mod warning, the dead opt-out sets in e2e_vex_redirect::vex_json_env, the inline VIRTUAL_ENV removals in hatch.rs, and the stale cli() doc comments. Not fixed: the POETRY_*/UV_* sweeps and vex_e2e_common/uv.rs (reasons above), and a suggested shared cli_command() helper. The four call sites set different extra env, so a helper wouldn't remove much code.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Wx5bWkf2EhmUyjqAXPj9fu


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
The VEX suites (e2e_vex_lockfile, e2e_vex_build, e2e_vex,
e2e_vex_redirect, e2e_embedded_vex) and the shared run_vex helper
built the socket-patch child with their own SOCKET_* sweeps. The
sweeps differed by file: some kept an ambient SOCKET_NO_CONFIG,
none forced the update check off, several left telemetry on, and
two hatch revert spawns scrubbed nothing at all. A developer's
shell could change what those tests exercised.

They now go through tests/common/hermetic.rs, and 16 files leave
the PENDING_RAW_SPAWNS ratchet. Test-only; no product change.

Refs #823

Assisted-by: Claude Code:claude-opus-5-5
Drop the opt-out env sets that the hermetic builder already forces,
route the remaining VIRTUAL_ENV removals through Extra::Venv, mark
the extra hermetic.rs load in vex_e2e_common as an intended
duplicate, and say what cli() really does in the doc comments.

Refs #823

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 12, 2026 23:36
@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 68732af. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

Labeled Ready for review by the burn-down agent.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does: Moves 16 VEX test files (e2e_vex*, e2e_embedded_vex, the e2e_vex_lockfile/* and e2e_vex_build/* cells) and the shared vex_e2e_common::run_vex helper off their own SOCKET_* sweeps and onto the hermetic spawn builder (tests/common/hermetic.rs), and takes them off the PENDING_RAW_SPAWNS ratchet. Before this, the sweeps disagreed (several kept an ambient SOCKET_NO_CONFIG, only bun forced the update-check off, and hatch's two vendor --revert spawns scrubbed nothing). Test-only; refs #823 (slice 7).

Risk: low. Only test harness code changes, and the child env only gets stricter. Per-test env (SOCKET_NO_API_TOKEN, CARGO_HOME, GOMODCACHE, NUGET_PACKAGES, UV_CACHE_DIR, ...) is still applied after the scrub, so explicit injections survive. The POETRY_*/UV_* sweeps stay as they are on purpose until #1452 adds Extra::Poetry.

Look here:

  • vex_e2e_common/mod.rs:114 and :427: hermetic loaded as a pub mod (with clippy::duplicate_mod allowed, like the other shared modules), and run_vex built on it.
  • e2e_vex_build/hatch.rs:337: the two spawns that previously had no scrub at all.
  • e2e_vex_lockfile/poetry.rs:1361:`` builder first, then the remaining private POETRY_* sweep, then caller env.
  • e2e_vex_redirect.rs:733: vex_json_env keeps SOCKET_NO_API_TOKEN and the per-call envs after the scrub.
  • spawn_env_hygiene.rs:67: 16 list entries removed (vex_e2e_common/uv.rs deliberately kept for the next slice).

Verified:

  • Read the full diff. Checked the env ordering (scrub, then suite env, then per-test env) in every changed spawn site, and that the forced opt-outs don't hide anything a test in these files asserts on.
  • Merged this PR and CI: hermetic spawns in redirect build tests (#823) #1457 onto current main (15c2c83) locally. Both merge cleanly, and spawn_env_hygiene, e2e_vex, e2e_embedded_vex and e2e_vex_redirect pass on the combined tree.
  • CI on 68732af: 36 success / 50 skipped of 86, ci-ok and clippy green. Cursor Bugbot: success, no findings. No unresolved review threads. CHANGELOG.md untouched.

Changes I made: none.

Open questions: none.

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


Generated by Claude Code

Merged via the queue into main with commit f0e53f2 Oct 13, 2026
86 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-ci-hermetic-vex branch October 13, 2026 13:37
github-merge-queue Bot pushed a commit that referenced this pull request Oct 13, 2026
* ci(release): publish npm from release.yml, not publish-npm.yml

Every socket-patch npm package's trusted publisher is registered against
release.yml, and npm allows only one publisher per package. The 5.0.0
release dispatched publish-npm.yml, whose OIDC token named that file, so
npm rejected it with E401 before staging anything.

Move the npm leg back inline as release.yml's npm-publish job (same steps
as publish-npm.yml) and delete publish-npm.yml, so the existing
registration keeps working. Add an `npm-retry-version` input that skips
the version gate, build, tag, GitHub release and crates.io jobs and
re-stages npm from an existing release's assets; this retries 5.0.0 now
and replaces the manual Publish npm dispatch. crates.io stays dispatched
through publish-cargo.yml, since crates.io allows several publishers per
crate.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): fold 22 more PRs into the 5.0.0 notes

Cover the PRs merged to main after #1432's base (1779317..4cb3643):
Cargo source replacement and registry = "crates-io" (#338/#455, #386),
pnpm lockfile=false (#1074), berry removed descriptors (#1435), yarnrc
${VAR} reads (#1100), CRLF .npmrc append (#1196), Composer vendor-dir
cascade and global home (#439/#686, #586), Go +incompatible vex (#484),
gem moved BUNDLE_GEMFILE and commented Gemfile revert (#952, #988),
Poetry source = "pypi" (#1282) and Hatch path characters (#547).

No entries for #1411 (macOS fix to code #1393 added in this release;
the #1393 entry already describes the intended behavior), #1426 (perf
regression introduced and fixed within 5.0.0) or the CI/test/bench PRs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): cover #1419, #1434, #1440, #1442, #1444 in the 5.0.0 notes

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): 5.0.1

Cut [Unreleased] into [5.0.1] — 2026-10-13 and stamp 5.0.1 with
scripts/version-sync.sh. The 5.0.1 notes cover main through 501c6d9
(#1449), i.e. every PR after the v5.0.0 tag (6ad6add): the npm
publish fix (5.0.0 never reached npm or crates.io, 5.0.1 is the first
v5 there), #1465, #759, #1460, #1462, #1445, #869, #1447, #1450,
#1467, and #1449 under Maintenance. CI/test-only PRs (#1448, #1452,
#1454, #1457, #1459) have no entries.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): 5.0.1 notes for PRs merged since the cut

Covers #1469, #1470, #1474, #1475, #1478, #1479, #1480 and #1482; #1484 is
CI-only. The notes now cover main through 285a5e8.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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