From 9da0b2cc14521513f2d53ffa0042855b2aae1af5 Mon Sep 17 00:00:00 2001 From: drewyd Date: Wed, 26 Aug 2026 11:52:35 +1000 Subject: [PATCH] fix: resolve model aliases on review and adversarial-review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `handleReviewCommand` accepted `--model`/`-m` but forwarded the raw string to `executeReviewRun`, so `normalizeRequestedModel()` — the only thing that maps `MODEL_ALIASES` — never ran on the review path. `--model spark` reached `thread/start` as the literal `spark` and came back as: The 'spark' model is not supported when using Codex with a ChatGPT account. which names the account as the cause when the account is fine and `spark` was never a model id. The alias is documented for the runtime in skills/codex-cli-runtime/SKILL.md and agents/codex-rescue.md with nothing marking it task-only. Normalize in `handleReviewCommand` the way `handleTask` already does, so both command families resolve aliases identically. Tests: the fake app-server now records the model it receives on `thread/start` (it only recorded `turn/start`, so nothing could observe the review path), plus a regression test per command mirroring the existing task-path one. Both fail on main with `actual: 'spark', expected: 'gpt-5.3-codex-spark'`. Fixes #687 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QdCdeAZa2tCJjFq69meyK9 --- plugins/codex/scripts/codex-companion.mjs | 3 +- tests/fake-codex-fixture.mjs | 2 ++ tests/runtime.test.mjs | 44 +++++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..d2c6fb86e 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -720,6 +720,7 @@ async function handleReviewCommand(argv, config) { const cwd = resolveCommandCwd(options); const workspaceRoot = resolveCommandWorkspace(options); + const model = normalizeRequestedModel(options.model); const focusText = positionals.join(" ").trim(); const target = resolveReviewTarget(cwd, { base: options.base, @@ -743,7 +744,7 @@ async function handleReviewCommand(argv, config) { cwd, base: options.base, scope: options.scope, - model: options.model, + model, focusText, reviewName: config.reviewName, onProgress: progress diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index f83c96a0d..e770b6f87 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -313,6 +313,8 @@ rl.on("line", (line) => { throw new Error("thread/start.persistFullHistory requires experimentalApi capability"); } const thread = nextThread(state, message.params.cwd, message.params.ephemeral); + state.lastThreadStart = { threadId: thread.id, model: message.params.model ?? null }; + saveState(state); send({ id: message.id, result: { thread: buildThread(thread), model: message.params.model || "gpt-5.4", modelProvider: "openai", serviceTier: null, cwd: thread.cwd, approvalPolicy: "never", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: null } }); send({ method: "thread/started", params: { thread: { id: thread.id } } }); break; diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..5b5068ea9 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -784,6 +784,50 @@ test("task forwards model selection and reasoning effort to app-server turn/star assert.equal(fakeState.lastTurnStart.effort, "low"); }); +test("review resolves model aliases the same way task does", () => { + 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 = 1;\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 = 2;\n"); + + const result = run("node", [SCRIPT, "review", "--model", "spark"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.model, "gpt-5.3-codex-spark"); +}); + +test("adversarial review resolves model aliases the same way task does", () => { + 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 = 1;\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 = 2;\n"); + + const result = run("node", [SCRIPT, "adversarial-review", "--model", "spark"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.model, "gpt-5.3-codex-spark"); +}); + test("task logs reasoning summaries and assistant messages to the job log", () => { const repo = makeTempDir(); const binDir = makeTempDir();