From 8469e98765ad020a63a3b2a10dc6fb5c7716f9d8 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 8 Nov 2026 12:57:40 +0100 Subject: [PATCH] Send a profile's options on every write, empty included The profile form always sends options. The daemon reads a write without them as one from an app that predates options and folds featureValues over the stored ones; a write that carries them, empty included, replaces them. A profile left with no options is stored without the key, so removing the last one persists. --- .../internal/profile-form-model.test.ts | 7 +- .../internal/profile-form-model.ts | 5 +- .../src/server/agent/option-controls.ts | 29 ++++--- .../src/server/daemon-config-store.test.ts | 76 +++++++++++-------- 4 files changed, 67 insertions(+), 50 deletions(-) diff --git a/packages/app/src/agent-profiles/internal/profile-form-model.test.ts b/packages/app/src/agent-profiles/internal/profile-form-model.test.ts index 7ec5733593..4756bc127a 100644 --- a/packages/app/src/agent-profiles/internal/profile-form-model.test.ts +++ b/packages/app/src/agent-profiles/internal/profile-form-model.test.ts @@ -174,6 +174,7 @@ describe("openAgentProfileForm", () => { provider: "codex", model: "gpt-5.2-codex", modeId: "read-only", + options: {}, }); }); @@ -544,7 +545,9 @@ describe("openAgentProfileForm", () => { model.setModel("claude-haiku-4-5", { label: "Haiku 4.5" }); model.applyControls(model.getState().controlsRequestKey ?? "", NO_CONTROLS); - expect(model.getState().submitValue?.options).toBeUndefined(); + // Empty, not absent: the daemon reads a profile write without `options` as one from an + // app that predates them and keeps the stored options. + expect(model.getState().submitValue?.options).toEqual({}); expect(model.getState().controlsRequest?.providerOptions).toBeUndefined(); }); @@ -576,7 +579,7 @@ describe("openAgentProfileForm", () => { model.setModel("claude-haiku-4-5", { label: "Haiku 4.5" }); model.applyControls(model.getState().controlsRequestKey ?? "", NO_CONTROLS); - expect(model.getState().submitValue?.options).toBeUndefined(); + expect(model.getState().submitValue?.options).toEqual({}); }); it("never drops an option no control has drawn", () => { diff --git a/packages/app/src/agent-profiles/internal/profile-form-model.ts b/packages/app/src/agent-profiles/internal/profile-form-model.ts index 5320993f6f..37756376bd 100644 --- a/packages/app/src/agent-profiles/internal/profile-form-model.ts +++ b/packages/app/src/agent-profiles/internal/profile-form-model.ts @@ -391,7 +391,6 @@ function buildSubmitValue(state: AgentProfileFormState): AgentProfileValue | nul if (!name || !state.provider) { return null; } - const options = ownOptionsOf(state); return { name, ...(state.icon ? { icon: state.icon } : {}), @@ -400,7 +399,9 @@ function buildSubmitValue(state: AgentProfileFormState): AgentProfileValue | nul ...(state.modelId ? { model: state.modelId } : {}), ...(state.modeId ? { modeId: state.modeId } : {}), ...(state.thinkingOptionId ? { thinkingOptionId: state.thinkingOptionId } : {}), - ...(Object.keys(options).length > 0 ? { options } : {}), + // Always sent, empty included: the daemon reads a write without `options` as one from an + // app that predates them and keeps the profile's stored options. + options: ownOptionsOf(state), ...(notes ? { notes } : {}), }; } diff --git a/packages/server/src/server/agent/option-controls.ts b/packages/server/src/server/agent/option-controls.ts index f3893fcfef..1fd13cb8f6 100644 --- a/packages/server/src/server/agent/option-controls.ts +++ b/packages/server/src/server/agent/option-controls.ts @@ -158,12 +158,13 @@ interface ProfileOptionFields { * Profiles saved before options carry `featureValues`; when one comes from storage, `options` * wins over a stale `featureValues` key. * - * A write that carries `options` is from a client that knows them: its `featureValues` fold - * over those. A write without `options` is from the unchanged app, whose form only knows the - * controls: its `featureValues` are a patch (`null` deletes) over the options stored for that - * profile, so options the form never shows, such as `sandbox_mode`, survive an edit. A profile - * moved to another provider starts from no options, because the old provider's keys mean - * nothing to the new one. + * A write that carries `options`, empty included, is from a client that knows them: its + * `featureValues` fold over those. A write without `options` is from the unchanged app, whose + * form only knows the controls: its `featureValues` are a patch (`null` deletes) over the + * options stored for that profile, so options the form never shows, such as `sandbox_mode`, + * survive an edit. A profile moved to another provider starts from no options, because the old + * provider's keys mean nothing to the new one. A profile with no options is stored without the + * key. */ export function foldProfileFeatureValues(input: { profile: T; @@ -183,15 +184,13 @@ export function foldProfileFeatureValues(input: { return ownOptions ? { ...rest, options: ownOptions } : rest; } const base = - ownOptions ?? - (stored?.provider === rest.provider ? convert(provider, stored?.options) : undefined); - // The form sends no `featureValues` when no control is set, which is still an edit of a - // profile that may hold other options. - if (!featureValues && !base) return rest; - return { - ...rest, - options: applyProviderOptionsPatch({ options: base, patch: featureValues ?? {} }), - }; + rest.options !== undefined + ? ownOptions + : stored?.provider === rest.provider + ? convert(provider, stored?.options) + : undefined; + const options = applyProviderOptionsPatch({ options: base, patch: featureValues ?? {} }); + return Object.keys(options).length > 0 ? { ...rest, options } : { ...rest, options: undefined }; } /** diff --git a/packages/server/src/server/daemon-config-store.test.ts b/packages/server/src/server/daemon-config-store.test.ts index 1bb6680259..0478a145c4 100644 --- a/packages/server/src/server/daemon-config-store.test.ts +++ b/packages/server/src/server/daemon-config-store.test.ts @@ -250,32 +250,46 @@ describe("DaemonConfigStore", () => { ]); }); - describe("a profile edited by the unchanged app", () => { - function storeWithProfile(): DaemonConfigStore { - const paseoHome = mkdtempSync(path.join(tmpdir(), "paseo-daemon-config-store-")); - tempDirs.push(paseoHome); - return new DaemonConfigStore(paseoHome, { - relay: { enabled: false }, - mcp: { injectIntoAgents: false }, - browserTools: { enabled: false }, - providers: {}, - metadataGeneration: { providers: [] }, - autoArchiveAfterMerge: false, - enableTerminalAgentHooks: false, - appendSystemPrompt: "", - agentProfiles: [ - { - id: "p", - name: "P", - provider: "codex", - options: { sandbox_mode: "read-only", plan_mode: false }, - }, - ], - }); - } + function storeWithProfile(): { store: DaemonConfigStore; paseoHome: string } { + const paseoHome = mkdtempSync(path.join(tmpdir(), "paseo-daemon-config-store-")); + tempDirs.push(paseoHome); + const store = new DaemonConfigStore(paseoHome, { + relay: { enabled: false }, + mcp: { injectIntoAgents: false }, + browserTools: { enabled: false }, + providers: {}, + metadataGeneration: { providers: [] }, + autoArchiveAfterMerge: false, + enableTerminalAgentHooks: false, + appendSystemPrompt: "", + agentProfiles: [ + { + id: "p", + name: "P", + provider: "codex", + options: { sandbox_mode: "read-only", plan_mode: false }, + }, + ], + }); + return { store, paseoHome }; + } + test("a profile written with empty options, as the form sends it once the last one is removed, keeps none", async () => { + const { store, paseoHome } = storeWithProfile(); + + await store.patch({ + agentProfiles: [{ id: "p", name: "P", provider: "codex", options: {} }], + }); + + expect(store.get().agentProfiles).toEqual([{ id: "p", name: "P", provider: "codex" }]); + expect(loadPersistedConfig(paseoHome).daemon?.agentProfiles).toEqual([ + { id: "p", name: "P", provider: "codex" }, + ]); + }); + + describe("a profile edited by the unchanged app", () => { test("keeps the options its form never shows when it changes a control", async () => { - const store = storeWithProfile(); + const { store } = storeWithProfile(); await store.patch({ agentProfiles: [ @@ -294,7 +308,7 @@ describe("DaemonConfigStore", () => { }); test("keeps every option when it sends no featureValues at all", async () => { - const store = storeWithProfile(); + const { store } = storeWithProfile(); await store.patch({ agentProfiles: [{ id: "p", name: "Renamed", provider: "codex" }] }); @@ -309,7 +323,7 @@ describe("DaemonConfigStore", () => { }); test("starts from no options when it moves the profile to another provider", async () => { - const store = storeWithProfile(); + const { store } = storeWithProfile(); await store.patch({ agentProfiles: [ @@ -323,7 +337,7 @@ describe("DaemonConfigStore", () => { }); test("drops the old provider's options when it moves the profile with no featureValues", async () => { - const store = storeWithProfile(); + const { store } = storeWithProfile(); await store.patch({ agentProfiles: [{ id: "p", name: "P", provider: "claude" }] }); @@ -331,7 +345,7 @@ describe("DaemonConfigStore", () => { }); test("folds featureValues over the options a write carries, not over the stored ones", async () => { - const store = storeWithProfile(); + const { store } = storeWithProfile(); await store.patch({ agentProfiles: [ @@ -356,7 +370,7 @@ describe("DaemonConfigStore", () => { }); test("converts a Codex profile's legacy fast_mode to service_tier", async () => { - const store = storeWithProfile(); + const { store } = storeWithProfile(); await store.patch({ agentProfiles: [ @@ -375,7 +389,7 @@ describe("DaemonConfigStore", () => { }); test("removes an option whose featureValues entry is null", async () => { - const store = storeWithProfile(); + const { store } = storeWithProfile(); await store.patch({ agentProfiles: [ @@ -465,7 +479,7 @@ describe("DaemonConfigStore", () => { agentProfiles: [{ id, name: id, provider: id, featureValues: { service_tier: null } }], }); - expect(store.get().agentProfiles).toEqual([{ id, name: id, provider: id, options: {} }]); + expect(store.get().agentProfiles).toEqual([{ id, name: id, provider: id }]); }); test("a provider edit in the same write does not hide which adapter a profile runs on", async () => {