Merge pull request #1 from dementev-dev/feature/subagent-orchestration

refactor(skill): extract codex orchestration to Haiku subagent
This commit is contained in:
ddadmin authored and GitHub committed 2026-04-19 19:51:04 +03:00
commit 9cff0c7986
6 files changed
+1702 -224

No files matched your search

+15
View File
@@ -24,3 +24,18 @@ A Claude Code skill that orchestrates adversarial review: Claude writes an adver
- Smoke test: `docs/DESIGN.md §7` — copy-paste bash, runs in ~2 minutes.
- End-to-end: dogfood via `/adversarial-review code` against any branch with commits vs master.
- No automated CI (§9.6 explains why).
## Architecture
The skill runs in two processes:
**Main orchestrator** (`SKILL.md`, main Claude thread): mode detection, REVIEW_ID, REPO_ROOT capture, review-material prep (Steps 1-3), review display (Step 5), code fixes (Step 6), final summary (Step 8), cleanup (Step 9), and round counting.
**Runner subagent** (`references/runner.md`, dispatched via Claude Code's Agent tool with `model: haiku`): builds the launch prompt with per-attempt session marker, invokes `codex exec` (or resume or fresh-exec), runs strict checks on the result, captures the session id via two-tier lookup (primary JSONL `thread_id`, secondary rollout content-match), retries once on infrastructure failure, returns a small JSON summary.
**Why the split:** Every codex-exec invocation produces stdout JSONL, a stderr file, and a rollout file under `~/.codex/sessions/`. Keeping these inside the subagent means the main thread's context never sees them — only the final review markdown (~5K) flows back. This eliminates the ~48M-token cache-read residue observed before the split.
**Boundary invariants:**
- Main never reads `/tmp/codex-stdout-*`, `/tmp/codex-stderr-*`, or any rollout file directly.
- Subagent never interprets findings, applies fixes, or decides whether to start another round.
- The review file at `/tmp/codex-review-${REVIEW_ID}.md` is the sole artifact that crosses the boundary.
+38 -11
View File
@@ -12,8 +12,9 @@ and tries to break confidence in the change. It looks for what will fail
in production, not what might be nice to improve.
This is a [Claude Code skill](https://docs.anthropic.com/en/docs/claude-code)
— a single `SKILL.md` file that teaches Claude how to run adversarial reviews
through an external AI model (currently OpenAI Codex).
— a `SKILL.md` file plus a small `references/runner.md` that together
teach Claude how to run adversarial reviews through an external AI model
(currently OpenAI Codex).
## Key features
@@ -23,8 +24,9 @@ through an external AI model (currently OpenAI Codex).
- **Code-vs-plan** — verify the implementation matches the plan
- **Iterative** — Claude fixes issues based on reviewer feedback and resubmits
for re-review. Up to 5 rounds until approved
- **Lightweight** — one `SKILL.md` file, no server, no broker. Compare with
[codex-plugin-cc](https://github.com/openai/codex-plugin-cc):
- **Lightweight** — `SKILL.md` + one `references/runner.md` (thin runner
subagent spec), no server, no broker, no external runtime deps. Compare
with [codex-plugin-cc](https://github.com/openai/codex-plugin-cc):
~15 JS modules, App Server, JSON-RPC broker, lifecycle hooks
## How it works
@@ -77,22 +79,28 @@ If Codex is missing: `npm install -g @openai/codex`
```bash
git clone https://github.com/dementev-dev/adversarial-review.git
ln -s "$(pwd)/adversarial-review" ~/.agents/skills/adversarial-review
mkdir -p ~/.claude/skills
ln -sfn "$(pwd)/adversarial-review" ~/.claude/skills/adversarial-review
```
Verify the skill is visible to Claude Code:
Verify both the skill entry-point AND the runner subagent spec are in place:
```bash
ls -la ~/.agents/skills/adversarial-review/SKILL.md
ls -la ~/.claude/skills/adversarial-review/SKILL.md
ls -la ~/.claude/skills/adversarial-review/references/runner.md
```
> **Migrating from a previous install at `~/.agents/skills/`**: delete
> the old symlink (`rm ~/.agents/skills/adversarial-review`) and
> install at the new path above. Claude Code ≥ 2.x uses `~/.claude/skills/`.
### 3. Add permissions
The skill runs `git`, `codex exec`, and writes temp files to `/tmp`.
Without pre-approved permissions, Claude Code will prompt for each action.
**Where to add.** Since the skill is installed globally
(`~/.agents/skills/`), permissions should go into the global config
(`~/.claude/skills/`), permissions should go into the global config
so they work in any project:
| Install scope | Config file |
@@ -129,7 +137,19 @@ chosen config file:
"Bash(mv /tmp/codex-stdout-* /tmp/codex-stdout-*-failed-resume.jsonl)",
"Bash(mv /tmp/codex-stderr-* /tmp/codex-stderr-*-failed-resume.txt)",
// Cleanup
"Bash(rm -f /tmp/codex-*)"
"Bash(rm -f /tmp/codex-*)",
// Main thread: write prompt body for the runner subagent (NEW in refactor)
"Write(/tmp/codex-body-*)",
// Main thread: read the structured JSON result returned by the runner (NEW)
"Read(/tmp/codex-runner-result-*)",
// Runner subagent (inherited): read the prompt body main wrote (NEW)
"Read(/tmp/codex-body-*)",
// Runner subagent (inherited): write the result JSON main reads (NEW)
"Write(/tmp/codex-runner-result-*)",
// Runner-spec discovery: tier 1 (user-scoped install)
"Bash(ls ~/.claude/skills/adversarial-review/references/runner.md*)",
// Runner-spec discovery: tier 2 (plugin-marketplace cache)
"Bash(ls ~/.claude/plugins/cache/*/*/*/skills/adversarial-review/references/runner.md*)"
```
<details>
@@ -156,7 +176,13 @@ chosen config file:
"Read(/tmp/codex-stderr-*)",
"Bash(mv /tmp/codex-stdout-* /tmp/codex-stdout-*-failed-resume.jsonl)",
"Bash(mv /tmp/codex-stderr-* /tmp/codex-stderr-*-failed-resume.txt)",
"Bash(rm -f /tmp/codex-*)"
"Bash(rm -f /tmp/codex-*)",
"Write(/tmp/codex-body-*)",
"Read(/tmp/codex-runner-result-*)",
"Read(/tmp/codex-body-*)",
"Write(/tmp/codex-runner-result-*)",
"Bash(ls ~/.claude/skills/adversarial-review/references/runner.md*)",
"Bash(ls ~/.claude/plugins/cache/*/*/*/skills/adversarial-review/references/runner.md*)"
]
}
}
@@ -301,7 +327,8 @@ checklist, finding bar, calibration rules.
What we did differently:
- **Iterative loop** — Claude fixes issues and resubmits (not "stop and ask user")
- **Plan review** — reviews plans before code, not just code
- **Single file** — one SKILL.md vs 15+ JS modules
- **Minimal install** — `SKILL.md` + one `references/runner.md`, no
server, no broker, vs 15+ JS modules
- **Verbatim output** — reviewer findings shown as-is, not rephrased
## License
+158 -213
View File
@@ -126,27 +126,13 @@ If all sources are empty — no changes to review, inform the user.
**Code-vs-plan review:** prepare the plan path AND collect the list of changed files (as above).
### Step 4: Build the prompt and launch the first round
### Step 4: Build the prompt body, dispatch the runner subagent
Build the prompt depending on the mode. All prompts use the adversarial stance.
Main thread composes the prompt BODY (without the session marker — the subagent adds it). Select the right template from below based on review mode.
**All prompts begin with a per-launch session marker.** The FIRST line of every prompt (plan, code, code-vs-plan, resume, fresh-exec fallback) must be a literal HTML-style comment that includes the current `${REVIEW_ID}` AND a fresh `${ATTEMPT_ID}`:
**Prompt body for plan review:**
```
<!-- ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID} -->
```
Example: `<!-- ADVERSARIAL-REVIEW-SESSION: 1711872000-48217593-487201 -->`.
- `${REVIEW_ID}` is stable for the whole review (generated at Step 2).
- `${ATTEMPT_ID}` is a **new** 6-digit random integer generated immediately before writing the prompt for this launch. Generate a different value for the initial exec, any retry of the initial exec, each resume in Step 7, and any fresh-exec fallback. Do NOT reuse an earlier launch's ATTEMPT_ID within the same review.
The comment is ignored by Codex as content but becomes part of the rollout transcript on disk. Check 4 below positively binds the rollout to THIS launch by grepping the rollout JSONL for the exact `ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID}` string. Attempt-scoping eliminates same-review retry ambiguity (a timed-out first attempt leaves a rollout with the OLD attempt id; the retry's fallback only matches the NEW one).
**Prompt for plan review:**
```
<!-- ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID} -->
<role>
You are a senior adversarial reviewer of implementation plans.
Your job is to break confidence in the plan, not to validate it.
@@ -212,10 +198,9 @@ VERDICT: REVISE
</output_format>
```
**Prompt for code review (<= 50 files):**
**Prompt body for code review (≤ 50 files):**
```
<!-- ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID} -->
<role>
You are a senior adversarial code reviewer.
Your job is to break confidence in the change, not to validate it.
@@ -290,9 +275,10 @@ VERDICT: REVISE
</output_format>
```
**Prompt for code review (> 50 files):**
**Prompt body for code review (> 50 files):**
Same as ≤ 50 files above, but the `<task>` section is replaced with:
Same prompt as above, but the `<task>` section without the file list:
```
<task>
Review the code changes in this repo.
@@ -301,121 +287,133 @@ Run <git diff commands> to see changed files and full diffs.
</task>
```
**Prompt for code-vs-plan review:**
**Prompt body for code-vs-plan review:**
Same prompt as code review, but the `<task>` section is extended:
```
<task>
Review the code changes in this repo against the implementation plan in <plan-path>.
Changed files:
Same as code review (≤ 50 or > 50 variant depending on file count), but:
- `<task>` is extended to reference the plan file: `Review the code changes in this repo against the implementation plan in <plan-path>.`
- `<attack_surface>` appends these three items:
```
- Completeness: does the implementation cover all plan steps?
- Deviations: where does the code differ from the plan? Are deviations justified?
- Missing: what from the plan is not yet implemented?
```
<file list or empty if > 50>
**Substitute template placeholders BEFORE writing to disk:**
Changes include: <type>.
Run <git diff commands> to see the full diffs.
</task>
```
The inlined prompt bodies above contain template placeholders that main must resolve with real captured values before the Write. Placeholders per mode:
And the following items are added to `<attack_surface>`:
```
- Completeness: does the implementation cover all plan steps?
- Deviations: where does the code differ from the plan? Are deviations justified?
- Missing: what from the plan is not yet implemented?
```
| Placeholder | Value source | Applies to |
|---|---|---|
| `${BASE_BRANCH}` | captured at Step 2 (code & code-vs-plan only) | code, code-vs-plan |
| `<plan-path>` | captured at Step 3 | plan, code-vs-plan |
| `<file list from --name-only>` | result of `git diff --name-only` + `git diff --cached --name-only` (or branch diff) from Step 3 | code, code-vs-plan (≤50 files only) |
| `<unstaged changes / staged changes / ...>` | human-readable description derived from which diff commands had content | code, code-vs-plan |
| `<git diff commands>` | the exact commands main determined at Step 3 (e.g. `git diff`, `git diff --cached`, `git diff ${BASE_BRANCH}...HEAD`) | code, code-vs-plan |
**Launching Codex — command template:**
Substitute `${BASE_BRANCH}` first (it appears nested inside `<unstaged changes / staged changes / ...>`), then compute the outer human-readable description based on which diffs have content. Main writes the substituted string to the Write tool — no template placeholders should remain in the body file sent to the runner.
Flags:
- `--json` — stdout becomes JSONL events (primary path for session-ID capture). In some sandbox configurations this stream ends up empty; the filesystem fallback in check 4 below handles that case.
- `-m gpt-5.4` — model (overridden by `model:...` argument)
- `-c model_reasoning_effort=high` — reasoning depth (overridden by `xhigh`, `low`, etc.)
- `-s read-only` — reviewer only reads, does not write
- `-C "${REPO_ROOT}"` — pin codex workdir to absolute repo root
- `-o /tmp/codex-review-${REVIEW_ID}.md` — file for capturing final agent text
**Capture user overrides for `CODEX_MODEL` / `CODEX_REASONING` at Step 1:**
**Prompt delivery:** write the prompt to `/tmp/codex-prompt-${REVIEW_ID}.md` via **Write tool**, then feed it to codex via `cat file | codex exec ... -`. This avoids shell quoting issues with long XML prompts and is environment-portable (the alternative `- < file` stdin-redirect form is accepted by codex but fails with `EXIT=1` in some Claude Code sandbox configurations).
The skill supports overrides like `/adversarial-review xhigh`, `/adversarial-review medium`, `/adversarial-review model:gpt-5.3-codex`. At Step 1, capture:
**Plan Mode note:** Writing to `/tmp` via Write tool may trigger a permission prompt or exit Plan Mode. This is a known Claude Code limitation — Plan Mode restricts edits to the plan file only. If this happens, it does not affect review correctness: the review mode is already determined, and the skill only edits the plan file and `/tmp` temp files.
- `CODEX_MODEL` — default `gpt-5.4`. Overridden by any argument matching `^model:(.+)$`; use the capture group.
- `CODEX_REASONING` — default `high`. Overridden by any argument exactly matching `low`, `medium`, `high`, or `xhigh`.
The prompt file (`/tmp/codex-prompt-${REVIEW_ID}.md`) just written serves as the anchor for the filesystem session-id fallback: its mtime is strictly earlier than any rollout file codex will create for this session, and it exists on disk without requiring any extra write. Check 4 below uses `find -newer` against this file instead of a timestamp arithmetic computation.
These are passed into the runner YAML input block below.
**Write the prompt body to disk via Write tool:**
Write `/tmp/codex-body-${REVIEW_ID}.md` containing the substituted body text (no session marker — the runner adds it).
> **Plan Mode note:** Writing to `/tmp` via Write tool may trigger a permission prompt or exit Plan Mode. This is a known Claude Code limitation. Additionally, dispatching a subagent under Plan Mode may inherit the restriction — empirical behavior documented in DESIGN.md §12.7.
**Resolve the runner spec path:**
The runner spec lives at `references/runner.md` within the skill's install directory. Main cannot reliably introspect Claude Code's skill-invocation header from inside its own context (there is no tool for reading one's own system prompt — any attempt would be a hallucination risk). Therefore the discovery uses only concrete filesystem checks, in this priority order:
1. **User-scoped install** (primary): check `~/.claude/skills/adversarial-review/references/runner.md`:
```bash
cat /tmp/codex-prompt-${REVIEW_ID}.md | timeout 600 codex exec --json \
-m gpt-5.4 \
-c model_reasoning_effort=high \
-s read-only \
-C "${REPO_ROOT}" \
-o /tmp/codex-review-${REVIEW_ID}.md \
- \
> /tmp/codex-stdout-${REVIEW_ID}.jsonl \
2>/tmp/codex-stderr-${REVIEW_ID}.txt
ls ~/.claude/skills/adversarial-review/references/runner.md 2>/dev/null
```
> **CRITICAL — the Bash tool result is NOT the review.** stdout is redirected to `/tmp/codex-stdout-${REVIEW_ID}.jsonl` (machine-readable JSONL events when populated, empty when the sandbox suppresses it — either way, never human-readable review text). The human-readable review exists ONLY in `/tmp/codex-review-${REVIEW_ID}.md`. Do not attempt to extract review text from the Bash result — there is none.
If exit 0, set `RUNNER_SPEC_PATH` to the expanded absolute path and proceed.
**Important:**
- Always wrap `codex exec` in `timeout 600` (10 minutes). If Codex hangs — the command exits with code 124.
- Use `timeout: 620000` parameter in Bash tool for headroom.
- The command is **synchronous**: when it returns, all four files (`-o`, stdout jsonl, stderr, prompt) are in their final state. Do **NOT** use a poll-loop.
- With `--json`, stderr is empty on success. It contains content only on errors (e.g. "Failed to write last message file ..."). Use stderr for diagnostics, NOT for session-id capture.
2. **Plugin-marketplace install** (secondary): Claude Code's plugin system installs skills at paths like `~/.claude/plugins/cache/<marketplace>/<plugin>/<version>/skills/adversarial-review/`. Glob to find it:
**Post-launch strict check order (do each before moving to the next):**
```bash
ls ~/.claude/plugins/cache/*/*/*/skills/adversarial-review/references/runner.md 2>/dev/null | head -1
```
1. **Exit code.**
- `124` → timeout. Tell the user "Reviewer did not respond within 10 minutes" and offer retry. Retry does NOT consume the round counter; max 1 retry per round.
- `≠ 0 and ≠ 124` → launch error. Read `/tmp/codex-stderr-${REVIEW_ID}.txt` (if it exists), show its contents to the user, abort the skill.
- `0` → proceed.
If the Glob returns one or more paths, take the first and set `RUNNER_SPEC_PATH`.
2. **Stderr sanity (even on exit 0).** Read `/tmp/codex-stderr-${REVIEW_ID}.txt`.
- If file missing → redirect itself failed; tell user `Could not create stderr file — check /tmp writability`, abort.
- If file contains a line matching `^Error:` or `Failed to write` → codex reported an infrastructure failure despite exit 0. Show stderr to user, route to launch-failure retry (max 1 per round; after retry failure → hard abort).
- Otherwise → proceed.
3. **Dev checkout** (tertiary): if neither above, try `$(git rev-parse --show-toplevel)/references/runner.md`:
3. **Review file sanity.** Read `/tmp/codex-review-${REVIEW_ID}.md`. It must exist and contain a line matching `^VERDICT: (APPROVED|REVISE)$`; if REVISE, it must also contain at least one line matching `\[severity:\s*(critical|high|medium)` (a structured finding). The full semantic-check logic is in Step 5; do the same thing here.
- Fails → route to launch-failure retry (max 1 per round; after retry failure → hard abort). Do NOT capture `CODEX_SESSION_ID` — if the review itself is broken, the session is of no use.
- Passes with `VERDICT: APPROVED` → `CODEX_SESSION_ID` is not needed (no Step 7 resume will happen). Skip the capture below entirely and proceed to Step 5.
- Passes with `VERDICT: REVISE` → capture `CODEX_SESSION_ID` next (check 4).
```bash
REPO=$(git rev-parse --show-toplevel 2>/dev/null) && ls "$REPO/references/runner.md" 2>/dev/null
```
4. **Capture `CODEX_SESSION_ID` — two-tier.** Only reached when the review was valid AND the verdict is REVISE (Step 7 resume is about to happen).
4. **Abort**: if no path yields a readable file, tell the user: `Could not locate references/runner.md. Expected locations: (1) ~/.claude/skills/adversarial-review/references/runner.md, (2) ~/.claude/plugins/cache/*/*/*/skills/adversarial-review/references/runner.md, (3) $(git rev-parse --show-toplevel)/references/runner.md. Re-install the skill.` Abort the skill.
**What you are looking for.** A UUID string (format `[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}`) that identifies the codex session, so `codex exec resume <UUID>` in Step 7 can continue this conversation. Try the cheap source first, fall back to the filesystem only if needed.
Save the resolved absolute path as `RUNNER_SPEC_PATH`. Do NOT attempt to extract the path from any "Base directory for this skill:" line in the conversation — that line is a system injection Claude cannot reliably read from inside its own context.
**Primary: first line of JSONL stdout.** Read `/tmp/codex-stdout-${REVIEW_ID}.jsonl`. The expected first-line shape is:
```json
{"type":"thread.started","thread_id":"<uuid>","...":...}
```
**Dispatch the runner subagent via Agent tool:**
- **First line parses as JSON AND has a valid `thread_id` UUID** → save as `CODEX_SESSION_ID`, proceed to Step 5.
- **Any other case** (file empty / 0 bytes, first line not valid JSON, JSON has no `thread_id`, `thread_id` is not a UUID, partial/garbage output) → fall through to the secondary path below. Do NOT save an empty or malformed `CODEX_SESSION_ID`.
**Do NOT Read `${RUNNER_SPEC_PATH}` in main.** Pass the path to the subagent; it reads the spec itself. This keeps runner.md (~12K) out of main's context — both the Read result AND the Agent prompt duplication. Saves ~12K per round × up to 5 rounds per review.
**Secondary: rollout content-match.** The primary fails for two independent reasons: (a) in some Claude Code sandbox configurations `--json` stdout is empty (0 bytes) even on exit 0 with populated `-o`; (b) partial or format-drifted output from a future codex version. In both cases the session is recoverable from disk: every `codex exec` writes a rollout file named `rollout-<ISO-timestamp>-<UUID>.jsonl` under `~/.codex/sessions/YYYY/MM/DD/` (see `DESIGN.md §2.3`). The trailing UUID in the filename is the session id — but blindly picking the newest rollout risks binding to a parallel codex invocation (silent corruption). To bind positively, the skill matches **both** (i) rollout mtime newer than the prompt file (timestamp anchor) AND (ii) rollout contains the session marker (content anchor).
Invoke the Agent tool with:
- `subagent_type: "general-purpose"`
- `model: "haiku"`
- `description: "Adversarial-review runner, round N"` (N is the current round number)
- `prompt:` a short bootstrap instruction + YAML input block (no inlined runner.md):
Run this single POSIX-portable invocation, grepping for the **current launch's** `${REVIEW_ID}-${ATTEMPT_ID}` marker (not just `${REVIEW_ID}`):
```
Read your full instruction spec at ${RUNNER_SPEC_PATH} and follow the steps there using this input:
```bash
find ~/.codex/sessions -name 'rollout-*.jsonl' -newer /tmp/codex-prompt-${REVIEW_ID}.md -exec grep -l 'ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID}' {} + 2>/dev/null
```
---
REVIEW_ID: 1711872000-48217593
REPO_ROOT: /home/dementev/sources/myproject
OPERATION: initial
CODEX_MODEL: gpt-5.4
CODEX_REASONING: high
PROMPT_BODY_PATH: /tmp/codex-body-1711872000-48217593.md
RESULT_PATH: /tmp/codex-runner-result-1711872000-48217593.json
---
```
Substitute the actual `REVIEW_ID` in the prompt path and the actual `REVIEW_ID-ATTEMPT_ID` combined marker in the grep pattern. `-newer FILE`, `-exec ... {} +`, and `grep -l` are all POSIX — works identically on Linux and macOS.
Substitute the actual resolved `${RUNNER_SPEC_PATH}` (absolute path) and real values for every other placeholder. `RESULT_PATH` always follows the pattern `/tmp/codex-runner-result-${REVIEW_ID}.json`.
The output is zero or more rollout paths that (a) postdate our prompt file AND (b) contain this launch's specific marker. From the result:
**Do NOT run the Agent tool call in background.** Wait for the subagent to return. (Runner's own codex exec is also synchronous per runner Step R3.)
- **Exactly one path** (the expected case) → this is our rollout. Extract the trailing UUID from the filename (the 36-char hex-and-dashes pattern above) and save as `CODEX_SESSION_ID`.
- **Zero paths** → **fail closed.** Either codex did not create a rollout, or something prevented the marker from reaching disk. We cannot safely guess. Before aborting, surface diagnostic context to the user: the contents of `/tmp/codex-stdout-${REVIEW_ID}.jsonl` (if non-empty), `/tmp/codex-stderr-${REVIEW_ID}.txt`, and the 3 most-recent rollout filenames (`ls -t ~/.codex/sessions/*/*/*/rollout-*.jsonl 2>/dev/null | head -3`). Then treat as launch failure, generate a **new** `ATTEMPT_ID` for the retry (so the retry's fallback won't match this launch's rollout if it later appears), rewrite the prompt with the new marker, retry once, then abort.
- **Multiple paths** → **fail closed.** This should not happen: `ATTEMPT_ID` is per-launch, so two rollouts sharing both `REVIEW_ID-ATTEMPT_ID` would require either a 10⁻⁶ collision on `ATTEMPT_ID` or a mistaken reuse. Do NOT pick arbitrarily — abort the round with a diagnostic listing all matching rollout paths. Silent session drift is worse than visible failure.
**Parse the subagent's response — two-channel protocol:**
**Why positive-bind instead of newest-by-mtime:** Round 6 of adversarial review flagged that picking newest-by-mtime allows a parallel codex invocation (user running codex in another terminal, CI job, etc.) to create a newer rollout during the race window, which our secondary would silently pick — Step 7 resume would succeed against that wrong session, and the skill would apply fixes informed by an unrelated review. Positive content-match with per-launch `ATTEMPT_ID` eliminates this both cross-review (parallel codex) and intra-review (retries): only a rollout containing **this launch's specific** marker is accepted; everything else is invisible.
Apply the regex `RUNNER_RESULT_AT:\s+(\S+)` (UNANCHORED — matches anywhere in the Agent tool's result text, tolerant of markdown fences and preamble). Take the first match's capture group as the result-file path.
**Where `thread_id` / session id is NOT:**
If the regex finds NO match in the subagent's response, fall back to a Glob for the deterministic path `/tmp/codex-runner-result-${REVIEW_ID}.json` — REVIEW_ID is already known to main. If Glob also returns nothing, treat as `infra_error` with `errors: "runner did not write result file at deterministic path and did not emit RUNNER_RESULT_AT line"` and abort.
- NOT in `/tmp/codex-review-${REVIEW_ID}.md` (only contains the final agent text)
- NOT in stderr file under `--json` (empty on success; error text only on failure)
- NOT in the middle or tail of stdout — only the **first line** of the JSONL file (when it is populated at all)
Read the file at the resolved path. Parse as JSON. Extract `result`, `verdict`, `review_file`, `codex_session_id`, `errors`, `user_warning`, `archived_stdout`, `archived_stderr`.
**Notes:**
- Default model: `gpt-5.4` with `model_reasoning_effort=high`. User can override via arguments.
- Always `-s read-only` — reviewer must not write files.
- Do **NOT** run in background.
**If `user_warning` is non-null, surface it as a SEPARATE short user-visible message BEFORE the Step 5 verbatim-review message.** Format:
```
⚠ <user_warning contents>
```
Emit this on its own turn — do NOT concatenate into the Step 5 `## Adversarial Review — Round N` header message (that message's body must remain the review's verbatim content, nothing else). Emit the warning FIRST, then the Step 5 message. This preserves both the pre-refactor §2.4.4 "no-op refresh" diagnostic AND the Step 5 verbatim-display contract.
Dispatch based on `result`:
| `result` value | Main thread action |
|---|---|
| `success` | Save `codex_session_id` (keep prior if `null` per §2.4.4). Surface `user_warning` if set. Proceed to Step 5. |
| `timeout` | **TERMINAL — do NOT re-dispatch.** Runner already attempted twice internally (R4.1 + R5 retry = 2 × 10min). Tell user: "Reviewer timed out after two attempts (20 minutes total)." Abort the skill. User can re-invoke `/adversarial-review` to start a fresh review. |
| `launch_failure` | **TERMINAL — do NOT re-dispatch.** The runner already retried once internally (Step R5). Show `errors` to user, abort the skill. This keeps the total-attempts-per-round invariant at 2 (matches pre-refactor: 1 initial + 1 retry). |
| `infra_error` | Show `errors` to user (infrastructure: /tmp not writable, stderr file missing, RUNNER_RESULT_AT line absent). Abort. |
| `input_error` | Bug in orchestration. Show `errors` to user. Abort. |
**Round-level attempt invariant:** exactly ONE runner dispatch per round. Every failure result is terminal at main. The runner owns the full retry budget (≤2 attempts per dispatch, internal) regardless of failure type. Total codex invocations per round ≤ 2.
> **CRITICAL — main thread does NOT read stdout/stderr/JSONL/rollout files BY CONTENT.** Those live and die inside the subagent. Main reads: the runner result JSON at `RESULT_PATH`, the review file at `review_file`, and nothing else from `/tmp/codex-*`. Archival `mv` (on resume failure) is done by the runner, not main — main never references `/tmp/codex-stdout-*` or `/tmp/codex-stderr-*` in any Bash argv.
### Step 5: Read the review, show it, then check the verdict
@@ -436,7 +434,7 @@ If any check fails → this is a **launch failure** (model produced no actionabl
**3. Show the review to the user. This is mandatory and blocking.**
> Your VERY NEXT MESSAGE to the user must begin with the header below, followed by the file contents **verbatim**. Not "I've received the review", not "The reviewer said:", not a summary — the literal file content.
> Your next user-visible message that is NOT a one-line `⚠ <warning>` diagnostic must begin with the header below, followed by the file contents **verbatim**. Not "I've received the review", not "The reviewer said:", not a summary — the literal file content.
>
> Do NOT wrap the review in a code fence (the review is already markdown, and an outer fence would break on inner fences).
>
@@ -458,7 +456,7 @@ Message format:
### Step 6: Apply fixes
> **Precondition gate (check first).** Before calling any Edit, Write, or other fix-applying tool: confirm that you have already sent a user-visible message in THIS round whose body contains the verbatim review text. If you have not — STOP. Go back to Step 5 and send the review message now. This is the same rule that protects the "user sees the review" contract; a literal reader may otherwise slip past it.
> **Precondition gate (check first).** Before calling any Edit, Write, or other fix-applying tool: confirm that you have already sent a user-visible message in THIS round whose body contains the verbatim review text (short `⚠ <user_warning>` diagnostic messages do NOT count). If you have not — STOP. Go back to Step 5 and send the review message now. This is the same rule that protects the "user sees the review" contract; a literal reader may otherwise slip past it.
Based on the reviewer's findings:
@@ -480,17 +478,17 @@ Based on the reviewer's findings:
### Step 7: Resubmit to Codex (Rounds 2-5)
**Resume is the primary path.** Saves tokens and preserves session context. A fresh `codex exec` without resume is an **emergency fallback** — costly in tokens, and requires rebuilding prior-round context.
**Resume is the primary path.** Saves tokens and preserves session context. A fresh `codex exec` without resume is an **emergency fallback** when resume itself fails.
**1. Write the resume prompt** to `/tmp/codex-resume-prompt-${REVIEW_ID}.md` via **Write tool**. Use a separate file from the initial prompt so round-1 material remains available for diagnostics. **Generate a fresh `${ATTEMPT_ID}` for this resume launch** (different from the initial exec's ATTEMPT_ID and from every prior resume's ATTEMPT_ID). The resume prompt must begin with the per-launch marker:
**Step 7.1: Write the resume prompt body to disk.**
Write `/tmp/codex-resume-body-${REVIEW_ID}.md` containing:
```
<!-- ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID} -->
I've revised based on your feedback.
Here's what I changed:
[List of fixes]
[List of fixes from Step 6]
Re-review with the same adversarial stance. Focus on:
1. Whether my fixes actually resolve the reported issues
@@ -499,56 +497,44 @@ Re-review with the same adversarial stance. Focus on:
End with VERDICT: APPROVED or VERDICT: REVISE
```
**2. Run resume.** Resume does NOT accept `-C`, so prefix the command with an explicit `cd` to `${REPO_ROOT}` (captured at Step 2). Use single quotes around `${REPO_ROOT}` — the path was validated at Step 2 to contain no single quotes.
Substitute the fixes list from Step 6 (one bullet per finding addressed). Do NOT include the session marker — the subagent adds it.
The resume prompt file (`/tmp/codex-resume-prompt-${REVIEW_ID}.md`) acts as the anchor for this resume's filesystem fallback, the same way the initial prompt file anchors Step 4. Launch resume via the `cat | ... -` pattern:
**Step 7.2: Dispatch the runner subagent for resume.**
```bash
cd '${REPO_ROOT}' && cat /tmp/codex-resume-prompt-${REVIEW_ID}.md | timeout 600 codex exec resume --json \
${CODEX_SESSION_ID} \
-o /tmp/codex-review-${REVIEW_ID}.md \
- \
> /tmp/codex-stdout-${REVIEW_ID}.jsonl \
2>/tmp/codex-stderr-${REVIEW_ID}.txt
Same Agent tool invocation as Step 4 (bootstrap instruction with `${RUNNER_SPEC_PATH}` + YAML input block; subagent Reads the spec itself). Reuse the `RUNNER_SPEC_PATH` resolved in Step 4 (do not re-resolve). Input block:
```yaml
---
REVIEW_ID: <same as initial round>
REPO_ROOT: <same>
OPERATION: resume
CODEX_MODEL: <same>
CODEX_REASONING: <same>
PROMPT_BODY_PATH: /tmp/codex-resume-body-<REVIEW_ID>.md
RESULT_PATH: /tmp/codex-runner-result-<REVIEW_ID>.json
CODEX_SESSION_ID: <uuid from previous round's runner result>
---
```
Use `timeout: 620000` in Bash tool parameters.
**Step 7.3: Parse the two-channel result.**
**Note:** Resume does NOT accept `-s` (sandbox — inherited from the original session; always `read-only` here) or `-C` (see above). It DOES accept `--json`, `-o`, `-m`, and `-i`.
Extract `RUNNER_RESULT_AT:` line (same tolerant regex + Glob fallback as Step 4), read the JSON file, extract fields. If `user_warning` is non-null, emit it as its own `⚠ <user_warning>` message BEFORE any other action (including before the Step 5 verbatim review) — see Step 4's user_warning rule.
**3. Post-resume strict check order (do each before moving to the next):**
| `result` value | Main thread action |
|---|---|
| `success`, verdict `APPROVED` | Read `review_file`, go to Step 5 (it will dispatch to Step 8 on APPROVED). |
| `success`, verdict `REVISE` | Save new `codex_session_id`. If the subagent returned null (zero-find resume), keep the prior id per §2.4.4 — `user_warning` will already have been surfaced. Go to Step 5. |
| `timeout` | **TERMINAL for this round** — runner already attempted twice. Route to fallback below. (Fresh-exec is a NEW round from the 5-round counter — its own ≤2-attempts budget applies.) No user-offered retry; that would compound. |
| `launch_failure` | **TERMINAL for this round** — runner already retried once internally. Route to fallback below (runner already archived stdout/stderr to `-failed-resume.*` — paths in `archived_stdout` / `archived_stderr`). |
| `infra_error` | Show `errors` to user, abort. |
1. **Exit code.**
- `124` → timeout. Tell the user and offer retry. Retry does not consume the round counter.
- `≠ 0` → resume failed. Do NOT update `CODEX_SESSION_ID`. Route to fallback.
- `0` → proceed.
**Round-level attempt invariant:** exactly ONE runner dispatch per resume round. Every failure result routes to fallback (not re-dispatch within the same round). Fallback's fresh-exec dispatch consumes a NEW round from the 5-round counter, which has its own independent 2-attempts-per-round budget. Total codex invocations per round ≤ 2 regardless of failure type — matches pre-refactor; closes Round-2 finding #1.
2. **Stderr error check** (exit 0 can hide `Error:` or `thread/resume failed`). Read `/tmp/codex-stderr-${REVIEW_ID}.txt`:
- If file is missing → redirect failed; tell user `/tmp not writable`, abort.
- If contains a line matching `thread/resume failed` or `^Error:` → route to fallback. Do NOT update `CODEX_SESSION_ID`.
**Step 7.4: Fallback chain** — triggered by `launch_failure` or repeated `timeout` from the runner.
3. **Review file sanity.** Read `/tmp/codex-review-${REVIEW_ID}.md` and apply the same checks as Step 5.2:
- Missing / empty / no `^VERDICT: (APPROVED|REVISE)$` line / REVISE without `[severity:` lines → route to fallback. Do NOT update `CODEX_SESSION_ID`.
*Severity classification:* parse the PREVIOUS round's review (kept in conversation history from Step 5.3's verbatim display) for the highest `[severity:` level. Default to `critical` if zero matches (format drift).
**4. Only if all three checks pass AND the verdict is REVISE** → refresh `CODEX_SESSION_ID` using two tiers (primary = first JSONL line of `/tmp/codex-stdout-${REVIEW_ID}.jsonl`; secondary = rollout file that is both newer than `/tmp/codex-resume-prompt-${REVIEW_ID}.md` AND contains the `ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID}` marker for THIS resume's ATTEMPT_ID, with UUID extracted from the basename — same positive-binding approach as Step 4 check 4 but anchored on the resume prompt). On APPROVED verdict, skip the refresh — there is no round N+1.
> **Important — NOT identical to Step 4 check 4 on the failure side.** Step 4 check 4 treats "no matching rollout" as a launch failure because in Step 4 the session id is needed for resume to even happen. In Step 7 the resume has **already succeeded** (checks 1-3 passed), and per `DESIGN.md §2.4.4` the thread id does not rotate across resumes — so if both tiers yield nothing here, **do NOT abort and do NOT retry**: keep the previous `CODEX_SESSION_ID` unchanged, log a one-line warning to the user (`"Step 7 session-id refresh: both tiers empty, continuing with previous ID per §2.4.4"`), and continue to Step 5.
After the refresh (or the no-op refresh on zero-find), return to **Step 5** with the new review.
---
**Fallback chain** — triggered when any of the three resume checks above fails.
> `--last` is deliberately NOT used. `codex exec resume --last` picks the newest session in the current cwd, which may be an unrelated codex invocation and cannot be distinguished from the intended one until after damage is done.
**Severity classification** — parse the **previous** round's review file (which is still in `/tmp/codex-review-${REVIEW_ID}.md` only if the resume overwrote the current-round result but not the previous-round; in general, rely on **conversation history** where prior rounds were shown verbatim per Step 5.3).
Parse case-insensitively for `\[severity:\s*(critical|high|medium)\b` and take the highest. If zero matches (reviewer format drift), default to `critical` to force re-verification in non-interactive mode.
**Interactive mode** (you received a direct user message earlier in this session, not a trigger/cron):
Ask the user:
*Interactive mode* (direct user message earlier in this session): ask the user:
```
Resume failed — the reviewer's re-review did not produce a usable result.
@@ -559,57 +545,15 @@ Options:
(b) Conclude the review — show current findings as NOT VERIFIED
```
- (a) → fresh-exec path below.
- (b) → Step 8 with the **not-verified** terminal state (same as maximum-reached, but with a different header).
*Non-interactive mode:*
- Max severity `critical` or `high` → fresh exec automatically.
- Max severity `medium` only → Step 8 with the not-verified terminal state.
**Non-interactive mode** (headless, scheduled run, no direct user message in this conversation):
*Fresh-exec dispatch:* build a new PROMPT_BODY that is the original Step 4 prompt for the current mode, followed by sections `## Previous review rounds` (verbatim round-1..N reviews + fixes from conversation history) and `## Current state of the artifact`. Write to `/tmp/codex-body-${REVIEW_ID}.md` (overwriting the original).
- Max severity `critical` or `high` → fresh exec automatically. The risk of silently skipping a serious finding outweighs the token cost.
- Max severity `medium` only → Step 8 with the **not-verified** terminal state.
**Archival note:** if the fallback was triggered by `launch_failure`, the runner already archived failed-resume stdout/stderr to `-failed-resume.*` paths during Step R5 — main does NOT need to `mv` anything. If triggered by repeated `timeout`, no archival happened (no second codex invocation produced useful diagnostics); main can proceed directly. Either way, main never touches `/tmp/codex-stdout-*` or `/tmp/codex-stderr-*` itself.
**Fresh-exec prompt template.** The lead rebuilds prior-round context from the conversation (all prior rounds were shown verbatim in Step 5.3 user messages, so they are available in context). Generate a fresh `${ATTEMPT_ID}` for this fresh-exec launch, then begin the prompt with the per-launch marker:
```
<!-- ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID} -->
[Original adversarial prompt for the current mode, from Step 4]
## Previous review rounds
### Round 1 findings (verbatim from earlier in this conversation):
<copy the round-1 review text that you already output as a user message>
### Round 1 fixes:
<copy the round-1 fixes list you output in Step 6>
### Round 2 findings (verbatim):
<...>
### Round 2 fixes:
<...>
## Current state of the artifact
[plan mode] Full current plan text: <insert>
[code mode] Run `git diff` from ${REPO_ROOT} to see the current changes.
Re-review. Focus on whether prior fixes resolved the reported issues and on any NEW issues introduced by the fixes.
End with VERDICT: APPROVED or VERDICT: REVISE.
```
**Archive failed-resume diagnostics BEFORE launching the fresh exec** (the fresh exec reuses the same stderr/stdout paths and would overwrite them):
```bash
mv /tmp/codex-stdout-${REVIEW_ID}.jsonl /tmp/codex-stdout-${REVIEW_ID}-failed-resume.jsonl 2>/dev/null
mv /tmp/codex-stderr-${REVIEW_ID}.txt /tmp/codex-stderr-${REVIEW_ID}-failed-resume.txt 2>/dev/null
```
If the fresh exec later needs investigating, both the failed-resume trail (`*-failed-resume.*`) and the fresh-exec trail (the unsuffixed files) survive side-by-side. Cleanup at Step 9 removes both (the cleanup glob `/tmp/codex-*-${REVIEW_ID}*` covers the suffixed variants).
Write the fresh-exec prompt to `/tmp/codex-prompt-${REVIEW_ID}.md` (overwriting the original is acceptable).
Launch using the **same command template as Step 4** (`cat file | timeout 600 codex exec --json ... -` with `-C`, `-o`, stdout jsonl, stderr). Because this fresh-exec path **overwrites** `/tmp/codex-prompt-${REVIEW_ID}.md` with new content just written above, that file's mtime is automatically the post-write moment — it serves as the `-newer` anchor for the two-tier secondary session-id capture on the fresh exec's rollout, the same way Step 4 uses it on the initial exec's rollout. Apply the same post-launch strict check order including the two-tier session-id capture, then return to **Step 5**.
> This fresh exec consumes one round from the 5-round counter — same as a successful resume would have.
Dispatch the runner subagent with `OPERATION=fresh-exec` (same input schema, new PROMPT_BODY_PATH pointing at the rebuilt prompt). The fresh-exec consumes one round from the 5-round counter. Return to Step 5 with the new review.
### Step 8: Final result
@@ -677,7 +621,9 @@ rm -f /tmp/codex-plan-${REVIEW_ID}.md \
/tmp/codex-stdout-${REVIEW_ID}.jsonl \
/tmp/codex-stderr-${REVIEW_ID}.txt \
/tmp/codex-stdout-${REVIEW_ID}-failed-resume.jsonl \
/tmp/codex-stderr-${REVIEW_ID}-failed-resume.txt
/tmp/codex-stderr-${REVIEW_ID}-failed-resume.txt \
/tmp/codex-body-${REVIEW_ID}.md \
/tmp/codex-runner-result-${REVIEW_ID}.json
```
If the user declined `rm` — continue without error.
@@ -687,21 +633,20 @@ Do NOT delete plan files that existed before the review (only temp files created
## Rules
- Claude **actively fixes** issues based on reviewer feedback — this is NOT just message forwarding.
- Reviewer findings are shown **verbatim** — do not rephrase or shorten. The Step 5 "YOUR NEXT MESSAGE" instruction is blocking: no edit/fix tool may be called until that message has been sent.
- Reviewer findings are shown **verbatim** — do not rephrase or shorten. The Step 5 "YOUR NEXT MESSAGE" instruction is blocking.
- Auto-detect review mode from context; user arguments take priority.
- With explicit `plan` argument or in Claude Code Plan Mode: skip git checks and base branch detection.
- **`REPO_ROOT` is captured at Step 2** via `git rev-parse --show-toplevel` and substituted as an absolute literal path into every codex command. Never use `$(pwd)` inside codex commands — cwd drift between Bash calls makes it unreliable.
- **Resume requires `cd '${REPO_ROOT}' && ...`** because `codex exec resume` has no `-C` flag; cwd is inherited from the shell. The initial exec uses `-C "${REPO_ROOT}"` instead.
- **`CODEX_SESSION_ID` is updated only on full success** — ALL of (exit=0 AND stderr has no `Error:`/`thread/resume failed` line AND review file contains a valid `VERDICT:` line with findings on REVISE). On any failure, leave it unchanged and route to the fallback.
- **Session ID capture is two-tier.** Primary: `thread_id` from the first JSONL line of stdout. Secondary (primary empty / malformed / missing `thread_id`): the rollout file that is both `-newer` than the prompt file AND contains the `ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID}` marker for THIS launch, with UUID from the filename. Positive content-binding eliminates wrong-session hazard from both parallel codex invocations AND same-review retries.
- **Every prompt starts with `<!-- ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID} -->` as its first line.** Generate a **fresh 6-digit ATTEMPT_ID per launch** (initial exec, any retry of that exec, each resume in Step 7, any fresh-exec fallback). Never reuse an ATTEMPT_ID within the same review — doing so would let a prior attempt's rollout match the current launch's grep, reintroducing session drift.
- **Secondary-path multi-match is fail-closed, not pick-any.** If the `find ... -exec grep -l ... {} +` returns two or more rollout paths for a single `${REVIEW_ID}-${ATTEMPT_ID}`, abort the round with a diagnostic. This should not happen under correct attempt-scoping; if it does, something is structurally wrong and silent picking would mask it.
- **Prompt delivery is `cat file | codex exec ... -`.** The `- < file` stdin-redirect form is accepted by codex but exits 1 with empty stderr in some Claude Code sandbox configurations. Pipe is portable across both envs observed.
- **The `--json` stdout stream is never human-readable review text** — JSONL events when populated, empty when suppressed by sandbox. Never treat Bash result as review content; the review lives exclusively in `/tmp/codex-review-*.md`.
- **Launch-failure retry** is capped at 1 per round and does NOT consume the 5-round counter. The retry counter is per-round; it resets at the start of every new round and is tracked only in that round's reasoning.
- **Resume is the primary path for rounds 2-5.** Fresh exec is a fallback that runs only when resume fails; it consumes one round from the counter just as a successful resume would.
- **`--last` is never used** — cwd filtering is insufficient to distinguish the current skill session from unrelated parallel codex invocations in the same repo.
- **Fallback after resume failure:** interactive → ask the user (fresh exec vs conclude as not-verified); non-interactive → auto fresh exec if max severity is critical/high, auto conclude-as-not-verified if only medium.
- **`REPO_ROOT` is captured at Step 2** and passed as an absolute literal to every runner dispatch.
- **`RUNNER_SPEC_PATH` is resolved at Step 4** (once per review) with priority: (1) `~/.claude/skills/adversarial-review/references/runner.md` (user-scoped install), (2) Glob `~/.claude/plugins/cache/**/skills/adversarial-review/references/runner.md` and take first hit (plugin-marketplace install), (3) `$(git rev-parse --show-toplevel)/references/runner.md` (dev checkout), (4) abort with installation error. Main never attempts to read Claude's own system prompt / skill-invocation header — that path is hallucination-prone and is explicitly disallowed.
- **Codex-exec mechanics live in the runner subagent** (`references/runner.md`): ATTEMPT_ID generation, prompt-with-marker writing (with a repeated Write call for mtime freshness — NOT Bash `touch`, which may be gated by inherited Plan Mode), synchronous launch, strict checks, two-tier session-id capture with positive content-bind, ONE internal retry on ANY failure type (launch_failure, timeout, stderr-infra), archival mv on resume failure. Main thread never reads codex stdout/stderr/rollout file CONTENT, and never references those paths in its own Bash argv.
- **Two-channel result protocol.** Runner writes structured JSON to `/tmp/codex-runner-result-${REVIEW_ID}.json` (authoritative) AND returns a single `RUNNER_RESULT_AT: <path>` line as its final message. Main extracts the path via regex (tolerant to markdown fences / minor wrapping), reads the JSON, and never relies on raw-JSON-in-message parsing.
- **Main thread reads only**: the runner result JSON at `RESULT_PATH` and the review file at `review_file`. Main does NOT Read `references/runner.md` — the runner spec is passed by path to the subagent, which Reads it itself. No other `/tmp/codex-*` reads.
- **Runner is dispatched via Agent tool** with `subagent_type: general-purpose, model: haiku`. Agent tool call is synchronous (not `run_in_background`).
- **ALL runner failure results are TERMINAL at main** (`launch_failure`, `timeout`, `infra_error`, `input_error`). Runner retries once internally on ANY failure. Main does NOT re-dispatch and does NOT offer the user a retry — those lanes would compound retries across layers. Total codex invocations per round ≤ 2 (matches pre-refactor invariant: 1 initial + 1 retry). Fresh-exec fallback is a NEW round with its own independent 2-attempts budget.
- **`user_warning` from the runner must be surfaced to the user** on a single line BEFORE any other action. This preserves the pre-refactor §2.4.4 "both tiers empty, continuing with previous ID" diagnostic.
- **`CODEX_MODEL` / `CODEX_REASONING`** in the runner input schema refer to the model codex CLI launches (e.g. `gpt-5.4`). The runner's OWN model is Haiku, set via Agent tool's `model: "haiku"`. Do NOT conflate.
- **Resume is the primary path for rounds 2-5.** Fresh-exec fallback consumes one round from the 5-round counter.
- **Step 9 cleanup `rm` glob is UNCHANGED from pre-refactor.** It still covers `/tmp/codex-plan-${REVIEW_ID}.md`, `/tmp/codex-prompt-${REVIEW_ID}.md`, `/tmp/codex-resume-prompt-${REVIEW_ID}.md`, `/tmp/codex-review-${REVIEW_ID}.md`, `/tmp/codex-stdout-${REVIEW_ID}.jsonl`, `/tmp/codex-stderr-${REVIEW_ID}.txt`, `/tmp/codex-stdout-${REVIEW_ID}-failed-resume.jsonl`, `/tmp/codex-stderr-${REVIEW_ID}-failed-resume.txt`. ADD the two new paths introduced by the refactor: `/tmp/codex-body-${REVIEW_ID}.md` and `/tmp/codex-runner-result-${REVIEW_ID}.json`.
- Cleanup is **conditional on terminal state**: remove temp files on approved/max-reached/not-verified; LEAVE them on abort (diagnostic value). Skip all cleanup in Plan Mode.
- Always read-only sandbox — reviewer never writes files.
- Maximum 5 rounds to protect against infinite loops.
+52
View File
@@ -1440,3 +1440,55 @@ the current generation. Rules of thumb:
- Codex CLI command-line reference: https://developers.openai.com/codex/cli/reference
- Codex GitHub issue #12538 (resume `-o` support): https://github.com/openai/codex/issues/12538
- Codex GitHub issue #14544 (resume exec sessions): https://github.com/openai/codex/issues/14544
## §12. Subagent architecture
### §12.1 The residue problem
Before this split, the entire skill ran in the main Claude thread. Each round's `Bash` tool call produced result content that Claude's context caches: the codex stdout JSONL stream (often 200-500KB when populated), the stderr file (small but growing), and the grep-over-rollout that reads file content. Across 5 rounds and a long conversation, cache-read residue accumulated to ~48M tokens per invocation.
### §12.2 The split
Claude Code's Agent tool dispatches a fresh subagent with its own isolated context. When the subagent returns, its context is discarded; only its final text message crosses to main. By making the runner subagent own every codex-exec artifact and return only a ~1KB JSON summary file plus the small review file path, the main thread no longer pays the residue tax.
The runner uses a two-channel protocol: the authoritative structured result is written to `/tmp/codex-runner-result-${REVIEW_ID}.json`, and the runner's final message is a single `RUNNER_RESULT_AT: <path>` line. Main extracts the path with a tolerant regex (markdown fences and minor wrapping do not break parsing) and reads the JSON file directly. This avoids the brittle "raw JSON in message" contract that Haiku's conversational output style would otherwise stress.
**Runner spec is passed by path, not inlined.** Main resolves `RUNNER_SPEC_PATH` (3-tier filesystem lookup in SKILL.md Step 4) and passes the absolute path to the subagent — the subagent Reads runner.md itself. Main never Reads runner.md. Rationale: inlining the full spec (~12K) into every Agent-tool prompt would re-add ~12K × rounds (up to 5) to main's context per review — 60K of avoidable overhead. Bootstrap instruction in the Agent prompt is ~400 bytes; the spec lives only in the disposable subagent context.
### §12.3 Retry budget — single owner
Retry lives in the runner alone, across ALL failure types (launch_failure, timeout, stderr-infrastructure-error). Runner retries once internally (same ATTEMPT_ID-rotation as pre-refactor's round-level retry). Main treats EVERY failure result as terminal-for-this-round: on the initial Step-4 dispatch, terminal = abort; on the Step-7 resume dispatch, terminal = route to fallback (fresh-exec, which is a new round with its own budget).
**The full invariant:** *exactly 2 codex invocations per round, maximum, across all failure types.* The pre-refactor skill had the same budget; the refactor does not loosen it. Putting the retry at a single layer (runner) prevents the worst-case compounding that would occur if main also re-dispatched on failure — specifically Round-2 finding #1 identified that a user-offered timeout retry → fresh-exec cascade could produce up to 6 invocations per round if retry logic existed at multiple layers. Terminal-at-main closes this lane.
Fresh-exec fallback is a *separate round* that consumes a round counter slot and gets its own fresh 2-attempts budget; this matches pre-refactor §2.4 where fresh-exec also counted as one round.
### §12.4 Archival ownership
Resume-to-fresh-exec fallback requires archiving the failed-resume stdout/stderr so they survive the next dispatch's file-reuse. Pre-refactor, main did this via its own `mv`. Post-refactor the runner does it inside Step R5 (only on resume failure). Rationale: main never references `/tmp/codex-stdout-*` or `/tmp/codex-stderr-*` paths in its own Bash argv, strengthening the isolation claim (no leak by path-reference). Archived paths are returned in the result JSON as `archived_stdout` / `archived_stderr`; main includes them in Step 9 cleanup per its unchanged `rm` glob.
### §12.5 Model choice
The runner is a pure pipeline executor: parse inputs, call Bash, validate output, retry once, archive on resume failure, write result JSON. No code understanding, no severity judgment, no review interpretation. Haiku 4.5 is sufficient and ~15× cheaper per token than Opus. The main thread (Opus) keeps all judgment work (applying fixes, deciding round progression, user interaction).
To avoid confusion, the runner's input schema uses `CODEX_MODEL` (the model codex CLI launches) — distinct from the runner's OWN model, which is passed via the Agent tool's `model: "haiku"` parameter. The names are intentionally non-overlapping.
### §12.6 Invariants preserved
The attempt-scoped `ADVERSARIAL-REVIEW-SESSION` marker (round-7 finding) and positive content-bind on rollout files (round-6 finding) both live in the runner. The orchestration-level invariants (5-round cap, resume-before-fresh-exec preference, not-verified terminal state, §2.4.4 user-facing warning on secondary-find zero) all stay in main or flow through the runner's `user_warning` channel.
### §12.7 Plan Mode inheritance (populated by Task 7 Step 4 — hypothesis until verified)
> **Status:** this subsection is empirically determined by Task 7 Step 4's Plan Mode smoke test. Until that task runs and the implementer updates this subsection with observed behavior, treat every claim here as a HYPOTHESIS, not documented fact.
**Hypothesis to test:** Plan Mode restrictions propagate from main to any subagent main dispatches; the subagent inherits limitations on Write/Edit/Bash. If this holds:
- Main's Write to `/tmp/codex-body-*.md` may trigger a permission prompt or exit Plan Mode.
- The runner's Writes to `/tmp/codex-prompt-*.md` (Step R2, including the mtime-bump repeat Write) may also trigger prompts.
- The runner's `codex exec` (read-only sandbox) should be unaffected since it writes nothing to the user's repo.
**What Task 7 Step 4 must determine:**
1. Does dispatching the Agent tool from Plan Mode work (is it blocked, does it prompt, does it just work)?
2. Does the subagent inherit Plan Mode, or does it run as unrestricted?
3. If Plan Mode propagates, does the subagent's Write (for the repeat-Write mtime bump) fail, prompt, or succeed silently?
**After Task 7 Step 4 runs, REWRITE this subsection with the observed answers.** Delete the hypothesis framing and state the observed behavior as fact. Until then, anyone reading §12.7 should understand it is speculative.
File diff suppressed because it is too large. Load diff
+258
View File
@@ -0,0 +1,258 @@
# Adversarial-Review Runner Subagent
> This file is read by a Haiku subagent dispatched from `SKILL.md` Step 4 or Step 7. It is NOT loaded in the main thread.
You are a thin runner subagent. Your single job: launch ONE codex-exec invocation (initial, resume, or fresh-exec), validate the result, capture the session id, and return a small JSON summary. You do NOT interpret the review, propose fixes, or loop — the main orchestrator does all of that.
## Input contract
The main thread dispatches you via Claude Code's Agent tool. The prompt contains a YAML-like input block. Parse these fields; if any required field is missing, write an input_error result to the result file (see Output contract) and return.
```yaml
REVIEW_ID: <string, format "{unix_ts}-{8-digit}">
REPO_ROOT: <absolute path, validated by main>
OPERATION: initial | resume | fresh-exec
CODEX_MODEL: <e.g. gpt-5.4> # the model codex CLI launches; DO NOT confuse with your own (Haiku) model
CODEX_REASONING: <low | medium | high | xhigh>
PROMPT_BODY_PATH: <absolute path to file containing the review prompt body WITHOUT the session marker; main writes this before dispatch>
CODEX_SESSION_ID: <UUID, required only when OPERATION=resume>
RESULT_PATH: /tmp/codex-runner-result-<REVIEW_ID>.json # you write the structured result here
```
For `OPERATION=initial` and `OPERATION=fresh-exec`, `CODEX_SESSION_ID` is absent (ignore if present).
## Output contract — two-channel
To avoid fragility of JSON-in-final-message (Haiku frequently wraps structured output in markdown fences or adds preamble), you return results via TWO channels:
**Channel 1 — result file (authoritative).** Write the JSON object below to `${RESULT_PATH}` via Write tool. Main reads this file directly; its bytes are the contract. Do NOT omit any field — use `null` for absent values.
```json
{
"result": "success" | "launch_failure" | "timeout" | "infra_error" | "input_error",
"verdict": "APPROVED" | "REVISE" | null,
"review_file": "/tmp/codex-review-<REVIEW_ID>.md" | null,
"codex_session_id": "<uuid>" | null,
"attempt_id": "<6-digit>",
"errors": "<short diagnostic, ≤500 chars>" | null,
"archived_stdout": "/tmp/codex-stdout-<REVIEW_ID>-failed-resume.jsonl" | null,
"archived_stderr": "/tmp/codex-stderr-<REVIEW_ID>-failed-resume.txt" | null,
"user_warning": "<one-line message main should surface to user>" | null
}
```
**Channel 2 — final message (short).** Your FINAL message to main should be a single line:
```
RUNNER_RESULT_AT: <RESULT_PATH>
```
Example: `RUNNER_RESULT_AT: /tmp/codex-runner-result-1711872000-48217593.json`
Main's parser is tolerant: it searches the ENTIRE message for a match of the unanchored regex `RUNNER_RESULT_AT:\s+(\S+)` (first match wins; works inside markdown fences, after preamble, or surrounded by other text). Even so, emitting the spec line cleanly (no fence, no preamble) eliminates edge cases.
If your message lacks the line entirely, main falls back to a filesystem Glob for `/tmp/codex-runner-result-${REVIEW_ID}.json` — the path is deterministic from REVIEW_ID, which main already holds. If the Glob also fails (file not written), main treats the run as `infra_error`.
Rules:
- `result=success` ⇒ `verdict` and `review_file` must be set. `codex_session_id` must be set iff `verdict=REVISE` (or null per §2.4.4 on resume zero-find — see Step R4.4).
- `result=timeout` ⇒ codex timed out (exit 124). `review_file` may be null.
- `result=launch_failure` ⇒ infrastructure retry (one internal retry) already failed. Main treats this as TERMINAL — it will NOT re-dispatch you. `errors` MUST include the tail of stderr.
- `result=infra_error` ⇒ something outside codex (e.g. `/tmp` not writable).
- `user_warning` is non-null when main should surface a one-line warning to the user (e.g. §2.4.4 zero-find on resume).
- Do NOT return the review text in the JSON. Main reads `review_file` directly.
## Step-by-step
### Step R1: Generate ATTEMPT_ID
Generate a fresh 6-digit random integer. Use `printf` with `$RANDOM`:
```bash
printf '%06d\n' $((RANDOM * RANDOM % 1000000))
```
Save the output as `${ATTEMPT_ID}` for this invocation. Generate a NEW ATTEMPT_ID on every retry (Step R5).
### Step R2: Build the launch prompt file
Read `${PROMPT_BODY_PATH}` (main wrote it before dispatching you).
For `OPERATION=initial` or `OPERATION=fresh-exec`:
- Write `/tmp/codex-prompt-${REVIEW_ID}.md` with first line `<!-- ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID} -->` followed by the body.
For `OPERATION=resume`:
- Write `/tmp/codex-resume-prompt-${REVIEW_ID}.md` with the same marker-first structure.
Use the Write tool (not `cat <<EOF` via Bash — Write is simpler and does not have quoting edge cases).
**Bump mtime via a second Write** to ensure the prompt file's mtime is strictly later than any rollout file from a prior attempt. Rewrite the same bytes to the same path (Write tool, not Bash `touch` — Bash may be restricted under inherited Plan Mode; Write is already the tool used for the initial body, so whatever gating applies is already cleared by the first Write).
On systems with coarse mtime granularity (1s), two successive Writes within the same second can produce identical mtimes; the repeat Write forces the kernel to update the mtime. On Plan Mode-inherited subagents the first Write may have already prompted the user; the second Write to the identical path reuses that same permission grant.
Alternatively and equivalently safe: skip the mtime bump entirely and rely on ATTEMPT_ID rotation alone — the positive content-match in Step R4.4 binds on the marker, not solely on `-newer`. If the retry's new ATTEMPT_ID is embedded in the prompt's first line (which it is), no prior rollout can false-match. The `-newer` condition is a second guard, not a primary one. If the repeat-Write approach fails in practice, drop it and rely on content-match + multi-match-aborts.
### Step R3: Launch codex
**Synchronous launch only.** Always invoke the Bash tool with `run_in_background: false` (the default). Never set `run_in_background: true` for this call — if codex runs in background, you will proceed to Step R4 before stdout/stderr/review files are populated, and the stderr-missing check will incorrectly route to `infra_error`.
For `OPERATION=initial` and `OPERATION=fresh-exec`:
```bash
cat /tmp/codex-prompt-${REVIEW_ID}.md | timeout 600 codex exec --json \
-m ${CODEX_MODEL} \
-c model_reasoning_effort=${CODEX_REASONING} \
-s read-only \
-C "${REPO_ROOT}" \
-o /tmp/codex-review-${REVIEW_ID}.md \
- \
> /tmp/codex-stdout-${REVIEW_ID}.jsonl \
2>/tmp/codex-stderr-${REVIEW_ID}.txt
```
Bash tool `timeout` parameter: `620000` (10 min + headroom).
For `OPERATION=resume`:
```bash
cd "${REPO_ROOT}" && cat /tmp/codex-resume-prompt-${REVIEW_ID}.md | timeout 600 codex exec resume --json \
${CODEX_SESSION_ID} \
-o /tmp/codex-review-${REVIEW_ID}.md \
- \
> /tmp/codex-stdout-${REVIEW_ID}.jsonl \
2>/tmp/codex-stderr-${REVIEW_ID}.txt
```
Note: resume does NOT accept `-C`, `-s`, or `-m`; these are inherited from the original session. Use `cd` to pin cwd.
Substitute literal values for every `${...}` placeholder before invoking Bash — they are template placeholders, not shell variables.
### Step R4: Post-launch strict checks
Do these in order. Stop and return as soon as one fails.
**Check R4.1: Exit code.**
- `124` → route to retry (Step R5). Same retry budget as any other failure — ONE retry per dispatch total. If retry also returns 124, write `{"result":"timeout",...}` and return. (Retrying on timeout keeps the 2-attempts-per-round invariant consistent across failure types; main treats timeout as terminal just like launch_failure.)
- `≠ 0 and ≠ 124` → read `/tmp/codex-stderr-${REVIEW_ID}.txt`, route to retry (Step R5).
- `0` → proceed.
**Check R4.2: Stderr sanity.** Read `/tmp/codex-stderr-${REVIEW_ID}.txt`.
- File missing → return `{"result":"infra_error","errors":"stderr file missing — /tmp writability?",...}`.
- File contains a line matching `^Error:` or `Failed to write` → route to retry (Step R5).
**Check R4.3: Review file sanity.** Read `/tmp/codex-review-${REVIEW_ID}.md`.
- File missing or empty → route to retry (Step R5).
- Does NOT contain a line matching `^VERDICT: (APPROVED|REVISE)$` → route to retry.
- Verdict is `REVISE` AND file contains NO line matching `\[severity:\s*(critical|high|medium)` → route to retry (reviewer format drift).
- Verdict is `APPROVED` → write this EXACT JSON object to `${RESULT_PATH}` and return the `RUNNER_RESULT_AT:` line:
```json
{
"result": "success",
"verdict": "APPROVED",
"review_file": "<absolute path to /tmp/codex-review-REVIEW_ID.md, substituted>",
"codex_session_id": null,
"attempt_id": "<the current ATTEMPT_ID string>",
"errors": null,
"archived_stdout": null,
"archived_stderr": null,
"user_warning": null
}
```
- Verdict is `REVISE` → proceed to Check R4.4.
**Check R4.4: Capture session id — two tiers.**
*Primary — first line of JSONL stdout:*
Read `/tmp/codex-stdout-${REVIEW_ID}.jsonl`. If the first line parses as JSON with a `thread_id` field matching `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, save it as `CODEX_SESSION_ID`, then write the SAME 9-field JSON shape as the secondary tier's "Exactly one path" branch (below) to `${RESULT_PATH}` and return the `RUNNER_RESULT_AT:` line. Otherwise fall through to the secondary tier.
*Secondary — rollout content-match:*
The anchor file is `/tmp/codex-prompt-${REVIEW_ID}.md` for initial/fresh-exec, or `/tmp/codex-resume-prompt-${REVIEW_ID}.md` for resume.
```bash
find ~/.codex/sessions -name 'rollout-*.jsonl' -newer <anchor> -exec grep -l 'ADVERSARIAL-REVIEW-SESSION: ${REVIEW_ID}-${ATTEMPT_ID}' {} + 2>/dev/null
```
**Interpret the result by STDOUT, not exit code.** Split the command's stdout on newlines; count non-empty lines. Empty stdout means ZERO paths regardless of the pipeline's exit status (`find` returning no matches and `grep -l` matching nothing in found files both yield empty stdout with different exit codes; treat both as zero).
- **Exactly one path** → extract the trailing UUID from the filename (pattern `rollout-<ISO-timestamp>-<UUID>.jsonl`; UUID is the 36-char hex-and-dashes segment before `.jsonl`), then write this EXACT JSON object to `${RESULT_PATH}` (all 9 fields explicitly; do NOT leave any omitted or as literal placeholder like `"<uuid>"`):
```json
{
"result": "success",
"verdict": "REVISE",
"review_file": "<absolute path to /tmp/codex-review-REVIEW_ID.md, substituted>",
"codex_session_id": "<actual 36-char UUID you extracted from the filename>",
"attempt_id": "<the current ATTEMPT_ID string>",
"errors": null,
"archived_stdout": null,
"archived_stderr": null,
"user_warning": null
}
```
- **Zero paths for resume** → write this EXACT JSON object (9 fields, `codex_session_id` is null, `user_warning` carries the §2.4.4 diagnostic):
```json
{
"result": "success",
"verdict": "REVISE",
"review_file": "<absolute path substituted>",
"codex_session_id": null,
"attempt_id": "<ATTEMPT_ID>",
"errors": null,
"archived_stdout": null,
"archived_stderr": null,
"user_warning": "Step 7 session-id refresh: both tiers empty, continuing with previous ID per DESIGN.md §2.4.4"
}
```
- **Zero paths for initial/fresh-exec** → route to retry (Step R5). Main needs the id to launch next round. Set `errors: "session-id capture failed: both tiers empty on initial/fresh-exec"`.
- **Multiple paths** → write `launch_failure` result with `errors: "multiple rollouts matched marker — aborting to avoid wrong-session bind"`. Do NOT pick one.
(The two `success` JSON shapes are inlined above per branch. Every success path through R4.4 MUST emit a complete 9-field JSON object — never rely on implicit defaults, never leave a field omitted, never write a literal placeholder like `"<uuid>"` in the output.)
### Step R5: Retry once on any failure (TERMINAL — main will not re-dispatch)
You have at most ONE retry per dispatch. This retry is the ONLY retry in the system — main treats your `launch_failure` result as terminal and will NOT re-dispatch you. Track the retry counter in your reasoning.
On retry:
1. Generate a NEW `ATTEMPT_ID` (the old one stays in the old rollout; we must not let the grep match it again).
2. Rewrite the prompt file with the new marker (using the Write tool; the write itself bumps mtime — do NOT use Bash `touch`, which may be gated by inherited Plan Mode on the subagent).
3. Re-launch (same Step R3 command, still `run_in_background: false`).
4. Re-run checks R4.1–R4.4. **This is the second and final attempt.** On this re-run, any check's "route to retry" outcome becomes terminal — do NOT re-enter R5. Apply the terminal-result rule below (timeout if exit 124 again, else launch_failure).
If the second attempt also fails any check:
- For `OPERATION=resume`: before writing the `launch_failure` result, **archive the diagnostic files** (main will need them for the fallback fresh-exec which reuses the same base paths):
```bash
mv /tmp/codex-stdout-${REVIEW_ID}.jsonl /tmp/codex-stdout-${REVIEW_ID}-failed-resume.jsonl 2>/dev/null
mv /tmp/codex-stderr-${REVIEW_ID}.txt /tmp/codex-stderr-${REVIEW_ID}-failed-resume.txt 2>/dev/null
```
Then write the result with `archived_stdout` and `archived_stderr` set to the `-failed-resume.*` paths.
- For `OPERATION=initial` or `OPERATION=fresh-exec`: no archival needed (there is no next attempt within this REVIEW_ID to collide). Leave files at their normal paths for main's diagnostic read (main is allowed to `mv`/`rm` by path; it just doesn't read content).
Write the appropriate terminal result and return the `RUNNER_RESULT_AT: ...` line:
- Second attempt exit was 124 → write `{"result":"timeout","errors":"codex exceeded 600s on both attempts", ...}` (9 fields, all others null as applicable).
- Any other failure mode → write `launch_failure` with stderr tail (≤500 chars) in `errors`.
In both cases, fill all 9 fields (set `archived_stdout`/`archived_stderr` only when the archival mv in the OPERATION=resume branch ran, else null; set `user_warning` null; set `verdict` null; set `review_file` to `/tmp/codex-review-${REVIEW_ID}.md` only if that file contains a valid VERDICT line, else null).
### Step R6: Cleanup
Do NOT delete `/tmp/codex-*` files. The main orchestrator owns the review-lifecycle cleanup at SKILL.md Step 9. Leaving files in place lets main:
- Read `review_file` after parsing your result.
- Keep the `-failed-resume.*` archives available for the fresh-exec fallback.
- Clean the whole set (including `-failed-resume.*`) when the review concludes via its existing Step 9 `rm` glob.
The one exception is the `mv` in Step R5 above — this is NOT cleanup (files are preserved, just renamed to avoid collision with the imminent fresh-exec). Doing the `mv` in the runner rather than main eliminates the isolation-claim drift that would otherwise occur if main had to touch stdout/stderr paths in its own Bash argv.
## Notes
- You run as a Haiku subagent. Your 250K context is disposed when you return. Anything you read (stderr files, rollout paths, JSONL streams) does NOT reach the main thread — that is the whole point.
- Do NOT ask the main thread clarifying questions. If input is missing or malformed, write an `input_error` result to `${RESULT_PATH}` and return the `RUNNER_RESULT_AT:` line.
- Do NOT attempt to apply fixes, interpret severity, or re-run more than one retry. The 5-round orchestration loop lives in main.
- The final line of your message is ONLY `RUNNER_RESULT_AT: <path>` — nothing before, nothing after, no markdown fence. Main's regex tolerates minor wrapping, but adhering to the spec eliminates edge cases entirely.