fix(codex): keep mock CLIs shadowed in bash login shells

[dmorosanu]

Problem

Codex issues every shell command as /bin/bash -lc. The login shell re-sources the system profile chain, which resets PATH and silently drops the env_path_prepend mock-CLI prepend - so uip resolves to the real CLI and mocked eval tasks investigate the real tenant instead of the fixtures. In the 2026-07-15 gpt-5.4 weekly run, 5 of 16 uipath-troubleshoot failures were this contamination (transcripts cite real-tenant jobs absent from all fixtures, while the mocked evidence was never served).

Fix

When mocks are configured (POSIX only), CodexAgent.start() generates a per-task HOME containing a .bash_profile/.profile that:

1. exports the ORIGINAL home back (bash uses env HOME only to pick the profile file; git config, tool caches, and $HOME-relative sourcing keep working),
2. sources the original home's own profile chain,
3. re-prepends the mock dirs to PATH - last word after any reset.

The app-server env points HOME at the generated dir and pins CODEX_HOME to the real codex state dir (auth and sub-agent rollout recovery unaffected). Cleaned up on stop()/kill_sync(), exception-safe, rerunnable across start() retries.

Known residual gap: a nested login shell inside a command re-reads the real profiles and loses the prepend again (documented in the docstring).

Validation

• Real bash -lc + the real skills-repo mock dispatcher in WSL Ubuntu 24.04: pre-fix resolves the real CLI, post-fix returns the scenario's manifest fixture.
• Hermetic end-to-end test: mock dir first on PATH, $HOME restored, original $HOME/.bashrc sourced - even when the original profile resets PATH.
• 91/91 tests pass on Linux, 90 + 1 posix-skip on Windows; ruff + pyright clean; no regressions in the full suite.

[github-actions]

Claude finished @dmorosanu's task in 1m 22s —— View job

---

Code Review in Progress

• Read review guidelines (.github/code_review.md)
• Read project conventions (CLAUDE.md)
• Analyze the full diff (git diff origin/main...HEAD)
• Review src/coder_eval/agents/codex_agent.py changes
• Review tests/test_codex_agent.py changes
• Check for cross-file consistency
• Perform "what's missing" analysis
• Design-level scrutiny
• Post comprehensive review feedback

[uipreliga]

Review: coder_eval — pr:26

Scope: pr:26 · branch fix/codex-harness-shim-exec · b802957 · 2026-07-16T19:04Z · workflow variant

Change class: complex — introduces login-shell HOME shimming (temp-dir lifecycle, generated shell-profile files, PATH re-prepend) whose correctness depends on shell sourcing order, $HOME semantics, and shlex quoting

coder_eval is in exceptional health — a perfect 10/10 with clean type safety, tests, security, architecture, and harness scoring, and the only blemishes are two low-severity maintainability nits in the Codex agent that pose no correctness, security, or scoring risk, so the bottom line is ship-as-is while tidying the two codex_agent.py items opportunistically.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	9.9 / 10	0	0	0	1	Implicit magic-string coupling between _LOGIN_PROFILE_NAMES tuple and the ".bash_profile" literal branch
2. Type Safety	10 / 10	0	0	0	0	—
3. Test Health	10 / 10	0	0	0	0	—
4. Security	10 / 10	0	0	0	0	—
5. Architecture & Design	10 / 10	0	0	0	0	—
6. Error Handling & Resilience	10 / 10	0	0	0	0	—
7. API Surface & Maintainability	9.9 / 10	0	0	0	1	_build_codex_env docstring omits the HOME/CODEX_HOME override it now performs
8. Evaluation Harness Quality	10 / 10	0	0	0	0	—

Overall Score: 10 / 10 · Weakest Axis: Code Quality & Style at 9.9 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 0 · 🔵 2 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

None.

Nits

1. [Axis 1] Implicit magic-string coupling between _LOGIN_PROFILE_NAMES tuple and the ".bash_profile" literal branch (src/coder_eval/agents/codex_agent.py:1168) — The set of generated profile files is declared once as _LOGIN_PROFILE_NAMES = (".bash_profile", ".profile") (line 223) and driven through the loop for name in _LOGIN_PROFILE_NAMES: (line 1135), but _login_profile_content distinguishes the two kinds by re-matching the literal string: if profile_name == ".bash_profile": (line 1168), with everything else falling into the sh/.profile else. This is a second source of truth: adding e.g. .zprofile to the tuple would silently route it through the POSIX .profile branch (which sources only .profile, not the bash first-found chain), with no error. Minor for two files, but the bash-vs-POSIX distinction would be clearer as an explicit flag/parameter (e.g. pass is_bash=name == ".bash_profile" at the call site, or key a small mapping) rather than re-deriving it from the filename inside the content builder.
2. [Axis 7] _build_codex_env docstring omits the HOME/CODEX_HOME override it now performs (src/coder_eval/agents/codex_agent.py:1063) — The method-level docstring at lines 1063-1074 still enumerates only the two env vars the method used to set: it says the env "Carries the API key (CODEX_API_KEY...) and, when the sandbox resolved mock_path_dirs, a PATH with those directories prepended..." — but the method now also injects env["HOME"] = str(self._login_shell_home) and env["CODEX_HOME"] = str(self._codex_home()) (lines 1090-1091). Overriding HOME is the single most surprising side effect in this diff: it applies to the whole Codex app-server process (not just the bash -lc shells the design targets), which is a maintainability/discoverability trap for a future reader who reads the docstring as the method's contract. The behavior IS explained by the inline comment at lines 1086-1089, but the docstring — the first thing a maintainer reads — is now stale/incomplete. Add one sentence to the docstring noting that HOME is repointed at the per-task login-shell profile dir (with CODEX_HOME pinned back to the real codex state dir so auth/rollouts are unaffected), cross-referencing _setup_login_shell_home.

What's Missing

Parallel paths:

• 🟡 The login-shell PATH-restore guard lives only in CodexAgent, but ClaudeCodeAgent (claude_code_agent.py:706,1062) and antigravity_agent (antigravity_agent.py:314,378-396) consume the SAME env_path_prepend mock-PATH prepend the sandbox hands every agent. If either spawns a login shell (bash -lc or any -l), the identical /etc/profile PATH reset drops the mock prepend and bare commands resolve to the REAL CLIs (real-tenant contamination) with no error. The PR should state that only Codex issues login shells, or the guard belongs on the shared Agent base rather than one agent. (trigger: src/coder_eval/agents/codex_agent.py)

Daily/nightly:

• 🟡 The fix exists for the containerized nightly Codex path (mock CLIs shadowing real-tenant CLIs) and hard-codes container-shell assumptions: that /etc/profile resets PATH and that ~/.bash_profile is sourced AFTER it. If the nightly base image's /etc/profile or default login shell differs, the shim silently no-ops and contamination returns with no failure signal. The PR does not state whether the nightly container image must be rebuilt or validated against these assumptions before the fix is relied on. (trigger: src/coder_eval/agents/codex_agent.py)
• 🔵 _build_codex_env repoints HOME for the ENTIRE codex app-server process in the nightly container, not just the bash -lc shells the design targets; any $HOME-relative behavior of the codex binary itself now resolves to the ephemeral temp dir (only CODEX_HOME is pinned back). The whole-process blast radius of this override is undocumented and shares root cause with the Axis 7 docstring-omission finding. (trigger: src/coder_eval/agents/codex_agent.py) _(restates: Axis 7: build_codex_env docstring omits the HOME/CODEX_HOME override)

Tests:

• 🔵 The only real-shell end-to-end test (test_login_shell_restores_mock_prepend_end_to_end) exercises bash -lc exclusively; the generated .profile twin (read by sh/dash login shells) is asserted at file-content level only and never executed by a real sh/dash login shell, so a sourcing bug or accidental bashism specific to the sh path would pass unit tests. (trigger: tests/test_codex_agent.py)

Harness & Lint Improvements

Static checks (lint / type):

• (none)

Harness improvements (not statically reachable):

• Add a parametrized unit test over _LOGIN_PROFILE_NAMES in tests/ that, for every entry in the tuple, invokes _login_profile_content(name) and asserts the generated content matches that name's expected shell semantics — the bash first-found chain (.bash_profile/.bashrc) vs. the POSIX .profile sourcing — rather than asserting only that content is non-empty. This makes the tuple↔content-builder coupling self-checking: adding e.g. .zprofile to _LOGIN_PROFILE_NAMES without teaching _login_profile_content about it (so it silently falls into the POSIX else branch) trips a failing assertion instead of shipping wrong login-shell wiring. Ideally the test also asserts _login_profile_content has an explicit branch/mapping entry for each name (no unhandled name falls through to a generic default). Why not static: The correct content for a given profile filename is a semantic property of shell login-file sourcing behavior (which chain a shell reads, in what order), not a grep-/AST-detectable pattern. A lint rule can't judge whether the else branch is right for a newly added filename; only executing the content builder and checking the emitted script against expected sourcing semantics can. And the underlying 'literal re-derived instead of parameterized' shape is exactly the kind of second-source-of-truth design judgment a mechanical rule would either miss or over-flag. Prevents: Finding 1 (implicit magic-string coupling between _LOGIN_PROFILE_NAMES and the .bash_profile literal branch in _login_profile_content) — the silent mis-routing of a future profile name through the wrong shell branch.

Top 5 Priority Actions

1. None of the confirmed findings can change a task's score or final_status for identical agent output, so there are no scoring-integrity fixes to front-load — proceed in pure impact order below.
2. Remove the second source of truth in src/coder_eval/agents/codex_agent.py:1168 by passing an explicit is_bash flag (or keying a small mapping) from the _LOGIN_PROFILE_NAMES loop into _login_profile_content, so adding a profile like .zprofile can't silently fall through the POSIX .profile branch.
3. Update the _build_codex_env docstring at src/coder_eval/agents/codex_agent.py:1063 to document the new HOME/CODEX_HOME repointing side effect (HOME → per-task login-shell profile dir, CODEX_HOME pinned to the real state dir), cross-referencing _setup_login_shell_home, since it is the most surprising behavior in the diff and the docstring is a maintainer's first contract.
4. Convert the magic-string coupling into permanent enforcement by adding a custom lint rule (tests/lint/rules/, CE-series) that flags re-deriving branch behavior from a literal already enumerated in a canonical tuple, per CLAUDE.md's 'could a lint rule have prevented this?' guidance.
5. Hold the line on the perfect axes by keeping the merge-unification, reconciliation-invariant, and classification-aggregate test guards green as the Codex/login-shell surface evolves, treating any new drift between docstrings and behavior as a regression.

---

Stats: 0 🔴 · 0 🟠 · 0 🟡 · 2 🔵 across 8 axes reviewed.

[bai-uipath]

Approve — correct, well-contained, and now validated end-to-end in the real nightly container. Notes below are optional.

What we verified

• Unit + CI: all 15 login-shell tests pass (including the real-bash -lc e2e that simulates the /etc/profile reset), and CI's "Live Integration Tests (Codex)" is green on Linux.
• Real-environment e2e: ran the actual PR-branch CodexAgent inside skills-image:latest — the container the nightly/weekly codex runs actually use — against its real /etc/profile, with and without the fix.

Findings

• ✅ The bug reproduces in-container. In skills-image, a login shell resets PATH to the literal /usr/local/sbin:…:/bin and drops the mock-CLI prepend, so a bare uip resolves to the baked-in real /usr/bin/uip. That is exactly the real-tenant contamination the PR targets, confirmed in the true environment (macOS couldn't reproduce it — zsh + non-destructive path_helper; the VM host couldn't either — its profile augments rather than resets PATH).
• ✅ The fix restores the prepend. With _setup_login_shell_home + _build_codex_env composed the way the SDK merges the env over os.environ, a bare uip resolves to the mock and $HOME is correctly restored to /root. Together with the unit test that asserts start() folds HOME/CODEX_HOME/PATH into the CodexConfig env, the full chain is covered.
• ✅ Blast radius is contained. The HOME/CODEX_HOME redirect only activates when _env_path_prepend is set (mocked tasks) on POSIX — non-mocked codex tasks and the claude/antigravity agents are untouched (codex_agent.py:1085).
• ✅ CODEX_HOME pin is right. Redirecting HOME would otherwise move codex auth/rollout state into the temp dir; pinning it to _codex_home() (resolved in the parent's real env) keeps auth and sub-agent rollout recovery working.