From 335a426982a276024512ba7ce2fd7fd94f4983bc Mon Sep 17 00:00:00 2001 From: Wes Date: Mon, 6 Apr 2026 15:43:51 -0700 Subject: [PATCH] Polish members sidebar UX and clean up orphaned agents (#252) Co-authored-by: Claude Opus 4.6 (1M context) --- .../src-tauri/src/managed_agents/runtime.rs | 54 ++- .../features/channels/cleanupChannelAgents.ts | 88 ++-- desktop/src/features/channels/hooks.ts | 86 +--- .../src/features/channels/lib/channelCache.ts | 66 +++ .../src/features/channels/lib/memberUtils.ts | 42 ++ .../channels/lib/useClassifiedMembers.ts | 72 +++ .../channels/ui/ChannelManagementSheet.tsx | 417 ++++++------------ .../channels/ui/ChannelMemberInviteCard.tsx | 180 +++++--- .../channels/ui/ChannelMembersBar.tsx | 78 +--- .../features/channels/ui/ChannelScreen.tsx | 10 + .../features/channels/ui/MembersSidebar.tsx | 226 ++++++++++ .../channels/useLiveChannelUpdates.ts | 6 +- desktop/src/features/messages/hooks.ts | 2 +- desktop/tests/e2e/channels.spec.ts | 155 ++++++- 14 files changed, 934 insertions(+), 548 deletions(-) create mode 100644 desktop/src/features/channels/lib/channelCache.ts create mode 100644 desktop/src/features/channels/lib/memberUtils.ts create mode 100644 desktop/src/features/channels/lib/useClassifiedMembers.ts create mode 100644 desktop/src/features/channels/ui/MembersSidebar.tsx diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 0fe5e7125..37ad92772 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -110,22 +110,47 @@ pub(crate) fn process_belongs_to_us(_pid: u32) -> bool { } #[cfg(unix)] -pub(crate) fn terminate_process(pid: u32) -> Result<(), String> { - // The child was spawned with process_group(0), so pid == pgid. - // Kill the entire process group to avoid orphaning MCP servers - // and agent subprocesses. +fn signal_process_group_or_leader(pid: u32, signal: i32, action: &str) -> Result<(), String> { let pgid = -(pid as i32); - // Try graceful shutdown first (SIGTERM to the group). - if unsafe { libc::kill(pgid, libc::SIGTERM) } != 0 { - // ESRCH means the process is already gone — that's fine. - let err = std::io::Error::last_os_error(); - if err.raw_os_error() != Some(libc::ESRCH) && process_is_running(pid) { - return Err(format!("failed to terminate process group {pid}: {err}")); - } + if unsafe { libc::kill(pgid, signal) } == 0 { return Ok(()); } + let group_err = std::io::Error::last_os_error(); + if !process_is_running(pid) { + return Ok(()); + } + + // Some local agent trees can no longer be signalled as a process group + // (for example if the leader changed groups, or macOS returns EPERM for one + // descendant). Fall back to the leader PID so stop/delete can still recover. + if matches!( + group_err.raw_os_error(), + Some(libc::EPERM) | Some(libc::ESRCH) + ) { + if unsafe { libc::kill(pid as i32, signal) } == 0 { + return Ok(()); + } + + let leader_err = std::io::Error::last_os_error(); + if leader_err.raw_os_error() == Some(libc::ESRCH) || !process_is_running(pid) { + return Ok(()); + } + + return Err(format!("failed to {action} process {pid}: {leader_err}")); + } + + Err(format!( + "failed to {action} process group {pid}: {group_err}" + )) +} + +#[cfg(unix)] +pub(crate) fn terminate_process(pid: u32) -> Result<(), String> { + // Try graceful shutdown first (SIGTERM to the group). + signal_process_group_or_leader(pid, libc::SIGTERM, "terminate")?; + // Wait up to 1s for graceful exit. for _ in 0..10 { if !process_is_running(pid) { @@ -135,12 +160,7 @@ pub(crate) fn terminate_process(pid: u32) -> Result<(), String> { } // Escalate to SIGKILL on the entire group. - if unsafe { libc::kill(pgid, libc::SIGKILL) } != 0 { - let err = std::io::Error::last_os_error(); - if err.raw_os_error() != Some(libc::ESRCH) && process_is_running(pid) { - return Err(format!("failed to kill process group {pid}: {err}")); - } - } + signal_process_group_or_leader(pid, libc::SIGKILL, "kill")?; Ok(()) } diff --git a/desktop/src/features/channels/cleanupChannelAgents.ts b/desktop/src/features/channels/cleanupChannelAgents.ts index 9fce5ba8f..3c01d6b2e 100644 --- a/desktop/src/features/channels/cleanupChannelAgents.ts +++ b/desktop/src/features/channels/cleanupChannelAgents.ts @@ -1,9 +1,9 @@ /** - * Best-effort cleanup of managed agents when a channel is deleted. + * Best-effort cleanup of channel-scoped managed agents. * - * Each agent added via the "Add agents" dialog is a unique process scoped to - * the channel. When the channel is deleted these orphaned agents should be - * removed — but only if they are not members of any other channel. + * Each agent added via the "Add agents" dialog is a dedicated managed-agent + * record. If that agent is no longer present in any channel, the managed-agent + * record should be removed as well. */ import { deleteManagedAgent, @@ -12,40 +12,66 @@ import { listRelayAgents, } from "@/shared/api/tauri"; -export async function cleanupChannelAgents(channelId: string): Promise { - const [members, managedAgents, relayAgents] = await Promise.all([ - getChannelMembers(channelId), +async function cleanupManagedAgentsByPubkey( + pubkeys: readonly string[], + options?: { ignoreChannelId?: string }, +): Promise { + const normalizedPubkeys = new Set( + pubkeys + .map((pubkey) => pubkey.trim().toLowerCase()) + .filter((pubkey) => pubkey.length > 0), + ); + + if (normalizedPubkeys.size === 0) { + return; + } + + const [managedAgents, relayAgents] = await Promise.all([ listManagedAgents(), listRelayAgents(), ]); - const memberPubkeys = new Set( - members.map((member) => member.pubkey.toLowerCase()), + const agentsToDelete = managedAgents.filter((agent) => + normalizedPubkeys.has(agent.pubkey.toLowerCase()), ); - // Find managed agents that are members of this channel. - const agentsInChannel = managedAgents.filter((agent) => - memberPubkeys.has(agent.pubkey.toLowerCase()), - ); - - // Only delete agents that are NOT members of any other channel. - const agentsToDelete = agentsInChannel.filter((agent) => { - const relayAgent = relayAgents.find( - (ra) => ra.pubkey.toLowerCase() === agent.pubkey.toLowerCase(), - ); - if (!relayAgent) { - // Not found in relay — safe to delete. - return true; - } - // Only delete if this is the agent's only channel. - const otherChannels = relayAgent.channelIds.filter( - (id) => id !== channelId, - ); - return otherChannels.length === 0; - }); - // Delete orphaned agents (best-effort — don't block channel deletion). await Promise.allSettled( - agentsToDelete.map((agent) => deleteManagedAgent(agent.pubkey)), + agentsToDelete + .filter((agent) => { + const relayAgent = relayAgents.find( + (candidate) => + candidate.pubkey.toLowerCase() === agent.pubkey.toLowerCase(), + ); + if (!relayAgent) { + // Not found in relay — safe to delete. + return true; + } + + const activeChannelIds = relayAgent.channelIds.filter( + (channelId) => channelId !== options?.ignoreChannelId, + ); + return activeChannelIds.length === 0; + }) + .map((agent) => deleteManagedAgent(agent.pubkey)), + ); +} + +export async function cleanupManagedAgentIfOrphaned( + pubkey: string, + channelId?: string, +): Promise { + await cleanupManagedAgentsByPubkey([pubkey], { + ignoreChannelId: channelId, + }); +} + +export async function cleanupChannelAgents(channelId: string): Promise { + const members = await getChannelMembers(channelId); + await cleanupManagedAgentsByPubkey( + members.map((member) => member.pubkey), + { + ignoreChannelId: channelId, + }, ); } diff --git a/desktop/src/features/channels/hooks.ts b/desktop/src/features/channels/hooks.ts index 89e4bad3d..12aea3d87 100644 --- a/desktop/src/features/channels/hooks.ts +++ b/desktop/src/features/channels/hooks.ts @@ -1,10 +1,5 @@ import * as React from "react"; -import { - useMutation, - useQuery, - useQueryClient, - type QueryClient, -} from "@tanstack/react-query"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { addChannelMembers, @@ -26,7 +21,10 @@ import { unarchiveChannel, updateChannel, } from "@/shared/api/tauri"; -import { cleanupChannelAgents } from "@/features/channels/cleanupChannelAgents"; +import { + cleanupChannelAgents, + cleanupManagedAgentIfOrphaned, +} from "@/features/channels/cleanupChannelAgents"; import type { AddChannelMembersInput, Channel, @@ -68,68 +66,6 @@ function sortChannels(channels: Channel[]) { }); } -function parseTimestamp(value: string | null | undefined) { - if (!value) { - return null; - } - - const timestamp = Date.parse(value); - return Number.isNaN(timestamp) ? null : timestamp; -} - -function isNewerTimestamp( - candidate: string | null | undefined, - current: string | null | undefined, -) { - const candidateTimestamp = parseTimestamp(candidate); - if (candidateTimestamp === null) { - return false; - } - - const currentTimestamp = parseTimestamp(current); - return currentTimestamp === null || candidateTimestamp > currentTimestamp; -} - -export function updateChannelLastMessageAt( - queryClient: QueryClient, - channelId: string, - lastMessageAt: string | null | undefined, -) { - const lastMessageTimestamp = parseTimestamp(lastMessageAt); - const normalizedLastMessageAt = - lastMessageTimestamp === null - ? null - : new Date(lastMessageTimestamp).toISOString(); - - if (!normalizedLastMessageAt) { - return; - } - - queryClient.setQueryData(channelsQueryKey, (current) => { - if (!current) { - return current; - } - - let didUpdate = false; - const nextChannels = current.map((channel) => { - if ( - channel.id !== channelId || - !isNewerTimestamp(normalizedLastMessageAt, channel.lastMessageAt) - ) { - return channel; - } - - didUpdate = true; - return { - ...channel, - lastMessageAt: normalizedLastMessageAt, - }; - }); - - return didUpdate ? nextChannels : current; - }); -} - async function invalidateChannelState( queryClient: ReturnType, channelId: string | null | undefined, @@ -430,9 +366,19 @@ export function useRemoveChannelMemberMutation(channelId: string | null) { } await removeChannelMember(channelId, pubkey); + + try { + await cleanupManagedAgentIfOrphaned(pubkey, channelId); + } catch (error) { + console.warn("Failed to clean up managed agent:", error); + } }, onSettled: async () => { - await invalidateChannelState(queryClient, channelId); + await Promise.all([ + invalidateChannelState(queryClient, channelId), + queryClient.invalidateQueries({ queryKey: ["managed-agents"] }), + queryClient.invalidateQueries({ queryKey: ["relay-agents"] }), + ]); }, }); } diff --git a/desktop/src/features/channels/lib/channelCache.ts b/desktop/src/features/channels/lib/channelCache.ts new file mode 100644 index 000000000..340854953 --- /dev/null +++ b/desktop/src/features/channels/lib/channelCache.ts @@ -0,0 +1,66 @@ +import type { QueryClient } from "@tanstack/react-query"; + +import type { Channel } from "@/shared/api/types"; +import { channelsQueryKey } from "@/features/channels/hooks"; + +function parseTimestamp(value: string | null | undefined) { + if (!value) { + return null; + } + + const timestamp = Date.parse(value); + return Number.isNaN(timestamp) ? null : timestamp; +} + +function isNewerTimestamp( + candidate: string | null | undefined, + current: string | null | undefined, +) { + const candidateTimestamp = parseTimestamp(candidate); + if (candidateTimestamp === null) { + return false; + } + + const currentTimestamp = parseTimestamp(current); + return currentTimestamp === null || candidateTimestamp > currentTimestamp; +} + +export function updateChannelLastMessageAt( + queryClient: QueryClient, + channelId: string, + lastMessageAt: string | null | undefined, +) { + const lastMessageTimestamp = parseTimestamp(lastMessageAt); + const normalizedLastMessageAt = + lastMessageTimestamp === null + ? null + : new Date(lastMessageTimestamp).toISOString(); + + if (!normalizedLastMessageAt) { + return; + } + + queryClient.setQueryData(channelsQueryKey, (current) => { + if (!current) { + return current; + } + + let didUpdate = false; + const nextChannels = current.map((channel) => { + if ( + channel.id !== channelId || + !isNewerTimestamp(normalizedLastMessageAt, channel.lastMessageAt) + ) { + return channel; + } + + didUpdate = true; + return { + ...channel, + lastMessageAt: normalizedLastMessageAt, + }; + }); + + return didUpdate ? nextChannels : current; + }); +} diff --git a/desktop/src/features/channels/lib/memberUtils.ts b/desktop/src/features/channels/lib/memberUtils.ts new file mode 100644 index 000000000..1834f46d7 --- /dev/null +++ b/desktop/src/features/channels/lib/memberUtils.ts @@ -0,0 +1,42 @@ +import type { ChannelMember } from "@/shared/api/types"; + +export const roleOrder: Record = { + owner: 0, + admin: 1, + member: 2, + guest: 3, + bot: 4, +}; + +export function formatPubkey(pubkey: string) { + return `${pubkey.slice(0, 8)}\u2026${pubkey.slice(-4)}`; +} + +export function formatMemberName( + member: ChannelMember, + currentPubkey?: string, +) { + if (currentPubkey && member.pubkey === currentPubkey) { + return "You"; + } + + return member.displayName ?? formatPubkey(member.pubkey); +} + +export function compareMembersByRole( + left: ChannelMember, + right: ChannelMember, + currentPubkey?: string, +): number { + if (currentPubkey && left.pubkey === currentPubkey) { + return -1; + } + if (currentPubkey && right.pubkey === currentPubkey) { + return 1; + } + const roleDelta = roleOrder[left.role] - roleOrder[right.role]; + if (roleDelta !== 0) { + return roleDelta; + } + return formatMemberName(left).localeCompare(formatMemberName(right)); +} diff --git a/desktop/src/features/channels/lib/useClassifiedMembers.ts b/desktop/src/features/channels/lib/useClassifiedMembers.ts new file mode 100644 index 000000000..00a3af09a --- /dev/null +++ b/desktop/src/features/channels/lib/useClassifiedMembers.ts @@ -0,0 +1,72 @@ +import * as React from "react"; + +import { + useManagedAgentsQuery, + useRelayAgentsQuery, +} from "@/features/agents/hooks"; +import type { ChannelMember } from "@/shared/api/types"; +import { normalizePubkey } from "@/shared/lib/pubkey"; + +import { compareMembersByRole } from "./memberUtils"; + +export function useClassifiedMembers( + members: ChannelMember[], + currentPubkey?: string, +) { + const managedAgentsQuery = useManagedAgentsQuery(); + const relayAgentsQuery = useRelayAgentsQuery(); + + const managedAgents = managedAgentsQuery.data ?? []; + const relayAgents = relayAgentsQuery.data ?? []; + + const managedAgentPubkeys = React.useMemo( + () => new Set(managedAgents.map((agent) => normalizePubkey(agent.pubkey))), + [managedAgents], + ); + const relayAgentPubkeys = React.useMemo( + () => new Set(relayAgents.map((agent) => normalizePubkey(agent.pubkey))), + [relayAgents], + ); + + const isBot = React.useCallback( + (member: ChannelMember) => { + const normalized = normalizePubkey(member.pubkey); + return ( + member.role === "bot" || + managedAgentPubkeys.has(normalized) || + relayAgentPubkeys.has(normalized) + ); + }, + [managedAgentPubkeys, relayAgentPubkeys], + ); + + const { people, bots } = React.useMemo(() => { + const peopleList: ChannelMember[] = []; + const botList: ChannelMember[] = []; + + for (const member of members) { + if (isBot(member)) { + botList.push(member); + } else { + peopleList.push(member); + } + } + + const sort = (list: ChannelMember[]) => + [...list].sort((left, right) => + compareMembersByRole(left, right, currentPubkey), + ); + + return { people: sort(peopleList), bots: sort(botList) }; + }, [currentPubkey, isBot, members]); + + return { + people, + bots, + peopleCount: people.length, + botCount: bots.length, + isBot, + managedAgentsQuery, + relayAgentsQuery, + }; +} diff --git a/desktop/src/features/channels/ui/ChannelManagementSheet.tsx b/desktop/src/features/channels/ui/ChannelManagementSheet.tsx index ac25fea0a..8ec47403f 100644 --- a/desktop/src/features/channels/ui/ChannelManagementSheet.tsx +++ b/desktop/src/features/channels/ui/ChannelManagementSheet.tsx @@ -1,36 +1,30 @@ import { Archive, ArchiveRestore, - Crown, DoorClosed, DoorOpen, FileText, Hash, Lock, MessageSquare, - Shield, - User, Users, } from "lucide-react"; import * as React from "react"; import { - useAddChannelMembersMutation, useArchiveChannelMutation, useChannelDetailsQuery, useChannelMembersQuery, useDeleteChannelMutation, useJoinChannelMutation, useLeaveChannelMutation, - useRemoveChannelMemberMutation, useSetChannelPurposeMutation, useSetChannelTopicMutation, useUnarchiveChannelMutation, useUpdateChannelMutation, } from "@/features/channels/hooks"; -import { usePresenceQuery } from "@/features/presence/hooks"; -import { PresenceBadge } from "@/features/presence/ui/PresenceBadge"; -import type { Channel, ChannelMember } from "@/shared/api/types"; +import { compareMembersByRole } from "@/features/channels/lib/memberUtils"; +import type { Channel } from "@/shared/api/types"; import { AlertDialog, AlertDialogAction, @@ -54,7 +48,6 @@ import { } from "@/shared/ui/sheet"; import { Textarea } from "@/shared/ui/textarea"; import { ChannelCanvas } from "./ChannelCanvas"; -import { ChannelMemberInviteCard } from "./ChannelMemberInviteCard"; type ChannelManagementSheetProps = { channel: Channel | null; @@ -64,26 +57,6 @@ type ChannelManagementSheetProps = { open: boolean; }; -const roleOrder: Record = { - owner: 0, - admin: 1, - member: 2, - guest: 3, - bot: 4, -}; - -function formatPubkey(pubkey: string) { - return `${pubkey.slice(0, 8)}…${pubkey.slice(-4)}`; -} - -function formatMemberName(member: ChannelMember, currentPubkey?: string) { - if (currentPubkey && member.pubkey === currentPubkey) { - return "You"; - } - - return member.displayName ?? formatPubkey(member.pubkey); -} - function Section({ title, description, @@ -120,17 +93,6 @@ function MetadataPill({ ); } -function roleIcon(role: ChannelMember["role"]) { - switch (role) { - case "owner": - return Crown; - case "admin": - return Shield; - default: - return User; - } -} - export function ChannelManagementSheet({ channel, currentPubkey, @@ -147,36 +109,16 @@ export function ChannelManagementSheet({ const archiveChannelMutation = useArchiveChannelMutation(channelId); const unarchiveChannelMutation = useUnarchiveChannelMutation(channelId); const deleteChannelMutation = useDeleteChannelMutation(channelId); - const addMembersMutation = useAddChannelMembersMutation(channelId); - const removeMemberMutation = useRemoveChannelMemberMutation(channelId); const joinChannelMutation = useJoinChannelMutation(channelId); const leaveChannelMutation = useLeaveChannelMutation(channelId); const detail = detailsQuery.data ?? channel; const members = React.useMemo(() => { const currentMembers = membersQuery.data ?? []; - return [...currentMembers].sort((left, right) => { - if (currentPubkey && left.pubkey === currentPubkey) { - return -1; - } - - if (currentPubkey && right.pubkey === currentPubkey) { - return 1; - } - - const roleDelta = roleOrder[left.role] - roleOrder[right.role]; - if (roleDelta !== 0) { - return roleDelta; - } - - return formatMemberName(left).localeCompare(formatMemberName(right)); - }); + return [...currentMembers].sort((left, right) => + compareMembersByRole(left, right, currentPubkey), + ); }, [currentPubkey, membersQuery.data]); - const memberPresenceQuery = usePresenceQuery( - members.map((member) => member.pubkey), - { enabled: open && members.length > 0 }, - ); - const selfMember = members.find((member) => member.pubkey === currentPubkey) ?? null; const hasResolvedMembership = membersQuery.data !== undefined; @@ -197,6 +139,11 @@ export function ChannelManagementSheet({ detail?.channelType !== "dm" && !isArchived && selfMember !== null; + const showAccessSection = + canJoin || + canLeave || + joinChannelMutation.error instanceof Error || + leaveChannelMutation.error instanceof Error; const [nameDraft, setNameDraft] = React.useState(""); const [descriptionDraft, setDescriptionDraft] = React.useState(""); @@ -312,122 +259,75 @@ export function ChannelManagementSheet({

) : null} -
-
- {canJoin ? ( - - ) : null} - - {canLeave ? ( - - ) : null} -
- {joinChannelMutation.error instanceof Error ? ( -

- {joinChannelMutation.error.message} -

- ) : null} - {leaveChannelMutation.error instanceof Error ? ( -

- {leaveChannelMutation.error.message} -

- ) : null} -
- - - -
-
{ - event.preventDefault(); - void updateChannelMutation.mutateAsync({ - description: descriptionDraft.trim() || undefined, - name: nameDraft.trim() || undefined, - }); - }} - > -
- - setNameDraft(event.target.value)} - value={nameDraft} - /> -
-
- -