From 161b2c237833c2cb3b36718262fbf395ed761a41 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 7 Apr 2026 14:30:32 +0700 Subject: [PATCH] fix(app): port optimistic workspace tab closing --- .../workspace/workspace-bulk-close.test.ts | 35 +++++----- .../screens/workspace/workspace-bulk-close.ts | 45 +++--------- .../screens/workspace/workspace-screen.tsx | 69 +++++++------------ 3 files changed, 52 insertions(+), 97 deletions(-) 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-screen.tsx b/packages/app/src/screens/workspace/workspace-screen.tsx index 29c6ffb81..186d81283 100644 --- a/packages/app/src/screens/workspace/workspace-screen.tsx +++ b/packages/app/src/screens/workspace/workspace-screen.tsx @@ -79,7 +79,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"; @@ -1214,10 +1214,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; @@ -1227,13 +1223,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 }); + }); }); }, [ @@ -1254,18 +1255,22 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) return; } - const confirmed = await confirmDialog({ - title: "Archive agent?", - message: "This closes the tab and archives the agent.", - confirmLabel: "Archive", - cancelLabel: "Cancel", - destructive: true, - }); - if (!confirmed) { - return; + const agent = + useSessionStore.getState().sessions[normalizedServerId]?.agents?.get(agentId) ?? null; + + if (agent?.status !== "idle") { + const confirmed = await confirmDialog({ + title: "Archive agent?", + message: "This closes the tab and archives the agent.", + confirmLabel: "Archive", + cancelLabel: "Cancel", + destructive: true, + }); + if (!confirmed) { + return; + } } - await archiveAgent({ serverId: normalizedServerId, agentId }); setHoveredTabKey((current) => (current === tabId ? null : current)); setHoveredCloseTabKey((current) => (current === tabId ? null : current)); if (persistenceKey) { @@ -1274,6 +1279,8 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) target: { kind: "agent", agentId }, }); } + + void archiveAgent({ serverId: normalizedServerId, agentId }); }); }, [archiveAgent, closeTab, closeWorkspaceTabWithCleanup, normalizedServerId, persistenceKey], @@ -1418,7 +1425,7 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) return; } - const closeItemsPayload = await closeBulkWorkspaceTabs({ + await closeBulkWorkspaceTabs({ client, groups, closeTab, @@ -1434,31 +1441,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)); @@ -1467,10 +1449,7 @@ function WorkspaceScreenContent({ serverId, workspaceId }: WorkspaceScreenProps) client, closeTab, closeWorkspaceTabWithCleanup, - normalizedServerId, persistenceKey, - queryClient, - terminalsQueryKey, ], );