From d0a5d4d970e5bf37b0ed05ca7bab9bb5230bfcef Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 2 Nov 2026 01:43:01 +0100 Subject: [PATCH] fix(server): an apply never reads a requested option as one the plugin dropped A plugin may run an option it never reports. Only a key it reported before can be reported as removed, and a rewind reopens with the accepted request plus what the plugin reported back. --- .../src/server/agent/plugin-provider.test.ts | 77 +++++++++++++++++++ .../src/server/agent/plugin-provider.ts | 34 +++++--- 2 files changed, 99 insertions(+), 12 deletions(-) diff --git a/packages/server/src/server/agent/plugin-provider.test.ts b/packages/server/src/server/agent/plugin-provider.test.ts index e93283c390..50b0d284ad 100644 --- a/packages/server/src/server/agent/plugin-provider.test.ts +++ b/packages/server/src/server/agent/plugin-provider.test.ts @@ -566,6 +566,83 @@ test("a rewind reopens with the options as they stand, a reset included", async await registry.shutdown(); }); +// A plugin may report no options at all while it runs the ones it was given. +async function withUnreportedOptionSession( + run: ( + session: Awaited>, + harness: ReturnType, + ) => Promise, +) { + const harness = createRewindProviderHarness({ + handleInput: async (input, emit) => { + if (input.type === "session.open" && input.history !== "replay") { + 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, + }); + 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: [], + controls: [], + providerOptions: {}, + }, + }); + emit({ type: "session.ready", requestId: input.requestId, sessionId: input.sessionId }); + return true; + } + if (input.type === "session.configure") { + 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: { token_budget: 1000 }, + }); + try { + await run(session, harness); + } finally { + await session.close(); + await registry.shutdown(); + } +} + +test("applying an option the plugin never reports leaves the stored option alone", async () => { + await withUnreportedOptionSession(async (session) => { + // The same value as a whole-profile patch carries it, then a new one. + expect(await session.applyOptions?.({ token_budget: 1000 })).toEqual({}); + expect(await session.applyOptions?.({ token_budget: 2000 })).toEqual({}); + }); +}); + +test("a rewind reopens with the value the plugin accepted for an option it never reports", async () => { + await withUnreportedOptionSession(async (session, harness) => { + await session.run("A"); + await session.applyOptions?.({ token_budget: 2000 }); + + await session.revertConversation!({ messageId: "user-A" }); + + const reopened = harness.inputs.findLast((input) => input.type === "session.open"); + expect(reopened?.config.providerOptions).toEqual({ token_budget: 2000 }); + }); +}); + test("plugin rewind resumes the latest persistence rather than the original handle", async () => { await withPluginRewindSession( { diff --git a/packages/server/src/server/agent/plugin-provider.ts b/packages/server/src/server/agent/plugin-provider.ts index 60350ec9aa..9e2508f83b 100644 --- a/packages/server/src/server/agent/plugin-provider.ts +++ b/packages/server/src/server/agent/plugin-provider.ts @@ -1462,11 +1462,10 @@ class PluginAgentSession implements AgentSession { } finally { this.applyingOptions = false; } - const ended = this.optionChanges({ ...before, ...settings }); - for (const [id, value] of Object.entries(settings)) { - if (value === null) delete this.committedOptions[id]; - } - this.commitOptions(); + // A requested key is a request, not a report: only a key the plugin reported before can be + // one it has now dropped. + const ended = this.optionChanges({ ...before, ...settings }, before); + this.commitOptions(settings); return ended; } @@ -1662,8 +1661,16 @@ class PluginAgentSession implements AgentSession { return patch; } - /** Take the plugin's reported options as known; a key it stopped reporting was reset. */ - private commitOptions(): void { + /** + * Settle the options a reopened session starts with: what is committed, then the request the + * plugin accepted, then what it reported back. A key it reported before and no longer reports + * was reset. + */ + private commitOptions(accepted: Readonly> = {}): void { + for (const [id, value] of Object.entries(accepted)) { + if (value === null) delete this.committedOptions[id]; + else this.committedOptions[id] = value; + } const current = this.currentOptions(); for (const id of Object.keys(this.knownOptions)) { if (!(id in current)) delete this.committedOptions[id]; @@ -1674,20 +1681,23 @@ class PluginAgentSession implements AgentSession { /** * What changed against the options `known` to be set: values that differ, and `null` for a key - * the plugin no longer reports, which went back to its default. + * in `reported` that the plugin no longer reports, which went back to its default. */ - private optionChanges(known: Readonly>): Record { + private optionChanges( + known: Readonly>, + reported: Readonly>, + ): Record { const patch = this.optionDifferences(known); const current = this.currentOptions(); - for (const [id, value] of Object.entries(known)) { - if (value !== null && !(id in current)) patch[id] = null; + for (const id of Object.keys(reported)) { + if (known[id] !== null && !(id in current)) patch[id] = null; } return patch; } private reportOptionChanges(): AgentStreamEvent[] { if (this.applyingOptions) return []; - const patch = this.optionChanges(this.knownOptions); + const patch = this.optionChanges(this.knownOptions, this.knownOptions); if (Object.keys(patch).length === 0) return []; this.commitOptions(); return [{ type: "options_changed", provider: this.provider, patch }];