diff --git a/docs/RELEASES.md b/docs/RELEASES.md index 0eef4ff5..ed898a90 100644 --- a/docs/RELEASES.md +++ b/docs/RELEASES.md @@ -41,7 +41,7 @@ Once the workflow finishes, confirm both outputs landed: ```sh gh release view "v" -npm view "@agentclientprotocol/codex-acp@" +npm view "$(node -p "require('./package.json').name")@" ``` ## How the version is chosen diff --git a/src/AcpExtensions.ts b/src/AcpExtensions.ts index b450c8bd..5f1b75a5 100644 --- a/src/AcpExtensions.ts +++ b/src/AcpExtensions.ts @@ -65,6 +65,7 @@ export type LegacyNewSessionResponse = NewSessionResponse & { } export type LegacyLoadSessionResponse = LoadSessionResponse & { + sessionId: SessionId; models?: LegacySessionModelState | null; } diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 7040057f..6f27cd93 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -535,7 +535,7 @@ export class CodexAcpClient { excludeTurns: true, config: await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? []), cwd: request.cwd, - modelProvider: await this.getResumeModelProvider(), + ...(await this.resumeModelProviderParams()), threadId: request.sessionId, }); onSubscribed?.(); @@ -559,7 +559,7 @@ export class CodexAcpClient { refreshSkills: (cwd, directories) => this.refreshSkills(cwd, directories), createSessionConfig: (cwd, directories, mcpServers) => this.createSessionConfig(cwd, directories, mcpServers), - getResumeModelProvider: () => this.getResumeModelProvider(), + getResumeModelProviderParams: () => this.resumeModelProviderParams(), fetchAvailableModels: () => this.fetchAvailableModels(), createCurrentModelId: (models, model, reasoningEffort) => this.createModelId(models, model, reasoningEffort).toString(), @@ -575,7 +575,7 @@ export class CodexAcpClient { excludeTurns: true, config: await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? []), cwd: request.cwd, - modelProvider: await this.getResumeModelProvider(), + ...(await this.resumeModelProviderParams()), threadId: request.sessionId, }); onSubscribed?.(); @@ -797,10 +797,17 @@ export class CodexAcpClient { return this.gatewayConfig?.modelProvider ?? this.modelProvider; } - private async getResumeModelProvider(): Promise { - // Prefer an explicit/gateway provider, then the provider persisted in Codex config. - // Keep OpenAI as the final fallback for ChatGPT-authenticated sessions without a configured provider. - return (await this.getCurrentModelProvider()) ?? "openai"; + /** + * Resume-time provider override, as `thread/resume` params. + * + * Prefer an explicit/gateway provider, then the provider persisted in Codex config. + * When neither is configured the field is omitted entirely: supplying one makes the + * app-server re-resolve the thread's model and reasoning effort from config, which + * discards the picks stored on the thread itself. + */ + private async resumeModelProviderParams(): Promise<{modelProvider?: string}> { + const modelProvider = await this.getCurrentModelProvider(); + return modelProvider ? {modelProvider} : {}; } private async refreshSkills( @@ -1083,6 +1090,20 @@ export class CodexAcpClient { }); } + async setModelAndEffort( + sessionId: string, + currentModelId: string, + collaborationMode: ModeKind, + ): Promise { + const modelId = ModelId.fromString(currentModelId); + await this.codexClient.threadSettingsUpdate({ + threadId: sessionId, + model: modelId.model, + effort: modelId.effort as ReasoningEffort, + collaborationMode: createCodexCollaborationMode(collaborationMode, currentModelId), + }); + } + private getCollaborationMode(sessionId: string): ModeKind { return this.codexClient.getThreadSettings(sessionId)?.collaborationMode.mode ?? "default"; } diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 4dc15e01..c8ae9e5b 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -280,6 +280,7 @@ export class CodexAcpServer { private readonly sessionGenerations: Map; private readonly sessionOpenGenerations: Map; private readonly goalControlGenerations: Map; + private readonly sessionConfigUpdates: Map>; private readonly permissionLifecycleContexts: WeakMap; private readonly codexProcessState: CodexProcessState | null; private codexProcessGeneration = 0; @@ -303,6 +304,7 @@ export class CodexAcpServer { this.sessionGenerations = new Map(); this.sessionOpenGenerations = new Map(); this.goalControlGenerations = new Map(); + this.sessionConfigUpdates = new Map(); this.permissionLifecycleContexts = new WeakMap(); this.connection = connection; this.codexAcpClient = codexAcpClient; @@ -795,6 +797,7 @@ export class CodexAcpServer { availableModelCount: modelState.availableModels.length }); return { + sessionId, models: modelState, modes: modeState, ...this.createSessionConfigOptionsResponse(this.getSessionState(sessionId)), @@ -1327,7 +1330,10 @@ export class CodexAcpServer { const sessionState = this.sessions.get(params.sessionId); if (!sessionState) throw new Error(`Session ${params.sessionId} not found`); - await this.applySessionConfigOption(sessionState, params); + await this.runSessionConfigUpdate( + params.sessionId, + () => this.applySessionConfigOption(sessionState, params), + ); return { configOptions: this.createSessionConfigOptions(sessionState), @@ -1346,16 +1352,29 @@ export class CodexAcpServer { await this.applyCollaborationModeChange(sessionState, this.stringConfigValue(params)); break; case MODEL_CONFIG_ID: - this.applyModelChange(sessionState, this.stringConfigValue(params)); + await this.applyModelChange(sessionState, this.stringConfigValue(params)); break; case REASONING_EFFORT_CONFIG_ID: - this.applyReasoningEffortChange(sessionState, this.stringConfigValue(params)); + await this.applyReasoningEffortChange(sessionState, this.stringConfigValue(params)); break; default: throw RequestError.invalidParams(); } } + private async runSessionConfigUpdate(sessionId: string, operation: () => Promise): Promise { + const previous = this.sessionConfigUpdates.get(sessionId); + const update = previous ? previous.then(operation, operation) : operation(); + this.sessionConfigUpdates.set(sessionId, update); + try { + return await update; + } finally { + if (this.sessionConfigUpdates.get(sessionId) === update) { + this.sessionConfigUpdates.delete(sessionId); + } + } + } + private applyFastModeChange(sessionState: SessionState, params: acp.SetSessionConfigOptionRequest): void { const value = params.value; if (typeof value === "boolean") { @@ -1392,7 +1411,7 @@ export class CodexAcpServer { sessionState.collaborationMode = mode; } - private applyModelChange(sessionState: SessionState, value: string): void { + private async applyModelChange(sessionState: SessionState, value: string): Promise { const model = sessionState.availableModels.find(m => m.id === value); if (!model) { const currentModel = ModelId.fromString(sessionState.currentModelId).model; @@ -1404,16 +1423,27 @@ export class CodexAcpServer { const currentEffort = ModelId.fromString(sessionState.currentModelId).effort; const effort = findSupportedEffort(model.supportedReasoningEfforts, currentEffort) ?? model.defaultReasoningEffort; + await this.codexAcpClient.setModelAndEffort( + sessionState.sessionId, + ModelId.fromComponents(model, effort).toString(), + sessionState.collaborationMode, + ); this.applyModelAndEffort(sessionState, model, effort); } - private applyReasoningEffortChange(sessionState: SessionState, value: string): void { + private async applyReasoningEffortChange(sessionState: SessionState, value: string): Promise { const effort = findSupportedEffort(sessionState.supportedReasoningEfforts, value); if (!effort) { throw RequestError.invalidParams(); } const {model} = ModelId.fromString(sessionState.currentModelId); - sessionState.currentModelId = ModelId.create(model, effort).toString(); + const currentModelId = ModelId.create(model, effort).toString(); + await this.codexAcpClient.setModelAndEffort( + sessionState.sessionId, + currentModelId, + sessionState.collaborationMode, + ); + sessionState.currentModelId = currentModelId; } private applyModelAndEffort(sessionState: SessionState, model: Model, effort: ReasoningEffort): void { @@ -1424,34 +1454,41 @@ export class CodexAcpServer { } async unstable_setSessionModel(params: LegacySetSessionModelRequest): Promise { - logger.log("Set session model requested", { - sessionId: params.sessionId, - modelId: params.modelId - }); - const sessionState = this.sessions.get(params.sessionId); - if (!sessionState) throw new Error(`Session ${params.sessionId} not found`); + return await this.runSessionConfigUpdate(params.sessionId, async () => { + logger.log("Set session model requested", { + sessionId: params.sessionId, + modelId: params.modelId + }); + const sessionState = this.sessions.get(params.sessionId); + if (!sessionState) throw new Error(`Session ${params.sessionId} not found`); - const {model: requestedModelName, effort: requestedEffort} = ModelId.fromString(params.modelId); + const {model: requestedModelName, effort: requestedEffort} = ModelId.fromString(params.modelId); - const models = await this.codexAcpClient.fetchAvailableModels(); - const model = models.find(m => m.id === requestedModelName); - if (!model) throw new Error(`Unknown model ${params.modelId}`); + const models = await this.codexAcpClient.fetchAvailableModels(); + const model = models.find(m => m.id === requestedModelName); + if (!model) throw new Error(`Unknown model ${params.modelId}`); - let reasoningEffort: ReasoningEffort; - if (requestedEffort) { - const matchedEffort = findSupportedEffort(model.supportedReasoningEfforts, requestedEffort); - if (!matchedEffort) { - throw new Error(`Unsupported reasoning effort ${requestedEffort} for model ${requestedModelName}`); + let reasoningEffort: ReasoningEffort; + if (requestedEffort) { + const matchedEffort = findSupportedEffort(model.supportedReasoningEfforts, requestedEffort); + if (!matchedEffort) { + throw new Error(`Unsupported reasoning effort ${requestedEffort} for model ${requestedModelName}`); + } + reasoningEffort = matchedEffort; + } else { + reasoningEffort = model.defaultReasoningEffort; } - reasoningEffort = matchedEffort; - } else { - reasoningEffort = model.defaultReasoningEffort; - } - sessionState.availableModels = models; - this.applyModelAndEffort(sessionState, model, reasoningEffort); + await this.codexAcpClient.setModelAndEffort( + sessionState.sessionId, + ModelId.fromComponents(model, reasoningEffort).toString(), + sessionState.collaborationMode, + ); + sessionState.availableModels = models; + this.applyModelAndEffort(sessionState, model, reasoningEffort); - return {}; + return {}; + }); } private parseLegacySetSessionModelParams(params: Record): LegacySetSessionModelRequest { @@ -2739,6 +2776,18 @@ export class CodexAcpServer { if (this.providerUpdate !== null) { await this.providerUpdate; } + const pendingConfigUpdate = this.sessionConfigUpdates.get(params.sessionId); + if (pendingConfigUpdate !== undefined) { + try { + await pendingConfigUpdate; + } catch (error) { + logger.error(`Pending session configuration update failed for ${params.sessionId}`, error); + throw RequestError.invalidRequest( + undefined, + "Prompt blocked because a pending session configuration update failed", + ); + } + } logger.log("Prompt received", { sessionId: params.sessionId, prompt: params.prompt, @@ -2853,11 +2902,14 @@ export class CodexAcpServer { onTurnStarted?.(); }, setConfigOption: async (configId, value) => { - await this.applySessionConfigOption(sessionState, { - sessionId: sessionState.sessionId, - configId, - value, - }); + await this.runSessionConfigUpdate( + sessionState.sessionId, + () => this.applySessionConfigOption(sessionState, { + sessionId: sessionState.sessionId, + configId, + value, + }), + ); const session = new ACPSessionConnection(this.connection, sessionState.sessionId); await session.update({ sessionUpdate: "config_option_update", @@ -3033,7 +3085,10 @@ export class CodexAcpServer { return cancelledPromptResponse(); } if (approved && !this.promptShouldStop(params.sessionId, activePrompt)) { - await this.applyCollaborationModeChange(sessionState, DEFAULT_COLLABORATION_MODE); + await this.runSessionConfigUpdate( + sessionState.sessionId, + () => this.applyCollaborationModeChange(sessionState, DEFAULT_COLLABORATION_MODE), + ); const session = new ACPSessionConnection(this.connection, sessionState.sessionId); await session.update({ sessionUpdate: "config_option_update", diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index daa7e875..c5f53ded 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -3,6 +3,7 @@ import type { ClientRequest, InitializeParams, InitializeResponse, + ReasoningEffort, ServerNotification } from "./app-server"; import type { @@ -562,7 +563,7 @@ export class CodexAppServerClient { return this.threadSettings.get(threadId); } - async threadSettingsUpdate(params: ExperimentalThreadSettingsUpdateParams): Promise { + async threadSettingsUpdate(params: ThreadSettingsUpdateParams): Promise { await this.connection.sendRequest("thread/settings/update", params); } @@ -1085,9 +1086,11 @@ type DistributiveOmit = T extends any ? Omit : never; -export interface ExperimentalThreadSettingsUpdateParams { +export interface ThreadSettingsUpdateParams { threadId: string; - collaborationMode: { + model?: string; + effort?: ReasoningEffort; + collaborationMode?: { mode: "default" | "plan"; settings: { model: string; diff --git a/src/SessionFork.ts b/src/SessionFork.ts index 1a533df7..85427dc1 100644 --- a/src/SessionFork.ts +++ b/src/SessionFork.ts @@ -15,7 +15,7 @@ export type SessionForkDependencies = { additionalDirectories: string[], mcpServers: acp.McpServer[], ): Promise>; - getResumeModelProvider(): Promise; + getResumeModelProviderParams(): Promise<{modelProvider?: string}>; fetchAvailableModels(): Promise; createCurrentModelId(models: Model[], model: string, reasoningEffort: string | null): string; getCollaborationMode(sessionId: string): ModeKind; @@ -37,7 +37,7 @@ export async function forkSession( ), cwd: request.cwd, ...(lastTurnId !== undefined && {lastTurnId}), - modelProvider: await dependencies.getResumeModelProvider(), + ...(await dependencies.getResumeModelProviderParams()), threadId: request.sessionId, }); await dependencies.codexClient.threadUnsubscribe({threadId: response.thread.id}); diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index fe51f160..176d7955 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -601,6 +601,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(forked.sessionId).toBe("fork-id"); expect(forked.additionalDirectories).toEqual(["/workspace/extra"]); + expect(threadForkSpy.mock.calls[0]![0]).not.toHaveProperty("modelProvider"); expect(threadForkSpy).toHaveBeenCalledWith(expect.objectContaining({ excludeTurns: true, threadId: "source-id", @@ -615,6 +616,40 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(threadUnsubscribeSpy).toHaveBeenCalledWith({threadId: "fork-id"}); }); + it('uses the configured model provider when forking a session', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAcpClient = mockFixture.getCodexAcpClient(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + + vi.spyOn(codexAppServerClient, "skillsExtraRootsSet").mockResolvedValue(undefined); + vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({data: []}); + vi.spyOn(codexAppServerClient, "configRead").mockResolvedValue({ + config: {model_provider: "azure"}, + } as any); + const threadForkSpy = vi.spyOn(codexAppServerClient, "threadFork").mockResolvedValue({ + thread: {id: "fork-id"}, + model: "gpt-5", + modelProvider: "azure", + reasoningEffort: "medium", + serviceTier: null, + } as any); + vi.spyOn(codexAppServerClient, "threadUnsubscribe").mockResolvedValue({status: "unsubscribed"}); + vi.spyOn(codexAppServerClient, "listModels").mockResolvedValue({ + data: [createTestModel({id: "gpt-5"})], + nextCursor: null, + }); + + await codexAcpClient.forkSession({ + sessionId: "source-id", + cwd: "/workspace", + }); + + expect(threadForkSpy).toHaveBeenCalledWith(expect.objectContaining({ + threadId: "source-id", + modelProvider: "azure", + })); + }); + it('maps an AIR fork message id to the containing Codex turn', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpClient = mockFixture.getCodexAcpClient(); @@ -804,6 +839,47 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(threadResumeSpy.mock.calls[1]![0].modelProvider).toBe("azure"); }); + it('omits the model provider when none is configured so the thread keeps its model and effort', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAcpClient = mockFixture.getCodexAcpClient(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + + vi.spyOn(codexAcpClient, "getModelProvider").mockReturnValue(null); + vi.spyOn(codexAppServerClient, "skillsExtraRootsSet").mockResolvedValue(undefined); + vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({data: []}); + vi.spyOn(codexAppServerClient, "configRead").mockResolvedValue({config: {}} as any); + const threadResumeSpy = vi.spyOn(codexAppServerClient, "threadResume").mockResolvedValue({ + thread: {id: "thread-id"} as any, + model: "gpt-5", + reasoningEffort: "high", + serviceTier: null, + } as any); + vi.spyOn(codexAppServerClient, "threadReadWithHistory").mockResolvedValue({ + thread: {id: "thread-id"} as any, + }); + vi.spyOn(codexAppServerClient, "listModels").mockResolvedValue({ + data: [createTestModel({id: "gpt-5", defaultReasoningEffort: "medium"})], + nextCursor: null, + }); + + const resumed = await codexAcpClient.resumeSession({ + sessionId: "resume-id", + cwd: "/workspace", + }); + const loaded = await codexAcpClient.loadSession({ + sessionId: "load-id", + cwd: "/workspace", + mcpServers: [], + }); + + // Supplying a provider makes the app-server re-resolve model/effort from config, + // discarding the picks stored on the thread (issue #343). + expect(threadResumeSpy.mock.calls[0]![0]).not.toHaveProperty("modelProvider"); + expect(threadResumeSpy.mock.calls[1]![0]).not.toHaveProperty("modelProvider"); + expect(resumed.currentModelId).toBe("gpt-5[high]"); + expect(loaded.currentModelId).toBe("gpt-5[high]"); + }); + it('tracks configured model provider auth state for resumed and loaded sessions', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpAgent = mockFixture.getCodexAcpAgent(); diff --git a/src/__tests__/CodexACPAgent/fast-mode-config.test.ts b/src/__tests__/CodexACPAgent/fast-mode-config.test.ts index 6007d529..11fd8ced 100644 --- a/src/__tests__/CodexACPAgent/fast-mode-config.test.ts +++ b/src/__tests__/CodexACPAgent/fast-mode-config.test.ts @@ -47,6 +47,7 @@ describe("Fast mode session config", () => { currentServiceTier, additionalDirectories: [], }); + vi.spyOn((codexAcpClient as any).codexClient, "threadSettingsUpdate").mockResolvedValue(undefined); await codexAcpAgent.initialize({ protocolVersion: acp.PROTOCOL_VERSION, diff --git a/src/__tests__/CodexACPAgent/load-session.test.ts b/src/__tests__/CodexACPAgent/load-session.test.ts index 05440d65..eec0b394 100644 --- a/src/__tests__/CodexACPAgent/load-session.test.ts +++ b/src/__tests__/CodexACPAgent/load-session.test.ts @@ -413,8 +413,9 @@ describe("CodexACPAgent - loadSession", () => { cwd: "/test/project", mcpServers: [], }; - await codexAcpAgent.loadSession(loadParams); + const response = await codexAcpAgent.loadSession(loadParams); + expect(response.sessionId).toBe(thread.id); expect(codexAppServerClient.threadReadWithHistory).toHaveBeenCalledWith(thread.id); expect(codexAppServerClient.threadGoalGet).toHaveBeenCalledWith({ threadId: thread.id }); await expect(fixture.getAcpConnectionDump([])).toMatchFileSnapshot( diff --git a/src/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index e895d282..8e05ac12 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -1,5 +1,8 @@ import {describe, expect, it, vi} from "vitest"; -import {createCodexMockTestFixture, createTestModel} from "../acp-test-utils"; +import { + createCodexMockTestFixture, + createTestModel, +} from "../acp-test-utils"; import {AgentMode, MODE_CONFIG_ID} from "../../AgentMode"; import { MODEL_CONFIG_ID, @@ -49,9 +52,11 @@ async function createSession(currentModelId: string, availableModels: Array { @@ -173,8 +178,7 @@ describe("Session config options", () => { it("changes collaboration mode without starting a model turn", async () => { const {fast} = buildModels(); - const {codexAcpAgent, codexAcpClient} = await createSession("fast-model[medium]", [fast]); - const update = vi.spyOn((codexAcpClient as any).codexClient, "threadSettingsUpdate").mockResolvedValue(undefined); + const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); const result = await codexAcpAgent.setSessionConfigOption({ sessionId: "session-id", @@ -192,8 +196,7 @@ describe("Session config options", () => { it("toggles collaboration mode with /plan without starting a model turn", async () => { const {fast} = buildModels(); - const {fixture, codexAcpAgent, codexAcpClient} = await createSession("fast-model[medium]", [fast]); - const update = vi.spyOn((codexAcpClient as any).codexClient, "threadSettingsUpdate").mockResolvedValue(undefined); + const {fixture, codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); const turnStart = vi.spyOn(fixture.getCodexAppServerClient(), "turnStart"); const enabledResponse = await codexAcpAgent.prompt({ @@ -245,9 +248,54 @@ describe("Session config options", () => { })); }); + it("serializes a model change behind an in-flight /plan mode change", async () => { + const {fast, slow} = buildModels(); + const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast, slow]); + let releasePlan!: () => void; + update + .mockImplementationOnce(() => new Promise(resolve => { + releasePlan = resolve; + })) + .mockResolvedValueOnce(undefined); + + const planPromise = codexAcpAgent.prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "/plan"}], + }); + await vi.waitFor(() => expect(update).toHaveBeenCalledTimes(1)); + const modelPromise = codexAcpAgent.setSessionConfigOption({ + sessionId: "session-id", + configId: MODEL_CONFIG_ID, + value: "slow-model", + }); + + await Promise.resolve(); + expect(update).toHaveBeenCalledTimes(1); + releasePlan(); + await planPromise; + await modelPromise; + + expect(update).toHaveBeenCalledTimes(2); + expect(update.mock.calls[1]![0]).toMatchObject({ + model: "slow-model", + effort: "medium", + collaborationMode: { + mode: "plan", + settings: { + model: "slow-model", + reasoning_effort: "medium", + }, + }, + }); + expect(codexAcpAgent.getSessionState("session-id")).toMatchObject({ + currentModelId: "slow-model[medium]", + collaborationMode: "plan", + }); + }); + it("changes the model and keeps the current reasoning effort when supported", async () => { const {fast, slow} = buildModels(); - const {codexAcpAgent} = await createSession("fast-model[medium]", [fast, slow]); + const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast, slow]); await codexAcpAgent.setSessionConfigOption({ sessionId: "session-id", @@ -256,6 +304,19 @@ describe("Session config options", () => { }); expect(codexAcpAgent.getSessionState("session-id").currentModelId).toBe("slow-model[medium]"); + expect(update).toHaveBeenCalledWith({ + threadId: "session-id", + model: "slow-model", + effort: "medium", + collaborationMode: { + mode: "default", + settings: { + model: "slow-model", + reasoning_effort: "medium", + developer_instructions: null, + }, + }, + }); }); it("falls back to the new model's default effort when the current effort is unsupported", async () => { @@ -271,9 +332,41 @@ describe("Session config options", () => { expect(codexAcpAgent.getSessionState("session-id").currentModelId).toBe("slow-model[low]"); }); + it("refreshes the collaboration-mode model snapshot when the model changes", async () => { + const {fast, slow} = buildModels(); + const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast, slow]); + + await codexAcpAgent.setSessionConfigOption({ + sessionId: "session-id", + configId: COLLABORATION_MODE_CONFIG_ID, + value: PLAN_COLLABORATION_MODE, + }); + update.mockClear(); + + await codexAcpAgent.setSessionConfigOption({ + sessionId: "session-id", + configId: MODEL_CONFIG_ID, + value: "slow-model", + }); + + expect(update).toHaveBeenCalledWith(expect.objectContaining({ + threadId: "session-id", + model: "slow-model", + effort: "medium", + collaborationMode: { + mode: "plan", + settings: { + model: "slow-model", + reasoning_effort: "medium", + developer_instructions: null, + }, + }, + })); + }); + it("changes only the reasoning effort", async () => { const {fast} = buildModels(); - const {codexAcpAgent} = await createSession("fast-model[medium]", [fast]); + const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); await codexAcpAgent.setSessionConfigOption({ sessionId: "session-id", @@ -282,6 +375,19 @@ describe("Session config options", () => { }); expect(codexAcpAgent.getSessionState("session-id").currentModelId).toBe("fast-model[high]"); + expect(update).toHaveBeenCalledWith({ + threadId: "session-id", + model: "fast-model", + effort: "high", + collaborationMode: { + mode: "default", + settings: { + model: "fast-model", + reasoning_effort: "high", + developer_instructions: null, + }, + }, + }); }); it("refreshes the cached model list when unstable_setSessionModel picks a freshly fetched model", async () => { @@ -309,6 +415,7 @@ describe("Session config options", () => { defaultReasoningEffort: "medium", }); vi.spyOn(codexAcpClient, "fetchAvailableModels").mockResolvedValue([fast, extraModel]); + vi.spyOn((codexAcpClient as any).codexClient, "threadSettingsUpdate").mockResolvedValue(undefined); await codexAcpAgent.unstable_setSessionModel({ sessionId: "session-id", @@ -319,6 +426,147 @@ describe("Session config options", () => { expect(sessionState.availableModels.map(m => m.id)).toEqual(["fast-model", "extra-model"]); }); + it("keeps the previous cached model list when legacy model persistence fails", async () => { + const {fast} = buildModels(); + const {codexAcpAgent, codexAcpClient, update} = await createSession("fast-model[medium]", [fast]); + const extraModel = createTestModel({ + id: "extra-model", + supportedReasoningEfforts: [mediumEffort], + defaultReasoningEffort: "medium", + }); + vi.spyOn(codexAcpClient, "fetchAvailableModels").mockResolvedValue([fast, extraModel]); + update.mockRejectedValueOnce(new Error("settings update failed")); + + await expect(codexAcpAgent.unstable_setSessionModel({ + sessionId: "session-id", + modelId: "extra-model[medium]", + })).rejects.toThrow("settings update failed"); + + expect(codexAcpAgent.getSessionState("session-id").availableModels.map(m => m.id)).toEqual(["fast-model"]); + }); + + it("waits for an in-flight config update before starting a pipelined prompt", async () => { + const {fast, slow} = buildModels(); + const {codexAcpAgent, codexAcpClient, update} = await createSession("fast-model[medium]", [fast, slow]); + let releaseUpdate!: () => void; + update.mockImplementationOnce(() => new Promise(resolve => { + releaseUpdate = resolve; + })); + const sendPrompt = vi.spyOn(codexAcpClient, "sendPrompt").mockResolvedValue(null); + + const configPromise = codexAcpAgent.setSessionConfigOption({ + sessionId: "session-id", + configId: MODEL_CONFIG_ID, + value: "slow-model", + }); + await vi.waitFor(() => expect(update).toHaveBeenCalled()); + const promptPromise = codexAcpAgent.prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "use the configured model"}], + }); + + await Promise.resolve(); + expect(sendPrompt).not.toHaveBeenCalled(); + releaseUpdate(); + await configPromise; + await promptPromise; + + expect(sendPrompt.mock.calls[0]![2].toString()).toBe("slow-model[medium]"); + }); + + it("waits for an in-flight legacy model update before starting a pipelined prompt", async () => { + const {fast, slow} = buildModels(); + const {codexAcpAgent, codexAcpClient, update} = await createSession("fast-model[medium]", [fast]); + vi.spyOn(codexAcpClient, "fetchAvailableModels").mockResolvedValue([fast, slow]); + let releaseUpdate!: () => void; + update.mockImplementationOnce(() => new Promise(resolve => { + releaseUpdate = resolve; + })); + const sendPrompt = vi.spyOn(codexAcpClient, "sendPrompt").mockResolvedValue(null); + + const configPromise = codexAcpAgent.unstable_setSessionModel({ + sessionId: "session-id", + modelId: "slow-model[medium]", + }); + await vi.waitFor(() => expect(update).toHaveBeenCalled()); + const promptPromise = codexAcpAgent.prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "use the legacy-configured model"}], + }); + + await Promise.resolve(); + expect(sendPrompt).not.toHaveBeenCalled(); + releaseUpdate(); + await configPromise; + await promptPromise; + + expect(sendPrompt.mock.calls[0]![2].toString()).toBe("slow-model[medium]"); + }); + + it("serializes config updates and continues after an earlier update fails", async () => { + const {fast, slow} = buildModels(); + const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast, slow]); + let rejectFirst!: (error: Error) => void; + update + .mockImplementationOnce(() => new Promise((_resolve, reject) => { + rejectFirst = reject; + })) + .mockResolvedValueOnce(undefined); + + const first = codexAcpAgent.setSessionConfigOption({ + sessionId: "session-id", + configId: REASONING_EFFORT_CONFIG_ID, + value: "high", + }); + await vi.waitFor(() => expect(update).toHaveBeenCalledTimes(1)); + const second = codexAcpAgent.setSessionConfigOption({ + sessionId: "session-id", + configId: MODEL_CONFIG_ID, + value: "slow-model", + }); + + await Promise.resolve(); + expect(update).toHaveBeenCalledTimes(1); + rejectFirst(new Error("first update failed")); + await expect(first).rejects.toThrow("first update failed"); + await expect(second).resolves.toBeDefined(); + + expect(update).toHaveBeenCalledTimes(2); + expect(update.mock.calls[1]![0]).toMatchObject({ + model: "slow-model", + effort: "medium", + }); + expect(codexAcpAgent.getSessionState("session-id").currentModelId).toBe("slow-model[medium]"); + }); + + it("fails a pipelined prompt closed with a configuration-specific error", async () => { + const {fast} = buildModels(); + const {codexAcpAgent, codexAcpClient, update} = await createSession("fast-model[medium]", [fast]); + let rejectUpdate!: (error: Error) => void; + update.mockImplementationOnce(() => new Promise((_resolve, reject) => { + rejectUpdate = reject; + })); + const sendPrompt = vi.spyOn(codexAcpClient, "sendPrompt").mockResolvedValue(null); + + const configPromise = codexAcpAgent.setSessionConfigOption({ + sessionId: "session-id", + configId: REASONING_EFFORT_CONFIG_ID, + value: "high", + }); + await vi.waitFor(() => expect(update).toHaveBeenCalled()); + const promptPromise = codexAcpAgent.prompt({ + sessionId: "session-id", + prompt: [{type: "text", text: "do not run on stale configuration"}], + }); + + rejectUpdate(new Error("settings update failed")); + await expect(configPromise).rejects.toThrow("settings update failed"); + await expect(promptPromise).rejects.toThrow( + "Prompt blocked because a pending session configuration update failed", + ); + expect(sendPrompt).not.toHaveBeenCalled(); + }); + it("changes the model through the legacy session/set_model extMethod", async () => { const {fast, slow} = buildModels(); const {codexAcpAgent, codexAcpClient} = await createSession("fast-model[medium]", [fast]);