diff --git a/packages/server/src/server/agent/agent-manager.ts b/packages/server/src/server/agent/agent-manager.ts index 5681d8e636..f3beed5d27 100644 --- a/packages/server/src/server/agent/agent-manager.ts +++ b/packages/server/src/server/agent/agent-manager.ts @@ -2323,9 +2323,18 @@ export class AgentManager { * `settings.features` sends composer values as `featureValues`; this is where they become an * options patch for the agent's provider. */ - async setAgentFeatureValues(agentId: string, values: Record): Promise { + async setAgentFeatureValues(input: { + agentId: string; + values: Record; + /** Options sent with the values; they win over a value of the same name. */ + options?: Record; + }): Promise { + const { agentId, values, options } = input; const provider = this.requireAgent(agentId).provider; - await this.setAgentOptions(agentId, convertLegacyOptions({ provider, options: values })); + await this.setAgentOptions(agentId, { + ...convertLegacyOptions({ provider, options: values }), + ...options, + }); } /** The config as the opened session left it: its own difference from the options it was given. */ diff --git a/packages/server/src/server/agent/mcp-server.test.ts b/packages/server/src/server/agent/mcp-server.test.ts index 1d24d942e9..de878bcde0 100644 --- a/packages/server/src/server/agent/mcp-server.test.ts +++ b/packages/server/src/server/agent/mcp-server.test.ts @@ -4718,8 +4718,9 @@ describe("update_agent MCP tool", () => { expect(spies.agentManager.setAgentMode).toHaveBeenCalledWith("agent-1", "full-access"); expect(spies.agentManager.setAgentModel).toHaveBeenCalledWith("agent-1", "gpt-5.4"); expect(spies.agentManager.setAgentThinkingOption).toHaveBeenCalledWith("agent-1", "high"); - expect(spies.agentManager.setAgentFeatureValues).toHaveBeenCalledWith("agent-1", { - fast_mode: true, + expect(spies.agentManager.setAgentFeatureValues).toHaveBeenCalledWith({ + agentId: "agent-1", + values: { fast_mode: true }, }); expect(spies.agentManager.updateAgentMetadata).toHaveBeenCalledWith("agent-1", { title: "Updated agent", diff --git a/packages/server/src/server/agent/option-controls.test.ts b/packages/server/src/server/agent/option-controls.test.ts index 7e3198d00f..c9d7a88bbc 100644 --- a/packages/server/src/server/agent/option-controls.test.ts +++ b/packages/server/src/server/agent/option-controls.test.ts @@ -49,4 +49,17 @@ describe("legacy Codex Fast at the other entry points", () => { service_tier: "fast", }); }); + + test("a hook that replaces the speed it was sent with fast_mode selects Fast", () => { + const sent = { + provider: "codex", + providerOptions: { service_tier: "default" }, + featureValues: { service_tier: "default" }, + }; + const returned = { ...sent, featureValues: { fast_mode: true } }; + + expect(foldHookFeatureValues({ returned, sent }).providerOptions).toEqual({ + service_tier: "fast", + }); + }); }); diff --git a/packages/server/src/server/agent/option-controls.ts b/packages/server/src/server/agent/option-controls.ts index 846b840233..2bbb1a1e6a 100644 --- a/packages/server/src/server/agent/option-controls.ts +++ b/packages/server/src/server/agent/option-controls.ts @@ -111,7 +111,9 @@ export function foldHookFeatureValues< // No copy was sent, so everything the hook put in `featureValues` is new. return foldFeatureValues({ config: returned, provider: returned.provider }); } - const after = featureValues ?? {}; + // The copy the hook was sent is canonical; what it returns may name the old key, so both + // sides are compared in the same form and a replaced speed is not read as a deletion. + const after = convertLegacyOptions({ provider: returned.provider, options: featureValues ?? {} }); const patch: ProviderOptionsPatch = {}; for (const key of new Set([...Object.keys(before), ...Object.keys(after)])) { if (isDeepStrictEqual(after[key], before[key])) continue; @@ -124,7 +126,7 @@ export function foldHookFeatureValues< provider: returned.provider, options: rest.providerOptions ?? undefined, }), - patch: convertLegacyOptions({ provider: returned.provider, options: patch }), + patch, }), }; } 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 292130b1f1..1857fa7ba5 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 @@ -797,7 +797,7 @@ describe("Codex app-server provider controls", () => { const agentId = await create(); expect(shownSpeed(agentId).selected).toBe("default"); - await manager.setAgentFeatureValues(agentId, { fast_mode: true }); + await manager.setAgentFeatureValues({ agentId, values: { fast_mode: true } }); expect(shownSpeed(agentId)).toMatchObject({ own: { service_tier: "priority" }, ...FAST }); await runTurn(agentId, "priority"); @@ -818,7 +818,24 @@ describe("Codex app-server provider controls", () => { const { manager, create, shownSpeed, runTurn } = createLegacyFastHarness(); const agentId = await create({ service_tier: "priority" }); - await manager.setAgentFeatureValues(agentId, { service_tier: "default", fast_mode: true }); + await manager.setAgentFeatureValues({ + agentId, + values: { service_tier: "default", fast_mode: true }, + }); + + expect(shownSpeed(agentId)).toMatchObject({ own: { service_tier: "default" } }); + await runTurn(agentId, "default"); + }); + + test("options sent with the older values replace them before anything is applied", async () => { + const { manager, create, shownSpeed, runTurn } = createLegacyFastHarness(); + const agentId = await create({ service_tier: "priority" }); + + await manager.setAgentFeatureValues({ + agentId, + values: { service_tier: "retired-tier" }, + options: { service_tier: "default" }, + }); expect(shownSpeed(agentId)).toMatchObject({ own: { service_tier: "default" } }); await runTurn(agentId, "default"); @@ -828,7 +845,7 @@ describe("Codex app-server provider controls", () => { const { manager, create, shownSpeed, runTurn } = createLegacyFastHarness(); const agentId = await create({ service_tier: "priority" }); - await manager.setAgentFeatureValues(agentId, { fast_mode: false }); + await manager.setAgentFeatureValues({ agentId, values: { fast_mode: false } }); expect(shownSpeed(agentId)).toMatchObject({ own: { service_tier: "default" } }); await runTurn(agentId, "default"); diff --git a/packages/server/src/server/agent/tools/paseo-tools.ts b/packages/server/src/server/agent/tools/paseo-tools.ts index 4e40c9b249..f83d982ac8 100644 --- a/packages/server/src/server/agent/tools/paseo-tools.ts +++ b/packages/server/src/server/agent/tools/paseo-tools.ts @@ -2292,7 +2292,7 @@ function createToolCatalog( await agentManager.setAgentThinkingOption(agentId, settings.thinkingOptionId); } if (settings?.features) { - await agentManager.setAgentFeatureValues(agentId, settings.features); + await agentManager.setAgentFeatureValues({ agentId, values: settings.features }); } await updateAgentCommand({ agentManager }, { agentId, name, labels }); diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index 26f6cd5614..927db78077 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -1067,7 +1067,7 @@ export class Session { (await setAgentModeCommand({ agentManager }, { agentId, modeId })).notice, setModel: (agentId, modelId) => agentManager.setAgentModel(agentId, modelId), setOptions: (agentId, patch) => agentManager.setAgentOptions(agentId, patch), - setFeatureValues: (agentId, values) => agentManager.setAgentFeatureValues(agentId, values), + setFeatureValues: (input) => agentManager.setAgentFeatureValues(input), setThinking: (agentId, thinkingOptionId) => agentManager.setAgentThinkingOption(agentId, thinkingOptionId), }, diff --git a/packages/server/src/server/session/agent-config/agent-config-session.test.ts b/packages/server/src/server/session/agent-config/agent-config-session.test.ts index 084cfcbe45..a9869f0118 100644 --- a/packages/server/src/server/session/agent-config/agent-config-session.test.ts +++ b/packages/server/src/server/session/agent-config/agent-config-session.test.ts @@ -13,7 +13,11 @@ class FakeAgentConfigOperations implements AgentConfigOperations { readonly modeCalls: Array<{ agentId: string; modeId: string }> = []; readonly modelCalls: Array<{ agentId: string; modelId: string | null }> = []; readonly optionsCalls: Array<{ agentId: string; patch: Record }> = []; - readonly featureValueCalls: Array<{ agentId: string; values: Record }> = []; + readonly featureValueCalls: Array<{ + agentId: string; + values: Record; + options?: Record; + }> = []; readonly thinkingCalls: Array<{ agentId: string; thinkingOptionId: string | null }> = []; /** Cross-operation ordering, which the per-operation arrays above cannot show. */ readonly callLog: string[] = []; @@ -46,8 +50,12 @@ class FakeAgentConfigOperations implements AgentConfigOperations { if (this.failWith) throw this.failWith; } - async setFeatureValues(agentId: string, values: Record): Promise { - this.featureValueCalls.push({ agentId, values }); + async setFeatureValues(input: { + agentId: string; + values: Record; + options?: Record; + }): Promise { + this.featureValueCalls.push(input); this.callLog.push("featureValues"); if (this.failWith) throw this.failWith; } @@ -366,7 +374,7 @@ describe("AgentConfigSession", () => { ]); }); - test("apply: legacy featureValues are applied first and options after, so options win", async () => { + test("apply: legacy featureValues and options go in one call, options winning", async () => { const { subsystem, operations } = makeSubsystem(); await subsystem.handleAgentConfigApplyRequest({ @@ -379,11 +387,14 @@ describe("AgentConfigSession", () => { requestId: "req-1", }); - expect(operations.callLog).toEqual(["featureValues", "options"]); + expect(operations.callLog).toEqual(["featureValues"]); expect(operations.featureValueCalls).toEqual([ - { agentId: "agent-1", values: { fast_mode: true, service_tier: "fast" } }, + { + agentId: "agent-1", + values: { fast_mode: true, service_tier: "fast" }, + options: { fast_mode: false }, + }, ]); - expect(operations.optionsCalls).toEqual([{ agentId: "agent-1", patch: { fast_mode: false } }]); }); test("apply: leaves omitted fields alone", async () => { diff --git a/packages/server/src/server/session/agent-config/agent-config-session.ts b/packages/server/src/server/session/agent-config/agent-config-session.ts index 9f857a1103..db471c111f 100644 --- a/packages/server/src/server/session/agent-config/agent-config-session.ts +++ b/packages/server/src/server/session/agent-config/agent-config-session.ts @@ -31,7 +31,11 @@ export interface AgentConfigOperations { setModel(agentId: string, modelId: string | null): Promise; setOptions(agentId: string, patch: ProviderOptionsPatch): Promise; /** COMPAT(featureValues): added in v0.15.0, remove after 2027-05-01. Older clients' values. */ - setFeatureValues(agentId: string, values: Record): Promise; + setFeatureValues(input: { + agentId: string; + values: Record; + options?: ProviderOptionsPatch; + }): Promise; setThinking( agentId: string, thinkingOptionId: string | null, @@ -117,7 +121,7 @@ export class AgentConfigSession { failureText: "Failed to set agent feature", // COMPAT(featureValues): added in v0.15.0, remove after 2027-05-01. A feature is a one-key options patch. run: async () => { - await this.operations.setFeatureValues(agentId, { [featureId]: value }); + await this.operations.setFeatureValues({ agentId, values: { [featureId]: value } }); return undefined; }, emitResponse: (payload) => this.host.emit({ type: "set_agent_feature_response", payload }), @@ -194,11 +198,16 @@ export class AgentConfigSession { notice ??= thinkingNotice; } // COMPAT(featureValues): added in v0.15.0, remove after 2027-05-01. `featureValues` is an options patch. - if (config.featureValues && Object.keys(config.featureValues).length > 0) { - await this.operations.setFeatureValues(agentId, config.featureValues); - } - if (config.options && Object.keys(config.options).length > 0) { - await this.operations.setOptions(agentId, config.options); + const hasValues = config.featureValues && Object.keys(config.featureValues).length > 0; + const hasOptions = config.options && Object.keys(config.options).length > 0; + if (hasValues) { + await this.operations.setFeatureValues({ + agentId, + values: config.featureValues ?? {}, + options: config.options, + }); + } else if (hasOptions) { + await this.operations.setOptions(agentId, config.options ?? {}); } return notice;