From 9c1c2cea76beebe6ef9d4805c25c37b98840be27 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sat, 9 May 2026 23:15:27 +0800 Subject: [PATCH] refactor(app): extract workspace archive transaction (#848) --- .../src/components/sidebar-workspace-list.tsx | 104 ++-------- .../src/workspace/workspace-archive.test.ts | 191 ++++++++++++++++++ .../app/src/workspace/workspace-archive.ts | 122 +++++++++++ 3 files changed, 332 insertions(+), 85 deletions(-) create mode 100644 packages/app/src/workspace/workspace-archive.test.ts create mode 100644 packages/app/src/workspace/workspace-archive.ts diff --git a/packages/app/src/components/sidebar-workspace-list.tsx b/packages/app/src/components/sidebar-workspace-list.tsx index 12f588c83..83f82a006 100644 --- a/packages/app/src/components/sidebar-workspace-list.tsx +++ b/packages/app/src/components/sidebar-workspace-list.tsx @@ -101,16 +101,15 @@ import { useActiveWorkspaceSelection } from "@/stores/navigation-active-workspac import { useSessionStore, type WorkspaceDescriptor } from "@/stores/session-store"; import { useWorkspaceFields } from "@/stores/session-store-hooks"; import { redirectIfArchivingActiveWorkspace } from "@/utils/sidebar-workspace-archive-redirect"; -import { - clearWorkspaceArchivePending, - markWorkspaceArchivePending, -} from "@/contexts/session-workspace-upserts"; import { openExternalUrl } from "@/utils/open-external-url"; import { requireWorkspaceExecutionDirectory, - resolveWorkspaceMapKeyByIdentity, resolveWorkspaceExecutionDirectory, } from "@/utils/workspace-execution"; +import { + archiveWorkspaceOptimistically, + archiveWorkspacesOptimistically, +} from "@/workspace/workspace-archive"; import { WorkspaceHoverCard } from "@/components/workspace-hover-card"; import { GitHubIcon } from "@/components/icons/github-icon"; import { isWeb as platformIsWeb, isNative as platformIsNative } from "@/constants/platform"; @@ -126,35 +125,6 @@ const workspaceKeyExtractor = (workspace: SidebarWorkspaceEntry) => workspace.wo const projectKeyExtractor = (project: SidebarProjectEntry) => project.projectKey; -function hideWorkspaceOptimistically(workspace: SidebarWorkspaceEntry): WorkspaceDescriptor | null { - const workspaces = useSessionStore.getState().sessions[workspace.serverId]?.workspaces; - const workspaceKey = resolveWorkspaceMapKeyByIdentity({ - workspaces, - workspaceId: workspace.workspaceId, - }); - const snapshot = workspaceKey ? (workspaces?.get(workspaceKey) ?? null) : null; - markWorkspaceArchivePending({ - serverId: workspace.serverId, - workspaceId: workspace.workspaceId, - workspaceDirectory: workspace.workspaceDirectory, - }); - useSessionStore.getState().removeWorkspace(workspace.serverId, workspace.workspaceId); - return snapshot; -} - -function restoreOptimisticallyHiddenWorkspace(input: { - serverId: string; - workspaceId: string; - snapshot: WorkspaceDescriptor | null; -}): void { - clearWorkspaceArchivePending({ - serverId: input.serverId, - workspaceId: input.workspaceId, - }); - if (input.snapshot) { - useSessionStore.getState().mergeWorkspaces(input.serverId, [input.snapshot]); - } -} const WORKSPACE_STATUS_DOT_WIDTH = 14; const DEFAULT_STATUS_DOT_SIZE = 7; const EMPHASIZED_STATUS_DOT_SIZE = 9; @@ -1574,21 +1544,14 @@ function WorkspaceRowWithMenu({ } setIsArchivingWorkspace(true); - const snapshot = hideWorkspaceOptimistically(workspace); - redirectAfterArchive(); - void (async () => { try { - const payload = await client.archiveWorkspace(workspace.workspaceId); - if (payload.error) { - throw new Error(payload.error); - } - } catch (error) { - restoreOptimisticallyHiddenWorkspace({ - serverId: workspace.serverId, - workspaceId: workspace.workspaceId, - snapshot, + await archiveWorkspaceOptimistically({ + client, + workspace, + afterHide: redirectAfterArchive, }); + } catch (error) { toast.error(error instanceof Error ? error.message : "Failed to hide workspace"); } finally { setIsArchivingWorkspace(false); @@ -1719,21 +1682,14 @@ function NonGitProjectRowWithMenuContent({ } setIsArchivingWorkspace(true); - const snapshot = hideWorkspaceOptimistically(workspace); - redirectAfterArchive(); - void (async () => { try { - const payload = await client.archiveWorkspace(workspace.workspaceId); - if (payload.error) { - throw new Error(payload.error); - } - } catch (error) { - restoreOptimisticallyHiddenWorkspace({ - serverId: workspace.serverId, - workspaceId: workspace.workspaceId, - snapshot, + await archiveWorkspaceOptimistically({ + client, + workspace, + afterHide: redirectAfterArchive, }); + } catch (error) { toast.error(error instanceof Error ? error.message : "Failed to hide workspace"); } finally { setIsArchivingWorkspace(false); @@ -2145,33 +2101,11 @@ function ProjectBlock({ } setIsRemovingProject(true); - const snapshots = new Map( - project.workspaces.map((workspace) => [ - workspace.workspaceId, - hideWorkspaceOptimistically(workspace), - ]), - ); - - const isRejected = (r: PromiseSettledResult) => r.status === "rejected"; - void Promise.allSettled( - project.workspaces.map(async (ws) => { - try { - const payload = await client.archiveWorkspace(ws.workspaceId); - if (payload.error) { - throw new Error(payload.error); - } - } catch (error) { - restoreOptimisticallyHiddenWorkspace({ - serverId, - workspaceId: ws.workspaceId, - snapshot: snapshots.get(ws.workspaceId) ?? null, - }); - throw error; - } - }), - ).then((results) => { - const failed = results.filter(isRejected); - if (failed.length > 0) { + void archiveWorkspacesOptimistically({ + client, + workspaces: project.workspaces, + }).then((failures) => { + if (failures.length > 0) { toast.error("Failed to remove some workspaces"); } setIsRemovingProject(false); diff --git a/packages/app/src/workspace/workspace-archive.test.ts b/packages/app/src/workspace/workspace-archive.test.ts new file mode 100644 index 000000000..91b6ba493 --- /dev/null +++ b/packages/app/src/workspace/workspace-archive.test.ts @@ -0,0 +1,191 @@ +import type { DaemonClient } from "@server/client/daemon-client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + clearWorkspaceArchivePending, + isWorkspaceArchivePending, +} from "@/contexts/session-workspace-upserts"; +import { useSessionStore, type WorkspaceDescriptor } from "@/stores/session-store"; +import { + archiveWorkspaceOptimistically, + archiveWorkspacesOptimistically, + type WorkspaceArchiveTarget, +} from "@/workspace/workspace-archive"; + +const SERVER_ID = "workspace-archive-test"; + +type ArchiveWorkspacePayload = Awaited>; + +function archivePayload(input: { + workspaceId: string; + error?: string | null; +}): ArchiveWorkspacePayload { + return { + requestId: "request", + workspaceId: input.workspaceId, + archivedAt: null, + error: input.error ?? null, + }; +} + +function workspace(input?: Partial): WorkspaceDescriptor { + return { + id: "workspace-1", + projectId: "project-1", + projectDisplayName: "Project", + projectRootPath: "/repo/project", + workspaceDirectory: "/repo/project/workspace-1", + projectKind: "git", + workspaceKind: "worktree", + name: "workspace-1", + status: "done", + archivingAt: null, + diffStat: null, + scripts: [], + ...input, + }; +} + +function target(input?: Partial): WorkspaceArchiveTarget { + const base = workspace(); + return { + serverId: SERVER_ID, + workspaceId: base.id, + workspaceDirectory: base.workspaceDirectory, + ...input, + }; +} + +function createClient( + archiveWorkspace: DaemonClient["archiveWorkspace"], +): Pick { + return { archiveWorkspace }; +} + +function deferred(): { + promise: Promise; + resolve: (value: T) => void; + reject: (error: unknown) => void; +} { + let resolve: (value: T) => void = () => {}; + let reject: (error: unknown) => void = () => {}; + const promise = new Promise((promiseResolve, promiseReject) => { + resolve = promiseResolve; + reject = promiseReject; + }); + return { promise, resolve, reject }; +} + +function storedWorkspace(id: string): WorkspaceDescriptor | undefined { + return useSessionStore.getState().sessions[SERVER_ID]?.workspaces.get(id); +} + +beforeEach(() => { + useSessionStore.getState().initializeSession(SERVER_ID, {} as DaemonClient); +}); + +afterEach(() => { + clearWorkspaceArchivePending({ serverId: SERVER_ID, workspaceId: "workspace-1" }); + clearWorkspaceArchivePending({ serverId: SERVER_ID, workspaceId: "workspace-2" }); + useSessionStore.setState((state) => ({ ...state, sessions: {} })); +}); + +describe("archiveWorkspaceOptimistically", () => { + it("hides the workspace and marks the archive pending while the daemon call runs", async () => { + const archived = workspace(); + useSessionStore.getState().mergeWorkspaces(SERVER_ID, [archived]); + const releaseArchive = deferred(); + const client = createClient(vi.fn(async () => releaseArchive.promise)); + + const archive = archiveWorkspaceOptimistically({ + client, + workspace: target(), + }); + + expect(storedWorkspace(archived.id)).toBeUndefined(); + expect( + isWorkspaceArchivePending({ + serverId: SERVER_ID, + workspaceId: archived.id, + workspaceDirectory: archived.workspaceDirectory, + }), + ).toBe(true); + + releaseArchive.resolve(archivePayload({ workspaceId: archived.id })); + await archive; + + expect(storedWorkspace(archived.id)).toBeUndefined(); + }); + + it("restores the workspace and clears pending state when the daemon rejects the archive", async () => { + const archived = workspace(); + useSessionStore.getState().mergeWorkspaces(SERVER_ID, [archived]); + const client = createClient( + vi.fn(async () => archivePayload({ workspaceId: archived.id, error: "nope" })), + ); + + await expect( + archiveWorkspaceOptimistically({ + client, + workspace: target(), + }), + ).rejects.toThrow("nope"); + + expect(storedWorkspace(archived.id)).toEqual(archived); + expect( + isWorkspaceArchivePending({ + serverId: SERVER_ID, + workspaceId: archived.id, + }), + ).toBe(false); + }); + + it("runs the after-hide hook after local state is hidden", async () => { + const archived = workspace(); + useSessionStore.getState().mergeWorkspaces(SERVER_ID, [archived]); + const client = createClient(vi.fn(async () => archivePayload({ workspaceId: archived.id }))); + const afterHide = vi.fn(() => { + expect(storedWorkspace(archived.id)).toBeUndefined(); + }); + + await archiveWorkspaceOptimistically({ + client, + workspace: target(), + afterHide, + }); + + expect(afterHide).toHaveBeenCalledOnce(); + }); +}); + +describe("archiveWorkspacesOptimistically", () => { + it("returns failures and restores only the workspaces whose archive failed", async () => { + const first = workspace({ id: "workspace-1" }); + const second = workspace({ + id: "workspace-2", + workspaceDirectory: "/repo/project/workspace-2", + name: "workspace-2", + }); + useSessionStore.getState().mergeWorkspaces(SERVER_ID, [first, second]); + const client = createClient( + vi.fn(async (workspaceId) => + archivePayload({ + workspaceId, + error: workspaceId === second.id ? "failed" : null, + }), + ), + ); + + const failures = await archiveWorkspacesOptimistically({ + client, + workspaces: [ + target({ workspaceId: first.id, workspaceDirectory: first.workspaceDirectory }), + target({ workspaceId: second.id, workspaceDirectory: second.workspaceDirectory }), + ], + }); + + expect(failures).toHaveLength(1); + expect(failures[0]?.workspaceId).toBe(second.id); + expect(storedWorkspace(first.id)).toBeUndefined(); + expect(storedWorkspace(second.id)).toEqual(second); + }); +}); diff --git a/packages/app/src/workspace/workspace-archive.ts b/packages/app/src/workspace/workspace-archive.ts new file mode 100644 index 000000000..7d0f68621 --- /dev/null +++ b/packages/app/src/workspace/workspace-archive.ts @@ -0,0 +1,122 @@ +import { + clearWorkspaceArchivePending, + markWorkspaceArchivePending, +} from "@/contexts/session-workspace-upserts"; +import { useSessionStore, type WorkspaceDescriptor } from "@/stores/session-store"; +import { resolveWorkspaceMapKeyByIdentity } from "@/utils/workspace-execution"; + +export interface WorkspaceArchiveTarget { + serverId: string; + workspaceId: string; + workspaceDirectory?: string | null; +} + +interface WorkspaceArchiveClient { + archiveWorkspace: (workspaceId: string) => Promise<{ error: string | null }>; +} + +interface OptimisticWorkspaceArchiveSnapshot { + workspace: WorkspaceDescriptor | null; +} + +export interface WorkspaceArchiveFailure { + workspaceId: string; + error: unknown; +} + +function isWorkspaceArchiveFailure(error: unknown): error is WorkspaceArchiveFailure { + return ( + typeof error === "object" && + error !== null && + "workspaceId" in error && + typeof error.workspaceId === "string" && + "error" in error + ); +} + +function hideWorkspaceOptimistically( + workspace: WorkspaceArchiveTarget, +): OptimisticWorkspaceArchiveSnapshot { + const workspaces = useSessionStore.getState().sessions[workspace.serverId]?.workspaces; + const workspaceKey = resolveWorkspaceMapKeyByIdentity({ + workspaces, + workspaceId: workspace.workspaceId, + }); + const snapshot = workspaceKey ? (workspaces?.get(workspaceKey) ?? null) : null; + markWorkspaceArchivePending({ + serverId: workspace.serverId, + workspaceId: workspace.workspaceId, + workspaceDirectory: workspace.workspaceDirectory, + }); + useSessionStore.getState().removeWorkspace(workspace.serverId, workspace.workspaceId); + return { workspace: snapshot }; +} + +function restoreOptimisticallyHiddenWorkspace(input: { + serverId: string; + workspaceId: string; + snapshot: OptimisticWorkspaceArchiveSnapshot; +}): void { + clearWorkspaceArchivePending({ + serverId: input.serverId, + workspaceId: input.workspaceId, + }); + if (input.snapshot.workspace) { + useSessionStore.getState().mergeWorkspaces(input.serverId, [input.snapshot.workspace]); + } +} + +async function archiveWorkspaceOrThrow(input: { + client: WorkspaceArchiveClient; + workspaceId: string; +}): Promise { + const payload = await input.client.archiveWorkspace(input.workspaceId); + if (payload.error) { + throw new Error(payload.error); + } +} + +export async function archiveWorkspaceOptimistically(input: { + client: WorkspaceArchiveClient; + workspace: WorkspaceArchiveTarget; + afterHide?: () => void; +}): Promise { + const snapshot = hideWorkspaceOptimistically(input.workspace); + input.afterHide?.(); + + try { + await archiveWorkspaceOrThrow({ + client: input.client, + workspaceId: input.workspace.workspaceId, + }); + } catch (error) { + restoreOptimisticallyHiddenWorkspace({ + serverId: input.workspace.serverId, + workspaceId: input.workspace.workspaceId, + snapshot, + }); + throw error; + } +} + +export async function archiveWorkspacesOptimistically(input: { + client: WorkspaceArchiveClient; + workspaces: WorkspaceArchiveTarget[]; +}): Promise { + const results = await Promise.allSettled( + input.workspaces.map(async (workspace) => { + try { + await archiveWorkspaceOptimistically({ + client: input.client, + workspace, + }); + } catch (error) { + throw { workspaceId: workspace.workspaceId, error } satisfies WorkspaceArchiveFailure; + } + }), + ); + + return results.flatMap((result) => + result.status === "rejected" && isWorkspaceArchiveFailure(result.reason) ? [result.reason] : [], + ); +}