From ce9cc979116de341b980c24c62cbd7e13319af3d Mon Sep 17 00:00:00 2001 From: Gilbert Leung Date: Sun, 26 Jul 2026 14:47:36 +0800 Subject: [PATCH 01/13] fix: persist session config in Codex threads (cherry picked from commit a943bcef232d68b2e05705411bd7575a4d2026c6) --- src/AgentMode.ts | 12 +++++ src/CodexAcpClient.ts | 23 ++++++++ src/CodexAcpServer.ts | 31 +++++++---- src/CodexAppServerClient.ts | 13 +++-- .../CodexACPAgent/CodexAcpClient.test.ts | 10 +++- .../CodexACPAgent/fast-mode-config.test.ts | 1 + .../session-config-options.test.ts | 52 +++++++++++++++---- 7 files changed, 117 insertions(+), 25 deletions(-) diff --git a/src/AgentMode.ts b/src/AgentMode.ts index c8c1ad03..e581eaf9 100644 --- a/src/AgentMode.ts +++ b/src/AgentMode.ts @@ -122,6 +122,18 @@ export class AgentMode { return match ?? null; } + static fromSettings( + approvalPolicy: AskForApproval | undefined, + sandboxPolicy: SandboxPolicy | undefined, + ): AgentMode | null { + if (!sandboxPolicy) return null; + const match = AgentMode.all().find(mode => + mode.approvalPolicy === approvalPolicy + && mode.sandboxPolicy.type === sandboxPolicy.type + ); + return match ?? null; + } + static getInitialAgentMode(): AgentMode { const predefinedAgentMode = process.env["INITIAL_AGENT_MODE"]; if (predefinedAgentMode) { diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index a5590ce7..b85411ad 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -490,6 +490,8 @@ export class CodexAcpClient { sessionId: request.sessionId, currentModelId: currentModelId, models: codexModels, + agentMode: AgentMode.fromSettings(response.approvalPolicy, response.sandbox) + ?? AgentMode.getInitialAgentMode(), collaborationMode: this.getCollaborationMode(response.thread.id), modelProvider: response.modelProvider, currentServiceTier: response.serviceTier as ServiceTier ?? null, @@ -533,6 +535,8 @@ export class CodexAcpClient { sessionId: request.sessionId, currentModelId: currentModelId, models: codexModels, + agentMode: AgentMode.fromSettings(response.approvalPolicy, response.sandbox) + ?? AgentMode.getInitialAgentMode(), collaborationMode: this.getCollaborationMode(response.thread.id), modelProvider: response.modelProvider, currentServiceTier: response.serviceTier as ServiceTier ?? null, @@ -567,6 +571,8 @@ export class CodexAcpClient { sessionId: response.thread.id, currentModelId: currentModelId, models: codexModels, + agentMode: AgentMode.fromSettings(response.approvalPolicy, response.sandbox) + ?? AgentMode.getInitialAgentMode(), collaborationMode: this.getCollaborationMode(response.thread.id), modelProvider: response.modelProvider, currentServiceTier: response.serviceTier as ServiceTier ?? null, @@ -1023,6 +1029,23 @@ export class CodexAcpClient { }); } + async setAgentMode(sessionId: string, mode: AgentMode): Promise { + await this.codexClient.threadSettingsUpdate({ + threadId: sessionId, + approvalPolicy: mode.approvalPolicy, + sandboxPolicy: mode.sandboxPolicy, + }); + } + + async setModelAndEffort(sessionId: string, currentModelId: string): Promise { + const modelId = ModelId.fromString(currentModelId); + await this.codexClient.threadSettingsUpdate({ + threadId: sessionId, + model: modelId.model, + effort: modelId.effort as ReasoningEffort, + }); + } + private getCollaborationMode(sessionId: string): ModeKind { return this.codexClient.getThreadSettings(sessionId)?.collaborationMode.mode ?? "default"; } diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 234e1cd3..856666a5 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -621,7 +621,7 @@ export class CodexAcpServer { availableModels: models, supportedReasoningEfforts: currentModel?.supportedReasoningEfforts ?? [], supportedInputModalities: currentModel?.inputModalities ?? ["text", "image"], - agentMode: AgentMode.getInitialAgentMode(), + agentMode: sessionMetadata.agentMode ?? AgentMode.getInitialAgentMode(), collaborationMode: sessionMetadata.collaborationMode, currentTurnId: null, lastTokenUsage: null, @@ -1072,7 +1072,7 @@ export class CodexAcpServer { const sessionState = this.sessions.get(_params.sessionId); if (!sessionState) throw new Error(`Session ${_params.sessionId} not found`); - this.applyModeChange(sessionState, _params.modeId); + await this.applyModeChange(sessionState, _params.modeId); return {}; } @@ -1097,16 +1097,16 @@ export class CodexAcpServer { this.applyFastModeChange(sessionState, params); break; case MODE_CONFIG_ID: - this.applyModeChange(sessionState, this.stringConfigValue(params)); + await this.applyModeChange(sessionState, this.stringConfigValue(params)); break; case COLLABORATION_MODE_CONFIG_ID: 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(); @@ -1132,11 +1132,12 @@ export class CodexAcpServer { return params.value; } - private applyModeChange(sessionState: SessionState, value: string): void { + private async applyModeChange(sessionState: SessionState, value: string): Promise { const newMode = AgentMode.find(value); if (!newMode) { throw RequestError.invalidParams(); } + await this.codexAcpClient.setAgentMode(sessionState.sessionId, newMode); sessionState.agentMode = newMode; } @@ -1149,7 +1150,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; @@ -1161,16 +1162,22 @@ 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(), + ); 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.currentModelId = currentModelId; } private applyModelAndEffort(sessionState: SessionState, model: Model, effort: ReasoningEffort): void { @@ -1206,6 +1213,10 @@ export class CodexAcpServer { } sessionState.availableModels = models; + await this.codexAcpClient.setModelAndEffort( + sessionState.sessionId, + ModelId.fromComponents(model, reasoningEffort).toString(), + ); this.applyModelAndEffort(sessionState, model, reasoningEffort); return {}; @@ -1672,7 +1683,7 @@ export class CodexAcpServer { availableModels: models, supportedReasoningEfforts: currentModel?.supportedReasoningEfforts ?? [], supportedInputModalities: currentModel?.inputModalities ?? ["text", "image"], - agentMode: AgentMode.getInitialAgentMode(), + agentMode: sessionMetadata.agentMode ?? AgentMode.getInitialAgentMode(), collaborationMode: sessionMetadata.collaborationMode, currentTurnId: null, lastTokenUsage: null, diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index 51521928..7398e4dd 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -3,9 +3,11 @@ import type { ClientRequest, InitializeParams, InitializeResponse, + ReasoningEffort, ServerNotification } from "./app-server"; import type { + AskForApproval, CancelLoginAccountParams, CancelLoginAccountResponse, ConfigReadParams, @@ -73,6 +75,7 @@ import type { TurnStartResponse, TurnSteerParams, TurnSteerResponse, + SandboxPolicy, CommandExecutionRequestApprovalParams, CommandExecutionRequestApprovalResponse, FileChangeRequestApprovalParams, @@ -552,7 +555,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); } @@ -1022,9 +1025,13 @@ type DistributiveOmit = T extends any ? Omit : never; -export interface ExperimentalThreadSettingsUpdateParams { +export interface ThreadSettingsUpdateParams { threadId: string; - collaborationMode: { + approvalPolicy?: AskForApproval; + sandboxPolicy?: SandboxPolicy; + model?: string; + effort?: ReasoningEffort; + collaborationMode?: { mode: "default" | "plan"; settings: { model: string; diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 2f9e2dcb..f64acd21 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -594,6 +594,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({ threadId: "source-id", cwd: "/workspace", @@ -696,7 +697,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { })); }); - it('restores collaboration mode for resumed and loaded sessions', async () => { + it('restores collaboration and agent modes for resumed and loaded sessions', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpAgent = mockFixture.getCodexAcpAgent(); const codexAcpClient = mockFixture.getCodexAcpClient(); @@ -725,6 +726,9 @@ describe('ACP server test', { timeout: 40_000 }, () => { modelProvider: "openai", reasoningEffort: "medium", serviceTier: null, + approvalPolicy: "never", + approvalsReviewer: "user", + sandbox: {type: "dangerFullAccess"}, } as any; }); vi.spyOn(codexAppServerClient, "threadRead").mockImplementation(async ({threadId}) => ({ @@ -747,8 +751,12 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(codexAcpAgent.getSessionState("resume-id").collaborationMode).toBe("plan"); expect(codexAcpAgent.getSessionState("load-id").collaborationMode).toBe("plan"); + expect(codexAcpAgent.getSessionState("resume-id").agentMode).toBe(AgentMode.AgentFullAccess); + expect(codexAcpAgent.getSessionState("load-id").agentMode).toBe(AgentMode.AgentFullAccess); expect(resumed.configOptions?.find(option => option.id === "collaboration_mode")).toMatchObject({currentValue: "plan"}); expect(loaded.configOptions?.find(option => option.id === "collaboration_mode")).toMatchObject({currentValue: "plan"}); + expect(resumed.configOptions?.find(option => option.id === "mode")).toMatchObject({currentValue: "agent-full-access"}); + expect(loaded.configOptions?.find(option => option.id === "mode")).toMatchObject({currentValue: "agent-full-access"}); }); it('uses configured model provider when resuming sessions without an explicit provider', async () => { 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/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index e895d282..68ed43cb 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -49,12 +49,27 @@ async function createSession(currentModelId: string, availableModels: Array { + it("distinguishes modes that share an approval policy and sandbox", () => { + expect(AgentMode.fromSettings( + AgentMode.ReadOnly.approvalPolicy, + AgentMode.ReadOnly.approvalsReviewer, + AgentMode.ReadOnly.sandboxPolicy, + )).toBe(AgentMode.ReadOnly); + expect(AgentMode.fromSettings( + AgentMode.Agent.approvalPolicy, + AgentMode.Agent.approvalsReviewer, + AgentMode.Agent.sandboxPolicy, + )).toBe(AgentMode.Agent); + }); + it("exposes mode, model, reasoning_effort and fast-mode in the new session response", async () => { const {fast, slow} = buildModels(); const {response} = await createSession("fast-model[medium]", [fast, slow]); @@ -158,23 +173,28 @@ describe("Session config options", () => { it("changes the agent mode via setSessionConfigOption", async () => { const {fast} = buildModels(); - const {codexAcpAgent} = await createSession("fast-model[medium]", [fast]); + const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); const result = await codexAcpAgent.setSessionConfigOption({ sessionId: "session-id", configId: MODE_CONFIG_ID, - value: AgentMode.Agent.id, + value: AgentMode.ReadOnly.id, }); - expect(codexAcpAgent.getSessionState("session-id").agentMode).toBe(AgentMode.Agent); + expect(codexAcpAgent.getSessionState("session-id").agentMode).toBe(AgentMode.ReadOnly); + expect(update).toHaveBeenCalledWith({ + threadId: "session-id", + approvalPolicy: AgentMode.ReadOnly.approvalPolicy, + approvalsReviewer: AgentMode.ReadOnly.approvalsReviewer, + sandboxPolicy: AgentMode.ReadOnly.sandboxPolicy, + }); const modeOption = result.configOptions?.find(o => o.id === MODE_CONFIG_ID); - expect((modeOption as any).currentValue).toBe(AgentMode.Agent.id); + expect((modeOption as any).currentValue).toBe(AgentMode.ReadOnly.id); }); 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 +212,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({ @@ -247,7 +266,7 @@ describe("Session config options", () => { 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 +275,11 @@ 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", + }); }); it("falls back to the new model's default effort when the current effort is unsupported", async () => { @@ -273,7 +297,7 @@ describe("Session config options", () => { 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 +306,11 @@ 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", + }); }); it("refreshes the cached model list when unstable_setSessionModel picks a freshly fetched model", async () => { @@ -309,6 +338,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", From 3622e39b79aaf2fa27bcbe456440843a08ba6ffa Mon Sep 17 00:00:00 2001 From: Gilbert Leung Date: Mon, 27 Jul 2026 10:38:32 +0800 Subject: [PATCH 02/13] fix: apply agent mode before persisting (cherry picked from commit edd908bbd561498a1e82f01f9ef90934eecc20d3) --- src/CodexAcpServer.ts | 8 ++- .../session-config-options.test.ts | 62 ++++++++++++++++++- 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 856666a5..0e1abcde 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -1137,8 +1137,14 @@ export class CodexAcpServer { if (!newMode) { throw RequestError.invalidParams(); } - await this.codexAcpClient.setAgentMode(sessionState.sessionId, newMode); + const previousMode = sessionState.agentMode; sessionState.agentMode = newMode; + try { + await this.codexAcpClient.setAgentMode(sessionState.sessionId, newMode); + } catch (error) { + sessionState.agentMode = previousMode; + throw error; + } } private async applyCollaborationModeChange(sessionState: SessionState, value: string): Promise { diff --git a/src/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index 68ed43cb..2474d11e 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -1,5 +1,9 @@ import {describe, expect, it, vi} from "vitest"; -import {createCodexMockTestFixture, createTestModel} from "../acp-test-utils"; +import { + createCodexMockTestFixture, + createTestModel, + mockPromptTurn, +} from "../acp-test-utils"; import {AgentMode, MODE_CONFIG_ID} from "../../AgentMode"; import { MODEL_CONFIG_ID, @@ -16,6 +20,17 @@ const lowEffort: ReasoningEffortOption = {reasoningEffort: "low", description: " const mediumEffort: ReasoningEffortOption = {reasoningEffort: "medium", description: "Balanced"}; const highEffort: ReasoningEffortOption = {reasoningEffort: "high", description: "Thorough"}; +function deferred(): { + promise: Promise; + resolve: (value: T | PromiseLike) => void; +} { + let resolve!: (value: T | PromiseLike) => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return {promise, resolve}; +} + function buildModels(): {fast: Model; slow: Model} { const fast = createTestModel({ id: "fast-model", @@ -192,6 +207,51 @@ describe("Session config options", () => { expect((modeOption as any).currentValue).toBe(AgentMode.ReadOnly.id); }); + it("uses a new agent mode for prompts while thread persistence is pending", async () => { + const {fast} = buildModels(); + const {fixture, codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); + const sessionState = codexAcpAgent.getSessionState("session-id"); + const turnStartSpy = mockPromptTurn(fixture, sessionState.sessionId); + const persistence = deferred(); + update.mockReturnValue(persistence.promise); + + const modeChange = codexAcpAgent.setSessionConfigOption({ + sessionId: sessionState.sessionId, + configId: MODE_CONFIG_ID, + value: AgentMode.AgentFullAccess.id, + }); + + expect(sessionState.agentMode).toBe(AgentMode.AgentFullAccess); + + await codexAcpAgent.prompt({ + sessionId: sessionState.sessionId, + prompt: [{type: "text", text: "test"}], + }); + + expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ + approvalPolicy: AgentMode.AgentFullAccess.approvalPolicy, + sandboxPolicy: AgentMode.AgentFullAccess.sandboxPolicy, + })); + + persistence.resolve(); + await modeChange; + }); + + it("rolls back the agent mode when thread persistence fails", async () => { + const {fast} = buildModels(); + const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); + const sessionState = codexAcpAgent.getSessionState("session-id"); + update.mockRejectedValue(new Error("settings update failed")); + + await expect(codexAcpAgent.setSessionConfigOption({ + sessionId: sessionState.sessionId, + configId: MODE_CONFIG_ID, + value: AgentMode.AgentFullAccess.id, + })).rejects.toThrow("settings update failed"); + + expect(sessionState.agentMode).toBe(AgentMode.Agent); + }); + it("changes collaboration mode without starting a model turn", async () => { const {fast} = buildModels(); const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); From 7cade42fc7d49a663a71e6b0d3cc2ebbd97e49e8 Mon Sep 17 00:00:00 2001 From: Gilbert Leung Date: Mon, 27 Jul 2026 10:52:37 +0800 Subject: [PATCH 03/13] fix: return session id when loading sessions (cherry picked from commit 3487c8bcff942ef9ed83f2802fd7522cee64e243) --- src/AcpExtensions.ts | 1 + src/CodexAcpServer.ts | 1 + src/__tests__/CodexACPAgent/load-session.test.ts | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/AcpExtensions.ts b/src/AcpExtensions.ts index a469d515..3eeec4f0 100644 --- a/src/AcpExtensions.ts +++ b/src/AcpExtensions.ts @@ -50,6 +50,7 @@ export type LegacyNewSessionResponse = NewSessionResponse & { } export type LegacyLoadSessionResponse = LoadSessionResponse & { + sessionId: SessionId; models?: LegacySessionModelState | null; } diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 0e1abcde..fe6459f9 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -729,6 +729,7 @@ export class CodexAcpServer { availableModelCount: modelState.availableModels.length }); return { + sessionId, models: modelState, modes: modeState, ...this.createSessionConfigOptionsResponse(this.getSessionState(sessionId)), diff --git a/src/__tests__/CodexACPAgent/load-session.test.ts b/src/__tests__/CodexACPAgent/load-session.test.ts index 6811f532..67d4a3cd 100644 --- a/src/__tests__/CodexACPAgent/load-session.test.ts +++ b/src/__tests__/CodexACPAgent/load-session.test.ts @@ -377,8 +377,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.threadRead).toHaveBeenCalledWith({ threadId: thread.id, includeTurns: true, From c5cb675c593eec361bdf51f3e05f78d49ebaca6b Mon Sep 17 00:00:00 2001 From: Gilbert Leung Date: Mon, 27 Jul 2026 15:15:53 +0800 Subject: [PATCH 04/13] fix: use latest agent mode when starting turns (cherry picked from commit 10f6541a68dfcb16e65ec90c47d3f8df6258a621) --- src/CodexAcpClient.ts | 3 +- src/CodexAcpServer.ts | 5 ++-- .../session-config-options.test.ts | 29 +++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index b85411ad..a423cd92 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -882,7 +882,7 @@ export class CodexAcpClient { async sendPrompt( request: acp.PromptRequest, - agentMode: AgentMode, + getAgentMode: () => AgentMode, modelId: ModelId, serviceTier: ServiceTier | null, disableSummary: boolean, @@ -897,6 +897,7 @@ export class CodexAcpClient { if (shouldCancel?.()) { return null; } + const agentMode = getAgentMode(); return await this.codexClient.runTurn({ threadId: request.sessionId, input: input, diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index fe6459f9..84b067bd 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -2698,7 +2698,6 @@ export class CodexAcpServer { if (!sessionState.supportedInputModalities.includes("image") && effectiveParams.prompt.some(b => b.type === "image")) { throw RequestError.invalidRequest("The current model does not support image input"); } - const agentMode = sessionState.agentMode; const serviceTier = resolveFastServiceTier( sessionState.fastModeEnabled, sessionState.currentModelSupportsFast, @@ -2707,7 +2706,7 @@ export class CodexAcpServer { const sendPromptPromise = this.runWithProcessCheck( () => this.codexAcpClient.sendPrompt( effectiveParams, - agentMode, + () => sessionState.agentMode, modelId, serviceTier, disableSummary, @@ -2807,7 +2806,7 @@ export class CodexAcpServer { const implementationPromise = this.runWithProcessCheck( () => this.codexAcpClient.sendPrompt( implementationRequest, - agentMode, + () => sessionState.agentMode, modelId, serviceTier, disableSummary, diff --git a/src/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index 2474d11e..5e0eede1 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -237,6 +237,35 @@ describe("Session config options", () => { await modeChange; }); + it("uses a new agent mode when it changes during prompt preparation", async () => { + const {fast} = buildModels(); + const {fixture, codexAcpAgent} = await createSession("fast-model[medium]", [fast]); + const sessionState = codexAcpAgent.getSessionState("session-id"); + const turnStartSpy = mockPromptTurn(fixture, sessionState.sessionId); + const skillRefresh = deferred<{data: []}>(); + const listSkillsSpy = vi.spyOn(fixture.getCodexAppServerClient(), "listSkills") + .mockReturnValue(skillRefresh.promise); + + const prompt = codexAcpAgent.prompt({ + sessionId: sessionState.sessionId, + prompt: [{type: "text", text: "test"}], + }); + await vi.waitFor(() => expect(listSkillsSpy).toHaveBeenCalled()); + + await codexAcpAgent.setSessionConfigOption({ + sessionId: sessionState.sessionId, + configId: MODE_CONFIG_ID, + value: AgentMode.AgentFullAccess.id, + }); + skillRefresh.resolve({data: []}); + await prompt; + + expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ + approvalPolicy: AgentMode.AgentFullAccess.approvalPolicy, + sandboxPolicy: AgentMode.AgentFullAccess.sandboxPolicy, + })); + }); + it("rolls back the agent mode when thread persistence fails", async () => { const {fast} = buildModels(); const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); From 27c806236bf511306179e28080956c4f6042e204 Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Wed, 12 Aug 2026 20:07:08 +0200 Subject: [PATCH 05/13] fix: keep thread model and effort when resuming without a configured provider --- src/CodexAcpClient.ts | 19 ++++++--- .../CodexACPAgent/CodexAcpClient.test.ts | 41 +++++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index a423cd92..04f3aa6a 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -480,7 +480,7 @@ export class CodexAcpClient { const response = await this.codexClient.threadResume({ config: await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? []), cwd: request.cwd, - modelProvider: await this.getResumeModelProvider(), + ...(await this.resumeModelProviderParams()), threadId: request.sessionId, }); onSubscribed?.(); @@ -521,7 +521,7 @@ export class CodexAcpClient { const response = await this.codexClient.threadResume({ config: await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? []), cwd: request.cwd, - modelProvider: await this.getResumeModelProvider(), + ...(await this.resumeModelProviderParams()), threadId: request.sessionId, }); onSubscribed?.(); @@ -744,10 +744,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( diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index f64acd21..f9a11514 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -799,6 +799,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, "threadRead").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(); From 7601a3192688f3f33c7127c13d0ca94b3af7b92d Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Tue, 1 Sep 2026 00:49:25 +0200 Subject: [PATCH 06/13] fix: make agent mode persistence atomic --- src/CodexAcpClient.ts | 8 +++-- src/CodexAcpServer.ts | 8 +---- .../CodexACPAgent/CodexAcpClient.test.ts | 32 +++++++++++++++++++ .../session-config-options.test.ts | 11 ++++--- 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 04f3aa6a..dca7a54d 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -556,10 +556,15 @@ export class CodexAcpClient { const additionalDirectories = readAdditionalDirectories(request.cwd, request.additionalDirectories, request._meta); await this.refreshSkills(request.cwd, additionalDirectories); + const initialAgentMode = AgentMode.getInitialAgentMode(); + const response = await this.codexClient.threadStart({ config: await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers), modelProvider: this.getModelProvider(), cwd: request.cwd, + approvalPolicy: initialAgentMode.approvalPolicy, + approvalsReviewer: initialAgentMode.approvalsReviewer, + sandbox: initialAgentMode.sandboxMode, }); const codexModels = await this.fetchAvailableModels(); @@ -571,8 +576,7 @@ export class CodexAcpClient { sessionId: response.thread.id, currentModelId: currentModelId, models: codexModels, - agentMode: AgentMode.fromSettings(response.approvalPolicy, response.sandbox) - ?? AgentMode.getInitialAgentMode(), + agentMode: initialAgentMode, collaborationMode: this.getCollaborationMode(response.thread.id), modelProvider: response.modelProvider, currentServiceTier: response.serviceTier as ServiceTier ?? null, diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 84b067bd..b82ed36c 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -1138,14 +1138,8 @@ export class CodexAcpServer { if (!newMode) { throw RequestError.invalidParams(); } - const previousMode = sessionState.agentMode; + await this.codexAcpClient.setAgentMode(sessionState.sessionId, newMode); sessionState.agentMode = newMode; - try { - await this.codexAcpClient.setAgentMode(sessionState.sessionId, newMode); - } catch (error) { - sessionState.agentMode = previousMode; - throw error; - } } private async applyCollaborationModeChange(sessionState: SessionState, value: string): Promise { diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index f9a11514..6ecde2c6 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -514,6 +514,38 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); }); + it('seeds new Codex threads with the configured initial agent mode', async () => { + const mockFixture = createCodexMockTestFixture(); + const codexAcpClient = mockFixture.getCodexAcpClient(); + const codexAppServerClient = mockFixture.getCodexAppServerClient(); + vi.stubEnv("INITIAL_AGENT_MODE", AgentMode.AgentFullAccess.id); + + vi.spyOn(codexAppServerClient, "skillsExtraRootsSet").mockResolvedValue(undefined); + vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({data: []}); + const threadStartSpy = vi.spyOn(codexAppServerClient, "threadStart").mockResolvedValue({ + thread: {id: "thread-id"} as any, + model: "gpt-5", + reasoningEffort: "medium", + serviceTier: null, + approvalPolicy: AgentMode.ReadOnly.approvalPolicy, + approvalsReviewer: AgentMode.ReadOnly.approvalsReviewer, + sandbox: AgentMode.ReadOnly.sandboxPolicy, + } as any); + vi.spyOn(codexAppServerClient, "listModels").mockResolvedValue({ + data: [createTestModel({id: "gpt-5"})], + nextCursor: null, + }); + + const session = await codexAcpClient.newSession({cwd: "/workspace", mcpServers: []}); + + expect(threadStartSpy).toHaveBeenCalledWith(expect.objectContaining({ + approvalPolicy: AgentMode.AgentFullAccess.approvalPolicy, + approvalsReviewer: AgentMode.AgentFullAccess.approvalsReviewer, + sandbox: AgentMode.AgentFullAccess.sandboxMode, + })); + expect(session.agentMode).toBe(AgentMode.AgentFullAccess); + }); + it('applies ACP additional directories to resumed and loaded sessions explicitly', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpClient = mockFixture.getCodexAcpClient(); diff --git a/src/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index 5e0eede1..78c463b1 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -207,7 +207,7 @@ describe("Session config options", () => { expect((modeOption as any).currentValue).toBe(AgentMode.ReadOnly.id); }); - it("uses a new agent mode for prompts while thread persistence is pending", async () => { + it("does not use a new agent mode for prompts until thread persistence succeeds", async () => { const {fast} = buildModels(); const {fixture, codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); const sessionState = codexAcpAgent.getSessionState("session-id"); @@ -221,7 +221,7 @@ describe("Session config options", () => { value: AgentMode.AgentFullAccess.id, }); - expect(sessionState.agentMode).toBe(AgentMode.AgentFullAccess); + expect(sessionState.agentMode).toBe(AgentMode.Agent); await codexAcpAgent.prompt({ sessionId: sessionState.sessionId, @@ -229,12 +229,13 @@ describe("Session config options", () => { }); expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ - approvalPolicy: AgentMode.AgentFullAccess.approvalPolicy, - sandboxPolicy: AgentMode.AgentFullAccess.sandboxPolicy, + approvalPolicy: AgentMode.Agent.approvalPolicy, + sandboxPolicy: AgentMode.Agent.sandboxPolicy, })); persistence.resolve(); await modeChange; + expect(sessionState.agentMode).toBe(AgentMode.AgentFullAccess); }); it("uses a new agent mode when it changes during prompt preparation", async () => { @@ -266,7 +267,7 @@ describe("Session config options", () => { })); }); - it("rolls back the agent mode when thread persistence fails", async () => { + it("keeps the agent mode unchanged when thread persistence fails", async () => { const {fast} = buildModels(); const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); const sessionState = codexAcpAgent.getSessionState("session-id"); From d33a6aab7bb4a7b2f381c8d4a0932a2d0e72066f Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Tue, 1 Sep 2026 01:01:04 +0200 Subject: [PATCH 07/13] refactor: narrow persistence fix to model settings --- src/AgentMode.ts | 12 -- src/CodexAcpClient.ts | 21 +--- src/CodexAcpServer.ts | 16 +-- src/CodexAppServerClient.ts | 4 - .../CodexACPAgent/CodexAcpClient.test.ts | 41 +------ .../session-config-options.test.ts | 114 +----------------- 6 files changed, 14 insertions(+), 194 deletions(-) diff --git a/src/AgentMode.ts b/src/AgentMode.ts index e581eaf9..c8c1ad03 100644 --- a/src/AgentMode.ts +++ b/src/AgentMode.ts @@ -122,18 +122,6 @@ export class AgentMode { return match ?? null; } - static fromSettings( - approvalPolicy: AskForApproval | undefined, - sandboxPolicy: SandboxPolicy | undefined, - ): AgentMode | null { - if (!sandboxPolicy) return null; - const match = AgentMode.all().find(mode => - mode.approvalPolicy === approvalPolicy - && mode.sandboxPolicy.type === sandboxPolicy.type - ); - return match ?? null; - } - static getInitialAgentMode(): AgentMode { const predefinedAgentMode = process.env["INITIAL_AGENT_MODE"]; if (predefinedAgentMode) { diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index dca7a54d..3f0980fa 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -490,8 +490,6 @@ export class CodexAcpClient { sessionId: request.sessionId, currentModelId: currentModelId, models: codexModels, - agentMode: AgentMode.fromSettings(response.approvalPolicy, response.sandbox) - ?? AgentMode.getInitialAgentMode(), collaborationMode: this.getCollaborationMode(response.thread.id), modelProvider: response.modelProvider, currentServiceTier: response.serviceTier as ServiceTier ?? null, @@ -535,8 +533,6 @@ export class CodexAcpClient { sessionId: request.sessionId, currentModelId: currentModelId, models: codexModels, - agentMode: AgentMode.fromSettings(response.approvalPolicy, response.sandbox) - ?? AgentMode.getInitialAgentMode(), collaborationMode: this.getCollaborationMode(response.thread.id), modelProvider: response.modelProvider, currentServiceTier: response.serviceTier as ServiceTier ?? null, @@ -556,15 +552,10 @@ export class CodexAcpClient { const additionalDirectories = readAdditionalDirectories(request.cwd, request.additionalDirectories, request._meta); await this.refreshSkills(request.cwd, additionalDirectories); - const initialAgentMode = AgentMode.getInitialAgentMode(); - const response = await this.codexClient.threadStart({ config: await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers), modelProvider: this.getModelProvider(), cwd: request.cwd, - approvalPolicy: initialAgentMode.approvalPolicy, - approvalsReviewer: initialAgentMode.approvalsReviewer, - sandbox: initialAgentMode.sandboxMode, }); const codexModels = await this.fetchAvailableModels(); @@ -576,7 +567,6 @@ export class CodexAcpClient { sessionId: response.thread.id, currentModelId: currentModelId, models: codexModels, - agentMode: initialAgentMode, collaborationMode: this.getCollaborationMode(response.thread.id), modelProvider: response.modelProvider, currentServiceTier: response.serviceTier as ServiceTier ?? null, @@ -893,7 +883,7 @@ export class CodexAcpClient { async sendPrompt( request: acp.PromptRequest, - getAgentMode: () => AgentMode, + agentMode: AgentMode, modelId: ModelId, serviceTier: ServiceTier | null, disableSummary: boolean, @@ -908,7 +898,6 @@ export class CodexAcpClient { if (shouldCancel?.()) { return null; } - const agentMode = getAgentMode(); return await this.codexClient.runTurn({ threadId: request.sessionId, input: input, @@ -1041,14 +1030,6 @@ export class CodexAcpClient { }); } - async setAgentMode(sessionId: string, mode: AgentMode): Promise { - await this.codexClient.threadSettingsUpdate({ - threadId: sessionId, - approvalPolicy: mode.approvalPolicy, - sandboxPolicy: mode.sandboxPolicy, - }); - } - async setModelAndEffort(sessionId: string, currentModelId: string): Promise { const modelId = ModelId.fromString(currentModelId); await this.codexClient.threadSettingsUpdate({ diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index b82ed36c..d7f79cc7 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -621,7 +621,7 @@ export class CodexAcpServer { availableModels: models, supportedReasoningEfforts: currentModel?.supportedReasoningEfforts ?? [], supportedInputModalities: currentModel?.inputModalities ?? ["text", "image"], - agentMode: sessionMetadata.agentMode ?? AgentMode.getInitialAgentMode(), + agentMode: AgentMode.getInitialAgentMode(), collaborationMode: sessionMetadata.collaborationMode, currentTurnId: null, lastTokenUsage: null, @@ -1073,7 +1073,7 @@ export class CodexAcpServer { const sessionState = this.sessions.get(_params.sessionId); if (!sessionState) throw new Error(`Session ${_params.sessionId} not found`); - await this.applyModeChange(sessionState, _params.modeId); + this.applyModeChange(sessionState, _params.modeId); return {}; } @@ -1098,7 +1098,7 @@ export class CodexAcpServer { this.applyFastModeChange(sessionState, params); break; case MODE_CONFIG_ID: - await this.applyModeChange(sessionState, this.stringConfigValue(params)); + this.applyModeChange(sessionState, this.stringConfigValue(params)); break; case COLLABORATION_MODE_CONFIG_ID: await this.applyCollaborationModeChange(sessionState, this.stringConfigValue(params)); @@ -1133,12 +1133,11 @@ export class CodexAcpServer { return params.value; } - private async applyModeChange(sessionState: SessionState, value: string): Promise { + private applyModeChange(sessionState: SessionState, value: string): void { const newMode = AgentMode.find(value); if (!newMode) { throw RequestError.invalidParams(); } - await this.codexAcpClient.setAgentMode(sessionState.sessionId, newMode); sessionState.agentMode = newMode; } @@ -1684,7 +1683,7 @@ export class CodexAcpServer { availableModels: models, supportedReasoningEfforts: currentModel?.supportedReasoningEfforts ?? [], supportedInputModalities: currentModel?.inputModalities ?? ["text", "image"], - agentMode: sessionMetadata.agentMode ?? AgentMode.getInitialAgentMode(), + agentMode: AgentMode.getInitialAgentMode(), collaborationMode: sessionMetadata.collaborationMode, currentTurnId: null, lastTokenUsage: null, @@ -2692,6 +2691,7 @@ export class CodexAcpServer { if (!sessionState.supportedInputModalities.includes("image") && effectiveParams.prompt.some(b => b.type === "image")) { throw RequestError.invalidRequest("The current model does not support image input"); } + const agentMode = sessionState.agentMode; const serviceTier = resolveFastServiceTier( sessionState.fastModeEnabled, sessionState.currentModelSupportsFast, @@ -2700,7 +2700,7 @@ export class CodexAcpServer { const sendPromptPromise = this.runWithProcessCheck( () => this.codexAcpClient.sendPrompt( effectiveParams, - () => sessionState.agentMode, + agentMode, modelId, serviceTier, disableSummary, @@ -2800,7 +2800,7 @@ export class CodexAcpServer { const implementationPromise = this.runWithProcessCheck( () => this.codexAcpClient.sendPrompt( implementationRequest, - () => sessionState.agentMode, + agentMode, modelId, serviceTier, disableSummary, diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index 7398e4dd..8bef58c4 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -7,7 +7,6 @@ import type { ServerNotification } from "./app-server"; import type { - AskForApproval, CancelLoginAccountParams, CancelLoginAccountResponse, ConfigReadParams, @@ -75,7 +74,6 @@ import type { TurnStartResponse, TurnSteerParams, TurnSteerResponse, - SandboxPolicy, CommandExecutionRequestApprovalParams, CommandExecutionRequestApprovalResponse, FileChangeRequestApprovalParams, @@ -1027,8 +1025,6 @@ type DistributiveOmit = T extends any export interface ThreadSettingsUpdateParams { threadId: string; - approvalPolicy?: AskForApproval; - sandboxPolicy?: SandboxPolicy; model?: string; effort?: ReasoningEffort; collaborationMode?: { diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index 6ecde2c6..a5dddc37 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -514,38 +514,6 @@ describe('ACP server test', { timeout: 40_000 }, () => { }); }); - it('seeds new Codex threads with the configured initial agent mode', async () => { - const mockFixture = createCodexMockTestFixture(); - const codexAcpClient = mockFixture.getCodexAcpClient(); - const codexAppServerClient = mockFixture.getCodexAppServerClient(); - vi.stubEnv("INITIAL_AGENT_MODE", AgentMode.AgentFullAccess.id); - - vi.spyOn(codexAppServerClient, "skillsExtraRootsSet").mockResolvedValue(undefined); - vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({data: []}); - const threadStartSpy = vi.spyOn(codexAppServerClient, "threadStart").mockResolvedValue({ - thread: {id: "thread-id"} as any, - model: "gpt-5", - reasoningEffort: "medium", - serviceTier: null, - approvalPolicy: AgentMode.ReadOnly.approvalPolicy, - approvalsReviewer: AgentMode.ReadOnly.approvalsReviewer, - sandbox: AgentMode.ReadOnly.sandboxPolicy, - } as any); - vi.spyOn(codexAppServerClient, "listModels").mockResolvedValue({ - data: [createTestModel({id: "gpt-5"})], - nextCursor: null, - }); - - const session = await codexAcpClient.newSession({cwd: "/workspace", mcpServers: []}); - - expect(threadStartSpy).toHaveBeenCalledWith(expect.objectContaining({ - approvalPolicy: AgentMode.AgentFullAccess.approvalPolicy, - approvalsReviewer: AgentMode.AgentFullAccess.approvalsReviewer, - sandbox: AgentMode.AgentFullAccess.sandboxMode, - })); - expect(session.agentMode).toBe(AgentMode.AgentFullAccess); - }); - it('applies ACP additional directories to resumed and loaded sessions explicitly', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpClient = mockFixture.getCodexAcpClient(); @@ -729,7 +697,7 @@ describe('ACP server test', { timeout: 40_000 }, () => { })); }); - it('restores collaboration and agent modes for resumed and loaded sessions', async () => { + it('restores collaboration mode for resumed and loaded sessions', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpAgent = mockFixture.getCodexAcpAgent(); const codexAcpClient = mockFixture.getCodexAcpClient(); @@ -758,9 +726,6 @@ describe('ACP server test', { timeout: 40_000 }, () => { modelProvider: "openai", reasoningEffort: "medium", serviceTier: null, - approvalPolicy: "never", - approvalsReviewer: "user", - sandbox: {type: "dangerFullAccess"}, } as any; }); vi.spyOn(codexAppServerClient, "threadRead").mockImplementation(async ({threadId}) => ({ @@ -783,12 +748,8 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(codexAcpAgent.getSessionState("resume-id").collaborationMode).toBe("plan"); expect(codexAcpAgent.getSessionState("load-id").collaborationMode).toBe("plan"); - expect(codexAcpAgent.getSessionState("resume-id").agentMode).toBe(AgentMode.AgentFullAccess); - expect(codexAcpAgent.getSessionState("load-id").agentMode).toBe(AgentMode.AgentFullAccess); expect(resumed.configOptions?.find(option => option.id === "collaboration_mode")).toMatchObject({currentValue: "plan"}); expect(loaded.configOptions?.find(option => option.id === "collaboration_mode")).toMatchObject({currentValue: "plan"}); - expect(resumed.configOptions?.find(option => option.id === "mode")).toMatchObject({currentValue: "agent-full-access"}); - expect(loaded.configOptions?.find(option => option.id === "mode")).toMatchObject({currentValue: "agent-full-access"}); }); it('uses configured model provider when resuming sessions without an explicit provider', async () => { diff --git a/src/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index 78c463b1..a2e393e2 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -2,7 +2,6 @@ import {describe, expect, it, vi} from "vitest"; import { createCodexMockTestFixture, createTestModel, - mockPromptTurn, } from "../acp-test-utils"; import {AgentMode, MODE_CONFIG_ID} from "../../AgentMode"; import { @@ -20,17 +19,6 @@ const lowEffort: ReasoningEffortOption = {reasoningEffort: "low", description: " const mediumEffort: ReasoningEffortOption = {reasoningEffort: "medium", description: "Balanced"}; const highEffort: ReasoningEffortOption = {reasoningEffort: "high", description: "Thorough"}; -function deferred(): { - promise: Promise; - resolve: (value: T | PromiseLike) => void; -} { - let resolve!: (value: T | PromiseLike) => void; - const promise = new Promise((resolvePromise) => { - resolve = resolvePromise; - }); - return {promise, resolve}; -} - function buildModels(): {fast: Model; slow: Model} { const fast = createTestModel({ id: "fast-model", @@ -72,19 +60,6 @@ async function createSession(currentModelId: string, availableModels: Array { - it("distinguishes modes that share an approval policy and sandbox", () => { - expect(AgentMode.fromSettings( - AgentMode.ReadOnly.approvalPolicy, - AgentMode.ReadOnly.approvalsReviewer, - AgentMode.ReadOnly.sandboxPolicy, - )).toBe(AgentMode.ReadOnly); - expect(AgentMode.fromSettings( - AgentMode.Agent.approvalPolicy, - AgentMode.Agent.approvalsReviewer, - AgentMode.Agent.sandboxPolicy, - )).toBe(AgentMode.Agent); - }); - it("exposes mode, model, reasoning_effort and fast-mode in the new session response", async () => { const {fast, slow} = buildModels(); const {response} = await createSession("fast-model[medium]", [fast, slow]); @@ -188,98 +163,17 @@ describe("Session config options", () => { it("changes the agent mode via setSessionConfigOption", async () => { const {fast} = buildModels(); - const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); + const {codexAcpAgent} = await createSession("fast-model[medium]", [fast]); const result = await codexAcpAgent.setSessionConfigOption({ sessionId: "session-id", configId: MODE_CONFIG_ID, - value: AgentMode.ReadOnly.id, + value: AgentMode.Agent.id, }); - expect(codexAcpAgent.getSessionState("session-id").agentMode).toBe(AgentMode.ReadOnly); - expect(update).toHaveBeenCalledWith({ - threadId: "session-id", - approvalPolicy: AgentMode.ReadOnly.approvalPolicy, - approvalsReviewer: AgentMode.ReadOnly.approvalsReviewer, - sandboxPolicy: AgentMode.ReadOnly.sandboxPolicy, - }); + expect(codexAcpAgent.getSessionState("session-id").agentMode).toBe(AgentMode.Agent); const modeOption = result.configOptions?.find(o => o.id === MODE_CONFIG_ID); - expect((modeOption as any).currentValue).toBe(AgentMode.ReadOnly.id); - }); - - it("does not use a new agent mode for prompts until thread persistence succeeds", async () => { - const {fast} = buildModels(); - const {fixture, codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); - const sessionState = codexAcpAgent.getSessionState("session-id"); - const turnStartSpy = mockPromptTurn(fixture, sessionState.sessionId); - const persistence = deferred(); - update.mockReturnValue(persistence.promise); - - const modeChange = codexAcpAgent.setSessionConfigOption({ - sessionId: sessionState.sessionId, - configId: MODE_CONFIG_ID, - value: AgentMode.AgentFullAccess.id, - }); - - expect(sessionState.agentMode).toBe(AgentMode.Agent); - - await codexAcpAgent.prompt({ - sessionId: sessionState.sessionId, - prompt: [{type: "text", text: "test"}], - }); - - expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ - approvalPolicy: AgentMode.Agent.approvalPolicy, - sandboxPolicy: AgentMode.Agent.sandboxPolicy, - })); - - persistence.resolve(); - await modeChange; - expect(sessionState.agentMode).toBe(AgentMode.AgentFullAccess); - }); - - it("uses a new agent mode when it changes during prompt preparation", async () => { - const {fast} = buildModels(); - const {fixture, codexAcpAgent} = await createSession("fast-model[medium]", [fast]); - const sessionState = codexAcpAgent.getSessionState("session-id"); - const turnStartSpy = mockPromptTurn(fixture, sessionState.sessionId); - const skillRefresh = deferred<{data: []}>(); - const listSkillsSpy = vi.spyOn(fixture.getCodexAppServerClient(), "listSkills") - .mockReturnValue(skillRefresh.promise); - - const prompt = codexAcpAgent.prompt({ - sessionId: sessionState.sessionId, - prompt: [{type: "text", text: "test"}], - }); - await vi.waitFor(() => expect(listSkillsSpy).toHaveBeenCalled()); - - await codexAcpAgent.setSessionConfigOption({ - sessionId: sessionState.sessionId, - configId: MODE_CONFIG_ID, - value: AgentMode.AgentFullAccess.id, - }); - skillRefresh.resolve({data: []}); - await prompt; - - expect(turnStartSpy).toHaveBeenCalledWith(expect.objectContaining({ - approvalPolicy: AgentMode.AgentFullAccess.approvalPolicy, - sandboxPolicy: AgentMode.AgentFullAccess.sandboxPolicy, - })); - }); - - it("keeps the agent mode unchanged when thread persistence fails", async () => { - const {fast} = buildModels(); - const {codexAcpAgent, update} = await createSession("fast-model[medium]", [fast]); - const sessionState = codexAcpAgent.getSessionState("session-id"); - update.mockRejectedValue(new Error("settings update failed")); - - await expect(codexAcpAgent.setSessionConfigOption({ - sessionId: sessionState.sessionId, - configId: MODE_CONFIG_ID, - value: AgentMode.AgentFullAccess.id, - })).rejects.toThrow("settings update failed"); - - expect(sessionState.agentMode).toBe(AgentMode.Agent); + expect((modeOption as any).currentValue).toBe(AgentMode.Agent.id); }); it("changes collaboration mode without starting a model turn", async () => { From b866b95478526a4171264376f9ce922dc3be1f58 Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Tue, 1 Sep 2026 21:42:42 +0200 Subject: [PATCH 08/13] fix: preserve fork session model settings --- src/CodexAcpClient.ts | 2 +- src/SessionFork.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 3f0980fa..18ec93a7 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -504,7 +504,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(), diff --git a/src/SessionFork.ts b/src/SessionFork.ts index b52b7cae..e4b810ee 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; @@ -36,7 +36,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}); From 33c92489440b228b8ad7b5b32e61d6d18bdd2207 Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Tue, 1 Sep 2026 22:05:19 +0200 Subject: [PATCH 09/13] fix: close fork release review findings --- README.md | 16 +++- src/CodexAcpClient.ts | 7 +- src/CodexAcpServer.ts | 28 +++++- .../CodexACPAgent/CodexAcpClient.test.ts | 34 +++++++ .../session-config-options.test.ts | 96 +++++++++++++++++++ 5 files changed, 173 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 092f61ba..17140af7 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # ACP adapter for Codex CLI -[![npm version](https://img.shields.io/npm/v/%40agentclientprotocol%2Fcodex-acp)](https://www.npmjs.com/package/@agentclientprotocol/codex-acp) +[![npm version](https://img.shields.io/npm/v/%40superbiche%2Fcodex-acp)](https://www.npmjs.com/package/@superbiche/codex-acp) Use [OpenAI Codex](https://github.com/openai/codex) from [Agent Client Protocol](https://agentclientprotocol.com/) clients. @@ -20,23 +20,31 @@ Use [OpenAI Codex](https://github.com/openai/codex) from [Agent Client Protocol] ## Installation +> **Fork release:** this public package is the fleet-pinned +> `@superbiche/codex-acp` fork. Model and reasoning-effort changes persist +> across later turns and reconnects. When an explicit `model_provider` +> override is configured, Codex app-server re-resolves settings from that +> provider during resume/load/fork; stored model/effort persistence across +> those operations therefore applies only when no provider override is +> supplied. + Run the published package directly: ```bash -npx -y @agentclientprotocol/codex-acp +npx -y @superbiche/codex-acp ``` Or install it globally: ```bash -npm install -g @agentclientprotocol/codex-acp +npm install -g @superbiche/codex-acp codex-acp --version ``` The npm package includes a compatible `@openai/codex` dependency. Set `CODEX_PATH` only when you want the adapter to run a different Codex binary: ```bash -CODEX_PATH=/path/to/codex npx -y @agentclientprotocol/codex-acp +CODEX_PATH=/path/to/codex npx -y @superbiche/codex-acp ``` ## Authentication diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 18ec93a7..224650dd 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -1030,12 +1030,17 @@ export class CodexAcpClient { }); } - async setModelAndEffort(sessionId: string, currentModelId: string): Promise { + 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), }); } diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index d7f79cc7..1bd47509 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -255,6 +255,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 initializeRequest: acp.InitializeRequest | null = null; @@ -277,6 +278,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; @@ -1085,7 +1087,17 @@ 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); + const run = () => this.applySessionConfigOption(sessionState, params); + const previous = this.sessionConfigUpdates.get(params.sessionId); + const update = previous ? previous.then(run, run) : run(); + this.sessionConfigUpdates.set(params.sessionId, update); + try { + await update; + } finally { + if (this.sessionConfigUpdates.get(params.sessionId) === update) { + this.sessionConfigUpdates.delete(params.sessionId); + } + } return { configOptions: this.createSessionConfigOptions(sessionState), @@ -1165,6 +1177,7 @@ export class CodexAcpServer { await this.codexAcpClient.setModelAndEffort( sessionState.sessionId, ModelId.fromComponents(model, effort).toString(), + sessionState.collaborationMode, ); this.applyModelAndEffort(sessionState, model, effort); } @@ -1176,7 +1189,11 @@ export class CodexAcpServer { } const {model} = ModelId.fromString(sessionState.currentModelId); const currentModelId = ModelId.create(model, effort).toString(); - await this.codexAcpClient.setModelAndEffort(sessionState.sessionId, currentModelId); + await this.codexAcpClient.setModelAndEffort( + sessionState.sessionId, + currentModelId, + sessionState.collaborationMode, + ); sessionState.currentModelId = currentModelId; } @@ -1212,11 +1229,12 @@ export class CodexAcpServer { reasoningEffort = model.defaultReasoningEffort; } - sessionState.availableModels = models; await this.codexAcpClient.setModelAndEffort( sessionState.sessionId, ModelId.fromComponents(model, reasoningEffort).toString(), + sessionState.collaborationMode, ); + sessionState.availableModels = models; this.applyModelAndEffort(sessionState, model, reasoningEffort); return {}; @@ -2492,6 +2510,10 @@ export class CodexAcpServer { if (this.providerUpdate !== null) { await this.providerUpdate; } + const pendingConfigUpdate = this.sessionConfigUpdates.get(params.sessionId); + if (pendingConfigUpdate !== undefined) { + await pendingConfigUpdate; + } logger.log("Prompt received", { sessionId: params.sessionId, prompt: params.prompt, diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index a5dddc37..f1a9ff42 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -608,6 +608,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(); diff --git a/src/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index a2e393e2..8b3b6ae1 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -263,6 +263,14 @@ describe("Session config options", () => { threadId: "session-id", model: "slow-model", effort: "medium", + collaborationMode: { + mode: "default", + settings: { + model: "slow-model", + reasoning_effort: "medium", + developer_instructions: null, + }, + }, }); }); @@ -279,6 +287,38 @@ 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, update} = await createSession("fast-model[medium]", [fast]); @@ -294,6 +334,14 @@ describe("Session config options", () => { threadId: "session-id", model: "fast-model", effort: "high", + collaborationMode: { + mode: "default", + settings: { + model: "fast-model", + reasoning_effort: "high", + developer_instructions: null, + }, + }, }); }); @@ -333,6 +381,54 @@ 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("changes the model through the legacy session/set_model extMethod", async () => { const {fast, slow} = buildModels(); const {codexAcpAgent, codexAcpClient} = await createSession("fast-model[medium]", [fast]); From 4d525be99a9e6f42b862434aaf437c7d70de9834 Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Tue, 1 Sep 2026 22:08:03 +0200 Subject: [PATCH 10/13] docs: keep upstream installation instructions --- README.md | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 17140af7..092f61ba 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # ACP adapter for Codex CLI -[![npm version](https://img.shields.io/npm/v/%40superbiche%2Fcodex-acp)](https://www.npmjs.com/package/@superbiche/codex-acp) +[![npm version](https://img.shields.io/npm/v/%40agentclientprotocol%2Fcodex-acp)](https://www.npmjs.com/package/@agentclientprotocol/codex-acp) Use [OpenAI Codex](https://github.com/openai/codex) from [Agent Client Protocol](https://agentclientprotocol.com/) clients. @@ -20,31 +20,23 @@ Use [OpenAI Codex](https://github.com/openai/codex) from [Agent Client Protocol] ## Installation -> **Fork release:** this public package is the fleet-pinned -> `@superbiche/codex-acp` fork. Model and reasoning-effort changes persist -> across later turns and reconnects. When an explicit `model_provider` -> override is configured, Codex app-server re-resolves settings from that -> provider during resume/load/fork; stored model/effort persistence across -> those operations therefore applies only when no provider override is -> supplied. - Run the published package directly: ```bash -npx -y @superbiche/codex-acp +npx -y @agentclientprotocol/codex-acp ``` Or install it globally: ```bash -npm install -g @superbiche/codex-acp +npm install -g @agentclientprotocol/codex-acp codex-acp --version ``` The npm package includes a compatible `@openai/codex` dependency. Set `CODEX_PATH` only when you want the adapter to run a different Codex binary: ```bash -CODEX_PATH=/path/to/codex npx -y @superbiche/codex-acp +CODEX_PATH=/path/to/codex npx -y @agentclientprotocol/codex-acp ``` ## Authentication From e242ca4d8f18d75aac141150c042017216805fb9 Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Tue, 1 Sep 2026 22:15:46 +0200 Subject: [PATCH 11/13] fix: serialize legacy model updates with prompts --- src/CodexAcpServer.ts | 86 ++++++++++--------- .../session-config-options.test.ts | 29 +++++++ 2 files changed, 76 insertions(+), 39 deletions(-) diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 1bd47509..20121349 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -255,7 +255,7 @@ export class CodexAcpServer { private readonly sessionGenerations: Map; private readonly sessionOpenGenerations: Map; private readonly goalControlGenerations: Map; - private readonly sessionConfigUpdates: Map>; + private readonly sessionConfigUpdates: Map>; private readonly permissionLifecycleContexts: WeakMap; private readonly codexProcessState: CodexProcessState | null; private initializeRequest: acp.InitializeRequest | null = null; @@ -1087,17 +1087,10 @@ export class CodexAcpServer { const sessionState = this.sessions.get(params.sessionId); if (!sessionState) throw new Error(`Session ${params.sessionId} not found`); - const run = () => this.applySessionConfigOption(sessionState, params); - const previous = this.sessionConfigUpdates.get(params.sessionId); - const update = previous ? previous.then(run, run) : run(); - this.sessionConfigUpdates.set(params.sessionId, update); - try { - await update; - } finally { - if (this.sessionConfigUpdates.get(params.sessionId) === update) { - this.sessionConfigUpdates.delete(params.sessionId); - } - } + await this.runSessionConfigUpdate( + params.sessionId, + () => this.applySessionConfigOption(sessionState, params), + ); return { configOptions: this.createSessionConfigOptions(sessionState), @@ -1126,6 +1119,19 @@ export class CodexAcpServer { } } + 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") { @@ -1205,39 +1211,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; - } - await this.codexAcpClient.setModelAndEffort( - sessionState.sessionId, - ModelId.fromComponents(model, reasoningEffort).toString(), - sessionState.collaborationMode, - ); - 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 { diff --git a/src/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index 8b3b6ae1..4fbe6808 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -429,6 +429,35 @@ describe("Session config options", () => { 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("changes the model through the legacy session/set_model extMethod", async () => { const {fast, slow} = buildModels(); const {codexAcpAgent, codexAcpClient} = await createSession("fast-model[medium]", [fast]); From 11c05df6fde522bd3956714dd64e19d176de9944 Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Tue, 1 Sep 2026 22:24:23 +0200 Subject: [PATCH 12/13] fix: serialize mode and model settings updates --- docs/RELEASES.md | 2 +- src/CodexAcpServer.ts | 27 +++-- .../session-config-options.test.ts | 109 ++++++++++++++++++ 3 files changed, 130 insertions(+), 8 deletions(-) diff --git a/docs/RELEASES.md b/docs/RELEASES.md index 0eef4ff5..e28ddb75 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/CodexAcpServer.ts b/src/CodexAcpServer.ts index 20121349..3d7eaf53 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -2520,7 +2520,14 @@ export class CodexAcpServer { } const pendingConfigUpdate = this.sessionConfigUpdates.get(params.sessionId); if (pendingConfigUpdate !== undefined) { - await pendingConfigUpdate; + try { + await pendingConfigUpdate; + } catch { + throw RequestError.invalidRequest( + undefined, + "Prompt blocked because a pending session configuration update failed", + ); + } } logger.log("Prompt received", { sessionId: params.sessionId, @@ -2635,11 +2642,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", @@ -2814,7 +2824,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/__tests__/CodexACPAgent/session-config-options.test.ts b/src/__tests__/CodexACPAgent/session-config-options.test.ts index 4fbe6808..8e05ac12 100644 --- a/src/__tests__/CodexACPAgent/session-config-options.test.ts +++ b/src/__tests__/CodexACPAgent/session-config-options.test.ts @@ -248,6 +248,51 @@ 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, update} = await createSession("fast-model[medium]", [fast, slow]); @@ -458,6 +503,70 @@ describe("Session config options", () => { 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]); From 4014f8bd8972958d311c176684c9f96435f29870 Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Tue, 1 Sep 2026 22:38:57 +0200 Subject: [PATCH 13/13] fix: close persistence review residuals --- docs/RELEASES.md | 2 +- src/CodexAcpServer.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/RELEASES.md b/docs/RELEASES.md index e28ddb75..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 "$(node -p \"require('./package.json').name\")@" +npm view "$(node -p "require('./package.json').name")@" ``` ## How the version is chosen diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 3d7eaf53..023e30b2 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -2522,7 +2522,8 @@ export class CodexAcpServer { if (pendingConfigUpdate !== undefined) { try { await pendingConfigUpdate; - } catch { + } 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",