From a381a0876bd6b14baba8f3ec9fde89c8352babdf Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 4 Nov 2026 08:09:16 +0100 Subject: [PATCH] fix(server): an ACP removal reports the choice the select lands on, and the Codex plan tests enable plan mode A removal asks the select for its empty choice and compared the result against that same empty choice, so applyOptions returned nothing and the manager stored the removal as null while the session ran the Default choice. The difference is now computed against what was asked (null), so the options hold the value the session runs with. The Codex plan-mode end-to-end tests called the removed session.setFeature through an optional call and silently skipped plan mode; they apply the option and fail loudly if the method is gone. The MCP parity wrapper forwards listControls instead of the removed listFeatures. --- .../server/src/server/agent/mcp-parity.e2e.test.ts | 4 ++-- .../src/server/agent/providers/acp-agent.test.ts | 10 ++++++++-- .../server/src/server/agent/providers/acp-agent.ts | 8 +++++--- .../providers/codex-app-server-agent.local.e2e.test.ts | 2 +- .../agent/providers/codex-plan-mode.real.e2e.test.ts | 2 +- 5 files changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/server/src/server/agent/mcp-parity.e2e.test.ts b/packages/server/src/server/agent/mcp-parity.e2e.test.ts index 80d6da20a7..4b81efb607 100644 --- a/packages/server/src/server/agent/mcp-parity.e2e.test.ts +++ b/packages/server/src/server/agent/mcp-parity.e2e.test.ts @@ -186,8 +186,8 @@ function createRecordingAgentClients(): Record { if (client.listCommands) { wrappedClient.listCommands = async (config) => await client.listCommands!(config); } - if (client.listFeatures) { - wrappedClient.listFeatures = async (config) => await client.listFeatures!(config); + if (client.listControls) { + wrappedClient.listControls = async (config) => await client.listControls!(config); } if (client.listImportableSessions) { wrappedClient.listImportableSessions = async (options) => diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index a4da489c22..17834e0490 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -1959,13 +1959,19 @@ describe("ACPAgentSession options against an ACP agent", () => { ]); }); - test("a removal writes the select's empty choice and returns nothing when that took", async () => { + test("a removal writes the select's empty choice and returns it, since the options hold what the session runs", async () => { const { session, optionEvents } = await openSession({ agent: "Probe Agent" }); - await expect(session.applyOptions({ agent: null })).resolves.toEqual({}); + await expect(session.applyOptions({ agent: null })).resolves.toEqual({ agent: "" }); expect(optionEvents).toEqual([]); }); + test("a removal of a select already on its empty choice returns that choice too", async () => { + const { session } = await openSession({ agent: "" }); + + await expect(session.applyOptions({ agent: null })).resolves.toEqual({ agent: "" }); + }); + test("rejects a live auto_accept the creation schema would reject, and writes nothing", async () => { const { session, optionEvents } = await openSession({ scenario: { onWrite: "notify-then-reject" }, diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 4ca469be54..12e2e8f65b 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -2476,10 +2476,12 @@ export class ACPAgentSession implements AgentSession, ACPClient { for (const [id, value] of Object.entries(options)) { const featureOption = this.configFeatureOptions.find((candidate) => candidate.id === id); if (!featureOption) continue; - // A removal asks for the select's empty choice; a provider without one rejects it. - requested[id] = value ?? ""; + // A removal asks for the select's empty choice; a provider without one rejects it. The + // choice it lands on is reported against the `null` that was asked, so the options + // hold the value the session runs with. + requested[id] = value; const option = findSelectConfigFeatureOption(this.configOptions, featureOption); - if (!option || requested[id] !== option.currentValue) { + if (!option || (value ?? "") !== option.currentValue) { await this.setConfigFeature(id, value); } } diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.local.e2e.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.local.e2e.test.ts index dc7f920993..76c46c1d5c 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.local.e2e.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.local.e2e.test.ts @@ -386,7 +386,7 @@ describe("Codex app-server provider (local e2e)", () => { ); try { - await session.setFeature?.("plan_mode", true); + await session.applyOptions!({ plan_mode: true }); const events: AgentStreamEvent[] = []; session.subscribe((event) => { diff --git a/packages/server/src/server/agent/providers/codex-plan-mode.real.e2e.test.ts b/packages/server/src/server/agent/providers/codex-plan-mode.real.e2e.test.ts index 7c55a5f0f7..1582e6101f 100644 --- a/packages/server/src/server/agent/providers/codex-plan-mode.real.e2e.test.ts +++ b/packages/server/src/server/agent/providers/codex-plan-mode.real.e2e.test.ts @@ -40,7 +40,7 @@ describe("Codex app-server provider (real) plan mode", () => { }); try { - await session.setFeature?.("plan_mode", true); + await session.applyOptions!({ plan_mode: true }); const result = await session.run( "You are in plan mode. Produce a markdown plan with a short heading and exactly 3 bullets for implementing a login screen. Do not ask questions.",