diff --git a/plugins/codex/commands/review.md b/plugins/codex/commands/review.md index fb70a4876..bbd462240 100644 --- a/plugins/codex/commands/review.md +++ b/plugins/codex/commands/review.md @@ -36,7 +36,7 @@ Argument handling: - Do not strip `--wait` or `--background` yourself. - Do not add extra review instructions or rewrite the user's intent. - The companion script parses `--wait` and `--background`, but Claude Code's `Bash(..., run_in_background: true)` is what actually detaches the run. -- `/codex:review` is native-review only. It does not support staged-only review, unstaged-only review, or extra focus text. +- `/codex:review` is native-review only. It does not support staged-only review or unstaged-only review. Positional focus text is ignored by the native reviewer (for interface parity with `/codex:adversarial-review`); it does not abort the review. - If the user needs custom review instructions or more adversarial framing, they should use `/codex:adversarial-review`. Foreground flow: diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..e2418aa4b 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -268,13 +268,13 @@ function buildNativeReviewTarget(target) { return null; } -function validateNativeReviewRequest(target, focusText) { - if (focusText.trim()) { - throw new Error( - `\`/codex:review\` now maps directly to the built-in reviewer and does not support custom focus text. Retry with \`/codex:adversarial-review ${focusText.trim()}\` for focused review instructions.` - ); - } - +function validateNativeReviewRequest(target) { + // Parity with /codex:adversarial-review: positional focus text no longer + // aborts the native review. The native reviewer (review/start) does not + // consume focus text, so leftover positional words are silently ignored + // rather than rejecting the invocation. This keeps `/codex:review --model sol` + // usable when a host forwards residual positional text alongside flags, + // and removes an interface-parity gap with /codex:adversarial-review (#522). const nativeTarget = buildNativeReviewTarget(target); if (!nativeTarget) { throw new Error("This `/codex:review` target is not supported by the built-in reviewer. Retry with `/codex:adversarial-review` for custom targeting."); @@ -363,10 +363,9 @@ async function executeReviewRun(request) { base: request.base, scope: request.scope }); - const focusText = request.focusText?.trim() ?? ""; const reviewName = request.reviewName ?? "Review"; if (reviewName === "Review") { - const reviewTarget = validateNativeReviewRequest(target, focusText); + const reviewTarget = validateNativeReviewRequest(target); const result = await runAppServerReview(request.cwd, { target: reviewTarget, model: request.model, @@ -407,6 +406,7 @@ async function executeReviewRun(request) { } const context = collectReviewContext(request.cwd, target); + const focusText = request.focusText?.trim() ?? ""; const prompt = buildAdversarialReviewPrompt(context, focusText); const result = await runAppServerTurn(context.repoRoot, { prompt, @@ -726,7 +726,7 @@ async function handleReviewCommand(argv, config) { scope: options.scope }); - config.validateRequest?.(target, focusText); + config.validateRequest?.(target); const metadata = buildReviewJobMetadata(config.reviewName, target); const job = createCompanionJob({ prefix: "review", diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index c34b06059..598e3bd47 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -36,7 +36,8 @@ test("review command uses AskUserQuestion and background Bash while staying revi assert.match(source, /Claude Code's `Bash\(..., run_in_background: true\)` is what actually detaches the run/i); assert.match(source, /When in doubt, run the review/i); assert.match(source, /\(Recommended\)/); - assert.match(source, /does not support staged-only review, unstaged-only review, or extra focus text/i); + assert.match(source, /does not support staged-only review or unstaged-only review/i); + assert.match(source, /Positional focus text is ignored by the native reviewer/i); }); test("adversarial review command uses AskUserQuestion and background Bash while staying review-only", () => { diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..27dbc0ee7 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -969,7 +969,7 @@ test("task --background enqueues a detached worker and exposes per-job status", assert.match(resultPayload.storedJob.rendered, /Handled the requested task/); }); -test("review rejects focus text because it is native-review only", () => { +test("review accepts (ignores) positional focus text for parity with adversarial-review", () => { const repo = makeTempDir(); const binDir = makeTempDir(); installFakeCodex(binDir); @@ -984,9 +984,13 @@ test("review rejects focus text because it is native-review only", () => { env: buildEnv(binDir) }); - assert.equal(result.status > 0, true); - assert.match(result.stderr, /does not support custom focus text/i); - assert.match(result.stderr, /\/codex:adversarial-review focus on auth/i); + // Parity with /codex:adversarial-review: positional args no longer abort the + // native review. The native reviewer (review/start) does not consume focus + // text, so leftover words are ignored rather than rejecting the invocation. + // This keeps `/codex:review --model sol` usable when a host (e.g. Claude) + // forwards residual positional text alongside flags. + assert.equal(result.status, 0); + assert.doesNotMatch(result.stderr, /does not support custom focus text/i); }); test("review rejects staged-only scope because it is native-review only", () => {