From 6622ade45fd966b59e9f51c8b33e82a0ad541b34 Mon Sep 17 00:00:00 2001 From: Christopher Steigerwald Date: Mon, 24 Aug 2026 14:41:43 +0000 Subject: [PATCH 1/9] feat(companion): handle --help per subcommand so it cannot start a run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--help` is only recognised as the first argument. For anything else it lands in argv, and because every subcommand parser treats an unrecognised token as a positional, it is carried into the command as data. For `adversarial-review` that means `--help` is joined into the review's focus text and a full review runs: minutes of wall clock and a real model turn, for someone who asked what the flags were. The same shape applies to any subcommand whose parser accepts positionals. main() now checks for --help, -h or help in argv before the dispatch switch, and prints usage for that subcommand alone. Checking before the switch is the point: a help request can never reach a handler that would dispatch. Bare `--help`, `-h` and `help` as the subcommand still print the full usage block, unchanged. The usage lines move into a Map keyed by subcommand so a single line can be printed without duplicating the text. The full block prints in the same order as before. Tests: `adversarial-review --help` prints usage and — the assertion that matters — starts no Codex turn, checked against the fake app server's recorded lastTurnStart; and `task -h` prints only the task line. Both verified non-vacuous against db52e28, where they fail. --- plugins/codex/scripts/codex-companion.mjs | 44 +++++++++++++++-------- tests/runtime.test.mjs | 38 ++++++++++++++++++++ 2 files changed, 67 insertions(+), 15 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..3d6f0209f 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -72,20 +72,28 @@ const VALID_REASONING_EFFORTS = new Set(["none", "minimal", "low", "medium", "hi const MODEL_ALIASES = new Map([["spark", "gpt-5.3-codex-spark"]]); const STOP_REVIEW_TASK_MARKER = "Run a stop-gate review of the previous Claude turn."; -function printUsage() { - console.log( - [ - "Usage:", - " node scripts/codex-companion.mjs setup [--enable-review-gate|--disable-review-gate] [--json]", - " node scripts/codex-companion.mjs review [--wait|--background] [--base ] [--scope ]", - " node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [focus text]", - " node scripts/codex-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]", - " node scripts/codex-companion.mjs transfer [--source ] [--json]", - " node scripts/codex-companion.mjs status [job-id] [--all] [--json]", - " node scripts/codex-companion.mjs result [job-id] [--json]", - " node scripts/codex-companion.mjs cancel [job-id] [--json]" - ].join("\n") - ); +const USAGE_LINES = new Map([ + ["setup", " node scripts/codex-companion.mjs setup [--enable-review-gate|--disable-review-gate] [--json]"], + ["review", " node scripts/codex-companion.mjs review [--wait|--background] [--base ] [--scope ]"], + ["adversarial-review", " node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [focus text]"], + ["task", " node scripts/codex-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]"], + ["transfer", " node scripts/codex-companion.mjs transfer [--source ] [--json]"], + ["status", " node scripts/codex-companion.mjs status [job-id] [--all] [--json]"], + ["result", " node scripts/codex-companion.mjs result [job-id] [--json]"], + ["cancel", " node scripts/codex-companion.mjs cancel [job-id] [--json]"] +]); + +// A help request must never become a dispatch. Every subcommand parser treats an +// unrecognised token as a positional, so `adversarial-review --help` was joined into the +// review's focus text and ran a full review -- minutes of wall clock and a model turn, +// for someone who asked what the flags were. +function isHelpRequest(argv) { + return argv.some((token) => token === "--help" || token === "-h" || token === "help"); +} + +function printUsage(subcommand) { + const line = subcommand ? USAGE_LINES.get(subcommand) : null; + console.log(["Usage:", ...(line ? [line] : USAGE_LINES.values())].join("\n")); } function outputResult(value, asJson) { @@ -1023,11 +1031,17 @@ async function handleCancel(argv) { async function main() { const [subcommand, ...argv] = process.argv.slice(2); - if (!subcommand || subcommand === "help" || subcommand === "--help") { + if (!subcommand || subcommand === "help" || subcommand === "--help" || subcommand === "-h") { printUsage(); return; } + // Checked before the switch, so help can never reach a handler that would dispatch. + if (USAGE_LINES.has(subcommand) && isHelpRequest(argv)) { + printUsage(subcommand); + return; + } + switch (subcommand) { case "setup": await handleSetup(argv); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..f869cdd4f 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -969,6 +969,44 @@ test("task --background enqueues a detached worker and exposes per-job status", assert.match(resultPayload.storedJob.rendered, /Handled the requested task/); }); +test("adversarial-review --help prints usage without dispatching a review", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "README.md"), "hello again\n"); + + const result = run("node", [SCRIPT, "adversarial-review", "--help"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /^Usage:/); + assert.match(result.stdout, /adversarial-review \[--wait\|--background\]/); + + // The point of the change: an unrecognised flag used to become focus text, so asking + // for help started a real review. Nothing may reach the model. + const state = fs.existsSync(statePath) ? JSON.parse(fs.readFileSync(statePath, "utf8")) : {}; + assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); +}); + +test("subcommand help accepts -h and prints only that subcommand", () => { + const binDir = makeTempDir(); + installFakeCodex(binDir); + + const result = run("node", [SCRIPT, "task", "-h"], { cwd: makeTempDir(), env: buildEnv(binDir) }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /codex-companion\.mjs task \[--background\]/); + // Scoped, not the whole usage block. + assert.equal(/adversarial-review/.test(result.stdout), false, "task -h printed other subcommands"); +}); + test("review rejects focus text because it is native-review only", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From 5770f78873b74ead624d45711e189263466ef1e6 Mon Sep 17 00:00:00 2001 From: Christopher Steigerwald Date: Mon, 24 Aug 2026 18:26:35 +0000 Subject: [PATCH 2/9] fix(companion): normalize argv before detecting a help request Addresses the P1 review finding on #681. The plugin commands invoke the companion with "$ARGUMENTS" as a single quoted argument (plugins/codex/commands/adversarial-review.md:50), so `/codex:adversarial-review --base main --help` reaches main() as argv === ["--base main --help"]. Comparing raw tokens misses the flag, the request falls through to the handler, and parseCommandInput then splits the string itself and starts a full review with --help as focus text -- preserving the exact behaviour this change exists to prevent whenever help is combined with any other argument. The check now runs normalizeArgv(argv) first, which is the same normalization parseCommandInput applies, so detection sees the same tokens the handler would. Normalizing has a consequence that must be handled at the same time: the bare word "help" now appears as a token in ordinary focus text. Matching it would turn `adversarial-review "review the help system"` into a usage dump instead of the review the user asked for. isHelpRequest therefore matches only --help and -h. A bare `help` subcommand is still handled separately before dispatch, so `codex-companion.mjs help` is unaffected. Tests: - help combined with another flag in one quoted string prints usage and starts no Codex turn. Verified non-vacuous against 6622ade, where it fails. - focus text containing the word "help" still runs the review and reaches turn/start with the focus intact. This one passes both with and without the fix: it is a guard against the regression normalizing would otherwise introduce, not evidence of the original bug. --- plugins/codex/scripts/codex-companion.mjs | 12 +++++- tests/runtime.test.mjs | 52 +++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 3d6f0209f..f05e70e4a 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -87,8 +87,12 @@ const USAGE_LINES = new Map([ // unrecognised token as a positional, so `adversarial-review --help` was joined into the // review's focus text and ran a full review -- minutes of wall clock and a model turn, // for someone who asked what the flags were. +// Only the flags, never the bare word "help": argv is normalized before this runs, so a +// focus string like "review the help system" tokenizes to include "help", and matching it +// would print usage instead of running the review the user asked for. A bare `help` +// subcommand is still handled separately, before dispatch. function isHelpRequest(argv) { - return argv.some((token) => token === "--help" || token === "-h" || token === "help"); + return argv.some((token) => token === "--help" || token === "-h"); } function printUsage(subcommand) { @@ -1036,8 +1040,12 @@ async function main() { return; } + // Normalize first. The plugin commands pass "$ARGUMENTS" as ONE quoted argument, so + // `/codex:adversarial-review --base main --help` arrives here as a single string and a + // raw token comparison misses the flag -- the handler would then split it itself and + // start a full review with --help as focus text, which is exactly what this prevents. // Checked before the switch, so help can never reach a handler that would dispatch. - if (USAGE_LINES.has(subcommand) && isHelpRequest(argv)) { + if (USAGE_LINES.has(subcommand) && isHelpRequest(normalizeArgv(argv))) { printUsage(subcommand); return; } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index f869cdd4f..a96406952 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -995,6 +995,58 @@ test("adversarial-review --help prints usage without dispatching a review", () = assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); }); +test("help is detected when the plugin passes all arguments as one string", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "README.md"), "hello again\n"); + + // The plugin commands invoke the companion with "$ARGUMENTS" as a SINGLE quoted + // argument, so this is the shape real usage takes. Comparing raw tokens misses the + // flag here, and the handler then splits the string itself and reviews with --help as + // focus text. + const result = run("node", [SCRIPT, "adversarial-review", "--base main --help"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /^Usage:/); + const state = fs.existsSync(statePath) ? JSON.parse(fs.readFileSync(statePath, "utf8")) : {}; + assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); +}); + +test("focus text mentioning help still runs the review", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.mkdirSync(path.join(repo, "src")); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = items[0];\n"); + run("git", ["add", "src/app.js"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = items[0].id;\n"); + + // Because argv is normalized before the check, a bare "help" token appears in ordinary + // focus text. Matching it would silently swap the user's review for a usage dump. + const result = run("node", [SCRIPT, "adversarial-review", "review the help system"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.equal(/^Usage:/.test(result.stdout), false, "focus text containing 'help' printed usage"); + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.ok(state.lastTurnStart, "the review never started"); + assert.match(state.lastTurnStart.prompt, /help system/); +}); + test("subcommand help accepts -h and prints only that subcommand", () => { const binDir = makeTempDir(); installFakeCodex(binDir); From 50648ea762ea45b5db839196089fd2461432ad3f Mon Sep 17 00:00:00 2001 From: Christopher Steigerwald Date: Mon, 24 Aug 2026 18:29:36 +0000 Subject: [PATCH 3/9] fix(companion): parse for help instead of scanning tokens Self-review follow-up to 5770f78, which introduced this while fixing the P1. Normalizing argv before help detection is necessary, but it also splits focus text into tokens. A token scan therefore matched user prose: `adversarial-review "why does --help start a review"` produced a --help token and printed usage, so that review could never be run at all. That is a hard block on legitimate input, not merely a surprise, and it is a defect the previous commit introduced. Help detection now parses rather than scans, and treats the request as help only when the flag is present AND nothing else was asked for -- no focus text left over: return options.help === true && positionals.length === 0; That keeps every case right: --help -> usage --base main --help -> usage (main is consumed as a value) --model spark -h -> usage "why does --help start a review" -> review, focus intact "review the help system" -> review, focus intact Test: focus text containing --help reaches turn/start with the focus preserved. Verified non-vacuous against 5770f78, where it fails. Full suite: 96 passing / 0 failing of 96. --- plugins/codex/scripts/codex-companion.mjs | 31 ++++++++++++++++++----- tests/runtime.test.mjs | 26 +++++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index f05e70e4a..00d3b1579 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -87,12 +87,31 @@ const USAGE_LINES = new Map([ // unrecognised token as a positional, so `adversarial-review --help` was joined into the // review's focus text and ran a full review -- minutes of wall clock and a model turn, // for someone who asked what the flags were. -// Only the flags, never the bare word "help": argv is normalized before this runs, so a -// focus string like "review the help system" tokenizes to include "help", and matching it -// would print usage instead of running the review the user asked for. A bare `help` -// subcommand is still handled separately, before dispatch. +// Detecting help by scanning tokens is not safe once argv is normalized. Focus text is +// split into tokens too, so `adversarial-review "why does --help start a review"` yields a +// --help token and a token scan would make that review impossible to run -- a hard block, +// not just a surprise. Parse instead, and treat it as help only when nothing else was +// asked for: the help flag present AND no focus text left over. The bare word "help" is +// never matched here; a `help` subcommand is handled separately before dispatch. +const HELP_DETECTION_VALUE_OPTIONS = [ + "base", + "scope", + "model", + "cwd", + "effort", + "prompt-file", + "source", + "timeout-ms", + "poll-interval-ms" +]; + function isHelpRequest(argv) { - return argv.some((token) => token === "--help" || token === "-h"); + const { options, positionals } = parseArgs(normalizeArgv(argv), { + valueOptions: HELP_DETECTION_VALUE_OPTIONS, + booleanOptions: ["help"], + aliasMap: { h: "help" } + }); + return options.help === true && positionals.length === 0; } function printUsage(subcommand) { @@ -1045,7 +1064,7 @@ async function main() { // raw token comparison misses the flag -- the handler would then split it itself and // start a full review with --help as focus text, which is exactly what this prevents. // Checked before the switch, so help can never reach a handler that would dispatch. - if (USAGE_LINES.has(subcommand) && isHelpRequest(normalizeArgv(argv))) { + if (USAGE_LINES.has(subcommand) && isHelpRequest(argv)) { printUsage(subcommand); return; } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index a96406952..b090996b2 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1021,6 +1021,32 @@ test("help is detected when the plugin passes all arguments as one string", () = assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); }); +test("focus text containing --help is reviewed, not swallowed as a help request", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.mkdirSync(path.join(repo, "src")); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = items[0];\n"); + run("git", ["add", "src/app.js"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = items[0].id;\n"); + + // Normalization splits focus text into tokens, so a token scan would see --help here and + // make this review impossible to run. Help means the flag AND nothing else asked for. + const result = run("node", [SCRIPT, "adversarial-review", "why does --help start a review"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.equal(/^Usage:/.test(result.stdout), false, "focus text containing --help printed usage"); + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.ok(state.lastTurnStart, "the review never started"); + assert.match(state.lastTurnStart.prompt, /start a review/); +}); + test("focus text mentioning help still runs the review", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From addad6f0717adb24c39e21a77a75265060c5dc3a Mon Sep 17 00:00:00 2001 From: Christopher Steigerwald Date: Mon, 24 Aug 2026 19:03:17 +0000 Subject: [PATCH 4/9] fix(companion): give help detection the subcommand's real option schema Addresses the P1 review finding on 50648ea. Help detection parsed with its own hand-maintained option list, so it did not know options the handlers accept. `adversarial-review "--wait --help"` therefore put --wait into positionals, the "no focus text left over" rule saw a positional and returned false, and the real parser then consumed --wait and started a full review with --help as focus text -- the behaviour this PR exists to prevent. The list was the defect, not its contents: any option added to a handler later would reintroduce the same gap silently. There is now one schema per subcommand in COMMAND_OPTION_SCHEMAS, read by both the handler and help detection, so the two cannot disagree. The handlers' inline literals are replaced by lookups into it, which is the only way the guarantee holds over time. Parsing with the real schema also fixes the `--` case for free: anything after the delimiter is a positional, so a literal `--help` in focus text stays focus text rather than being read as a flag. Behaviour: --help -> usage --wait --help -> usage --background --json --help -> usage --base main --help -> usage status --all --help -> usage "why does --help start a review" -> review, focus intact "review --help handling" -> review, focus intact "-- --help" -> review, focus intact Tests: help following another recognized flag prints usage and starts no turn (non-vacuous against 50648ea, where it fails); and a quoted focus argument keeps its help-looking words, which passes both ways and is a guard rather than evidence, since 50648ea already handled that case. Full suite: 98 passing / 0 failing of 98. --- plugins/codex/scripts/codex-companion.mjs | 84 ++++++++++++----------- tests/runtime.test.mjs | 49 +++++++++++++ 2 files changed, 94 insertions(+), 39 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 00d3b1579..44c9ea247 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -87,29 +87,46 @@ const USAGE_LINES = new Map([ // unrecognised token as a positional, so `adversarial-review --help` was joined into the // review's focus text and ran a full review -- minutes of wall clock and a model turn, // for someone who asked what the flags were. -// Detecting help by scanning tokens is not safe once argv is normalized. Focus text is -// split into tokens too, so `adversarial-review "why does --help start a review"` yields a -// --help token and a token scan would make that review impossible to run -- a hard block, -// not just a surprise. Parse instead, and treat it as help only when nothing else was -// asked for: the help flag present AND no focus text left over. The bare word "help" is -// never matched here; a `help` subcommand is handled separately before dispatch. -const HELP_DETECTION_VALUE_OPTIONS = [ - "base", - "scope", - "model", - "cwd", - "effort", - "prompt-file", - "source", - "timeout-ms", - "poll-interval-ms" -]; - -function isHelpRequest(argv) { +const REVIEW_OPTION_SCHEMA = { valueOptions: ["base", "scope", "model", "cwd"], booleanOptions: ["json", "background", "wait"], aliasMap: { m: "model" } }; + +// One schema per subcommand, read by BOTH the handler and help detection. Keeping a +// separate hand-maintained list for help detection is what broke it: an option the help +// parser did not know (`--wait`) became a positional, help was not detected, and the real +// parser then consumed the option and reviewed `--help` as focus text. Any option added +// here is automatically known to both, so the two can never disagree again. +const COMMAND_OPTION_SCHEMAS = new Map([ + ["setup", { valueOptions: ["cwd"], booleanOptions: ["json", "enable-review-gate", "disable-review-gate"] }], + ["review", REVIEW_OPTION_SCHEMA], + ["adversarial-review", REVIEW_OPTION_SCHEMA], + [ + "task", + { + valueOptions: ["model", "effort", "cwd", "prompt-file"], + booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background"], + aliasMap: { m: "model" } + } + ], + ["transfer", { valueOptions: ["cwd", "source"], booleanOptions: ["json"] }], + ["status", { valueOptions: ["cwd", "timeout-ms", "poll-interval-ms"], booleanOptions: ["json", "all", "wait"] }], + ["result", { valueOptions: ["cwd"], booleanOptions: ["json"] }], + ["cancel", { valueOptions: ["cwd"], booleanOptions: ["json"] }] +]); + +// Parse with the subcommand's real schema, then treat it as help only when the flag is +// present AND nothing else was asked for. Scanning tokens cannot work here: argv is +// normalized first, so focus text is split into tokens too, and a scan would match +// `adversarial-review "why does --help start a review"` and make that review impossible +// to run. Parsing also gets `--` right for free -- anything after it is a positional, so +// a literal `--help` in focus text stays focus text. +function isHelpRequest(subcommand, argv) { + const schema = COMMAND_OPTION_SCHEMAS.get(subcommand); + if (!schema) { + return false; + } const { options, positionals } = parseArgs(normalizeArgv(argv), { - valueOptions: HELP_DETECTION_VALUE_OPTIONS, - booleanOptions: ["help"], - aliasMap: { h: "help" } + valueOptions: schema.valueOptions ?? [], + booleanOptions: [...(schema.booleanOptions ?? []), "help"], + aliasMap: { ...(schema.aliasMap ?? {}), h: "help" } }); return options.help === true && positionals.length === 0; } @@ -245,8 +262,7 @@ async function buildSetupReport(cwd, actionsTaken = []) { async function handleSetup(argv) { const { options } = parseCommandInput(argv, { - valueOptions: ["cwd"], - booleanOptions: ["json", "enable-review-gate", "disable-review-gate"] + ...COMMAND_OPTION_SCHEMAS.get("setup") }); if (options["enable-review-gate"] && options["disable-review-gate"]) { @@ -742,11 +758,7 @@ function enqueueBackgroundTask(cwd, job, request) { async function handleReviewCommand(argv, config) { const { options, positionals } = parseCommandInput(argv, { - valueOptions: ["base", "scope", "model", "cwd"], - booleanOptions: ["json", "background", "wait"], - aliasMap: { - m: "model" - } + ...REVIEW_OPTION_SCHEMA }); const cwd = resolveCommandCwd(options); @@ -792,11 +804,7 @@ async function handleReview(argv) { async function handleTask(argv) { const { options, positionals } = parseCommandInput(argv, { - valueOptions: ["model", "effort", "cwd", "prompt-file"], - booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background"], - aliasMap: { - m: "model" - } + ...COMMAND_OPTION_SCHEMAS.get("task") }); const cwd = resolveCommandCwd(options); @@ -855,8 +863,7 @@ async function handleTask(argv) { async function handleTransfer(argv) { const { options } = parseCommandInput(argv, { - valueOptions: ["cwd", "source"], - booleanOptions: ["json"] + ...COMMAND_OPTION_SCHEMAS.get("transfer") }); const cwd = resolveCommandCwd(options); @@ -913,8 +920,7 @@ async function handleTaskWorker(argv) { async function handleStatus(argv) { const { options, positionals } = parseCommandInput(argv, { - valueOptions: ["cwd", "timeout-ms", "poll-interval-ms"], - booleanOptions: ["json", "all", "wait"] + ...COMMAND_OPTION_SCHEMAS.get("status") }); const cwd = resolveCommandCwd(options); @@ -1064,7 +1070,7 @@ async function main() { // raw token comparison misses the flag -- the handler would then split it itself and // start a full review with --help as focus text, which is exactly what this prevents. // Checked before the switch, so help can never reach a handler that would dispatch. - if (USAGE_LINES.has(subcommand) && isHelpRequest(argv)) { + if (USAGE_LINES.has(subcommand) && isHelpRequest(subcommand, argv)) { printUsage(subcommand); return; } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index b090996b2..199aaf812 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1021,6 +1021,55 @@ test("help is detected when the plugin passes all arguments as one string", () = assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); }); +test("help is detected when it follows another recognized flag", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "README.md"), "hello again\n"); + + // Help detection must parse with the subcommand's real schema. A separate list would + // not know --wait, which would land in positionals, defeat the no-focus-text rule, and + // let the real parser consume --wait and review "--help" as focus text. + const result = run("node", [SCRIPT, "adversarial-review", "--wait --help"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /^Usage:/); + const state = fs.existsSync(statePath) ? JSON.parse(fs.readFileSync(statePath, "utf8")) : {}; + assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); +}); + +test("a quoted focus argument keeps its help-looking words as focus text", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.mkdirSync(path.join(repo, "src")); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = items[0];\n"); + run("git", ["add", "src/app.js"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = items[0].id;\n"); + + const result = run("node", [SCRIPT, "adversarial-review", "review --help handling"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.equal(/^Usage:/.test(result.stdout), false, "a focused review printed usage instead"); + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.ok(state.lastTurnStart, "the review never started"); + assert.match(state.lastTurnStart.prompt, /handling/); +}); + test("focus text containing --help is reviewed, not swallowed as a help request", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From fda50dce8925352d104a68b7fffbd965075d2264 Mon Sep 17 00:00:00 2001 From: Christopher Steigerwald Date: Mon, 24 Aug 2026 19:08:25 +0000 Subject: [PATCH 5/9] fix(companion): finish wiring result and cancel to the shared schema addad6f claimed the handlers' inline parse literals were replaced by lookups into COMMAND_OPTION_SCHEMAS. That was true for setup, review, adversarial-review, task, transfer and status, but not for result and cancel, which kept literals duplicating their map entries. Those two could therefore still drift from help detection -- the exact failure that commit set out to make impossible. Both now read the shared schema, so every subcommand in USAGE_LINES has one definition. handleTaskWorker and handleTaskResumeCandidate keep their own literals deliberately: neither is in USAGE_LINES, so neither is help-handled, and task-worker's --job-id has no business in a user-facing schema. No behaviour change -- the literals and the map entries were identical, which is why the suite did not catch the gap. Verified by hand as well: result --json -> parses (reports no finished jobs) cancel --json -> parses (reports nothing to cancel) result --help -> usage cancel "--json --help" -> usage Full suite: 98 passing / 0 failing of 98. --- plugins/codex/scripts/codex-companion.mjs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 44c9ea247..f11c9ce11 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -946,8 +946,7 @@ async function handleStatus(argv) { function handleResult(argv) { const { options, positionals } = parseCommandInput(argv, { - valueOptions: ["cwd"], - booleanOptions: ["json"] + ...COMMAND_OPTION_SCHEMAS.get("result") }); const cwd = resolveCommandCwd(options); @@ -999,8 +998,7 @@ function handleTaskResumeCandidate(argv) { async function handleCancel(argv) { const { options, positionals } = parseCommandInput(argv, { - valueOptions: ["cwd"], - booleanOptions: ["json"] + ...COMMAND_OPTION_SCHEMAS.get("cancel") }); const cwd = resolveCommandCwd(options); From a057dac675dc60a65a8afbc3ae2676964cab7cd8 Mon Sep 17 00:00:00 2001 From: Christopher Steigerwald Date: Mon, 24 Aug 2026 19:15:38 +0000 Subject: [PATCH 6/9] fix(companion): detect help through parseCommandInput so -C is understood Addresses the second P1 on #681. It still reproduced on fda50dc, which the automated pass had reported clean. Help detection called parseArgs directly, so it never saw the `C: "cwd"` alias that parseCommandInput injects for every subcommand. `-C --help` therefore left -C and its value as positionals, the "no focus text left over" rule returned false, and the handler dispatched with --help still in the input: adversarial-review "-C /tmp --help" -> started a review task "-C /tmp --help" -> started a task status "-C /tmp --help" -> looked for a job named "--help" Detection now goes through parseCommandInput, the same entry point the handlers use, so it inherits the shared alias and argv normalization instead of restating them. That is the same reason the option schemas were unified: every copy of the parser's knowledge is a copy that can drift, and this was the last one left. -C /tmp --help -> usage "why does --help start a review" -> review --cwd /tmp --help -> usage "review --help handling" -> review --wait --help -> usage "-- --help" -> review Test: help combined with -C prints usage and starts no turn. Verified non-vacuous against fda50dc, where it fails. Full suite: 99 passing / 0 failing of 99. --- plugins/codex/scripts/codex-companion.mjs | 9 ++++++-- tests/runtime.test.mjs | 25 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index f11c9ce11..3164b0c58 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -123,8 +123,13 @@ function isHelpRequest(subcommand, argv) { if (!schema) { return false; } - const { options, positionals } = parseArgs(normalizeArgv(argv), { - valueOptions: schema.valueOptions ?? [], + // Detect through parseCommandInput, not parseArgs: the handlers reach the parser that + // way, so this inherits the shared `-C` alias and the argv normalization instead of + // restating them. Calling parseArgs directly is what missed `-C --help` -- the + // alias was unknown here, so the flag and its value became positionals, help was not + // detected, and the handler then dispatched with --help left as input. + const { options, positionals } = parseCommandInput(argv, { + ...schema, booleanOptions: [...(schema.booleanOptions ?? []), "help"], aliasMap: { ...(schema.aliasMap ?? {}), h: "help" } }); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 199aaf812..fdc3f4457 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1021,6 +1021,31 @@ test("help is detected when the plugin passes all arguments as one string", () = assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); }); +test("help is detected when combined with the shared -C alias", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "README.md"), "hello again\n"); + + // -C is injected by parseCommandInput, not by any subcommand schema. Detecting help + // with a bare parseArgs call misses it, so -C and its value become positionals, the + // no-focus-text rule fails, and the handler dispatches with --help still in the input. + const result = run("node", [SCRIPT, "adversarial-review", `-C ${repo} --help`], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /^Usage:/); + const state = fs.existsSync(statePath) ? JSON.parse(fs.readFileSync(statePath, "utf8")) : {}; + assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); +}); + test("help is detected when it follows another recognized flag", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From 4747ec2956668b9ec17215342c2c06235a7c0b97 Mon Sep 17 00:00:00 2001 From: Christopher Steigerwald Date: Mon, 24 Aug 2026 19:25:49 +0000 Subject: [PATCH 7/9] fix(companion): let help win over a structured job id Addresses the P1 on a057dac. This one was destructive rather than wasteful. The "help only when no positionals remain" rule was applied to every subcommand. It exists because review focus text and task prompts are arbitrary user prose, where a --help token may be something the user meant literally. status, result and cancel take a structured job id instead, and there the leftover positional is not ambiguous at all -- so the rule rejected the help request and the handler proceeded: cancel "task-live --help" -> cancelled task-live status "task-live --help" -> looked up task-live result "task-live --help" -> looked up task-live Asking what a command does should never destroy the thing being asked about. Schemas now mark which subcommands take free-form positionals, and only those require the positional list to be empty: return schema.freeFormPositionals !== true || positionals.length === 0; review, adversarial-review and task are free-form; setup, transfer, status, result and cancel are not. cancel "job-x --help" -> usage "why does --help start a review" -> review status "job-x --help" -> usage "review --help handling" -> review result "job-x --help" -> usage "explain --help output" (task) -> task --wait --help / -C /tmp --help -> usage Test: a running job in state, `cancel "task-live --help"`, then assert usage was printed AND the job is still running -- the assertion that matters, since printing usage alone would not prove the job survived. Verified non-vacuous against a057dac, where the job is cancelled. Full suite: 100 passing / 0 failing of 100. --- plugins/codex/scripts/codex-companion.mjs | 23 +++++++++++-- tests/runtime.test.mjs | 42 +++++++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 3164b0c58..d662a6b5d 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -87,7 +87,17 @@ const USAGE_LINES = new Map([ // unrecognised token as a positional, so `adversarial-review --help` was joined into the // review's focus text and ran a full review -- minutes of wall clock and a model turn, // for someone who asked what the flags were. -const REVIEW_OPTION_SCHEMA = { valueOptions: ["base", "scope", "model", "cwd"], booleanOptions: ["json", "background", "wait"], aliasMap: { m: "model" } }; +// freeFormPositionals marks the subcommands whose positionals are arbitrary user prose +// (review focus text, task prompt). Only those need the "no positionals" rule, because +// only there can a flag-looking token be something the user meant literally. status, +// result and cancel take a structured job id instead, so help must win over it -- asking +// for help while naming a job must never cancel that job. +const REVIEW_OPTION_SCHEMA = { + valueOptions: ["base", "scope", "model", "cwd"], + booleanOptions: ["json", "background", "wait"], + aliasMap: { m: "model" }, + freeFormPositionals: true +}; // One schema per subcommand, read by BOTH the handler and help detection. Keeping a // separate hand-maintained list for help detection is what broke it: an option the help @@ -103,7 +113,8 @@ const COMMAND_OPTION_SCHEMAS = new Map([ { valueOptions: ["model", "effort", "cwd", "prompt-file"], booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background"], - aliasMap: { m: "model" } + aliasMap: { m: "model" }, + freeFormPositionals: true } ], ["transfer", { valueOptions: ["cwd", "source"], booleanOptions: ["json"] }], @@ -133,7 +144,13 @@ function isHelpRequest(subcommand, argv) { booleanOptions: [...(schema.booleanOptions ?? []), "help"], aliasMap: { ...(schema.aliasMap ?? {}), h: "help" } }); - return options.help === true && positionals.length === 0; + if (options.help !== true) { + return false; + } + // Only free-form subcommands need to defend against a flag-looking token that the user + // meant as prose. Where the positional is a structured job id, a leftover positional is + // not a reason to dispatch -- `cancel "job-1 --help"` must print usage, not cancel job-1. + return schema.freeFormPositionals !== true || positionals.length === 0; } function printUsage(subcommand) { diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index fdc3f4457..d8988042e 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1021,6 +1021,48 @@ test("help is detected when the plugin passes all arguments as one string", () = assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); }); +test("cancel with a job id and --help prints usage without cancelling the job", () => { + const workspace = makeTempDir(); + const stateDir = resolveStateDir(workspace); + fs.mkdirSync(path.join(stateDir, "jobs"), { recursive: true }); + fs.writeFileSync( + path.join(stateDir, "state.json"), + `${JSON.stringify( + { + version: 1, + config: { stopReviewGate: false }, + jobs: [ + { + id: "task-live", + kind: "task", + kindLabel: "task", + status: "running", + title: "Codex Task", + jobClass: "task", + summary: "A job that must survive a help request", + createdAt: "2026-03-18T15:30:00.000Z", + updatedAt: "2026-03-18T15:30:03.000Z" + } + ] + }, + null, + 2 + )}\n`, + "utf8" + ); + + // cancel takes a structured job id, not free-form text, so a leftover positional is no + // reason to dispatch. Applying the free-form rule here made `cancel "task-live --help"` + // cancel task-live instead of explaining the command. + const result = run("node", [SCRIPT, "cancel", "task-live --help"], { cwd: workspace }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /^Usage:/); + + const after = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + assert.equal(after.jobs[0].status, "running", "a help request cancelled the job"); +}); + test("help is detected when combined with the shared -C alias", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From b15c07ab79f7f3be3aa9504bc204795237b62e67 Mon Sep 17 00:00:00 2001 From: Christopher Steigerwald Date: Mon, 24 Aug 2026 19:51:17 +0000 Subject: [PATCH 8/9] fix(companion): stop native review inheriting free-form positionals Addresses the P2 on 4747ec2. review and adversarial-review share a handler, so they shared one schema and review inherited freeFormPositionals: true. But the two differ precisely on positionals: adversarial-review takes focus text, where a --help token may be prose the user meant literally, while validateNativeReviewRequest rejects ALL focus text, so review has no free-form use at all. The consequence was a help request answered with an unrelated complaint: review "--scope working-tree focus --help" -> `/codex:review` ... does not support custom focus text The parse options stay shared as REVIEW_PARSE_OPTIONS, since they genuinely are identical. Only the positional classification differs: adversarial-review adds freeFormPositionals, review does not. review "--scope working-tree focus --help" -> usage review "some focus text" -> unchanged focus-text error adversarial-review "review --help handling" -> review, focus intact cancel "job-x --help" -> usage Test asserts usage is printed, the focus-text error is NOT emitted, and no turn starts. Verified non-vacuous against 4747ec2, where it fails. Full suite: 101 passing / 0 failing of 101. --- plugins/codex/scripts/codex-companion.mjs | 16 ++++++++------ tests/runtime.test.mjs | 26 +++++++++++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index d662a6b5d..bea810258 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -92,11 +92,10 @@ const USAGE_LINES = new Map([ // only there can a flag-looking token be something the user meant literally. status, // result and cancel take a structured job id instead, so help must win over it -- asking // for help while naming a job must never cancel that job. -const REVIEW_OPTION_SCHEMA = { +const REVIEW_PARSE_OPTIONS = { valueOptions: ["base", "scope", "model", "cwd"], booleanOptions: ["json", "background", "wait"], - aliasMap: { m: "model" }, - freeFormPositionals: true + aliasMap: { m: "model" } }; // One schema per subcommand, read by BOTH the handler and help detection. Keeping a @@ -106,8 +105,13 @@ const REVIEW_OPTION_SCHEMA = { // here is automatically known to both, so the two can never disagree again. const COMMAND_OPTION_SCHEMAS = new Map([ ["setup", { valueOptions: ["cwd"], booleanOptions: ["json", "enable-review-gate", "disable-review-gate"] }], - ["review", REVIEW_OPTION_SCHEMA], - ["adversarial-review", REVIEW_OPTION_SCHEMA], + // Same parsing, different positional semantics. adversarial-review takes focus text, so + // a --help token there may be prose. Native review rejects ALL focus text in + // validateNativeReviewRequest, so it has no free-form use and help can win over any + // positional -- otherwise `review "--scope working-tree focus --help"` answers a help + // request with a confusing complaint about custom focus text. + ["review", REVIEW_PARSE_OPTIONS], + ["adversarial-review", { ...REVIEW_PARSE_OPTIONS, freeFormPositionals: true }], [ "task", { @@ -780,7 +784,7 @@ function enqueueBackgroundTask(cwd, job, request) { async function handleReviewCommand(argv, config) { const { options, positionals } = parseCommandInput(argv, { - ...REVIEW_OPTION_SCHEMA + ...REVIEW_PARSE_OPTIONS }); const cwd = resolveCommandCwd(options); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index d8988042e..0a852a036 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1021,6 +1021,32 @@ test("help is detected when the plugin passes all arguments as one string", () = assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); }); +test("native review honours help even when a positional is present", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "README.md"), "hello again\n"); + + // Native review rejects all focus text in validateNativeReviewRequest, so unlike + // adversarial-review it has no free-form positional use. Sharing the free-form + // classification made a help request answered with a complaint about focus text. + const result = run("node", [SCRIPT, "review", "--scope working-tree focus --help"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /^Usage:/); + assert.equal(/custom focus text/.test(result.stderr), false, "help was answered with the focus-text error"); + const state = fs.existsSync(statePath) ? JSON.parse(fs.readFileSync(statePath, "utf8")) : {}; + assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); +}); + test("cancel with a job id and --help prints usage without cancelling the job", () => { const workspace = makeTempDir(); const stateDir = resolveStateDir(workspace); From bc4cb6fdf7e98f49577c7041412360870ccac1cb Mon Sep 17 00:00:00 2001 From: Christopher Steigerwald Date: Mon, 24 Aug 2026 20:11:14 +0000 Subject: [PATCH 9/9] fix(companion): only let a positional block help when it is really the input Addresses the P1 on b15c07a. task was marked free-form unconditionally, but readTaskPrompt returns the file whenever --prompt-file is given and never looks at the positional. A positional alongside --prompt-file is therefore discarded, so there is no literal prompt text to protect -- yet it still suppressed help: task "--prompt-file prompt.txt ignored --help" -> started a real Codex turn using prompt.txt The classification is now a predicate over the parsed options rather than a flag, because whether the positional is the input depends on the other options: positionalsAreFreeForm: (options) => !options["prompt-file"] adversarial-review declares () => true; its focus text is always used. Every other subcommand omits it, so help wins over a structured job id or an ignored positional. task "--prompt-file f.txt ignored --help" -> usage task "--prompt-file f.txt --help" -> usage task "do the thing --help" -> task, prompt intact adversarial-review "why does --help ..." -> review, focus intact cancel "job-x --help" -> usage review "--scope ... focus --help" -> usage Tests: help wins when --prompt-file displaces the positional (non-vacuous against b15c07a, where a turn starts); and a literal prompt mentioning --help still runs with the prompt intact. The second passes both ways -- it guards the protection this change narrows rather than evidencing the bug. Full suite: 103 passing / 0 failing of 103. --- plugins/codex/scripts/codex-companion.mjs | 30 ++++++++------ tests/runtime.test.mjs | 48 +++++++++++++++++++++++ 2 files changed, 67 insertions(+), 11 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index bea810258..c334a506e 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -87,11 +87,12 @@ const USAGE_LINES = new Map([ // unrecognised token as a positional, so `adversarial-review --help` was joined into the // review's focus text and ran a full review -- minutes of wall clock and a model turn, // for someone who asked what the flags were. -// freeFormPositionals marks the subcommands whose positionals are arbitrary user prose -// (review focus text, task prompt). Only those need the "no positionals" rule, because -// only there can a flag-looking token be something the user meant literally. status, -// result and cancel take a structured job id instead, so help must win over it -- asking -// for help while naming a job must never cancel that job. +// positionalsAreFreeForm marks the subcommands whose positionals are arbitrary user +// prose, which is the only case where a leftover positional should block a help request: +// there, a --help token may be something the user meant literally. It is a predicate over +// the parsed options rather than a flag, because for task it depends on whether +// --prompt-file displaced the positional. Everything else -- a structured job id, or a +// positional the handler discards -- must let help win. const REVIEW_PARSE_OPTIONS = { valueOptions: ["base", "scope", "model", "cwd"], booleanOptions: ["json", "background", "wait"], @@ -111,14 +112,18 @@ const COMMAND_OPTION_SCHEMAS = new Map([ // positional -- otherwise `review "--scope working-tree focus --help"` answers a help // request with a confusing complaint about custom focus text. ["review", REVIEW_PARSE_OPTIONS], - ["adversarial-review", { ...REVIEW_PARSE_OPTIONS, freeFormPositionals: true }], + ["adversarial-review", { ...REVIEW_PARSE_OPTIONS, positionalsAreFreeForm: () => true }], [ "task", { valueOptions: ["model", "effort", "cwd", "prompt-file"], booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background"], aliasMap: { m: "model" }, - freeFormPositionals: true + // Only when the positional is actually the prompt. readTaskPrompt returns the file + // unconditionally when --prompt-file is given, so a positional alongside it is + // discarded -- there is no literal prompt text to protect, and suppressing help + // over it starts a real turn for someone who asked what the flags were. + positionalsAreFreeForm: (options) => !options["prompt-file"] } ], ["transfer", { valueOptions: ["cwd", "source"], booleanOptions: ["json"] }], @@ -151,10 +156,13 @@ function isHelpRequest(subcommand, argv) { if (options.help !== true) { return false; } - // Only free-form subcommands need to defend against a flag-looking token that the user - // meant as prose. Where the positional is a structured job id, a leftover positional is - // not a reason to dispatch -- `cancel "job-1 --help"` must print usage, not cancel job-1. - return schema.freeFormPositionals !== true || positionals.length === 0; + // A positional only blocks help when it is genuinely free-form input the user may have + // meant literally. Where it is a structured job id (`cancel "job-1 --help"`), or where + // the handler discards it anyway (`task --prompt-file f.txt ignored`), it is not a + // reason to dispatch. + const freeForm = + typeof schema.positionalsAreFreeForm === "function" && schema.positionalsAreFreeForm(options); + return !freeForm || positionals.length === 0; } function printUsage(subcommand) { diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 0a852a036..55e979912 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1021,6 +1021,54 @@ test("help is detected when the plugin passes all arguments as one string", () = assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); }); +test("task honours help when --prompt-file makes the positional irrelevant", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "prompt.txt"), "the real prompt\n"); + + // readTaskPrompt returns the file unconditionally when --prompt-file is set, so the + // positional is discarded. Treating it as protected prompt text suppressed help and + // started a real turn using prompt.txt. + const result = run("node", [SCRIPT, "task", "--prompt-file prompt.txt ignored --help"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /^Usage:/); + const state = fs.existsSync(statePath) ? JSON.parse(fs.readFileSync(statePath, "utf8")) : {}; + assert.equal(state.lastTurnStart ?? null, null, "a help request started a Codex turn"); +}); + +test("task still protects a literal prompt that mentions --help", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + // Without --prompt-file the positional IS the prompt, so it must still block help. + const result = run("node", [SCRIPT, "task", "explain what --help prints"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.equal(/^Usage:/.test(result.stdout), false, "a literal prompt printed usage"); + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.ok(state.lastTurnStart, "the task never started"); + assert.match(state.lastTurnStart.prompt, /what --help prints/); +}); + test("native review honours help even when a positional is present", () => { const repo = makeTempDir(); const binDir = makeTempDir();