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 1/2] 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 6fc34438..38f76ef3 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 355e330a..e9100a38 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. From 44c343bbf38e649b05d14f49ef8e46c551412b72 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Fri, 26 Jun 2026 14:38:15 +0200 Subject: [PATCH 2/2] Handle uncataloged current models in config --- src/CodexAcpServer.ts | 15 ++++++++-- src/ModelConfigOption.ts | 19 ++++++++---- .../session-config-options.test.ts | 29 +++++++++++++++++++ 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 720ff95f..f5d628a0 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -694,6 +694,10 @@ export class CodexAcpServer { private applyModelChange(sessionState: SessionState, value: string): void { const model = sessionState.availableModels.find(m => m.id === value); if (!model) { + const currentModel = ModelId.fromString(sessionState.currentModelId).model; + if (value === currentModel) { + return; + } throw RequestError.invalidParams(); } const currentEffort = ModelId.fromString(sessionState.currentModelId).effort; @@ -763,12 +767,17 @@ export class CodexAcpServer { private createSessionConfigOptions(sessionState: SessionState): Array { const currentModelId = ModelId.fromString(sessionState.currentModelId); - return [ + const configOptions = [ sessionState.agentMode.toConfigOption(), createModelConfigOption(sessionState.availableModels, currentModelId.model), - createReasoningEffortConfigOption(sessionState.supportedReasoningEfforts, currentModelId.effort), - createFastModeConfigOption(sessionState.fastModeEnabled), ]; + if (sessionState.supportedReasoningEfforts.length > 0) { + configOptions.push( + createReasoningEffortConfigOption(sessionState.supportedReasoningEfforts, currentModelId.effort), + ); + } + configOptions.push(createFastModeConfigOption(sessionState.fastModeEnabled)); + return configOptions; } private createSessionConfigOptionsResponse(sessionState: SessionState): { diff --git a/src/ModelConfigOption.ts b/src/ModelConfigOption.ts index 00bb5153..035b59fb 100644 --- a/src/ModelConfigOption.ts +++ b/src/ModelConfigOption.ts @@ -14,6 +14,19 @@ export function findSupportedEffort( } export function createModelConfigOption(availableModels: Array, currentBaseModelId: string): SessionConfigOption { + const options: Array<{ value: string; name: string; description: string | null }> = availableModels.map(model => ({ + value: model.id, + name: model.displayName, + description: model.description, + })); + if (!availableModels.some(model => model.id === currentBaseModelId)) { + options.unshift({ + value: currentBaseModelId, + name: currentBaseModelId, + description: null, + }); + } + return { id: MODEL_CONFIG_ID, name: "Model", @@ -21,11 +34,7 @@ export function createModelConfigOption(availableModels: Array, currentBa category: "model", type: "select", currentValue: currentBaseModelId, - options: availableModels.map(model => ({ - value: model.id, - name: model.displayName, - description: model.description, - })), + options, }; } diff --git a/src/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index bf8dc5cd..391cc043 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -90,6 +90,35 @@ describe("Session config options", () => { ); }); + it("shows the current uncataloged model as its own selectable option", async () => { + const {fast, slow} = buildModels(); + const {codexAcpAgent, response} = await createSession("custom-model[high]", [fast, slow]); + + const ids = response.configOptions?.map(o => o.id); + expect(ids).toEqual([MODE_CONFIG_ID, MODEL_CONFIG_ID, "fast-mode"]); + + const modelOption = response.configOptions?.find(o => o.id === MODEL_CONFIG_ID); + expect(modelOption).toMatchObject({ + category: "model", + currentValue: "custom-model", + type: "select", + options: [ + {value: "custom-model", name: "custom-model", description: null}, + {value: "fast-model", name: "Fast model", description: "Frontier"}, + {value: "slow-model", name: "Slow model", description: "Strong"}, + ], + }); + expect(response.configOptions?.some(o => o.id === REASONING_EFFORT_CONFIG_ID)).toBe(false); + + await codexAcpAgent.setSessionConfigOption({ + sessionId: "session-id", + configId: MODEL_CONFIG_ID, + value: "custom-model", + }); + + expect(codexAcpAgent.getSessionState("session-id").currentModelId).toBe("custom-model[high]"); + }); + it("keeps the legacy models list as combined model/effort entries", async () => { const {fast, slow} = buildModels(); const {response} = await createSession("fast-model[medium]", [fast, slow]);