Skip to content

Commit c5b32b5

Browse files
committed
fix: keep configured model id for custom providers instead of forcing the default
createModelId() looked up the configured/returned model id in Codex's listModels() catalog and, when it was absent, substituted availableModels.find(m => m.isDefault). Custom providers (self-hosted or third-party endpoints) expose model ids that the catalog does not enumerate, so this silently replaced the user's configured model with the built-in default. That default id was then pinned onto every runTurn, making requests to the custom endpoint fail with "unknown model '<default>'". The Codex CLI handles the same case by keeping the configured model id and warning "Model metadata not found. Defaulting to fallback metadata." This change mirrors that behavior: keep the requested model id when it is not in the catalog, and only fall back to isDefault when no model id was provided at all. Downstream session state already degrades gracefully for unknown models (supportedReasoningEfforts ?? [], inputModalities ?? ["text","image"]). Adds unit tests covering an uncatalogued model id with and without an explicit reasoning effort.
1 parent 5a0a2c2 commit c5b32b5

2 files changed

Lines changed: 30 additions & 5 deletions

File tree

src/CodexAcpClient.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -376,15 +376,30 @@ export class CodexAcpClient {
376376
* Falls back to model defaults if parameters are missing or unsupported.
377377
*/
378378
createModelId(availableModels: Model[], modelId: string | null, reasoningEffort: ReasoningEffort | null): ModelId {
379-
const selectedModel =
380-
availableModels.find(m => m.id === modelId) ??
381-
availableModels.find(m => m.isDefault);
379+
const selectedModel = availableModels.find(m => m.id === modelId);
380+
if (selectedModel) {
381+
return ModelId.create(selectedModel.id, reasoningEffort ?? selectedModel.defaultReasoningEffort);
382+
}
383+
384+
// The configured model is not in Codex's advertised catalog. This is
385+
// expected for custom providers (e.g. a self-hosted or third-party
386+
// model), whose model ids the catalog does not enumerate. Keep the
387+
// requested model id instead of silently substituting the built-in
388+
// default. This mirrors the Codex CLI, which keeps the configured model
389+
// and merely warns "Model metadata not found. Defaulting to fallback
390+
// metadata." Substituting the default here pins a wrong model id onto
391+
// every turn and makes requests to custom-provider endpoints fail with
392+
// "unknown model".
393+
if (modelId) {
394+
return ModelId.create(modelId, reasoningEffort ?? "medium");
395+
}
382396

383-
if (!selectedModel) {
397+
const defaultModel = availableModels.find(m => m.isDefault);
398+
if (!defaultModel) {
384399
throw new Error(`Model selection failed: No model found for ID "${modelId}" and no default model is defined.`);
385400
}
386401

387-
return ModelId.create(selectedModel.id, reasoningEffort ?? selectedModel.defaultReasoningEffort);
402+
return ModelId.create(defaultModel.id, reasoningEffort ?? defaultModel.defaultReasoningEffort);
388403
}
389404

390405
async subscribeToSessionEvents(

src/__tests__/CodexACPAgent/CodexAcpClient.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,16 @@ describe('ACP server test', { timeout: 40_000 }, () => {
966966
expect(result).toEqual(ModelId.create('5.2-codex', 'medium'));
967967
});
968968

969+
it('should keep a model id that is not in the advertised catalog (custom provider)', () => {
970+
const result = fixture.getCodexAcpClient().createModelId(mockModels, 'MiniMax-M3', 'high');
971+
expect(result).toEqual(ModelId.create('MiniMax-M3', 'high'));
972+
});
973+
974+
it('should default the effort for an uncatalogued model when reasoningEffort is null', () => {
975+
const result = fixture.getCodexAcpClient().createModelId(mockModels, 'MiniMax-M3', null);
976+
expect(result).toEqual(ModelId.create('MiniMax-M3', 'medium'));
977+
});
978+
969979
/**
970980
* Sets up a mock fixture with turnStart/awaitTurnCompleted spied on,
971981
* and a given session state. Returns the fixture and turnStart spy.

0 commit comments

Comments
 (0)