Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 3 additions & 36 deletions plugins/codex/scripts/codex-companion.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
44 changes: 44 additions & 0 deletions plugins/codex/scripts/lib/args.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
});
}
57 changes: 57 additions & 0 deletions tests/task-args.test.mjs
Original file line number Diff line number Diff line change
@@ -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");
});