From 941257ab45d9b864eb7e2238ebca9d5f07311b5d Mon Sep 17 00:00:00 2001 From: klopez4212 Date: Mon, 22 Jun 2026 11:08:41 +0100 Subject: [PATCH] Address profile panel review feedback --- .../features/profile/ui/UserProfilePanel.tsx | 6 + .../ui/UserProfilePanelPersonaSubmit.test.mjs | 138 ++++++++++++++++++ .../ui/UserProfilePanelPersonaSubmit.ts | 47 ++++++ desktop/src/shared/api/tauriPersonas.ts | 2 + desktop/src/testing/e2eBridge.ts | 7 + 5 files changed, 200 insertions(+) create mode 100644 desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs diff --git a/desktop/src/features/profile/ui/UserProfilePanel.tsx b/desktop/src/features/profile/ui/UserProfilePanel.tsx index 7a9f84f7cf..47e04c5399 100644 --- a/desktop/src/features/profile/ui/UserProfilePanel.tsx +++ b/desktop/src/features/profile/ui/UserProfilePanel.tsx @@ -583,6 +583,12 @@ export function UserProfilePanel({ const handleConfirmDeletePersona = React.useCallback( async (personaToConfirm: AgentPersona) => { + if (personaToConfirm.sourceTeam) { + toast.error("This agent is managed by a team."); + setPersonaToDelete(null); + return; + } + try { const deletedInstances = await deleteManagedAgentsForPersona(personaToConfirm); diff --git a/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs b/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs new file mode 100644 index 0000000000..406cc3f096 --- /dev/null +++ b/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.test.mjs @@ -0,0 +1,138 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { validateLinkedAgentRuntimeEdit } from "./UserProfilePanelPersonaSubmit.ts"; + +function agent(overrides = {}) { + return { + pubkey: "deadbeef".repeat(8), + name: "Fizz", + personaId: "persona-1", + relayUrl: "ws://localhost:3000", + acpCommand: "buzz-acp", + agentCommand: "goose", + agentArgs: [], + mcpCommand: "", + turnTimeoutSeconds: 320, + idleTimeoutSeconds: null, + maxTurnDurationSeconds: null, + parallelism: 1, + systemPrompt: "Prompt", + avatarUrl: null, + model: null, + mcpToolsets: null, + envVars: {}, + status: "stopped", + pid: null, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + lastStartedAt: null, + lastStoppedAt: null, + lastExitCode: null, + lastError: null, + logPath: null, + startOnAppLaunch: true, + backend: { type: "local" }, + backendAgentId: null, + respondTo: "owner-only", + respondToAllowlist: [], + ...overrides, + }; +} + +function persona(overrides = {}) { + return { + id: "persona-1", + displayName: "Fizz", + avatarUrl: null, + systemPrompt: "Prompt", + runtime: "goose", + model: null, + provider: null, + namePool: [], + isBuiltIn: false, + isActive: true, + envVars: {}, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + ...overrides, + }; +} + +function updateInput(overrides = {}) { + return { + id: "persona-1", + displayName: "Fizz", + avatarUrl: undefined, + systemPrompt: "Prompt", + runtime: "claude", + model: undefined, + provider: undefined, + namePool: [], + ...overrides, + }; +} + +function runtime(overrides = {}) { + return { + id: "claude", + label: "Claude Code", + avatarUrl: "", + availability: "available", + command: "claude", + binaryPath: "/usr/local/bin/claude", + defaultArgs: [], + mcpCommand: null, + installHint: "", + installInstructionsUrl: "", + canAutoInstall: false, + underlyingCliPath: null, + ...overrides, + }; +} + +test("validateLinkedAgentRuntimeEdit allows available runtime changes", () => { + assert.equal( + validateLinkedAgentRuntimeEdit({ + input: updateInput({ runtime: "claude" }), + managedAgent: agent(), + previousPersona: persona({ runtime: "goose" }), + runtimes: [runtime()], + }), + null, + ); +}); + +test("validateLinkedAgentRuntimeEdit rejects unavailable linked-agent runtime changes", () => { + assert.equal( + validateLinkedAgentRuntimeEdit({ + input: updateInput({ runtime: "claude" }), + managedAgent: agent(), + previousPersona: persona({ runtime: "goose" }), + runtimes: [runtime({ availability: "cli_missing", command: null })], + }), + "Claude Code is not available. Install it before saving this linked agent.", + ); +}); + +test("validateLinkedAgentRuntimeEdit allows unchanged or unlinked runtime preferences", () => { + assert.equal( + validateLinkedAgentRuntimeEdit({ + input: updateInput({ runtime: "goose" }), + managedAgent: agent(), + previousPersona: persona({ runtime: "goose" }), + runtimes: [], + }), + null, + ); + + assert.equal( + validateLinkedAgentRuntimeEdit({ + input: updateInput({ runtime: "claude" }), + managedAgent: undefined, + 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 e667e710c1..eff9a1af10 100644 --- a/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.ts +++ b/desktop/src/features/profile/ui/UserProfilePanelPersonaSubmit.ts @@ -27,6 +27,42 @@ type SubmitProfilePersonaDialogOptions = { updatePersona: (input: UpdatePersonaInput) => Promise; }; +type ValidateLinkedAgentRuntimeEditOptions = { + input: UpdatePersonaInput; + managedAgent: ManagedAgent | undefined; + previousPersona?: AgentPersona; + runtimes?: readonly AcpRuntimeCatalogEntry[]; +}; + +function normalizeRuntimePreference(value: string | null | undefined): string { + return value?.trim() ?? ""; +} + +export function validateLinkedAgentRuntimeEdit({ + input, + managedAgent, + previousPersona, + runtimes, +}: ValidateLinkedAgentRuntimeEditOptions): string | null { + if (!managedAgent || !previousPersona) { + return null; + } + + const previousRuntime = normalizeRuntimePreference(previousPersona.runtime); + const nextRuntime = normalizeRuntimePreference(input.runtime); + if (previousRuntime === nextRuntime) { + return null; + } + + const runtime = runtimes?.find((candidate) => candidate.id === nextRuntime); + if (runtime?.availability === "available" && runtime.command) { + return null; + } + + const runtimeLabel = runtime?.label ?? "This provider"; + return `${runtimeLabel} is not available. Install it before saving this linked agent.`; +} + export async function submitProfilePersonaDialog({ createManagedAgentForPersona, createPersona, @@ -40,6 +76,17 @@ export async function submitProfilePersonaDialog({ }: SubmitProfilePersonaDialogOptions) { try { if ("id" in input) { + const runtimeEditError = validateLinkedAgentRuntimeEdit({ + input, + managedAgent, + previousPersona, + runtimes, + }); + if (runtimeEditError) { + toast.error(runtimeEditError); + return; + } + const persona = await updatePersona(input); const agentUpdate = managedAgent ? personaManagedAgentUpdate(managedAgent, persona, { diff --git a/desktop/src/shared/api/tauriPersonas.ts b/desktop/src/shared/api/tauriPersonas.ts index 1ec2c5ec82..ca9957f6f9 100644 --- a/desktop/src/shared/api/tauriPersonas.ts +++ b/desktop/src/shared/api/tauriPersonas.ts @@ -62,6 +62,7 @@ type RawPersona = { name_pool?: string[]; is_builtin: boolean; is_active?: boolean; + source_team?: string | null; env_vars?: Record; created_at: string; updated_at: string; @@ -79,6 +80,7 @@ function fromRawPersona(persona: RawPersona): AgentPersona { namePool: persona.name_pool ?? [], isBuiltIn: persona.is_builtin, isActive: persona.is_active ?? true, + sourceTeam: persona.source_team ?? null, envVars: persona.env_vars ?? {}, createdAt: persona.created_at, updatedAt: persona.updated_at, diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index d1b07dca93..219ba2f229 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -437,6 +437,7 @@ type RawPersona = { name_pool?: string[]; is_builtin: boolean; is_active: boolean; + source_team?: string | null; env_vars?: Record; created_at: string; updated_at: string; @@ -4785,6 +4786,7 @@ async function handleCreatePersona(args: { name_pool: [...(args.input.namePool ?? [])], is_builtin: false, is_active: true, + source_team: null, env_vars: { ...(args.input.envVars ?? {}) }, created_at: now, updated_at: now, @@ -4840,6 +4842,11 @@ async function handleDeletePersona(args: { id: string }): Promise { if (persona.is_builtin) { throw new Error("Built-in personas cannot be deleted."); } + if (persona.source_team) { + throw new Error( + `${persona.display_name} belongs to a team. Delete the team to remove all team personas together.`, + ); + } if (mockTeams.some((team) => team.persona_ids.includes(args.id))) { throw new Error( `${persona.display_name} is still referenced by a team. Remove it from those teams first.`,