fix: accept max and ultra reasoning efforts for task --effort #454
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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(", ")}.` | ||
| ); | ||
|
Comment on lines
+1170
to
+1173
There was a problem hiding this comment.
When Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Addressed in ef1d739, though not via either suggested option — both have a sharper edge than the mismatch they fix:
What changed: the values stay in the syntactic allowlist, the catalog check still applies to them (a rejected pair fails with that model's real tier list, which is strictly more actionable than the pre-PR dispatch-and-hang), and the runtime-skill doc now states that supported tiers vary by model and catalog-rejected pairs fail before dispatch. |
||
| } | ||
| } | ||
|
|
||
| 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); | ||
| } | ||
|
There was a problem hiding this comment.
For an unsupported model/effort pair, this validation runs after Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Confirmed — |
||
|
|
||
| const turnState = await captureTurn( | ||
| client, | ||
| threadId, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When validating an explicit
--model/--effortpair, this callsmodel/listwith default params. The Codex app-server docs showmodel/listreturns picker-visible models by default and says to setincludeHidden: truefor the full list; the response is also paginated withnextCursor(https://developers.openai.com/codex/app-server). For a user who passes a hidden or later-page model with an unsupported effort, we skip validation as “not in the model catalog” and still sendturn/start, which reintroduces the unsupported-pair hang this change is trying to prevent. Please request the full catalog and page until the requested model is found or the catalog is exhausted.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed against the live app-server before changing anything: default
model/listreturns 6 of 7 models (the hiddencodex-auto-reviewentry is absent),includeHidden: truereturns all 7, andnextCursorisnullin every case today (pageSizehints are ignored) — so the hidden-model hole is real now and pagination is future-proofing. Fixed in the latest commit:fetchModelCatalogsendsincludeHidden: trueand followsnextCursorup to 5 pages under the same single 3s deadline; a partial catalog can only fail open (skip + warn) for models on unfetched pages, never falsely reject. Regression test drives a paginated fixture catalog with the target model on page two — red on the parent commit, green now. Suite 103/103.