From f11a1c68e257f1c91dad9e7cd3e15606b31abbd9 Mon Sep 17 00:00:00 2001 From: klopez4212 Date: Thu, 25 Jun 2026 14:23:15 +0100 Subject: [PATCH] Address profile sidebar review feedback --- .../features/profile/ui/UserProfilePanel.tsx | 15 +++----- .../ui/UserProfilePanelPersonaSubmit.test.mjs | 12 ++++++ .../ui/UserProfilePanelPersonaSubmit.ts | 3 ++ .../profile/ui/UserProfilePanelUtils.test.mjs | 38 ++++++++++++++++--- .../profile/ui/UserProfilePanelUtils.ts | 30 ++++++++++++++- 5 files changed, 82 insertions(+), 16 deletions(-) diff --git a/desktop/src/features/profile/ui/UserProfilePanel.tsx b/desktop/src/features/profile/ui/UserProfilePanel.tsx index f7beecea4..4b6e7a1bf 100644 --- a/desktop/src/features/profile/ui/UserProfilePanel.tsx +++ b/desktop/src/features/profile/ui/UserProfilePanel.tsx @@ -151,16 +151,13 @@ export function UserProfilePanel({ const personasQuery = usePersonasQuery(); const managedAgentsQuery = useManagedAgentsQuery({ enabled: true }); const managedAgent = React.useMemo(() => { + if (!pubkey) { + return undefined; + } const agents = managedAgentsQuery.data ?? []; - if (pubkey) { - const pubkeyLower = pubkey.toLowerCase(); - return agents.find((agent) => agent.pubkey.toLowerCase() === pubkeyLower); - } - if (persona) { - return agents.find((agent) => agent.personaId === persona.id); - } - return undefined; - }, [managedAgentsQuery.data, persona, pubkey]); + const pubkeyLower = pubkey.toLowerCase(); + return agents.find((agent) => agent.pubkey.toLowerCase() === pubkeyLower); + }, [managedAgentsQuery.data, pubkey]); const resolvedPersonaFromSource = React.useMemo(() => { const personaId = persona?.id ?? managedAgent?.personaId; if (personaId) { diff --git a/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs b/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs index 406cc3f09..7f264748c 100644 --- a/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs +++ b/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs @@ -136,3 +136,15 @@ test("validateLinkedAgentRuntimeEdit allows unchanged or unlinked runtime prefer null, ); }); + +test("validateLinkedAgentRuntimeEdit allows clearing linked runtime preference", () => { + assert.equal( + validateLinkedAgentRuntimeEdit({ + input: updateInput({ runtime: undefined }), + managedAgent: agent(), + previousPersona: persona({ runtime: "goose" }), + runtimes: [], + }), + null, + ); +}); diff --git a/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.ts b/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.ts index eff9a1af1..00e2ee817 100644 --- a/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.ts +++ b/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.ts @@ -53,6 +53,9 @@ export function validateLinkedAgentRuntimeEdit({ if (previousRuntime === nextRuntime) { return null; } + if (!nextRuntime) { + return null; + } const runtime = runtimes?.find((candidate) => candidate.id === nextRuntime); if (runtime?.availability === "available" && runtime.command) { diff --git a/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs b/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs index 287e944d5..9af5fe3bc 100644 --- a/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs +++ b/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs @@ -113,16 +113,26 @@ test("personaManagedAgentUpdate skips unrelated or unchanged agents", () => { test("personaManagedAgentUpdate maps changed persona runtime to linked agent commands", () => { assert.deepEqual( - personaManagedAgentUpdate(agent(), persona({ runtime: "claude" }), { - previousPersona: persona({ runtime: "goose" }), - runtimes: [runtime()], - }), + personaManagedAgentUpdate( + agent({ envVars: { SHARED: "old", AGENT_ONLY: "keep" } }), + persona({ + runtime: "claude", + envVars: { SHARED: "new", PERSONA_ONLY: "set" }, + }), + { + previousPersona: persona({ + runtime: "goose", + envVars: { SHARED: "old" }, + }), + runtimes: [runtime()], + }, + ), { pubkey: "deadbeef".repeat(8), name: "Fizz Prime", systemPrompt: "New prompt", model: "new-model", - envVars: { NEW_KEY: "2" }, + envVars: { SHARED: "new", PERSONA_ONLY: "set", AGENT_ONLY: "keep" }, agentCommand: "claude", agentArgs: ["mcp", "serve"], mcpCommand: "claude-mcp", @@ -130,6 +140,24 @@ test("personaManagedAgentUpdate maps changed persona runtime to linked agent com ); }); +test("personaManagedAgentUpdate preserves agent env overrides when persona env is unchanged", () => { + assert.deepEqual( + personaManagedAgentUpdate( + agent({ + name: "Fizz Prime", + systemPrompt: "New prompt", + model: "new-model", + envVars: { API_KEY: "agent-secret" }, + }), + persona({ envVars: { API_KEY: "persona-default" } }), + { + previousPersona: persona({ envVars: { API_KEY: "persona-default" } }), + }, + ), + null, + ); +}); + test("personaManagedAgentUpdate leaves runtime fields alone when runtime is unchanged", () => { assert.equal( personaManagedAgentUpdate( diff --git a/desktop/src/features/profile/ui/UserProfilePanelUtils.ts b/desktop/src/features/profile/ui/UserProfilePanelUtils.ts index 026c2dbc4..d5bd0506b 100644 --- a/desktop/src/features/profile/ui/UserProfilePanelUtils.ts +++ b/desktop/src/features/profile/ui/UserProfilePanelUtils.ts @@ -254,8 +254,13 @@ export function personaManagedAgentUpdate( hasChanges = true; } - if (!stringRecordEqual(persona.envVars, agent.envVars)) { - input.envVars = persona.envVars; + const nextEnvVars = mergedPersonaEnvVarsForAgent( + agent, + persona, + options.previousPersona, + ); + if (!stringRecordEqual(nextEnvVars, agent.envVars)) { + input.envVars = nextEnvVars; hasChanges = true; } @@ -286,6 +291,27 @@ export function personaManagedAgentUpdate( return hasChanges ? input : null; } +function mergedPersonaEnvVarsForAgent( + agent: ManagedAgent, + persona: AgentPersona, + previousPersona: AgentPersona | undefined, +) { + if (!previousPersona) { + return persona.envVars; + } + if (stringRecordEqual(persona.envVars, previousPersona.envVars)) { + return agent.envVars; + } + + const nextEnvVars = { ...persona.envVars }; + for (const [key, value] of Object.entries(agent.envVars)) { + if (previousPersona.envVars[key] !== value) { + nextEnvVars[key] = value; + } + } + return nextEnvVars; +} + function stringArrayEqual(left: readonly string[], right: readonly string[]) { if (left.length !== right.length) return false;