Add model and effort flags to review commands

[axisrow]

Propagate --model and --effort to /codex:review and /codex:adversarial-review, matching the existing task behavior.

Validation:

• node --test tests/commands.test.mjs
• node --test tests/runtime.test.mjs

[axisrow]

🔍 Local review (cycle 1/3) — clean

Reviewed locally (Claude subagent + Codex companion), no bots pinged.

Verdict	Reviewer	Finding	Location
HALLUCINATION	codex	"review/start fields break TS type-checking — npm run build fails"	plugins/codex/scripts/lib/codex.mjs:1030

Claude subagent: no findings.

Codex (1 critical, confidence 0.99) → HALLUCINATION. Codex claimed adding model/effort to the review/start RPC params triggers TypeScript excess-property errors during npm run build because ReviewStartParams only allows threadId/target/delivery. Verified against ground truth:

• npm run build → BUILD_EXIT=0, no errors.
• The schema read was correct (ReviewStartParams lacks model/effort), but the conclusion is wrong: the review/start call is wrapped in captureTurn(client, threadId, () => client.request('review/start', {...}), …), whose startRequest parameter is untyped (no JSDoc), so TypeScript never contextually types the arrow body and excess-property checking does not fire.
• Proven empirically: replacing model: with __bad_field: at line 1030 produced 0 tsc errors; the same change at the thread/start call (not behind captureTurn) correctly reports TS2353.
• The existing task/turn-start path ships the same model/effort fields (TurnStartParams includes them) — consistent.
• All review tests pass, incl. review forwards model selection and reasoning effort to app-server review/start.

No FIX verdicts → PR is clean. Cycle complete.

Note: this is local review mode — no auto-merge. Merge is yours to trigger when ready.

[axisrow]

🔍 Re-triage (cycle 1, corrected) — Codex finding is a REAL functional gap

My initial HALLUCINATION verdict on Codex's finding was too hasty. Re-verifying with actual tests + the real app-server (not just code-reading) confirms Codex was right about the protocol, wrong only about "the build fails." The real defect is more subtle than either reviewer first said.

Ground truth

1. Protocol type — confirmed via generated types + tsc
review/start is typed ReviewStartParams = { threadId, target, delivery? } — no model/effort (plugins/codex/.generated/app-server-types/v2/ReviewStartParams.ts). model/effort live only on TurnStartParams. A type probe assigning the PR's literal directly to the typed params:

probe.ts(14,3): error TS2353: 'model' does not exist in type 'ReviewStartParams'.

2. npm run build does NOT fail — BUILD_EXIT=0. The review/start literal is behind an untyped captureTurn(client, id, () => client.request("review/start", {...}), …) (startRequest has no JSDoc type, codex.mjs:559), so tsc never contextually checks it. Codex was wrong here.

3. Real app-server behavior — the actual defect (codex-cli 0.144.1, direct JSON-RPC probe)

• review/start WITHOUT model/effort → ACCEPTED (1 ms)
• review/start WITH model + effort → ACCEPTED (1 ms, no validation error)

The Rust app-server uses serde without deny_unknown_fields, so it silently drops unknown fields. → model/effort on review/start have no effect for native review.

Corrected verdict: FIX (functional gap, not a build break)

The PR advertises --model/--effort for /codex:review as "matching the existing task behavior." But:

• model is already applied via thread/start (buildThreadParams → model), so the review/start copy is redundant.
• effort has no working path for native review: TurnStartParams has effort (used by task), but native review sends review/start (no effort field), and the server discards it. So --effort on /codex:review is effectively a no-op flag.

Suggested fix: drop model/effort from the review/start params (they're dropped by the server anyway); keep --model (already honored via thread/start); and either remove --effort from /codex:review or document that it only affects the adversarial path (runAppServerTurn), not native review.

Local review mode — no auto-merge. This re-triage supersedes the earlier "clean / HALLUCINATION" comment.

[axisrow]

Closing this PR — it's the wrong fix and I don't want to ship inert/misleading changes.

What this PR did: added model/effort to the review/start RPC params and wired --effort parsing into the review commands.

Why it's wrong (verified against the real app-server, codex-cli 0.144.1):

• review/start accepts model/effort without error, but the Rust server silently drops unknown fields (serde, no deny_unknown_fields). The fields have no effect.
• ReviewStartParams = { threadId, target, delivery? } has no model/effort field at all — a direct type-probe gives TS2353.
• The model field is inert across all RPCs (thread/start/turn/start/review/start); the only effective model override is -c model="…" at app-server spawn time — see upstream fix(app-server): pass -c model="..." to codex app-server so options.model takes effect openai/codex-plugin-cc#408.

Full analysis and server-probe evidence posted on upstream openai#476 and openai#408. Upstream PR openai#477 (this same change) also closed.

For a correct fix: --effort on adversarial-review is the only part that's salvageable (via turn/start, per openai#476); native review has no effort field in the protocol, and model needs the openai#408 spawn-override regardless of command.