Skip to content

CI: hermetic spawns in scan/vendor tests (#823) - #1448

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

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

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 5 of the hermetic-spawn migration; the issue stays open for the remaining files).

What was wrong

13 test files (scan_*, top-level vendor_*, vex_terminal_output) spawned socket-patch with a bare Command::new and their own SOCKET_* scrub. The copies disagreed, and many spawns scrubbed nothing:

  • 11 of the 13 spawns in scan_vendor_e2e only set SOCKET_TELEMETRY_DISABLED.
  • scan_requirements_lock_only and scan_vendor_requirements_unwired scrubbed no SOCKET_* var at all.
  • vex_terminal_output, vendor_eject*, scan_rollout_e2e and vendor_jvm_cli also removed SOCKET_NO_CONFIG. So a developer who ran socket login could have their real token picked up through the config fallback.

Change (test-only)

  • Every spawn in the 13 files now goes through tests/common/hermetic.rs (hermetic_command / hermetic::binary_command). The hand-rolled loops and the redundant telemetry and no-config sets are gone. Env the tests inject themselves (SOCKET_MAX_NEW_PATCHES, SOCKET_PATCH_FAILPOINT, SOCKET_VENDOR_URL, SOCKET_API_URL, extra_env) is still applied after the builder, so it wins.
  • The 13 files leave PENDING_RAW_SPAWNS (126 → 113 on main), so the ratchet in spawn_env_hygiene keeps them hermetic.
  • No overlap with CI: hermetic spawns in cli/covgap tests (#823) #1441 (slice 4): different files and non-adjacent list lines, and hermetic.rs is untouched. Whichever merges second rebases cleanly.
  • No workflow change. A lean PR run keeps the same job count (~28 Linux jobs) and job-minutes.

Evidence

  • cargo test -p socket-patch-cli --all-features over the 14 changed targets: 272/272 pass.
  • Hostile shell (SOCKET_DRY_RUN=true SOCKET_JSON=true SOCKET_SILENT=true SOCKET_API_TOKEN=hostile SOCKET_API_URL=http://127.0.0.1:1 SOCKET_PROXY_URL=http://127.0.0.1:1 SOCKET_NO_CONFIG=0 SOCKET_ORG_SLUG=hostile SOCKET_ECOSYSTEMS=cargo SOCKET_OFFLINE=true) over the 13 suites:
    • main: 28 failures. scan_requirements_lock_only 10/10, scan_vendor_e2e 14/58, scan_vendor_requirements_unwired 4/4.
    • this branch: 0 failures, 258/258.
  • spawn_env_hygiene 14/14, including no_new_bare_binary_spawns. cargo fmt --all -- --check is clean. The changed code is test-only and CI's cargo clippy --workspace doesn't lint test targets. Running clippy on these test targets reports the same errors as main, all in the shared prebuilt_common/ and common/jvm_env helpers.

/code-review high

No correctness findings. Fixed: stale "every ambient SOCKET_* var scrubbed" doc comments in the vendor_eject* helpers. Not fixed here:

  • Extra::Venv removes only VIRTUAL_ENV, but the Python crawler also falls back to CONDA_PREFIX. So scan_requirements_lock_only and scan_vendor_requirements_unwired still remove both by hand, and that stays correct. Widening Extra::Venv changes every suite that uses it, so it belongs in its own change. I'll note it on Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today #823.
  • vex_terminal_output::cli() is now a one-line wrapper around hermetic::binary_command(). I kept it to keep the diff small, since it has many call sites.

Follow-ups for #823: the 113 remaining PENDING_RAW_SPAWNS files (mostly e2e_*, in_process_*, vex_*, vlt_*), the PTY spawns, and Extra::Venv covering CONDA_PREFIX.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YZvGQWp5o1nrjdaai8HpoG


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
13 scan, vendor and vex test files started socket-patch with a bare
Command::new and their own SOCKET_* scrub. The copies disagreed, and
most of scan_vendor_e2e's spawns plus every spawn in the two
requirements-lock scan suites scrubbed nothing. A developer shell with
SOCKET_API_URL, SOCKET_JSON or SOCKET_DRY_RUN set failed 28 of their
tests on main.

Every spawn now goes through tests/common/hermetic.rs, and the files
leave PENDING_RAW_SPAWNS (126 -> 113) so the ratchet keeps them
hermetic. Refs #823.

Assisted-by: Claude Code:claude-opus-5-5
The run helpers' comments still said every ambient SOCKET_* var is
removed. The hermetic builder instead keeps telemetry opt-outs and
forces SOCKET_NO_CONFIG, SOCKET_NO_UPDATE_CHECK and telemetry off.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 12, 2026 21:12

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

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

Copy link
Copy Markdown
Collaborator Author

Ready for review (burn-down agent)


Generated by Claude Code

Merged via the queue into main with commit 15c2c83 Oct 12, 2026
86 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-ci-hermetic-scan-vendor branch October 12, 2026 22:57
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