CI: hermetic spawns in cli/covgap tests (#823)

[Mikola Lysenko (mikolalysenko)]

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

Refs #823 (slice 4 of the hermetic-spawn migration; the issue stays open for the remaining files).

What was wrong

20 CLI test files (cli/, cli_*, coverage_fix_*, covgap_*) spawned socket-patch with a bare Command::new and their own hand-rolled SOCKET_* scrub. The copies disagreed. Several removed only the global flag vars, so SOCKET_API_TOKEN, SOCKET_API_URL or a per-command var like SOCKET_SKIP_ROLLBACK from a developer shell still reached the child. Two plain list runs in cli_parse_list scrubbed nothing, and none forced the update notifier off.

There was also a race in the shared builder. hermetic::command removed only the SOCKET_* names present when it swept the parent env. Test binaries that set flag-bound vars in-process (apply_env_toggles, save/restore helpers) could leak one set between the sweep and the spawn, and each suite patched that with its own removal list. Those lists had already drifted: 4 vars in one place, 2 in another, the full list in a third.

Change (test-only)

• All 20 files spawn through tests/common/hermetic.rs and leave PENDING_RAW_SPAWNS (126 → 106), so the ratchet keeps them hermetic. Deliberate opt-ins stay as caller env: the config-layer suites still set SOCKET_NO_CONFIG=0, and the telemetry suite still re-enables telemetry.
• hermetic::command now removes every flag-bound var (GLOBAL_ARG_ENV_VARS ∪ LOCAL_ARG_ENV_VARS) by name, which closes the race once and deletes the per-file lists. SOCKET_CWD joins the hostile seeds (the telemetry suite used to seed it locally). New contract test: command_drops_flag_bound_vars_set_after_it_is_built.
• No workflow change. A lean PR run keeps the same job count (~28 Linux jobs) and job-minutes.

Evidence

• Hostile shell (SOCKET_DRY_RUN=true SOCKET_JSON=true SOCKET_API_TOKEN=… SOCKET_API_URL=http://127.0.0.1:1 SOCKET_PROXY_URL=… SOCKET_SKIP_ROLLBACK=true SOCKET_ALL_RELEASES=true SOCKET_ORG_SLUG=… SOCKET_NO_CONFIG=0) over the 19 affected targets:
  • main: 11 failures. 6 spawn tests in cli_parse_list (*_via_binary), covgap_commands_vendor::npm_only_human_vendor_does_not_read_composer_lock, plus 4 noted below.
  • this branch: the 7 spawn tests pass. What remains is cli_parse_list::defaults_match_contract (an in-process clap parse, not a spawn; out of scope here) and the 3 root-only failures below.
• command_drops_flag_bound_vars_set_after_it_is_built fails against main's hermetic.rs (SOCKET_OFFLINE set after build leaked) and passes with this change. spawn_env_hygiene: 15/15.
• cargo test --workspace --all-features --no-fail-fast, 249 targets: everything passes except 13 tests that need a non-root user to make a file unwritable (*_write_failure_*, *unremovable*, relax_loop_must_not_traverse_symlinked_root) and e2e_bun_lockb::binary_shared_bundled_record_hosted_pin_is_managed. My container runs as root. Each of these fails the same way with main's hermetic.rs and test files, so none are from this PR. CI runs as non-root.
• cargo fmt --all -- --check and cargo clippy --locked --workspace --all-features -- -D warnings: clean.

/code-review high: findings not fixed here

• PTY spawns (portable_pty::CommandBuilder::new(binary()), 7 sites, including covgap_commands_scan_mod.rs and covgap_commands_rollback.rs) keep their own scrubs, and the ratchet does not detect them. That needs a CommandBuilder counterpart of the builder, so it is a separate slice, recorded on Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today #823.
• spawn_scrubbed (rollback) doesn't apply jvm_env::isolate_cli. No caller spawns a JVM fixture, and its doc now says so.

Follow-ups for #823: the 106 remaining PENDING_RAW_SPAWNS files (mostly e2e_*, in_process_*, vex_*), the PTY spawns, and the in-process parse in cli_parse_list.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CQp9sCPnUPTSFrEGsvTQmQ

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ 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 6d44a29. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review (burn-down agent)

• Head: 6d44a29a
• CI: 86/86 settled on the current head (36 success, 50 path-skipped); no main-wide failures.
• Mergeable against main, no CHANGELOG.md change.
• Bugbot: reviewed 6d44a29, no findings; no open review threads.
• Reviewer focus: the shared tests/common/hermetic.rs helper that the CLI/covgap tests now use to spawn the binary with a scrubbed environment.

---

Generated by Claude Code