From ce117a7f7bc13ee9097ea7deaf66b3da35a2fc61 Mon Sep 17 00:00:00 2001 From: Smith Labs LLC <232409717+SmithLabsLLC@users.noreply.github.com> Date: Tue, 4 Aug 2026 12:05:02 -0400 Subject: [PATCH] fix(desktop): align team agent deletion feedback Signed-off-by: Smith Labs LLC <232409717+SmithLabsLLC@users.noreply.github.com> --- desktop/src-tauri/src/commands/agents.rs | 16 +++-- .../src-tauri/src/commands/agents_tests.rs | 38 ++++++++++++ .../src-tauri/src/managed_agents/personas.rs | 41 ++++++++++--- .../src/managed_agents/personas/tests.rs | 17 +++++- desktop/src-tauri/src/managed_agents/teams.rs | 16 +++-- .../src/managed_agents/teams_tests.rs | 11 +++- .../lib/managedAgentControlActions.test.mjs | 61 +++++++++++++++++++ .../agents/lib/managedAgentControlActions.ts | 14 +++++ .../agents/ui/useManagedAgentActions.ts | 7 +++ .../profile/ui/UserProfilePanelDeletion.ts | 7 ++- desktop/src/testing/e2eBridge.ts | 31 ++++++---- desktop/tests/e2e/agents.spec.ts | 42 +++++++++++-- 12 files changed, 254 insertions(+), 47 deletions(-) diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 3b114b0474..912836730f 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -9,10 +9,10 @@ use crate::{ load_teams, managed_agent_avatar_url, normalize_agent_args, provider_deploy, resolve_provider_binary, save_managed_agents, start_managed_agent_process, stop_managed_agent_process, stop_managed_agent_workspace_pair, - sync_managed_agent_processes, try_regenerate_nest, validate_provider_config, BackendKind, - CreateManagedAgentRequest, CreateManagedAgentResponse, ManagedAgentRecord, - ManagedAgentSummary, RelayMeshConfig, DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM, - DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, + sync_managed_agent_processes, try_regenerate_nest, validate_managed_agent_team_deletion, + validate_provider_config, BackendKind, CreateManagedAgentRequest, + CreateManagedAgentResponse, ManagedAgentRecord, ManagedAgentSummary, RelayMeshConfig, + DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM, DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, }, relay::{relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, @@ -1304,12 +1304,10 @@ pub async fn delete_managed_agent( state.clear_agent_session_caches(pubkey); } - // Guard: reject deletion of deployed remote agents unless explicitly forced. - // This turns "don't orphan remote infra" from a UI convention into a backend - // invariant — a buggy or compromised IPC caller cannot silently orphan a live - // remote deployment. The frontend sends force_remote_delete: true only after - // the user confirms the orphan warning. + // Backend guards prevent a stale or compromised IPC caller from + // deleting current team members or silently orphaning remote infra. if let Some(record) = records.iter().find(|r| r.pubkey == pubkey) { + validate_managed_agent_team_deletion(record, &load_teams(&app)?)?; if record.backend != BackendKind::Local && record.backend_agent_id.is_some() && !force_remote_delete.unwrap_or(false) diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index df135298c4..efb86cb72c 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -90,6 +90,44 @@ fn persona_record(id: &str, model: Option<&str>, provider: Option<&str>) -> Agen } } +fn team_record(id: &str, persona_ids: &[&str]) -> crate::managed_agents::TeamRecord { + crate::managed_agents::TeamRecord { + id: id.to_string(), + name: "Test Team".to_string(), + description: None, + instructions: None, + persona_ids: persona_ids.iter().map(|id| (*id).to_string()).collect(), + is_builtin: false, + source_dir: None, + is_symlink: false, + symlink_target: None, + version: None, + created_at: "2026-08-04T00:00:00Z".to_string(), + updated_at: "2026-08-04T00:00:00Z".to_string(), + } +} + +#[test] +fn managed_agent_deletion_rejects_current_team_members() { + let record = bare_agent_record(Some("builtin:fizz"), None, None); + let teams = vec![team_record("team-1", &["builtin:fizz"])]; + + let error = validate_managed_agent_team_deletion(&record, &teams).unwrap_err(); + + assert_eq!( + error, + "Cannot remove Agent: this agent belongs to a team. Remove it from every team first." + ); +} + +#[test] +fn managed_agent_deletion_allows_agents_removed_from_teams() { + let record = bare_agent_record(Some("builtin:fizz"), None, None); + let teams = vec![team_record("team-1", &["builtin:honey"])]; + + assert!(validate_managed_agent_team_deletion(&record, &teams).is_ok()); +} + /// Auto-archive uses the same NIP-IA wire builder as the explicit GUI action, /// attaches owner consent, and marks a deliberate delete as `retired`. #[test] diff --git a/desktop/src-tauri/src/managed_agents/personas.rs b/desktop/src-tauri/src/managed_agents/personas.rs index 9bf7ab74b0..b08c8679da 100644 --- a/desktop/src-tauri/src/managed_agents/personas.rs +++ b/desktop/src-tauri/src/managed_agents/personas.rs @@ -2,7 +2,10 @@ use std::fs; use tauri::AppHandle; -use crate::{managed_agents::AgentDefinition, util::now_iso}; +use crate::{ + managed_agents::{AgentDefinition, ManagedAgentRecord, TeamRecord}, + util::now_iso, +}; struct BuiltInPersona { id: &'static str, @@ -288,16 +291,37 @@ pub fn validate_persona_deletion( )); } + validate_agent_not_in_team(&persona.display_name, referenced_by_team)?; + + Ok(()) +} + +pub fn validate_agent_not_in_team( + display_name: &str, + referenced_by_team: bool, +) -> Result<(), String> { if referenced_by_team { return Err(format!( - "{} is still referenced by a team. Remove it from those teams first.", - persona.display_name + "Cannot remove {display_name}: this agent belongs to a team. \ + Remove it from every team first." )); } Ok(()) } +pub fn validate_managed_agent_team_deletion( + record: &ManagedAgentRecord, + teams: &[TeamRecord], +) -> Result<(), String> { + let referenced_by_team = record.persona_id.as_deref().is_some_and(|persona_id| { + teams + .iter() + .any(|team| team.persona_ids.iter().any(|id| id == persona_id)) + }); + validate_agent_not_in_team(&record.name, referenced_by_team) +} + pub fn validate_persona_activation_change( persona: &AgentDefinition, active: bool, @@ -308,16 +332,13 @@ pub fn validate_persona_activation_change( return Err("Only built-in agents can be added to or removed from My Agents.".to_string()); } - if !active && referenced_by_managed_agent { - return Err(format!( - "{} is still assigned to a managed agent. Remove or reassign those agents first.", - persona.display_name - )); + if !active { + validate_agent_not_in_team(&persona.display_name, referenced_by_team)?; } - if !active && referenced_by_team { + if !active && referenced_by_managed_agent { return Err(format!( - "{} is still referenced by a team. Remove it from those teams first.", + "{} is still assigned to a managed agent. Remove or reassign those agents first.", persona.display_name )); } diff --git a/desktop/src-tauri/src/managed_agents/personas/tests.rs b/desktop/src-tauri/src/managed_agents/personas/tests.rs index 387b4d72c6..5d36c4af1d 100644 --- a/desktop/src-tauri/src/managed_agents/personas/tests.rs +++ b/desktop/src-tauri/src/managed_agents/personas/tests.rs @@ -224,7 +224,20 @@ fn validate_persona_activation_change_rejects_team_references() { assert_eq!( err, - "Fizz is still referenced by a team. Remove it from those teams first." + "Cannot remove Fizz: this agent belongs to a team. Remove it from every team first." + ); +} + +#[test] +fn validate_persona_activation_change_prioritizes_team_membership() { + let mut persona = custom_persona("builtin:fizz", "Fizz"); + persona.is_builtin = true; + + let err = validate_persona_activation_change(&persona, false, true, true).unwrap_err(); + + assert_eq!( + err, + "Cannot remove Fizz: this agent belongs to a team. Remove it from every team first." ); } @@ -255,7 +268,7 @@ fn validate_persona_deletion_rejects_team_references() { assert_eq!( err, - "Alpha is still referenced by a team. Remove it from those teams first." + "Cannot remove Alpha: this agent belongs to a team. Remove it from every team first." ); } diff --git a/desktop/src-tauri/src/managed_agents/teams.rs b/desktop/src-tauri/src/managed_agents/teams.rs index 937893d531..73ee8a36f2 100644 --- a/desktop/src-tauri/src/managed_agents/teams.rs +++ b/desktop/src-tauri/src/managed_agents/teams.rs @@ -229,6 +229,15 @@ fn agents_referencing_team<'a>( .collect() } +fn team_in_use_deletion_error(team_id: &str, agent_names: &[&str]) -> String { + format!( + "Cannot delete team \"{team_id}\": {} agent(s) still reference it ({}). \ + Edit the team to remove those agents, then delete or reconfigure their instances.", + agent_names.len(), + agent_names.join(", ") + ) +} + /// Delete a team, cascading removal of its sourced personas and backing dir. /// /// Returns the d-tags of the personas removed by the cascade so the caller can @@ -248,12 +257,7 @@ pub fn delete_team_with_cascade(app: &AppHandle, team_id: &str) -> Result { assert.equal(calledWith, "deadbeef".repeat(8)); }); +test("remote team members are rejected before shutdown or deletion", async () => { + const teamAgent = agent({ + backend: { + type: "provider", + id: "remote-host", + config: {}, + }, + backendAgentId: "remote-agent-1", + name: "Team Agent", + personaId: "custom:team-agent", + status: "deployed", + }); + let deleteCalled = false; + + await assert.rejects( + deleteManagedAgentWithRules({ + agent: teamAgent, + channels: [{ id: "channel-1", name: "general" }], + deleteManagedAgent: async () => { + deleteCalled = true; + }, + presenceLookup: { + [teamAgent.pubkey]: "online", + }, + relayAgents: [ + { + pubkey: teamAgent.pubkey, + channelIds: ["channel-1"], + channels: ["general"], + }, + ], + teams: [{ personaIds: ["custom:team-agent"] }], + }), + { + message: + "Cannot remove Team Agent: this agent belongs to a team. Remove it from every team first.", + }, + ); + assert.equal(deleteCalled, false); +}); + +test("agents removed from their teams can still be deleted", async () => { + const formerTeamAgent = agent({ + personaId: "custom:former-team-agent", + }); + let deletedPubkey = null; + + await deleteManagedAgentWithRules({ + agent: formerTeamAgent, + channels: [], + deleteManagedAgent: async ({ pubkey }) => { + deletedPubkey = pubkey; + }, + relayAgents: [], + teams: [{ personaIds: ["custom:other-agent"] }], + }); + + assert.equal(deletedPubkey, formerTeamAgent.pubkey); +}); + // --- respawnManagedAgentWithRules: stop→clear→start boundary tests ----------- test("test_respawn_stop_success_start_failure_onStopped_still_fires", async () => { diff --git a/desktop/src/features/agents/lib/managedAgentControlActions.ts b/desktop/src/features/agents/lib/managedAgentControlActions.ts index dbaaaba803..94235ce231 100644 --- a/desktop/src/features/agents/lib/managedAgentControlActions.ts +++ b/desktop/src/features/agents/lib/managedAgentControlActions.ts @@ -1,5 +1,6 @@ import { sendChannelMessage } from "@/shared/api/tauri"; import type { + AgentTeam, Channel, ManagedAgent, PresenceLookup, @@ -24,6 +25,7 @@ type ManagedAgentChannelContext = { type ManagedAgentActionContext = ManagedAgentChannelContext & { presenceLookup?: PresenceLookup | null; + teams: readonly AgentTeam[]; }; export type ManagedAgentActionResult = { @@ -149,11 +151,23 @@ export async function deleteManagedAgentWithRules({ presenceLookup, relayAgents, skipRemoteDeleteConfirm = false, + teams, }: { agent: ManagedAgent; deleteManagedAgent: DeleteManagedAgent; skipRemoteDeleteConfirm?: boolean; } & ManagedAgentActionContext): Promise { + const personaId = agent.personaId; + const belongsToTeam = + personaId !== null && + teams.some((team) => team.personaIds.includes(personaId)); + if (belongsToTeam) { + throw new Error( + `Cannot remove ${agent.name}: this agent belongs to a team. ` + + "Remove it from every team first.", + ); + } + if (agent.backend.type === "provider" && agent.backendAgentId) { const presence = presenceLookup?.[normalizePubkey(agent.pubkey)]; const channelId = resolveManagedAgentChannelId(agent, { diff --git a/desktop/src/features/agents/ui/useManagedAgentActions.ts b/desktop/src/features/agents/ui/useManagedAgentActions.ts index e1c2e9c9fc..9e487a075f 100644 --- a/desktop/src/features/agents/ui/useManagedAgentActions.ts +++ b/desktop/src/features/agents/ui/useManagedAgentActions.ts @@ -22,6 +22,7 @@ import type { ManagedAgent, } from "@/shared/api/types"; import { removeChannelMember } from "@/shared/api/tauri"; +import { listTeams } from "@/shared/api/tauriTeams"; import { normalizePubkey } from "@/shared/lib/pubkey"; import { deleteManagedAgentWithRules, @@ -237,6 +238,10 @@ export function useManagedAgentActions() { return result.data ?? []; } + async function getTeamsForAction() { + return listTeams(); + } + async function handleStop(pubkey: string) { clearFeedback(); try { @@ -284,12 +289,14 @@ export function useManagedAgentActions() { const agent = managedAgents.find((a) => a.pubkey === pubkey); if (!agent) return; const channels = await getChannelsForAction(); + const teams = await getTeamsForAction(); const result = await deleteManagedAgentWithRules({ agent, channels, deleteManagedAgent: deleteMutation.mutateAsync, presenceLookup: managedPresenceQuery.data, relayAgents: relayAgentsQuery.data ?? [], + teams, }); if (result.cancelled) return; await removeAgentFromAllChannels(pubkey); diff --git a/desktop/src/features/profile/ui/UserProfilePanelDeletion.ts b/desktop/src/features/profile/ui/UserProfilePanelDeletion.ts index e52864727f..43c65e7172 100644 --- a/desktop/src/features/profile/ui/UserProfilePanelDeletion.ts +++ b/desktop/src/features/profile/ui/UserProfilePanelDeletion.ts @@ -5,6 +5,7 @@ import { type ManagedAgentActionResult, } from "@/features/agents/lib/managedAgentControlActions"; import { removeChannelMember } from "@/shared/api/tauri"; +import { listTeams } from "@/shared/api/tauriTeams"; import type { AgentPersona, Channel, @@ -72,7 +73,7 @@ export function useProfileAgentDeletion({ ); const deleteManagedAgentRecord = React.useCallback( - (agentToDelete: ManagedAgent) => + async (agentToDelete: ManagedAgent) => deleteProfileManagedAgent(agentToDelete, { channels: channels ?? [], deleteManagedAgent, @@ -80,6 +81,7 @@ export function useProfileAgentDeletion({ relayAgents: relayAgents ?? [], removeAgentFromAllChannels, skipRemoteDeleteConfirm: true, + teams: await listTeams(), }), [ channels, @@ -91,7 +93,7 @@ export function useProfileAgentDeletion({ ); const deleteManagedAgentsForPersona = React.useCallback( - (persona: AgentPersona) => + async (persona: AgentPersona) => deleteProfileManagedAgentsForPersona(persona, { channels: channels ?? [], deleteManagedAgent, @@ -100,6 +102,7 @@ export function useProfileAgentDeletion({ relayAgents: relayAgents ?? [], removeAgentFromAllChannels, selectedAgent: managedAgent, + teams: await listTeams(), }), [ channels, diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 33b1fb38f4..ebf39e35e4 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -7885,6 +7885,17 @@ async function applyMockPersonaUpdate( return persona; } +function teamMembershipDeletionError(displayName: string): string { + return `Cannot remove ${displayName}: this agent belongs to a team. Remove it from every team first.`; +} + +function personaIsCurrentTeamMember(personaId: string | null): boolean { + return ( + personaId !== null && + mockTeams.some((team) => team.persona_ids.includes(personaId)) + ); +} + async function handleDeletePersona(args: { id: string }): Promise { const persona = mockPersonas.find((candidate) => candidate.id === args.id); if (!persona) { @@ -7893,10 +7904,8 @@ async function handleDeletePersona(args: { id: string }): Promise { if (persona.is_builtin) { throw new Error("Built-in agents cannot be deleted."); } - 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.`, - ); + if (personaIsCurrentTeamMember(args.id)) { + throw new Error(teamMembershipDeletionError(persona.display_name)); } mockPersonas = mockPersonas.filter((candidate) => candidate.id !== args.id); @@ -7922,6 +7931,9 @@ async function handleSetPersonaActive(args: { "Only built-in agents can be added to or removed from My Agents.", ); } + if (!args.active && personaIsCurrentTeamMember(args.id)) { + throw new Error(teamMembershipDeletionError(persona.display_name)); + } if ( !args.active && mockManagedAgents.some((agent) => agent.persona_id === args.id) @@ -7930,14 +7942,6 @@ async function handleSetPersonaActive(args: { `${persona.display_name} is still assigned to a managed agent. Remove or reassign those agents first.`, ); } - if ( - !args.active && - 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.`, - ); - } persona.is_active = args.active; persona.updated_at = new Date().toISOString(); @@ -8509,6 +8513,9 @@ async function handleDeleteManagedAgent(args: { // Model the backend invariant: reject deletion of deployed remote agents // unless force_remote_delete is true. const agent = mockManagedAgents.find((a) => a.pubkey === args.pubkey); + if (agent && personaIsCurrentTeamMember(agent.persona_id)) { + throw new Error(teamMembershipDeletionError(agent.name)); + } if ( agent && agent.backend.type === "provider" && diff --git a/desktop/tests/e2e/agents.spec.ts b/desktop/tests/e2e/agents.spec.ts index 3cbe097c05..e4636bfd8c 100644 --- a/desktop/tests/e2e/agents.spec.ts +++ b/desktop/tests/e2e/agents.spec.ts @@ -2387,9 +2387,22 @@ test("inactive built-ins cannot be used to create teams", async ({ page }) => { expect(error).toBe("Honey is not in My Agents."); }); -test("built-in removal failures show up from My Agents", async ({ page }) => { +test("team members show the same delete error from card and profile", async ({ + page, +}) => { + const honeyPubkey = TEST_IDENTITIES.alice.pubkey; + const teamDeleteError = + "Cannot remove Honey: this agent belongs to a team. Remove it from every team first."; await installMockBridge(page, { activePersonaIds: ["builtin:honey"], + managedAgents: [ + { + pubkey: honeyPubkey, + name: "Honey", + personaId: "builtin:honey", + status: "running", + }, + ], }); await gotoApp(page); @@ -2404,10 +2417,29 @@ test("built-in removal failures show up from My Agents", async ({ page }) => { await page.getByLabel("Open actions for Honey").click(); await page.getByRole("menuitem", { name: "Delete" }).click(); + const cardErrorToast = page + .locator("[data-sonner-toast]") + .filter({ hasText: teamDeleteError }); + await expect(cardErrorToast).toBeVisible(); + await expect(cardErrorToast).not.toBeVisible({ timeout: 10_000 }); + + await page.getByTestId("persona-agent-row-builtin:honey").click(); + await page.getByTestId("user-profile-settings-menu-trigger").click(); + await page.getByTestId(`user-profile-agent-delete-${honeyPubkey}`).click(); + await page.getByTestId("agent-delete-confirm-action").click(); + await expect( - page - .locator("[data-sonner-toast]") - .filter({ hasText: "Honey is still referenced by a team." }), + page.locator("[data-sonner-toast]").filter({ hasText: teamDeleteError }), + ).toBeVisible(); + await expect( + page.locator("[data-sonner-toast]").filter({ hasText: "Deleted Honey." }), + ).toHaveCount(0); + await expect( + page.getByTestId(`user-profile-agent-delete-${honeyPubkey}`), + ).toHaveCount(0); + await page.getByTestId("user-profile-settings-menu-trigger").click(); + await expect( + page.getByTestId(`user-profile-agent-delete-${honeyPubkey}`), ).toBeVisible(); }); @@ -2433,7 +2465,7 @@ test("personas referenced by teams cannot be deleted", async ({ page }) => { }); expect(error).toBe( - "Analyst is still referenced by a team. Remove it from those teams first.", + "Cannot remove Analyst: this agent belongs to a team. Remove it from every team first.", ); });