From d3e5d6929d5a20442c9899bcf60a0cfc937b66e6 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 6 Apr 2026 08:32:10 +0700 Subject: [PATCH] fix: optimistic archiving/closing and tab UX improvements - Archive agents optimistically on close with rollback on error - Close tabs immediately instead of waiting for RPC response - Kill terminals in background with cache invalidation on failure - Bulk close fires RPC in background, closes all tabs upfront - Move new-tab button out of scroll area, rename to "New agent tab" - Support invalidateQueries option in applyArchivedAgentCloseResults - Fix workspace descriptor id/projectId types (string not number) --- .../app/src/hooks/use-archive-agent.test.ts | 32 +++++ packages/app/src/hooks/use-archive-agent.ts | 125 ++++++++++++++++-- .../src/hooks/use-sidebar-workspaces-list.ts | 4 +- .../workspace/workspace-bulk-close.test.ts | 35 ++--- .../screens/workspace/workspace-bulk-close.ts | 45 ++----- .../workspace/workspace-desktop-tabs-row.tsx | 47 +++---- .../screens/workspace/workspace-screen.tsx | 50 ++----- packages/app/src/stores/session-store.test.ts | 9 +- 8 files changed, 212 insertions(+), 135 deletions(-) diff --git a/packages/app/src/hooks/use-archive-agent.test.ts b/packages/app/src/hooks/use-archive-agent.test.ts index 173aa9ba1..0e3ec4fdc 100644 --- a/packages/app/src/hooks/use-archive-agent.test.ts +++ b/packages/app/src/hooks/use-archive-agent.test.ts @@ -139,4 +139,36 @@ describe("useArchiveAgent", () => { entries: [{ agent: { id: "agent-2" } }], }); }); + + it("can apply archived agent close results without invalidating cached lists", () => { + const queryClient = new QueryClient(); + useSessionStore + .getState() + .initializeSession("server-a", {} as DaemonClient); + useSessionStore.getState().setAgents( + "server-a", + new Map([ + [ + "agent-1", + makeAgent(), + ], + ]), + ); + queryClient.setQueryData(["sidebarAgentsList", "server-a"], { + entries: [{ agent: { id: "agent-1" } }, { agent: { id: "agent-2" } }], + }); + queryClient.setQueryData(["allAgents", "server-a"], { + entries: [{ agent: { id: "agent-1" } }, { agent: { id: "agent-2" } }], + }); + + applyArchivedAgentCloseResults({ + queryClient, + serverId: "server-a", + results: [{ agentId: "agent-1", archivedAt: "2026-04-01T04:00:00.000Z" }], + invalidateQueries: false, + }); + + expect(queryClient.getQueryState(["sidebarAgentsList", "server-a"])?.isInvalidated).toBe(false); + expect(queryClient.getQueryState(["allAgents", "server-a"])?.isInvalidated).toBe(false); + }); }); diff --git a/packages/app/src/hooks/use-archive-agent.ts b/packages/app/src/hooks/use-archive-agent.ts index 278319454..922f2d229 100644 --- a/packages/app/src/hooks/use-archive-agent.ts +++ b/packages/app/src/hooks/use-archive-agent.ts @@ -29,6 +29,16 @@ interface AgentsListQueryData { entries?: Array<{ agent?: { id?: string | null } | null } | null>; } +interface ArchivedAgentListCacheSnapshot { + sidebarAgentsList: AgentsListQueryData | undefined; + allAgents: AgentsListQueryData | undefined; +} + +interface ArchiveAgentMutationContext { + agent: ReturnType; + lists: ArchivedAgentListCacheSnapshot; +} + function toArchiveKey(input: ArchiveAgentInput): string { const serverId = input.serverId.trim(); const agentId = input.agentId.trim(); @@ -111,6 +121,70 @@ function removeAgentFromCachedLists(queryClient: QueryClient, input: ArchiveAgen ); } +function getStoredAgentSnapshot(input: ArchiveAgentInput) { + return useSessionStore.getState().sessions[input.serverId]?.agents.get(input.agentId); +} + +function restoreAgentSnapshot( + input: ArchiveAgentInput & { agent: ReturnType }, +): void { + const setAgents = useSessionStore.getState().setAgents; + setAgents(input.serverId, (prev) => { + const hasAgent = prev.has(input.agentId); + if (!input.agent) { + if (!hasAgent) { + return prev; + } + const next = new Map(prev); + next.delete(input.agentId); + return next; + } + + const current = prev.get(input.agentId); + if (current === input.agent) { + return prev; + } + + const next = new Map(prev); + next.set(input.agentId, input.agent); + return next; + }); +} + +function getArchivedAgentListCacheSnapshot( + queryClient: QueryClient, + serverId: string, +): ArchivedAgentListCacheSnapshot { + return { + sidebarAgentsList: queryClient.getQueryData([ + "sidebarAgentsList", + serverId, + ]), + allAgents: queryClient.getQueryData(["allAgents", serverId]), + }; +} + +function restoreCachedListSnapshot( + queryClient: QueryClient, + queryKey: readonly [string, string], + snapshot: AgentsListQueryData | undefined, +): void { + if (snapshot === undefined) { + queryClient.removeQueries({ queryKey, exact: true }); + return; + } + queryClient.setQueryData(queryKey, snapshot); +} + +function restoreArchivedAgentListCacheSnapshot( + queryClient: QueryClient, + serverId: string, + snapshot: ArchivedAgentListCacheSnapshot, +): void { + restoreCachedListSnapshot(queryClient, ["sidebarAgentsList", serverId], snapshot.sidebarAgentsList); + restoreCachedListSnapshot(queryClient, ["allAgents", serverId], snapshot.allAgents); +} + function markAgentArchivedInStore(input: ArchiveAgentInput & { archivedAt: string }): void { const archivedAt = new Date(input.archivedAt); if (Number.isNaN(archivedAt.getTime())) { @@ -139,6 +213,7 @@ interface ApplyArchivedAgentCloseResultsInput { queryClient: QueryClient; serverId: string; results: ArchivedAgentCloseResult[]; + invalidateQueries?: boolean; } export function applyArchivedAgentCloseResults( @@ -160,12 +235,14 @@ export function applyArchivedAgentCloseResults( }); } - void input.queryClient.invalidateQueries({ - queryKey: ["sidebarAgentsList", input.serverId], - }); - void input.queryClient.invalidateQueries({ - queryKey: ["allAgents", input.serverId], - }); + if (input.invalidateQueries ?? true) { + void input.queryClient.invalidateQueries({ + queryKey: ["sidebarAgentsList", input.serverId], + }); + void input.queryClient.invalidateQueries({ + queryKey: ["allAgents", input.serverId], + }); + } } export function clearArchiveAgentPending(input: IsAgentArchivingInput): void { @@ -195,26 +272,56 @@ export function useArchiveAgent() { return await client.archiveAgent(input.agentId); }, onMutate: (input) => { + const context: ArchiveAgentMutationContext = { + agent: getStoredAgentSnapshot(input), + lists: getArchivedAgentListCacheSnapshot(queryClient, input.serverId), + }; + const archivedAt = new Date().toISOString(); + + applyArchivedAgentCloseResults({ + queryClient, + serverId: input.serverId, + results: [{ agentId: input.agentId, archivedAt }], + invalidateQueries: false, + }); setAgentArchiving({ queryClient, serverId: input.serverId, agentId: input.agentId, isArchiving: true, }); + return context; }, onSuccess: (result, input) => { - applyArchivedAgentCloseResults({ - queryClient, + markAgentArchivedInStore({ serverId: input.serverId, - results: [{ agentId: input.agentId, archivedAt: result.archivedAt }], + agentId: input.agentId, + archivedAt: result.archivedAt, }); }, + onError: (_error, input, context) => { + if (!context) { + return; + } + restoreAgentSnapshot({ + serverId: input.serverId, + agentId: input.agentId, + agent: context.agent, + }); + restoreArchivedAgentListCacheSnapshot(queryClient, input.serverId, context.lists); + }, onSettled: (_result, _error, input) => { clearArchiveAgentPending({ queryClient, serverId: input.serverId, agentId: input.agentId, }); + void queryClient.invalidateQueries({ + queryKey: ["sidebarAgentsList", input.serverId], + }); + void queryClient.invalidateQueries({ + queryKey: ["allAgents", input.serverId], + }); }, }); diff --git a/packages/app/src/hooks/use-sidebar-workspaces-list.ts b/packages/app/src/hooks/use-sidebar-workspaces-list.ts index ee7f162a5..e341d32d6 100644 --- a/packages/app/src/hooks/use-sidebar-workspaces-list.ts +++ b/packages/app/src/hooks/use-sidebar-workspaces-list.ts @@ -259,8 +259,8 @@ function getWorkspaceOrderScopeKey(serverId: string, projectKey: string): string } function toWorkspaceDescriptor(payload: { - id: number; - projectId: number; + id: string; + projectId: string; projectDisplayName: string; projectRootPath: string; workspaceDirectory: string; diff --git a/packages/app/src/screens/workspace/workspace-bulk-close.test.ts b/packages/app/src/screens/workspace/workspace-bulk-close.test.ts index e154824c4..7327b253a 100644 --- a/packages/app/src/screens/workspace/workspace-bulk-close.test.ts +++ b/packages/app/src/screens/workspace/workspace-bulk-close.test.ts @@ -73,7 +73,7 @@ describe("workspace bulk close helpers", () => { ); }); - it("uses one mixed closeItems RPC for agent and terminal tabs, then applies local cleanup", async () => { + it("closes all tabs immediately and fires one mixed closeItems RPC in the background", async () => { const groups = classifyBulkClosableTabs([ makeAgentTab("a1"), makeTerminalTab("t1"), @@ -91,7 +91,7 @@ describe("workspace bulk close helpers", () => { requestId: "req-1", })); - const result = await closeBulkWorkspaceTabs({ + await closeBulkWorkspaceTabs({ groups, client: { closeItems }, closeTab: async (tabId, action) => { @@ -109,23 +109,21 @@ describe("workspace bulk close helpers", () => { agentIds: ["a1"], terminalIds: ["t1", "t2"], }); - expect(result).toEqual({ - agents: [{ agentId: "a1", archivedAt: "2026-04-01T04:00:00.000Z" }], - terminals: [ - { terminalId: "t1", success: true }, - { terminalId: "t2", success: false }, - ], - requestId: "req-1", - }); - expect(closedTabIds).toEqual(["agent_a1", "terminal_t1", "file_/repo/README.md"]); + expect(closedTabIds).toEqual([ + "agent_a1", + "terminal_t1", + "terminal_t2", + "file_/repo/README.md", + ]); expect(cleanupCalls).toEqual([ { tabId: "agent_a1", target: { kind: "agent", agentId: "a1" } }, { tabId: "terminal_t1", target: { kind: "terminal", terminalId: "t1" } }, + { tabId: "terminal_t2", target: { kind: "terminal", terminalId: "t2" } }, { tabId: "file_/repo/README.md" }, ]); }); - it("still closes passive tabs when the mixed closeItems RPC fails", async () => { + it("still closes all tabs when the mixed closeItems RPC fails", async () => { const groups = classifyBulkClosableTabs([ makeAgentTab("a1"), makeTerminalTab("t1"), @@ -135,7 +133,7 @@ describe("workspace bulk close helpers", () => { const cleanupCalls: Array<{ tabId: string; target?: WorkspaceTabDescriptor["target"] }> = []; const warn = vi.fn(); - const result = await closeBulkWorkspaceTabs({ + await closeBulkWorkspaceTabs({ groups, client: { closeItems: async () => { @@ -153,9 +151,14 @@ describe("workspace bulk close helpers", () => { logLabel: "others", }); + await Promise.resolve(); + expect(warn).toHaveBeenCalledTimes(1); - expect(result).toBeNull(); - expect(closedTabIds).toEqual(["file_/repo/README.md"]); - expect(cleanupCalls).toEqual([{ tabId: "file_/repo/README.md" }]); + expect(closedTabIds).toEqual(["agent_a1", "terminal_t1", "file_/repo/README.md"]); + expect(cleanupCalls).toEqual([ + { tabId: "agent_a1", target: { kind: "agent", agentId: "a1" } }, + { tabId: "terminal_t1", target: { kind: "terminal", terminalId: "t1" } }, + { tabId: "file_/repo/README.md" }, + ]); }); }); diff --git a/packages/app/src/screens/workspace/workspace-bulk-close.ts b/packages/app/src/screens/workspace/workspace-bulk-close.ts index 70b662d12..647799b46 100644 --- a/packages/app/src/screens/workspace/workspace-bulk-close.ts +++ b/packages/app/src/screens/workspace/workspace-bulk-close.ts @@ -7,8 +7,6 @@ export type BulkClosableTabGroups = { otherTabs: Array<{ tabId: string }>; }; -type CloseItemsPayload = Awaited>; - interface CloseWorkspaceTabWithCleanupInput { tabId: string; target?: WorkspaceTabDescriptor["target"]; @@ -68,47 +66,27 @@ export function buildBulkCloseConfirmationMessage(input: BulkClosableTabGroups): return `This will archive ${agentTabs.length} agent(s).`; } -function toSuccessfulAgentIds(payload: CloseItemsPayload | null): Set { - return new Set(payload?.agents.map((agent) => agent.agentId) ?? []); -} - -function toSuccessfulTerminalIds(payload: CloseItemsPayload | null): Set { - return new Set( - payload?.terminals.filter((terminal) => terminal.success).map((terminal) => terminal.terminalId) ?? - [], - ); -} - -export async function closeBulkWorkspaceTabs( - input: CloseBulkWorkspaceTabsInput, -): Promise { +export async function closeBulkWorkspaceTabs(input: CloseBulkWorkspaceTabsInput): Promise { const { client, groups, closeTab, closeWorkspaceTabWithCleanup, logLabel, warn } = input; const hasDestructiveTabs = groups.agentTabs.length > 0 || groups.terminalTabs.length > 0; - let payload: CloseItemsPayload | null = null; if (hasDestructiveTabs && client) { - try { - payload = await client.closeItems({ + void client + .closeItems({ agentIds: groups.agentTabs.map((tab) => tab.agentId), terminalIds: groups.terminalTabs.map((tab) => tab.terminalId), + }) + .catch((error) => { + warn?.(`[WorkspaceScreen] Failed to bulk close tabs ${logLabel}`, { error }); }); - } catch (error) { - warn?.(`[WorkspaceScreen] Failed to bulk close tabs ${logLabel}`, { error }); - } } else if (hasDestructiveTabs) { warn?.(`[WorkspaceScreen] Failed to bulk close tabs ${logLabel}`, { error: new Error("Daemon client not available"), }); } - const successfulAgentIds = toSuccessfulAgentIds(payload); - const successfulTerminalIds = toSuccessfulTerminalIds(payload); - for (const { tabId, agentId } of groups.agentTabs) { - if (!successfulAgentIds.has(agentId)) { - continue; - } - await closeTab(tabId, async () => { + void closeTab(tabId, async () => { closeWorkspaceTabWithCleanup({ tabId, target: { kind: "agent", agentId }, @@ -117,10 +95,7 @@ export async function closeBulkWorkspaceTabs( } for (const { tabId, terminalId } of groups.terminalTabs) { - if (!successfulTerminalIds.has(terminalId)) { - continue; - } - await closeTab(tabId, async () => { + void closeTab(tabId, async () => { closeWorkspaceTabWithCleanup({ tabId, target: { kind: "terminal", terminalId }, @@ -129,10 +104,8 @@ export async function closeBulkWorkspaceTabs( } for (const { tabId } of groups.otherTabs) { - await closeTab(tabId, async () => { + void closeTab(tabId, async () => { closeWorkspaceTabWithCleanup({ tabId }); }); } - - return payload; } diff --git a/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx b/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx index 919bc41e6..9be66a853 100644 --- a/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx +++ b/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx @@ -14,8 +14,8 @@ import { ArrowRightToLine, Columns2, Copy, - Plus, Rows2, + SquarePen, SquareTerminal, X, } from "lucide-react-native"; @@ -473,42 +473,36 @@ export function WorkspaceDesktopTabsRow({ ); }} /> + + - - onCreateDraftTab({ paneId })} - accessibilityRole="button" - accessibilityLabel="New tab" - style={styles.inlineNewTabButton} - > - {({ hovered, pressed }) => ( - - )} - + onCreateDraftTab({ paneId })} + accessibilityRole="button" + accessibilityLabel="New agent tab" + style={({ hovered, pressed }) => [ + styles.newTabActionButton, + (hovered || pressed) && styles.newTabActionButtonHovered, + ]} + > + - New tab + New agent tab {newTabKeys ? ( ) : null} - - onCreateTerminalTab({ paneId })} accessibilityRole="button" - accessibilityLabel="New terminal" + accessibilityLabel="New terminal tab" style={({ hovered, pressed }) => [ styles.newTabActionButton, (hovered || pressed) && styles.newTabActionButtonHovered, @@ -518,7 +512,7 @@ export function WorkspaceDesktopTabsRow({ - New terminal + New terminal tab {newTerminalKeys ? ( ) : null} @@ -819,13 +813,6 @@ const styles = StyleSheet.create((theme) => ({ tabCloseButtonActive: { backgroundColor: theme.colors.surface3, }, - inlineNewTabButton: { - paddingHorizontal: theme.spacing[3], - height: "100%", - alignItems: "center", - justifyContent: "center", - borderRadius: 0, - }, newTabActionButton: { width: 22, height: 22, diff --git a/packages/app/src/screens/workspace/workspace-screen.tsx b/packages/app/src/screens/workspace/workspace-screen.tsx index 4342490ad..5bbba76d1 100644 --- a/packages/app/src/screens/workspace/workspace-screen.tsx +++ b/packages/app/src/screens/workspace/workspace-screen.tsx @@ -63,7 +63,6 @@ import type { WorkspaceTab, WorkspaceTabTarget } from "@/stores/workspace-tabs-s import { useKeyboardActionHandler } from "@/hooks/use-keyboard-action-handler"; import type { KeyboardActionDefinition } from "@/keyboard/keyboard-action-dispatcher"; import { useCreateFlowStore } from "@/stores/create-flow-store"; -import { decodeWorkspaceIdFromPathSegment } from "@/utils/host-routes"; import { normalizeWorkspaceTabTarget, workspaceTabTargetsEqual, @@ -79,7 +78,7 @@ import { import type { ListTerminalsResponse } from "@server/shared/messages"; import { upsertTerminalListEntry } from "@/utils/terminal-list"; import { confirmDialog } from "@/utils/confirm-dialog"; -import { applyArchivedAgentCloseResults, useArchiveAgent } from "@/hooks/use-archive-agent"; +import { useArchiveAgent } from "@/hooks/use-archive-agent"; import { useStableEvent } from "@/hooks/use-stable-event"; import { buildProviderCommand } from "@/utils/provider-command-templates"; import { generateDraftId } from "@/stores/draft-keys"; @@ -607,7 +606,7 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) const normalizedWorkspaceId = resolveWorkspaceRouteId({ - routeWorkspaceId: decodeWorkspaceIdFromPathSegment(workspaceId), + routeWorkspaceId: workspaceId, }) ?? ""; const sessionWorkspaces = useSessionStore( (state) => state.sessions[normalizedServerId]?.workspaces, @@ -1198,6 +1197,7 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) if (!workspaceDirectory) { return; } + createTerminalMutation.mutate(input); }, [createTerminalMutation, workspaceDirectory], @@ -1243,10 +1243,6 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) return; } - await killTerminalAsync(terminalId); - setHoveredTabKey((current) => (current === tabId ? null : current)); - setHoveredCloseTabKey((current) => (current === tabId ? null : current)); - queryClient.setQueryData(terminalsQueryKey, (current) => { if (!current) { return current; @@ -1256,13 +1252,18 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) terminals: current.terminals.filter((terminal) => terminal.id !== terminalId), }; }); - + setHoveredTabKey((current) => (current === tabId ? null : current)); + setHoveredCloseTabKey((current) => (current === tabId ? null : current)); if (persistenceKey) { closeWorkspaceTabWithCleanup({ tabId, target: { kind: "terminal", terminalId }, }); } + + void killTerminalAsync(terminalId).catch(() => { + void queryClient.invalidateQueries({ queryKey: terminalsQueryKey }); + }); }); }, [ @@ -1294,7 +1295,6 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) return; } - await archiveAgent({ serverId: normalizedServerId, agentId }); setHoveredTabKey((current) => (current === tabId ? null : current)); setHoveredCloseTabKey((current) => (current === tabId ? null : current)); if (persistenceKey) { @@ -1303,6 +1303,8 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) target: { kind: "agent", agentId }, }); } + + void archiveAgent({ serverId: normalizedServerId, agentId }); }); }, [archiveAgent, closeTab, closeWorkspaceTabWithCleanup, normalizedServerId, persistenceKey], @@ -1464,7 +1466,7 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) return; } - const closeItemsPayload = await closeBulkWorkspaceTabs({ + await closeBulkWorkspaceTabs({ client, groups, closeTab, @@ -1480,31 +1482,6 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) }, }); - if (closeItemsPayload) { - for (const terminal of closeItemsPayload.terminals) { - if (!terminal.success) { - continue; - } - queryClient.setQueryData(terminalsQueryKey, (current) => { - if (!current) { - return current; - } - return { - ...current, - terminals: current.terminals.filter((entry) => entry.id !== terminal.terminalId), - }; - }); - } - - if (normalizedServerId) { - applyArchivedAgentCloseResults({ - queryClient, - serverId: normalizedServerId, - results: closeItemsPayload.agents, - }); - } - } - const closedKeys = new Set(tabsToClose.map((tab) => tab.key)); setHoveredTabKey((current) => (current && closedKeys.has(current) ? null : current)); setHoveredCloseTabKey((current) => (current && closedKeys.has(current) ? null : current)); @@ -1513,10 +1490,7 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) client, closeTab, closeWorkspaceTabWithCleanup, - normalizedServerId, persistenceKey, - queryClient, - terminalsQueryKey, ], ); diff --git a/packages/app/src/stores/session-store.test.ts b/packages/app/src/stores/session-store.test.ts index ce0bf67c5..fb85b468c 100644 --- a/packages/app/src/stores/session-store.test.ts +++ b/packages/app/src/stores/session-store.test.ts @@ -42,8 +42,8 @@ describe("normalizeWorkspaceDescriptor", () => { }, ]; const workspace = normalizeWorkspaceDescriptor({ - id: 1, - projectId: 1, + id: "1", + projectId: "1", projectDisplayName: "Project 1", projectRootPath: "/repo", workspaceDirectory: "/repo", @@ -71,8 +71,8 @@ describe("normalizeWorkspaceDescriptor", () => { it("defaults missing services to an empty array", () => { const payload = { - id: 1, - projectId: 1, + id: "1", + projectId: "1", projectDisplayName: "Project 1", projectRootPath: "/repo", workspaceDirectory: "/repo", @@ -82,6 +82,7 @@ describe("normalizeWorkspaceDescriptor", () => { status: "done", activityAt: null, diffStat: null, + services: [], } as WorkspaceDescriptorPayload; const workspace = normalizeWorkspaceDescriptor(payload);