From 67e4f17dee0999784fd4cab4aee27f34b794a369 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 1 Nov 2026 09:52:06 +0100 Subject: [PATCH] fix(server): honor the opened session's capabilities, reconcile startup-only plugin settings, keep ACP resets absent A plugin session that declines session.configure although the connection offers it is reopened with its settings in the open request, so the requested setting is not lost. Opening settings the plugin settles on are compared with the agent's options for every plugin, including those that cannot be configured. An ACP provider notification that arrives before the response to a reset write no longer stores the default as an explicit override: while options are being applied, the requested options are the ones held. --- .../src/server/agent/plugin-provider.test.ts | 58 ++++++++++++++++++- .../src/server/agent/plugin-provider.ts | 42 +++++++++++--- .../src/server/agent/providers/acp-agent.ts | 16 ++++- .../agent/providers/cursor-acp-agent.test.ts | 34 +++++++++++ 4 files changed, 138 insertions(+), 12 deletions(-) diff --git a/packages/server/src/server/agent/plugin-provider.test.ts b/packages/server/src/server/agent/plugin-provider.test.ts index 07208aaedc..709035f851 100644 --- a/packages/server/src/server/agent/plugin-provider.test.ts +++ b/packages/server/src/server/agent/plugin-provider.test.ts @@ -898,7 +898,12 @@ function voiceConfig(voice: string) { * A plugin whose `voice` setting defaults to "direct". With `acceptsVoice: false` it completes a * configure request but keeps its voice. */ -function startVoicePlugin(input: { acceptsVoice: boolean; configurable?: boolean }) { +function startVoicePlugin(input: { + acceptsVoice: boolean; + configurable?: boolean; + /** The connection offers `session.configure` but each opened session leaves it out. */ + sessionDeclinesConfigure?: boolean; +}) { const capabilities = [ ...CAPABILITIES.filter((name) => input.configurable !== false || name !== "session.configure"), "session.revert.conversation", @@ -911,7 +916,9 @@ function startVoicePlugin(input: { acceptsVoice: boolean; configurable?: boolean type: "session.opened", requestId: request.requestId, sessionId: request.sessionId, - capabilities, + capabilities: input.sessionDeclinesConfigure + ? capabilities.filter((name) => name !== "session.configure") + : capabilities, restoration: "core", persistence: request.persistence ?? { version: 1, data: { token: "root" } }, cwd: request.config.cwd, @@ -919,7 +926,9 @@ function startVoicePlugin(input: { acceptsVoice: boolean; configurable?: boolean emit({ type: "session.config", sessionId: request.sessionId, - config: voiceConfig(String(request.config.settings?.voice ?? "direct")), + config: voiceConfig( + input.acceptsVoice ? String(request.config.settings?.voice ?? "direct") : "direct", + ), }); emit({ type: "session.ready", requestId: request.requestId, sessionId: request.sessionId }); return true; @@ -1783,6 +1792,49 @@ describe("PluginAgentClientRegistry", () => { } }); + test("a setting passed to a session that declines configuration goes in the open request", async () => { + const { harness, manager, registry } = startVoicePlugin({ + acceptsVoice: true, + sessionDeclinesConfigure: true, + }); + try { + const agent = await manager.createAgent( + { provider: harness.registration.id, cwd: tmpdir(), providerOptions: { voice: "soft" } }, + undefined, + { workspaceId: undefined }, + ); + + const opens = harness.inputs.flatMap((input) => + input.type === "session.open" ? [input.config.settings] : [], + ); + expect(opens.at(-1)).toEqual({ voice: "soft" }); + expect(configuredSettings(harness)).toEqual([]); + expect(toAgentPayload(manager.getAgent(agent.id)!).options).toEqual({ voice: "soft" }); + } finally { + await registry.shutdown(); + } + }); + + test("a setting a plugin without configuration normalizes while opening replaces the requested option", async () => { + const { harness, manager, registry } = startVoicePlugin({ + acceptsVoice: false, + configurable: false, + }); + try { + const agent = await manager.createAgent( + { provider: harness.registration.id, cwd: tmpdir(), providerOptions: { voice: "soft" } }, + undefined, + { workspaceId: undefined }, + ); + + const payload = toAgentPayload(manager.getAgent(agent.id)!); + expect(payload.options).toEqual({ voice: "direct" }); + expect(voiceFeature(payload)).toMatchObject({ value: "direct" }); + } finally { + await registry.shutdown(); + } + }); + test("a selection the plugin settles on while the agent is created replaces the requested option", async () => { const { harness, manager, registry } = startVoicePlugin({ acceptsVoice: false }); try { diff --git a/packages/server/src/server/agent/plugin-provider.ts b/packages/server/src/server/agent/plugin-provider.ts index d594e215e3..0150129d7f 100644 --- a/packages/server/src/server/agent/plugin-provider.ts +++ b/packages/server/src/server/agent/plugin-provider.ts @@ -1100,18 +1100,32 @@ class PluginAgentClient implements AgentClient { history: "replay" | "skip"; persist: boolean; }): Promise { - const sessionId = randomUUID(); // The plugin reports its own setting values once open, and those are what a later option // removal returns to. So a plugin that can be configured opens without setting overrides and // has them applied after. One that cannot takes them in the open request. - const configurable = (await this.runtime.connectedCapabilities()).includes("session.configure"); const openConfig = mapSessionConfig(input.config, input.launchContext, input.persist); - const bridge = await this.runtime.openSession({ - sessionId, - config: configurable ? { ...openConfig, settings: {} } : openConfig, - persistence: input.persistence, - history: input.history, - }); + const openWith = (config: ProviderSessionConfig) => + this.runtime.openSession({ + sessionId: randomUUID(), + config, + persistence: input.persistence, + history: input.history, + }); + let bridge: ProviderRuntimeSession; + let configurable = (await this.runtime.connectedCapabilities()).includes("session.configure"); + if (configurable) { + bridge = await openWith({ ...openConfig, settings: {} }); + // The session may decline what the connection offers. + if (!bridge.negotiatedCapabilities.includes("session.configure")) { + configurable = false; + if (Object.keys(openConfig.settings ?? {}).length > 0) { + await bridge.close(); + bridge = await openWith(openConfig); + } + } + } else { + bridge = await openWith(openConfig); + } const forget = () => { for (const [id, owner] of this.rootsBySession) { if (owner === session) this.rootsBySession.delete(id); @@ -1149,6 +1163,7 @@ class PluginAgentClient implements AgentClient { bind(bridge); try { if (configurable) await session.applyOptions(input.config.providerOptions ?? {}); + session.reportOpeningSettings(); } catch (error) { await session.close(); throw error; @@ -1570,6 +1585,17 @@ class PluginAgentSession implements AgentSession { * changed a setting on its own, or kept its previous value when asked for another, reports the * value that is in effect. */ + /** + * Holds back, for the manager, the opening settings the plugin settled on that differ from the + * options the agent was given. Reports that arrive after open are handled as they come. + */ + reportOpeningSettings(): void { + const patch = this.changedSettings(this.bridge.config.settings); + if (patch) { + this.optionsChangedBeforeSubscribe = { ...this.optionsChangedBeforeSubscribe, ...patch }; + } + } + private changedSettings(settings: readonly ProviderSetting[]): Record | null { const changed: Record = {}; for (const [id, value] of settingValues(settings)) { diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 7483019557..a1d5e4298f 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -1758,6 +1758,8 @@ export class ACPAgentSession implements AgentSession, ACPClient { private lastActivityAt: string | null = null; private configOptions: SessionConfigOption[] = []; private readonly configFeatureDefaults = new Map(); + /** The options a pending `applyOptions` was given, until the provider has answered every write. */ + private optionsInFlight: ProviderOptions | null = null; private cachedCommands: AgentSlashCommand[] = []; /** When the last command batch arrived; null while nothing has been published. */ private commandsUpdatedAt: number | null = null; @@ -2417,8 +2419,11 @@ export class ACPAgentSession implements AgentSession, ACPClient { */ private reportSelectionChanges(): AgentStreamEvent[] { const patch: Record = {}; + // While options are being applied, the requested options are the ones held: a key being + // removed is not an override that a provider notification could be changing. + const held = this.optionsInFlight ?? this.config.providerOptions; for (const featureOption of this.configFeatureOptions) { - const stored = this.config.providerOptions?.[featureOption.id]; + const stored = held?.[featureOption.id]; if (typeof stored !== "string") continue; const current = findSelectConfigFeatureOption( this.configOptions, @@ -2432,6 +2437,15 @@ export class ACPAgentSession implements AgentSession, ACPClient { } async applyOptions(options: ProviderOptions): Promise { + this.optionsInFlight = options; + try { + await this.applyOptionWrites(options); + } finally { + this.optionsInFlight = null; + } + } + + private async applyOptionWrites(options: ProviderOptions): Promise { // What the provider kept for each selection written here, which can differ from the request. const acknowledged: Record = {}; for (const featureOption of this.configFeatureOptions) { 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 e4e50c09ea..f2d221216d 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 @@ -455,6 +455,40 @@ describe("CursorACPAgentClient session start", () => { expect(events.filter((event) => event.type === "options_changed")).toEqual([]); }); + test("a notification that arrives before the reset response does not restore a removed option", async () => { + const notifications: Array<() => Promise> = []; + const setSessionConfigOption = vi.fn(async (request: { value: string }) => { + // The provider announces the new value before it answers the write. + if (request.value === "false") await notifications[0]?.(); + return { 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(); + notifications.push(() => + session.sessionUpdate({ + sessionId: "session-1", + update: { + sessionUpdate: "config_option_update", + configOptions: [fastConfigOption("false")], + }, + }), + ); + const events: AgentStreamEvent[] = []; + session.subscribe((event) => events.push(event)); + + await session.applyOptions({ [CURSOR_FAST_FEATURE_OPTION.id]: "true" }); + await session.applyOptions({}); + + expect(events.filter((event) => event.type === "options_changed")).toEqual([]); + }); + 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")],