diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..c2c7c9ee5 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -6,7 +6,7 @@ import path from "node:path"; import process from "node:process"; import { fileURLToPath } from "node:url"; -import { parseArgs, splitRawArgumentString } from "./lib/args.mjs"; +import { parseCommandInput, parseReviewArgv, parseTaskArgv } from "./lib/args.mjs"; import { buildPersistentTaskThreadName, DEFAULT_CONTINUE_PROMPT, @@ -127,27 +127,6 @@ function normalizeReasoningEffort(effort) { return normalized; } -function normalizeArgv(argv) { - if (argv.length === 1) { - const [raw] = argv; - if (!raw || !raw.trim()) { - return []; - } - return splitRawArgumentString(raw); - } - return argv; -} - -function parseCommandInput(argv, config = {}) { - return parseArgs(normalizeArgv(argv), { - ...config, - aliasMap: { - C: "cwd", - ...(config.aliasMap ?? {}) - } - }); -} - function resolveCommandCwd(options = {}) { return options.cwd ? path.resolve(process.cwd(), options.cwd) : process.cwd(); } @@ -710,13 +689,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" - } - }); + const { options, positionals } = parseReviewArgv(argv); const cwd = resolveCommandCwd(options); const workspaceRoot = resolveCommandWorkspace(options); @@ -760,13 +733,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" - } - }); + const { options, positionals } = parseTaskArgv(argv); const cwd = resolveCommandCwd(options); const workspaceRoot = resolveCommandWorkspace(options); diff --git a/plugins/codex/scripts/lib/args.mjs b/plugins/codex/scripts/lib/args.mjs index 6b1518502..2e9103c61 100644 --- a/plugins/codex/scripts/lib/args.mjs +++ b/plugins/codex/scripts/lib/args.mjs @@ -126,3 +126,47 @@ export function splitRawArgumentString(raw) { return tokens; } + +// A companion subcommand is often invoked with the whole request as a single +// argument (e.g. `codex-companion.mjs task "fix the flaky test"`). Shell-split it +// so option parsing sees individual tokens; a multi-argument argv is passed through. +export function normalizeArgv(argv) { + if (argv.length === 1) { + const [raw] = argv; + if (!raw || !raw.trim()) { + return []; + } + return splitRawArgumentString(raw); + } + return argv; +} + +export function parseCommandInput(argv, config = {}) { + return parseArgs(normalizeArgv(argv), { + ...config, + aliasMap: { + C: "cwd", + ...(config.aliasMap ?? {}) + } + }); +} + +// Command argument schemas live here so the parser and its tests share one source +// of truth. There is intentionally no short `-m` alias for `--model`: `task` and +// `review` prompts are free-form text that routinely contains tokens like +// `python -m pytest`, and a greedy `-m` alias swallowed the following word as the +// model (yielding e.g. `--model pytest` -> gateway 404) while dropping it from the +// prompt. Only the documented long `--model` form selects a model. See #699. +export function parseTaskArgv(argv) { + return parseCommandInput(argv, { + valueOptions: ["model", "effort", "cwd", "prompt-file"], + booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background"] + }); +} + +export function parseReviewArgv(argv) { + return parseCommandInput(argv, { + valueOptions: ["base", "scope", "model", "cwd"], + booleanOptions: ["json", "background", "wait"] + }); +} diff --git a/tests/task-args.test.mjs b/tests/task-args.test.mjs new file mode 100644 index 000000000..6eadd0ba4 --- /dev/null +++ b/tests/task-args.test.mjs @@ -0,0 +1,57 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { parseReviewArgv, parseTaskArgv } from "../plugins/codex/scripts/lib/args.mjs"; + +// Regression coverage for issue #699 (defect 1): free-form prompt text was parsed +// as CLI options. A task prompt containing `python -m pytest` was tokenized and the +// undocumented `-m` alias consumed the next word as `--model` (e.g. `--model pytest`, +// which the gateway rejects with a 404), while also dropping those words from the +// prompt. Only the documented long `--model` form should select a model. + +test("task prompt keeps `-m` tokens instead of hijacking --model", () => { + const { options, positionals } = parseTaskArgv([ + "fix the failing python -m pytest tests suite" + ]); + + assert.equal(options.model, undefined, "no model should be inferred from prompt text"); + assert.equal( + positionals.join(" "), + "fix the failing python -m pytest tests suite", + "the prompt text must be preserved verbatim, including `-m pytest`" + ); +}); + +test("bare `-m value` in a task prompt no longer sets the model", () => { + const { options, positionals } = parseTaskArgv(["-m pytest"]); + + assert.equal(options.model, undefined); + assert.deepEqual(positionals, ["-m", "pytest"]); +}); + +test("task still honors the documented long --model flag", () => { + const { options, positionals } = parseTaskArgv(["--model spark do the thing"]); + + assert.equal(options.model, "spark"); + assert.equal(positionals.join(" "), "do the thing"); +}); + +test("task boolean flags still parse alongside a prompt", () => { + const { options, positionals } = parseTaskArgv([ + "--background --write refactor the payment module" + ]); + + assert.equal(options.background, true); + assert.equal(options.write, true); + assert.equal(positionals.join(" "), "refactor the payment module"); +}); + +test("review focus text keeps `-m` tokens but still honors --model", () => { + const swallowed = parseReviewArgv(["check the -m pytest invocation in ci"]); + assert.equal(swallowed.options.model, undefined); + assert.equal(swallowed.positionals.join(" "), "check the -m pytest invocation in ci"); + + const explicit = parseReviewArgv(["--model spark --scope branch"]); + assert.equal(explicit.options.model, "spark"); + assert.equal(explicit.options.scope, "branch"); +});