From 70a01a5c38b43095ee6ffc21baf689aa21258fc1 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 1 Nov 2026 12:43:27 +0100 Subject: [PATCH] fix(server): keep removed options out of live apply, keep Auto Accept inheritance, honor dropped featureValues - setAgentOptions hands the running provider the agent's own options instead of the config.json defaults merged in, so a removed key is absent and the provider keeps its setting. Defaults apply when the session is next opened. - ACP and OpenCode classify a parent as unattended from the live Auto Accept control when the stored option was removed. - A create hook that omits or unsets featureValues removes every key it was sent. --- .../src/server/agent/agent-manager.test.ts | 28 ++++++++++++++++--- .../server/src/server/agent/agent-manager.ts | 8 ++++-- .../src/server/agent/option-controls.ts | 28 +++++++++++++++---- .../agent/provider-snapshot-manager.test.ts | 20 +++++++++++++ .../server/agent/providers/acp-agent.test.ts | 19 +++++++++++++ .../src/server/agent/providers/acp-agent.ts | 3 +- .../server/agent/providers/opencode-agent.ts | 3 +- 7 files changed, 95 insertions(+), 14 deletions(-) diff --git a/packages/server/src/server/agent/agent-manager.test.ts b/packages/server/src/server/agent/agent-manager.test.ts index 5b1f1e6d90..347954a787 100644 --- a/packages/server/src/server/agent/agent-manager.test.ts +++ b/packages/server/src/server/agent/agent-manager.test.ts @@ -12778,10 +12778,16 @@ describe("agent options", () => { await manager.setAgentOptions(agentId, { service_tier: null }); - expect(client.session?.applied).toEqual([{ service_tier: "default" }]); + // The provider is handed the agent's own options, so the removed key is absent and the + // running provider keeps its setting; the default applies when the session is next opened. + expect(client.session?.applied).toEqual([{}]); const agent = manager.getAgent(agentId); expect(agent?.config.providerOptions).toBeUndefined(); expect(agent?.effectiveOptions).toEqual({ service_tier: "default" }); + + // An unrelated patch afterwards still leaves the removed key to the running provider. + await manager.setAgentOptions(agentId, { plan_mode: true }); + expect(client.session?.applied).toEqual([{}, { plan_mode: true }]); }); test("the snapshot reports the configured options and resolves aliases through controls", async () => { @@ -12830,9 +12836,7 @@ describe("agent options", () => { await manager.setAgentOptions(agentId, { sandbox_mode: "read-only" }); - expect(client.session?.applied).toEqual([ - { service_tier: "default", sandbox_mode: "read-only" }, - ]); + expect(client.session?.applied).toEqual([{ sandbox_mode: "read-only" }]); const agent = manager.getAgent(agentId); expect(agent?.config.providerOptions).toEqual({ sandbox_mode: "read-only" }); expect(toAgentPayload(agent!).options).toEqual({ @@ -13002,6 +13006,22 @@ describe("agent options", () => { expect(options).toEqual({}); }); + test("agent.create.before hook omitting featureValues leaves the agent without options", async () => { + const options = await optionsAfterCreateHook((config) => { + const { featureValues: _omitted, ...rest } = config; + return rest; + }); + expect(options).toEqual({}); + }); + + test("agent.create.before hook returning undefined featureValues leaves the agent without options", async () => { + const options = await optionsAfterCreateHook((config) => ({ + ...config, + featureValues: undefined, + })); + expect(options).toEqual({}); + }); + test("agent.create.before hook deleting one featureValues key keeps the others", async () => { const options = await optionsAfterCreateHook((config) => ({ ...config, diff --git a/packages/server/src/server/agent/agent-manager.ts b/packages/server/src/server/agent/agent-manager.ts index aa1e5a5f80..f82eae0665 100644 --- a/packages/server/src/server/agent/agent-manager.ts +++ b/packages/server/src/server/agent/agent-manager.ts @@ -2273,8 +2273,8 @@ export class AgentManager { } /** - * Patches the agent's own options and hands the provider the resulting effective options. The - * provider applies what it can live and reads the rest at next open; a provider that rejects a + * Patches the agent's own options and hands the provider the result. The provider applies what + * it can live and reads the rest, with the `config.json` defaults, at next open; a provider that rejects a * value leaves the stored options unchanged. */ async setAgentOptions(agentId: string, patch: ProviderOptionsPatch): Promise { @@ -2290,7 +2290,9 @@ export class AgentManager { // a rejection leaves the record, its snapshots and its stored copy as they were. this.pendingOptionCandidates.set(agentId, candidate); try { - await agent.session.applyOptions(this.effectiveOptionsFor(agent.provider, candidate)); + // Only the agent's own options: a removed key is absent, so the running provider keeps + // its setting. `config.json` defaults apply when the session is next opened. + await agent.session.applyOptions(candidate); } finally { // Events the provider queued before answering must reach the candidate, not the record. await this.drainSessionEvents(agentId); diff --git a/packages/server/src/server/agent/option-controls.ts b/packages/server/src/server/agent/option-controls.ts index b689c595b4..931e6b9480 100644 --- a/packages/server/src/server/agent/option-controls.ts +++ b/packages/server/src/server/agent/option-controls.ts @@ -57,7 +57,8 @@ export function foldFeatureValues< * A plugin written against the previous SDK reads and edits `config.featureValues`, so a * hook sees the config's options there. The options the hook returns are the result: a key * it removed from `providerOptions` stays removed. Keys it added, changed, or deleted in - * `featureValues` (compared with what it was sent) are applied on top. + * `featureValues` (compared with what it was sent, and an absent field counts as empty) are + * applied on top. */ export function exposeFeatureValuesToHook< T extends { providerOptions?: ProviderOptions | null; featureValues?: Record }, @@ -75,11 +76,13 @@ export function foldHookFeatureValues< const options: ProviderOptions = mirrored ? { ...rest.providerOptions } : { ...featureValues, ...rest.providerOptions }; - if (mirrored && featureValues) { + if (mirrored) { + // A hook that dropped the whole field removed every key it was sent. + const after = featureValues ?? {}; const before = sent.featureValues ?? {}; - for (const key of new Set([...Object.keys(before), ...Object.keys(featureValues)])) { - if (isDeepStrictEqual(featureValues[key], before[key])) continue; - if (key in featureValues) options[key] = featureValues[key]; + for (const key of new Set([...Object.keys(before), ...Object.keys(after)])) { + if (isDeepStrictEqual(after[key], before[key])) continue; + if (key in after) options[key] = after[key]; else delete options[key]; } } @@ -145,6 +148,21 @@ function describe(control: { description?: string; tooltip?: string; icon?: stri }; } +/** + * Whether a toggle is on: the effective option, else the control's own value (what the running + * provider shows after the option was removed). + */ +export function isToggleOn( + key: string, + options: ProviderOptions | undefined, + controls: readonly AgentOptionControl[] | undefined, +): boolean { + const option = options?.[key]; + if (typeof option === "boolean") return option; + const control = controls?.find((candidate) => candidate.key === key); + return control?.type === "toggle" && control.defaultValue === true; +} + function featureFromControl(control: AgentOptionControl, effective: ProviderOptions): AgentFeature { if (control.type === "toggle") { const value = effective[control.key]; 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 286232ff49..777d531b3e 100644 --- a/packages/server/src/server/agent/provider-snapshot-manager.test.ts +++ b/packages/server/src/server/agent/provider-snapshot-manager.test.ts @@ -1538,6 +1538,26 @@ describe("ProviderSnapshotManager public surface", () => { }); }); +test("an OpenCode parent whose auto accept option was removed but is still on is unattended", async () => { + const openCode = new OpenCodeAgentClient(createTestLogger()); + const parent: ManagedAgent = { + ...CLOSED_PARENT, + id: "parent-agent", + provider: "opencode", + config: { provider: "opencode", cwd: "/tmp/project" }, + controls: [{ type: "toggle", key: "auto_accept", label: "Auto Accept", defaultValue: true }], + }; + + expect( + openCode.isCreateConfigUnattended({ + modeId: parent.currentModeId, + config: parent.config, + controls: parent.controls, + availableModes: [], + }), + ).toBe(true); +}); + describe("ProviderSnapshotManager applyMutableProviderConfig", () => { test("adds a derived provider and includes it in subsequent reads", async () => { const manager = new ProviderSnapshotManager({ diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index f4804a8ada..f39cbd1938 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -2126,6 +2126,25 @@ describe("ACPAgentClient config features", () => { ).toBe(true); }); + test("an ACP session whose Auto Accept option was removed but is still on stays unattended", () => { + const client = new ACPAgentClient({ + provider: "generic-acp", + logger: createTestLogger(), + defaultCommand: ["generic-acp", "acp"], + }); + + expect( + client.isCreateConfigUnattended({ + modeId: null, + config: { provider: "generic-acp", cwd: "/tmp/acp-features", providerOptions: {} }, + controls: [ + { type: "toggle", key: "auto_accept", label: "Auto Accept", defaultValue: true }, + ], + availableModes: [], + }), + ).toBe(true); + }); + test("exposes Auto Accept for every ACP provider without starting a probe", async () => { const client = new ACPAgentClient({ provider: "generic-acp", diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 43dae2c98f..cb68b0107e 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -112,6 +112,7 @@ import { isDefaultAgentCreateConfigUnattended, resolveDefaultAgentCreateConfig, } from "../create-agent-mode.js"; +import { isToggleOn } from "../option-controls.js"; import { importSessionFromPersistence } from "../provider-session-import.js"; import { checkProviderLaunchAvailable, @@ -917,7 +918,7 @@ function resolveACPCreateConfig( function isACPCreateConfigUnattended(input: AgentCreateConfigUnattendedInput): boolean { return ( isDefaultAgentCreateConfigUnattended(input) || - input.config.providerOptions?.[ACP_AUTO_ACCEPT_FEATURE_ID] === true + isToggleOn(ACP_AUTO_ACCEPT_FEATURE_ID, input.config.providerOptions, input.controls) ); } diff --git a/packages/server/src/server/agent/providers/opencode-agent.ts b/packages/server/src/server/agent/providers/opencode-agent.ts index b2f12ec2c6..409e121454 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.ts @@ -60,6 +60,7 @@ import { type ToolCallDetail, type ToolCallTimelineItem, } from "../agent-sdk-types.js"; +import { isToggleOn } from "../option-controls.js"; import { importSessionFromPersistence } from "../provider-session-import.js"; import { raceProviderRefreshAbort, @@ -287,7 +288,7 @@ function isOpenCodeCreateConfigUnattended( ): boolean { return ( isDefaultAgentCreateConfigUnattended(input) || - input.config.providerOptions?.[OPENCODE_AUTO_ACCEPT_FEATURE_ID] === true + isToggleOn(OPENCODE_AUTO_ACCEPT_FEATURE_ID, input.config.providerOptions, input.controls) ); }