diff --git a/packages/server/src/server/agent/plugin-provider.test.ts b/packages/server/src/server/agent/plugin-provider.test.ts index 986e65fbeb..c79039a4a8 100644 --- a/packages/server/src/server/agent/plugin-provider.test.ts +++ b/packages/server/src/server/agent/plugin-provider.test.ts @@ -505,6 +505,67 @@ test("combined plugin rewind replays the retained native branch", async () => { }); }); +test("a rewind reopens with the options as they stand, a reset included", async () => { + const options: Record = {}; + const report = (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: [], + controls: [{ type: "toggle", key: "concise", label: "Concise" }], + providerOptions: { ...options }, + }, + }); + const harness = createRewindProviderHarness({ + handleInput: async (input, emit) => { + if (input.type === "session.open" && input.history !== "replay") { + options.concise = true; + emit({ + type: "session.opened", + requestId: input.requestId, + sessionId: input.sessionId, + capabilities: [...CAPABILITIES, "session.revert.conversation"], + restoration: "core", + persistence: { version: 1, data: { token: "root" } }, + cwd: input.config.cwd, + }); + report(emit, input.sessionId); + emit({ type: "session.ready", requestId: input.requestId, sessionId: input.sessionId }); + return true; + } + if (input.type === "session.configure") { + // A plugin that represents the default by leaving the key out. + delete options.concise; + report(emit, input.sessionId); + emit({ type: "request.completed", requestId: input.requestId }); + return true; + } + return false; + }, + }); + const registry = new PluginAgentClientRegistry(createTestLogger()); + registry.replace([harness.registration]); + const session = await registry.clients()[harness.registration.id]!.createSession({ + provider: harness.registration.id, + cwd: tmpdir(), + providerOptions: { concise: true }, + }); + await session.run("A"); + await session.applyOptions?.({ concise: null }); + + await session.revertConversation!({ messageId: "user-A" }); + + const reopened = harness.inputs.findLast((input) => input.type === "session.open"); + expect(reopened?.config.providerOptions).toEqual({}); + await session.close(); + await registry.shutdown(); +}); + test("plugin rewind resumes the latest persistence rather than the original handle", async () => { await withPluginRewindSession( { @@ -1096,7 +1157,9 @@ describe("PluginAgentClientRegistry", () => { return true; } if (input.type === "session.configure") { - const { loud, token_budget: tokenBudget } = input.changes.providerOptions ?? {}; + const patch = + shape === "settings" ? input.changes.settings : input.changes.providerOptions; + const { loud, token_budget: tokenBudget } = patch ?? {}; // A plugin that takes `token_budget` live refuses the whole request on a bad value. if (tokenBudget !== undefined && typeof tokenBudget !== "number") { emit({ @@ -1201,11 +1264,18 @@ describe("PluginAgentClientRegistry", () => { await session.close(); }); - test("draws a control from what the plugin reports, with its icon and default", async () => { + test("draws a control from what the plugin reports", async () => { const { session } = await openStubbornVoiceSession({}); expect((session.controls ?? []).map(controlKey)).toEqual(["voice", "loud"]); - if (shape === "controls") { + await session.close(); + }); + + test.runIf(shape === "controls")( + "keeps a control's icon, default and choices as the plugin declared them", + async () => { + const { session } = await openStubbornVoiceSession({}); + expect(session.controls?.[0]).toMatchObject({ icon: "Mic", defaultValue: "direct", @@ -1214,9 +1284,9 @@ describe("PluginAgentClientRegistry", () => { { label: "Other", value: "other" }, ], }); - } - await session.close(); - }); + await session.close(); + }, + ); test.runIf(shape === "controls")( "sends a live option that has no control, and the plugin's answer decides", diff --git a/packages/server/src/server/agent/plugin-provider.ts b/packages/server/src/server/agent/plugin-provider.ts index f06f70079b..07181abc1c 100644 --- a/packages/server/src/server/agent/plugin-provider.ts +++ b/packages/server/src/server/agent/plugin-provider.ts @@ -1118,9 +1118,8 @@ class PluginAgentClient implements AgentClient { this.provider, bridge, forget, - async (persistence, state) => { + async (persistence, state, options) => { forget(); - const { options } = readPluginOptionState(state); return await this.runtime.openSession({ sessionId: randomUUID(), config: { @@ -1128,8 +1127,8 @@ class PluginAgentClient implements AgentClient { model: state.model, mode: state.mode, thinkingOption: state.thinkingOption, - settings: { ...config.settings, ...options }, - providerOptions: { ...config.providerOptions, ...options }, + settings: options, + providerOptions: options, }, persistence, history: "replay", @@ -1208,6 +1207,9 @@ class PluginAgentSession implements AgentSession { /** How the opened session differs from the options it was given. */ readonly openOptionsPatch: Record; private knownOptions: Record = {}; + // The options a reopened session starts with: what it launched with, then every value the + // plugin reported or was given, minus every key that was reset. + private readonly committedOptions: Record; /** While `applyOptions` waits on the plugin, its `session.config` answers that call. */ private applyingOptions = false; private currentTurnId: string | null = null; @@ -1228,14 +1230,16 @@ class PluginAgentSession implements AgentSession { private readonly reopen: ( persistence: ProviderPersistence, state: ProviderConfigState, + options: Readonly>, ) => Promise, private readonly onBind: (bridge: ProviderRuntimeSession) => void, private readonly givenOptions: Readonly>, ) { this.subagentIdsBySession.set(bridge.id, null); // Every setting the session opened with that was not given, including ones nobody set. + this.committedOptions = { ...givenOptions }; this.openOptionsPatch = this.optionDifferences(givenOptions); - this.knownOptions = this.currentOptions(); + this.commitOptions(); for (const event of bridge.history) this.accept(event, false); // Opening notices arrive before the manager subscribes to the session. // Commit them on registration and replay the same timestamps from history. @@ -1459,7 +1463,10 @@ class PluginAgentSession implements AgentSession { this.applyingOptions = false; } const ended = this.optionDifferences({ ...before, ...settings }); - this.knownOptions = this.currentOptions(); + for (const [id, value] of Object.entries(settings)) { + if (value === null) delete this.committedOptions[id]; + } + this.commitOptions(); return ended; } @@ -1655,11 +1662,21 @@ class PluginAgentSession implements AgentSession { return patch; } + /** Take the plugin's reported options as known; a key it stopped reporting was reset. */ + private commitOptions(): void { + const current = this.currentOptions(); + for (const id of Object.keys(this.knownOptions)) { + if (!(id in current)) delete this.committedOptions[id]; + } + Object.assign(this.committedOptions, current); + this.knownOptions = current; + } + private reportOptionChanges(): AgentStreamEvent[] { if (this.applyingOptions) return []; const patch = this.optionDifferences(this.knownOptions); if (Object.keys(patch).length === 0) return []; - this.knownOptions = this.currentOptions(); + this.commitOptions(); return [{ type: "options_changed", provider: this.provider, patch }]; } @@ -1818,7 +1835,7 @@ class PluginAgentSession implements AgentSession { await this.bridge.close(); replay.closed = true; } - const bridge = await this.reopen(replay.persistence, replay.config); + const bridge = await this.reopen(replay.persistence, replay.config, this.committedOptions); this.bridge = bridge; this.subagentIdsBySession.set(bridge.id, null); for (const event of bridge.history) this.accept(event, false); diff --git a/plugin-examples/provider-direct/server/provider.test.ts b/plugin-examples/provider-direct/server/provider.test.ts index 0b27b24cd7..3fbbbdb35e 100644 --- a/plugin-examples/provider-direct/server/provider.test.ts +++ b/plugin-examples/provider-direct/server/provider.test.ts @@ -98,11 +98,21 @@ describe("direct provider example", () => { type: "session.configure", requestId: "configure-refused", sessionId: "session-complete", - changes: { providerOptions: { voice: "loud" } }, + changes: { model: "other-model", providerOptions: { voice: "loud" } }, }); expect(events).toContainEqual( expect.objectContaining({ type: "request.failed", requestId: "configure-refused" }), ); + // A refused request changes nothing, the model it named included. + await connection.send({ + type: "session.configure", + requestId: "configure-after-refused", + sessionId: "session-complete", + changes: {}, + }); + expect(events.findLast((event) => event.type === "session.config")).not.toMatchObject({ + config: { model: "other-model" }, + }); await connection.send({ type: "session.prompt", sessionId: "session-complete", diff --git a/plugin-examples/provider-direct/server/provider.ts b/plugin-examples/provider-direct/server/provider.ts index e7735dd2f5..d4c83bf3a1 100644 --- a/plugin-examples/provider-direct/server/provider.ts +++ b/plugin-examples/provider-direct/server/provider.ts @@ -372,6 +372,19 @@ function configureSession( ): void { const session = requireSession(state, input.sessionId); const changes = input.changes; + // Work out everything first, so a refused request changes nothing. + let options: ExampleSession["options"]; + try { + options = applyOptions(session.options, changes.providerOptions ?? {}); + } catch (error) { + state.emit({ + type: "request.failed", + requestId: input.requestId, + error: { message: error instanceof Error ? error.message : String(error) }, + }); + return; + } + session.options = options; session.config = { ...session.config, model: changes.model === null ? undefined : (changes.model ?? session.config.model), @@ -381,16 +394,6 @@ function configureSession( ? undefined : (changes.thinkingOption ?? session.config.thinkingOption), }; - try { - session.options = applyOptions(session.options, changes.providerOptions ?? {}); - } catch (error) { - state.emit({ - type: "request.failed", - requestId: input.requestId, - error: { message: error instanceof Error ? error.message : String(error) }, - }); - return; - } state.emit({ type: "session.config", sessionId: input.sessionId, config: configState(session) }); state.emit({ type: "request.completed", requestId: input.requestId }); }