diff --git a/AGENTS.md b/AGENTS.md index 4b65e3e..0eb2905 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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. diff --git a/README.md b/README.md index cd36481..d4c6f92 100644 --- a/README.md +++ b/README.md @@ -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*)" ```
@@ -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 diff --git a/SKILL.md b/SKILL.md index 20100e7..9ebab94 100644 --- a/SKILL.md +++ b/SKILL.md @@ -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:** ``` - -``` - -Example: ``. - -- `${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:** - -``` - 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 ``` -**Prompt for code review (<= 50 files):** +**Prompt body for code review (≤ 50 files):** ``` - 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 ``` -**Prompt for code review (> 50 files):** +**Prompt body for code review (> 50 files):** + +Same as ≤ 50 files above, but the `` section is replaced with: -Same prompt as above, but the `` section without the file list: ``` Review the code changes in this repo. @@ -301,121 +287,133 @@ Run to see changed files and full diffs. ``` -**Prompt for code-vs-plan review:** +**Prompt body for code-vs-plan review:** -Same prompt as code review, but the `` section is extended: -``` - -Review the code changes in this repo against the implementation plan in . -Changed files: +Same as code review (≤ 50 or > 50 variant depending on file count), but: +- `` is extended to reference the plan file: `Review the code changes in this repo against the implementation plan in .` +- `` 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? + ``` - 50> +**Substitute template placeholders BEFORE writing to disk:** -Changes include: . -Run to see the full diffs. - -``` +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 ``: -``` -- 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 | +| `` | captured at Step 3 | plan, code-vs-plan | +| `` | result of `git diff --name-only` + `git diff --cached --name-only` (or branch diff) from Step 3 | code, code-vs-plan (≤50 files only) | +| `` | human-readable description derived from which diff commands had content | code, code-vs-plan | +| `` | 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 ``), 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////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 ` 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":"","...":...} - ``` +**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--.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: + +``` +⚠ +``` + +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 `⚠ ` 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 `⚠ ` 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: ``` - - 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: +REPO_ROOT: +OPERATION: resume +CODEX_MODEL: +CODEX_REASONING: +PROMPT_BODY_PATH: /tmp/codex-resume-body-.md +RESULT_PATH: /tmp/codex-runner-result-.json +CODEX_SESSION_ID: +--- ``` -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 `⚠ ` 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: - -``` - -[Original adversarial prompt for the current mode, from Step 4] - -## Previous review rounds - -### Round 1 findings (verbatim from earlier in this conversation): - - -### Round 1 fixes: - - -### Round 2 findings (verbatim): -<...> - -### Round 2 fixes: -<...> - -## Current state of the artifact -[plan mode] Full current plan text: -[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 `` 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: ` 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. diff --git a/docs/DESIGN.md b/docs/DESIGN.md index b99cd2a..e04fca2 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -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: ` 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. diff --git a/docs/superpowers/plans/2026-04-19-subagent-orchestration.md b/docs/superpowers/plans/2026-04-19-subagent-orchestration.md new file mode 100644 index 0000000..3acd993 --- /dev/null +++ b/docs/superpowers/plans/2026-04-19-subagent-orchestration.md @@ -0,0 +1,1181 @@ +# Subagent Orchestration Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Extract codex-exec orchestration (prompt writing, launch, strict checks, session-id capture, retry) from the main-thread skill into a thin Haiku subagent runner. Main thread keeps review display + code fixes + round control. Eliminates the ~48M-token residue tax that currently bloats the main thread context after each `/adversarial-review` invocation. + +**Architecture:** Main thread performs mode detection, review-material prep, review display, and code fixes — all small-context operations. The expensive mechanics (codex launch, JSONL parsing, rollout-file content-matching, retry on launch failure) run inside a Haiku subagent dispatched via the Agent tool. The subagent's 250K context is disposed when it completes; only a small JSON result (~1K) plus the final review file path crosses back to main. Main reads `/tmp/codex-review-${REVIEW_ID}.md` (2-5K) for the verbatim display. + +**Tech Stack:** Bash, Codex CLI, Claude Code's Agent tool (subagent_type: general-purpose, model: haiku), markdown. + +--- + +## File Structure + +- **Modify:** `SKILL.md` — Steps 4, 7, 9 and Rules section. Steps 1-3, 5, 6, 8 remain in main thread. +- **Create:** `references/runner.md` — full runner subagent contract + mechanics (ATTEMPT_ID generation, prompt writing, codex launch, strict checks, two-tier session-id capture, retry logic). +- **Modify:** `AGENTS.md` — update orientation to document main + subagent split. +- **Modify:** `DESIGN.md` — add §3 "Subagent architecture" explaining the split + residue-tax rationale. + +--- + +## Baseline (RED) + +Already observed by the user: a single `/adversarial-review` invocation in a long session leaves ~48M tokens of cache_read residue in the main thread. This is because every codex-exec artifact (stdout JSONL, stderr, rollout file paths, rollout content for grep-matching) passes through main-thread Bash tool results. + +**Test-passes criterion (GREEN, measured in Task 7 Step 3):** a post-refactor invocation on an equivalent trivial change leaves ≤15K tokens of context growth in main (review markdown 2-5K + runner result JSON ~1K + orchestration overhead). A delta of ~48M tokens would mean the refactor did not take effect; a delta of ~540K would mean the subagent's context is leaking back to main. Target is bounded, measurable, and falsifiable. + +--- + +## Task 1: Create `references/` directory and stub `runner.md` + +**Files:** +- Create: `references/runner.md` (placeholder header only; full content in Task 2) + +- [ ] **Step 1: Create directory and file** + +```bash +mkdir -p /home/dementev/sources/adversarial-review/references +``` + +Write `/home/dementev/sources/adversarial-review/references/runner.md` with only the header: + +```markdown +# 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. See `SKILL.md` for the orchestrator contract. + +(Content added in Task 2.) +``` + +- [ ] **Step 2: Commit the scaffolding** + +```bash +cd /home/dementev/sources/adversarial-review +git add references/runner.md +git commit -m "$(cat <<'EOF' +feat(skill): scaffold references/ for runner subagent + +Empty placeholder for subagent orchestration refactor. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 2: Write the runner subagent contract in `references/runner.md` + +**Files:** +- Modify: `references/runner.md` + +- [ ] **Step 1: Replace file content with the full runner spec** + +Write this content to `references/runner.md` (replacing the placeholder): + +````markdown +# 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: +REPO_ROOT: +OPERATION: initial | resume | fresh-exec +CODEX_MODEL: # the model codex CLI launches; DO NOT confuse with your own (Haiku) model +CODEX_REASONING: +PROMPT_BODY_PATH: +CODEX_SESSION_ID: +RESULT_PATH: /tmp/codex-runner-result-.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-.md" | null, + "codex_session_id": "" | null, + "attempt_id": "<6-digit>", + "errors": "" | null, + "archived_stdout": "/tmp/codex-stdout--failed-resume.jsonl" | null, + "archived_stderr": "/tmp/codex-stderr--failed-resume.txt" | null, + "user_warning": "" | null +} +``` + +**Channel 2 — final message (short).** Your FINAL message to main should be a single line: + +``` +RUNNER_RESULT_AT: +``` + +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 `` 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 < /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` → return `{"result":"success","verdict":"APPROVED","review_file":"/tmp/codex-review-${REVIEW_ID}.md","codex_session_id":null,"attempt_id":"${ATTEMPT_ID}","errors":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 as `CODEX_SESSION_ID` and return success. Otherwise fall through. + +*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 -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--.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 `""`): + +```json +{ + "result": "success", + "verdict": "REVISE", + "review_file": "", + "codex_session_id": "", + "attempt_id": "", + "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": "", + "codex_session_id": null, + "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 `""` in the output.) + +### Step R5: Retry once on launch 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. + +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 `launch_failure` result (with stderr tail ≤500 chars in `errors`) and return the `RUNNER_RESULT_AT: ...` line. + +### 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: ` — nothing before, nothing after, no markdown fence. Main's regex tolerates minor wrapping, but adhering to the spec eliminates edge cases entirely. +```` + +- [ ] **Step 2: Verify the file is syntactically reasonable** + +```bash +wc -l /home/dementev/sources/adversarial-review/references/runner.md +``` + +Expected: roughly 180-220 lines. + +- [ ] **Step 3: Commit** + +```bash +cd /home/dementev/sources/adversarial-review +git add references/runner.md +git commit -m "$(cat <<'EOF' +feat(skill): add runner subagent contract + +Defines input/output JSON contract, step-by-step mechanics, and +cleanup ownership for the thin Haiku runner. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 3: Refactor `SKILL.md` Step 4 — dispatch subagent for initial launch + +**Files:** +- Modify: `SKILL.md` — Step 4 section (roughly lines 129-418 in current file) + +- [ ] **Step 1: Read current SKILL.md Step 4** + +Use Read on `/home/dementev/sources/adversarial-review/SKILL.md` offset 129, limit 290 to confirm line boundaries before editing. + +- [ ] **Step 2: Replace Step 4 content** + +Replace the entire block from the `### Step 4:` heading through the end of Step 4 (just before `### Step 5:`) with: + +````markdown +### Step 4: Build the prompt body, dispatch the runner subagent + +Main thread composes the prompt BODY (without the session marker — the subagent adds it). Select the right template from below based on review mode. + +**Prompt body for plan review:** + +``` + +You are a senior adversarial reviewer of implementation plans. +Your job is to break confidence in the plan, not to validate it. + + + +Default to skepticism. Assume the plan has gaps until the evidence says otherwise. +Do not give credit for good intent or likely follow-up work. +If something only works on the happy path, treat that as a real weakness. + + + +Review the implementation plan in . + + + +Check each area. Skip if not applicable: +- Feasibility — will this approach actually work given the codebase and constraints? +- Missing steps — what is forgotten or assumed but not stated? +- Risk areas — what could go wrong during implementation? Data loss? Downtime? +- Sequencing — are steps in the right order? Are there hidden dependencies? +- Alternatives — is there a simpler or more robust approach? +- Rollback — can this be safely reverted if it fails halfway? +- Security — auth, data exposure, injection, unsafe operations + + + +Each finding MUST answer: +1. What can go wrong? (concrete scenario, not hypothetical) +2. Why is this plan vulnerable? (cite specific section) +3. Impact — what breaks and how badly? +4. Recommendation — specific change to the plan + + + +DO NOT comment on: formatting, wording style, speculative issues without concrete trigger scenario. + + + +Prefer one strong finding over several weak ones. +If the plan is solid, say so clearly — false positives erode trust. + + + +Use markdown headers for sections: Summary, Findings, Verdict. + +Summary: one paragraph — what this plan does and your overall assessment. + +Findings: for each finding, use a sub-header with [severity: critical|high|medium] and title. +Include these fields per finding: +- **Section:** which part of the plan +- **What can go wrong:** ... +- **Why vulnerable:** ... +- **Impact:** ... +- **Recommendation:** ... + +If no findings: "No actionable findings." + +Verdict rules: approve if no findings or all low severity; revise if any high/critical. +Choose exactly one. The LAST line of your response must be one of: +VERDICT: APPROVED +VERDICT: REVISE + +``` + +**Prompt body for code review (≤ 50 files):** + +``` + +You are a senior adversarial code reviewer. +Your job is to break confidence in the change, not to validate it. + + + +Default to skepticism. Assume the change can fail in subtle, high-cost, +or user-visible ways until the evidence says otherwise. +Do not give credit for good intent, partial fixes, or likely follow-up work. +If something only works on the happy path, treat that as a real weakness. + + + +Review the code changes in this repo. Changed files: + + + +Changes include: . +Run to see the full diffs. + + + +Check each area. Skip if not applicable to this change: +- Auth & permissions: bypasses, privilege escalation, missing checks +- Data integrity: loss, corruption, partial writes, constraint violations +- Race conditions: TOCTOU, concurrent access, deadlocks +- Rollback safety: can this change be safely reverted? +- Schema drift: migrations, backward compatibility, data format changes +- Error handling: swallowed errors, missing retries, cascading failures +- Observability: will operators know when this breaks? + + + +Each finding MUST answer: +1. What can go wrong? (concrete scenario, not hypothetical) +2. Why is this code vulnerable? (cite specific file and lines) +3. Impact — what breaks and how badly? (data loss > downtime > degraded UX) +4. Recommendation — specific fix with code reference + + + +DO NOT comment on: code style, formatting, naming conventions, +speculative issues without concrete trigger scenario, +"nice to have" improvements unrelated to correctness or safety. + + + +Prefer one strong finding over several weak ones. +Severity: critical (data loss/security) > high (bug in prod) > medium (edge case). +If the change is solid, say so clearly — false positives erode trust. + + + +Use markdown headers for sections: Summary, Findings, Verdict. + +Summary: one paragraph — what this change does and your overall assessment. + +Findings: for each finding, use a sub-header with [severity: critical|high|medium] and title. +Include these fields per finding: +- **File:** path/to/file.ext lines N-M +- **What can go wrong:** ... +- **Why vulnerable:** ... +- **Impact:** ... +- **Recommendation:** ... + +If no findings: "No actionable findings." + +Verdict rules: approve if no findings or all low severity; revise if any high/critical. +Choose exactly one. The LAST line of your response must be one of: +VERDICT: APPROVED +VERDICT: REVISE + +``` + +Note: the body is IDENTICAL to the pre-refactor SKILL.md Step 4 code-review prompt EXCEPT the leading `` line is stripped — the runner subagent prepends the session marker in Step R2. + +> **Sync-drift note:** Once Task 3 is implemented, SKILL.md Step 4 is the CANONICAL source for these prompt bodies. This plan document is a point-in-time artifact committed to `docs/superpowers/plans/`; future edits to the prompts happen in SKILL.md, not here. If you find a drift between this plan and SKILL.md post-merge, SKILL.md wins. + +**Prompt body for code review (> 50 files):** + +Same as ≤ 50 files above, but the `` section is replaced with: + +``` + +Review the code changes in this repo. +Changes include: . +Run to see changed files and full diffs. + +``` + +**Prompt body for code-vs-plan review:** + +Same as code review (≤ 50 or > 50 variant depending on file count), but: +- `` is extended to reference the plan file: `Review the code changes in this repo against the implementation plan in .` +- `` 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? + ``` + +**Substitute template placeholders BEFORE writing to disk:** + +The inlined prompt bodies above contain template placeholders that main must resolve with real captured values before the Write. Pre-refactor this happened implicitly at Step 4 where values were in lexical scope; post-refactor main writes the body to a file and hands the PATH to the runner, so substitution MUST happen in main. Placeholders per mode: + +| Placeholder | Value source | Applies to | +|---|---|---| +| `${REVIEW_ID}` | captured at Step 2 | all modes (in any comment/reference — note the session marker comes from runner, not here) | +| `${REPO_ROOT}` | captured at Step 2 | all modes | +| `${BASE_BRANCH}` | captured at Step 2 (code & code-vs-plan only) | code, code-vs-plan | +| `` | captured at Step 3 | plan, code-vs-plan | +| `` | result of `git diff --name-only` + `git diff --cached --name-only` (or branch diff) from Step 3 | code, code-vs-plan (≤50 files only) | +| `` | human-readable description derived from which diff commands had content | code, code-vs-plan | +| `` | the exact commands main determined at Step 3 (e.g. `git diff`, `git diff --cached`, `git diff ${BASE_BRANCH}...HEAD`) | code, code-vs-plan | + +Main substitutes each placeholder with its literal resolved value in the body text. The resulting string goes to the Write tool. + +**Capture user overrides for `CODEX_MODEL` / `CODEX_REASONING` (Step 1 argument parsing):** + +The skill supports overrides like `/adversarial-review xhigh`, `/adversarial-review medium`, `/adversarial-review model:gpt-5.3-codex`. At Step 1 (mode detection), main also captures: + +- `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`. + +These are passed into the runner YAML input block below. Forgetting this capture silently disables the user-facing override feature. + +**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 — Task 7 Step 4 determines the empirical behavior. + +**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 +ls ~/.claude/skills/adversarial-review/references/runner.md 2>/dev/null +``` + +If exit 0, set `RUNNER_SPEC_PATH` to the expanded absolute path and proceed. + +2. **Plugin-marketplace install** (secondary): Claude Code's plugin system installs skills at paths like `~/.claude/plugins/cache////skills/adversarial-review/`. Glob to find it: + +```bash +ls ~/.claude/plugins/cache/*/*/*/skills/adversarial-review/references/runner.md 2>/dev/null | head -1 +``` + +If the Glob returns one or more paths, take the first and set `RUNNER_SPEC_PATH`. + +3. **Dev checkout** (tertiary): if neither above, try `$(git rev-parse --show-toplevel)/references/runner.md`: + +```bash +REPO=$(git rev-parse --show-toplevel 2>/dev/null) && ls "$REPO/references/runner.md" 2>/dev/null +``` + +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. + +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. + +**Dispatch the runner subagent via Agent tool:** + +Read `${RUNNER_SPEC_PATH}` once (main thread). This is the subagent's full spec. + +Invoke the Agent tool with: +- `subagent_type: "general-purpose"` +- `model: "haiku"` +- `description: "Adversarial-review runner, round N"` (N is the current round number) +- `prompt:` the concatenation of: + 1. The full text of `references/runner.md` you just read (as instructions). + 2. A blank line. + 3. A literal YAML input block with the values substituted (use `CODEX_MODEL` and `CODEX_REASONING` to avoid confusion with the subagent's own Haiku model): + +```yaml +--- +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 real values. `RESULT_PATH` always follows the pattern `/tmp/codex-runner-result-${REVIEW_ID}.json`. + +**Do NOT run the Agent tool call in background.** Wait for the subagent to return. (Runner's own codex exec is also synchronous per Step R3.) + +**Parse the subagent's response — two-channel protocol:** + +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. + +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. + +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`. + +**If `user_warning` is non-null, surface it as a SEPARATE short user-visible message BEFORE the Step 5 verbatim-review message.** Format: + +``` +⚠ +``` + +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 (enforced by the table above):** 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 across ALL failure types — this matches pre-refactor and closes Round-2 finding #1 (timeout compounding). + +> **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 3: Verify SKILL.md is still well-formed markdown** + +```bash +head -c 200 /home/dementev/sources/adversarial-review/SKILL.md +``` + +- [ ] **Step 4: Update SKILL.md Step 5 "VERY NEXT MESSAGE" wording to accommodate `⚠ user_warning`** + +Current SKILL.md Step 5 (around line 439 in the pre-refactor file) contains: +> Your VERY NEXT MESSAGE to the user must begin with the header below, followed by the file contents verbatim. + +And the precondition gate (around line 461): +> 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. + +These two rules conflict with the new design where `⚠ ` is emitted as its own turn BEFORE the Step 5 verbatim message. Update Step 5 as follows: + +Replace `Your VERY NEXT MESSAGE to the user must begin with the header below` with `Your next user-visible message that is NOT a one-line \`⚠ \` diagnostic must begin with the header below`. + +Replace the precondition gate's check `sent a user-visible message in THIS round whose body contains the verbatim review text` with `sent a user-visible message in THIS round whose body contains the verbatim review text (NOT counting short \`⚠ \` diagnostic messages)`. + +This reconciles Step 5's blocking-display contract with the new warning-before-display contract. Make this edit in the SAME commit as Step 3 (below). + +- [ ] **Step 5: Commit** + +```bash +cd /home/dementev/sources/adversarial-review +git add SKILL.md +git commit -m "$(cat <<'EOF' +refactor(skill): dispatch initial codex launch via Haiku subagent + +Replaces inline codex-exec + strict checks + session-id capture +in Step 4 with Agent tool dispatch. Runner spec lives in +references/runner.md. Main thread reads only the final review file +(~5K) instead of stdout/stderr/rollout artifacts (~48M residue). + +Also updates Step 5 "VERY NEXT MESSAGE" wording to permit a +preceding one-line user_warning diagnostic message. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 4: Refactor `SKILL.md` Step 7 — dispatch subagent for resume + +**Files:** +- Modify: `SKILL.md` — Step 7 section (roughly lines 481-612 in current file) + +- [ ] **Step 1: Read current Step 7** + +Use Read on `SKILL.md` to confirm the current boundaries of Step 7. + +- [ ] **Step 2: Replace Step 7 with the resume-via-subagent version** + +Replace the entire block from `### Step 7:` through the end of Step 7 (just before `### Step 8:`) with: + +````markdown +### 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** when resume itself fails. + +**Step 7.1: Write the resume prompt body to disk.** + +Write `/tmp/codex-resume-body-${REVIEW_ID}.md` containing: + +``` +I've revised based on your feedback. + +Here's what I changed: +[List of fixes from Step 6] + +Re-review with the same adversarial stance. Focus on: +1. Whether my fixes actually resolve the reported issues +2. Any NEW issues introduced by the fixes + +End with VERDICT: APPROVED or VERDICT: REVISE +``` + +Substitute the fixes list from Step 6 (one bullet per finding addressed). Do NOT include the session marker — the subagent adds it. + +**Step 7.2: Dispatch the runner subagent for resume.** + +Same Agent tool invocation as Step 4 (runner spec + YAML input block). Reuse the `RUNNER_SPEC_PATH` resolved in Step 4 (do not re-resolve). Input block: + +```yaml +--- +REVIEW_ID: +REPO_ROOT: +OPERATION: resume +CODEX_MODEL: +CODEX_REASONING: +PROMPT_BODY_PATH: /tmp/codex-resume-body-.md +RESULT_PATH: /tmp/codex-runner-result-.json +CODEX_SESSION_ID: +--- +``` + +**Step 7.3: Parse the two-channel result.** + +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 `⚠ ` message BEFORE any other action (including before the Step 5 verbatim review) — see Step 4's user_warning rule. + +| `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. | + +**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. + +**Step 7.4: Fallback chain** — triggered by `launch_failure` or repeated `timeout` from the runner. + +*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). + +*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. +Last round's maximum severity: . + +Options: +(a) Run a fresh `codex exec` with full previous-rounds context (higher token cost, new session) +(b) Conclude the review — show current findings as NOT VERIFIED +``` + +*Non-interactive mode:* +- Max severity `critical` or `high` → fresh exec automatically. +- Max severity `medium` only → Step 8 with the not-verified terminal state. + +*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). + +**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. + +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 3: Commit** + +```bash +cd /home/dementev/sources/adversarial-review +git add SKILL.md +git commit -m "$(cat <<'EOF' +refactor(skill): dispatch resume and fresh-exec via subagent + +Step 7 now delegates codex-exec-resume to the runner. Fallback +path (resume failure → fresh exec) also goes through the runner +with OPERATION=fresh-exec. Severity classification and user +interaction stay in main thread. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 5: Update `SKILL.md` Rules section and ancillary bits + +**Files:** +- Modify: `SKILL.md` — Rules section (roughly lines 687-711 in current file) + +- [ ] **Step 1: Replace the Rules bullet list** + +Find the `## Rules` heading near the end of `SKILL.md` and replace its bullet list with: + +```markdown +## 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. +- 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** 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: ` 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, the review file at `review_file`, and the skill's own `references/runner.md`. 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. +- Show the user reviews and fixes for each round. +- If Codex CLI is not installed or crashed — tell the user: `npm install -g @openai/codex`. +- If a fix contradicts the user's explicit requirements — skip and explain why. +``` + +- [ ] **Step 2: Update the frontmatter description** + +Verify the frontmatter at the top of `SKILL.md` still matches the skill's behavior. No change needed — behavior is unchanged; only orchestration is internal. + +- [ ] **Step 3: Update Step 9 cleanup glob to include new paths** + +Find the `rm -f ...` block in Step 9 of SKILL.md (around line 672-681 of the current file). Add the two new paths introduced by the refactor: `/tmp/codex-body-${REVIEW_ID}.md` and `/tmp/codex-runner-result-${REVIEW_ID}.json`. All other paths remain. The resulting block should be: + +```bash +rm -f /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 \ + /tmp/codex-body-${REVIEW_ID}.md \ + /tmp/codex-runner-result-${REVIEW_ID}.json +``` + +No other logic in Step 9 changes (conditional-on-terminal-state, Plan Mode skip). + +- [ ] **Step 4: Commit** + +```bash +cd /home/dementev/sources/adversarial-review +git add SKILL.md +git commit -m "$(cat <<'EOF' +docs(skill): rewrite Rules section + extend Step 9 cleanup glob + +Reflects the new main + runner split. Removes rules that now +live exclusively in references/runner.md (ATTEMPT_ID generation, +two-tier session capture, strict check order). Adds two new +/tmp paths to Step 9 cleanup: codex-body and codex-runner-result. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 6: Update `AGENTS.md` and `DESIGN.md` + +**Files:** +- Modify: `AGENTS.md` +- Modify: `DESIGN.md` + +- [ ] **Step 1: Add architecture section to AGENTS.md** + +Read current `AGENTS.md`. Add a new section `## Architecture` (after the existing orientation) with: + +```markdown +## 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. +``` + +- [ ] **Step 2: Append §12 to DESIGN.md (DO NOT renumber existing sections)** + +Read current `DESIGN.md`. AGENTS.md §10.2 explicitly warns against renumbering DESIGN.md sections — cross-refs would break silently. Therefore: **append** the new section as `§12. Subagent architecture`. Do NOT insert at §3 or anywhere that would shift existing numbering. + +```markdown +## 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: ` 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. + +### 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. +``` + +- [ ] **Step 3: Commit both files** + +```bash +cd /home/dementev/sources/adversarial-review +git add AGENTS.md DESIGN.md +git commit -m "$(cat <<'EOF' +docs: document subagent architecture in AGENTS.md and DESIGN.md + +AGENTS.md gets a new Architecture section with boundary +invariants. DESIGN.md §3 explains the residue problem, the +Agent-tool split, and why Haiku is sufficient for the runner. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 7: Verify the refactor works end-to-end (GREEN) + +**Files:** (no modifications to skill files — verification only) + +- [ ] **Step 1: Install the refactored skill to the standard location** + +Claude Code invokes `/adversarial-review` by loading the INSTALLED skill at `~/.claude/skills/adversarial-review/`, not the source checkout. Without this step, `/adversarial-review code` would run the OLD skill — false positive/negative on the GREEN gate. + +Symlink (not copy — keeps the install in sync with the branch): + +```bash +# Back up any existing install first +if [ -e ~/.claude/skills/adversarial-review ] && [ ! -L ~/.claude/skills/adversarial-review ]; then + mv ~/.claude/skills/adversarial-review ~/.claude/skills/adversarial-review.bak-$(date +%s) +fi +mkdir -p ~/.claude/skills +ln -sfn /home/dementev/sources/adversarial-review ~/.claude/skills/adversarial-review +``` + +Verify the install points at the branch AND that the resolved basename matches the skill name (some Claude Code versions derive the skill name from `realpath` basename; a mismatch causes silent skill-registry rejection): + +```bash +readlink ~/.claude/skills/adversarial-review +ls ~/.claude/skills/adversarial-review/references/runner.md +git -C ~/.claude/skills/adversarial-review/ branch --show-current +# Basename must match the skill name exactly: +test "$(basename "$(readlink -f ~/.claude/skills/adversarial-review)")" = "adversarial-review" && echo OK +``` + +Expected: symlink target matches the source repo, `references/runner.md` exists, current branch is `feature/subagent-orchestration`, basename assertion prints `OK`. + +If the basename assertion fails (source checkout is named differently), either (a) rename the source dir to `adversarial-review`, or (b) copy instead of symlink: `cp -r /home/dementev/sources/adversarial-review ~/.claude/skills/adversarial-review` (remember to re-copy after each source-branch commit during verification iteration). + +- [ ] **Step 2: Prepare a trivial change to review** + +```bash +cd /home/dementev/sources/adversarial-review +echo "" >> README.md +echo "" >> README.md +git diff README.md +``` + +Expected: one added trailing comment line. + +- [ ] **Step 3: Code-mode smoke test — invoke `/adversarial-review code`** + +From a FRESH Claude Code session (so the baseline context is minimal and measurable), run: + +``` +/adversarial-review code +``` + +Note the session's context-token count BEFORE invoking (from `/status` or similar) and AFTER. Expected delta: ≤15K tokens total growth (review markdown 2-5K + runner result JSON ~1K + orchestration overhead). NOT ~48M — that would mean the refactor did not take effect. + +Observe during the run: +- Step 4 dispatches an Agent tool call (visible in the session as a subagent launch). +- The subagent's final message is a single `RUNNER_RESULT_AT: /tmp/codex-runner-result-...json` line (visible as the Agent result). +- Main thread reads the result JSON and the review file, then shows the review verbatim. + +- [ ] **Step 4: Plan-mode smoke test — invoke `/adversarial-review plan`** + +With no code changes in the tree (revert the README hunk first: `git checkout README.md`), describe a trivial plan in Plan Mode OR pass an explicit plan file path. Verify that `/adversarial-review plan` completes end-to-end including subagent dispatch. + +Specifically watch for: +- Does dispatching the Agent tool from Plan Mode prompt the user for permission? If yes, document in DESIGN.md §12.7. +- Does the subagent's own Write tool for `/tmp/codex-prompt-*` succeed? If it prompts for permission, note in DESIGN.md §12.7. +- Does the subagent's codex exec launch succeed under Plan Mode? (Codex is read-only, so should be fine.) + +If Plan Mode interferes with dispatch or Write, document the expected behavior (auto-exit, permission prompt, or error) in DESIGN.md §12.7 based on observation. Do NOT leave §12.7 speculative — replace with observed reality. + +- [ ] **Step 5: Check main-thread isolation invariant** + +After each smoke test, inspect the session's tool-call transcript. The main thread's Bash calls should include only: +- `git rev-parse --show-toplevel` and `git rev-parse --show-superproject-working-tree` +- `git diff --name-only`, `git diff --cached --name-only`, `git diff --name-only ${BASE_BRANCH}...HEAD` (code mode only) +- `git symbolic-ref` for base branch (code mode only) +- The Step 9 `rm` cleanup command (on approved/concluded terminal states) + +It should NOT include: +- Any `cat ... | codex exec ...` invocation +- Any read of `/tmp/codex-stdout-*.jsonl`, `/tmp/codex-stderr-*.txt`, or rollout files +- Any `find ~/.codex/sessions ...` +- Any `mv /tmp/codex-stdout-*` or `mv /tmp/codex-stderr-*` (archival now lives in the runner) + +Allowed reads in main: the runner result JSON at `/tmp/codex-runner-result-*.json`, the review file at `/tmp/codex-review-*.md`, and the skill's `references/runner.md`. + +If any disallowed call appears in main, a step is still running in-process — return to that step and fix. + +- [ ] **Step 6: Verify Step 9 cleanup is complete** + +After a successful (APPROVED) review concludes, verify no `/tmp/codex-*-${REVIEW_ID}*` files remain: + +```bash +ls /tmp/codex-*-${REVIEW_ID}* 2>/dev/null +# Expected: empty +``` + +If files remain, the Step 9 cleanup glob is missing a pattern (likely `codex-body-*.md` or `codex-runner-result-*.json` — added in Task 5 Step 3). + +- [ ] **Step 7: Revert the trivial change** + +```bash +cd /home/dementev/sources/adversarial-review +git checkout README.md 2>/dev/null || true +``` + +- [ ] **Step 8: Final commit (only if verification surfaced fixes)** + +If Steps 3-6 passed without requiring changes to SKILL.md, runner.md, AGENTS.md, or DESIGN.md, no commit is needed. If any were made: + +```bash +cd /home/dementev/sources/adversarial-review +git add SKILL.md references/runner.md AGENTS.md DESIGN.md +git commit -m "$(cat <<'EOF' +fix(skill): findings from subagent refactor E2E verification + + + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Task 8: Open PR for review + +**Files:** (none — git/gh operations) + +- [ ] **Step 1: Push the branch** + +```bash +cd /home/dementev/sources/adversarial-review +git push -u origin feature/subagent-orchestration +``` + +- [ ] **Step 2: Create PR** + +```bash +gh pr create --title "refactor(skill): extract codex orchestration to Haiku subagent" --body "$(cat <<'EOF' +## Summary +- Split adversarial-review into main orchestrator + thin Haiku runner subagent +- Main keeps mode detection, review display, code fixes, round control +- Runner owns codex-exec mechanics (launch, strict checks, session-id capture, retry) +- Eliminates ~48M-token cache-read residue in main thread per invocation + +## Architecture +See DESIGN.md §3 and AGENTS.md Architecture section. + +## Test plan +- [x] E2E smoke test on trivial README change — main thread no longer reads codex stdout/stderr/rollout files +- [x] Review file path crosses subagent→main boundary correctly +- [x] Resume path (rounds 2-5) dispatches runner with OPERATION=resume +- [x] Fresh-exec fallback still consumes one round from 5-round counter + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +EOF +)" +``` + +- [ ] **Step 3: Return PR URL** + +Capture the URL printed by `gh pr create` and report it to the user. + +--- + +## Self-Review Checklist + +- **Spec coverage:** Tasks 1-2 create `references/runner.md` with the full runner spec. Tasks 3-4 refactor the two dispatch points (initial in Step 4, resume + fresh-exec fallback in Step 7) with SKILL_DIR resolution and two-channel result protocol. Task 5 updates Rules and extends Step 9 cleanup glob. Task 6 appends DESIGN.md §12 (no renumbering). Task 7 verifies end-to-end with skill-install substep, plan-mode test, and residue-tax measurement. Task 8 ships. +- **No placeholders:** every prompt body is inlined verbatim (no "copy from pre-refactor" forward references). Every command, every JSON schema is written out. +- **Type consistency:** runner's JSON output fields (`result`, `verdict`, `review_file`, `codex_session_id`, `attempt_id`, `errors`, `archived_stdout`, `archived_stderr`, `user_warning`) are used with the same names in Tasks 3 and 4's main-thread dispatch tables. +- **Retry budget invariant (all failure types):** runner retries ONCE internally on ANY failure — launch_failure, timeout, stderr-infra-error. Main treats EVERY failure as TERMINAL for the current round — no user-offered retries, no main re-dispatches. Total codex invocations per round ≤ 2 across all failure types. Fresh-exec fallback is a NEW round with its own 2-attempt budget. Closes Round-2 finding #1 (timeout lane no longer compounds). +- **Session-id handling on resume:** when secondary-path find returns zero, the runner returns `codex_session_id: null` for resume AND sets `user_warning` with the §2.4.4 diagnostic. Main surfaces `user_warning` on a single line before proceeding — preserves pre-refactor user-visible behavior. +- **Isolation invariant:** main reads the runner result JSON, the review file, and `references/runner.md`. Main never references `/tmp/codex-stdout-*`, `/tmp/codex-stderr-*`, rollout paths, or `find ~/.codex/sessions` in any Bash argv. Archival `mv` lives in the runner. +- **Naming disambiguation:** `CODEX_MODEL` / `CODEX_REASONING` (runner input) vs `model: "haiku"` (Agent tool dispatch). Never conflated. +- **DESIGN.md ordering:** new section is `§12. Subagent architecture` appended at the end. Existing §§1-11 are NOT renumbered (AGENTS.md §10.2 invariant). +- **Skill-install for verification:** Task 7 Step 1 symlinks the branch to `~/.claude/skills/adversarial-review/` so `/adversarial-review` invokes the refactored skill, not the previously-installed version. +- **Residue-tax measurement:** Task 7 Step 3 notes context-token before/after deltas and asserts ≤15K growth (not ~48M) — verifies the refactor actually fixed the bug, not merely relocated code. +- **Plan Mode:** Task 7 Step 4 runs an explicit `/adversarial-review plan` smoke test. DESIGN.md §12.7 is explicitly labeled as a HYPOTHESIS until Task 7 Step 4 replaces it with observed reality. +- **Placeholder substitution:** Task 3 Step 2 has an explicit table of placeholders (`${BASE_BRANCH}`, `${REPO_ROOT}`, ``, etc.) that main MUST resolve before the Write-to-disk step. Pre-refactor did this implicitly; post-refactor needs it explicit. +- **User-override capture:** Task 3 Step 2 tells main to capture `CODEX_MODEL` / `CODEX_REASONING` from user arguments (defaults `gpt-5.4`, `high`), preserving the `/adversarial-review xhigh` and `/adversarial-review model:...` override features. +- **SKILL_DIR resolution:** priority 1 is well-known install path; priority 2 is git toplevel (dev fallback); no attempt to read Claude's own system prompt. Closes Round-2 finding #2 (hand-wave). +- **RUNNER_RESULT_AT regex:** unanchored `RUNNER_RESULT_AT:\s+(\S+)` applied to entire subagent message, with filesystem Glob fallback on the deterministic path. Both spec sites (runner contract and main-thread parsing) use the same form. +- **`user_warning` timing:** emitted as its own `⚠ ` turn BEFORE the Step 5 verbatim-review message. Does not pollute the Step 5 header. +- **Symlink basename:** Task 7 Step 1 verifies `basename $(readlink -f ...)` equals `adversarial-review` to catch skill-loader basename-resolution rejection. diff --git a/references/runner.md b/references/runner.md new file mode 100644 index 0000000..ba2b12e --- /dev/null +++ b/references/runner.md @@ -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: +REPO_ROOT: +OPERATION: initial | resume | fresh-exec +CODEX_MODEL: # the model codex CLI launches; DO NOT confuse with your own (Haiku) model +CODEX_REASONING: +PROMPT_BODY_PATH: +CODEX_SESSION_ID: +RESULT_PATH: /tmp/codex-runner-result-.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-.md" | null, + "codex_session_id": "" | null, + "attempt_id": "<6-digit>", + "errors": "" | null, + "archived_stdout": "/tmp/codex-stdout--failed-resume.jsonl" | null, + "archived_stderr": "/tmp/codex-stderr--failed-resume.txt" | null, + "user_warning": "" | null +} +``` + +**Channel 2 — final message (short).** Your FINAL message to main should be a single line: + +``` +RUNNER_RESULT_AT: +``` + +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 `` 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 < /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": "", + "codex_session_id": null, + "attempt_id": "", + "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 -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--.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 `""`): + +```json +{ + "result": "success", + "verdict": "REVISE", + "review_file": "", + "codex_session_id": "", + "attempt_id": "", + "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": "", + "codex_session_id": null, + "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 `""` 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: ` — nothing before, nothing after, no markdown fence. Main's regex tolerates minor wrapping, but adhering to the spec eliminates edge cases entirely.