From c5b32b554cd2dde0acf907c444649c4605bfdfa4 Mon Sep 17 00:00:00 2001 From: scottzx <139930214@qq.com> Date: Mon, 15 Jun 2026 01:49:05 +0800 Subject: [PATCH] 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 ''". 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. --- src/CodexAcpClient.ts | 25 +++++++++++++++---- .../CodexACPAgent/CodexAcpClient.test.ts | 10 ++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 6fc3443..38f76ef 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -376,15 +376,30 @@ export class CodexAcpClient { * Falls back to model defaults if parameters are missing or unsupported. */ createModelId(availableModels: Model[], modelId: string | null, reasoningEffort: ReasoningEffort | null): ModelId { - const selectedModel = - availableModels.find(m => m.id === modelId) ?? - availableModels.find(m => m.isDefault); + const selectedModel = availableModels.find(m => m.id === modelId); + if (selectedModel) { + return ModelId.create(selectedModel.id, reasoningEffort ?? selectedModel.defaultReasoningEffort); + } + + // The configured model is not in Codex's advertised catalog. This is + // expected for custom providers (e.g. a self-hosted or third-party + // model), whose model ids the catalog does not enumerate. Keep the + // requested model id instead of silently substituting the built-in + // default. This mirrors the Codex CLI, which keeps the configured model + // and merely warns "Model metadata not found. Defaulting to fallback + // metadata." Substituting the default here pins a wrong model id onto + // every turn and makes requests to custom-provider endpoints fail with + // "unknown model". + if (modelId) { + return ModelId.create(modelId, reasoningEffort ?? "medium"); + } - if (!selectedModel) { + const defaultModel = availableModels.find(m => m.isDefault); + if (!defaultModel) { throw new Error(`Model selection failed: No model found for ID "${modelId}" and no default model is defined.`); } - return ModelId.create(selectedModel.id, reasoningEffort ?? selectedModel.defaultReasoningEffort); + return ModelId.create(defaultModel.id, reasoningEffort ?? defaultModel.defaultReasoningEffort); } async subscribeToSessionEvents( diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 355e330..e9100a3 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -966,6 +966,16 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(result).toEqual(ModelId.create('5.2-codex', 'medium')); }); + it('should keep a model id that is not in the advertised catalog (custom provider)', () => { + const result = fixture.getCodexAcpClient().createModelId(mockModels, 'MiniMax-M3', 'high'); + expect(result).toEqual(ModelId.create('MiniMax-M3', 'high')); + }); + + it('should default the effort for an uncatalogued model when reasoningEffort is null', () => { + const result = fixture.getCodexAcpClient().createModelId(mockModels, 'MiniMax-M3', null); + expect(result).toEqual(ModelId.create('MiniMax-M3', 'medium')); + }); + /** * Sets up a mock fixture with turnStart/awaitTurnCompleted spied on, * and a given session state. Returns the fixture and turnStart spy.