From c51767af9b6efbee3fbd7789057c7adf0b620081 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 1 Nov 2026 12:31:37 +0100 Subject: [PATCH] refactor(server): removing an option leaves a running provider on its current setting Removing an option from a running agent drops the key from its stored and snapshot options, but providers no longer revert to a starting value. The provider default applies from the next open, and each control's default value shows the live setting. This deletes the reset-to-baseline machinery in the plugin, ACP, OMP, Claude, Codex and OpenCode sessions and keeps the options_changed reconciliation. Also fixes two review findings: - ACP startup overrides are reported once after all startup writes, so an intermediate selection change no longer overwrites a requested option. - A create hook that removes providerOptions or featureValues keys keeps them removed instead of having the unchanged featureValues mirror restore them. --- .../src/server/agent/agent-manager.test.ts | 58 +++++ .../src/server/agent/option-controls.ts | 29 ++- .../src/server/agent/plugin-provider.test.ts | 239 ++---------------- .../src/server/agent/plugin-provider.ts | 71 +----- .../src/server/agent/providers/acp-agent.ts | 102 +++----- .../agent/providers/claude/agent.test.ts | 32 ++- .../server/agent/providers/claude/agent.ts | 7 +- .../providers/claude/feature-definitions.ts | 9 +- .../codex-app-server-agent.features.test.ts | 29 ++- .../agent/providers/codex-app-server-agent.ts | 9 +- .../providers/codex-feature-definitions.ts | 18 +- .../agent/providers/cursor-acp-agent.test.ts | 85 +++---- .../server/agent/providers/omp/agent.test.ts | 11 +- .../src/server/agent/providers/omp/agent.ts | 27 +- .../opencode-agent.full-access.test.ts | 8 +- .../server/agent/providers/opencode-agent.ts | 9 +- .../providers/opencode/v2/configuration.ts | 14 +- .../agent/providers/opencode/v2/session.ts | 9 +- 18 files changed, 280 insertions(+), 486 deletions(-) diff --git a/packages/server/src/server/agent/agent-manager.test.ts b/packages/server/src/server/agent/agent-manager.test.ts index ccc1b41f79..5b1f1e6d90 100644 --- a/packages/server/src/server/agent/agent-manager.test.ts +++ b/packages/server/src/server/agent/agent-manager.test.ts @@ -12952,6 +12952,64 @@ describe("agent options", () => { }); }); + async function optionsAfterCreateHook( + edit: (config: AgentSessionConfig) => AgentSessionConfig, + ): Promise { + const workdir = mkdtempSync(join(tmpdir(), "agent-manager-hook-")); + const storage = new AgentStorage(join(workdir, "agents"), logger); + const manager = new AgentManager({ + clients: { codex: new OptionsClient() }, + registry: storage, + logger, + pluginLifecycle: { + emit: () => {}, + before: async (name, request) => + name === "agent.create.before" ? { ...request, config: edit(request.config) } : request, + }, + }); + const agent = await manager.createAgent( + { + provider: "codex", + cwd: workdir, + providerOptions: { fast_mode: true, plan_mode: true }, + }, + undefined, + { workspaceId: undefined }, + ); + return manager.getAgent(agent.id)?.config.providerOptions; + } + + function withoutFastMode(values: Record | null | undefined) { + const { fast_mode: _removed, ...rest } = values ?? {}; + return rest; + } + + test("agent.create.before hook clearing providerOptions leaves the agent without options", async () => { + const options = await optionsAfterCreateHook((config) => ({ ...config, providerOptions: {} })); + expect(options).toEqual({}); + }); + + test("agent.create.before hook deleting one providerOptions key keeps the others", async () => { + const options = await optionsAfterCreateHook((config) => ({ + ...config, + providerOptions: withoutFastMode(config.providerOptions), + })); + expect(options).toEqual({ plan_mode: true }); + }); + + test("agent.create.before hook clearing featureValues leaves the agent without options", async () => { + const options = await optionsAfterCreateHook((config) => ({ ...config, featureValues: {} })); + expect(options).toEqual({}); + }); + + test("agent.create.before hook deleting one featureValues key keeps the others", async () => { + const options = await optionsAfterCreateHook((config) => ({ + ...config, + featureValues: withoutFastMode(config.featureValues), + })); + expect(options).toEqual({ plan_mode: true }); + }); + test("overlapping patches to different keys both reach the provider and the stored record", async () => { let release: () => void = () => {}; const firstApplyHeld = new Promise((resolve) => { diff --git a/packages/server/src/server/agent/option-controls.ts b/packages/server/src/server/agent/option-controls.ts index b0ed1305fa..b689c595b4 100644 --- a/packages/server/src/server/agent/option-controls.ts +++ b/packages/server/src/server/agent/option-controls.ts @@ -7,8 +7,9 @@ import type { } from "@getpaseo/protocol/agent-types"; /** - * A patch over an agent's own options: top-level keys replace, `null` removes the key so the - * `config.json` default (or the provider's own default) applies again. + * A patch over an agent's own options: top-level keys replace, `null` removes the key. Removal + * does not revert a running provider: it keeps its current setting, and the `config.json` + * default (or the provider's own default) applies from the next open. */ export type ProviderOptionsPatch = Record; @@ -54,8 +55,9 @@ export function foldFeatureValues< /** * COMPAT(featureValues): added in v0.15.0, remove after 2027-05-01. * A plugin written against the previous SDK reads and edits `config.featureValues`, so a - * hook sees the config's options there. Keys the hook changed come back as options and - * override what the config already held; keys it left alone do not. + * 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. */ export function exposeFeatureValuesToHook< T extends { providerOptions?: ProviderOptions | null; featureValues?: Record }, @@ -68,13 +70,20 @@ export function foldHookFeatureValues< T extends { providerOptions?: ProviderOptions | null; featureValues?: Record }, >(returned: T, sent: T): Omit { const { featureValues, ...rest } = returned; - if (!featureValues) return rest; - const before = sent.featureValues ?? {}; - const edits: Record = {}; - for (const [key, value] of Object.entries(featureValues)) { - if (!isDeepStrictEqual(value, before[key])) edits[key] = value; + const mirrored = sent.providerOptions !== undefined && sent.featureValues !== undefined; + // Without a mirror the hook's `featureValues` are all new options, as for any other client. + const options: ProviderOptions = mirrored + ? { ...rest.providerOptions } + : { ...featureValues, ...rest.providerOptions }; + if (mirrored && 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]; + else delete options[key]; + } } - return { ...rest, providerOptions: { ...featureValues, ...rest.providerOptions, ...edits } }; + return { ...rest, providerOptions: options }; } interface ProfileOptionFields { diff --git a/packages/server/src/server/agent/plugin-provider.test.ts b/packages/server/src/server/agent/plugin-provider.test.ts index 52cec0de75..4e55f10055 100644 --- a/packages/server/src/server/agent/plugin-provider.test.ts +++ b/packages/server/src/server/agent/plugin-provider.test.ts @@ -898,17 +898,10 @@ 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; - /** 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", - ]; - let nativeSessions = 0; +function startVoicePlugin(input: { acceptsVoice: boolean; configurable?: boolean }) { + const capabilities = CAPABILITIES.filter( + (name) => input.configurable !== false || name !== "session.configure", + ); const harness = createProviderHarness({ capabilities, async handleInput(request, emit) { @@ -917,14 +910,9 @@ function startVoicePlugin(input: { type: "session.opened", requestId: request.requestId, sessionId: request.sessionId, - capabilities: input.sessionDeclinesConfigure - ? capabilities.filter((name) => name !== "session.configure") - : capabilities, + capabilities, restoration: "core", - persistence: request.persistence ?? { - version: 1, - data: { token: `native-${++nativeSessions}` }, - }, + persistence: { version: 1, data: { token: "root" } }, cwd: request.config.cwd, }); emit({ @@ -937,31 +925,6 @@ function startVoicePlugin(input: { emit({ type: "session.ready", requestId: request.requestId, sessionId: request.sessionId }); return true; } - if (request.type === "session.prompt") { - if (request.prompt.input.type !== "message") throw new Error("Expected a message"); - const text = request.prompt.input.content - .flatMap((part) => (part.type === "text" ? [part.text] : [])) - .join(""); - const turnId = `turn-${text}`; - emit({ - type: "session.prompt_result", - sessionId: request.sessionId, - clientMessageId: request.prompt.clientMessageId, - result: { type: "turn", turnId }, - }); - emit({ type: "session.turn", sessionId: request.sessionId, turnId, state: "started" }); - emit({ - type: "timeline.item", - sessionId: request.sessionId, - item: { type: "user_message", id: `user-${text}`, text, revertToken: text }, - }); - emit({ type: "session.turn", sessionId: request.sessionId, turnId, state: "completed" }); - return true; - } - if (request.type === "session.revert") { - emit({ type: "request.completed", requestId: request.requestId }); - return true; - } if (request.type !== "session.configure") return false; const requested = request.changes.settings?.voice; emit({ @@ -1666,142 +1629,6 @@ describe("PluginAgentClientRegistry", () => { } }); - test("removing an option returns the plugin's setting to the value it started with", async () => { - const voices = [ - { label: "Direct", value: "direct" }, - { label: "Soft", value: "soft" }, - ]; - const harness = createProviderHarness({ - async handleInput(input, emit) { - if (input.type !== "session.configure") return false; - const voice = input.changes.settings?.voice ?? "direct"; - emit({ - type: "session.config", - sessionId: input.sessionId, - config: { - model: "plugin-model", - mode: "build", - models: [{ id: "plugin-model", label: "Plugin model" }], - modes: [{ id: "build", label: "Build" }], - thinkingOptions: [], - settings: [ - { - type: "select", - id: "voice", - label: "Voice", - value: String(voice), - options: voices, - }, - ], - }, - }); - emit({ type: "request.completed", requestId: input.requestId }); - return true; - }, - }); - const logger = createTestLogger(); - const registry = new PluginAgentClientRegistry(logger); - registry.replace([harness.registration]); - const manager = new AgentManager({ - logger, - clients: registry.clients(), - providerDefinitions: registry.definitions(), - }); - try { - const agent = await manager.createAgent( - { provider: harness.registration.id, cwd: tmpdir() }, - undefined, - { workspaceId: undefined }, - ); - const configured = () => - harness.inputs.flatMap((input) => - input.type === "session.configure" ? [input.changes.settings] : [], - ); - - await manager.setAgentOptions(agent.id, { voice: "soft" }); - await manager.setAgentOptions(agent.id, { voice: null }); - - expect(configured()).toEqual([{ voice: "soft" }, { voice: "direct" }]); - const payload = toAgentPayload(manager.getAgent(agent.id)!); - expect(payload.options).toEqual({}); - expect(payload.features?.find((feature) => feature.id === "voice")).toMatchObject({ - value: "direct", - }); - } finally { - await registry.shutdown(); - } - }); - - test("removing an option the agent was created with returns the setting to the plugin's default", async () => { - const { harness, manager, registry } = startVoicePlugin({ acceptsVoice: true }); - try { - const agent = await manager.createAgent( - { provider: harness.registration.id, cwd: tmpdir(), providerOptions: { voice: "soft" } }, - undefined, - { workspaceId: undefined }, - ); - expect(toAgentPayload(manager.getAgent(agent.id)!).options).toEqual({ voice: "soft" }); - - await manager.setAgentOptions(agent.id, { voice: null }); - - expect(configuredSettings(harness)).toEqual([{ voice: "soft" }, { voice: "direct" }]); - const payload = toAgentPayload(manager.getAgent(agent.id)!); - expect(payload.options).toEqual({}); - expect(voiceFeature(payload)).toMatchObject({ value: "direct" }); - } finally { - await registry.shutdown(); - } - }); - - 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 { - const client = registry.clients()[harness.registration.id]!; - const first = await client.createSession({ - provider: harness.registration.id, - cwd: tmpdir(), - }); - const resumed = await client.resumeSession(first.describePersistence()!, { - cwd: tmpdir(), - providerOptions: { voice: "soft" }, - }); - - await resumed.applyOptions!({}); - - expect(configuredSettings(harness)).toEqual([{ voice: "soft" }, { voice: "direct" }]); - await resumed.close(); - } finally { - await registry.shutdown(); - } - }); - test("a setting passed to a plugin that cannot be configured goes in the open request", async () => { const { harness, manager, registry } = startVoicePlugin({ acceptsVoice: true, @@ -1823,34 +1650,6 @@ 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" }); - // The second open continues the native session the first one created. - const persistence = harness.inputs.flatMap((input) => - input.type === "session.open" ? [input.persistence] : [], - ); - expect(persistence).toEqual([undefined, { version: 1, data: { token: "native-1" } }]); - 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, @@ -1888,23 +1687,21 @@ describe("PluginAgentClientRegistry", () => { } }); - test("removing an option after a conversation rewind returns the setting to the plugin's default", async () => { - const { harness, registry } = startVoicePlugin({ acceptsVoice: true }); + test("removing an option leaves the plugin's setting as it is", async () => { + const { harness, manager, registry } = startVoicePlugin({ acceptsVoice: true }); try { - const client = registry.clients()[harness.registration.id]!; - const session = await client.createSession({ - provider: harness.registration.id, - cwd: tmpdir(), - providerOptions: { voice: "soft" }, - }); - await session.run("A"); - await session.run("B"); - await session.revertConversation!({ messageId: "user-B" }); + const agent = await manager.createAgent( + { provider: harness.registration.id, cwd: tmpdir(), providerOptions: { voice: "soft" } }, + undefined, + { workspaceId: undefined }, + ); - await session.applyOptions!({}); + await manager.setAgentOptions(agent.id, { voice: null }); - expect(configuredSettings(harness)).toEqual([{ voice: "soft" }, { voice: "direct" }]); - await session.close(); + const payload = toAgentPayload(manager.getAgent(agent.id)!); + expect(payload.options).toEqual({}); + expect(configuredSettings(harness)).toEqual([]); + expect(voiceFeature(payload)).toMatchObject({ value: "soft" }); } finally { await registry.shutdown(); } diff --git a/packages/server/src/server/agent/plugin-provider.ts b/packages/server/src/server/agent/plugin-provider.ts index 47dd1411ad..4c9a418d73 100644 --- a/packages/server/src/server/agent/plugin-provider.ts +++ b/packages/server/src/server/agent/plugin-provider.ts @@ -195,11 +195,6 @@ class ProviderRuntime { return this.connection?.capabilities ?? []; } - /** The capabilities of the connection, connecting first if it is not open yet. */ - async connectedCapabilities(): Promise { - return (await this.getConnection()).capabilities; - } - get isClosed(): boolean { return this.closed; } @@ -1100,34 +1095,12 @@ class PluginAgentClient implements AgentClient { history: "replay" | "skip"; persist: boolean; }): Promise { - // 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 openConfig = mapSessionConfig(input.config, input.launchContext, input.persist); - const openWith = (config: ProviderSessionConfig, persistence = input.persistence) => - this.runtime.openSession({ - sessionId: randomUUID(), - config, - 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) { - // Reopen the native session the first open created rather than creating another. - const created = input.persistence ?? bridge.persistence ?? undefined; - await bridge.close(); - bridge = await openWith(openConfig, created); - } - } - } else { - bridge = await openWith(openConfig); - } + const bridge = await this.runtime.openSession({ + sessionId: randomUUID(), + config: mapSessionConfig(input.config, input.launchContext, input.persist), + persistence: input.persistence, + history: input.history, + }); const forget = () => { for (const [id, owner] of this.rootsBySession) { if (owner === session) this.rootsBySession.delete(id); @@ -1163,13 +1136,7 @@ class PluginAgentClient implements AgentClient { input.config.providerOptions ?? {}, ); bind(bridge); - try { - if (configurable) await session.applyOptions(input.config.providerOptions ?? {}); - session.reportOpeningSettings(); - } catch (error) { - await session.close(); - throw error; - } + session.reportOpeningSettings(); return session; } @@ -1246,11 +1213,6 @@ class PluginAgentSession implements AgentSession { closed: boolean; } | null = null; private replaying: Promise | null = null; - /** - * Each setting's value before Paseo first changed it, for an option that is later removed. - * It belongs to the logical session, so it survives the session being reopened after a rewind. - */ - private startingSettings = new Map(); /** Option changes the plugin made before anything listened, delivered to the first subscriber. */ private optionsChangedBeforeSubscribe: Record | null = null; @@ -1475,19 +1437,11 @@ class PluginAgentSession implements AgentSession { async applyOptions(options: ProviderOptions): Promise { this.assertReplayComplete(); + // A setting the options leave out stays as the plugin has it. const settings: Record = {}; - const startingValues = new Map(); 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. - // 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); + const value = options[setting.id]; if (value === undefined || value === setting.value) continue; - startingValues.set(setting.id, setting.value); settings[setting.id] = toJsonValue(value, `setting '${setting.id}'`); } // Held before the request, because the plugin's config report arrives while it is pending. @@ -1499,13 +1453,6 @@ class PluginAgentSession implements AgentSession { this.heldOptions = previous; throw error; } - 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 99f2eaf6ed..43dae2c98f 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -67,6 +67,7 @@ import { type AgentClient, type AgentCreateConfigUnattendedInput, type AgentOptionControl, + type AgentOptionToggleControl, type AgentLaunchContext, type AgentMetadata, type AgentMode, @@ -883,7 +884,7 @@ function isACPAutoAcceptEnabled(config: AgentSessionConfig): boolean { return config.providerOptions?.[ACP_AUTO_ACCEPT_FEATURE_ID] === true; } -const ACP_AUTO_ACCEPT_CONTROL: AgentOptionControl = { +const ACP_AUTO_ACCEPT_CONTROL: AgentOptionToggleControl = { type: "toggle", key: ACP_AUTO_ACCEPT_FEATURE_ID, label: "Auto Accept", @@ -1757,9 +1758,10 @@ export class ACPAgentSession implements AgentSession, ACPClient { private currentTitle: string | null = null; 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; + /** Whether permission prompts are approved. Removing the option leaves it as it is. */ + private autoAcceptEnabled: boolean; + /** True while startup overrides are written, whose intermediate values are not provider changes. */ + private applyingStartupOverrides = false; /** Option changes made before anything listened, delivered to the first subscriber. */ private optionsChangedBeforeSubscribe: Record | null = null; private cachedCommands: AgentSlashCommand[] = []; @@ -1781,6 +1783,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { constructor(config: AgentSessionConfig, options: ACPAgentSessionOptions) { this.provider = options.provider; + this.autoAcceptEnabled = isACPAutoAcceptEnabled(config); this.terminateProcess = options.terminateProcess ?? terminateWithTreeKill; this.capabilities = options.capabilities; this.logger = options.logger.child({ module: "agent", provider: options.provider }); @@ -2062,7 +2065,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { get controls(): AgentOptionControl[] { return [ - ACP_AUTO_ACCEPT_CONTROL, + { ...ACP_AUTO_ACCEPT_CONTROL, defaultValue: this.autoAcceptEnabled }, ...deriveControlsFromACP(this.configOptions, this.configFeatureOptions), ]; } @@ -2404,31 +2407,15 @@ export class ACPAgentSession implements AgentSession, ACPClient { }); } - /** - * The value a select reported before Paseo wrote to it. An option removed from the agent's - * options returns the select to this value. - */ - private recordConfigFeatureDefaults(): void { - for (const featureOption of this.configFeatureOptions) { - if (this.configFeatureDefaults.has(featureOption.id)) continue; - const current = findSelectConfigFeatureOption( - this.configOptions, - featureOption, - )?.currentValue; - if (current != null) this.configFeatureDefaults.set(featureOption.id, current); - } - } - /** * When the provider reports a different value for a select the agent holds an explicit option * for, the stored option follows the report. Selections with no stored option are shown through * the control's default instead. */ private reportSelectionChanges(): AgentStreamEvent[] { + if (this.applyingStartupOverrides) return []; 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; + const held = this.config.providerOptions; for (const featureOption of this.configFeatureOptions) { const stored = held?.[featureOption.id]; if (typeof stored !== "string") continue; @@ -2444,50 +2431,24 @@ 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. + // A key the options leave out is not written: the running provider keeps its selection. 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); - if (value === undefined) continue; + if (!Object.prototype.hasOwnProperty.call(options, featureOption.id)) continue; + const value = options[featureOption.id]; const option = findSelectConfigFeatureOption(this.configOptions, featureOption); if (option && value === option.currentValue) continue; - if (!explicit && (!option || !findSelectConfigChoice({ option, value: String(value) }))) { - // A default recorded under a previous model may not exist on the current one. - 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, - ); + if (Object.prototype.hasOwnProperty.call(options, ACP_AUTO_ACCEPT_FEATURE_ID)) { + this.autoAcceptEnabled = 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; + this.config.providerOptions = { ...options, ...acknowledged }; } private async setConfigFeature(featureId: string, value: unknown): Promise { @@ -2495,14 +2456,6 @@ export class ACPAgentSession implements AgentSession, ACPClient { throw new Error("ACP session not initialized"); } - if (featureId === ACP_AUTO_ACCEPT_FEATURE_ID) { - this.config.providerOptions = { - ...this.config.providerOptions, - [ACP_AUTO_ACCEPT_FEATURE_ID]: value === true, - }; - return; - } - const featureOption = this.configFeatureOptions.find((option) => option.id === featureId); if (!featureOption) { throw new Error(`Unknown ${this.provider} feature: ${featureId}`); @@ -2536,11 +2489,8 @@ export class ACPAgentSession implements AgentSession, ACPClient { requestedValue, label: featureOption.label, }); - // A write that only returns an absent option to its default has no stored option to correct. - const held = this.optionsInFlight ?? this.config.providerOptions; - const isHeld = Object.prototype.hasOwnProperty.call(held ?? {}, featureId); this.config.providerOptions = { ...this.config.providerOptions, [featureId]: currentValue }; - if (isHeld && currentValue !== requestedValue) { + if (currentValue !== requestedValue) { // The provider kept a different selection than the one asked for. this.pushEvent({ type: "options_changed", @@ -2718,8 +2668,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { } async requestPermission(params: RequestPermissionRequest): Promise { - const canAutoAccept = - isACPAutoAcceptEnabled(this.config) && !isACPChooserRequest(params.options); + const canAutoAccept = this.autoAcceptEnabled && !isACPChooserRequest(params.options); if (canAutoAccept) { const allowOption = selectPermissionOption(params.options, { behavior: "allow" }); if (allowOption) { @@ -3070,7 +3019,6 @@ export class ACPAgentSession implements AgentSession, ACPClient { : response; this.configOptions = this.transformConfigOptions(transformed.configOptions ?? []); - this.recordConfigFeatureDefaults(); const modeInfo = deriveModesFromACP(this.defaultModes, transformed.modes, this.configOptions); this.availableModes = modeInfo.modes; @@ -3094,6 +3042,18 @@ export class ACPAgentSession implements AgentSession, ACPClient { } private async applyConfiguredOverrides(): Promise { + // Mode, model and thinking writes move other selections through values that are not final + // yet; the requested options are reconciled once everything has been written. + this.applyingStartupOverrides = true; + try { + await this.writeConfiguredOverrides(); + } finally { + this.applyingStartupOverrides = false; + } + for (const event of this.reportSelectionChanges()) this.pushEvent(event); + } + + private async writeConfiguredOverrides(): Promise { const configuredModeId = this.config.modeId; if (configuredModeId && configuredModeId !== this.currentMode) { const selection = resolveACPModeSelection({ diff --git a/packages/server/src/server/agent/providers/claude/agent.test.ts b/packages/server/src/server/agent/providers/claude/agent.test.ts index d1fa82f2c2..ecfc29cd30 100644 --- a/packages/server/src/server/agent/providers/claude/agent.test.ts +++ b/packages/server/src/server/agent/providers/claude/agent.test.ts @@ -1555,11 +1555,35 @@ describe("ClaudeAgentSession features", () => { ).resolves.toBeUndefined(); expect(queryMock.applyFlagSettings).not.toHaveBeenCalled(); - // Turning Fast on for a model without it is still refused. + // Removing the option keeps the preference as it is; nothing is sent to the runtime. await expect(session.applyOptions?.({ allowedTools: ["Read"] })).resolves.toBeUndefined(); - await expect( - session.applyOptions?.({ fast_mode: true, allowedTools: ["Read"] }), - ).rejects.toThrow("Claude fast mode is not available for model 'claude-sonnet-5'"); + expect(queryMock.applyFlagSettings).not.toHaveBeenCalled(); + + await session.close(); + }); + + test("removing the fast_mode option leaves Fast on and the control shows it", async () => { + const { queryFactory, queryMock } = createQueryMock(); + const client = new ClaudeAgentClient({ + logger, + queryFactory, + resolveBinary: async () => "/test/claude/bin", + }); + const session = await client.createSession({ + provider: "claude", + cwd: process.cwd(), + model: "claude-opus-4-8", + }); + await (session as unknown as { ensureQuery(): Promise }).ensureQuery(); + await session.applyOptions?.({ fast_mode: true }); + queryMock.applyFlagSettings.mockClear(); + + await session.applyOptions?.({}); + + expect(queryMock.applyFlagSettings).not.toHaveBeenCalled(); + expect(session.controls).toEqual([ + expect.objectContaining({ key: "fast_mode", defaultValue: true }), + ]); await session.close(); }); diff --git a/packages/server/src/server/agent/providers/claude/agent.ts b/packages/server/src/server/agent/providers/claude/agent.ts index bc779c489f..de70858db5 100644 --- a/packages/server/src/server/agent/providers/claude/agent.ts +++ b/packages/server/src/server/agent/providers/claude/agent.ts @@ -2444,7 +2444,7 @@ class ClaudeAgentSession implements AgentSession { } get controls(): AgentOptionControl[] { - return buildClaudeControls(this.config.model); + return buildClaudeControls(this.config.model, this.config.providerOptions.fast_mode === true); } async getRuntimeInfo(): Promise { @@ -2800,8 +2800,9 @@ class ClaudeAgentSession implements AgentSession { async applyOptions(options: ProviderOptions): Promise { const providerOptions = validateProviderOptions("claude", ClaudeProviderOptionsSchema, options) ?? {}; - const wantsFast = providerOptions.fast_mode === true; const hadFast = this.config.providerOptions.fast_mode === true; + // An absent option leaves Fast as it is; only an explicit value changes it. + const wantsFast = providerOptions.fast_mode === undefined ? hadFast : providerOptions.fast_mode; const modelSupportsFast = claudeModelSupportsFastMode(this.config.model); if (wantsFast && !hadFast && !modelSupportsFast) { throw new Error( @@ -2816,7 +2817,7 @@ class ClaudeAgentSession implements AgentSession { this.cachedRuntimeInfo = null; } // Held until the runtime accepts, so a rejected change is retried instead of skipped. - this.config.providerOptions = providerOptions; + this.config.providerOptions = { ...providerOptions, ...(wantsFast ? { fast_mode: true } : {}) }; } getPendingPermissions(): AgentPermissionRequest[] { diff --git a/packages/server/src/server/agent/providers/claude/feature-definitions.ts b/packages/server/src/server/agent/providers/claude/feature-definitions.ts index 0606860745..afd8435145 100644 --- a/packages/server/src/server/agent/providers/claude/feature-definitions.ts +++ b/packages/server/src/server/agent/providers/claude/feature-definitions.ts @@ -14,6 +14,11 @@ export function claudeModelSupportsFastMode(modelId: string | null | undefined): return claudeManifestModelSupportsFastMode(modelId); } -export function buildClaudeControls(modelId: string | null | undefined): AgentOptionControl[] { - return claudeModelSupportsFastMode(modelId) ? [{ ...CLAUDE_FAST_MODE_CONTROL }] : []; +export function buildClaudeControls( + modelId: string | null | undefined, + fastEnabled = false, +): AgentOptionControl[] { + return claudeModelSupportsFastMode(modelId) + ? [{ ...CLAUDE_FAST_MODE_CONTROL, defaultValue: fastEnabled }] + : []; } diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.features.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.features.test.ts index df0c689bac..be3af8332f 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.features.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.features.test.ts @@ -61,14 +61,15 @@ function speedControl(defaultValue = "default") { }; } -const PLAN_CONTROL = { +const planControl = (defaultValue = false) => ({ type: "toggle", + defaultValue, key: "plan_mode", label: "Plan", description: "Switch Codex into planning-only collaboration mode", tooltip: "Toggle plan mode", icon: "list-todo", -}; +}); const TEST_COLLABORATION_MODES: CollaborationModeRecord[] = [ { @@ -419,7 +420,7 @@ describe("Codex app-server provider controls", () => { test("controls return the speed selector and plan toggle when supported", async () => { const { session, appServer } = await createConnectedSession(); - expect(session.controls).toEqual([speedControl(), PLAN_CONTROL]); + expect(session.controls).toEqual([speedControl(), planControl()]); await session.applyOptions?.({ service_tier: "priority", plan_mode: true }); await session.startTurn("hello"); @@ -433,7 +434,21 @@ describe("Codex app-server provider controls", () => { test("controls return only the plan toggle when model does not support fast mode", async () => { const { session } = await createConnectedSession({ model: "gpt-3.5-turbo" }); - expect(session.controls).toEqual([PLAN_CONTROL]); + expect(session.controls).toEqual([planControl()]); + }); + + test("removing the speed and plan options leaves the session as it is", async () => { + const { session, appServer } = await createConnectedSession(); + await session.applyOptions?.({ service_tier: "priority", plan_mode: true }); + + await session.applyOptions?.({}); + + expect(session.controls).toEqual([speedControl("priority"), planControl(true)]); + await session.startTurn("hello"); + await expect(appServer.waitForTurnStart()).resolves.toMatchObject({ + serviceTier: "priority", + collaborationMode: expect.objectContaining({ mode: "plan" }), + }); }); test("constructor ignores restored fast mode when model does not support it", async () => { @@ -442,7 +457,7 @@ describe("Codex app-server provider controls", () => { providerOptions: { fast_mode: true }, }); - expect(session.controls).toEqual([PLAN_CONTROL]); + expect(session.controls).toEqual([planControl()]); await session.startTurn("hello"); await expect(appServer.waitForTurnStart()).resolves.toMatchObject({ @@ -501,7 +516,7 @@ describe("Codex app-server provider controls", () => { providerOptions: { fast_mode: true, plan_mode: true }, }); - expect(session.controls).toEqual([speedControl("priority"), PLAN_CONTROL]); + expect(session.controls).toEqual([speedControl("priority"), planControl(true)]); await session.startTurn("hello"); await expect(appServer.waitForTurnStart()).resolves.toMatchObject({ @@ -750,7 +765,7 @@ describe("Codex app-server provider controls", () => { await session.applyOptions?.({ service_tier: "priority" }); await session.setModel("gpt-3.5-turbo"); - expect(session.controls).toEqual([PLAN_CONTROL]); + expect(session.controls).toEqual([planControl()]); await session.startTurn("hello"); await expect(appServer.waitForTurnStart()).resolves.toMatchObject({ diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index 5454bf4b97..4e4e63d046 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -3589,7 +3589,8 @@ export class CodexAppServerAgentSession implements AgentSession { get controls(): AgentOptionControl[] { return buildCodexControls({ serviceTiers: this.currentServiceTiers(), - options: this.providerOptions, + runningServiceTier: this.requestedServiceTier, + planModeEnabled: this.planModeEnabled, planModeAvailable: this.hasPlanCollaborationMode(), }); } @@ -4659,7 +4660,9 @@ export class CodexAppServerAgentSession implements AgentSession { async applyOptions(options: ProviderOptions): Promise { const next = validateProviderOptions("codex", CodexProviderOptionsSchema, options) ?? {}; - const requestedTier = readCodexServiceTier(next); + // An absent speed or plan option leaves the session as it is. + const speedGiven = next.service_tier !== undefined || next.fast_mode !== undefined; + const requestedTier = speedGiven ? readCodexServiceTier(next) : this.requestedServiceTier; if ( requestedTier !== this.requestedServiceTier && !this.isServiceTierAvailable(requestedTier) @@ -4670,7 +4673,7 @@ export class CodexAppServerAgentSession implements AgentSession { } this.providerOptions = next; this.requestedServiceTier = requestedTier; - this.setPlanMode(next.plan_mode === true); + this.setPlanMode(next.plan_mode === undefined ? this.planModeEnabled : next.plan_mode === true); this.cachedRuntimeInfo = null; } diff --git a/packages/server/src/server/agent/providers/codex-feature-definitions.ts b/packages/server/src/server/agent/providers/codex-feature-definitions.ts index 9ce0e11999..fe198bcb94 100644 --- a/packages/server/src/server/agent/providers/codex-feature-definitions.ts +++ b/packages/server/src/server/agent/providers/codex-feature-definitions.ts @@ -9,13 +9,11 @@ export const CodexServiceTierSchema = z.object({ }); export type CodexServiceTier = z.infer; -// Which speed applies, for the runtime and for this control: an explicit `service_tier` wins; -// the pre-v0.10 `fast_mode: true` boolean counts only while `service_tier` is absent -// (`readCodexServiceTier`). The control shows it as its default, because a client selects the -// option's value when `service_tier` is set and the default when it is not. +// The control shows the speed the session is running as its default: a client selects the +// option's value when `service_tier` is set and the running speed when it is not. function buildCodexSpeedControl( tiers: CodexServiceTier[], - options: Record | undefined, + runningTier: string | null, ): AgentOptionControl[] { if (tiers.length === 0) return []; const choices = [ @@ -27,9 +25,8 @@ function buildCodexSpeedControl( ...(tier.id !== "fast" && tier.name.toLowerCase() === "fast" ? { aliases: ["fast"] } : {}), })), ]; - const implied = options?.service_tier === undefined ? readCodexServiceTier(options) : null; const defaultChoice = choices.find( - (choice) => choice.value === implied || choice.aliases?.includes(implied ?? ""), + (choice) => choice.value === runningTier || choice.aliases?.includes(runningTier ?? ""), ); return [ { @@ -57,12 +54,13 @@ export const CODEX_PLAN_MODE_CONTROL: AgentOptionToggleControl = { export function buildCodexControls(input: { serviceTiers: CodexServiceTier[]; - options?: Record; + runningServiceTier: string | null; + planModeEnabled: boolean; planModeAvailable?: boolean; }): AgentOptionControl[] { - const controls = buildCodexSpeedControl(input.serviceTiers, input.options); + const controls = buildCodexSpeedControl(input.serviceTiers, input.runningServiceTier); if (input.planModeAvailable !== false) { - controls.push({ ...CODEX_PLAN_MODE_CONTROL }); + controls.push({ ...CODEX_PLAN_MODE_CONTROL, defaultValue: input.planModeEnabled }); } return controls; } 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 dace841ff1..87849b6d49 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 @@ -328,7 +328,7 @@ describe("CursorACPAgentClient session start", () => { ); }); - test("removing auto_accept stops answering permission requests on its own", async () => { + test("removing auto_accept keeps answering permission requests and the control shows it", async () => { const session = createCursorSession({ model: "composer-2.5" }); await session.initializeNewSession(); const request = { @@ -338,13 +338,14 @@ describe("CursorACPAgentClient session start", () => { } as const; await session.applyOptions({ auto_accept: true }); + await session.applyOptions({}); + await expect(session.requestPermission({ ...request })).resolves.toEqual({ outcome: { outcome: "selected", optionId: "allow-always" }, }); - - await session.applyOptions({}); - void session.requestPermission({ ...request }); - await vi.waitFor(() => expect(session.getPendingPermissions()).toHaveLength(1)); + expect(session.controls).toContainEqual( + expect.objectContaining({ key: "auto_accept", defaultValue: true }), + ); }); test("a rejected options update leaves permission handling as it was", async () => { @@ -455,60 +456,43 @@ 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")] }; + test("startup writes keep the requested Fast option while a thinking change settles", async () => { + const state = { reasoning: "max", fast: "false" }; + const setSessionConfigOption = vi.fn(async (request: { configId: string; value: string }) => { + if (request.configId === "reasoning") state.reasoning = request.value; + if (request.configId === "fast") state.fast = request.value; + return { + configOptions: [ + { ...KIMI_K3_CONFIG_OPTIONS[0], currentValue: state.reasoning } as SessionConfigOption, + fastConfigOption(state.fast as "true" | "false"), + ], + }; }); const session = createCursorSession( - { model: "composer-2.5" }, + { + model: "composer-2.5", + thinkingOptionId: "low", + providerOptions: { [CURSOR_FAST_FEATURE_OPTION.id]: "true" }, + }, { currentModelId: "composer-2.5", - configOptions: [fastConfigOption("false")], + configOptions: [KIMI_K3_CONFIG_OPTIONS[0]!, 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("a reset response that keeps the previous value does not store the removed option", async () => { - const setSessionConfigOption = vi.fn(async () => ({ - configOptions: [fastConfigOption("true")], - })); - 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({}); - + expect(setSessionConfigOption.mock.calls.map(([request]) => request.value)).toEqual([ + "low", + "true", + ]); expect(events.filter((event) => event.type === "options_changed")).toEqual([]); + expect(session.controls).toContainEqual( + expect.objectContaining({ key: "fast", defaultValue: "true" }), + ); }); test("an options patch that never held fast leaves a selection the provider made alone", async () => { @@ -555,7 +539,7 @@ describe("CursorACPAgentClient session start", () => { ]); }); - test("removing the Fast option returns the session to the value it started with", async () => { + test("removing the Fast option leaves the provider on its current selection", async () => { const setSessionConfigOption = vi.fn(async (request: { value: string }) => ({ configOptions: [fastConfigOption(request.value === "true" ? "true" : "false")], })); @@ -573,12 +557,9 @@ describe("CursorACPAgentClient session start", () => { // `{ fast: null }` with no config.json default reaches the provider as no key at all. await session.applyOptions({}); - expect(setSessionConfigOption.mock.calls.map(([request]) => request.value)).toEqual([ - "true", - "false", - ]); + expect(setSessionConfigOption.mock.calls.map(([request]) => request.value)).toEqual(["true"]); expect(session.controls).toContainEqual( - expect.objectContaining({ key: "fast", defaultValue: "false" }), + expect.objectContaining({ key: "fast", defaultValue: "true" }), ); }); diff --git a/packages/server/src/server/agent/providers/omp/agent.test.ts b/packages/server/src/server/agent/providers/omp/agent.test.ts index 96a7ab0d0c..f80e159fe2 100644 --- a/packages/server/src/server/agent/providers/omp/agent.test.ts +++ b/packages/server/src/server/agent/providers/omp/agent.test.ts @@ -638,25 +638,20 @@ describe("OMP agent client and session", () => { ]); }); - test("removing the fast_mode option restores the runtime's own setting", async () => { + test("removing the fast_mode option leaves the runtime on its current setting", async () => { const omp = new OmpHarness(); await omp.start(); const session = omp.requireSession(); - expect(omp.runtime().state.fastModeEnabled).toBe(false); omp.runtime().fastModeResult = { enabled: true, active: true }; await session.applyOptions?.({ fast_mode: true }); expect(omp.runtime().setFastModeRequests).toEqual([true]); // `{ fast_mode: null }` with no config.json default reaches the provider as no key at all. - omp.runtime().fastModeResult = { enabled: false, active: false }; await session.applyOptions?.({}); - expect(omp.runtime().setFastModeRequests).toEqual([true, false]); - expect(session.controls).toEqual([expect.objectContaining({ defaultValue: false })]); - // A later change to an unrelated option leaves the runtime alone. - await session.applyOptions?.({}); - expect(omp.runtime().setFastModeRequests).toEqual([true, false]); + expect(omp.runtime().setFastModeRequests).toEqual([true]); + expect(session.controls).toEqual([expect.objectContaining({ defaultValue: true })]); }); test("an options patch that never held fast_mode leaves the runtime's Fast setting alone", async () => { diff --git a/packages/server/src/server/agent/providers/omp/agent.ts b/packages/server/src/server/agent/providers/omp/agent.ts index 8080e68769..61a7ba88e2 100644 --- a/packages/server/src/server/agent/providers/omp/agent.ts +++ b/packages/server/src/server/agent/providers/omp/agent.ts @@ -203,8 +203,6 @@ interface OmpAgentSessionOptions { }>; config: AgentSessionConfig; initialState: OmpSessionState; - /** Fast mode as the runtime reported it before any option was applied; removal restores it. */ - fastModeBaseline?: boolean; currentModeId?: string | null; logger: Logger; providerIdleScheduler?: OmpProviderIdleScheduler; @@ -503,10 +501,6 @@ function isOmpSteerTransportFailure(error: unknown): boolean { ); } -function fastModeBaselineOf(state: OmpSessionState): boolean | undefined { - return typeof state.fastModeEnabled === "boolean" ? state.fastModeEnabled : undefined; -} - function hasOmpFastMode(state: OmpSessionState): boolean { return typeof state.fastModeEnabled === "boolean" && typeof state.fastModeActive === "boolean"; } @@ -757,7 +751,6 @@ export class OmpAgentSession implements AgentSession { this.restartRuntime = options.restartRuntime; this.config = options.config; this.state = options.initialState; - this.fastModeBaseline = options.fastModeBaseline; this.currentModeId = options.currentModeId ?? null; this.logger = options.logger; this.live = options.live ?? true; @@ -834,13 +827,10 @@ export class OmpAgentSession implements AgentSession { if (requested !== undefined && typeof requested !== "boolean") { throw new Error("OMP fast mode requires a boolean"); } - // Removing a held option returns Fast to the runtime's own setting at session start; an - // option that was already absent leaves whatever the runtime reports alone. - const removed = requested === undefined && this.config.providerOptions?.fast_mode !== undefined; - const target = removed ? this.fastModeBaseline : requested; - if (target !== undefined && target !== this.state.fastModeEnabled) { + // An absent option leaves Fast as the runtime has it. + if (requested !== undefined && requested !== this.state.fastModeEnabled) { if (!hasOmpFastMode(this.state)) throw new Error("OMP fast mode is unavailable"); - const result = await this.runtimeSession.setFastMode(target); + const result = await this.runtimeSession.setFastMode(requested); this.state = { ...this.state, fastModeEnabled: result.enabled, @@ -852,8 +842,6 @@ export class OmpAgentSession implements AgentSession { requested === undefined ? others : { ...others, fast_mode: requested }; } - private readonly fastModeBaseline: boolean | undefined; - async run(prompt: AgentPromptInput, options?: AgentRunOptions): Promise { return runProviderTurn({ prompt, @@ -2367,10 +2355,12 @@ export class OmpAgentClient implements AgentClient { config, startInput.env, ); - const startState = await runtimeSession.getState(); - const initialState = await this.restoreFastMode(runtimeSession, config, startState); + const initialState = await this.restoreFastMode( + runtimeSession, + config, + await runtimeSession.getState(), + ); return new OmpAgentSession({ - fastModeBaseline: fastModeBaselineOf(startState), runtimeSession, hostTools, restartRuntime: this.buildRestartRuntime(startInput, config, launchContext), @@ -2442,7 +2432,6 @@ export class OmpAgentClient implements AgentClient { }; const initialState = await this.restoreFastMode(runtimeSession, config, resumedState); return new OmpAgentSession({ - fastModeBaseline: fastModeBaselineOf(resumedState), runtimeSession, hostTools, restartRuntime: this.buildRestartRuntime(startInput, config, launchContext), diff --git a/packages/server/src/server/agent/providers/opencode-agent.full-access.test.ts b/packages/server/src/server/agent/providers/opencode-agent.full-access.test.ts index b7601bb3bb..b90bd6581f 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.full-access.test.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.full-access.test.ts @@ -290,7 +290,7 @@ describe("OpenCode auto_accept feature", () => { await session.close(); }); - test("stops auto-approving once auto_accept is removed from the options", async () => { + test("removing auto_accept keeps auto-approving and the control shows it", async () => { const { openCodeClient, runtime } = mockOpenCodeClient({ events: [toolPermissionEvent(), idleEvent()], }); @@ -309,9 +309,9 @@ describe("OpenCode auto_accept feature", () => { await session.applyOptions?.({}); await session.run("Run verification"); - expect(openCodeClient.calls.permissionReply).toEqual([]); - expect(session.getPendingPermissions()).toEqual([ - expect.objectContaining({ id: "permission-1" }), + expect(openCodeClient.calls.permissionReply).toHaveLength(1); + expect(session.controls).toEqual([ + expect.objectContaining({ key: "auto_accept", defaultValue: true }), ]); await session.close(); diff --git a/packages/server/src/server/agent/providers/opencode-agent.ts b/packages/server/src/server/agent/providers/opencode-agent.ts index ea541f4542..b2f12ec2c6 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.ts @@ -27,6 +27,7 @@ import { type AgentClient, type AgentCreateSessionOptions, type AgentOptionControl, + type AgentOptionToggleControl, type AgentLaunchContext, type AgentMode, type AgentModelDefinition, @@ -290,7 +291,7 @@ function isOpenCodeCreateConfigUnattended( ); } -const OPENCODE_AUTO_ACCEPT_CONTROL: AgentOptionControl = { +const OPENCODE_AUTO_ACCEPT_CONTROL: AgentOptionToggleControl = { type: "toggle", key: OPENCODE_AUTO_ACCEPT_FEATURE_ID, label: "Auto Accept", @@ -3557,7 +3558,7 @@ class OpenCodeAgentSession implements AgentSession { } get controls(): AgentOptionControl[] { - return [OPENCODE_AUTO_ACCEPT_CONTROL]; + return [{ ...OPENCODE_AUTO_ACCEPT_CONTROL, defaultValue: this.autoAcceptEnabled }]; } async getRuntimeInfo(): Promise { @@ -4964,7 +4965,9 @@ class OpenCodeAgentSession implements AgentSession { } async applyOptions(options: ProviderOptions): Promise { - this.setAutoAccept(options[OPENCODE_AUTO_ACCEPT_FEATURE_ID] === true); + // An absent option leaves auto-accept as it is. + const requested = options[OPENCODE_AUTO_ACCEPT_FEATURE_ID]; + if (requested !== undefined) this.setAutoAccept(requested === true); } private setAutoAccept(enabled: boolean): void { diff --git a/packages/server/src/server/agent/providers/opencode/v2/configuration.ts b/packages/server/src/server/agent/providers/opencode/v2/configuration.ts index ac41063b8c..ddfe613148 100644 --- a/packages/server/src/server/agent/providers/opencode/v2/configuration.ts +++ b/packages/server/src/server/agent/providers/opencode/v2/configuration.ts @@ -4,6 +4,7 @@ import type { V2Api } from "./api.js"; import type { AgentLaunchContext, AgentOptionControl, + AgentOptionToggleControl, AgentSessionConfig, } from "../../../agent-sdk-types.js"; @@ -11,9 +12,16 @@ import { OpenCodeProviderOptionsSchema, buildOpenCodePermissionRules } from "../ import { modelRef } from "./mapping.js"; -export const controls: AgentOptionControl[] = [ - { type: "toggle", key: "auto_accept", label: "Auto-accept" }, -]; +const autoAcceptControl: AgentOptionToggleControl = { + type: "toggle", + key: "auto_accept", + label: "Auto-accept", +}; +export const controls: AgentOptionControl[] = [autoAcceptControl]; +/** The controls of a running session, showing whether auto-accept is on. */ +export function controlsFor(autoAccept: boolean): AgentOptionControl[] { + return [{ ...autoAcceptControl, defaultValue: autoAccept }]; +} export function permissionRules(config: AgentSessionConfig): PermissionRuleset { const rules = buildOpenCodePermissionRules( diff --git a/packages/server/src/server/agent/providers/opencode/v2/session.ts b/packages/server/src/server/agent/providers/opencode/v2/session.ts index 4bb3a2877f..0ba987c5f6 100644 --- a/packages/server/src/server/agent/providers/opencode/v2/session.ts +++ b/packages/server/src/server/agent/providers/opencode/v2/session.ts @@ -32,7 +32,7 @@ import { type V2Connection } from "./runtime.js"; import { modelRef, modesFromV2 } from "./mapping.js"; import { V2_CAPABILITIES } from "./capabilities.js"; -import { controls } from "./configuration.js"; +import { controlsFor } from "./configuration.js"; import { commands } from "./commands.js"; import { messages } from "./history.js"; import { SessionPermissions } from "./permissions.js"; @@ -120,7 +120,7 @@ export class OpenCodeV2Session implements AgentSession { return this.info.id; } get controls() { - return controls; + return controlsFor(this.config.providerOptions?.auto_accept === true); } private get client() { return this.connection.client; @@ -360,8 +360,9 @@ export class OpenCodeV2Session implements AgentSession { this.emit({ type: "thinking_option_changed", provider: "opencode", thinkingOptionId: variant }); } async applyOptions(options: ProviderOptions) { - // An absent key is the default: approvals are asked for. - const value = options["auto_accept"] ?? false; + // An absent key leaves auto-accept as it is. + const value = options["auto_accept"]; + if (value === undefined) return; if (typeof value !== "boolean") throw new Error("OpenCode auto_accept requires a boolean"); this.config.providerOptions = { ...this.config.providerOptions, auto_accept: value }; if (value && !this.config.toolPolicy)