[Mikola Lysenko (mikolalysenko)]

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

Fixes #1958

Register row T59 (register); living document Part 5 (tests).

Why

hermetic::command removed the SOCKET_* env but none of the env the CLI's color and width policy reads (ui::color_enabled, ui::width_of). An ambient CLICOLOR_FORCE=1 put ANSI codes into captured stdout. Its HOSTILE_SEEDS were set and then removed by the same list, so a dropped scrub line could never show up in a test.

Changes (test-only)

• tests/common/hermetic.rs
  • Adds COLOR_ENV (NO_COLOR, CLICOLOR, CLICOLOR_FORCE, COLUMNS), which command removes. pty_command inherits this. TERM is left alone because the PTY suites choose their own.
  • Deletes HOSTILE_SEEDS, its seed loop and the false "turns tests red" doc. Deletes the Extra::Yarn/Pnpm/Bundle seed-then-remove pairs and their docs.
  • Keeps by-name removal of SOCKET_UPDATE_BASE_URL/SOCKET_UPDATE_STATE_DIR. They aren't flag-bound, and the deleted seed loop was the only thing removing them by name.
  • The module doc now says that only run_bin_with_env (and suites that call jvm_env::isolate_cli) pin HOME. Moving the pin into command would change HOME for about 150 suites and their package-manager grandchildren. That is a separate, riskier change, so this PR only corrects the doc.
• tests/spawn_env_hygiene.rs
  • Replaces command_never_leaks_its_hostile_seeds and the PTY variant with command_scrubs_hostile_ambient_env and pty_command_scrubs_hostile_ambient_env_and_keeps_caller_env. Both set every GLOBAL_ARG_ENV_VARS/LOCAL_ARG_ENV_VARS name (except the 3 forced opt-outs), the token, the update vars and the color env in the test process, then assert the child never sees any of them.
  • Deletes extra_seeds_are_scrubbed_whatever_the_ambient_value. It could not fail, because the ambient-seeded Extra tests above it already cover the same thing.
• Stale comments: fixed the hostile-seed comments in e2e_golang_build.rs, e2e_vendored_production.rs and vendor/e2e_golang_redirect.rs.

Evidence

• On main @ cbbbdb9, CLICOLOR_FORCE=1 cargo test -p socket-patch-cli --test cli_parse_list fails populated_manifest_plain_lists_full_record_via_binary (68/1). On this branch it passes 69/69, and also with CLICOLOR_FORCE=1 NO_COLOR= COLUMNS=20.
• Red→green: with CLICOLOR_FORCE removed from COLOR_ENV, both canary tests fail with ambient CLICOLOR_FORCE must be scrubbed.
• With CLICOLOR_FORCE=1 COLUMNS=20, the cli, scan, update, rollback and remove suites pass: 654 tests, including the PTY render and prompt tests.
• spawn_env_hygiene passes 19/19. clippy -D warnings is clean on spawn_env_hygiene. e2e_vendored_production's clippy --tests errors are pre-existing on main, and this PR changes only comments there.

Size

+69 / −82 across 5 test files. No production lines change.

Behavior

None in the product. Test children no longer inherit NO_COLOR/CLICOLOR/CLICOLOR_FORCE/COLUMNS, and a test that wants them sets them explicitly, after command.

Risk

Low. The change is test-only and no cfg(windows) code changed.

/code-review high raised 6 findings and all are fixed: the by-name update-var removal, 2 stale comments, the doc reflow, the PTY test's Option round-trip, and the canary now seeding every flag-bound var.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aePX7rzgjWUhucTZmxi

---

Note

Low Risk
Test-only changes to hermetic spawn helpers and hygiene tests; no production code paths modified.

Overview
Hermetic test spawns no longer inherit terminal color/width env (NO_COLOR, CLICOLOR, CLICOLOR_FORCE, COLUMNS), which the CLI uses for output formatting—so ambient values like CLICOLOR_FORCE=1 stop polluting captured stdout assertions.

The old HOSTILE_SEEDS pattern (set hostile values then remove them in the same builder) is removed because it could never fail if a scrub line was dropped. spawn_env_hygiene now seeds the full hostile set in the test process (flag-bound SOCKET_*, token/update vars, color env) and asserts the child never sees them; the same approach replaces the PTY canary. Package-manager Extra scrubs (Yarn/Pnpm/Bundle) drop their seed-then-remove steps; command still removes SOCKET_UPDATE_BASE_URL / SOCKET_UPDATE_STATE_DIR by name. Docs/comments in a few e2e files are updated to match.

Product behavior is unchanged—test harness only (~+69/−82 lines).

^{Reviewed by Cursor Bugbot for commit df7d599. Configure here.}

---

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 df7d599. Configure here.}

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Labeled Ready for review at df7d599.

• CI: 82/82 check runs on df7d599 passed or were skipped (32 success, 50 skipped). The PR is mergeable and doesn't touch CHANGELOG.md.
• Bugbot reviewed df7d599 and found nothing new. No review threads are open.
• For the reviewer: the change is test-only. Hermetic test children no longer inherit NO_COLOR/CLICOLOR/CLICOLOR_FORCE/COLUMNS. spawn_env_hygiene now seeds hostile values in the parent instead of setting and removing them in the same builder.
• Slack: not announced. The Slack send tool wasn't available in this run, so the next run should retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. Test-only. hermetic::command now also strips the CLI's color/width env (NO_COLOR, CLICOLOR, CLICOLOR_FORCE, COLUMNS) from spawned test children, so an ambient CLICOLOR_FORCE=1 can no longer put ANSI codes into captured stdout. It also deletes the HOSTILE_SEEDS set-then-remove pairs, which could never catch a dropped scrub. In their place, canary tests set the hostile values in the test process and assert the child never sees them.

Risk: low. No production code changes. The scrub only gets stricter. The one behavior change is that test children no longer inherit COLUMNS/NO_COLOR, and the PTY and render suites pass in CI.

Look here

• hermetic.rs:40: the new COLOR_ENV list. TERM is deliberately not on it.
• hermetic.rs:63-82: command. The seed loop is gone, and SOCKET_UPDATE_* are still removed by name.
• spawn_env_hygiene.rs:245: hostile_ambient(), which seeds every flag-bound var except the forced opt-outs, plus the token, update and color vars.
• spawn_env_hygiene.rs:265 and :361: the command and PTY canaries. Both are #[serial] and restore the env through with_ambient.

Verified

• I read the full diff (5 test files, +69/−82).
• cargo test -p socket-patch-cli --test spawn_env_hygiene: 19/19 pass.
• CLICOLOR_FORCE=1 COLUMNS=20 cargo test --test cli_parse_list: 69/69 pass. The PR reports that main fails this test.
• clippy -D warnings is clean on spawn_env_hygiene.
• No HOSTILE_SEEDS references remain, and CHANGELOG.md is untouched.
• CI: ci-ok and clippy are green, with 32 checks passing and 50 skipped. Cursor Bugbot passed. There are no unresolved threads.

Changes I made: none.

Open questions: none. Note that command still leaves HOME alone, and the PR explicitly defers that riskier change.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed df7d599bde11417b8194bd6f8534b5626db59b24. Checked color/environment scrubbing and hostile-ambient regression coverage for regular and PTY children. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.