diff --git a/site/src/pages/AgentsPage/AgentsPage.tsx b/site/src/pages/AgentsPage/AgentsPage.tsx index 03ec1b8403..65b4182d92 100644 --- a/site/src/pages/AgentsPage/AgentsPage.tsx +++ b/site/src/pages/AgentsPage/AgentsPage.tsx @@ -44,6 +44,7 @@ import { import { invalidateWorkspaceMutationQueries, workspaceById, + workspaceByIdKey, } from "#/api/queries/workspaces"; import type * as TypesGen from "#/api/typesGenerated"; import { ConfirmDialog } from "#/components/Dialogs/ConfirmDialog/ConfirmDialog"; @@ -60,7 +61,10 @@ import { useAgentsPageKeybindings } from "./hooks/useAgentsPageKeybindings"; import { useAgentsPWA } from "./hooks/useAgentsPWA"; import { getAgentSidebarFilters } from "./utils/agentSidebarFilters"; import { + ArchiveAndDeleteError, archiveChatAndDeleteWorkspace, + notifyArchiveAndDeleteFailed, + notifyDeleteQueueState, resolveArchiveAndDeleteAction, shouldNavigateAfterArchive, } from "./utils/agentWorkspaceUtils"; @@ -238,7 +242,7 @@ const AgentsPage: FC = () => { (id) => API.experimental.updateChat(id, { archived: true }), (id) => API.deleteWorkspace(id), ), - onSuccess: ({ chatId }) => { + onSuccess: ({ chatId, workspaceId, deleteBuild }) => { applyChatArchiveStateToCaches(queryClient, chatId, true); clearChatErrorReason(chatId); clearPersistedSidebarTabId(chatId); @@ -255,12 +259,30 @@ const AgentsPage: FC = () => { organizationName, username: user.username, }); - }, - onError: (error) => { - toast.error( - getErrorMessage(error, "Failed to archive and delete workspace."), + notifyDeleteQueueState( + queryClient.getQueryData( + workspaceByIdKey(workspaceId), + ), + deleteBuild, ); }, + onError: (error, { workspaceId }) => { + notifyArchiveAndDeleteFailed( + queryClient.getQueryData( + workspaceByIdKey(workspaceId), + ), + error, + (path) => navigate(path), + ); + // Archive failed after the delete already ran; refresh + // workspace state so consumers see the deletion. + if (error instanceof ArchiveAndDeleteError && error.step === "archive") { + void invalidateWorkspaceMutationQueries(queryClient, { + organizationName, + username: user.username, + }); + } + }, }); const [pendingArchiveChatId, setPendingArchiveChatId] = useState< string | null @@ -418,7 +440,7 @@ const AgentsPage: FC = () => { archiveAndDeleteMutation.mutate( { chatId, workspaceId }, { - onSettled: () => { + onSuccess: () => { navigateAfterArchive(chatId); }, }, @@ -429,11 +451,6 @@ const AgentsPage: FC = () => { // about interrupting a live workspace, which is moot // when the workspace no longer exists. archiveAgentMutation.mutate(chatId, { - // Navigate only on success. The proceed/confirm paths - // use onSettled because their pre-existing behavior - // navigates regardless of delete outcome. This path - // has no delete step, so a failed archive should not - // redirect the user. onSuccess: () => { navigateAfterArchive(chatId); }, @@ -453,6 +470,8 @@ const AgentsPage: FC = () => { archiveAndDeleteMutation.mutate(pendingArchiveAndDelete, { onSettled: () => { setPendingArchiveAndDelete(null); + }, + onSuccess: () => { navigateAfterArchive(archivedChatId); }, }); diff --git a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts index f8820a3294..941a23ee1a 100644 --- a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts +++ b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts @@ -1,9 +1,27 @@ -import { describe, expect, it, vi } from "vitest"; -import { PrebuildsSystemUserID } from "#/api/typesGenerated"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("sonner", () => ({ + toast: { + error: vi.fn(), + info: vi.fn(), + success: vi.fn(), + warning: vi.fn(), + }, +})); + +import { toast } from "sonner"; import { + PrebuildsSystemUserID, + type Workspace, + type WorkspaceBuild, +} from "#/api/typesGenerated"; +import { + ArchiveAndDeleteError, archiveChatAndDeleteWorkspace, isWorkspaceAutoCreated, isWorkspaceNotFound, + notifyArchiveAndDeleteFailed, + notifyDeleteQueueState, resolveArchiveAndDeleteAction, shouldNavigateAfterArchive, workspaceAcquiredAt, @@ -259,13 +277,18 @@ describe("isWorkspaceNotFound", () => { }); describe("archiveChatAndDeleteWorkspace", () => { - it("archives and deletes when both succeed", async () => { + const BUILD_OK = { + job: { queue_position: 0, queue_size: 1 }, + } as unknown as WorkspaceBuild; + + it("archives and deletes when both succeed, deleting first", async () => { const callOrder: string[] = []; const doArchive = vi.fn(async () => { callOrder.push("archive"); }); const doDelete = vi.fn(async () => { callOrder.push("delete"); + return BUILD_OK; }); await expect( @@ -275,17 +298,25 @@ describe("archiveChatAndDeleteWorkspace", () => { doArchive, doDelete, ), - ).resolves.toEqual({ chatId: "chat-1", workspaceId: "workspace-1" }); + ).resolves.toEqual({ + chatId: "chat-1", + workspaceId: "workspace-1", + deleteBuild: BUILD_OK, + }); expect(doArchive).toHaveBeenCalledTimes(1); expect(doArchive).toHaveBeenCalledWith("chat-1"); expect(doDelete).toHaveBeenCalledTimes(1); expect(doDelete).toHaveBeenCalledWith("workspace-1"); - expect(callOrder).toEqual(["archive", "delete"]); + expect(callOrder).toEqual(["delete", "archive"]); }); - it("succeeds when delete returns 404", async () => { - const doArchive = vi.fn(async () => undefined); + it("archives even when delete returns 404, with null deleteBuild", async () => { + const callOrder: string[] = []; + const doArchive = vi.fn(async () => { + callOrder.push("archive"); + }); const doDelete = vi.fn(async () => { + callOrder.push("delete"); throw { isAxiosError: true, response: { @@ -302,12 +333,15 @@ describe("archiveChatAndDeleteWorkspace", () => { doArchive, doDelete, ), - ).resolves.toEqual({ chatId: "chat-1", workspaceId: "workspace-1" }); - expect(doArchive).toHaveBeenCalledTimes(1); - expect(doDelete).toHaveBeenCalledTimes(1); + ).resolves.toEqual({ + chatId: "chat-1", + workspaceId: "workspace-1", + deleteBuild: null, + }); + expect(callOrder).toEqual(["delete", "archive"]); }); - it("succeeds when delete returns 410", async () => { + it("archives even when delete returns 410, with null deleteBuild", async () => { const doArchive = vi.fn(async () => undefined); const doDelete = vi.fn(async () => { throw { @@ -326,14 +360,18 @@ describe("archiveChatAndDeleteWorkspace", () => { doArchive, doDelete, ), - ).resolves.toEqual({ chatId: "chat-1", workspaceId: "workspace-1" }); + ).resolves.toEqual({ + chatId: "chat-1", + workspaceId: "workspace-1", + deleteBuild: null, + }); expect(doArchive).toHaveBeenCalledTimes(1); expect(doDelete).toHaveBeenCalledTimes(1); }); - it("throws when delete returns non-404-or-410 error", async () => { + it("wraps non-404-or-410 delete failures and skips archive", async () => { const doArchive = vi.fn(async () => undefined); - const error = { + const cause = { isAxiosError: true, response: { status: 500, @@ -341,38 +379,83 @@ describe("archiveChatAndDeleteWorkspace", () => { }, }; const doDelete = vi.fn(async () => { - throw error; + throw cause; }); - await expect( - archiveChatAndDeleteWorkspace( - "chat-1", - "workspace-1", - doArchive, - doDelete, - ), - ).rejects.toBe(error); - expect(doArchive).toHaveBeenCalledTimes(1); + const promise = archiveChatAndDeleteWorkspace( + "chat-1", + "workspace-1", + doArchive, + doDelete, + ); + await expect(promise).rejects.toBeInstanceOf(ArchiveAndDeleteError); + const err = await promise.catch((e: unknown) => e); + expect((err as ArchiveAndDeleteError).step).toBe("delete"); + expect((err as ArchiveAndDeleteError).cause).toBe(cause); expect(doDelete).toHaveBeenCalledTimes(1); + expect(doArchive).not.toHaveBeenCalled(); }); - it("throws when archive fails without attempting delete", async () => { - const error = new Error("archive failed"); + it("wraps archive failures that follow a successful delete", async () => { + const cause = new Error("archive failed"); const doArchive = vi.fn(async () => { - throw error; + throw cause; }); - const doDelete = vi.fn(async () => undefined); + const doDelete = vi.fn(async () => BUILD_OK); - await expect( - archiveChatAndDeleteWorkspace( - "chat-1", - "workspace-1", - doArchive, - doDelete, - ), - ).rejects.toBe(error); + const promise = archiveChatAndDeleteWorkspace( + "chat-1", + "workspace-1", + doArchive, + doDelete, + ); + await expect(promise).rejects.toBeInstanceOf(ArchiveAndDeleteError); + const err = await promise.catch((e: unknown) => e); + expect((err as ArchiveAndDeleteError).step).toBe("archive"); + expect((err as ArchiveAndDeleteError).cause).toBe(cause); + expect((err as ArchiveAndDeleteError).deleteEnqueued).toBe(true); + expect(doDelete).toHaveBeenCalledTimes(1); expect(doArchive).toHaveBeenCalledTimes(1); - expect(doDelete).not.toHaveBeenCalled(); + }); + + it("marks archive failures with deleteEnqueued=false when delete was skipped", async () => { + const doArchive = vi.fn(async () => { + throw new Error("archive failed"); + }); + const doDelete = vi.fn(async () => { + throw { + isAxiosError: true, + response: { status: 410, data: { message: "gone" } }, + }; + }); + + const promise = archiveChatAndDeleteWorkspace( + "chat-1", + "workspace-1", + doArchive, + doDelete, + ); + const err = (await promise.catch( + (e: unknown) => e, + )) as ArchiveAndDeleteError; + expect(err.step).toBe("archive"); + expect(err.deleteEnqueued).toBe(false); + }); + + it("returns the delete build payload on success", async () => { + const build = { + job: { queue_position: 4, queue_size: 7 }, + } as unknown as WorkspaceBuild; + const doArchive = vi.fn(async () => undefined); + const doDelete = vi.fn(async () => build); + + const result = await archiveChatAndDeleteWorkspace( + "chat-1", + "workspace-1", + doArchive, + doDelete, + ); + expect(result.deleteBuild).toBe(build); }); }); @@ -633,3 +716,190 @@ describe("shouldNavigateAfterArchive", () => { ).toBe(expected); }); }); + +const makeWorkspace = (overrides: Partial = {}): Workspace => + ({ + id: "ws-1", + name: "my-workspace", + owner_name: "alice", + ...overrides, + }) as Workspace; + +const makeDeleteBuild = (matched?: { + count: number; + available?: number; +}): WorkspaceBuild => + ({ + job: { queue_position: 0, queue_size: 0 }, + matched_provisioners: matched + ? { count: matched.count, available: matched.available ?? matched.count } + : undefined, + }) as unknown as WorkspaceBuild; + +describe("notifyDeleteQueueState", () => { + const toastWarning = toast.warning as unknown as ReturnType; + const toastInfo = toast.info as unknown as ReturnType; + const toastSuccess = toast.success as unknown as ReturnType; + + beforeEach(() => { + toastWarning.mockClear(); + toastInfo.mockClear(); + toastSuccess.mockClear(); + }); + + it("is silent when no build is returned (workspace already gone)", () => { + notifyDeleteQueueState(makeWorkspace(), null); + expect(toastWarning).not.toHaveBeenCalled(); + expect(toastInfo).not.toHaveBeenCalled(); + expect(toastSuccess).not.toHaveBeenCalled(); + }); + + it("is silent when workspace is not in cache", () => { + notifyDeleteQueueState(undefined, makeDeleteBuild({ count: 0 })); + expect(toastWarning).not.toHaveBeenCalled(); + }); + + it("is silent when matched_provisioners is absent (older servers)", () => { + notifyDeleteQueueState(makeWorkspace(), makeDeleteBuild()); + expect(toastWarning).not.toHaveBeenCalled(); + }); + + it("is silent on the happy path (at least one matching provisioner)", () => { + notifyDeleteQueueState(makeWorkspace(), makeDeleteBuild({ count: 2 })); + expect(toastWarning).not.toHaveBeenCalled(); + }); + + it("warns when no matching provisioners exist (count = 0)", () => { + notifyDeleteQueueState( + makeWorkspace({ name: "stuck-ws" }), + makeDeleteBuild({ count: 0 }), + ); + expect(toastWarning).toHaveBeenCalledTimes(1); + const message = toastWarning.mock.calls[0][0] as string; + expect(message).toContain("stuck-ws"); + expect(message).toContain("no matching provisioners"); + }); +}); + +describe("notifyArchiveAndDeleteFailed", () => { + const toastError = toast.error as unknown as ReturnType; + + beforeEach(() => { + toastError.mockClear(); + }); + + it("shows a generic delete-failed toast when workspace is not in cache", () => { + const onOpen = vi.fn(); + notifyArchiveAndDeleteFailed( + undefined, + new ArchiveAndDeleteError("delete", {}), + onOpen, + ); + expect(toastError).toHaveBeenCalledTimes(1); + expect(toastError.mock.calls[0][0]).toContain( + "Failed to delete workspace.", + ); + expect(toastError.mock.calls[0][1]).toBeUndefined(); + }); + + it("includes workspace name and an Open workspace action when delete fails", () => { + const onOpen = vi.fn(); + notifyArchiveAndDeleteFailed( + makeWorkspace({ name: "left-behind", owner_name: "bob" }), + new ArchiveAndDeleteError("delete", {}), + onOpen, + ); + expect(toastError).toHaveBeenCalledTimes(1); + const [message, options] = toastError.mock.calls[0] as [ + string, + { + description: string; + action: { label: string; onClick: () => void }; + }, + ]; + expect(message).toContain("left-behind"); + expect(options.description).toContain("not archived"); + expect(options.action.label).toBe("Open workspace"); + options.action.onClick(); + expect(onOpen).toHaveBeenCalledWith("/@bob/left-behind"); + }); + + it("announces partial success when only the archive step fails after enqueue", () => { + const onOpen = vi.fn(); + notifyArchiveAndDeleteFailed( + makeWorkspace({ name: "deleting-ws" }), + new ArchiveAndDeleteError("archive", new Error("forbidden"), true), + onOpen, + ); + expect(toastError).toHaveBeenCalledTimes(1); + const [message, options] = toastError.mock.calls[0] as [string, undefined]; + expect(message).toContain("deleting-ws"); + expect(message).toContain("Deleting"); + expect(message).toContain("failed to archive"); + expect(options).toBeUndefined(); + expect(onOpen).not.toHaveBeenCalled(); + }); + + it("omits the 'Deleting' claim when the workspace was already gone (delete swallowed)", () => { + const onOpen = vi.fn(); + notifyArchiveAndDeleteFailed( + makeWorkspace({ name: "already-gone" }), + new ArchiveAndDeleteError("archive", new Error("forbidden"), false), + onOpen, + ); + expect(toastError).toHaveBeenCalledTimes(1); + const message = toastError.mock.calls[0][0] as string; + expect(message).toContain("already-gone"); + expect(message).toContain("Failed to archive"); + expect(message).not.toContain("Deleting"); + }); + + it("handles archive-step failure with no workspace in cache", () => { + const onOpen = vi.fn(); + notifyArchiveAndDeleteFailed( + undefined, + new ArchiveAndDeleteError("archive", new Error("forbidden"), true), + onOpen, + ); + expect(toastError).toHaveBeenCalledTimes(1); + const [message, options] = toastError.mock.calls[0] as [string, undefined]; + expect(message).toContain("the workspace"); + expect(message).toContain("failed to archive"); + expect(options).toBeUndefined(); + }); + + it("surfaces the original error's message when present", () => { + const onOpen = vi.fn(); + notifyArchiveAndDeleteFailed( + makeWorkspace(), + new ArchiveAndDeleteError( + "delete", + new Error("template version archived"), + ), + onOpen, + ); + const message = toastError.mock.calls[0][0] as string; + expect(message).toContain("template version archived"); + }); + + it("falls back to the delete branch (with action) for non-tagged errors", () => { + const onOpen = vi.fn(); + notifyArchiveAndDeleteFailed( + makeWorkspace({ name: "raw-ws", owner_name: "carol" }), + new Error("raw"), + onOpen, + ); + expect(toastError).toHaveBeenCalledTimes(1); + const [message, options] = toastError.mock.calls[0] as [ + string, + { + description: string; + action: { label: string; onClick: () => void }; + }, + ]; + expect(message).toContain("raw"); + expect(options.action.label).toBe("Open workspace"); + options.action.onClick(); + expect(onOpen).toHaveBeenCalledWith("/@carol/raw-ws"); + }); +}); diff --git a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts index c2566e31e5..ae349c49ef 100644 --- a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts +++ b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts @@ -1,6 +1,9 @@ import { isAxiosError } from "axios"; +import { toast } from "sonner"; +import { getErrorMessage } from "#/api/errors"; import { PrebuildsSystemUserID, + type Workspace, type WorkspaceBuild, } from "#/api/typesGenerated"; @@ -86,26 +89,50 @@ export function isWorkspaceNotFound(error: unknown): boolean { return status === 404 || status === 410; } -/** - * Archives a chat and then deletes its associated workspace. - * If the workspace is already gone (404 or 410), the delete step is - * treated as a no-op so the archive still succeeds. - */ +export class ArchiveAndDeleteError extends Error { + readonly step: "delete" | "archive"; + readonly deleteEnqueued: boolean; + declare readonly cause: unknown; + + constructor( + step: "delete" | "archive", + cause: unknown, + deleteEnqueued = false, + ) { + super( + step === "delete" ? "workspace delete failed" : "chat archive failed", + { cause }, + ); + this.step = step; + this.deleteEnqueued = deleteEnqueued; + } +} + +// Delete-first, archive-second. 404/410 on delete falls through to archive. export async function archiveChatAndDeleteWorkspace( chatId: string, workspaceId: string, doArchive: (chatId: string) => Promise, - doDelete: (workspaceId: string) => Promise, -): Promise<{ chatId: string; workspaceId: string }> { - await doArchive(chatId); + doDelete: (workspaceId: string) => Promise, +): Promise<{ + chatId: string; + workspaceId: string; + deleteBuild: WorkspaceBuild | null; +}> { + let deleteBuild: WorkspaceBuild | null = null; try { - await doDelete(workspaceId); + deleteBuild = await doDelete(workspaceId); } catch (error) { if (!isWorkspaceNotFound(error)) { - throw error; + throw new ArchiveAndDeleteError("delete", error); } } - return { chatId, workspaceId }; + try { + await doArchive(chatId); + } catch (error) { + throw new ArchiveAndDeleteError("archive", error, deleteBuild !== null); + } + return { chatId, workspaceId, deleteBuild }; } /** @@ -189,3 +216,57 @@ export async function resolveArchiveAndDeleteAction( } return "confirm"; } + +export function notifyDeleteQueueState( + workspace: Workspace | undefined, + deleteBuild: WorkspaceBuild | null, +): void { + if (!deleteBuild || !workspace) { + return; + } + const matched = deleteBuild.matched_provisioners; + if (matched && matched.count === 0) { + toast.warning( + `Delete enqueued for "${workspace.name}", but no matching provisioners are available. The workspace will be deleted once one comes online.`, + ); + } +} + +export function notifyArchiveAndDeleteFailed( + workspace: Workspace | undefined, + error: unknown, + onOpenWorkspace: (path: string) => void, +): void { + const step = error instanceof ArchiveAndDeleteError ? error.step : undefined; + const cause = error instanceof ArchiveAndDeleteError ? error.cause : error; + + if (step === "archive") { + const label = workspace ? `"${workspace.name}"` : "the workspace"; + const deleteEnqueued = + error instanceof ArchiveAndDeleteError && error.deleteEnqueued; + const prefix = deleteEnqueued + ? `Deleting ${label}, but failed to archive the chat.` + : `Failed to archive the chat for ${label}.`; + const detail = getErrorMessage(cause, ""); + toast.error(detail ? `${prefix} ${detail}` : prefix); + return; + } + + if (!workspace) { + toast.error(getErrorMessage(cause, "Failed to delete workspace.")); + return; + } + + const path = `/@${workspace.owner_name}/${workspace.name}`; + toast.error( + getErrorMessage(cause, `Failed to delete workspace "${workspace.name}".`), + { + description: + "The chat was not archived. Open the workspace to delete it manually.", + action: { + label: "Open workspace", + onClick: () => onOpenWorkspace(path), + }, + }, + ); +}