diff --git a/docs/providers.md b/docs/providers.md index 7bf9db65e2..510ddd7677 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -82,8 +82,9 @@ or without a control. profile saves its stored options plus the form's edits, so options no control draws survive an edit. A daemon without `server_info.features.providerOptionControls` sends `features` and `featureValues`; `packages/client/src/compat/legacy-provider-options.ts` is the one place that - turns those into `controls` and `options` and back, so nothing in the app branches on it. Its - first create for a provider lists that provider's features to learn which options go out as + turns those into `controls` and `options` and back, so nothing in the app branches on it. A + create that carries options first lists the controls for its own provider, model, mode and + thinking level, since which options are controls differs by model, and sends those as `featureValues`; callers do not prime anything. - **Provider-initiated changes.** A setting the provider changes on its own is an `options_changed` event. The daemon stores and forwards it and does not push the old value back. diff --git a/packages/client/src/compat/legacy-provider-options.ts b/packages/client/src/compat/legacy-provider-options.ts index 9c015cfa71..ca095b39c8 100644 --- a/packages/client/src/compat/legacy-provider-options.ts +++ b/packages/client/src/compat/legacy-provider-options.ts @@ -24,14 +24,6 @@ export function usesLegacyProviderOptions(info: ServerInfoStatusPayload): boolea } export class LegacyProviderOptions { - /** Option keys a legacy daemon exposes as features, per provider, as last seen. */ - private readonly controlKeys = new Map>(); - - /** Whether the keys a provider exposes as features are known, so options can be routed. */ - knowsControlKeys(provider: string): boolean { - return this.controlKeys.has(provider); - } - /** Gives agent snapshots `controls` and `options`, built from their `features`. */ normalize(message: SessionOutboundMessage): SessionOutboundMessage { switch (message.type) { @@ -98,27 +90,42 @@ export class LegacyProviderOptions { config: this.daemonConfig(message.payload.config as MutableDaemonConfig), }, } as SessionOutboundMessage; - case "list_provider_features_response": - // Remember the keys even when the caller used the legacy listing directly. - this.remember(message.payload.provider, message.payload.features); - return message; default: return message; } } - /** Sends control keys as `featureValues`, which is where a legacy daemon reads them. */ + /** + * Splits a create's options for a legacy daemon: the ids it lists as features for that very + * config go out as `featureValues`, which is where it reads them, and the rest stay + * `providerOptions`. Which options are features depends on the model, mode and thinking level + * as well as the provider, so the ids come from a listing for this config, never from memory. + */ + routeCreateConfig< + T extends { + providerOptions?: Record; + featureValues?: Record; + }, + >(config: T, featureIds: ReadonlySet): T { + if (!config.providerOptions) return config; + const featureValues: Record = {}; + const providerOptions: Record = {}; + for (const [key, value] of Object.entries(config.providerOptions)) { + (featureIds.has(key) ? featureValues : providerOptions)[key] = value; + } + const { providerOptions: _options, ...rest } = config; + return { + ...(rest as T), + ...(Object.keys(providerOptions).length > 0 ? { providerOptions } : {}), + ...(Object.keys(featureValues).length > 0 + ? { featureValues: { ...config.featureValues, ...featureValues } } + : {}), + }; + } + + /** Sends options as `featureValues`, which is where a legacy daemon reads them. */ prepare(message: SessionInboundMessage): SessionInboundMessage { switch (message.type) { - case "create_agent_request": - case "agent.create.request": - return { ...message, config: this.config(message.config) }; - case "workspace.create.request": - if (!message.agent) return message; - return { - ...message, - agent: { ...message.agent, config: this.config(message.agent.config) }, - }; case "agent.config.apply.request": { const { options, ...config } = message.config; if (!options) return message; @@ -127,9 +134,17 @@ export class LegacyProviderOptions { config: { ...config, featureValues: { ...config.featureValues, ...options } }, }; } - case "list_commands_request": - if (!message.draftConfig) return message; - return { ...message, draftConfig: this.config(message.draftConfig) }; + case "list_commands_request": { + if (!message.draftConfig?.providerOptions) return message; + const { providerOptions, ...draftConfig } = message.draftConfig; + return { + ...message, + draftConfig: { + ...draftConfig, + featureValues: { ...draftConfig.featureValues, ...providerOptions }, + }, + }; + } case "set_daemon_config_request": if (!message.config.agentProfiles) return message; return { @@ -149,7 +164,6 @@ export class LegacyProviderOptions { legacy: ListProviderFeaturesResponseMessage["payload"], ): ProviderControlsListResponseMessage["payload"] { const { features, ...rest } = legacy; - this.remember(legacy.provider, features); return { ...rest, ...(features @@ -165,49 +179,12 @@ export class LegacyProviderOptions { private readonly snapshot = (agent: AgentSnapshotPayload): AgentSnapshotPayload => { if (agent.controls || !agent.features) return agent; - this.remember(agent.provider, agent.features); return { ...agent, controls: controlsFromFeatures(agent.features), options: optionsFromFeatures(agent.features), }; }; - - private remember(provider: string, features: Parameters[0]): void { - if (features) this.controlKeys.set(provider, new Set(features.map((feature) => feature.id))); - } - - private config< - T extends { - provider?: string; - providerOptions?: Record; - featureValues?: Record; - }, - >(config: T): T { - if (!config.provider || !config.providerOptions) return config; - const { featureValues, providerOptions } = this.split(config.provider, config.providerOptions); - const { providerOptions: _options, ...rest } = config; - return { - ...(rest as T), - ...(Object.keys(providerOptions).length > 0 ? { providerOptions } : {}), - ...(Object.keys(featureValues).length > 0 - ? { featureValues: { ...config.featureValues, ...featureValues } } - : {}), - }; - } - - private split( - provider: string, - options: Record, - ): { featureValues: Record; providerOptions: Record } { - const keys = this.controlKeys.get(provider); - const featureValues: Record = {}; - const providerOptions: Record = {}; - for (const [key, value] of Object.entries(options)) { - (keys?.has(key) ? featureValues : providerOptions)[key] = value; - } - return { featureValues, providerOptions }; - } } /** A legacy daemon stores a profile's options under `featureValues`. */ diff --git a/packages/client/src/daemon-client.test.ts b/packages/client/src/daemon-client.test.ts index 66489d0b14..3aee37c38c 100644 --- a/packages/client/src/daemon-client.test.ts +++ b/packages/client/src/daemon-client.test.ts @@ -9163,19 +9163,36 @@ describe("provider options across daemon versions", () => { }); }); - test("a cold legacy workspace create routes its agent's options the same way", async () => { + test("a legacy workspace create routes its agent's options from its own model's controls", async () => { const { client, sentMessages, mock } = await connect({ creationLifecycle: true }); + mock.triggerMessage( + wrapSessionMessage({ + type: "agent_update", + payload: { + kind: "upsert", + agent: { ...agentSnapshot, provider: "codex", model: "mini", features: [] }, + }, + }), + ); void client .createWorkspace({ source: { kind: "directory", path: "/repo" }, agent: { - config: { provider: "codex", cwd: "/repo", providerOptions: { service_tier: "fast" } }, + config: { + provider: "codex", + cwd: "/repo", + model: "gpt-5.5", + providerOptions: { service_tier: "fast" }, + }, }, } as never) .catch(() => {}); await vi.waitFor(() => expect(sentMessages()).toHaveLength(1)); const [listing] = sentMessages(); - expect(listing).toMatchObject({ type: "list_provider_features_request" }); + expect(listing).toMatchObject({ + type: "list_provider_features_request", + draftConfig: { provider: "codex", cwd: "/repo", model: "gpt-5.5" }, + }); mock.triggerMessage( wrapSessionMessage({ type: "list_provider_features_response", @@ -9196,27 +9213,58 @@ describe("provider options across daemon versions", () => { }); }); - test("a legacy daemon creates an agent with control keys as feature values", async () => { + test("a legacy create lists the controls of its own model, whatever a snapshot showed", async () => { const { client, sentMessages, mock } = await connect({}); + // A Haiku agent has no Fast control; that says nothing about Opus. mock.triggerMessage( wrapSessionMessage({ type: "agent_update", - payload: { kind: "upsert", agent: { ...agentSnapshot, features: [speedFeature] } }, + payload: { + kind: "upsert", + agent: { ...agentSnapshot, provider: "claude", model: "haiku", features: [] }, + }, }), ); void client .createAgent({ config: { - provider: "codex", + provider: "claude", cwd: "/repo", - providerOptions: { service_tier: "priority", sandbox_mode: "read-only" }, + model: "opus", + modeId: "plan", + thinkingOptionId: "high", + providerOptions: { fast_mode: true, sandbox_mode: "read-only" }, }, }) .catch(() => {}); await vi.waitFor(() => expect(sentMessages()).toHaveLength(1)); - const [request] = sentMessages(); - expect(request.config).toMatchObject({ - featureValues: { service_tier: "priority" }, + const [listing] = sentMessages(); + expect(listing).toMatchObject({ + type: "list_provider_features_request", + draftConfig: { + provider: "claude", + cwd: "/repo", + model: "opus", + modeId: "plan", + thinkingOptionId: "high", + }, + }); + mock.triggerMessage( + wrapSessionMessage({ + type: "list_provider_features_response", + payload: { + provider: "claude", + features: [{ type: "toggle", id: "fast_mode", label: "Fast", value: false }], + error: null, + fetchedAt: "2026-04-30T00:00:00.000Z", + requestId: listing.requestId, + }, + }), + ); + await vi.waitFor(() => expect(sentMessages()).toHaveLength(2)); + expect(sentMessages()[1].config).toMatchObject({ + model: "opus", + featureValues: { fast_mode: true }, providerOptions: { sandbox_mode: "read-only" }, }); }); diff --git a/packages/client/src/daemon-client.ts b/packages/client/src/daemon-client.ts index d47180911a..512db1da1d 100644 --- a/packages/client/src/daemon-client.ts +++ b/packages/client/src/daemon-client.ts @@ -3270,36 +3270,41 @@ export class DaemonClient { } if (options.background !== undefined) this.requireBackgroundWorkspaces(); const config = resolveAgentConfig(options); - // Awaited only when there is something to learn, so other creates still send synchronously. - const learning = this.learnLegacyControlKeys(config); - if (learning) await learning; - const result = await this.creations.createAgent({ ...options, config }); + const result = await this.creations.createAgent({ + ...options, + config: this.carriesLegacyOptions(config) + ? await this.routeLegacyCreateConfig(config) + : config, + }); if (result.error || !result.agent) throw new Error(result.error ?? "Agent creation failed"); return result.agent; } /** - * A legacy daemon reads control options as `featureValues`, so routing a create's options needs - * the provider's feature ids. Callers never prime that: the first create for a provider lists - * them, and the answer is remembered with the snapshots and listings that also teach it. + * A legacy daemon reads control options as `featureValues`, and which options are controls + * depends on the config (Claude's differ by model), so a create that carries options first lists + * the controls for its own config. Callers never prime anything. */ - private learnLegacyControlKeys(config: { - provider?: string; - cwd?: string; - providerOptions?: Record; - }): Promise | undefined { - const legacy = this.legacyProviderOptions(); - const { provider, providerOptions } = config; - if (!legacy || !provider || !providerOptions || Object.keys(providerOptions).length === 0) { - return undefined; - } - if (legacy.knowsControlKeys(provider)) return undefined; - return this.listFeaturesOrThrow(provider, config.cwd || "~"); + private carriesLegacyOptions(config: AgentSessionConfig): boolean { + return ( + this.legacyProviderOptions() !== null && Object.keys(config.providerOptions ?? {}).length > 0 + ); } - private async listFeaturesOrThrow(provider: string, cwd: string): Promise { - const payload = await this.listProviderFeatures({ provider, cwd }); + private async routeLegacyCreateConfig(config: AgentSessionConfig): Promise { + const payload = await this.listProviderFeatures({ + provider: config.provider, + cwd: config.cwd || "~", + ...(config.modeId ? { modeId: config.modeId } : {}), + ...(config.model ? { model: config.model } : {}), + ...(config.thinkingOptionId ? { thinkingOptionId: config.thinkingOptionId } : {}), + featureValues: config.providerOptions, + }); if (payload.error) throw new Error(payload.error); + return this.legacyOptions.routeCreateConfig( + config, + new Set((payload.features ?? []).map((feature) => feature.id)), + ); } private async createLegacyAgent( @@ -5053,9 +5058,10 @@ export class DaemonClient { if (input.background !== undefined || input.callerAgentId) this.requireBackgroundWorkspaces(); if (input.agent?.background !== undefined) throw new Error("Configure background on the workspace, not its initial agent."); - const agentConfig = input.agent ? resolveAgentConfig(input.agent) : undefined; - const learning = agentConfig ? this.learnLegacyControlKeys(agentConfig) : undefined; - if (learning) await learning; + let agentConfig = input.agent ? resolveAgentConfig(input.agent) : undefined; + if (agentConfig && this.carriesLegacyOptions(agentConfig)) { + agentConfig = await this.routeLegacyCreateConfig(agentConfig); + } const result = await this.creations.createWorkspace({ ...input, requestId: resolvedRequestId,