From 91fc5432565fd6b66d3381d43b4295e6e33d607f Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 1 Nov 2026 10:43:05 +0100 Subject: [PATCH] fix(server): reset ACP and plugin settings only when a held option is removed, deliver ACP startup acknowledgements An options patch that never mentioned a setting reset it to its starting value whenever the provider had changed it on its own. The reset now applies only when an option the session held becomes absent, for ACP selections and plugin settings alike. An ACP provider can answer a startup write with a different value. That option change was dropped because nothing was subscribed yet; the first subscriber now receives it, so the stored option matches what the provider kept. --- .../src/server/agent/plugin-provider.test.ts | 27 ++++++++++++ .../src/server/agent/plugin-provider.ts | 11 ++++- .../src/server/agent/providers/acp-agent.ts | 22 ++++++++++ .../agent/providers/cursor-acp-agent.test.ts | 44 +++++++++++++++++++ 4 files changed, 103 insertions(+), 1 deletion(-) diff --git a/packages/server/src/server/agent/plugin-provider.test.ts b/packages/server/src/server/agent/plugin-provider.test.ts index 1b02eae00e..52cec0de75 100644 --- a/packages/server/src/server/agent/plugin-provider.test.ts +++ b/packages/server/src/server/agent/plugin-provider.test.ts @@ -1753,6 +1753,33 @@ describe("PluginAgentClientRegistry", () => { } }); + test("an options patch after a removal leaves a setting the plugin changed on its own", async () => { + const { harness, manager, registry } = startVoicePlugin({ acceptsVoice: true }); + try { + const agent = await manager.createAgent( + { provider: harness.registration.id, cwd: tmpdir() }, + undefined, + { workspaceId: undefined }, + ); + await manager.setAgentOptions(agent.id, { voice: "soft" }); + await manager.setAgentOptions(agent.id, { voice: null }); + const opened = harness.inputs.find((input) => input.type === "session.open"); + if (opened?.type !== "session.open") throw new Error("session was not opened"); + // The plugin picks a voice itself, with no option held. + harness.emit({ + type: "session.config", + sessionId: opened.sessionId, + config: voiceConfig("soft"), + }); + + await manager.setAgentOptions(agent.id, { rpcTimeoutMs: 120000 }); + + expect(configuredSettings(harness)).toEqual([{ voice: "soft" }, { voice: "direct" }]); + } finally { + await registry.shutdown(); + } + }); + test("removing an option a resumed agent held returns the setting to the plugin's default", async () => { const { harness, registry } = startVoicePlugin({ acceptsVoice: true }); try { diff --git a/packages/server/src/server/agent/plugin-provider.ts b/packages/server/src/server/agent/plugin-provider.ts index 9e54b8230b..47dd1411ad 100644 --- a/packages/server/src/server/agent/plugin-provider.ts +++ b/packages/server/src/server/agent/plugin-provider.ts @@ -1480,7 +1480,12 @@ class PluginAgentSession implements AgentSession { for (const setting of this.bridge.config.settings) { // An option removed from the agent returns the setting to the value it had before Paseo // first changed it. - const value = options[setting.id] ?? this.startingSettings.get(setting.id); + // Only an option being removed returns to the starting value; a setting the plugin changed + // with no option held is left as it is. + const removed = + !Object.hasOwn(options, setting.id) && Object.hasOwn(this.heldOptions, setting.id); + const value = + options[setting.id] ?? (removed ? this.startingSettings.get(setting.id) : undefined); if (value === undefined || value === setting.value) continue; startingValues.set(setting.id, setting.value); settings[setting.id] = toJsonValue(value, `setting '${setting.id}'`); @@ -1497,6 +1502,10 @@ class PluginAgentSession implements AgentSession { for (const [id, value] of startingValues) { if (!this.startingSettings.has(id)) this.startingSettings.set(id, value); } + // A removed option has been reset, so the next one starts from what the plugin reports then. + for (const id of this.startingSettings.keys()) { + if (!Object.hasOwn(options, id)) this.startingSettings.delete(id); + } } async revertConversation(input: { messageId: string }): Promise { diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index d9439e26a5..99f2eaf6ed 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -1760,6 +1760,8 @@ export class ACPAgentSession implements AgentSession, ACPClient { private readonly configFeatureDefaults = new Map(); /** The options a pending `applyOptions` was given, until the provider has answered every write. */ private optionsInFlight: ProviderOptions | null = null; + /** Option changes made before anything listened, delivered to the first subscriber. */ + private optionsChangedBeforeSubscribe: Record | null = null; private cachedCommands: AgentSlashCommand[] = []; /** When the last command batch arrived; null while nothing has been published. */ private commandsUpdatedAt: number | null = null; @@ -2024,6 +2026,11 @@ export class ACPAgentSession implements AgentSession, ACPClient { sessionId: this.sessionId, }); } + if (this.optionsChangedBeforeSubscribe) { + const patch = this.optionsChangedBeforeSubscribe; + this.optionsChangedBeforeSubscribe = null; + callback({ type: "options_changed", provider: this.provider, patch }); + } return () => { this.subscribers.delete(callback); }; @@ -2450,6 +2457,12 @@ export class ACPAgentSession implements AgentSession, ACPClient { const acknowledged: Record = {}; for (const featureOption of this.configFeatureOptions) { const explicit = Object.prototype.hasOwnProperty.call(options, featureOption.id); + // Only an option being removed returns to the starting value; a selection the provider + // made with no option held is left as it is. + const removed = + !explicit && + Object.prototype.hasOwnProperty.call(this.config.providerOptions ?? {}, featureOption.id); + if (!explicit && !removed) continue; const value = explicit ? options[featureOption.id] : this.configFeatureDefaults.get(featureOption.id); @@ -3462,6 +3475,15 @@ export class ACPAgentSession implements AgentSession, ACPClient { }, "provider.acp.event_emit", ); + // The provider can settle on other options while the session starts, before the manager + // listens; the first subscriber receives them. + if (event.type === "options_changed" && this.subscribers.size === 0) { + this.optionsChangedBeforeSubscribe = { + ...this.optionsChangedBeforeSubscribe, + ...event.patch, + }; + return; + } for (const subscriber of this.subscribers) { subscriber(event); } diff --git a/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts b/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts index 5770ca1fb4..dace841ff1 100644 --- a/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/cursor-acp-agent.test.ts @@ -511,6 +511,50 @@ describe("CursorACPAgentClient session start", () => { expect(events.filter((event) => event.type === "options_changed")).toEqual([]); }); + test("an options patch that never held fast leaves a selection the provider made alone", async () => { + const setSessionConfigOption = vi.fn(async (request: { value: string }) => ({ + configOptions: [fastConfigOption(request.value === "true" ? "true" : "false")], + })); + const session = createCursorSession( + { model: "composer-2.5" }, + { + currentModelId: "composer-2.5", + configOptions: [fastConfigOption("false")], + setSessionConfigOption, + }, + ); + await session.initializeNewSession(); + await session.sessionUpdate({ + sessionId: "session-1", + update: { sessionUpdate: "config_option_update", configOptions: [fastConfigOption("true")] }, + }); + + await session.applyOptions({ rpcTimeoutMs: 120000 }); + + expect(setSessionConfigOption).not.toHaveBeenCalled(); + }); + + test("an acknowledgement given while the session starts reaches the first subscriber", async () => { + const session = createCursorSession( + { model: "composer-2.5", providerOptions: { [CURSOR_FAST_FEATURE_OPTION.id]: "true" } }, + { + currentModelId: "composer-2.5", + configOptions: [fastConfigOption("false")], + setSessionConfigOption: vi.fn(async () => ({ + configOptions: [fastConfigOption("false")], + })), + }, + ); + await session.initializeNewSession(); + const events: AgentStreamEvent[] = []; + + session.subscribe((event) => events.push(event)); + + expect(events.filter((event) => event.type === "options_changed")).toEqual([ + { type: "options_changed", provider: "acp", patch: { fast: "false" } }, + ]); + }); + test("removing the Fast option returns the session to the value it started with", async () => { const setSessionConfigOption = vi.fn(async (request: { value: string }) => ({ configOptions: [fastConfigOption(request.value === "true" ? "true" : "false")],