diff --git a/docs/providers.md b/docs/providers.md index ae57827b51..4674f431a6 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -36,18 +36,18 @@ for native keys and examples. option and read `options`; they hold no value of their own. Any option can be read and set with or without a control. -- **Create and resume.** The provider opens with the options it is given and reports nothing - about them while opening. If the session ended up different, it says so once afterwards, as an - `options_changed` event. Events emitted before the daemon subscribes are dropped, so emit that - report from `subscribe`. -- **Live apply.** `applyOptions` receives only the keys the patch set. The daemon stores the patch - after the call resolves, onto the agent's options as they are at that moment, so the last write - wins. Throwing rejects the whole patch. A result that differs from the request is reported - afterwards, not from inside the call. +- **Create and resume.** The provider opens with the options it is given and emits nothing about + them while opening. If the session ended up different, it sets `openOptionsPatch` before + `createSession`/`resumeSession` resolves. The daemon stores it before the agent is published. +- **Live apply.** `applyOptions` receives the keys the patch touches, resolved against the + `config.json` defaults. A removal arrives as the default the key falls back to, or `null` when + there is none; the provider returns the setting to its own default if it can, or throws to + reject. It returns the options it ended up with when they differ from the request. The daemon + stores the patch and that result in one step after the call resolves, onto the agent's options + as they are at that moment, so the last write wins. Throwing rejects the whole patch and + nothing is stored. Emit nothing from inside the call. - **Provider-initiated changes.** A setting the provider changes on its own is an - `options_changed`. The daemon stores and forwards it and does not push the old value back. -- **Removal.** Removing an option drops the key and sends the provider nothing. The session - keeps its current setting until the next open. + `options_changed` event. The daemon stores and forwards it and does not push the old value back. Exact MCP preapproval is a separate daemon-owned contract. A new provider must fail closed for Hub unattended execution until it can approve one exact injected MCP diff --git a/packages/server/src/server/agent/agent-manager.test.ts b/packages/server/src/server/agent/agent-manager.test.ts index 3e79764aff..47b4c5eea4 100644 --- a/packages/server/src/server/agent/agent-manager.test.ts +++ b/packages/server/src/server/agent/agent-manager.test.ts @@ -12715,16 +12715,21 @@ describe("agent options", () => { readonly applied: ProviderOptions[] = []; rejectWith: Error | null = null; applyGate: Promise | null = null; + /** What the session says it ended up with after the next apply. */ + ended: Record = {}; + openOptionsPatch: Record = {}; - async applyOptions(options: ProviderOptions): Promise { + async applyOptions(options: ProviderOptions): Promise> { if (this.rejectWith) throw this.rejectWith; await this.applyGate; this.applied.push(options); + return this.ended; } } class OptionsClient extends TestAgentClient { session: OptionsSession | null = null; + openOptionsPatch: Record = {}; // Stands in for the `config.json` provider defaults the registry wrapper merges in. effectiveOptions(own: ProviderOptions | undefined): ProviderOptions { @@ -12733,14 +12738,19 @@ describe("agent options", () => { override async createSession(config: AgentSessionConfig): Promise { this.session = new OptionsSession(config); + this.session.openOptionsPatch = this.openOptionsPatch; return this.session; } } - async function createOptionsAgent(providerOptions?: ProviderOptions) { + async function createOptionsAgent( + providerOptions?: ProviderOptions, + openOptionsPatch: Record = {}, + ) { const workdir = mkdtempSync(join(tmpdir(), "agent-manager-options-")); const storage = new AgentStorage(join(workdir, "agents"), logger); const client = new OptionsClient(); + client.openOptionsPatch = openOptionsPatch; const manager = new AgentManager({ clients: { codex: client }, registry: storage, logger }); const agent = await manager.createAgent( { provider: "codex", cwd: workdir, ...(providerOptions ? { providerOptions } : {}) }, @@ -12775,17 +12785,74 @@ describe("agent options", () => { }); }); - test("setAgentOptions removes a key with null and sends the provider nothing", async () => { + test("setAgentOptions sends a removal to the provider like any change, as the default it falls back to", async () => { const { manager, client, agentId } = await createOptionsAgent({ service_tier: "fast" }); await manager.setAgentOptions(agentId, { service_tier: null }); - expect(client.session?.applied).toEqual([]); + expect(client.session?.applied).toEqual([{ service_tier: "default" }]); const agent = manager.getAgent(agentId); expect(agent?.config.providerOptions).toBeUndefined(); expect(agent?.effectiveOptions).toEqual({ service_tier: "default" }); }); + test("a removal with no default is sent as null", async () => { + const { manager, client, agentId } = await createOptionsAgent({ fast_mode: true }); + + await manager.setAgentOptions(agentId, { fast_mode: null }); + + expect(client.session?.applied).toEqual([{ fast_mode: null }]); + expect(manager.getAgent(agentId)?.config.providerOptions).toBeUndefined(); + }); + + test("a value the provider kept through a removal is stored", async () => { + const { manager, client, agentId } = await createOptionsAgent({ fast_mode: true }); + client.session!.ended = { fast_mode: true }; + + await manager.setAgentOptions(agentId, { fast_mode: null }); + + const agent = manager.getAgent(agentId); + expect(agent?.config.providerOptions).toEqual({ fast_mode: true }); + expect(agent?.effectiveOptions).toEqual({ service_tier: "default", fast_mode: true }); + }); + + test("what the provider ended up with is stored together with the patch", async () => { + const { manager, client, agentId } = await createOptionsAgent(); + client.session!.ended = { service_tier: "slow" }; + + await manager.setAgentOptions(agentId, { service_tier: "fast", plan_mode: true }); + + expect(manager.getAgent(agentId)?.config.providerOptions).toEqual({ + service_tier: "slow", + plan_mode: true, + }); + }); + + test("the difference a session opened with is stored before the agent is published", async () => { + const workdir = mkdtempSync(join(tmpdir(), "agent-manager-options-")); + const client = new OptionsClient(); + client.openOptionsPatch = { service_tier: "slow", fast_mode: null }; + const manager = new AgentManager({ + clients: { codex: client }, + registry: new AgentStorage(join(workdir, "agents"), logger), + logger, + }); + const published: Array = []; + manager.subscribe((event) => { + if (event.type === "agent_state") published.push(event.agent.config.providerOptions); + }); + + const agent = await manager.createAgent( + { provider: "codex", cwd: workdir, providerOptions: { fast_mode: true } }, + undefined, + { workspaceId: undefined }, + ); + + expect(agent.config.providerOptions).toEqual({ service_tier: "slow" }); + expect(published.length).toBeGreaterThan(0); + expect(published.every((options) => options?.service_tier === "slow")).toBe(true); + }); + test("an option without a control is settable through the patch and readable in the snapshot", async () => { const { manager, client, storage, agentId } = await createOptionsAgent(); diff --git a/packages/server/src/server/agent/agent-manager.ts b/packages/server/src/server/agent/agent-manager.ts index c4b654f6ac..67c18d9317 100644 --- a/packages/server/src/server/agent/agent-manager.ts +++ b/packages/server/src/server/agent/agent-manager.ts @@ -2272,29 +2272,37 @@ export class AgentManager { /** * Applies an options patch to a running agent (see `AgentSession.applyOptions`). The provider - * gets only the keys the patch sets, each resolved against the `config.json` defaults; a - * removal sends it nothing and it keeps its current setting until the session is next opened. - * The patch is stored once the provider accepts it, over the options as they are at that - * moment, so concurrent writes to one agent resolve by arrival order. + * gets each key the patch touches, resolved against the `config.json` defaults, with `null` + * where nothing is set. The patch and whatever the provider says it ended up with are stored in + * one step once the provider accepts, over the options as they are at that moment, so concurrent + * writes to one agent resolve by arrival order. */ async setAgentOptions(agentId: string, patch: ProviderOptionsPatch): Promise { const agent = this.requireAgent(agentId); - const set = Object.fromEntries( - Object.entries(patch).filter(([, value]) => value !== null && value !== undefined), - ); - if (agent.session.applyOptions && Object.keys(set).length > 0) { + let ended: ProviderOptionsPatch = {}; + if (agent.session.applyOptions && Object.keys(patch).length > 0) { + const set = Object.fromEntries(Object.entries(patch).filter(([, value]) => value != null)); const resolved = this.effectiveOptionsFor(agent.provider, set); - await agent.session.applyOptions( - Object.fromEntries(Object.keys(set).map((key) => [key, resolved[key]])), - ); + ended = + (await agent.session.applyOptions( + Object.fromEntries(Object.keys(patch).map((key) => [key, resolved[key] ?? null])), + )) ?? {}; + // Changes the provider made on its own while applying are older than its final answer. + await this.drainSessionEvents(agentId); } - this.storeOptionsPatch(agent, patch); - // Differences the provider reports after applying arrive now and are stored as reported. - await this.drainSessionEvents(agentId); + this.storeOptionsPatch(agent, { ...patch, ...ended }); this.touchUpdatedAt(agent); this.emitState(agent); } + /** The config as the opened session left it: its own difference from the options it was given. */ + private withOpenedOptions(config: AgentSessionConfig, session: AgentSession): AgentSessionConfig { + const opened = session.openOptionsPatch; + if (!opened || Object.keys(opened).length === 0) return config; + const own = applyProviderOptionsPatch(config.providerOptions, opened); + return { ...config, providerOptions: Object.keys(own).length > 0 ? own : undefined }; + } + private storeOptionsPatch(agent: ManagedAgent, patch: ProviderOptionsPatch): void { const own = applyProviderOptionsPatch(agent.config.providerOptions, patch); agent.config.providerOptions = Object.keys(own).length > 0 ? own : undefined; @@ -3819,7 +3827,7 @@ export class AgentManager { private async registerSession( session: AgentSession, - config: AgentSessionConfig, + openedConfig: AgentSessionConfig, agentId: string, options?: { createdAt?: Date; @@ -3850,6 +3858,7 @@ export class AgentManager { let registered = false; try { this.assertAcceptingAgentRegistrations(); + const config = this.withOpenedOptions(openedConfig, session); const resolvedAgentId = validateAgentId(agentId, "registerSession"); if (this.agents.has(resolvedAgentId)) { throw new Error(`Agent with id ${resolvedAgentId} already exists`); diff --git a/packages/server/src/server/agent/agent-sdk-types.ts b/packages/server/src/server/agent/agent-sdk-types.ts index 234016e63b..a9e9152a08 100644 --- a/packages/server/src/server/agent/agent-sdk-types.ts +++ b/packages/server/src/server/agent/agent-sdk-types.ts @@ -433,8 +433,8 @@ export type AgentStreamEvent = * The provider changed options on its own: after `createSession`/`resumeSession` or * `applyOptions` it ended up different from what it was given, or later it changed a setting * itself. The daemon stores the patch and forwards it; it does not undo it. `null` removes the - * agent's own value. Emit it once, after the operation resolves and the daemon is subscribed - * (sessions typically emit the open report from `subscribe`), never while the operation runs. + * agent's own value. Only for changes that happen on the provider's own initiative: the result + * of opening goes in `openOptionsPatch` and the result of `applyOptions` is its return value. */ | { type: "options_changed"; provider: AgentProvider; patch: Record } | { @@ -688,6 +688,12 @@ export interface AgentSession { readonly capabilities: AgentCapabilityFlags; /** Composer controls over top-level option keys. Values come from the agent's effective options. */ readonly controls?: AgentOptionControl[]; + /** + * How the session ended up different from the options it was opened with, as a patch (`null` + * removes a key). Set by the time `createSession`/`resumeSession` resolves; the daemon stores it + * before the agent is published. + */ + readonly openOptionsPatch?: Record; /** New provider-owned rows to commit on registration. streamHistory must also * replay them at their original timestamps; restored sessions omit old rows. */ readonly initialTimeline?: ImportedTimelineEntry[]; @@ -723,14 +729,15 @@ export interface AgentSession { setModel?(modelId: string | null): Promise; setThinkingOption?(thinkingOptionId: string | null): Promise; /** - * Apply the options a patch just set. `options` holds only those keys, resolved against the - * `config.json` defaults, never the agent's whole record. Leave every other setting alone. - * Removing an option sends nothing: the session keeps its current setting until the next open. - * Throw to reject a value; the daemon stores the patch only when this resolves, so the call is - * all-or-nothing. Emit nothing from here. If the result differs from what was asked for, - * report it afterwards as `options_changed`. + * Apply an options patch. `options` holds only the keys the patch touches, resolved against + * the `config.json` defaults; `null` means nothing is set for the key, so return it to the + * session's own default if that can be done. Leave every other setting alone. Throw to reject a + * value; the daemon stores the patch only when this resolves, so the call is all-or-nothing. + * Emit nothing from here. Return the options the session ended up with when they differ from + * the request (a patch, `null` removes a key), for example a value it kept because it could not + * be reverted; the daemon stores it together with the patch. */ - applyOptions?(options: ProviderOptions): Promise; + applyOptions?(options: ProviderOptions): Promise | void>; revertConversation?(input: { messageId: string }): Promise; revertFiles?(input: { messageId: string }): Promise; revertBoth?(input: { messageId: string }): Promise; @@ -781,16 +788,16 @@ export interface AgentClient { readonly provider: AgentProvider; readonly capabilities: AgentCapabilityFlags; /** - * Create the agent with `config.providerOptions` (defaults already merged in). Report nothing - * about options while creating. If the agent ended up different from the options, say so once, - * afterwards, as `options_changed`. + * Create the agent with `config.providerOptions` (defaults already merged in). Emit nothing + * about options while creating. If the agent ended up different from the options, say so in the + * session's `openOptionsPatch`. */ createSession( config: AgentSessionConfig, launchContext?: AgentLaunchContext, options?: AgentCreateSessionOptions, ): Promise; - /** Same contract as `createSession`: open with the options, report any difference once, afterwards. */ + /** Same contract as `createSession`: open with the options, report any difference in `openOptionsPatch`. */ resumeSession( handle: AgentPersistenceHandle, overrides?: Partial, diff --git a/packages/server/src/server/agent/agent-storage.test.ts b/packages/server/src/server/agent/agent-storage.test.ts index b2d152559f..9c8296d0f6 100644 --- a/packages/server/src/server/agent/agent-storage.test.ts +++ b/packages/server/src/server/agent/agent-storage.test.ts @@ -252,12 +252,12 @@ describe("AgentStorage", () => { expect(persisted?.config).not.toHaveProperty("featureValues"); expect(persisted).not.toHaveProperty("features"); expect(persisted?.controls).toEqual([ - { type: "toggle", key: "fast_mode", label: "Fast mode", defaultValue: true }, + { type: "toggle", key: "fast_mode", label: "Fast mode" }, { type: "select", key: "service_tier", label: "Speed", - defaultValue: "fast", + defaultValue: "default", choices: [ { value: "default", label: "Normal" }, { value: "fast", label: "Fast" }, diff --git a/packages/server/src/server/agent/option-controls.ts b/packages/server/src/server/agent/option-controls.ts index 00dd9d909c..a5982c4f86 100644 --- a/packages/server/src/server/agent/option-controls.ts +++ b/packages/server/src/server/agent/option-controls.ts @@ -182,10 +182,7 @@ export function featuresFromControls( return (controls ?? []).map((control) => featureFromControl(control, effective)); } -/** - * A recorded feature's value is the control's unset value: it is what the agent showed before - * any option was stored, and an explicit effective option still wins over it. - */ +/** A recorded feature's value is not carried over: controls hold no state, only `options` do. */ function controlFromFeature(feature: AgentFeature): AgentOptionControl { const descriptor = { key: feature.id, @@ -193,15 +190,14 @@ function controlFromFeature(feature: AgentFeature): AgentOptionControl { ...describe(feature), }; if (feature.type === "toggle") { - return { type: "toggle", ...descriptor, defaultValue: feature.value }; + return { type: "toggle", ...descriptor }; } const defaultChoice = feature.options.find((option) => option.isDefault); - const unsetValue = feature.value ?? defaultChoice?.id; return { type: "select", ...descriptor, ...(feature.desktopTrigger ? { desktopTrigger: feature.desktopTrigger } : {}), - ...(unsetValue != null ? { defaultValue: unsetValue } : {}), + ...(defaultChoice ? { defaultValue: defaultChoice.id } : {}), choices: feature.options.map((option) => ({ value: option.id, label: option.label, diff --git a/packages/server/src/server/agent/plugin-provider.test.ts b/packages/server/src/server/agent/plugin-provider.test.ts index 71f8a87b7e..5db02d88f9 100644 --- a/packages/server/src/server/agent/plugin-provider.test.ts +++ b/packages/server/src/server/agent/plugin-provider.test.ts @@ -58,6 +58,10 @@ interface ProviderHarnessOptions { handleInput?: (input: ProviderInput, emit: (event: ProviderEvent) => void) => Promise; } +function lastConfigure(inputs: ProviderInput[]): ProviderInput | undefined { + return inputs.findLast((input) => input.type === "session.configure"); +} + function createProviderHarness(options: ProviderHarnessOptions = {}) { let listener: ((event: ProviderEvent) => void) | null = null; let closeCount = 0; @@ -999,76 +1003,104 @@ describe("PluginAgentClientRegistry", () => { await agent.session?.close(); }); - test("reports a setting the plugin opened differently once, and applies only the options it is given", async () => { - const settings = (voice: string) => [ - { - type: "select" as const, - id: "voice", - label: "Voice", - value: voice, - options: [ - { label: "Direct", value: "direct" }, - { label: "Other", value: "other" }, - ], - }, - { type: "toggle" as const, id: "loud", label: "Loud", value: false }, - ]; - const state = { voice: "direct" }; - const harness = createProviderHarness({ - async handleInput(input, emit) { - if (input.type === "session.open") { - emit({ - type: "session.opened", - requestId: input.requestId, - sessionId: input.sessionId, - capabilities: CAPABILITIES, - restoration: "core", - persistence: { version: 1, data: { token: "root" } }, - cwd: input.config.cwd, - }); - 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: settings(state.voice), - }, - }); - emit({ type: "session.ready", requestId: input.requestId, sessionId: input.sessionId }); - return true; - } - return false; - }, - }); - const logger = createTestLogger(); - const plugins = new PluginAgentClientRegistry(logger); - plugins.replace([harness.registration]); - const client = plugins.clients()["plugin-direct"]!; - const session = await client.createSession({ - provider: "plugin-direct", - cwd: tmpdir(), - providerOptions: { voice: "other", loud: false }, - }); - const events: AgentStreamEvent[] = []; - session.subscribe((event) => events.push(event)); - session.subscribe((event) => events.push(event)); + describe("settings", () => { + // A plugin that keeps `voice` on "direct" whatever it is asked, and takes `loud` as given. + async function openStubbornVoiceSession(given: Record) { + const state = { voice: "direct", loud: false }; + const settings = () => [ + { + type: "select" as const, + id: "voice", + label: "Voice", + value: state.voice, + options: [ + { label: "Direct", value: "direct" }, + { label: "Other", value: "other" }, + ], + }, + { type: "toggle" as const, id: "loud", label: "Loud", value: state.loud }, + ]; + const emitConfig = (emit: (event: ProviderEvent) => void, sessionId: string) => + emit({ + type: "session.config", + sessionId, + config: { + model: "plugin-model", + mode: "build", + models: [{ id: "plugin-model", label: "Plugin model" }], + modes: [{ id: "build", label: "Build" }], + thinkingOptions: [], + settings: settings(), + }, + }); + const harness = createProviderHarness({ + async handleInput(input, emit) { + if (input.type === "session.open") { + emit({ + type: "session.opened", + requestId: input.requestId, + sessionId: input.sessionId, + capabilities: CAPABILITIES, + restoration: "core", + persistence: { version: 1, data: { token: "root" } }, + cwd: input.config.cwd, + }); + emitConfig(emit, input.sessionId); + emit({ type: "session.ready", requestId: input.requestId, sessionId: input.sessionId }); + return true; + } + if (input.type === "session.configure") { + const loud = input.changes.settings?.loud; + if (typeof loud === "boolean") state.loud = loud; + emitConfig(emit, input.sessionId); + emit({ type: "request.completed", requestId: input.requestId }); + return true; + } + return false; + }, + }); + const plugins = new PluginAgentClientRegistry(createTestLogger()); + plugins.replace([harness.registration]); + const session = await plugins.clients()["plugin-direct"]!.createSession({ + provider: "plugin-direct", + cwd: tmpdir(), + providerOptions: given, + }); + const events: AgentStreamEvent[] = []; + session.subscribe((event) => events.push(event)); + return { harness, session, events }; + } - // The plugin opened on "direct" although "other" was asked for: said once, on subscribe. - expect(eventsOfType(events, "options_changed")).toEqual([ - { type: "options_changed", provider: "plugin-direct", patch: { voice: "direct" } }, - ]); + test("a setting the plugin opened differently is the session's open patch, not an event", async () => { + const { session, events } = await openStubbornVoiceSession({ voice: "other", loud: false }); - await session.applyOptions?.({ loud: true }); - const configure = harness.inputs.findLast((input) => input.type === "session.configure"); - expect(configure).toMatchObject({ changes: { settings: { loud: true } } }); - expect(Object.keys((configure as { changes: { settings: object } }).changes.settings)).toEqual([ - "loud", - ]); - await session.close(); + expect(session.openOptionsPatch).toEqual({ voice: "direct" }); + expect(eventsOfType(events, "options_changed")).toEqual([]); + await session.close(); + }); + + test("applying options configures only the settings it is given", async () => { + const { harness, session } = await openStubbornVoiceSession({}); + + await session.applyOptions?.({ loud: true }); + + const configure = lastConfigure(harness.inputs); + expect(configure).toMatchObject({ changes: { settings: { loud: true } } }); + expect( + Object.keys((configure as { changes: { settings: object } }).changes.settings), + ).toEqual(["loud"]); + await session.close(); + }); + + test("applying options returns the settings that ended up different, and emits nothing", async () => { + const { session, events } = await openStubbornVoiceSession({}); + + const ended = await session.applyOptions?.({ voice: "other", loud: true }); + + expect(ended).toEqual({ voice: "direct" }); + expect(eventsOfType(events, "options_changed")).toEqual([]); + await session.close(); + }); }); test.each([ diff --git a/packages/server/src/server/agent/plugin-provider.ts b/packages/server/src/server/agent/plugin-provider.ts index b4069ebda6..f88da5df67 100644 --- a/packages/server/src/server/agent/plugin-provider.ts +++ b/packages/server/src/server/agent/plugin-provider.ts @@ -1203,9 +1203,11 @@ class PluginAgentSession implements AgentSession { >(); private readonly childSnapshots = new Map>(); private unsubscribe: (() => void) | null = null; - /** How the opened session differs from the settings it was given; reported at first subscribe. */ - private openOptionReport: Record = {}; + /** How the opened session differs from the settings it was given. */ + readonly openOptionsPatch: Record; private knownSettings: Record = {}; + /** While `applyOptions` waits on the plugin, its `session.config` answers that call. */ + private applyingOptions = false; private currentTurnId: string | null = null; private closed = false; private replay: { @@ -1229,7 +1231,7 @@ class PluginAgentSession implements AgentSession { givenSettings: Readonly>, ) { this.subagentIdsBySession.set(bridge.id, null); - this.openOptionReport = Object.fromEntries( + this.openOptionsPatch = Object.fromEntries( Object.entries(this.settingDifferences(givenSettings)).filter(([id]) => id in givenSettings), ); this.knownSettings = this.currentSettings(); @@ -1337,10 +1339,6 @@ class PluginAgentSession implements AgentSession { subscribe(callback: (event: AgentStreamEvent) => void): () => void { this.listeners.add(callback); - if (Object.keys(this.openOptionReport).length > 0) { - callback({ type: "options_changed", provider: this.provider, patch: this.openOptionReport }); - this.openOptionReport = {}; - } return () => this.listeners.delete(callback); } @@ -1437,17 +1435,25 @@ class PluginAgentSession implements AgentSession { await this.bridge.configure({ thinkingOption: thinkingOptionId }); } - async applyOptions(options: ProviderOptions): Promise { + async applyOptions(options: ProviderOptions): Promise> { this.assertReplayComplete(); + const before = this.currentSettings(); const settings: Record = {}; for (const setting of this.bridge.config.settings) { const value = options[setting.id]; if (value === undefined) continue; settings[setting.id] = toJsonValue(value, `setting '${setting.id}'`); } - if (Object.keys(settings).length === 0) return; - Object.assign(this.knownSettings, settings); - await this.bridge.configure({ settings }); + if (Object.keys(settings).length === 0) return {}; + this.applyingOptions = true; + try { + await this.bridge.configure({ settings }); + } finally { + this.applyingOptions = false; + } + const ended = this.settingDifferences({ ...before, ...settings }); + this.knownSettings = this.currentSettings(); + return ended; } async revertConversation(input: { messageId: string }): Promise { @@ -1647,6 +1653,7 @@ class PluginAgentSession implements AgentSession { } private reportSettingChanges(): AgentStreamEvent[] { + if (this.applyingOptions) return []; const patch = this.settingDifferences(this.knownSettings); if (Object.keys(patch).length === 0) return []; this.knownSettings = this.currentSettings(); diff --git a/packages/server/src/server/agent/provider-registry.ts b/packages/server/src/server/agent/provider-registry.ts index 90af6506d9..a470d1a01a 100644 --- a/packages/server/src/server/agent/provider-registry.ts +++ b/packages/server/src/server/agent/provider-registry.ts @@ -450,6 +450,9 @@ export function wrapSessionProvider(provider: AgentProvider, inner: AgentSession get controls() { return inner.controls; }, + get openOptionsPatch() { + return inner.openOptionsPatch; + }, usageSession: inner.usageSession?.bind(inner), run: (prompt, options) => inner.run(prompt, options), startTurn: (prompt, options) => inner.startTurn(prompt, options), 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 1711645445..3118f6db90 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -1802,7 +1802,7 @@ describe("ACPAgentSession Zed parity", () => { ]); }); - test("reports once, at subscribe, a select the provider opened differently from the options", async () => { + test("a select the provider opened differently from the options is the session's open patch, not an event", async () => { const setSessionConfigOption = vi.fn(async () => ({ configOptions: [copilotAgentConfigOption("")], })); @@ -1813,40 +1813,57 @@ describe("ACPAgentSession Zed parity", () => { }); const emitted: AgentStreamEvent[] = []; session.subscribe((event) => emitted.push(event)); + await internals.applyConfiguredOverrides(); + + expect(session.openOptionsPatch).toEqual({ agent: "" }); expect(emitted.filter((event) => event.type === "options_changed")).toEqual([]); - - const first: AgentStreamEvent[] = []; - session.subscribe((event) => first.push(event)); - const second: AgentStreamEvent[] = []; - session.subscribe((event) => second.push(event)); - - expect(first.filter((event) => event.type === "options_changed")).toEqual([ - { type: "options_changed", provider: "copilot", patch: { agent: "" } }, - ]); - expect(second.filter((event) => event.type === "options_changed")).toEqual([]); }); - test("forwards a select the provider changes on its own, and applying a patch emits nothing", async () => { + test("applying a patch returns the selects that ended up different and emits nothing", async () => { const setSessionConfigOption = vi.fn(async () => ({ - configOptions: [copilotAgentConfigOption("Probe Agent")], + configOptions: [copilotAgentConfigOption("")], })); const session = createCopilotSessionWithConfig(); - const { internals } = prepareConfiguredOverrideSession(session, { + prepareConfiguredOverrideSession(session, { configOptions: [copilotAgentConfigOption("")], connection: { setSessionConfigOption }, }); - await internals.applyConfiguredOverrides(); const emitted: AgentStreamEvent[] = []; session.subscribe((event) => emitted.push(event)); - await session.applyOptions({ agent: "Probe Agent" }); + await expect(session.applyOptions({ agent: "Probe Agent" })).resolves.toEqual({ agent: "" }); expect(emitted.filter((event) => event.type === "options_changed")).toEqual([]); + }); + + test("a removal writes the select's empty choice and returns nothing when that took", async () => { + const setSessionConfigOption = vi.fn(async () => ({ + configOptions: [copilotAgentConfigOption("")], + })); + const session = createCopilotSessionWithConfig(); + prepareConfiguredOverrideSession(session, { + configOptions: [copilotAgentConfigOption("Probe Agent")], + connection: { setSessionConfigOption }, + }); + + await expect(session.applyOptions({ agent: null })).resolves.toEqual({}); + expect(setSessionConfigOption).toHaveBeenCalledWith(expect.objectContaining({ value: "" })); + }); + + test("forwards a select the provider changes on its own", async () => { + const session = createCopilotSessionWithConfig(); + prepareConfiguredOverrideSession(session, { + configOptions: [copilotAgentConfigOption("Probe Agent")], + connection: { setSessionConfigOption: vi.fn() }, + }); + const emitted: AgentStreamEvent[] = []; + session.subscribe((event) => emitted.push(event)); asInternals(session).translateSessionUpdate({ sessionUpdate: "config_option_update", configOptions: [copilotAgentConfigOption("")], }); + expect(emitted.filter((event) => event.type === "options_changed")).toEqual([ { type: "options_changed", provider: "copilot", patch: { agent: "" } }, ]); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 0e7de998cb..db0afc2b7c 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -1755,10 +1755,8 @@ export class ACPAgentSession implements AgentSession, ACPClient { private currentTitle: string | null = null; private lastActivityAt: string | null = null; private configOptions: SessionConfigOption[] = []; - /** The value of each select as Paseo last knew it; a different value is the provider's change. */ - private readonly knownFeatureValues = new Map(); - /** How the opened session differs from the options it was given; reported at first subscribe. */ - private openOptionReport: Record = {}; + /** How the opened session differs from the options it was given. */ + openOptionsPatch: Record = {}; private cachedCommands: AgentSlashCommand[] = []; /** When the last command batch arrived; null while nothing has been published. */ private commandsUpdatedAt: number | null = null; @@ -2023,10 +2021,6 @@ export class ACPAgentSession implements AgentSession, ACPClient { sessionId: this.sessionId, }); } - if (Object.keys(this.openOptionReport).length > 0) { - callback({ type: "options_changed", provider: this.provider, patch: this.openOptionReport }); - this.openOptionReport = {}; - } return () => { this.subscribers.delete(callback); }; @@ -2200,6 +2194,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { } const { connection, sessionId } = this; + const before = this.featureValues(); const response = await this.runACPRequest(() => connection.setSessionConfigOption({ sessionId, @@ -2214,7 +2209,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { requestedValue: modeId, label: "mode", }); - this.reportOptionChanges(); + this.reportFeatureChanges(before); this.availableModes = deriveModesFromACP(this.defaultModes, null, this.configOptions).modes; this.pushEvent({ type: "mode_changed", @@ -2326,6 +2321,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { } const { connection, sessionId } = this; + const before = this.featureValues(); const response = await this.runACPRequest(() => connection.setSessionConfigOption({ sessionId, @@ -2340,7 +2336,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { requestedValue: modelId, label: "model", }); - this.reportOptionChanges(); + this.reportFeatureChanges(before); this.pushEvent({ type: "model_changed", provider: this.provider, @@ -2378,6 +2374,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { throw new Error(`${this.provider} does not expose ACP thought-level selection`); } const { connection, sessionId } = this; + const before = this.featureValues(); const response = await this.runACPRequest(() => connection.setSessionConfigOption({ sessionId, @@ -2392,7 +2389,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { requestedValue: thinkingOptionId, label: "thought-level", }); - this.reportOptionChanges(); + this.reportFeatureChanges(before); this.pushEvent({ type: "thinking_option_changed", provider: this.provider, @@ -2400,17 +2397,19 @@ export class ACPAgentSession implements AgentSession, ACPClient { }); } - /** Applies only the options the patch sets. Differences are reported after this resolves. */ - async applyOptions(options: ProviderOptions): Promise { + /** Applies only the options the patch sets; returns the selects that ended up different. */ + async applyOptions(options: ProviderOptions): Promise> { + const before = this.featureValues(); + const requested: Record = {}; for (const [id, value] of Object.entries(options)) { const featureOption = this.configFeatureOptions.find((candidate) => candidate.id === id); if (!featureOption) continue; + // A removal asks for the select's empty choice; a provider without one rejects it. + requested[id] = value ?? ""; const option = findSelectConfigFeatureOption(this.configOptions, featureOption); - if (!option || value !== option.currentValue) { + if (!option || requested[id] !== option.currentValue) { await this.setConfigFeature(id, value); } - this.knownFeatureValues.set(id, String(value)); - this.config.providerOptions = { ...this.config.providerOptions, [id]: value }; } // Auto Accept is Paseo's own setting, so it changes only once every write has been accepted. if (ACP_AUTO_ACCEPT_FEATURE_ID in options) { @@ -2419,29 +2418,34 @@ export class ACPAgentSession implements AgentSession, ACPClient { [ACP_AUTO_ACCEPT_FEATURE_ID]: options[ACP_AUTO_ACCEPT_FEATURE_ID] === true, }; } - setImmediate(() => this.reportOptionChanges()); + return this.featureDifferences({ ...before, ...requested }); } - /** Reports selects the provider changed on its own since Paseo last knew their values. */ - private reportOptionChanges(): void { - const patch: Record = {}; + /** The selects as the provider reports them now. */ + private featureValues(): Record { + const values: Record = {}; for (const featureOption of this.configFeatureOptions) { const current = findSelectConfigFeatureOption( this.configOptions, featureOption, )?.currentValue; - const known = this.knownFeatureValues.get(featureOption.id); - if (current == null || known === current) continue; - this.knownFeatureValues.set(featureOption.id, current); - if (known === undefined) continue; - patch[featureOption.id] = current; - if (this.config.providerOptions && featureOption.id in this.config.providerOptions) { - this.config.providerOptions = { - ...this.config.providerOptions, - [featureOption.id]: current, - }; - } + if (current != null) values[featureOption.id] = current; } + return values; + } + + /** The selects whose value is not what `expected` says; selects it does not name are skipped. */ + private featureDifferences(expected: Record): Record { + return Object.fromEntries( + Object.entries(this.featureValues()).filter( + ([id, value]) => id in expected && expected[id] !== value, + ), + ); + } + + /** Reports the selects the provider moved on its own, compared with their values `before`. */ + private reportFeatureChanges(before: Record): void { + const patch = this.featureDifferences(before); if (Object.keys(patch).length > 0) { this.pushEvent({ type: "options_changed", provider: this.provider, patch }); } @@ -3086,26 +3090,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { ); } } - this.recordOpenedFeatureValues(configuredFeatureValues); - } - - /** Remembers what each select ended at, and what differs from the options the session was given. */ - private recordOpenedFeatureValues(given: Record): void { - for (const featureOption of this.configFeatureOptions) { - const current = findSelectConfigFeatureOption( - this.configOptions, - featureOption, - )?.currentValue; - if (current == null) continue; - this.knownFeatureValues.set(featureOption.id, current); - if (featureOption.id in given && given[featureOption.id] !== current) { - this.openOptionReport[featureOption.id] = current; - this.config.providerOptions = { - ...this.config.providerOptions, - [featureOption.id]: current, - }; - } - } + this.openOptionsPatch = this.featureDifferences(configuredFeatureValues); } /** @@ -3319,6 +3304,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { } private handleConfigOptionUpdate(update: ConfigOptionUpdate): AgentStreamEvent[] { + const before = this.featureValues(); this.configOptions = this.transformConfigOptions(update.configOptions); const modeInfo = deriveModesFromACP(this.defaultModes, null, this.configOptions); const nextMode = modeInfo.currentModeId; @@ -3329,7 +3315,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { this.currentMode = nextMode ?? this.currentMode; this.currentModel = nextModel ?? this.currentModel; this.thinkingOptionId = nextThinkingOptionId ?? this.thinkingOptionId; - this.reportOptionChanges(); + this.reportFeatureChanges(before); const events: AgentStreamEvent[] = []; if (nextMode !== null) { 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 e0d649e6ff..645898bdf4 100644 --- a/packages/server/src/server/agent/providers/claude/agent.test.ts +++ b/packages/server/src/server/agent/providers/claude/agent.test.ts @@ -1521,13 +1521,34 @@ describe("ClaudeAgentSession features", () => { await session.applyOptions?.({ fast_mode: true }); queryMock.applyFlagSettings.mockClear(); - // Removing fast_mode, or setting another option, sends only the keys set: Fast stays on. + // Setting another option sends only the keys set: Fast stays on. await session.applyOptions?.({ allowedTools: ["Read"] }); expect(queryMock.applyFlagSettings).not.toHaveBeenCalled(); await session.close(); }); + test("removing fast_mode turns Fast off", 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 }); + + await session.applyOptions?.({ fast_mode: null }); + + expect(queryMock.applyFlagSettings).toHaveBeenLastCalledWith({ fastMode: false }); + await session.close(); + }); + test("retries a fast mode change the runtime rejected", async () => { const { queryFactory, queryMock } = createQueryMock(); const client = new ClaudeAgentClient({ diff --git a/packages/server/src/server/agent/providers/claude/agent.ts b/packages/server/src/server/agent/providers/claude/agent.ts index 0666fb8dca..a7f890ff26 100644 --- a/packages/server/src/server/agent/providers/claude/agent.ts +++ b/packages/server/src/server/agent/providers/claude/agent.ts @@ -2798,11 +2798,13 @@ class ClaudeAgentSession implements AgentSession { } async applyOptions(options: ProviderOptions): Promise { + const merged: Record = { ...this.config.providerOptions }; + for (const [key, value] of Object.entries(options)) { + if (value === null) delete merged[key]; + else merged[key] = value; + } const providerOptions = - validateProviderOptions("claude", ClaudeProviderOptionsSchema, { - ...this.config.providerOptions, - ...options, - }) ?? {}; + validateProviderOptions("claude", ClaudeProviderOptionsSchema, merged) ?? {}; if (providerOptions.fast_mode === true && !claudeModelSupportsFastMode(this.config.model)) { throw new Error( `Claude fast mode is not available for model '${this.config.model ?? "default"}'`, 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 f8fb4fb92d..f70ecd6d76 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 @@ -506,17 +506,11 @@ describe("Codex app-server provider controls", () => { }); const events: AgentStreamEvent[] = []; session.subscribe((event) => events.push(event)); - session.subscribe((event) => events.push(event)); expect(session.controls).toEqual([speedControl(), PLAN_CONTROL]); - // A saved pre-v0.10 Fast preference opens as the model's Fast tier and says so once. - expect(events.filter((event) => event.type === "options_changed")).toEqual([ - { - type: "options_changed", - provider: "codex", - patch: { service_tier: "priority", fast_mode: null }, - }, - ]); + // A saved pre-v0.10 Fast preference opens as the model's Fast tier, which the open patch says. + expect(session.openOptionsPatch).toEqual({ service_tier: "priority", fast_mode: null }); + expect(events.filter((event) => event.type === "options_changed")).toEqual([]); await session.startTurn("hello"); await expect(appServer.waitForTurnStart()).resolves.toMatchObject({ @@ -544,7 +538,7 @@ describe("Codex app-server provider controls", () => { test("applyOptions accepts an option that has no control", async () => { const { session } = await createConnectedSession(); - await expect(session.applyOptions?.({ sandbox_mode: "read-only" })).resolves.toBeUndefined(); + await expect(session.applyOptions?.({ sandbox_mode: "read-only" })).resolves.toEqual({}); }); test("startTurn includes serviceTier when fast mode is enabled", async () => { @@ -558,20 +552,12 @@ describe("Codex app-server provider controls", () => { }); }); - test("a saved service_tier: fast runs as the model's Fast tier and is reported as that tier", async () => { + test("a saved service_tier: fast runs as the model's Fast tier and the open patch names that tier", async () => { const { session, appServer } = await createConnectedSession({ providerOptions: { service_tier: "fast" }, }); - const events: AgentStreamEvent[] = []; - session.subscribe((event) => events.push(event)); - expect(events.filter((event) => event.type === "options_changed")).toEqual([ - { - type: "options_changed", - provider: "codex", - patch: { service_tier: "priority", fast_mode: null }, - }, - ]); + expect(session.openOptionsPatch).toEqual({ service_tier: "priority", fast_mode: null }); await session.startTurn("hello"); await expect(appServer.waitForTurnStart()).resolves.toMatchObject({ serviceTier: "priority" }); @@ -707,12 +693,11 @@ describe("Codex app-server provider controls", () => { await runTurn(agentId, "priority"); }); - test("from config.json defaults runs Fast and shows it through the agent's own tier", async () => { - const { manager, create, shownSpeed, runTurn } = createLegacyFastHarness({ fast_mode: true }); + test("from config.json defaults runs Fast and is shown as Fast", async () => { + const { create, shownSpeed, runTurn } = createLegacyFastHarness({ fast_mode: true }); const agentId = await create(); - await manager.flush(); - expect(shownSpeed(agentId)).toMatchObject({ own: { service_tier: "priority" }, ...FAST }); + expect(shownSpeed(agentId)).toMatchObject(FAST); await runTurn(agentId, "priority"); }); @@ -722,10 +707,7 @@ describe("Codex app-server provider controls", () => { expect(shownSpeed(agentId).selected).toBe("default"); await manager.setAgentOptions(agentId, { fast_mode: true }); - // The provider names the tier it resolved on the next tick, after the patch is stored. - await new Promise((resolve) => setImmediate(resolve)); - await manager.flush(); - expect(shownSpeed(agentId)).toMatchObject(FAST); + expect(shownSpeed(agentId)).toMatchObject({ own: { service_tier: "priority" }, ...FAST }); await runTurn(agentId, "priority"); await manager.setAgentOptions(agentId, { service_tier: "default" }); @@ -740,6 +722,16 @@ describe("Codex app-server provider controls", () => { expect(shownSpeed(agentId)).toMatchObject({ selected: "priority" }); await runTurn(agentId, "priority"); }); + + test("removing the speed returns the agent to Normal", async () => { + const { create, manager, shownSpeed } = createLegacyFastHarness(); + const agentId = await create({ service_tier: "priority" }); + expect(shownSpeed(agentId)).toMatchObject(FAST); + + await manager.setAgentOptions(agentId, { service_tier: null }); + + expect(shownSpeed(agentId)).toMatchObject({ own: undefined, selected: "default" }); + }); }); test("startTurn logs a sanitized turn/start summary for fast mode observability", async () => { 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 86360f8cbc..3845d70c93 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 @@ -3455,7 +3455,7 @@ export class CodexAppServerAgentSession implements AgentSession { private readonly subscribers = new Set<(event: AgentStreamEvent) => void>(); private startupEvents: AgentStreamEvent[] | null = null; /** How the opened session differs from the options it was given; reported at first subscribe. */ - private openOptionReport: Record = {}; + openOptionsPatch: Record = {}; private nextTurnOrdinal = 0; private activeForegroundTurnId: string | null = null; private activeClientMessageId: string | null = null; @@ -3866,7 +3866,7 @@ export class CodexAppServerAgentSession implements AgentSession { /** * COMPAT(codexFastPreference): added in v0.10.0, remove after 2027-03-29 once saved Fast * preferences have migrated. A saved `fast_mode: true` or `service_tier: "fast"` runs as the - * model's Fast tier; the session says so once, at subscribe, so the options and the control agree. + * model's Fast tier; the session says so in `openOptionsPatch`, so the options and the control agree. */ private normalizeLegacySpeed(): void { const requested = this.requestedServiceTier; @@ -3876,7 +3876,7 @@ export class CodexAppServerAgentSession implements AgentSession { const { fast_mode: _legacy, ...rest } = this.providerOptions; this.providerOptions = { ...rest, service_tier: resolved }; this.requestedServiceTier = resolved; - this.openOptionReport = { service_tier: resolved, fast_mode: null }; + this.openOptionsPatch = { service_tier: resolved, fast_mode: null }; } private get serviceTier(): string | null { @@ -4595,10 +4595,6 @@ export class CodexAppServerAgentSession implements AgentSession { const startupEvents = this.startupEvents; this.startupEvents = null; for (const event of startupEvents ?? []) this.notifySubscribers(event); - if (Object.keys(this.openOptionReport).length > 0) { - callback({ type: "options_changed", provider: CODEX_PROVIDER, patch: this.openOptionReport }); - this.openOptionReport = {}; - } return () => { this.subscribers.delete(callback); }; @@ -4680,13 +4676,14 @@ export class CodexAppServerAgentSession implements AgentSession { } /** Applies only the options the patch sets; the rest of the running options stay as they are. */ - async applyOptions(options: ProviderOptions): Promise { + async applyOptions(options: ProviderOptions): Promise> { const { fast_mode: _legacyFast, ...running } = this.providerOptions; - const next = - validateProviderOptions("codex", CodexProviderOptionsSchema, { - ...running, - ...options, - }) ?? {}; + const next: Record = { ...running }; + for (const [key, value] of Object.entries(options)) { + if (value === null) delete next[key]; + else next[key] = value; + } + const valid = validateProviderOptions("codex", CodexProviderOptionsSchema, next) ?? {}; let requestedTier = this.requestedServiceTier; if ("service_tier" in options || "fast_mode" in options) { // COMPAT(codexFastPreference): added in v0.10.0, remove after 2027-03-29 once clients use service_tier. @@ -4700,31 +4697,24 @@ export class CodexAppServerAgentSession implements AgentSession { ); } } - this.providerOptions = { ...next, ...(requestedTier ? { service_tier: requestedTier } : {}) }; + this.providerOptions = { ...valid, ...(requestedTier ? { service_tier: requestedTier } : {}) }; this.requestedServiceTier = requestedTier; if ("plan_mode" in options) this.setPlanMode(options.plan_mode === true); this.cachedRuntimeInfo = null; // COMPAT(codexFastPreference): added in v0.10.0, remove after 2027-03-29 once clients use service_tier. - if ("fast_mode" in options && !("service_tier" in options)) { - setImmediate(() => this.reportSpeed({ fast_mode: null })); + const tier = this.serviceTier; + if ("fast_mode" in options && !("service_tier" in options) && options.fast_mode !== null) { + return tier === null ? {} : { service_tier: tier, fast_mode: null }; } + return {}; } private requestedTierFrom(options: ProviderOptions): string | null { if ("service_tier" in options) return readCodexServiceTier(options); + if (options.fast_mode === null) return null; return options.fast_mode ? "fast" : "default"; } - private reportSpeed(extra: Record): void { - const tier = this.serviceTier; - if (tier === null) return; - this.emitEvent({ - type: "options_changed", - provider: CODEX_PROVIDER, - patch: { service_tier: tier, ...extra }, - }); - } - private isServiceTierAvailable(tier: string | null): boolean { if (tier === null || tier === "default") return true; const tiers = this.currentServiceTiers(); 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 e81b07b812..9c7961b061 100644 --- a/packages/server/src/server/agent/providers/omp/agent.test.ts +++ b/packages/server/src/server/agent/providers/omp/agent.test.ts @@ -646,11 +646,38 @@ describe("OMP agent client and session", () => { await session.applyOptions?.({ fast_mode: true }); expect(omp.runtime().setFastModeRequests).toEqual([true]); - // Removing the option sends nothing: the patch carries no key, so the runtime keeps Fast. + // A patch without the key leaves the runtime alone. await session.applyOptions?.({}); expect(omp.runtime().setFastModeRequests).toEqual([true]); }); + test("a removal turns Fast off, and a Fast the runtime keeps is returned", async () => { + const omp = new OmpHarness(); + await omp.start(); + const session = omp.requireSession(); + omp.runtime().fastModeResult = { enabled: true, active: true }; + await session.applyOptions?.({ fast_mode: true }); + + omp.runtime().fastModeResult = { enabled: false, active: false }; + await expect(session.applyOptions?.({ fast_mode: null })).resolves.toEqual({}); + expect(omp.runtime().setFastModeRequests).toEqual([true, false]); + + omp.runtime().fastModeResult = { enabled: false, active: false }; + await expect(session.applyOptions?.({ fast_mode: true })).resolves.toEqual({ + fast_mode: false, + }); + expect(omp.eventTypes()).not.toContain("options_changed"); + }); + + test("a session opened without the Fast it was given says so in its open patch", async () => { + const omp = new OmpHarness(); + // The fake runtime answers the open-time Fast request with Fast off. + await omp.start({ providerOptions: { fast_mode: true } }); + + expect(omp.requireSession().openOptionsPatch).toEqual({ fast_mode: false }); + expect(omp.eventTypes()).not.toContain("options_changed"); + }); + test("reports a Fast change the runtime made on its own", async () => { const omp = new OmpHarness(); await omp.start(); diff --git a/packages/server/src/server/agent/providers/omp/agent.ts b/packages/server/src/server/agent/providers/omp/agent.ts index bab29a0d46..646bbb7cf2 100644 --- a/packages/server/src/server/agent/providers/omp/agent.ts +++ b/packages/server/src/server/agent/providers/omp/agent.ts @@ -751,7 +751,7 @@ export class OmpAgentSession implements AgentSession { this.restartRuntime = options.restartRuntime; this.config = options.config; this.state = options.initialState; - this.openOptionReport = this.fastModeDifference(options.config.providerOptions?.fast_mode); + this.openOptionsPatch = this.fastModeDifference(options.config.providerOptions?.fast_mode); this.currentModeId = options.currentModeId ?? null; this.logger = options.logger; this.live = options.live ?? true; @@ -821,9 +821,9 @@ export class OmpAgentSession implements AgentSession { ]; } - async applyOptions(options: ProviderOptions): Promise { - const requested = options.fast_mode; - if (requested === undefined) return; + async applyOptions(options: ProviderOptions): Promise> { + if (options.fast_mode === undefined) return {}; + const requested = options.fast_mode ?? false; if (typeof requested !== "boolean") throw new Error("OMP fast mode requires a boolean"); if (!hasOmpFastMode(this.state)) throw new Error("OMP fast mode is unavailable"); if (requested !== this.state.fastModeEnabled) { @@ -834,7 +834,7 @@ export class OmpAgentSession implements AgentSession { fastModeActive: result.active, }; } - this.config.providerOptions = { ...this.config.providerOptions, fast_mode: requested }; + return this.fastModeDifference(requested); } /** The option patch when the runtime's fast mode is not what `given` stands for; unset is off. */ @@ -844,8 +844,8 @@ export class OmpAgentSession implements AgentSession { return (given ?? false) === enabled ? {} : { fast_mode: enabled }; } - /** How the opened session differs from the options it was given; reported at first subscribe. */ - private openOptionReport: Record; + /** How the opened session differs from the options it was given. */ + readonly openOptionsPatch: Record; async run(prompt: AgentPromptInput, options?: AgentRunOptions): Promise { return runProviderTurn({ @@ -1007,10 +1007,6 @@ export class OmpAgentSession implements AgentSession { subscribe(callback: (event: AgentStreamEvent) => void): () => void { this.subscribers.add(callback); - if (Object.keys(this.openOptionReport).length > 0) { - callback({ type: "options_changed", provider: this.provider, patch: this.openOptionReport }); - this.openOptionReport = {}; - } return () => { this.subscribers.delete(callback); }; 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 ca0da89d45..1e6587389a 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 @@ -1,4 +1,4 @@ -import { describe, expect, test } from "vitest"; +import { describe, expect, test, vi } from "vitest"; import { createTestLogger } from "../../../test-utils/test-logger.js"; import type { AgentStreamEvent } from "../agent-sdk-types.js"; @@ -290,7 +290,7 @@ describe("OpenCode auto_accept feature", () => { await session.close(); }); - test("keeps auto-approving after auto_accept is removed from the options", async () => { + test("stops auto-approving once auto_accept is removed", async () => { const { openCodeClient, runtime } = mockOpenCodeClient({ events: [toolPermissionEvent(), idleEvent()], }); @@ -305,12 +305,11 @@ describe("OpenCode auto_accept feature", () => { providerOptions: { auto_accept: true }, }); - // Removal sends the provider nothing: it keeps its current setting until next open. - await session.applyOptions?.({}); - await session.run("Run verification"); + await session.applyOptions?.({ auto_accept: null }); + await session.startTurn("Run verification"); + await vi.waitFor(() => expect(session.getPendingPermissions()).toHaveLength(1)); - expect(openCodeClient.calls.permissionReply).toHaveLength(1); - expect(session.getPendingPermissions()).toEqual([]); + expect(openCodeClient.calls.permissionReply).toHaveLength(0); await session.close(); }); 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 867de53c90..cf5c1c2b7a 100644 --- a/packages/server/src/server/agent/providers/opencode/v2/session.ts +++ b/packages/server/src/server/agent/providers/opencode/v2/session.ts @@ -360,8 +360,8 @@ export class OpenCodeV2Session implements AgentSession { this.emit({ type: "thinking_option_changed", provider: "opencode", thinkingOptionId: variant }); } async applyOptions(options: ProviderOptions) { - const value = options["auto_accept"]; - if (value === undefined) return; + if (!("auto_accept" in options)) return; + const value = options["auto_accept"] ?? false; 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)