diff --git a/site/src/api/queries/chats.ts b/site/src/api/queries/chats.ts index 77cc3d2130..0d13f7b19f 100644 --- a/site/src/api/queries/chats.ts +++ b/site/src/api/queries/chats.ts @@ -11,7 +11,7 @@ export const chatKey = (chatId: string) => ["chats", chatId] as const; export const chatMessagesKey = (chatId: string) => ["chats", chatId, "messages"] as const; -const chatsByWorkspaceKeyPrefix = [...chatsKey, "by-workspace"] as const; +export const chatsByWorkspaceKeyPrefix = [...chatsKey, "by-workspace"] as const; export const chatsByWorkspace = (workspaceIds: string[]) => { const sorted = workspaceIds.toSorted(); diff --git a/site/src/pages/AgentsPage/AgentsPage.tsx b/site/src/pages/AgentsPage/AgentsPage.tsx index 8df092d318..07f6cfd62c 100644 --- a/site/src/pages/AgentsPage/AgentsPage.tsx +++ b/site/src/pages/AgentsPage/AgentsPage.tsx @@ -16,6 +16,7 @@ import { chatKey, chatModelConfigs, chatModels, + chatsByWorkspaceKeyPrefix, infiniteChats, invalidateChatListQueries, pinChat, @@ -39,6 +40,7 @@ import { emptyInputStorageKey } from "./components/AgentCreateForm"; import { useAgentsPageKeybindings } from "./hooks/useAgentsPageKeybindings"; import { useAgentsPWA } from "./hooks/useAgentsPWA"; import { + archiveChatAndDeleteWorkspace, resolveArchiveAndDeleteAction, shouldNavigateAfterArchive, } from "./utils/agentWorkspaceUtils"; @@ -165,17 +167,19 @@ const AgentsPage: FC = () => { }, }); const archiveAndDeleteMutation = useMutation({ - mutationFn: async ({ + mutationFn: ({ chatId, workspaceId, }: { chatId: string; workspaceId: string; - }) => { - await API.experimental.updateChat(chatId, { archived: true }); - await API.deleteWorkspace(workspaceId); - return { chatId, workspaceId }; - }, + }) => + archiveChatAndDeleteWorkspace( + chatId, + workspaceId, + (id) => API.experimental.updateChat(id, { archived: true }), + (id) => API.deleteWorkspace(id), + ), onSuccess: async ({ chatId }) => { clearChatErrorReason(chatId); await invalidateChatListQueries(queryClient); @@ -183,9 +187,14 @@ const AgentsPage: FC = () => { queryKey: chatKey(chatId), exact: true, }); + await queryClient.invalidateQueries({ + queryKey: chatsByWorkspaceKeyPrefix, + }); }, onError: (error) => { - toast.error(getErrorMessage(error, "Failed to archive agent.")); + toast.error( + getErrorMessage(error, "Failed to archive and delete workspace."), + ); }, }); const [pendingArchiveChatId, setPendingArchiveChatId] = useState< @@ -330,33 +339,32 @@ const AgentsPage: FC = () => { { chatId, workspaceId }, { onSettled: () => { - const activeChatId = activeChatIDRef.current; - if ( - shouldNavigateAfterArchive( - activeChatId, - chatId, - // Read root_chat_id from the per-chat - // cache, which survives WebSocket eviction - // of sub-agents (only the parent's chatKey - // is removed). Must be read at settle time - // so it reflects the user's current location. - activeChatId - ? queryClient.getQueryData( - chatKey(activeChatId), - )?.root_chat_id - : undefined, - ) - ) { - navigate("/agents"); - } + navigateAfterArchive(chatId); }, }, ); + } else if (action === "archive-only") { + // The workspace is already gone (404), so we skip the + // running-agent confirmation dialog. That dialog warns + // 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); + }, + }); } else { setPendingArchiveAndDelete({ chatId, workspaceId }); } - } catch { - toast.error("Failed to look up workspace for deletion."); + } catch (error) { + toast.error( + getErrorMessage(error, "Failed to look up workspace for deletion."), + ); } }; const handleConfirmArchiveAndDelete = () => { @@ -365,19 +373,7 @@ const AgentsPage: FC = () => { archiveAndDeleteMutation.mutate(pendingArchiveAndDelete, { onSettled: () => { setPendingArchiveAndDelete(null); - const activeChatId = activeChatIDRef.current; - if ( - shouldNavigateAfterArchive( - activeChatId, - archivedChatId, - activeChatId - ? queryClient.getQueryData(chatKey(activeChatId)) - ?.root_chat_id - : undefined, - ) - ) { - navigate("/agents"); - } + navigateAfterArchive(archivedChatId); }, }); } @@ -443,6 +439,26 @@ const AgentsPage: FC = () => { // WebSocket handler can read it without re-subscribing on // every navigation. const activeChatIDRef = useRef(agentId); + const navigateAfterArchive = (archivedChatId: string) => { + const activeChatId = activeChatIDRef.current; + if ( + shouldNavigateAfterArchive( + activeChatId, + archivedChatId, + // Read root_chat_id from the per-chat cache, which + // survives WebSocket eviction of sub-agents (only the + // parent's chatKey is removed). This must be read at + // callback time so it reflects the user's current + // location. + activeChatId + ? queryClient.getQueryData(chatKey(activeChatId)) + ?.root_chat_id + : undefined, + ) + ) { + navigate("/agents"); + } + }; useEffect(() => { activeChatIDRef.current = agentId; }); diff --git a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts index 3ba42b31b8..9e13e64c04 100644 --- a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts +++ b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.test.ts @@ -1,6 +1,8 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { + archiveChatAndDeleteWorkspace, isWorkspaceAutoCreated, + isWorkspaceNotFound, resolveArchiveAndDeleteAction, shouldNavigateAfterArchive, } from "./agentWorkspaceUtils"; @@ -42,6 +44,180 @@ describe("isWorkspaceAutoCreated", () => { }); }); +describe("isWorkspaceNotFound", () => { + it("returns true for Axios-style 404 Not Found errors", () => { + const error = { + isAxiosError: true, + response: { + status: 404, + data: { message: "Workspace not found" }, + }, + }; + + expect(isWorkspaceNotFound(error)).toBe(true); + }); + + it("returns true for Axios-style 410 errors", () => { + const error = { + isAxiosError: true, + response: { + status: 410, + data: { message: "Workspace gone" }, + }, + }; + + expect(isWorkspaceNotFound(error)).toBe(true); + }); + + it("returns false for Axios-style non-404-or-410 errors", () => { + const error = { + isAxiosError: true, + response: { + status: 500, + data: { message: "Internal server error" }, + }, + }; + + expect(isWorkspaceNotFound(error)).toBe(false); + }); + + it("returns false for axios errors without a response (network error)", () => { + const error = { + isAxiosError: true, + response: undefined, + }; + + expect(isWorkspaceNotFound(error)).toBe(false); + }); + + it("returns false for plain Error objects", () => { + expect(isWorkspaceNotFound(new Error("Workspace not found"))).toBe(false); + }); + + it("returns false for non-error values", () => { + expect(isWorkspaceNotFound("nope")).toBe(false); + expect(isWorkspaceNotFound(null)).toBe(false); + }); +}); + +describe("archiveChatAndDeleteWorkspace", () => { + it("archives and deletes when both succeed", async () => { + const callOrder: string[] = []; + const doArchive = vi.fn(async () => { + callOrder.push("archive"); + }); + const doDelete = vi.fn(async () => { + callOrder.push("delete"); + }); + + await expect( + archiveChatAndDeleteWorkspace( + "chat-1", + "workspace-1", + doArchive, + doDelete, + ), + ).resolves.toEqual({ chatId: "chat-1", workspaceId: "workspace-1" }); + expect(doArchive).toHaveBeenCalledTimes(1); + expect(doArchive).toHaveBeenCalledWith("chat-1"); + expect(doDelete).toHaveBeenCalledTimes(1); + expect(doDelete).toHaveBeenCalledWith("workspace-1"); + expect(callOrder).toEqual(["archive", "delete"]); + }); + + it("succeeds when delete returns 404", async () => { + const doArchive = vi.fn(async () => undefined); + const doDelete = vi.fn(async () => { + throw { + isAxiosError: true, + response: { + status: 404, + data: { message: "Workspace not found" }, + }, + }; + }); + + await expect( + archiveChatAndDeleteWorkspace( + "chat-1", + "workspace-1", + doArchive, + doDelete, + ), + ).resolves.toEqual({ chatId: "chat-1", workspaceId: "workspace-1" }); + expect(doArchive).toHaveBeenCalledTimes(1); + expect(doDelete).toHaveBeenCalledTimes(1); + }); + + it("succeeds when delete returns 410", async () => { + const doArchive = vi.fn(async () => undefined); + const doDelete = vi.fn(async () => { + throw { + isAxiosError: true, + response: { + status: 410, + data: { message: "Workspace gone" }, + }, + }; + }); + + await expect( + archiveChatAndDeleteWorkspace( + "chat-1", + "workspace-1", + doArchive, + doDelete, + ), + ).resolves.toEqual({ chatId: "chat-1", workspaceId: "workspace-1" }); + expect(doArchive).toHaveBeenCalledTimes(1); + expect(doDelete).toHaveBeenCalledTimes(1); + }); + + it("throws when delete returns non-404-or-410 error", async () => { + const doArchive = vi.fn(async () => undefined); + const error = { + isAxiosError: true, + response: { + status: 500, + data: { message: "Internal server error" }, + }, + }; + const doDelete = vi.fn(async () => { + throw error; + }); + + await expect( + archiveChatAndDeleteWorkspace( + "chat-1", + "workspace-1", + doArchive, + doDelete, + ), + ).rejects.toBe(error); + expect(doArchive).toHaveBeenCalledTimes(1); + expect(doDelete).toHaveBeenCalledTimes(1); + }); + + it("throws when archive fails without attempting delete", async () => { + const error = new Error("archive failed"); + const doArchive = vi.fn(async () => { + throw error; + }); + const doDelete = vi.fn(async () => undefined); + + await expect( + archiveChatAndDeleteWorkspace( + "chat-1", + "workspace-1", + doArchive, + doDelete, + ), + ).rejects.toBe(error); + expect(doArchive).toHaveBeenCalledTimes(1); + expect(doDelete).not.toHaveBeenCalled(); + }); +}); + describe("resolveArchiveAndDeleteAction", () => { it.each([ { @@ -70,15 +246,61 @@ describe("resolveArchiveAndDeleteAction", () => { expect(result).toBe(expected); }); - it("propagates workspace fetch errors", async () => { + it("propagates non-404-or-410 workspace fetch errors", async () => { + const error = { + isAxiosError: true, + response: { + status: 500, + data: { message: "Internal server error" }, + }, + }; + await expect( resolveArchiveAndDeleteAction( async () => { - throw new Error("not found"); + throw error; }, () => "2026-01-01T00:00:00Z", ), - ).rejects.toThrow("not found"); + ).rejects.toBe(error); + }); + + it("returns archive-only when the workspace fetch returns 404", async () => { + const error = { + isAxiosError: true, + response: { + status: 404, + data: { message: "Workspace not found" }, + }, + }; + + await expect( + resolveArchiveAndDeleteAction( + async () => { + throw error; + }, + () => "2026-01-01T00:00:00Z", + ), + ).resolves.toBe("archive-only"); + }); + + it("returns archive-only when the workspace fetch returns 410", async () => { + const error = { + isAxiosError: true, + response: { + status: 410, + data: { message: "Workspace gone" }, + }, + }; + + await expect( + resolveArchiveAndDeleteAction( + async () => { + throw error; + }, + () => "2026-01-01T00:00:00Z", + ), + ).resolves.toBe("archive-only"); }); }); diff --git a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts index a8377c64f4..c49472885a 100644 --- a/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts +++ b/site/src/pages/AgentsPage/utils/agentWorkspaceUtils.ts @@ -1,3 +1,5 @@ +import { isAxiosError } from "axios"; + /** * Determines whether a workspace was auto-created by a chat. * Workspaces created at or after the chat's creation time are @@ -12,6 +14,47 @@ export function isWorkspaceAutoCreated( return new Date(workspaceCreatedAt) >= new Date(chatCreatedAt); } +/** + * Detects whether an error indicates a missing or deleted workspace. + * + * The Coder backend returns 404 when a workspace does not exist or + * the user lacks access (to avoid leaking resource existence), and + * 410 Gone when a workspace has been soft-deleted. Both cases mean + * the workspace is unavailable for deletion. + * + * In the archive-and-delete flow this is acceptable: the workspace + * ID comes from the chat's own metadata, so if the user can see the + * chat they almost certainly had access to the workspace. Treating + * an auth 404 as "already gone" is a safe degradation because the + * user cannot delete a workspace they lack access to anyway. + */ +export function isWorkspaceNotFound(error: unknown): boolean { + const status = isAxiosError(error) ? error.response?.status : undefined; + 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 async function archiveChatAndDeleteWorkspace( + chatId: string, + workspaceId: string, + doArchive: (chatId: string) => Promise, + doDelete: (workspaceId: string) => Promise, +): Promise<{ chatId: string; workspaceId: string }> { + await doArchive(chatId); + try { + await doDelete(workspaceId); + } catch (error) { + if (!isWorkspaceNotFound(error)) { + throw error; + } + } + return { chatId, workspaceId }; +} + /** * Returns whether the browser should navigate to /agents after an * archive-and-delete mutation settles. Navigation is appropriate @@ -45,13 +88,23 @@ export function shouldNavigateAfterArchive( * `created_at`. * @param getChatCreatedAt - Returns the chat's `created_at` * timestamp, or `undefined` if the chat is not in the cache. - * @returns `"proceed"` to skip the dialog, `"confirm"` to show it. + * @returns `"proceed"` to skip the dialog, `"archive-only"` to archive + * without deleting because the workspace is already gone, or + * `"confirm"` to show the dialog. */ export async function resolveArchiveAndDeleteAction( fetchWorkspace: () => Promise<{ created_at: string }>, getChatCreatedAt: () => string | undefined, -): Promise<"proceed" | "confirm"> { - const workspace = await fetchWorkspace(); +): Promise<"proceed" | "confirm" | "archive-only"> { + let workspace: { created_at: string }; + try { + workspace = await fetchWorkspace(); + } catch (error) { + if (isWorkspaceNotFound(error)) { + return "archive-only"; + } + throw error; + } const chatCreatedAt = getChatCreatedAt(); if ( chatCreatedAt &&