From ca7a9c14ded1e8eedd61a8d85a8eef8a139de5b7 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 1 Nov 2026 09:03:26 +0100 Subject: [PATCH] fix(server): follow ACP selections changed by writes, keep removed ACP options absent, classify parents on effective options - ACP reports a select the provider changed in a mode, model or thinking write response as options_changed, the same as it does for config_option_update. - ACP applyOptions leaves the session holding exactly the options it was given, so a removed option stays absent and a later provider selection is not taken as a change to a stored one. - Unattended-parent classification reads the parent's effective options, so auto-accept supplied by provider configuration counts. --- .../agent/provider-snapshot-manager.test.ts | 62 +++++++++++++++++++ .../server/agent/provider-snapshot-manager.ts | 7 ++- .../src/server/agent/providers/acp-agent.ts | 13 ++++ .../agent/providers/cursor-acp-agent.test.ts | 58 +++++++++++++++++ 4 files changed, 139 insertions(+), 1 deletion(-) diff --git a/packages/server/src/server/agent/provider-snapshot-manager.test.ts b/packages/server/src/server/agent/provider-snapshot-manager.test.ts index cd30b81909..286232ff49 100644 --- a/packages/server/src/server/agent/provider-snapshot-manager.test.ts +++ b/packages/server/src/server/agent/provider-snapshot-manager.test.ts @@ -1363,6 +1363,68 @@ describe("ProviderSnapshotManager public surface", () => { } }); + test("resolveCreateConfig classifies a parent on its effective options", async () => { + const resolverInputs: ResolveAgentCreateConfigInput[] = []; + const manager = new ProviderSnapshotManager({ + logger: createTestLogger(), + providerOverrides: { + copilot: { enabled: false }, + opencode: { enabled: false }, + pi: { enabled: false }, + }, + extraClients: { + codex: createExtraClient("codex", { + async isAvailable() { + return true; + }, + async fetchCatalog() { + return { models: [] as AgentModelDefinition[], modes: [] }; + }, + resolveCreateConfig(input) { + resolverInputs.push(input); + return { modeId: undefined, providerOptions: undefined }; + }, + }), + claude: createExtraClient("claude", { + async isAvailable() { + return true; + }, + async fetchCatalog() { + return { models: [] as AgentModelDefinition[], modes: [] }; + }, + isCreateConfigUnattended(input) { + return input.config.providerOptions?.auto_accept === true; + }, + }), + }, + }); + try { + // The auto-accept comes from provider configuration, not from the agent's own options. + const parent: ManagedAgent = { + ...CLOSED_PARENT, + id: "parent-agent", + provider: "claude", + currentModeId: "default", + config: { provider: "claude", cwd: "/tmp/project" }, + effectiveOptions: { auto_accept: true }, + }; + + await manager.resolveCreateConfig({ + cwd: "/tmp/project", + provider: "codex", + requestedMode: undefined, + providerOptions: undefined, + parent, + unattended: false, + }); + + expect(resolverInputs[0]).toMatchObject({ parent: { isUnattended: true }, unattended: true }); + expect(parent.config.providerOptions).toBeUndefined(); + } finally { + manager.destroy(); + } + }); + test("resolveCreateConfig passes explicit unattended intent to provider policy", async () => { const resolverInputs: ResolveAgentCreateConfigInput[] = []; const modes: AgentMode[] = [{ id: "worker", label: "Worker", isUnattended: true }]; diff --git a/packages/server/src/server/agent/provider-snapshot-manager.ts b/packages/server/src/server/agent/provider-snapshot-manager.ts index 7590806407..7e728713be 100644 --- a/packages/server/src/server/agent/provider-snapshot-manager.ts +++ b/packages/server/src/server/agent/provider-snapshot-manager.ts @@ -744,7 +744,12 @@ export class ProviderSnapshotManager { modeId: parent.currentModeId, isUnattended: definition.isCreateConfigUnattended({ modeId: parent.currentModeId, - config: parent.config, + // Provider defaults from `config.json` apply to the parent too, so classify on the + // effective options while the stored overrides stay as they are. + config: { + ...parent.config, + providerOptions: parent.effectiveOptions ?? parent.config.providerOptions, + }, controls: parent.controls, availableModes: parent.availableModes ?? definition.modes ?? [], }), diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index f13061a207..7483019557 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -2209,6 +2209,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { requestedValue: modeId, label: "mode", }); + for (const event of this.reportSelectionChanges()) this.pushEvent(event); this.availableModes = deriveModesFromACP(this.defaultModes, null, this.configOptions).modes; this.pushEvent({ type: "mode_changed", @@ -2334,6 +2335,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { requestedValue: modelId, label: "model", }); + for (const event of this.reportSelectionChanges()) this.pushEvent(event); this.pushEvent({ type: "model_changed", provider: this.provider, @@ -2385,6 +2387,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { requestedValue: thinkingOptionId, label: "thought-level", }); + for (const event of this.reportSelectionChanges()) this.pushEvent(event); this.pushEvent({ type: "thinking_option_changed", provider: this.provider, @@ -2429,6 +2432,8 @@ export class ACPAgentSession implements AgentSession, ACPClient { } async applyOptions(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) { const explicit = Object.prototype.hasOwnProperty.call(options, featureOption.id); const value = explicit @@ -2442,12 +2447,20 @@ export class ACPAgentSession implements AgentSession, ACPClient { continue; } await this.setConfigFeature(featureOption.id, value); + acknowledged[featureOption.id] = this.config.providerOptions?.[featureOption.id]; } // Last, so a rejected selection leaves permission handling as it was. await this.setConfigFeature( ACP_AUTO_ACCEPT_FEATURE_ID, options[ACP_AUTO_ACCEPT_FEATURE_ID] === true, ); + // The session holds the options it was given, so a key the agent no longer has stays absent + // and a later provider report is not mistaken for a change to a stored override. + const next: ProviderOptions = { ...options }; + for (const id of Object.keys(acknowledged)) { + if (Object.prototype.hasOwnProperty.call(options, id)) next[id] = acknowledged[id]; + } + this.config.providerOptions = next; } private async setConfigFeature(featureId: string, value: unknown): Promise { 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 d149960394..e4e50c09ea 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 @@ -397,6 +397,64 @@ describe("CursorACPAgentClient session start", () => { }); }); + test("a selection a model or thinking write changes replaces the stored option", async () => { + const reasoning = (currentValue: string): SessionConfigOption => ({ + ...KIMI_K3_CONFIG_OPTIONS[0]!, + currentValue, + }); + const session = createCursorSession( + { model: "composer-2.5", providerOptions: { [CURSOR_FAST_FEATURE_OPTION.id]: "true" } }, + { + currentModelId: "composer-2.5", + configOptions: [reasoning("max"), fastConfigOption("true")], + // Changing the reasoning level also turns Fast off, with no separate notification. + setSessionConfigOption: async (request) => ({ + configOptions: [ + reasoning(request.value === "low" ? "low" : "max"), + fastConfigOption(request.value === "low" ? "false" : "true"), + ], + }), + }, + ); + await session.initializeNewSession(); + const events: AgentStreamEvent[] = []; + session.subscribe((event) => events.push(event)); + + await session.setThinkingOption("low"); + + expect(events).toContainEqual({ + type: "options_changed", + provider: "acp", + patch: { [CURSOR_FAST_FEATURE_OPTION.id]: "false" }, + }); + }); + + test("an option removed from the agent stays absent when the provider later selects it again", 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(); + const events: AgentStreamEvent[] = []; + session.subscribe((event) => events.push(event)); + + await session.applyOptions({ [CURSOR_FAST_FEATURE_OPTION.id]: "true" }); + await session.applyOptions({}); + await session.sessionUpdate({ + sessionId: "session-1", + update: { sessionUpdate: "config_option_update", configOptions: [fastConfigOption("true")] }, + }); + + 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")],