docs(claude-md): read Codex review findings from the job record, not stdout (#2457)

Claude-Session: https://claude.ai/code/session_014MXT7tDsb7Rsm1HhBnwhSj

Author: datlechin

.claude/skills/fix-issue/SKILL.md

@@ -190,21 +190,30 @@ UI tests have their own trap list, including an accessibility tree that differs
    node "$CODEX" adversarial-review --wait --scope working-tree \
      "<the mechanism the fix rests on, in one sentence>"       # attacks the approach, not the lines
    ```
+
+   **Then read the result out of the job record, because stdout is not the review.** This is the single easiest way to throw the whole step away. What a `--wait` run prints is the `[codex]` progress stream: `Reasoning summary captured: **Some Headline**`, one line per thought, and nothing else. The findings are never in it. A run that worked and a run that died look identical there, so judging by stdout means reporting "Codex returned no findings" over the top of a real verdict:
+   ```bash
+   node "$CODEX" status                    # job ids, state, and the one-line verdict per finished job
+   node "$CODEX" result <job-id>           # the findings, severities, file:line and recommendations
+   ```
+   Always run `result` and quote from that. This is not a fallback for when something looks wrong; it is how you read every review. Measured in the session that produced this rule: an `adversarial-review` printed 80 headline lines to stdout and nothing else, while `status` showed it `completed` with `No-ship: a supported Settings sequence can still create two Cmd+F menu claimants`, and `result` carried a medium-severity finding with a file:line, a reproduction and a recommended regression test. Acting on stdout alone shipped that defect to an open PR.
    `review` maps to Codex's built-in reviewer and hunts defects in the lines you wrote. `adversarial-review` challenges the approach itself: the design choice, the tradeoff, the assumption the fix depends on. It is the only one of the two that accepts focus text, and this workflow earns it, because a fix-issue change is a design decision as much as a diff. Give it the mechanism to attack in one sentence, the same claim the Phase 2 critique went after, so it can disagree with the blueprint now that the blueprint is real code. `review` rejects focus text outright rather than ignoring it.
 
    **Scope is worth stating rather than inferring.** With neither flag, `auto` picks the working tree when it is dirty and a diff against the detected default branch when it is clean. That is deterministic, but the two are not the same review, and after a partial commit the one you get is not the one you meant. Name it. Use `--cwd <path>` when you cannot run from the root.
 
-   **Background it.** `review` always runs in that process's foreground; the companion parses `--background` but does not act on it, so `run_in_background: true` on the `Bash` call is the only thing that actually detaches the run. Pass `--wait` and let the exit re-invoke you, so what lands is the finished review rather than a job handle. If a run does end up detached without one, `node "$CODEX" status` and `node "$CODEX" result <job-id>` reach it. Never run the two passes concurrently.
+   **Background it.** `review` always runs in that process's foreground; the companion parses `--background` but does not act on it, so `run_in_background: true` on the `Bash` call is the only thing that actually detaches the run. Pass `--wait` so the exit re-invokes you when the job is genuinely finished rather than leaving you to poll, then read it with `result` per above. A pass takes 5 to 15 minutes on a small diff, so foregrounding one blocks the session for no gain. Never run the two passes concurrently.
 
    **Act on the findings; do not stop and ask.** The plugin's own `/codex:review` is review-only and ends by asking the user which findings to fix. That contract belongs to the slash command, and this skill deliberately departs from it: you are calling the runtime directly, inside a workflow whose one gate is already behind you at Phase 2. Fix what it finds, or say why a finding does not apply. Treat its findings on lines you just wrote as seriously as its findings on old code; this pass has already caught a CHANGELOG heading deleted by a careless `Edit`. What survives the departure is the reporting half: every finding you dismissed, and why, goes in the final report.
 
    **The slash commands are not available to you.** `/codex:review` and `/codex:adversarial-review` are declared `disable-model-invocation: true`, so they only run when the user types them. Call the companion script.
 
-   **When Codex cannot review, say so and fall back to `Skill(code-review)`.** Three failures to expect, and none of them is a reason to skip the review:
+   **When Codex cannot review, say so and fall back to `Skill(code-review)`.** Diagnose from `status` and `result`, never from stdout, and never from the exit code: a `--wait` run exits 0 whether it produced a verdict or died. Three failures to expect, and none of them is a reason to skip the review:
    - `Codex CLI is not installed or is missing required runtime support`, meaning the plugin is there and the binary is not.
-   - `Your workspace is out of credits.` in the progress lines. `review` then ends `Reviewer failed to output a response.`; `adversarial-review` ends `Codex did not return valid structured JSON.` with the credit message as its parse error. Authentication is fine and `/codex:setup` still reports `ready: true`, because it checks login and not billing. This is the failure that looks most like a clean review with no findings, so read the body rather than the exit code.
+   - `Your workspace is out of credits.` in the progress lines. `review` then ends `Reviewer failed to output a response.`; `adversarial-review` ends `Codex did not return valid structured JSON.` with the credit message as its parse error. Authentication is fine and `/codex:setup` still reports `ready: true`, because it checks login and not billing. `status` is what separates this from a healthy run, because both print nothing but reasoning headlines to stdout.
    - A setup or authentication error, which is the one case to hand back to the user with `/codex:setup` rather than working around.
 
+   Only call it a failure when `status` says so. "Only headline lines came back" is what a healthy run looks like, so it is never on its own evidence of anything, and declaring a fallback on that basis tells the user no second model read the change when one did.
+
    Name in the final report which reviewer actually read the diff. A fallback that goes unmentioned lets the user believe a second model saw the change when none did.
    - **`Skill(security-review)`** whenever the change touches a security boundary: credentials, keychain, SQL construction, query execution, plugin loading, MCP, AI tool permissions, sync, or anything that widens what a user or a plugin can do. It reviews the pending changes on the branch, so run it after the diff is complete and before the commit.
    - **`Skill(simplify)`** when the change grew past a couple of files. It is a quality pass for reuse and duplication rather than a bug hunt, which is the gap a correctness review leaves.