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")],