Repository navigation
CI: hermetic spawns in in_process tests (#823) - #1452
Conversation
Assisted-by: Claude Code:claude-opus-5-5
Move the binary spawns in 16 in_process, mode_migration and hosted test files onto tests/common/hermetic.rs and drop them from the PENDING_RAW_SPAWNS ratchet. A developer's ambient SOCKET_* vars can no longer change what these suites exercise. Refs #823 Assisted-by: Claude Code:claude-opus-5-5
Review follow-ups for the in_process hermetic spawns: name the builder's forced opt-outs once so the env_clear harness re-applies them, add Extra::Pdm and Extra::Poetry instead of private sweeps, add cache_env::scrub_ambient_bun_tool_env for the bun suite's CLI child, and remove SOCKET_PYPI_JSON_API by name in the pdm and poetry scans so a serial test's mock URL can't leak into a parallel spawn. Refs #823 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 d87eb06. Configure here.
|
Ready for review (burn-down agent)
Generated by Claude Code |
Final review briefWhat it does. Sixteen test files used to scrub Risk: low. Only the test harness changes, and every child environment is the same as before or stricter. The builder removes every Look here
Verified
Changes I made: none. Open questions (non-blocking): the old seed-then-scrub canaries for Auto-merge is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
* 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>
LLM Description written by Claude Code:claude-opus-5-5
Refs #823 (slice 6 of the tracking issue; #823 stays open for the remaining files).
Problem
16 test files spawned the built
socket-patchbinary with a bareCommand::newand their ownSOCKET_*sweep. The sweeps differed from file to file. Some removedSOCKET_TELEMETRY_*andSOCKET_NO_CONFIG, some kept them, and none of them forcedSOCKET_NO_UPDATE_CHECK. Six files also kept their own copy of the "remove the fourapply_env_togglesvars" workaround.Change (test-only)
hosted_memory_{common,engine},hosted_superseding_pypi,in_process_redirect{,_pdm,_pnpm,_poetry},in_process_rollback_{hosted,vendored},in_process_vendor{,_bun_takeover,_npm_v1_takeover},mode_migration_{bun,cargo,pypi}andremove_rollback_api_overridesnow spawn throughtests/common/hermetic.rs, and they leavePENDING_RAW_SPAWNS(92 → 76). The tokio call sites wrap the builder withtokio::process::Command::from.SOCKET_*by name, so the per-file toggle lists andremove_rollback_api_overrides' ownSOCKET_ENV_VARScopy and its drift guard are gone.hermetic.rsgainsFORCED_OPT_OUTS(the three opt-outs, named once). Theenv_clearharness inhosted_memory_commonre-applies it instead of keeping a hand-copied list.hermetic.rsalso gainsExtra::Pdm(PDM_*+UV_PROJECT_ENVIRONMENT) andExtra::Poetry(POETRY_*), which replace the private sweeps.cache_env::scrub_ambient_bun_tool_envis the bun suites' shared scrub minusSOCKET_*, used for the CLI child inmode_migration_bun.SOCKET_PYPI_JSON_APIby name. Their#[serial]testsset_varit for their own mock, and a parallel spawn could otherwise inherit it between the sweep and the spawn.PENDING_RAW_SPAWNSlines aren't adjacent to these.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-featuresover the 18 affected targets (the 16 files plushosted_memory_{parity,rollout}, which sharehosted_memory_common) andspawn_env_hygiene: all pass, with 2 exceptions that come from the sandbox, not this change:in_process_redirectchmod-0555 write-failure tests fail only because the sandbox runs as uid 0. Run asnobody(setpriv --reuid=65534), they pass.e2e_bun_lockb(compiled becausecache_envchanged) has one test that needs live patches-api.socket.dev, which the sandbox proxy blocks.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 all 18 targets: no failures caused by the hostile env. The only failures were the root/sandbox ones above and 4 that also fail asnobodywithout the hostile env (a cargo/pipenv home artifact); the same 4 pass as root under the hostile env.spawn_env_hygiene15/15, includingno_new_bare_binary_spawns.cargo fmt --all -- --checkis clean./code-review high
4 findings, all fixed in d87eb06: the
SOCKET_PYPI_JSON_APIrace in the pdm/poetry scans, the hand-copied opt-outs afterenv_clear, the inline bun scrub, and the private PDM/Poetry sweeps.Follow-ups for #823: the remaining 76
PENDING_RAW_SPAWNSfiles (mostlye2e_*,vex_*,vlt_*), minus the 13 that #1448 removes.🤖 Generated with Claude Code
https://claude.ai/code/session_016h1P5jNLH36RrvXV8HUQp8
Generated by Claude Code