Skip to content

Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today #823

Open
Open
Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today#823
Labels
agent:claimedagent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)priority:p3refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

Description

@mikolalysenko
Mikola Lysenko (mikolalysenko)
opened on Oct 5, 2026

[agent] Filed by the scheduled architecture audit routine (CLI and core). Register: register comment.

Kind: refactor (test structure). Source: review Part 8.5 D (forked env scrubbers), plus a new finding (test files with no scrub at all); register C30 / C47.

Problem

The CLI test tree already has a hermetic spawner: common::run_bin_with_env seeds and scrubs the high-risk vars, prefix-sweeps the remaining SOCKET_* (keeping the telemetry opt-outs, SOCKET_NO_CONFIG and SOCKET_NO_UPDATE_CHECK), and forces SOCKET_NO_CONFIG=1 / SOCKET_NO_UPDATE_CHECK=1. But it only returns (code, stdout, stderr), so tests that need a Command (stdin, PTY, extra env removal, output() bytes) hand-roll their own:

  • 15 private scrub_socket_env copies with 14 different bodies. Some keep SOCKET_NO_CONFIG, some remove it (e2e_redirect_yarn_classic_build.rs#L98`` strips every SOCKET_*, including the `.cargo/config.toml` `SOCKET_NO_CONFIG=1` that keeps a developer's `socket login` token out of test children); none of the per-file copies keep `SOCKET_NO_UPDATE_CHECK`. Their real differences are per-PM extras (`YARN_`, `PNPM_`, `npm_config_*`, `VIRTUAL_ENV`).
  • 10 files that spawn the binary with no scrub at all (only SOCKET_TELEMETRY_DISABLED or a token removal): repair/repair_vendor_e2e.rs (L201-L212),`` scan/scan_invariants.rs (L53), `scan/scan_sync_e2e.rs`, `cli/api_client_errors_e2e.rs`, `cli_parse_remove.rs`, `apply/in_process_npm_multicopy.rs`, `self_update_e2e.rs`, `self_update_failures_e2e.rs`, `update/covgap_update_swap.rs`, `update/covgap_update_download.rs`. (Some of the update files spawn through a fixture helper; the child PR should check each.)

Proof (on 045d7ec, debug build, run twice, as root in a cloud container):

ambient var repair_vendor_e2e (unscrubbed, 19 tests) the other 97 tests in the repair target (scrubbed)
none 19 pass 95 pass, 2 fail*
SOCKET_DRY_RUN=true 1 pass, 18 fail ("setup must vendor the tarball") 95 pass, 2 fail*
SOCKET_OFFLINE=true 1 pass, 18 fail 95 pass, 2 fail*

*The two baseline failures are the "lock file unremovable" tests, which can't simulate an unremovable file as root; they are unrelated.

So the same ambient shell that the shared helper neutralizes silently changes what the unscrubbed suites exercise. Here it fails loudly; with a variable like SOCKET_ECOSYSTEMS, SOCKET_GLOBAL_PREFIX or SOCKET_API_TOKEN it can instead change the code path or aim a mutation at a real global cache.

Symptoms

None filed; contributors hit this as "passes in CI, fails locally".

Impact

Test hermeticity and maintenance: each new SOCKET_* variable has to be remembered in up to 16 places. Size: test-only, no production change.

Proposed change

  1. Split run_bin_with_env into pub fn hermetic_command(bin: &Path) -> Command (seed, scrub, force the two opt-outs) and the existing runner, which becomes hermetic_command(bin) + args + caller env + output().
  2. Give it the per-PM extras as an explicit opt-in, e.g. scrub_pm_env(&mut cmd, &[Pm::Yarn, Pm::Pnpm]), keeping the seed-then-scrub guards the yarn/pnpm copies document.
  3. Route the 10 unscrubbed files and the 15 scrub_socket_env copies through it, and delete the 15 copies.

Size and scope

~25 test files, net negative (~−250 lines). Out of scope: binary() / git_sha256 deduplication and a separate test-support crate (later children of #824), and any production code.

Acceptance criteria

  • common::hermetic_command exists, and run_bin_with_env is built on it.
  • grep -rn "fn scrub_socket_env" crates/socket-patch-cli/tests returns nothing.
  • Every test file that spawns CARGO_BIN_EXE_socket-patch (or a staged copy of it) does so through hermetic_command or the run* helpers; a guard test (or a grep in an existing lint test) keeps it that way.
  • SOCKET_DRY_RUN=true cargo test -p socket-patch-cli --test repair gives the same result as without the variable; the same for --test scan and --test cli.
  • All existing tests stay green in CI; the yarn/pnpm seed-then-scrub guards still fail red when the scrub is removed.

Dependencies

Blocked by nothing. First child of tracking issue #824. Independent of #793 (RunCtx), though fewer env-reading paths there will shrink what the scrub has to cover.

Activity

  1. Mikola Lysenko (mikolalysenko)
    added
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code
    on Oct 5, 2026
  2. Mikola Lysenko (mikolalysenko)
    added a parent issue on Oct 5, 2026
  3. Mikola Lysenko (mikolalysenko)
    added 2 commits that reference this issue on Oct 5, 2026

    audit-core: C28 rejected, C30 decomposed (#824, #823), C47 test herme…

    f90cba9

    refactor: re-rank queue at capacity (add #823)

    2323c1e
  4. Mikola Lysenko (mikolalysenko)
    added
    agent:triaged
    priority:p3
    on Oct 5, 2026

  5. mikolalysenko commented on Oct 5, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 5, 2026
    CollaboratorAuthor

    [agent] Triaged: priority:p3 (test-only refactor, CI hermeticity). Not a duplicate; child of tracking issue #824. No open PR addresses it yet.

    Generated by Claude Code

  6. mikolalysenko commented on Oct 5, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 5, 2026
    CollaboratorAuthor

    [agent] Claiming this issue for the architecture refactor routine (highest leverage: test-only, collapses the per-file scrub_socket_env copies into one common::hermetic_command, first child of #824). Branch: arch-refactor/823-hermetic-command. Claim-ID: 2026-10-05T11:56:39Z-93d7a1

    Slice: add hermetic_command, build run_bin_with_env on it, and migrate every spawner file that no open agent/fix-* PR touches. The files that open fix PRs change (#774, #802, #820, #837, #839, #849, #763) stay on an explicit allowlist in a guard test, to be migrated after those PRs land.

    Generated by Claude Code

  7. Mikola Lysenko (mikolalysenko)
    added
    agent:claimed
    on Oct 5, 2026

  8. mikolalysenko commented on Oct 5, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 5, 2026
    CollaboratorAuthor

    [agent] Draft PR: #850.

    Generated by Claude Code

  9. Mikola Lysenko (mikolalysenko)
    added 2 commits that reference this issue on Oct 5, 2026

    Spawn CLI test children via one hermetic builder

    37b5b4e

    Spawn cli_dry_run_paths through the hermetic builder

    4384385

  10. mikolalysenko commented on Oct 5, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 5, 2026
    CollaboratorAuthor

    [agent] Slice taken in #850:

    • common::hermetic_command exists, and run_bin_with_env is built on it.
    • 8 of the 15 scrub_socket_env copies are deleted.
    • 8 of the 10 unscrubbed files are now hermetic (6 moved, 2 already used run_installed), plus cli/cli_dry_run_paths_e2e.
    • A spawn_env_hygiene ratchet guards both lists.

    SOCKET_DRY_RUN=true now gives the same result for --test repair and --test cli as running without it.

    What remains:

    Generated by Claude Code

  11. Mikola Lysenko (mikolalysenko)
    added a commit that references this issue on Oct 5, 2026

    refactor: record #607 merge, open #850 (#823 slice 1)

    69f3347

  12. 28 remaining items

  13. mikolalysenko commented on Oct 12, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 12, 2026
    CollaboratorAuthor

    [agent] Claiming slice 5: move the 13 scan_*, top-level vendor_* and vex_terminal_output.rs files on PENDING_RAW_SPAWNS onto tests/common/hermetic.rs. These files and their list lines don't overlap #1441 (slice 4), and no other open PR touches them. Branch: agent/fix-ci-hermetic-scan-vendor. Claim-ID: 2026-10-12T20:51:07Z-e9aa37

    Generated by Claude Code

  14. mikolalysenko commented on Oct 12, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 12, 2026
    CollaboratorAuthor

    [agent] Slice 5 draft PR: #1448

    Generated by Claude Code

  15. Mikola Lysenko (mikolalysenko)
    added a commit that references this issue on Oct 12, 2026

    Spawn scan/vendor test children hermetically

    c918546

  16. mikolalysenko commented on Oct 12, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 12, 2026
    CollaboratorAuthor

    [agent] Slice 5 is up for review in #1448. The 13 scan_*, top-level vendor_* and vex_terminal_output.rs files now spawn through tests/common/hermetic.rs, and PENDING_RAW_SPAWNS goes from 126 to 113. Under a hostile SOCKET_* shell, main fails 28 tests across those suites and the branch fails none.

    New follow-up from the review: hermetic::Extra::Venv removes only VIRTUAL_ENV, but the Python crawler also falls back to CONDA_PREFIX. The two requirements-lock scan suites still remove both by hand for that reason. Widening Extra::Venv (plus CONDA_DEFAULT_ENV) would let them drop that.

    Generated by Claude Code

  17. mikolalysenko commented on Oct 12, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 12, 2026
    CollaboratorAuthor

    [agent] Claiming slice 6: move the 16 in_process_redirect*, in_process_rollback_*, in_process_vendor* takeover, mode_migration_{bun,cargo,pypi}, hosted_memory_*, hosted_superseding_pypi and remove_rollback_api_overrides files on PENDING_RAW_SPAWNS onto tests/common/hermetic.rs. No overlap with #1448 (slice 5): different files and non-adjacent list lines. Branch: agent/fix-ci-hermetic-in-process. Claim-ID: 2026-10-12T21:51:07Z-265cc2

    Generated by Claude Code

  18. mikolalysenko commented on Oct 12, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 12, 2026
    CollaboratorAuthor

    [agent] Slice 6 draft PR: #1452

    Generated by Claude Code

  19. Mikola Lysenko (mikolalysenko)
    added 2 commits that reference this issue on Oct 12, 2026

    CI: hermetic spawns in in_process tests

    b0d9db2

    Share scrubs from the hermetic test helpers

    d87eb06

  20. mikolalysenko commented on Oct 12, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 12, 2026
    CollaboratorAuthor

    [agent] Claiming slice 7: move the 16 e2e_vex_lockfile/*, e2e_vex_build/{deno,hatch,poetry}, e2e_vex.rs, e2e_vex_redirect.rs and e2e_embedded_vex.rs files on PENDING_RAW_SPAWNS onto tests/common/hermetic.rs. No overlap with #1448 or #1452: different files, and their PENDING_RAW_SPAWNS lines aren't adjacent to these. Branch: agent/fix-ci-hermetic-vex. Claim-ID: 2026-10-12T22:50:49Z-c3b511

    Generated by Claude Code

  21. mikolalysenko commented on Oct 12, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 12, 2026
    CollaboratorAuthor

    [agent] Slice 7 draft PR: #1454

    Generated by Claude Code

  22. Mikola Lysenko (mikolalysenko)
    added 2 commits that reference this issue on Oct 12, 2026

    Spawn VEX test children hermetically

    ebeda93

    Tidy VEX spawns after code review

    68732af

  23. mikolalysenko commented on Oct 12, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 12, 2026
    CollaboratorAuthor

    [agent] Slice 7 is up for review in #1454. The 16 VEX test files (e2e_vex_lockfile/*, e2e_vex_build/{deno,hatch,poetry}, e2e_vex.rs, e2e_vex_redirect.rs, e2e_embedded_vex.rs) and the shared vex_e2e_common::run_vex now spawn through tests/common/hermetic.rs, and PENDING_RAW_SPAWNS goes from 92 to 76 on main. CI is green and Bugbot found nothing.

    Left for later slices: vex_e2e_common/uv.rs (its list line is adjacent to lines #1448 removes), and the private POETRY_*/UV_* sweeps, which can move onto hermetic::Extra once #1452 lands Extra::Poetry.

    Generated by Claude Code

  24. mikolalysenko commented on Oct 12, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 12, 2026
    CollaboratorAuthor

    [agent] Claiming slice 8: move the six e2e_redirect_{bun,cargo,composer,gem,maven,npm}_build.rs files on PENDING_RAW_SPAWNS onto tests/common/hermetic.rs. No overlap with #1452 or #1454 (different files, non-adjacent list lines). e2e_redirect_cargo_shapes.rs is left out because #1429 edits it. Branch: agent/fix-ci-hermetic-redirect-build. Claim-ID: 2026-10-12T23:50:54Z-ddb8e9

    Generated by Claude Code

  25. mikolalysenko commented on Oct 13, 2026

    @mikolalysenko
    Mikola Lysenko (mikolalysenko)
    on Oct 13, 2026
    CollaboratorAuthor

    [agent] Slice 8 draft PR: #1457. Narrowed to five files: e2e_redirect_bun_build.rs waits for #1452, which adds the bun tool-env scrub it needs.

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:claimedagent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)priority:p3refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions