mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site): archive chats when workspace is already deleted (#23994)
When a user tries to archive-and-delete a chat from /agents but the workspace is already gone, the UI showed a "Failed to look up workspace for deletion" toast and blocked the archive. This change detects the workspace-gone response and archives the chat without attempting deletion. ## Changes The backend returns 410 Gone for soft-deleted workspaces and 404 for workspaces that do not exist or the user cannot access. `isWorkspaceNotFound()` detects both status codes. `resolveArchiveAndDeleteAction()` now returns `"archive"` when the workspace preflight fetch gets a 404 or 410, and the page branches on that action to call the existing archive mutation directly. The `archiveAndDeleteMutation` also tolerates these status codes from `deleteWorkspace()` to handle the race where the workspace disappears between the preflight lookup and the actual delete call. The mutation body was extracted into a testable `archiveAndDeleteWorkspace()` utility so the tolerance logic has direct test coverage. A `navigateAfterArchive()` helper consolidates the post-archive redirect logic that was previously duplicated across the proceed, confirm, and archive paths. ## Pre-existing patterns preserved - The `"proceed"` and `"confirm"` archive-and-delete paths use `onSettled` for navigation, matching the existing behavior before this change. Only the new `"archive"` path uses `onSuccess` since it has no workspace deletion step that should still navigate on partial failure. - `isWorkspaceNotFound()` uses the same `isAxiosError(error) && error.response?.status` pattern already used in several places in `site/src/api/api.ts`. The backend 404 ambiguity (deleted vs unauthorized) is documented in the JSDoc. - The pre-existing double-submit race during the async preflight window is unchanged.
This commit is contained in:
@@ -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();
|
||||
|
||||
@@ -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<TypesGen.Chat>(
|
||||
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<TypesGen.Chat>(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<TypesGen.Chat>(chatKey(activeChatId))
|
||||
?.root_chat_id
|
||||
: undefined,
|
||||
)
|
||||
) {
|
||||
navigate("/agents");
|
||||
}
|
||||
};
|
||||
useEffect(() => {
|
||||
activeChatIDRef.current = agentId;
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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<unknown>,
|
||||
doDelete: (workspaceId: string) => Promise<unknown>,
|
||||
): 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 &&
|
||||
|
||||
Reference in New Issue
Block a user