diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index ed8bdb1..a9d7f2b 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -441,15 +441,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); - - if (!selectedModel) { + 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"); + } + + 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/CodexAcpServer.ts b/src/CodexAcpServer.ts index 720ff95..f5d628a 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 00bb515..035b59f 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/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 54436c6..a1dea3a 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -1824,6 +1824,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. diff --git a/src/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index bf8dc5c..391cc04 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]);