fix(codex): recover v2 sub-agent tool calls

[costin-uipath]

With Codex multi-agent v2, the commands a sub-agent runs are missing from the run record. The spawn no longer arrives as a collabAgentToolCall that names the child thread, so the harness registers no child and never reads the child's rollout. A criterion that checks a command the sub-agent ran then fails, even though the command ran.

Changes

• Register the child from subAgentActivity. A v2 spawn is a subAgentActivity item with kind: started. Its id is the spawn call id and agent_thread_id is the child thread. The item now becomes the Agent spawn row (status success) and registers the child for rollout recovery. In v2 the wait call names no child. The other kinds (completed, interrupted, interacted) are dropped: they are not tool calls and do not open a model call.
• Read shell commands from v2 exec scripts. A v2 child rollout records each shell call as a custom tool exec whose input is JavaScript (tools.exec_command({"cmd": …}) or tools.shell_command({command: …})). The literal commands are read out of the script and recorded as Bash rows. A command built from a variable keeps the whole script as the command text, so it still matches as text.
• Record failed calls as errors. Script failed (v2 exec output) and a non-zero Exit code: N (v1 shell output) now mark the recovered row as an error. Before, these rows were recorded as success.
• Map the v1 shell_command function call to Bash. It was missing from the rollout tool-name map.
• Skip forked parent history. A spawn with fork_turns copies the parent's history, tool calls included, into the head of the child's rollout. In a forked rollout (session_meta carries forked_from_id), parsing starts at the child's own thread_settings_applied, so the parent's commands are not credited to the child. A non-forked rollout is parsed whole, because that event also appears mid-thread on a settings change.
• Notes. .claude/notes/agents.md § Codex rollout rebuild.

Validation

Real Codex v2 run (CLI 0.162, one parent and one spawned child):

Check	Result
started item id	equals the spawn_agent call id
wait item	empty receiver_thread_ids and agents_states
Child rollout	found as rollout-*-<agent_thread_id>.jsonl; forked from the parent
_parse_rollout_generations on the child rollout	exactly the child's 3 shell commands, 1 success and 2 failed, as in the rollout

Older non-forked rollouts (CLI 0.144–0.147) have thread_settings_applied without a thread_id: the boundary stays at 0, and all 96 calls in a sample rollout are parsed. The pinned CLI 0.156.1 binary contains SubAgentActivity, thread_settings_applied and forked_from_id.

Tests	Result
test_codex_agent.py	151 passed, 2 skipped (they need a POSIX bash or zsh)
ruff format and check, pyright on codex_agent.py, docs budget	pass
Full suite (Windows, local)	6,114 passed, 90.84% coverage. 18 failures, none in code this PR touches: 10 in test_judge_litellm.py (litellm not installed) and 8 symlink tests (WinError 1314, no symlink privilege)
CE lint rules (local)	734 passed; CE033 test_drift_is_detected fails on a Windows encoding error (0x97), not related to this change

Not run yet: an end-to-end run with the pinned CLI 0.156.1 in dialog mode (migrate_with_fixes_e2e).

Known limits

• The subAgentActivity item has no model field, so the child's messages carry the parent's model.
• On a CLI that writes thread_settings_applied without a thread_id, a forked child rollout is parsed whole. That can duplicate parent commands, but never drops child commands.

🤖 Generated with Claude Code

[github-actions]

Claude finished @costin-uipath's task in 1m 50s —— View job

---

Code Review in Progress

Todo List:

• Read .github/code_review.md for review standards
• Read CLAUDE.md project conventions
• Review git diff origin/main...HEAD
• Read full context for each changed file
• Perform cross-file consistency checks
• Conduct "what's missing" analysis
• Provide design-level scrutiny
• Post comprehensive review

[uipreliga]

Review: coder_eval — pr:227 (3 files) axis:1,2,3,4,5,6,7,8

Scope: pr:227 (3 files) axis:1,2,3,4,5,6,7,8 · branch fix/codex-subagent-activity-spawn (pr-227) · ca3ddd7 · 2026-10-16T17:31Z · workflow variant

Change class: complex — new control flow and a hand-written JavaScript string/brace parser that changes which sub-agent tool calls (and which statuses) land in the Codex run record

The codebase is healthy (9.3/10, with clean architecture and almost no security risk), but the new Codex sub-agent exec-script recovery in codex_agent.py can change command_executed and commands_efficiency results for identical agent output: it records failed commands as success, records non-shell scripts as Bash rows, and collapses or silently drops commands, so fix this recovery path before you trust Codex multi-agent scores.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	8.3 / 10	0	0	3	2	Hand-written JavaScript lexer added to the already large codex_agent.py
2. Type Safety	9.7 / 10	0	0	0	3	subAgentActivity test fake uses a SimpleNamespace with a str kind instead of the SDK enum or model
3. Test Health	8.9 / 10	0	0	2	1	Command-key regex matches text inside earlier string values, so the real exec command is lost (untested)
4. Security	9.9 / 10	0	0	0	1	Exec-script parser is quadratic on unbalanced input and runs synchronously on the event loop
5. Architecture & Design	10 / 10	0	0	0	0	—
6. Error Handling & Resilience	8.9 / 10	0	1	0	1	v2 exec row marked success when a shell command inside the script exits non-zero
7. API Surface & Maintainability	9.4 / 10	0	0	1	1	A multi-command exec script becomes one Bash row, which breaks per-command telemetry and require_success granularity
8. Evaluation Harness Quality	8.9 / 10	0	1	0	1	Every exec script becomes a Bash row, so non-shell scripts (apply_patch, update_plan, view_image) record their whole source as an executed shell command

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

Blockers

1. [Axis 6] v2 exec row marked success when a shell command inside the script exits non-zero (src/coder_eval/agents/codex_agent.py:2298) — Offending line: is_error = text.lstrip().startswith("Script failed") (line 2298). The Exit code: (\d+) branch (lines 2300-2301) catches only the v1 shell_command prefix. In a code-mode exec script, tools.exec_command(...) RETURNS the exit code and does not throw it. A failing command therefore produces Script completed.

I confirmed this against a real local rollout (~/.codex/sessions/2026/10/10/rollout-...-01a128d3-28a1-78e3-af0a-c163f0debe0d.jsonl). The input is const r = await tools.exec_command({cmd:"uip insights alert-history get-metrics ... > metrics.json", ...}); text(r.output); text(exit_code=${r.exit_code}); and the output is 'Script completed\nWall time 0.1 seconds\nOutput:\nexit_code=1'. On the PR worktree, CodexAgent._subagent_output({...that output...}) returns (..., False). _subagent_generation_blocks (line 2334) then maps it to result_status="success". criteria/command_executed.py:193 (if criterion.require_success and cmd.result_status != "success") counts the failed command as a success. For identical agent output, a require_success: true criterion can pass on a command that failed.

The new note says the opposite: .claude/notes/agents.md:413-414 says "Both outputs are read so a failed call is not recorded as a success." With Promise.all, one exec row carries N commands and one status, so a single failed command is also hidden.

The parallel unified exec_command string output is not handled either. Its real format is 'Chunk ID: ...\nWall time: ...\nProcess exited with code 2\n...', and _subagent_output returns False for it. aborted by user outputs are also recorded as success.

Fix: also mark the call as an error when the script body shows a non-zero exit. When that cannot be determined (multi-command scripts, a variable command), use a status other than success, such as "unknown", so require_success does not credit it. Add a (?m)^Process exited with code (\d+) check beside the Exit code: check. Correct the note at agents.md:413-414. Add a test that uses the real Script completed ... exit_code=1 shape.
2. [Axis 8] Every exec script becomes a Bash row, so non-shell scripts (apply_patch, update_plan, view_image) record their whole source as an executed shell command (src/coder_eval/agents/codex_agent.py:147) — Scoring-correctness. _ROLLOUT_FN_NAMES maps "exec": "Bash" with no condition (line 147). _subagent_parameters then falls back to return {"command": "\n".join(commands) if commands else script} (line 2265), so a script that makes NO shell call becomes a Bash row whose command is the JavaScript source. I checked this: an exec script await tools.apply_patch("*** Begin Patch\n*** Add File: deploy.sh\n+git push --force origin main\n*** End Patch") gives Bash {'command': 'await tools.apply_patch(... git push --force origin main ...)'}. Results: (1) a command_executed with tool_name: Bash + command_pattern: git push --force + max_count: 0 fails on a FILE WRITE that ran nothing. Before this PR the row was tool_name exec, so the Bash filter skipped it. (2) tool_name: Bash min_count / commands_efficiency counts grow by one for each update_plan/view_image script. The PR's own test test_other_tools_are_ignored shows these scripts exist. Fix: map exec to Bash only when _exec_shell_commands finds a shell call (or the _EXEC_SHELL_CALL regex matches). Otherwise keep the tool name exec with {"input": script}. Use the whole-script fallback only for a script that calls the shell tool with a non-literal command.

Non-blocking, but please consider before merge

1. [Axis 1] Hand-written JavaScript lexer added to the already large codex_agent.py (src/coder_eval/agents/codex_agent.py:292) — The PR adds about 80 lines of JS lexing: _js_string_at (L234), _matching_brace (L266), _exec_shell_commands (L292), _JS_ESCAPES (L159), and the regexes _EXEC_SHELL_CALL / _EXEC_COMMAND_KEY (L156/L158). The code finds each shell command in an exec script as a separate item. Then _subagent_parameters joins the list back into one string at L2265: return {"command": "\n".join(commands) if commands else script}. A Promise.all of N commands therefore still gives ONE Bash CommandTelemetry row. The per-command structure the lexer recovers does not reach min_count/max_count in command_executed or commands_efficiency. The fallback already records the whole script, and the PR's own note says it "still matches as text" for the usual unanchored command_pattern search. So most of what the lexer adds is display text and support for anchored patterns. The lexer is also hard to follow: L252-255 with contextlib.suppress(ValueError): out.append(chr(int(...))); j += 6; continue uses the suppress block to skip the continue, so a bad \u escape quietly appends u. Pick ONE of two options. (a) Use the list as intended: emit one telemetry row per command found, with a call_id suffix per command, so counts and efficiency are correct. (b) Delete the lexer and record the script as written. Do not keep the find-then-rejoin middle path.
2. [Axis 1] subAgentActivity is special-cased at 6 sites in 5 functions, which raises CC in each of them (src/coder_eval/agents/codex_agent.py:736) — The PR adds a root_type == _SUB_AGENT_ACTIVITY_ITEM branch, or the _is_subagent_lifecycle_notice filter, at 6 sites. The filter is at L706 and L743 (if _is_subagent_lifecycle_notice(root, root_type): return). The branches are at L797 (elif root_type == _SUB_AGENT_ACTIVITY_ITEM:), L1867 in _tool_parameters, L1938 in _extract_generic_telemetry (if root_type == _SUB_AGENT_ACTIVITY_ITEM and not status_str: status_str = "completed"), and L1974 in _summarize_tool_item. radon (main vs PR): on_item_completed D24 to D26, _tool_parameters C17 to C20, _subagent_parameters C11 to C14, _summarize_tool_item C11 to C13, _subagent_output B6 to C11, _extract_generic_telemetry B8 to B10, and _fork_boundary C11 (new). A v2 started item means the same thing as a v1 spawnAgent collab call. Convert it ONCE where items enter, before dispatch: drop notices, and map started to the spawn shape (operation, receiver thread, status). Then each per-type extractor does not need a new branch for every new Codex item kind. In _extract_generic_telemetry, the hardcoded "completed" status default for one item type is the magic-string special-case pattern.
3. [Axis 1] v1 and v2 child-registration logic are duplicated and have drifted (dedupe guard and model source on one side only) (src/coder_eval/agents/codex_agent.py:2034) — _handle_subagent_spawn (L2034-2037: if thread_id in spawn_by_thread: return / spawn_by_thread[thread_id] = tool_id / spawned_children.append((thread_id, tool_id, None))) does the same job as the v1 writer in _handle_collab_completion (L2009-2011: spawn_by_thread[thread_id] = tool_id / spawned_children.append((str(thread_id), tool_id, model))). Only the new copy has the duplicate guard. Suppose a v1 collab spawn names a child that a started item already registered. The v1 path appends it a second time, and the rollout is processed twice. That is the token and command doubling that test_v2_repeated_started_notice_registers_the_child_once guards against on the v2 side only. Also, the new method does not use self, and its only job is to change three _CodexTurnState fields that are passed in. Move to one _register_child(thread_id, tool_id, model) that has the guard, preferably on _CodexTurnState, and call it from both paths.
4. [Axis 3] Command-key regex matches text inside earlier string values, so the real exec command is lost (untested) (src/coder_eval/agents/codex_agent.py:304) — _EXEC_COMMAND_KEY (line 158: re.compile(r"""(?:["'](?:command|cmd)["']|\b(?:command|cmd))\s*:\s*""")) runs on the raw object-literal text at line 304 (key = _EXEC_COMMAND_KEY.search(body)). It does not skip string literals, so it can match text inside a value. I ran this on the PR tree: _exec_shell_commands('await Promise.all([tools.shell_command({justification:"re-run the command: tests", command:"pytest -q"}), tools.shell_command({command:"ls"})])') returns ['ls']. The pytest -q call is lost without any signal. Because another command was found, _subagent_parameters does not fall back to the whole script, so pytest is in no recovered row and a command_executed criterion for it does not match. None of the seven tests in TestExecScriptCommandExtraction has a key-like token inside a string. Add a regression test with this input that expects ['pytest -q', 'ls']. Then make the key search skip string literals: walk top-level keys with _js_string_at, as _matching_brace already does. Cross-axis: this is also a correctness and harness-scoring defect (axes 1 and 8).
5. [Axis 3] No test checks that a failed recovered sub-agent call ends up as result_status 'error' (only the success path is tested end to end) (tests/test_codex_agent.py:1447) — The PR's stated contract (.claude/notes/agents.md: "Both outputs are read so a failed call is not recorded as a success") is checked only at the helper level: _subagent_output returns True for 'Script failed' and for a non-zero Exit code:. The recovery test that runs the full path asserts only assert bash[0].result_status == "success" (line 1447). No test runs a child rollout whose exec output starts with Script failed, or whose shell_command output is Exit code: 1, through _parse_rollout_generations and _subagent_generation_blocks (result_status="error" if call["is_error"] else "success", codex_agent.py:2334). Then asserts result_status == "error" and the is_error flag on the tool_use block. This is the branch that decides whether command_executed(require_success=true) matches the call, so it is the gate that changes the score. Add a variant of test_v2_started_activity_is_the_spawn_and_feeds_recovery with a Script failed output part, plus a v1 Exit code: 1 row. Assert the error status on the recovered CommandTelemetry.
6. [Axis 7] A multi-command exec script becomes one Bash row, which breaks per-command telemetry and require_success granularity (src/coder_eval/agents/codex_agent.py:147) — Line 147 "exec": "Bash", labels every exec call as Bash. Lines 2262-2265 build its parameters: if payload.get("name") == "exec" ...: commands = _exec_shell_commands(script); return {"command": "\n".join(commands) if commands else script}. Downstream consumers read CommandTelemetry as one shell command per row. Examples: criteria/command_executed.py:199-200 (if cmd.tool_name == "Bash" and isinstance(raw_command, str)) counts one match per row; evaluation/summaries.py:33 and evaluation/judge_context.py:479 render it as one command. I ran the PR code to confirm three problems. (a) A Promise.all of pytest a and pytest b gives {'command': 'pytest a\npytest b'}. That is one row, so min_count: 2 scores 0.5. Also, the row has one Script failed flag, so require_success drops every command in the script when one of them fails. (b) A script that calls only non-shell tools gives a Bash row with the JavaScript as the command. For example, await tools.apply_patch({input: "*** Begin Patch"}); gives Bash {'command': 'await tools.apply_patch(...)'}, and test_other_tools_are_ignored shows that update_plan/view_image scripts take the same path. Shell-anchored patterns and max_count exclusions then see JavaScript text as if the agent ran it. (c) The v1 shell_command path and the parent commandExecution path give one row per command, so the same agent behaviour counts differently on different Codex CLI versions. Fix: emit one CommandTelemetry per extracted command, with ids such as <call_id>#<n>. Use the exec tool name only when the script holds no shell call, and do not label that row Bash, so it falls to the JSON (is_shell=False) haystack. Add a test that a two-command Promise.all gives two rows. Cross-axis: this is also an Axis 8 scoring-correctness concern.

Nits

1. [Axis 1] Magic strings are inconsistent: a constant is defined but the literal is still used, and the regex style is mixed (src/coder_eval/agents/codex_agent.py:120) — L120 "subAgentActivity": "Agent", uses the literal, but _SUB_AGENT_ACTIVITY_ITEM = "subAgentActivity" is defined at L135 and used everywhere else. L2262 if payload.get("name") == "exec" repeats the "exec": "Bash" key from _ROLLOUT_FN_NAMES (L147). L2300 re.match(r"Exit code: (\d+)", output) is compiled inline, but the sibling patterns are module constants (L156/L158). L2298 "Script failed" is a bare sentinel. Use the constant, or a named module constant, for each one, so the next Codex vocabulary change has one place to edit.
2. [Axis 1] Seven almost identical exec-extraction tests, each with its own local import (tests/test_codex_agent.py:1076) — Seven tests in TestExecScriptCommandExtraction repeat from coder_eval.agents.codex_agent import _exec_shell_commands (L1076, 1085, 1094, 1105, 1111, 1117, 1125), and each tests only script -> expected list. The module already has a top-level from coder_eval.agents.codex_agent import (...) block at L18. Move the import there, and merge the seven tests into one @pytest.mark.parametrize("script, expected", [...]) test with ids.
3. [Axis 2] subAgentActivity test fake uses a SimpleNamespace with a str kind instead of the SDK enum or model (tests/test_codex_agent.py:1192) — The new helper _sub_agent_activity(kind: str, ...) -> SimpleNamespace (lines 1192-1205) builds the item as SimpleNamespace(type="subAgentActivity", id=item_id, kind=kind, agent_thread_id=child_thread, agent_path=agent_path). The installed SDK has a typed model for this item: openai_codex.generated.v2_all.SubAgentActivityThreadItem, with kind: SubAgentActivityKind (an Enum), agent_thread_id: str and agent_path: str. A SimpleNamespace accepts any attribute name, so if the SDK renames agent_thread_id or agent_path, every v2 test still passes while production getattr(root, "agent_thread_id", None) silently returns None and recovers nothing (rubric criterion 17). Production works today: I checked that _is_subagent_lifecycle_notice returns False for a real started item and True for a real completed item, because _status_value unwraps .value. The test file already imports from openai_codex.generated.v2_all (line 504), so build the fake as SubAgentActivityThreadItem(id=item_id, kind=kind, agent_thread_id=child_thread, agent_path=agent_path, type="subAgentActivity") and type kind as SubAgentActivityKind | str. Then field drift fails at construction time.
4. [Axis 2] New _fork_boundary relies on a list[dict[str, Any]] annotation that is not true for unvalidated json.loads output (src/coder_eval/agents/codex_agent.py:2210) — def _fork_boundary(objs: list[dict[str, Any]]) -> int: calls o.get("type") / (o.get("payload") or {}) and then .get on every element. The list comes from objs: list[dict[str, Any]] = [] / objs.append(json.loads(raw)) (lines 2140-2146). json.loads returns Any, so the annotation is never checked. A valid JSON line that is not an object, or a payload that is not a dict, raises AttributeError. The broad except Exception in _recover_subagent_tool_calls swallows it, and the child loses all of its recovered calls. The annotation pre-dates this PR, but the PR adds one more consumer that trusts it (objs = objs[cls._fork_boundary(objs) :], line 2149). To make the type true, narrow at the append site: if isinstance(obj := json.loads(raw), dict): objs.append(obj).
5. [Axis 2] isinstance narrowing on payload.get("input") is lost because the value is read again by subscript (src/coder_eval/agents/codex_agent.py:2262) — if payload.get("name") == "exec" and isinstance(payload.get("input"), str): is followed by script = payload["input"]. Pyright cannot carry the isinstance check from .get(...) to a second payload["input"] lookup, so script is Any when it goes into _exec_shell_commands(script: str) and the str contract is not checked. Bind and narrow in one step: if payload.get("name") == "exec" and isinstance(script := payload.get("input"), str):.
6. [Axis 3] New hand-written JS-parser branches and v2 spawn edge branches have no tests (src/coder_eval/agents/codex_agent.py:252) — The coverage run on the PR tree (153 passed, codex_agent.py 83.12%) shows these new PR lines are not covered: 252-255 (if esc == "u" and j + 5 < len(text): with out.append(chr(int(text[j + 2 : j + 6], 16))), the documented \uXXXX decode), 263 (unterminated literal return None), 279 and 286->288/289 (the _matching_brace unbalanced fallback, whose docstring promises "a truncated script still yields whatever it holds"), 302 (if brace < 0: continue), 306 (if key is None: continue), 2032 (if not thread_id: return in _handle_subagent_spawn), and 1873->1875 (a started item without agent_path). Also, the interacted notice kind is not exercised; only completed and interrupted are. The notices-only test (tests line 1453) puts _delta("done") in the same turn, so it cannot detect a notice that opens a parent model call, although the notes promise this does not happen. Add parametrized cases to TestExecScriptCommandExtraction: a é escape, a truncated {command:"ls", workdir:"x (expects ['ls']), an unterminated {command:"ls -la (expects []), tools.shell_command(obj), and a call with no command key. Also add a notices-only turn with no delta that asserts no assistant message or API call is recorded.
7. [Axis 4] Exec-script parser is quadratic on unbalanced input and runs synchronously on the event loop (src/coder_eval/agents/codex_agent.py:303) — For each _EXEC_SHELL_CALL match, _exec_shell_commands (line 299 for call in _EXEC_SHELL_CALL.finditer(script):) calls body = script[brace : _matching_brace(script, brace)] (line 303). When the brace is unbalanced, _matching_brace (line 266) scans to len(text). For K calls in a script of length L this costs O(K*L). The script is the input of a sub-agent exec custom tool call, so the agent under test writes it. I measured it on the PR worktree: 'tools.exec_command({' * n takes 0.9 s at 20 KB, 3.6 s at 40 KB and 14.3 s at 80 KB, which is quadratic. A benign 128 KB script with 4000 calls takes 0.01 s. The parse runs in _parse_rollout_generations (line 2075 gens = self._parse_rollout_generations(path)) inside the async _recover_subagent_tool_calls, with no asyncio.to_thread. While it runs it blocks the event loop, so asyncio turn timeouts cannot fire, and other tasks on the same loop stall. Impact is limited to transient availability of the run, and only a pathological or adversarial script triggers it, so this is Low. Fix: bound the cost. Option 1: when _matching_brace returns len(text) (unbalanced), stop the loop and do not scan again from later matches. Option 2: limit the body window to a fixed size, for example script[brace : brace + 4096], the same way command_executed limits its search window with _MAX_PATTERN_SEARCH_LEN. Also consider moving the rollout parse off the event loop with asyncio.to_thread. CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:N/A:L
8. [Axis 6] _fork_boundary silently falls back to the whole rollout when there is no thread_settings_applied marker, and this path is untested (src/coder_eval/agents/codex_agent.py:2232) — Offending line: the final return 0 (line 2232) is reached after if not own or not meta.get("forked_from_id"): (line 2222) has already shown that the rollout IS a fork. If Codex renames or drops the thread_settings_applied event (format drift), the function returns 0 with no diagnostic. The rollout is then replayed whole, and the parent's copied commands become child rows again. This is the defect the PR fixes, and it would come back with no signal. Fix: keep the graceful fallback, but add self._log/module-logger debug or warning output that names the thread id when forked_from_id is set and no boundary is found. The drift is then visible in the logs and not silently masked.
9. [Axis 7] The design note says a variable-built command falls back to the whole script. The code drops that command silently when the script also holds a literal command. (.claude/notes/agents.md:412) — Notes lines 412-413 say: "A command built from a variable falls back to the whole script as the command text, which still matches as text." The code only falls back when no literal command exists (codex_agent.py:2265: "\n".join(commands) if commands else script). _exec_shell_commands (line 292) skips any call whose command is not a literal. I ran const c = "rm -rf build"; await tools.exec_command({"cmd": "ls"}); await tools.exec_command({cmd: c}); against the PR code. It returns ['ls'] and {'command': 'ls'}, so the rm -rf build call leaves no trace, and a max_count: 0 forbidden-command criterion cannot see it. Change the note to describe the real behaviour, or (better) make the code match the note: when any shell call's command is not a literal, keep the whole script in the row as well. Add a test for the mixed literal-plus-variable script. No existing test covers it: only the all-variable case test_rollout_exec_call_without_a_literal_keeps_the_script_as_command_text is tested.
10. [Axis 8] The Codex sub-agent run-record content changes (exec->Bash rename, new spawn rows, error status) with no note for cross-run consumers (.claude/notes/agents.md:398) — The task.json schema does not change (same CommandTelemetry fields), so the nightly does not break and no image or schema needs a lockstep rebuild. The CONTENT does change for Codex multi-agent rows. v1-child exec rows were tool_name: "exec" with {"input": ...} and are now Bash with {"command": ...}. v2 runs get new Agent rows (operation: spawnagent) and recovered child rows. v1 Exit code: N outputs with N != 0 are now error, not success. The coder-eval-uipath dashboard compares tool counts and command_executed/commands_efficiency scores across runs, so it will show a step change for Codex tasks at this version. That is a real baseline shift, not a regression of the agent. Add a release note / CHANGELOG line that names the affected criteria, so trend breaks are not read as model changes. The notes section added at line 398 records the mechanism but not the effect on grading.

What's Missing

Downstream consumers:

• 🟠 command_executed(require_success: true) reads result_status as the outcome of each command. The new v2 exec rows get that status from _subagent_output, which detects only the Script failed and v1 Exit code: N prefixes. A script that completes with a non-zero exit_code, or a unified exec_command output Process exited with code N, stays success. The grader then credits a failed command, and the note at agents.md:413-414 claims the opposite. (trigger: src/coder_eval/agents/codex_agent.py) (restates: Axis 6: v2 exec row marked success when a shell command inside the script exits non-zero)
• 🟠 command_executed (tool_name: Bash, min_count/max_count) and commands_efficiency count one shell command per CommandTelemetry row. The PR maps every exec to Bash and joins N commands into one row. A Promise.all of 2 commands therefore counts once. A script with no shell call (apply_patch, update_plan, view_image) becomes a Bash row whose command is the JavaScript source. No consumer was changed to handle either case, and the same agent behaviour now counts differently on v1 (shell_command, one row per command) and v2 (exec, one row per script). (trigger: src/coder_eval/agents/codex_agent.py) (restates: Axis 8: Every exec script becomes a Bash row, so non-shell scripts (apply_patch, update_plan, view_image) record their whole source as an executed shell command)

Parallel paths:

• 🟡 There are two child-registration paths. v1 is _handle_collab_completion and v2 is _handle_subagent_spawn. The duplicate guard and the model source were added to the v2 path only. If a v1 spawnAgent collab item and a v2 started item name the same child, the child is registered two times, its rollout is read two times, and its nested messages and tokens are counted two times. (trigger: src/coder_eval/agents/codex_agent.py) (restates: Axis 1: v1 and v2 child-registration logic are duplicated and have drifted (dedupe guard and model source on one side only))
• 🔵 Only .claude/notes/agents.md records the new v2 multi-agent behaviour: the subAgentActivity spawn, exec mapped to Bash, the whole-script fallback, and the fork-history skip. docs/agents/CODEX.md has no sub-agent section, and the Codex column of docs/agents/HARNESS_PARITY.md does not say how a child's exec script maps onto Bash rows. Task authors who write command_executed criteria against Codex sub-agents cannot find this in the user docs. (trigger: .claude/notes/agents.md)

Tests:

• 🟡 The max_turns cap tests (test_capped_turn_still_folds_sub_agent_tokens) and the cooperative-stop tests (test_cooperative_stop_still_skips_sub_agent_recovery) use only the v1 _collab_call("spawnAgent") shape. No test sends a v2 subAgentActivity started spawn through a capped or early-stopped turn. So no test shows that the new v2 registration still recovers the child's tools and tokens after the pump stops (HARNESS_PARITY.md says this happens on Codex), or that a cooperative stop still skips it. (trigger: tests/test_codex_agent.py)
• 🟡 No end-to-end test sends a recovered child call with a Script failed output, or an Exit code: 1 output, through _parse_rollout_generations and _subagent_generation_blocks and then asserts result_status == "error". The new error parsing is tested only on the helper. (trigger: tests/test_codex_agent.py) (restates: Axis 3: No test checks that a failed recovered sub-agent call ends up as result_status 'error' (only the success path is tested end to end))
• 🟡 No test covers these exec-script parser inputs: a command key inside an earlier string value (the pytest -q call is lost), a script that mixes a literal command with a variable-built command (the variable command is dropped), a Promise.all of two commands (one row or two), or a script with no shell call (it becomes a Bash row). (trigger: tests/test_codex_agent.py) (restates: Axis 3: Command-key regex matches text inside earlier string values, so the real exec command is lost (untested))
• 🔵 Untested new branches: a forked rollout (forked_from_id is set) that has no thread_settings_applied marker (_fork_boundary returns 0 and does not log), the \uXXXX and unterminated-literal paths in _js_string_at, the unbalanced fallback in _matching_brace, the interacted notice kind, and a turn that holds only notices and has no delta. (trigger: tests/test_codex_agent.py) (restates: Axis 3: New hand-written JS-parser branches and v2 spawn edge branches have no tests)

Display & mapping dicts:

• 🔵 evaluation/summaries.py:39 and evaluation/judge_context.py:485 render Agent rows only from params['description']. Codex Agent rows carry other keys: operation/model/prompt (v1), and operation/agent_thread_id/agent_path (the new v2 started row). So every Codex spawn shows an empty detail in the run summary and in the LLM-judge transcript. The judge cannot see which sub-agent was spawned or why. Extend the render branch to fall back to prompt/agent_path, or make _tool_parameters emit a description key. (trigger: src/coder_eval/agents/codex_agent.py)

Nightly pipeline:

• 🔵 The PR does not say how it changes Codex multi-agent run records in the nightly and the coder-eval-uipath dashboard. The task.json schema stays the same, but the content changes: exec rows become Bash, v2 runs get new Agent spawn rows and recovered child rows, and a v1 Exit code: N (N != 0) is now error. As a result, command_executed and commands_efficiency scores and the tool counts will show a step change for Codex tasks at this version. Without a release note, readers can take this for a model regression. (trigger: .claude/notes/agents.md) (restates: Axis 8: The Codex sub-agent run-record content changes (exec->Bash rename, new spawn rows, error status) with no note for cross-run consumers)

Harness & Lint Improvements

Static checks (lint / type):

• [ce-lint] CE069 (the next free id, per the note in tests/lint/runner.py): forbid a continue, break or return statement after the first statement in the body of with contextlib.suppress(...). If the first statement raises, the later statements are skipped without any signal, so the control flow the author wrote never runs. Put it in tests/lint/rules/ce069_suppress_body_no_trailing_control_flow.py, add it to ALL_RULES in tests/lint/runner.py, and add "CE069" to the ruff external list in pyproject.toml. An AST probe of main finds 0 hits, so the rule can land clean. Prevents: Finding 1's sub-defect at codex_agent.py:252-255 (with contextlib.suppress(ValueError): out.append(chr(int(...))); j += 6; continue). A bad \u escape quietly appends u.
• [ce-lint] CE070: flag an if whose test has isinstance(<expr>.get(<const K>), T) when the body then reads <expr>[K] by subscript. Pyright drops the narrowing, so the value is Any. Require a walrus bind instead: isinstance(v := <expr>.get(K), T). Put it in tests/lint/rules/ce070_isinstance_get_then_subscript.py and wire it in runner.py and the ruff external list. The probe finds 10 sites on main (codex_agent.py:295 and :2103, delegate_agent.py:1001, formatting.py:126, run_summary_rebuild.py:103 and :105, task_loader.py:437 and :439, and others). Fix them in the same change. Prevents: Finding 8 (codex_agent.py:2262, isinstance(payload.get("input"), str) followed by script = payload["input"], so script is Any when it goes into _exec_shell_commands).
• [ce-lint] CE071, a @pytest.mark.lint class in tests/test_custom_lint.py (it must import the SDK). In tests/, flag SimpleNamespace(type="<lit>", ...) when is a type Literal of a model in openai_codex.generated.v2_all (agentMessage, reasoning, webSearch, collabAgentToolCall, subAgentActivity, ...). The test must build the real SDK model, so a renamed field fails when the fake is built. A type value the SDK does not know, such as someNewTool, is a deliberate fake for an unknown type and passes. Today there are 29 SimpleNamespace(type=...) sites, in tests/test_codex_agent.py, tests/test_codex_agent_unit.py and tests/_fixtures/golden_streams/codex_fixtures.py. Prevents: Finding 6 (tests/test_codex_agent.py:1192-1205, the _sub_agent_activity fake). If the SDK renames agent_thread_id or agent_path, production getattr(..., None) recovers nothing and every v2 test still passes.
• [ce-lint] CE072, a single-writer rule for Codex child registration with the same shape as CE059/CE061. A subscript assignment to spawn_by_thread[...] and an .append(...) on spawned_children are allowed only inside one _CodexTurnState.register_child(thread_id, tool_id, model) method, which owns the duplicate guard. Make the rule a declarative SINGLE_WRITERS table of (module, attribute, owning function), so the next state that needs one writer is one more row and not a new rule. Put it in tests/lint/rules/ce072_single_writer_state.py. Prevents: Finding 3: the v1 writer in _handle_collab_completion (codex_agent.py:2009-2011) has no duplicate guard, and the guarded v2 copy in _handle_subagent_spawn (2034-2037) has drifted from it. The result is doubled nested messages, tokens and events when both items name one child.
• [ce-lint] CE073, scoped to src/coder_eval/agents/. A str literal (len >= 4) that equals the value of a module-level _UPPER_CASE str constant in the same module must use the constant. In the same scope, re.match / re.search / re.fullmatch / re.compile with a literal pattern inside a function body must instead be a module-level compiled constant. The probe finds 22 constant-duplicate hits in src on main (antigravity_agent.py 'run_command', opencode_agent.py 'error'/'text', delegate_agent.py 'delegate-stdio', _skills.py 'skills'). Fix them or add # noqa: CE073 in the same change. Blind spot: the rule does not catch a key repeated from a constant dict such as "exec" in _ROLLOUT_FN_NAMES. Prevents: Finding 4: codex_agent.py:120 has the literal "subAgentActivity" although _SUB_AGENT_ACTIVITY_ITEM is defined at L135, and L2300 compiles re.match(r"Exit code: (\d+)", ...) inline. Partly also finding 2's magic "completed" default at L1938, but only when a constant exists.
• [ce-lint] CE074: json.loads(...) must not flow directly into <x>.append(...) or into an assignment annotated with a non-Any type such as dict[str, Any] or list[...]. Bind the result first and narrow it with isinstance(obj, dict), because json.loads returns Any and pyright never checks the annotation. 1 site on main (codex_agent.py:1997, objs.append(json.loads(raw))). Prevents: Finding 7: _fork_boundary trusts list[dict[str, Any]], so a JSON line that is not an object raises AttributeError. The broad except swallows it, and the child loses all of its recovered calls.
• [ce-lint] A complexity and size ratchet, as a @pytest.mark.lint class that uses radon (already a runtime dependency). Store a baseline of radon CC per function and the line count per module in tests/lint/cc_baseline.json. Fail when an existing function's CC rises above its baseline, when a new function exceeds CC 10 (B), or when a module over 1500 lines grows, unless the baseline is updated in the same diff, which makes the growth visible in review. A global ruff C901 cap does not work: the current maximum is F46 (generate_variant_report), so any cap that passes main is far above this PR's growth. Prevents: Finding 2: on_item_started C11->C12, on_item_completed D24->D26, _tool_parameters C17->C20, _summarize_tool_item C11->C13, _extract_generic_telemetry B8->B10, _subagent_parameters C11->C14, _subagent_output B6->C11, and the new _fork_boundary at C11. Finding 1: about 80 lexer lines added to the 2453-line codex_agent.py.
• [ci-gate] Add a diff-coverage gate in .github/workflows/pr-checks.yml and make verify, for example diff-cover coverage.xml --compare-branch origin/main --fail-under 90. The 80% global --cov-fail-under cannot see uncovered new lines in a file whose total is above 80%. Prevents: Finding 10: new lines 252-255, 263, 279, 286-289, 302, 306, 1873->1875 and 2032 are uncovered, but codex_agent.py is at 83%, so the global gate passed. Finding 13: the untested return 0 fallback at codex_agent.py:2232.
• [ci-gate] When a PR changes a file under tests/_fixtures/golden_streams/expected/ (the recovered CommandTelemetry snapshots, see the first harness improvement), or changes a tool-name mapping table in src/coder_eval/agents/ (_ROLLOUT_FN_NAMES and similar dicts whose value is a tool_name), require a release-note / CHANGELOG line that names the affected criteria (command_executed, commands_efficiency). Use the diff-aware step that conventional-commits.yml already runs. Prevents: Finding 17: the exec->Bash rename, the new v2 Agent spawn rows and the v1 Exit code: N error status are a baseline shift for cross-run consumers (the coder-eval-uipath dashboard). No note says so, so the trend break can be read as a model change.

Harness improvements (not statically reachable):

• Add Codex sub-agent rollout golden fixtures to tests/_fixtures/golden_streams/codex_fixtures.py with expected snapshots in expected/. Use scrubbed real rollouts that cover: a v1 spawnAgent; v2 subAgentActivity started plus completed / interrupted / interacted notices; a v1 and a v2 spawn for the same child; a forked child rollout with and without thread_settings_applied; an exec Promise.all of 2 commands; an apply_patch-only exec script; a literal cmd plus a variable cmd; Script completed ... exit_code=1; Process exited with code 2; aborted by user; Script failed. Snapshot the full recovered CommandTelemetry: row count, tool_name, parameters and result_status. Why not static: It needs the real Codex output vocabulary and the rows the parser produces. Whether a row is correct depends on runtime parse results, not on the code's shape. Prevents: Findings 12 (a failed command recorded as success), 16 (a non-shell script becomes a Bash row), 14 and 1 (N commands become one row), 15 (a variable cmd is dropped silently), 3 (a child is registered twice), 13 (a silent fork-boundary fallback) and 17 (the snapshot diff makes the content change visible).
• Add a cross-agent command-telemetry parity test. One canonical scenario (two shell commands run in parallel, one exits non-zero, one file written by patch) is driven through each agent's golden fixture: Claude Code, Codex parent commandExecution, Codex v1 child shell_command, Codex v2 child exec. Assert the same number of Bash rows and the same result_status set. Each permitted divergence must be named in docs/agents/HARNESS_PARITY.md. Why not static: How many rows are counted and what a row means is a semantic contract between agents and criteria. Only a diff of produced records can show it. Prevents: Findings 14 and 1: a v2 exec Promise.all gives one row and the v1 and parent paths give one row per command, so min_count / commands_efficiency score the same behaviour differently by Codex version. Finding 16: a Bash row with no shell call.
• Add a property / fuzz test for _exec_shell_commands. Generate scripts from a known list of commands with adversarial sibling values: key-like tokens inside strings (justification:"re-run the command: x"), valid and invalid \u escapes, truncated and unterminated literals, and non-literal cmd. Assert that the commands round-trip exactly. Also add a time-budget case for unbalanced input ('tools.exec_command({' * n at 80 KB must finish in under 0.5 s). Why not static: A parser can only be shown correct, and fast enough, by running it on generated inputs. Prevents: Finding 9 (_EXEC_COMMAND_KEY matches inside a string value and loses pytest -q), finding 1's bad \u escape, finding 11 (quadratic _matching_brace scan, 14.3 s at 80 KB) and the untested parser branches in finding 10.
• Add a reusable end-to-end scoring-gate helper, for example assert_require_success_gate(agent_fixture). It feeds a failed recovered sub-agent call through rollout parsing -> CommandTelemetry -> command_executed(require_success=true) and asserts that the criterion fails. Call it for every agent and every recovery path, not only the success path. Why not static: The defect is in the data flow from parsed output to a score. Each function is locally correct, so only running the gate shows the failure. Prevents: Finding 10: only result_status == "success" is asserted end to end, at tests/test_codex_agent.py:1447. Finding 12: Script completed ... exit_code=1 is credited under require_success.
• Make the rollout parse in _recover_subagent_tool_calls run through asyncio.to_thread. Add a loop-lag guard to the agent test suite: run with asyncio debug mode, set loop.slow_callback_duration, and use a fixture that fails when a callback blocks the loop longer than a budget. Why not static: CE002 (no_blocking_io_in_async) can find blocking IO calls by name. CPU-bound work in a sync helper has no syntactic marker, so only measured loop lag can show it. Prevents: Finding 11: the synchronous exec-script parse blocks the event loop, so asyncio turn timeouts cannot fire while it runs.
• Add a Tested by: <test id> pointer to each behavioural claim in .claude/notes/agents.md § Codex sub-agent recovery, for example "a failed call is not recorded as a success" and "a command built from a variable falls back to the whole script". Add a doc-surface lint that each pointer resolves to an existing test. Add a live-marker canary that the installed Codex CLI still writes thread_settings_applied in forked rollouts, and log a warning in _fork_boundary when forked_from_id is set and no boundary is found. Why not static: The lint can check that a pointer resolves. Whether the claim is true needs the named test to run. Format drift in Codex can only be seen against a live CLI. Prevents: Finding 15 and the note at agents.md:413-414 in finding 12: both claims are false for mixed scripts and for completed scripts with a failed command. Finding 13: Codex format drift makes the fork boundary fall back silently to replaying the whole rollout.
• No gate for finding 5 (a repeated function-local import, and seven tests that could be one parametrized test) or for the design choice in finding 2 (normalise a v2 started item to the v1 spawn shape at entry). This is a deliberate decision, recorded here. Why not static: Ruff PLC0415 flags about 500 function-local imports in src and tests, most of them deliberate CE017 lazy imports, so the rule would only be noise. Whether tests should be parametrized, and where to normalise item kinds, are design judgments. The CC ratchet above covers the measurable part of finding 2. Prevents: Finding 5 and the normalisation part of finding 2. Both are left to review.

Top 5 Priority Actions

1. Fix exec error detection at src/coder_eval/agents/codex_agent.py:2298: a script that ends 'Script completed ... exit_code=1' or contains 'Process exited with code N' is now recorded as result_status 'success', so require_success criteria credit failed commands. Mark a non-zero exit as an error, use a non-success status when the exit is unknown, correct .claude/notes/agents.md:413-414, and add a test with the real rollout output shape.
2. Stop labelling every exec script as Bash at src/coder_eval/agents/codex_agent.py:147 and :2265: map to Bash only when the script holds a shell call (an apply_patch, update_plan or view_image script now becomes a Bash row whose JavaScript source can fail a max_count: 0 pattern), and emit one CommandTelemetry row per extracted command with ids '<call_id>#' so min_count, commands_efficiency and require_success count per command.
3. Stop silently dropping real commands: make _EXEC_COMMAND_KEY at src/coder_eval/agents/codex_agent.py:304 skip string literals (a 'command:' token inside a justification string now loses 'pytest -q'), and at :2265 keep the whole script when any shell call has a non-literal command so a forbidden variable-built command stays visible. Add regression tests for both inputs.
4. Merge the v1 and v2 child-registration paths (src/coder_eval/agents/codex_agent.py:2009-2011 and :2034-2037) into one guarded _register_child on _CodexTurnState, because a v1 spawnAgent after a v2 started item for the same child now reads the rollout twice and doubles that child's nested messages, tokens and emitted events.
5. Add an end-to-end test that a recovered 'Script failed' or 'Exit code: 1' call gives result_status 'error' (tests/test_codex_agent.py:1447 tests only success), and add a CHANGELOG/release note that names the affected criteria for the exec->Bash and error-status baseline shift. Also bound the quadratic _matching_brace scan at src/coder_eval/agents/codex_agent.py:303 and log the silent _fork_boundary fallback at :2232.

---

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

[bai-uipath]

Right approach, but the v2 exec path mis-scores. Please fix before merge.

• Failed v2 commands are recorded as success. tools.exec_command returns the exit code instead of throwing, so a failing command still prints Script completed, and only Script failed is treated as an error. Fix: also read exit_code=N and Process exited with code N, and use a non-success status when the exit code can't be determined.
• Non-shell scripts become Bash rows. A script that only calls apply_patch or update_plan is recorded as a Bash command whose text is the JavaScript, so patch content can trip a max_count: 0 pattern. Fix: map to Bash only when the script calls the shell tool, otherwise keep exec with {"input": script}.
• One row per script. A Promise.all of N commands counts once against min_count/max_count and shares one status. Fix: emit one row per extracted command (<call_id>#n).
• Command extraction drops commands. The key regex matches inside earlier string values, and a variable-built command is dropped silently when the script also holds a literal one. Fix: skip string literals when searching for the key, and fall back to the whole script when any shell call isn't a literal.
• Before merge: one end-to-end run of the failing task on the pinned CLI 0.156.1.

Minor: v1 and v2 child registration are now two copies and only the new one dedupes, so fold them into one helper. The fork-boundary fallback should log when it finds no marker.