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