From be9d6d97f1e698c6ef54c09d9f117076c992aff8 Mon Sep 17 00:00:00 2001 From: utpal singh Date: Tue, 8 Sep 2026 15:07:28 +0530 Subject: [PATCH] fix: surface workspace deletion errors --- packages/web/src/api/queries.ts | 11 +- .../src/components/shared/ConfirmDialog.tsx | 7 + .../components/workspaces/WorkspaceDetail.tsx | 27 ++- .../src/test/workspace-delete-error.test.tsx | 204 ++++++++++++++++++ 4 files changed, 245 insertions(+), 4 deletions(-) create mode 100644 packages/web/src/test/workspace-delete-error.test.tsx diff --git a/packages/web/src/api/queries.ts b/packages/web/src/api/queries.ts index b4dd926..2ab35c3 100644 --- a/packages/web/src/api/queries.ts +++ b/packages/web/src/api/queries.ts @@ -4,8 +4,17 @@ import { QK } from "./keys"; // ─── Helpers ────────────────────────────────────────────────────────────────── +function formatApiError(e: unknown): string { + if (typeof e === "object" && e !== null && "detail" in e && typeof e.detail === "string") { + const detail = e.detail.trim(); + if (detail) return detail; + } + if (typeof e === "object") return JSON.stringify(e); + return String(e); +} + function err(e: unknown): never { - throw new Error(typeof e === "object" ? JSON.stringify(e) : String(e)); + throw new Error(formatApiError(e)); } // ─── Workspaces ────────────────────────────────────────────────────────────── diff --git a/packages/web/src/components/shared/ConfirmDialog.tsx b/packages/web/src/components/shared/ConfirmDialog.tsx index 4d993cf..9bb833c 100644 --- a/packages/web/src/components/shared/ConfirmDialog.tsx +++ b/packages/web/src/components/shared/ConfirmDialog.tsx @@ -18,6 +18,7 @@ interface ConfirmDialogProps { onCancel: () => void; danger?: boolean; loading?: boolean; + error?: string; } export function ConfirmDialog({ @@ -29,6 +30,7 @@ export function ConfirmDialog({ onCancel, danger = true, loading = false, + error, }: ConfirmDialogProps) { return ( !o && onCancel()}> @@ -51,6 +53,11 @@ export function ConfirmDialog({ {description} + {error ? ( +

+ {error} +

+ ) : null} - @@ -344,8 +360,13 @@ export function WorkspaceDetail() { description={`This will permanently delete workspace "${mask(workspaceId)}" and all its data. This cannot be undone.`} confirmLabel="Delete workspace" onConfirm={handleDelete} - onCancel={() => setConfirmDelete(false)} + onCancel={cancelDelete} loading={deleteWorkspace.isPending} + error={ + deleteWorkspace.isError + ? deleteWorkspace.error?.message || WORKSPACE_DELETE_FAILED + : undefined + } /> ({ httpFetch: vi.fn() })); +vi.mock("@/lib/http", () => ({ httpFetch })); + +const WORKSPACE_ID = "ws-alpha"; +const CONFLICT_DETAIL = + "Cannot delete workspace 'ws-alpha': active session(s) remain. Delete all sessions first."; +const INSTANCE = { + id: "inst-1", + name: "Local", + baseUrl: "http://localhost:8000", + token: "secret-token", +}; + +function json(body: unknown, status = 200) { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +function requestOf(input: Request | string, init?: RequestInit) { + return typeof input === "string" ? new Request(input, init) : input; +} + +function mockHoncho(options: { onDelete?: (req: Request) => Promise | Response } = {}) { + httpFetch.mockImplementation(async (input: Request | string, init?: RequestInit) => { + const req = requestOf(input, init); + const url = req.url; + if (req.method === "DELETE") { + return options.onDelete ? options.onDelete(req) : json({}, 202); + } + if (url.includes("/queue/status")) { + return json({ + in_progress_work_units: 0, + pending_work_units: 0, + completed_work_units: 0, + total_work_units: 0, + }); + } + if (url.includes("/v3/workspaces") && !url.includes("list")) { + return json({ + id: WORKSPACE_ID, + metadata: {}, + created_at: "2026-01-01T00:00:00Z", + }); + } + return json({ + items: [{ id: WORKSPACE_ID, created_at: "2026-01-01T00:00:00Z" }], + total: 1, + page: 1, + size: 20, + pages: 1, + }); + }); +} + +function renderWorkspace() { + saveStore({ instances: [INSTANCE], activeId: INSTANCE.id }); + const router = createRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: [`/workspaces/${WORKSPACE_ID}`] }), + }); + const qc = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + return { + router, + ...render( + + + + {/* biome-ignore lint/suspicious/noExplicitAny: test router type */} + + + + , + ), + }; +} + +async function confirmDelete() { + const user = userEvent.setup(); + await user.click(await screen.findByRole("button", { name: "Delete" })); + const dialog = await screen.findByRole("dialog"); + await user.click(within(dialog).getByRole("button", { name: "Delete workspace" })); + return { user, dialog }; +} + +describe("workspace delete errors", () => { + afterEach(() => { + httpFetch.mockReset(); + localStorage.clear(); + }); + + it("shows the 409 detail after confirming deletion", async () => { + mockHoncho({ + onDelete: () => json({ detail: CONFLICT_DETAIL }, 409), + }); + renderWorkspace(); + await confirmDelete(); + expect((await screen.findByRole("alert")).textContent).toBe(CONFLICT_DETAIL); + }); + + it("keeps the confirmation dialog open after a 409", async () => { + mockHoncho({ + onDelete: () => json({ detail: CONFLICT_DETAIL }, 409), + }); + renderWorkspace(); + await confirmDelete(); + await screen.findByRole("alert"); + expect(screen.getByRole("dialog")).toHaveAccessibleName("Delete workspace"); + }); + + it("leaves the confirm button usable after a failed deletion", async () => { + mockHoncho({ + onDelete: () => json({ detail: CONFLICT_DETAIL }, 409), + }); + renderWorkspace(); + const { dialog } = await confirmDelete(); + await screen.findByRole("alert"); + expect(within(dialog).getByRole("button", { name: "Delete workspace" })).toBeEnabled(); + }); + + it("retries deletion from the open dialog after a 409", async () => { + let deletes = 0; + mockHoncho({ + onDelete: () => { + deletes += 1; + return json({ detail: CONFLICT_DETAIL }, 409); + }, + }); + renderWorkspace(); + const { user, dialog } = await confirmDelete(); + await screen.findByRole("alert"); + await user.click(within(dialog).getByRole("button", { name: "Delete workspace" })); + await waitFor(() => { + expect(deletes).toBe(2); + }); + }); + + it("does not keep a stale error after cancel and reopen", async () => { + mockHoncho({ + onDelete: () => json({ detail: CONFLICT_DETAIL }, 409), + }); + renderWorkspace(); + const { user, dialog } = await confirmDelete(); + await screen.findByRole("alert"); + await user.click(within(dialog).getByRole("button", { name: "Cancel" })); + await user.click(await screen.findByRole("button", { name: "Delete" })); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + }); + + it("shows a fallback message when deletion fails without an API detail", async () => { + mockHoncho({ + onDelete: () => Promise.reject(new Error("Network request failed")), + }); + renderWorkspace(); + await confirmDelete(); + expect((await screen.findByRole("alert")).textContent).toBe("Network request failed"); + }); + + it("navigates to the workspace list after a successful deletion", async () => { + mockHoncho(); + const { router } = renderWorkspace(); + await confirmDelete(); + await waitFor(() => { + expect(router.state.location.pathname).toBe("/workspaces"); + }); + }); + + it("does not render an alert when ConfirmDialog has no error", () => { + render( + {}} + onCancel={() => {}} + />, + ); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + }); + + it("does not put the instance token in the delete dialog", async () => { + mockHoncho({ + onDelete: () => json({ detail: CONFLICT_DETAIL }, 409), + }); + renderWorkspace(); + await confirmDelete(); + await screen.findByRole("alert"); + expect(screen.getByRole("dialog").textContent).not.toContain(INSTANCE.token); + }); +});