From 0fc4362e414389fd31d7d15f97df9040496e92eb Mon Sep 17 00:00:00 2001 From: caleb Date: Wed, 8 Jul 2026 01:15:35 -0700 Subject: [PATCH 1/7] fix: accept max and ultra reasoning efforts for task --effort Codex CLI 0.142.x model catalogs advertise max and ultra as supported reasoning levels (e.g. gpt-5.6-sol: low/medium/high/xhigh/max/ultra, per 'codex debug models'), but the companion's hardcoded VALID_REASONING_EFFORTS still stops at xhigh, so 'task --effort max' and 'task --effort ultra' are rejected before dispatch: Unsupported reasoning effort "max". Use one of: none, minimal, low, medium, high, xhigh. Users cannot request the top reasoning tiers through the plugin at all; the only workaround is setting model_reasoning_effort in ~/.codex/config.toml and omitting --effort. - add max and ultra to VALID_REASONING_EFFORTS - derive the error message from the set so the list can no longer drift - update the usage line, /codex:rescue argument-hint, and the codex-cli-runtime skill's accepted-values line - runtime tests: forward max/ultra to turn/start via the fake codex, and keep rejecting unknown values before a job starts Co-Authored-By: Claude Fable 5 --- plugins/codex/commands/rescue.md | 2 +- plugins/codex/scripts/codex-companion.mjs | 6 +-- .../codex/skills/codex-cli-runtime/SKILL.md | 2 +- tests/commands.test.mjs | 4 +- tests/runtime.test.mjs | 38 +++++++++++++++++++ 5 files changed, 45 insertions(+), 7 deletions(-) diff --git a/plugins/codex/commands/rescue.md b/plugins/codex/commands/rescue.md index 56de9555..2d610e7b 100644 --- a/plugins/codex/commands/rescue.md +++ b/plugins/codex/commands/rescue.md @@ -1,6 +1,6 @@ --- description: Delegate investigation, an explicit fix request, or follow-up rescue work to the Codex rescue subagent -argument-hint: "[--background|--wait] [--resume|--fresh] [--model ] [--effort ] [what Codex should investigate, solve, or continue]" +argument-hint: "[--background|--wait] [--resume|--fresh] [--model ] [--effort ] [what Codex should investigate, solve, or continue]" allowed-tools: Bash(node:*), AskUserQuestion, Agent --- diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468a..4dbd4a91 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -68,7 +68,7 @@ const ROOT_DIR = path.resolve(fileURLToPath(new URL("..", import.meta.url))); const REVIEW_SCHEMA = path.join(ROOT_DIR, "schemas", "review-output.schema.json"); const DEFAULT_STATUS_WAIT_TIMEOUT_MS = 240000; const DEFAULT_STATUS_POLL_INTERVAL_MS = 2000; -const VALID_REASONING_EFFORTS = new Set(["none", "minimal", "low", "medium", "high", "xhigh"]); +const VALID_REASONING_EFFORTS = new Set(["none", "minimal", "low", "medium", "high", "xhigh", "max", "ultra"]); 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."; @@ -79,7 +79,7 @@ function printUsage() { " 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 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]", @@ -121,7 +121,7 @@ function normalizeReasoningEffort(effort) { } if (!VALID_REASONING_EFFORTS.has(normalized)) { throw new Error( - `Unsupported reasoning effort "${effort}". Use one of: none, minimal, low, medium, high, xhigh.` + `Unsupported reasoning effort "${effort}". Use one of: ${[...VALID_REASONING_EFFORTS].join(", ")}.` ); } return normalized; diff --git a/plugins/codex/skills/codex-cli-runtime/SKILL.md b/plugins/codex/skills/codex-cli-runtime/SKILL.md index 0e91bfb5..3bc67752 100644 --- a/plugins/codex/skills/codex-cli-runtime/SKILL.md +++ b/plugins/codex/skills/codex-cli-runtime/SKILL.md @@ -32,7 +32,7 @@ Command selection: - If the forwarded request includes `--fresh`, strip that token from the task text and do not add `--resume-last`. - `--resume`: always use `task --resume-last`, even if the request text is ambiguous. - `--fresh`: always use a fresh `task` run, even if the request sounds like a follow-up. -- `--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`. +- `--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `ultra`. - `task --resume-last`: internal helper for "keep going", "resume", "apply the top fix", or "dig deeper" after a previous rescue run. Safety rules: diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index c34b0605..346ed2e8 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -104,7 +104,7 @@ test("rescue command absorbs continue semantics", () => { assert.match(rescue, /--background\|--wait/); assert.match(rescue, /--resume\|--fresh/); assert.match(rescue, /--model /); - assert.match(rescue, /--effort /); + assert.match(rescue, /--effort /); assert.match(rescue, /task-resume-candidate --json/); assert.match(rescue, /AskUserQuestion/); assert.match(rescue, /Continue current Codex thread/); @@ -150,7 +150,7 @@ test("rescue command absorbs continue semantics", () => { assert.match(runtimeSkill, /Map `spark` to `--model gpt-5\.3-codex-spark`/i); assert.match(runtimeSkill, /If the forwarded request includes `--background` or `--wait`, treat that as Claude-side execution control only/i); assert.match(runtimeSkill, /Strip it before calling `task`/i); - assert.match(runtimeSkill, /`--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`/i); + assert.match(runtimeSkill, /`--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `ultra`/i); assert.match(runtimeSkill, /Do not inspect the repository, read files, grep, monitor progress, poll status, fetch results, cancel jobs, summarize output, or do any follow-up work of your own/i); assert.match(runtimeSkill, /If the Bash call fails or Codex cannot be invoked, return nothing/i); assert.match(readme, /`codex:codex-rescue` subagent/i); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835..915fc287 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -784,6 +784,44 @@ test("task forwards model selection and reasoning effort to app-server turn/star assert.equal(fakeState.lastTurnStart.effort, "low"); }); +test("task accepts max and ultra reasoning efforts and forwards them to turn/start", () => { + 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 }); + + for (const effort of ["max", "ultra"]) { + const result = run("node", [SCRIPT, "task", "--fresh", "--effort", effort, "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.effort, effort); + } +}); + +test("task rejects an unknown reasoning effort before starting a job", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + initGitRepo(repo); + + const result = run("node", [SCRIPT, "task", "--effort", "hyperdrive", "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Unsupported reasoning effort "hyperdrive"/); + assert.match(result.stderr, /max, ultra/); +}); + test("task logs reasoning summaries and assistant messages to the job log", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From 917e058eec2883562752ce14deccc17c19770184 Mon Sep 17 00:00:00 2001 From: caleb wei Date: Wed, 8 Jul 2026 02:27:46 -0700 Subject: [PATCH 2/7] fix: validate explicit reasoning efforts against the resolved model's catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review of the first commit found that a bare allowlist extension leaves a severe failure mode reachable: Codex app-server accepts turn/start for model+effort pairs the model catalog does not list (e.g. gpt-5.4-mini + ultra) and then hangs with zero events and no timeout (reported upstream as openai/codex#31552). max/ultra are 5.6-family tiers, so the flags this PR adds would have walked users straight into that hang on older models. This commit adds model-aware validation on the task dispatch path: - capture the resolved model that thread/start / thread/resume already return (previously discarded), so effort-only dispatches are checked against the configured default model too; - before turn/start, when an explicit --effort is present, issue a bounded (3s) model/list request on the existing app-server connection and reject unsupported pairs eagerly, listing that model's supported tiers; - fail open with an explicit warning when the catalog is unavailable, malformed, or does not contain the resolved model (custom providers, older CLIs) — never block dispatch on catalog problems; - fake-codex fixture gains a model/list handler plus model-list-unsupported / model-list-malformed behaviors, and five new runtime tests cover: explicit unsupported pair, effort-only against the resolved default, catalog unavailable, catalog malformed, and unknown-model pass-through (spark alias). Suite: 98/98. Live check: gpt-5.4-mini --effort ultra now fails in ~4s with the actionable message instead of hanging indefinitely. Co-Authored-By: Claude Fable 5 --- plugins/codex/scripts/lib/codex.mjs | 65 ++++++++++++++++ tests/fake-codex-fixture.mjs | 18 +++++ tests/runtime.test.mjs | 114 +++++++++++++++++++++++++++- 3 files changed, 196 insertions(+), 1 deletion(-) diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index fead00cc..b210dfc9 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -1092,6 +1092,64 @@ export async function importExternalAgentSession(cwd, options = {}) { }); } +const MODEL_LIST_TIMEOUT_MS = 3000; + +async function validateReasoningEffortForModel(client, resolvedModel, effort, onProgress) { + const skip = (reason) => { + emitProgress( + onProgress, + `Warning: skipping reasoning-effort validation (${reason}); dispatching effort "${effort}" as requested.`, + "starting" + ); + }; + + const requestPromise = client.request("model/list", {}); + requestPromise.catch(() => {}); + let timer; + let response = null; + try { + response = await Promise.race([ + requestPromise, + new Promise((resolve) => { + timer = setTimeout(() => resolve(null), MODEL_LIST_TIMEOUT_MS); + }) + ]); + } catch { + response = null; + } finally { + clearTimeout(timer); + } + + const entries = Array.isArray(response?.data) ? response.data : null; + if (!entries) { + skip("model catalog unavailable or malformed"); + return; + } + if (!resolvedModel) { + skip("app-server did not report a resolved model"); + return; + } + const entry = entries.find((item) => item && (item.id === resolvedModel || item.model === resolvedModel)); + if (!entry) { + skip(`model "${resolvedModel}" is not in the model catalog`); + return; + } + const supported = Array.isArray(entry.supportedReasoningEfforts) + ? entry.supportedReasoningEfforts + .map((item) => item?.reasoningEffort) + .filter((value) => typeof value === "string" && value) + : []; + if (supported.length === 0) { + skip(`model "${resolvedModel}" reports no reasoning-effort catalog`); + return; + } + if (!supported.includes(effort)) { + throw new Error( + `Model "${resolvedModel}" does not support reasoning effort "${effort}". Supported: ${supported.join(", ")}.` + ); + } +} + export async function runAppServerTurn(cwd, options = {}) { const availability = getCodexAvailability(cwd); if (!availability.available) { @@ -1100,6 +1158,7 @@ export async function runAppServerTurn(cwd, options = {}) { return withAppServer(cwd, async (client) => { let threadId; + let resolvedModel = null; if (options.resumeThreadId) { emitProgress(options.onProgress, `Resuming thread ${options.resumeThreadId}.`, "starting"); @@ -1109,6 +1168,7 @@ export async function runAppServerTurn(cwd, options = {}) { ephemeral: false }); threadId = response.thread.id; + resolvedModel = response.model ?? null; } else { emitProgress(options.onProgress, "Starting Codex task thread.", "starting"); const response = await startThread(client, cwd, { @@ -1118,6 +1178,7 @@ export async function runAppServerTurn(cwd, options = {}) { threadName: options.persistThread ? options.threadName : options.threadName ?? null }); threadId = response.thread.id; + resolvedModel = response.model ?? null; } emitProgress(options.onProgress, `Thread ready (${threadId}).`, "starting", { @@ -1129,6 +1190,10 @@ export async function runAppServerTurn(cwd, options = {}) { throw new Error("A prompt is required for this Codex run."); } + if (options.effort) { + await validateReasoningEffortForModel(client, resolvedModel, options.effort, options.onProgress); + } + const turnState = await captureTurn( client, threadId, diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index f83c96a0..1457f3a3 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -351,6 +351,24 @@ rl.on("line", (line) => { break; } + case "model/list": { + if (BEHAVIOR === "model-list-unsupported") { + send({ id: message.id, error: { code: -32601, message: "Unsupported method: model/list" } }); + break; + } + if (BEHAVIOR === "model-list-malformed") { + send({ id: message.id, result: { data: "not-a-catalog" } }); + break; + } + const catalogEffort = (reasoningEffort) => ({ reasoningEffort, description: reasoningEffort }); + send({ id: message.id, result: { data: [ + { id: "gpt-5.6-sol", model: "gpt-5.6-sol", isDefault: false, defaultReasoningEffort: "medium", supportedReasoningEfforts: ["low", "medium", "high", "xhigh", "max", "ultra"].map(catalogEffort) }, + { id: "gpt-5.4", model: "gpt-5.4", isDefault: true, defaultReasoningEffort: "medium", supportedReasoningEfforts: ["low", "medium", "high", "xhigh"].map(catalogEffort) }, + { id: "gpt-5.4-mini", model: "gpt-5.4-mini", isDefault: false, defaultReasoningEffort: "medium", supportedReasoningEfforts: ["low", "medium", "high", "xhigh"].map(catalogEffort) } + ] } }); + break; + } + case "externalAgentConfig/import": { if (BEHAVIOR === "external-import-unsupported") { send({ id: message.id, error: { code: -32601, message: "Unsupported method: externalAgentConfig/import" } }); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 915fc287..462b6330 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -795,13 +795,14 @@ test("task accepts max and ultra reasoning efforts and forwards them to turn/sta run("git", ["commit", "-m", "init"], { cwd: repo }); for (const effort of ["max", "ultra"]) { - const result = run("node", [SCRIPT, "task", "--fresh", "--effort", effort, "diagnose the failing test"], { + const result = run("node", [SCRIPT, "task", "--fresh", "--model", "gpt-5.6-sol", "--effort", effort, "diagnose the failing test"], { cwd: repo, env: buildEnv(binDir) }); assert.equal(result.status, 0, result.stderr); const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.model, "gpt-5.6-sol"); assert.equal(fakeState.lastTurnStart.effort, effort); } }); @@ -822,6 +823,117 @@ test("task rejects an unknown reasoning effort before starting a job", () => { assert.match(result.stderr, /max, ultra/); }); +test("task rejects an effort the resolved model does not support before starting a turn", () => { + 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 }); + + const result = run("node", [SCRIPT, "task", "--model", "gpt-5.4-mini", "--effort", "ultra", "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Model "gpt-5\.4-mini" does not support reasoning effort "ultra"/); + assert.match(result.stderr, /low, medium, high, xhigh/); + if (fs.existsSync(statePath)) { + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart ?? null, null); + } +}); + +test("task validates an effort-only dispatch against the resolved default model", () => { + 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 }); + + const result = run("node", [SCRIPT, "task", "--effort", "ultra", "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Model "gpt-5\.4" does not support reasoning effort "ultra"/); + if (fs.existsSync(statePath)) { + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart ?? null, null); + } +}); + +test("task fails open with a warning when the model catalog is unavailable", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "model-list-unsupported"); + 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 }); + + const result = run("node", [SCRIPT, "task", "--model", "gpt-5.4-mini", "--effort", "ultra", "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout + result.stderr, /skipping reasoning-effort validation/); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.effort, "ultra"); +}); + +test("task fails open with a warning when the model catalog is malformed", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "model-list-malformed"); + 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 }); + + const result = run("node", [SCRIPT, "task", "--model", "gpt-5.4-mini", "--effort", "ultra", "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout + result.stderr, /skipping reasoning-effort validation/); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.effort, "ultra"); +}); + +test("task warns and proceeds when the resolved model is not in the catalog", () => { + 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 }); + + const result = run("node", [SCRIPT, "task", "--model", "spark", "--effort", "ultra", "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout + result.stderr, /is not in the model catalog/); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.model, "gpt-5.3-codex-spark"); + assert.equal(fakeState.lastTurnStart.effort, "ultra"); +}); + test("task logs reasoning summaries and assistant messages to the job log", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From ef1d739aad59c35caac7b6cb645b2c69fbb05341 Mon Sep 17 00:00:00 2001 From: caleb wei Date: Wed, 8 Jul 2026 09:39:00 -0700 Subject: [PATCH 3/7] fix: validate before thread creation and document per-model tier variance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (PR bot, two P2s): 1. Pre-turn validation failures no longer pollute --resume: for fresh dispatches the catalog check now runs BEFORE thread/start (effort-only dispatches resolve the target via the catalog's isDefault entry), so a rejected pair creates no thread at all — nothing for findLatestResumableTaskJob to latch onto, and no orphan ephemeral thread server-side. Resume dispatches validate against the resumed thread's reported model, reusing the same single model/list fetch; a failed check there leaves the pre-existing thread as the resume target, which is the correct candidate anyway. 2. none/minimal vs catalog: kept in the syntactic allowlist (older CLIs whose catalogs list them still pass the dynamic check; removing them would break those users), and NOT exempted from the catalog check (exempting reopens the silent turn/start hang for exactly those values). The runtime-skill doc now states that supported tiers vary by model and that catalog-rejected pairs fail before dispatch. Tests: rejection cases now also assert zero threads were created; docs assertion added. Suite 98/98. Co-Authored-By: Claude Fable 5 --- plugins/codex/scripts/lib/codex.mjs | 53 +++++++++++-------- .../codex/skills/codex-cli-runtime/SKILL.md | 2 +- tests/commands.test.mjs | 1 + tests/runtime.test.mjs | 2 + 4 files changed, 34 insertions(+), 24 deletions(-) diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index b210dfc9..25d341eb 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -1094,44 +1094,50 @@ export async function importExternalAgentSession(cwd, options = {}) { const MODEL_LIST_TIMEOUT_MS = 3000; -async function validateReasoningEffortForModel(client, resolvedModel, effort, onProgress) { - const skip = (reason) => { - emitProgress( - onProgress, - `Warning: skipping reasoning-effort validation (${reason}); dispatching effort "${effort}" as requested.`, - "starting" - ); - }; +function emitEffortValidationSkip(onProgress, effort, reason) { + emitProgress( + onProgress, + `Warning: skipping reasoning-effort validation (${reason}); dispatching effort "${effort}" as requested.`, + "starting" + ); +} +async function fetchModelCatalog(client) { const requestPromise = client.request("model/list", {}); requestPromise.catch(() => {}); let timer; - let response = null; try { - response = await Promise.race([ + const response = await Promise.race([ requestPromise, new Promise((resolve) => { timer = setTimeout(() => resolve(null), MODEL_LIST_TIMEOUT_MS); }) ]); + return Array.isArray(response?.data) ? response.data : null; } catch { - response = null; + return null; } finally { clearTimeout(timer); } +} - const entries = Array.isArray(response?.data) ? response.data : null; +function catalogDefaultModel(entries) { + const entry = Array.isArray(entries) ? entries.find((item) => item && item.isDefault) : null; + return entry ? entry.id ?? entry.model ?? null : null; +} + +function assertCatalogSupportsEffort(entries, resolvedModel, effort, onProgress) { if (!entries) { - skip("model catalog unavailable or malformed"); + emitEffortValidationSkip(onProgress, effort, "model catalog unavailable or malformed"); return; } if (!resolvedModel) { - skip("app-server did not report a resolved model"); + emitEffortValidationSkip(onProgress, effort, "could not resolve the target model"); return; } const entry = entries.find((item) => item && (item.id === resolvedModel || item.model === resolvedModel)); if (!entry) { - skip(`model "${resolvedModel}" is not in the model catalog`); + emitEffortValidationSkip(onProgress, effort, `model "${resolvedModel}" is not in the model catalog`); return; } const supported = Array.isArray(entry.supportedReasoningEfforts) @@ -1140,7 +1146,7 @@ async function validateReasoningEffortForModel(client, resolvedModel, effort, on .filter((value) => typeof value === "string" && value) : []; if (supported.length === 0) { - skip(`model "${resolvedModel}" reports no reasoning-effort catalog`); + emitEffortValidationSkip(onProgress, effort, `model "${resolvedModel}" reports no reasoning-effort catalog`); return; } if (!supported.includes(effort)) { @@ -1158,7 +1164,7 @@ export async function runAppServerTurn(cwd, options = {}) { return withAppServer(cwd, async (client) => { let threadId; - let resolvedModel = null; + const catalogEntries = options.effort ? await fetchModelCatalog(client) : null; if (options.resumeThreadId) { emitProgress(options.onProgress, `Resuming thread ${options.resumeThreadId}.`, "starting"); @@ -1168,8 +1174,14 @@ export async function runAppServerTurn(cwd, options = {}) { ephemeral: false }); threadId = response.thread.id; - resolvedModel = response.model ?? null; + if (options.effort) { + assertCatalogSupportsEffort(catalogEntries, response.model ?? null, options.effort, options.onProgress); + } } else { + if (options.effort) { + const targetModel = options.model ?? catalogDefaultModel(catalogEntries); + assertCatalogSupportsEffort(catalogEntries, targetModel, options.effort, options.onProgress); + } emitProgress(options.onProgress, "Starting Codex task thread.", "starting"); const response = await startThread(client, cwd, { model: options.model, @@ -1178,7 +1190,6 @@ export async function runAppServerTurn(cwd, options = {}) { threadName: options.persistThread ? options.threadName : options.threadName ?? null }); threadId = response.thread.id; - resolvedModel = response.model ?? null; } emitProgress(options.onProgress, `Thread ready (${threadId}).`, "starting", { @@ -1190,10 +1201,6 @@ export async function runAppServerTurn(cwd, options = {}) { throw new Error("A prompt is required for this Codex run."); } - if (options.effort) { - await validateReasoningEffortForModel(client, resolvedModel, options.effort, options.onProgress); - } - const turnState = await captureTurn( client, threadId, diff --git a/plugins/codex/skills/codex-cli-runtime/SKILL.md b/plugins/codex/skills/codex-cli-runtime/SKILL.md index 3bc67752..3335b81f 100644 --- a/plugins/codex/skills/codex-cli-runtime/SKILL.md +++ b/plugins/codex/skills/codex-cli-runtime/SKILL.md @@ -32,7 +32,7 @@ Command selection: - If the forwarded request includes `--fresh`, strip that token from the task text and do not add `--resume-last`. - `--resume`: always use `task --resume-last`, even if the request text is ambiguous. - `--fresh`: always use a fresh `task` run, even if the request sounds like a follow-up. -- `--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `ultra`. +- `--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `ultra`. Supported tiers vary by model (for example `max`/`ultra` are 5.6-family tiers); a pair the live model catalog rejects fails before dispatch with that model's supported list. - `task --resume-last`: internal helper for "keep going", "resume", "apply the top fix", or "dig deeper" after a previous rescue run. Safety rules: diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index 346ed2e8..de598e4b 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -151,6 +151,7 @@ test("rescue command absorbs continue semantics", () => { assert.match(runtimeSkill, /If the forwarded request includes `--background` or `--wait`, treat that as Claude-side execution control only/i); assert.match(runtimeSkill, /Strip it before calling `task`/i); assert.match(runtimeSkill, /`--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `ultra`/i); + assert.match(runtimeSkill, /Supported tiers vary by model/i); assert.match(runtimeSkill, /Do not inspect the repository, read files, grep, monitor progress, poll status, fetch results, cancel jobs, summarize output, or do any follow-up work of your own/i); assert.match(runtimeSkill, /If the Bash call fails or Codex cannot be invoked, return nothing/i); assert.match(readme, /`codex:codex-rescue` subagent/i); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 462b6330..e6146635 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -844,6 +844,7 @@ test("task rejects an effort the resolved model does not support before starting if (fs.existsSync(statePath)) { const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); assert.equal(fakeState.lastTurnStart ?? null, null); + assert.equal((fakeState.threads ?? []).length, 0, "no thread may be created for a rejected pair"); } }); @@ -867,6 +868,7 @@ test("task validates an effort-only dispatch against the resolved default model" if (fs.existsSync(statePath)) { const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); assert.equal(fakeState.lastTurnStart ?? null, null); + assert.equal((fakeState.threads ?? []).length, 0, "no thread may be created for a rejected pair"); } }); From f7f47ee95507a1e86fc6c12f7be7d61c966a00fe Mon Sep 17 00:00:00 2001 From: caleb wei Date: Wed, 8 Jul 2026 10:52:04 -0700 Subject: [PATCH 4/7] fix: validate only explicit model+effort pairs; keep broker stream ownership on request failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bug fixes, no behavior changes beyond them: 1. Effort validation now applies ONLY when both --model and --effort are explicit. The previous commit resolved effort-only dispatches via the catalog's global isDefault entry, but thread/start sends model:null and the server applies the config.toml default — a different model, so documented config-default flows could be falsely rejected (review comment on ef1d739). Effort-only dispatches now pass through unvalidated, exactly as before this PR. The same explicit-pair rule covers resume: resuming without --model continues on the thread's own model, and validating the config default against it falsely rejected e.g. resume-at-ultra after a successful ultra run. 2. The broker no longer clears activeStreamSocket when a NON-streaming request fails. A model/list that failed later than the client's 3s validation timeout stripped stream ownership from the in-flight turn on the same socket; its turn/completed notifications were then dropped and the client hung forever — reproducing openai/codex#31552 for fully supported pairs. Streaming-failure behavior is unchanged (the block never fired for streaming requests). 3. catalogDefaultModel() (the global-default guess) is removed. Regression tests (all red on the previous commit, green now): - effort-only dispatch forwards with no guessed model injected - resume-last with an explicit effort no longer validates against the config default - a slow-failing model/list no longer kills the turn's event stream on the broker transport (fixture gains model-list-slow-error; test run helper gains a timeout so a regression fails instead of hanging CI) Suite: 100/100. Co-Authored-By: Claude Fable 5 --- plugins/codex/scripts/app-server-broker.mjs | 6 +- plugins/codex/scripts/lib/codex.mjs | 21 ++++--- tests/fake-codex-fixture.mjs | 9 +++ tests/helpers.mjs | 1 + tests/runtime.test.mjs | 63 ++++++++++++++++++--- 5 files changed, 77 insertions(+), 23 deletions(-) diff --git a/plugins/codex/scripts/app-server-broker.mjs b/plugins/codex/scripts/app-server-broker.mjs index 1954274f..3de8a1f5 100644 --- a/plugins/codex/scripts/app-server-broker.mjs +++ b/plugins/codex/scripts/app-server-broker.mjs @@ -215,9 +215,9 @@ async function main() { if (activeRequestSocket === socket) { activeRequestSocket = null; } - if (activeStreamSocket === socket && !isStreaming) { - activeStreamSocket = null; - } + // Deliberately keep activeStreamSocket: a failed NON-streaming request must not + // strip stream ownership from an in-flight turn on the same socket, or its + // turn/completed notifications are dropped and the client hangs forever. } } }); diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index 25d341eb..24748ca9 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -1121,11 +1121,6 @@ async function fetchModelCatalog(client) { } } -function catalogDefaultModel(entries) { - const entry = Array.isArray(entries) ? entries.find((item) => item && item.isDefault) : null; - return entry ? entry.id ?? entry.model ?? null : null; -} - function assertCatalogSupportsEffort(entries, resolvedModel, effort, onProgress) { if (!entries) { emitEffortValidationSkip(onProgress, effort, "model catalog unavailable or malformed"); @@ -1164,7 +1159,12 @@ export async function runAppServerTurn(cwd, options = {}) { return withAppServer(cwd, async (client) => { let threadId; - const catalogEntries = options.effort ? await fetchModelCatalog(client) : null; + // Validate only an explicit --model/--effort pair. Without --model the target is + // whatever the server resolves (the config.toml default on fresh dispatches, the + // thread's own model on resume) — the catalog's global isDefault entry is NOT that + // model, so guessing here falsely rejects documented flows. + const shouldValidateEffort = Boolean(options.effort) && Boolean(options.model); + const catalogEntries = shouldValidateEffort ? await fetchModelCatalog(client) : null; if (options.resumeThreadId) { emitProgress(options.onProgress, `Resuming thread ${options.resumeThreadId}.`, "starting"); @@ -1174,13 +1174,12 @@ export async function runAppServerTurn(cwd, options = {}) { ephemeral: false }); threadId = response.thread.id; - if (options.effort) { - assertCatalogSupportsEffort(catalogEntries, response.model ?? null, options.effort, options.onProgress); + if (shouldValidateEffort) { + assertCatalogSupportsEffort(catalogEntries, response.model ?? options.model, options.effort, options.onProgress); } } else { - if (options.effort) { - const targetModel = options.model ?? catalogDefaultModel(catalogEntries); - assertCatalogSupportsEffort(catalogEntries, targetModel, options.effort, options.onProgress); + if (shouldValidateEffort) { + assertCatalogSupportsEffort(catalogEntries, options.model, options.effort, options.onProgress); } emitProgress(options.onProgress, "Starting Codex task thread.", "starting"); const response = await startThread(client, cwd, { diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index 1457f3a3..c4fb10cb 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -356,6 +356,12 @@ rl.on("line", (line) => { send({ id: message.id, error: { code: -32601, message: "Unsupported method: model/list" } }); break; } + if (BEHAVIOR === "model-list-slow-error") { + setTimeout(() => { + send({ id: message.id, error: { code: -32000, message: "model catalog backend timed out" } }); + }, 3500); + break; + } if (BEHAVIOR === "model-list-malformed") { send({ id: message.id, result: { data: "not-a-catalog" } }); break; @@ -620,6 +626,9 @@ rl.on("line", (line) => { interruptibleTurns.set(turnId, { threadId: thread.id, timer }); } else if (BEHAVIOR === "slow-task") { emitTurnCompletedLater(thread.id, turnId, items, 400); + } else if (BEHAVIOR === "model-list-slow-error") { + send({ method: "turn/started", params: { threadId: thread.id, turn: buildTurn(turnId) } }); + emitTurnCompletedLater(thread.id, turnId, items, 1800); } else { emitTurnCompleted(thread.id, turnId, items); } diff --git a/tests/helpers.mjs b/tests/helpers.mjs index d6981197..41ced54d 100644 --- a/tests/helpers.mjs +++ b/tests/helpers.mjs @@ -18,6 +18,7 @@ export function run(command, args, options = {}) { env: options.env, encoding: "utf8", input: options.input, + timeout: options.timeout, shell: options.shell ?? (process.platform === "win32" && !path.isAbsolute(command)), windowsHide: true }); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index e6146635..7bb7d167 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -848,7 +848,7 @@ test("task rejects an effort the resolved model does not support before starting } }); -test("task validates an effort-only dispatch against the resolved default model", () => { +test("task forwards an effort-only dispatch without validating against a guessed default model", () => { const repo = makeTempDir(); const binDir = makeTempDir(); const statePath = path.join(binDir, "fake-codex-state.json"); @@ -858,18 +858,15 @@ test("task validates an effort-only dispatch against the resolved default model" run("git", ["add", "README.md"], { cwd: repo }); run("git", ["commit", "-m", "init"], { cwd: repo }); - const result = run("node", [SCRIPT, "task", "--effort", "ultra", "diagnose the failing test"], { + const result = run("node", [SCRIPT, "task", "--fresh", "--effort", "ultra", "diagnose the failing test"], { cwd: repo, env: buildEnv(binDir) }); - assert.notEqual(result.status, 0); - assert.match(result.stderr, /Model "gpt-5\.4" does not support reasoning effort "ultra"/); - if (fs.existsSync(statePath)) { - const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); - assert.equal(fakeState.lastTurnStart ?? null, null); - assert.equal((fakeState.threads ?? []).length, 0, "no thread may be created for a rejected pair"); - } + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.effort, "ultra"); + assert.equal(fakeState.lastTurnStart.model, null, "no guessed model may be injected into turn/start"); }); test("task fails open with a warning when the model catalog is unavailable", () => { @@ -914,6 +911,54 @@ test("task fails open with a warning when the model catalog is malformed", () => assert.equal(fakeState.lastTurnStart.effort, "ultra"); }); +test("task --resume-last keeps an explicit effort without validating against the config default", () => { + 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 }); + + const firstRun = run("node", [SCRIPT, "task", "--fresh", "--model", "gpt-5.6-sol", "--effort", "ultra", "start the migration"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(firstRun.status, 0, firstRun.stderr); + + const resume = run("node", [SCRIPT, "task", "--resume-last", "--effort", "ultra", "keep going"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(resume.status, 0, resume.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.effort, "ultra"); +}); + +test("a slow-failing model/list does not kill the turn's event stream on the broker transport", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "model-list-slow-error"); + 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 }); + + const result = run("node", [SCRIPT, "task", "--fresh", "--model", "gpt-5.6-sol", "--effort", "ultra", "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir), + timeout: 30000 + }); + + assert.equal(result.status, 0, `companion did not complete (stream ownership lost?): ${result.stderr}`); + assert.match(result.stdout + result.stderr, /skipping reasoning-effort validation/); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.effort, "ultra"); +}); + test("task warns and proceeds when the resolved model is not in the catalog", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From c13e10632a9dadc05a2ee58394e02816dc7581a1 Mon Sep 17 00:00:00 2001 From: caleb wei Date: Wed, 8 Jul 2026 12:50:37 -0700 Subject: [PATCH 5/7] fix: validate the requested resume model before thread/resume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by a pre-push adversarial review: the resume path validated response.model ?? options.model, but turn/start dispatches options.model. When the two diverge (a loaded shared-broker thread reporting prior/config state), validation could falsely reject a valid requested pair or approve an unsupported one, and rejection landed only after thread/resume side effects. Validation now runs before thread/resume against exactly the model the turn will use — symmetric with the fresh path. Regression test drives thread/resume through a fixture behavior that reports a different model than requested (both directions). Red on the previous commit, green now. Co-Authored-By: Claude Fable 5 --- plugins/codex/scripts/lib/codex.mjs | 10 +++-- tests/runtime.test.mjs | 61 +++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index 24748ca9..7102f8de 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -1167,6 +1167,13 @@ export async function runAppServerTurn(cwd, options = {}) { const catalogEntries = shouldValidateEffort ? await fetchModelCatalog(client) : null; if (options.resumeThreadId) { + // Validate the model that turn/start will actually receive, before thread/resume + // has any side effect. The resume response's model is NOT that model (it reports + // the config default / prior thread state), so checking it can both falsely reject + // a valid requested pair and approve an unsupported one. + if (shouldValidateEffort) { + assertCatalogSupportsEffort(catalogEntries, options.model, options.effort, options.onProgress); + } emitProgress(options.onProgress, `Resuming thread ${options.resumeThreadId}.`, "starting"); const response = await resumeThread(client, options.resumeThreadId, cwd, { model: options.model, @@ -1174,9 +1181,6 @@ export async function runAppServerTurn(cwd, options = {}) { ephemeral: false }); threadId = response.thread.id; - if (shouldValidateEffort) { - assertCatalogSupportsEffort(catalogEntries, response.model ?? options.model, options.effort, options.onProgress); - } } else { if (shouldValidateEffort) { assertCatalogSupportsEffort(catalogEntries, options.model, options.effort, options.onProgress); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 7bb7d167..549975b3 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -937,6 +937,67 @@ test("task --resume-last keeps an explicit effort without validating against the assert.equal(fakeState.lastTurnStart.effort, "ultra"); }); +test("resume validates the requested model, not the model reported by thread/resume", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "resume-reports-different-model"); + 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 }); + + const firstRun = run("node", [SCRIPT, "task", "--fresh", "--model", "gpt-5.6-sol", "--effort", "ultra", "start the migration"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(firstRun.status, 0, firstRun.stderr); + + // thread/resume reports gpt-5.6-sol here; the requested gpt-5.4-mini is what + // turn/start would receive, so the pair must be rejected before any resume. + const reject = run("node", [SCRIPT, "task", "--resume-last", "--model", "gpt-5.4-mini", "--effort", "ultra", "keep going"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.notEqual(reject.status, 0); + assert.match(reject.stderr, /Model "gpt-5\.4-mini" does not support reasoning effort "ultra"/); + let fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.match(fakeState.lastTurnStart.prompt, /start the migration/, "rejected resume must not start a turn"); + + // thread/resume reports gpt-5.4-mini here; the requested gpt-5.6-sol supports + // ultra, so the resume must proceed instead of being falsely rejected. + const approve = run("node", [SCRIPT, "task", "--resume-last", "--model", "gpt-5.6-sol", "--effort", "ultra", "keep going"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(approve.status, 0, approve.stderr); + fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.model, "gpt-5.6-sol"); + assert.equal(fakeState.lastTurnStart.effort, "ultra"); +}); + +test("a model/list settling during an in-flight request does not drop turn events or broker serialization", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "model-list-slow-error-during-request"); + 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 }); + + const result = run("node", [SCRIPT, "task", "--fresh", "--model", "gpt-5.6-sol", "--effort", "ultra", "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir), + timeout: 30000 + }); + + assert.equal(result.status, 0, `companion did not complete (request slot lost?): ${result.stderr}`); + assert.match(result.stdout + result.stderr, /skipping reasoning-effort validation/); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.effort, "ultra"); +}); + test("a slow-failing model/list does not kill the turn's event stream on the broker transport", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From 77193b11b13461b8bc794e109b1e16f2fa1ef946 Mon Sep 17 00:00:00 2001 From: caleb wei Date: Wed, 8 Jul 2026 12:50:37 -0700 Subject: [PATCH 6/7] fix: refcount broker request ownership; document legacy none/minimal efforts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second pre-push review (independent, probe-verified) found that the abandoned model/list request also races the broker's single-slot activeRequestSocket: every completion — success or error — cleared the slot, so a model/list settling while turn/start was in flight on the same socket released ownership early. Turn notifications arriving in the same stdout chunk as the turn/start response are dispatched before the microtask that sets activeStreamSocket, so with the request slot already cleared they were dropped entirely: turn/completed never reached the client (permanent hang, no timeout), a second socket could pass the busy check mid-turn (cross-session serialization bypass), and a dropped turn/started left cancel unable to interrupt the server-side turn. Request ownership is now refcounted per socket and released only when the last in-flight request completes; socket close still clears everything. Regression test: a fixture behavior delays turn/start's response past the late model/list error and flushes the response plus all turn events as one stdout chunk — red (hang, killed by the run timeout) on the previous commit, green now. Docs: three independent reviewers tripped over none/minimal being advertised yet rejected for every cataloged model; the runtime skill now states they are legacy values that current catalogs do not list and will be rejected with an explicit --model. Behavior unchanged. Suite: 102/102. Co-Authored-By: Claude Fable 5 --- plugins/codex/scripts/app-server-broker.mjs | 27 ++++++++++++++----- .../codex/skills/codex-cli-runtime/SKILL.md | 2 +- tests/commands.test.mjs | 1 + tests/fake-codex-fixture.mjs | 24 ++++++++++++++++- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/plugins/codex/scripts/app-server-broker.mjs b/plugins/codex/scripts/app-server-broker.mjs index 3de8a1f5..4dce6e0c 100644 --- a/plugins/codex/scripts/app-server-broker.mjs +++ b/plugins/codex/scripts/app-server-broker.mjs @@ -70,8 +70,26 @@ async function main() { let activeStreamSocket = null; let activeStreamThreadIds = null; const sockets = new Set(); + // Requests on one socket can overlap (a client may stop waiting for a slow request + // and issue the next one), so request ownership must be refcounted: releasing the + // slot on the FIRST completion would drop notifications for the still-running + // request and let another socket bypass serialization mid-flight. + const inflightRequests = new Map(); + + function releaseRequestSlot(socket) { + const remaining = (inflightRequests.get(socket) ?? 1) - 1; + if (remaining > 0) { + inflightRequests.set(socket, remaining); + return; + } + inflightRequests.delete(socket); + if (activeRequestSocket === socket) { + activeRequestSocket = null; + } + } function clearSocketOwnership(socket) { + inflightRequests.delete(socket); if (activeRequestSocket === socket) { activeRequestSocket = null; } @@ -196,6 +214,7 @@ async function main() { const isStreaming = STREAMING_METHODS.has(message.method); activeRequestSocket = socket; + inflightRequests.set(socket, (inflightRequests.get(socket) ?? 0) + 1); try { const result = await appClient.request(message.method, message.params ?? {}); @@ -204,17 +223,13 @@ async function main() { activeStreamSocket = socket; activeStreamThreadIds = buildStreamThreadIds(message.method, message.params ?? {}, result); } - if (activeRequestSocket === socket) { - activeRequestSocket = null; - } + releaseRequestSlot(socket); } catch (error) { send(socket, { id: message.id, error: buildJsonRpcError(error.rpcCode ?? -32000, error.message) }); - if (activeRequestSocket === socket) { - activeRequestSocket = null; - } + releaseRequestSlot(socket); // Deliberately keep activeStreamSocket: a failed NON-streaming request must not // strip stream ownership from an in-flight turn on the same socket, or its // turn/completed notifications are dropped and the client hangs forever. diff --git a/plugins/codex/skills/codex-cli-runtime/SKILL.md b/plugins/codex/skills/codex-cli-runtime/SKILL.md index 3335b81f..9e5e32a7 100644 --- a/plugins/codex/skills/codex-cli-runtime/SKILL.md +++ b/plugins/codex/skills/codex-cli-runtime/SKILL.md @@ -32,7 +32,7 @@ Command selection: - If the forwarded request includes `--fresh`, strip that token from the task text and do not add `--resume-last`. - `--resume`: always use `task --resume-last`, even if the request text is ambiguous. - `--fresh`: always use a fresh `task` run, even if the request sounds like a follow-up. -- `--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `ultra`. Supported tiers vary by model (for example `max`/`ultra` are 5.6-family tiers); a pair the live model catalog rejects fails before dispatch with that model's supported list. +- `--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `ultra`. Supported tiers vary by model (for example `max`/`ultra` are 5.6-family tiers); a pair the live model catalog rejects fails before dispatch with that model's supported list. `none` and `minimal` are legacy values that current catalogs do not list for any model — combined with an explicit `--model` they will be rejected the same way. - `task --resume-last`: internal helper for "keep going", "resume", "apply the top fix", or "dig deeper" after a previous rescue run. Safety rules: diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index de598e4b..92466848 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -152,6 +152,7 @@ test("rescue command absorbs continue semantics", () => { assert.match(runtimeSkill, /Strip it before calling `task`/i); assert.match(runtimeSkill, /`--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `ultra`/i); assert.match(runtimeSkill, /Supported tiers vary by model/i); + assert.match(runtimeSkill, /`none` and `minimal` are legacy values/i); assert.match(runtimeSkill, /Do not inspect the repository, read files, grep, monitor progress, poll status, fetch results, cancel jobs, summarize output, or do any follow-up work of your own/i); assert.match(runtimeSkill, /If the Bash call fails or Codex cannot be invoked, return nothing/i); assert.match(readme, /`codex:codex-rescue` subagent/i); diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index c4fb10cb..19525492 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -341,6 +341,12 @@ rl.on("line", (line) => { } case "thread/resume": { + if (BEHAVIOR === "resume-reports-different-model") { + const resumed = ensureThread(state, message.params.threadId); + const reported = message.params.model === "gpt-5.4-mini" ? "gpt-5.6-sol" : "gpt-5.4-mini"; + send({ id: message.id, result: { thread: buildThread(resumed), model: reported, modelProvider: "openai", serviceTier: null, cwd: resumed.cwd, approvalPolicy: "never", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: null } }); + break; + } if (requiresExperimental("persistExtendedHistory", message, state) || requiresExperimental("persistFullHistory", message, state)) { throw new Error("thread/resume.persistFullHistory requires experimentalApi capability"); } @@ -356,7 +362,7 @@ rl.on("line", (line) => { send({ id: message.id, error: { code: -32601, message: "Unsupported method: model/list" } }); break; } - if (BEHAVIOR === "model-list-slow-error") { + if (BEHAVIOR === "model-list-slow-error" || BEHAVIOR === "model-list-slow-error-during-request") { setTimeout(() => { send({ id: message.id, error: { code: -32000, message: "model catalog backend timed out" } }); }, 3500); @@ -476,6 +482,22 @@ rl.on("line", (line) => { prompt }; saveState(state); + if (BEHAVIOR === "model-list-slow-error-during-request") { + // Keep turn/start pending long enough for the delayed model/list error to land + // mid-flight, then flush the response and every turn event as ONE stdout write + // (one chunk), so the notifications are processed before the microtask that + // resumes the broker's awaited turn/start request. + setTimeout(() => { + const burst = [ + { id: message.id, result: { turn: buildTurn(turnId) } }, + { method: "turn/started", params: { threadId: thread.id, turn: buildTurn(turnId) } }, + { method: "item/completed", params: { threadId: thread.id, turnId, item: { type: "agentMessage", id: "msg_" + turnId, text: taskPayload(prompt, false), phase: "final_answer" } } }, + { method: "turn/completed", params: { threadId: thread.id, turn: buildTurn(turnId, "completed") } } + ]; + process.stdout.write(burst.map((entry) => JSON.stringify(entry)).join("\\n") + "\\n"); + }, 800); + break; + } send({ id: message.id, result: { turn: buildTurn(turnId) } }); const payload = message.params.outputSchema && message.params.outputSchema.properties && message.params.outputSchema.properties.verdict From b8134d7a4fa09fac48f1e1279a76974193246e7b Mon Sep 17 00:00:00 2001 From: caleb wei Date: Wed, 8 Jul 2026 13:32:26 -0700 Subject: [PATCH 7/7] fix: fetch the full model catalog (includeHidden + pagination) for effort validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up, verified against the live app-server first: default model/list returns only picker-visible models (6 of 7 on codex-cli 0.142.4 — the hidden codex-auto-review entry is absent) and the response carries nextCursor. A hidden or later-page model passed via --model therefore dodged validation as 'not in the model catalog' and the unsupported pair was dispatched anyway. fetchModelCatalog now requests includeHidden: true and follows nextCursor up to 5 pages under the SAME single 3s deadline. A partial catalog can only produce a fail-open skip for models on unfetched pages — never a false rejection. Live probe also confirms today's server returns everything in one page (nextCursor: null), so the loop is a no-op there. Regression test: a paginated fixture catalog puts gpt-5.4-mini on page two; the unsupported pair is now rejected pre-dispatch. Red on the previous commit, green now. Suite 103/103. Co-Authored-By: Claude Fable 5 --- plugins/codex/scripts/lib/codex.mjs | 53 +++++++++++++++++++++-------- tests/fake-codex-fixture.mjs | 13 +++++++ tests/runtime.test.mjs | 26 ++++++++++++++ 3 files changed, 77 insertions(+), 15 deletions(-) diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index 7102f8de..d5e64961 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -1093,6 +1093,7 @@ export async function importExternalAgentSession(cwd, options = {}) { } const MODEL_LIST_TIMEOUT_MS = 3000; +const MODEL_LIST_MAX_PAGES = 5; function emitEffortValidationSkip(onProgress, effort, reason) { emitProgress( @@ -1103,22 +1104,44 @@ function emitEffortValidationSkip(onProgress, effort, reason) { } async function fetchModelCatalog(client) { - const requestPromise = client.request("model/list", {}); - requestPromise.catch(() => {}); - let timer; - try { - const response = await Promise.race([ - requestPromise, - new Promise((resolve) => { - timer = setTimeout(() => resolve(null), MODEL_LIST_TIMEOUT_MS); - }) - ]); - return Array.isArray(response?.data) ? response.data : null; - } catch { - return null; - } finally { - clearTimeout(timer); + // includeHidden: the default model/list only returns picker-visible models, so a + // hidden model passed via --model would otherwise dodge validation. The catalog is + // paginated; follow nextCursor under ONE shared deadline. A partial catalog can only + // cause a fail-open skip for models on unfetched pages — never a false rejection. + const deadline = Date.now() + MODEL_LIST_TIMEOUT_MS; + const entries = []; + let cursor = null; + for (let page = 0; page < MODEL_LIST_MAX_PAGES; page += 1) { + const remaining = deadline - Date.now(); + if (remaining <= 0) { + break; + } + const requestPromise = client.request("model/list", cursor ? { includeHidden: true, cursor } : { includeHidden: true }); + requestPromise.catch(() => {}); + let timer; + let response = null; + try { + response = await Promise.race([ + requestPromise, + new Promise((resolve) => { + timer = setTimeout(() => resolve(null), remaining); + }) + ]); + } catch { + response = null; + } finally { + clearTimeout(timer); + } + if (!response || !Array.isArray(response.data)) { + break; + } + entries.push(...response.data); + cursor = response.nextCursor ?? null; + if (!cursor) { + break; + } } + return entries.length > 0 ? entries : null; } function assertCatalogSupportsEffort(entries, resolvedModel, effort, onProgress) { diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index 19525492..9a5c403e 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -372,6 +372,19 @@ rl.on("line", (line) => { send({ id: message.id, result: { data: "not-a-catalog" } }); break; } + if (BEHAVIOR === "model-list-paginated") { + const pageEffort = (reasoningEffort) => ({ reasoningEffort, description: reasoningEffort }); + if (message.params && message.params.cursor === "page-2") { + send({ id: message.id, result: { data: [ + { id: "gpt-5.4-mini", model: "gpt-5.4-mini", hidden: true, isDefault: false, defaultReasoningEffort: "medium", supportedReasoningEfforts: ["low", "medium", "high", "xhigh"].map(pageEffort) } + ], nextCursor: null } }); + break; + } + send({ id: message.id, result: { data: [ + { id: "gpt-5.6-sol", model: "gpt-5.6-sol", isDefault: true, defaultReasoningEffort: "medium", supportedReasoningEfforts: ["low", "medium", "high", "xhigh", "max", "ultra"].map(pageEffort) } + ], nextCursor: "page-2" } }); + break; + } const catalogEffort = (reasoningEffort) => ({ reasoningEffort, description: reasoningEffort }); send({ id: message.id, result: { data: [ { id: "gpt-5.6-sol", model: "gpt-5.6-sol", isDefault: false, defaultReasoningEffort: "medium", supportedReasoningEfforts: ["low", "medium", "high", "xhigh", "max", "ultra"].map(catalogEffort) }, diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 549975b3..5a91c8a6 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -976,6 +976,32 @@ test("resume validates the requested model, not the model reported by thread/res assert.equal(fakeState.lastTurnStart.effort, "ultra"); }); +test("effort validation finds models on later catalog pages before dispatching", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "model-list-paginated"); + 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 }); + + // gpt-5.4-mini only appears on the second catalog page; skipping pagination would + // fail open and dispatch the unsupported pair instead of rejecting it. + const result = run("node", [SCRIPT, "task", "--model", "gpt-5.4-mini", "--effort", "ultra", "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Model "gpt-5\.4-mini" does not support reasoning effort "ultra"/); + if (fs.existsSync(statePath)) { + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart ?? null, null); + assert.equal((fakeState.threads ?? []).length, 0, "no thread may be created for a rejected pair"); + } +}); + test("a model/list settling during an in-flight request does not drop turn events or broker serialization", () => { const repo = makeTempDir(); const binDir = makeTempDir();