From 4a8ed8f58869dc7156db555acd1ce0344bb89d5e Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 1 Nov 2026 21:29:07 +0100 Subject: [PATCH] fix(server): ACP option applies run one at a time Two overlapping applies shared one transaction guard, so when the first was refused after the agent had moved a select, its report was dropped as 'still in a transaction' and the second saw no difference. Applies are serialized, as OpenCode's are, so each reconciles against the state the previous one left. --- .../src/server/agent/providers/acp-agent.test.ts | 13 +++++++++++++ .../server/src/server/agent/providers/acp-agent.ts | 10 +++++++++- .../agent/providers/test-utils/fake-acp-agent.mjs | 11 +++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) 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 ae7ede3579..964c5b01e4 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -1925,6 +1925,19 @@ describe("ACPAgentSession options against an ACP agent", () => { ]); }); + test("overlapping applies that both fail still report what the agent moved", async () => { + const { session, optionEvents } = await openSession({ scenario: { onWrite: "slow-reject" } }); + + const first = session.applyOptions({ agent: "Probe Agent" }); + const second = session.applyOptions({ agent: null }); + + await expect(first).rejects.toThrow(/write refused/); + await expect(second).rejects.toThrow(/write refused/); + expect(optionEvents).toEqual([ + { type: "options_changed", provider: "copilot", patch: { agent: "Probe Agent" } }, + ]); + }); + test("a removal writes the select's empty choice and returns nothing when that took", async () => { const { session, optionEvents } = await openSession({ agent: "Probe Agent" }); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 1055a56a2c..e982ce3e40 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -1761,6 +1761,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { openOptionsPatch: Record = {}; /** Selects the provider moves while Paseo is opening or applying options belong to that result, not to events. */ private optionsTransactionDepth = 0; + private applying: Promise = Promise.resolve(); private cachedCommands: AgentSlashCommand[] = []; /** When the last command batch arrived; null while nothing has been published. */ private commandsUpdatedAt: number | null = null; @@ -2406,7 +2407,14 @@ export class ACPAgentSession implements AgentSession, ACPClient { * Provider notifications that arrive meanwhile are part of that result. When a write is * rejected, what the provider moved is reported as an event, since nothing is stored. */ - async applyOptions(options: ProviderOptions): Promise> { + /** One apply at a time: each reconciles against the state the one before it left. */ + applyOptions(options: ProviderOptions): Promise> { + const run = this.applying.then(() => this.applyOptionsNow(options)); + this.applying = run.catch(() => undefined); + return run; + } + + private async applyOptionsNow(options: ProviderOptions): Promise> { assertChangesAreLive({ provider: this.config.provider, patch: options, diff --git a/packages/server/src/server/agent/providers/test-utils/fake-acp-agent.mjs b/packages/server/src/server/agent/providers/test-utils/fake-acp-agent.mjs index e284a326da..cb6bf65e67 100644 --- a/packages/server/src/server/agent/providers/test-utils/fake-acp-agent.mjs +++ b/packages/server/src/server/agent/providers/test-utils/fake-acp-agent.mjs @@ -3,6 +3,8 @@ // answers: { [configId]: value } the agent ends up with instead of the requested value // onWrite: "notify-first" sends config_option_update before answering the write; // "notify-then-reject" does the same and then fails the write +// "slow-reject" moves the select and announces it for a non-empty value, then refuses +// after a delay; an empty value is refused without moving anything // moves: { [configId]: { [id]: value } } writes that also move another select, announced // by a config_option_update after the write is answered // adds: { [configId]: option[] } writes that make more selects appear, announced the same way @@ -28,12 +30,21 @@ export const connection = new AgentSideConnection( return { sessionId: "session-1", modes: null, models: null, configOptions: options }; }, async setSessionConfigOption({ sessionId, configId, value }) { + if (scenario.onWrite === "slow-reject" && !value) { + await new Promise((resolve) => setTimeout(resolve, 150)); + throw new Error("write refused"); + } set(configId, scenario.answers?.[configId] ?? value); const announce = () => agentConnection.sessionUpdate({ sessionId, update: { sessionUpdate: "config_option_update", configOptions: options }, }); + if (scenario.onWrite === "slow-reject") { + await announce(); + await new Promise((resolve) => setTimeout(resolve, 100)); + throw new Error("write refused"); + } if (scenario.onWrite === "notify-first" || scenario.onWrite === "notify-then-reject") { await announce(); if (scenario.onWrite === "notify-then-reject") throw new Error("write refused");