From c3adc51ec95206d7b593adb09f477474154521af Mon Sep 17 00:00:00 2001 From: Wenchao An Date: Mon, 14 Sep 2026 06:46:29 +0800 Subject: [PATCH] fix(frontend): confirm sidebar chat deletion (#5406) * fix(frontend): confirm sidebar chat deletion * fix(frontend): preserve chat deletion retries after partial cleanup * fix(frontend): improve chat deletion failure feedback --- README.md | 2 + frontend/AGENTS.md | 9 + .../components/workspace/recent-chat-list.tsx | 54 +---- .../workspace/thread-delete-dialog.tsx | 173 ++++++++++++++++ .../workspace/workspace-sidebar.tsx | 5 +- frontend/src/core/i18n/locales/en-US.ts | 4 + frontend/src/core/i18n/locales/types.ts | 3 + frontend/src/core/i18n/locales/zh-CN.ts | 4 + frontend/src/core/threads/hooks.ts | 17 +- .../tests/e2e/thread-delete-confirm.spec.ts | 191 ++++++++++++++++++ frontend/tests/e2e/thread-history.spec.ts | 28 ++- .../thread-delete-dialog.dom.test.tsx | 104 ++++++++++ ...ad-sidebar-item-delete-gating.dom.test.tsx | 5 +- .../core/threads/delete-thread.dom.test.tsx | 102 ++++++++++ 14 files changed, 642 insertions(+), 59 deletions(-) create mode 100644 frontend/src/components/workspace/thread-delete-dialog.tsx create mode 100644 frontend/tests/e2e/thread-delete-confirm.spec.ts create mode 100644 frontend/tests/unit/components/workspace/thread-delete-dialog.dom.test.tsx create mode 100644 frontend/tests/unit/core/threads/delete-thread.dom.test.tsx diff --git a/README.md b/README.md index 41ecafd2b..4e313aa2b 100644 --- a/README.md +++ b/README.md @@ -1239,6 +1239,8 @@ See [`skills/public/claude-to-deerflow/SKILL.md`](skills/public/claude-to-deerfl ### Chat Archive +Deleting a chat from the sidebar requires confirmation showing its title. Deletion removes the conversation and its files and cannot be undone. + Use **Archive chat** in a recent chat's sidebar menu to hide completed work while keeping its messages, files, and original link. The success message offers **Undo**. Open **Chats → Archived** to find archived conversations and restore them individually; an open archived conversation also shows a restore button in its header. Search filters the titles of loaded conversations, with **Load more** for older entries. Archive and restore preserve the chat's activity time and pinned state. Archiving does not stop a running task or pause its schedules, and new activity does not automatically restore it. Use the existing Delete action when you intend to remove a conversation and its files. diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index 67b0f07c6..ae5909731 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -172,3 +172,12 @@ Custom skill export is admin-only and disabled in static demos. The lazy or user/skill changes. `core/skills/export.ts` owns the revision-bound Blob download; HTTP 409 requires explicit preview refresh. Keep file lists paginated and diagnostics localized. Browser handoff does not prove the file was saved to disk. + +Sidebar rows request deletion through `ThreadDeleteDialogProvider`, hosted in +`WorkspaceSidebar` outside the virtualized flat/project lists. Keep the selected +thread snapshot and retry UI alive when a partial deletion removes its row. +Focus Cancel on open and after a failed deletion has re-enabled the actions; +block dismissal while deletion is pending. Show the error message when available, +with a localized fallback, and log the rejection for debugging. The shared +delete helper accepts remote 404 (not 403) before retrying local cleanup, and +`onDeleted` runs only after both deletion steps succeed. diff --git a/frontend/src/components/workspace/recent-chat-list.tsx b/frontend/src/components/workspace/recent-chat-list.tsx index 8bec3278b..46132e65c 100644 --- a/frontend/src/components/workspace/recent-chat-list.tsx +++ b/frontend/src/components/workspace/recent-chat-list.tsx @@ -13,7 +13,7 @@ import { Trash2, } from "lucide-react"; import Link from "next/link"; -import { useParams, usePathname, useRouter } from "next/navigation"; +import { useParams, usePathname } from "next/navigation"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { toast } from "sonner"; @@ -45,7 +45,6 @@ import { SidebarMenuButton, SidebarMenuItem, } from "@/components/ui/sidebar"; -import { resetThreadChatAfterDelete } from "@/components/workspace/chats/use-thread-chat"; import { getAPIClient } from "@/core/api"; import { useAuth } from "@/core/auth/AuthProvider"; import { hasPermission, PERMISSIONS } from "@/core/auth/permissions"; @@ -56,7 +55,6 @@ import { useLocalSettings } from "@/core/settings"; import { isStaticWebsiteOnly } from "@/core/static-mode"; import { exportThread, type ThreadExportFormat } from "@/core/threads/export"; import { - useDeleteThread, useInfiniteThreads, useMoveThreadToProject, usePinThread, @@ -80,6 +78,7 @@ import { isIMEComposing } from "@/lib/ime"; import { MoveToProjectMenu, NewProjectDialog } from "./move-to-project-menu"; import { ThreadChannelIcon } from "./thread-channel-source"; +import { useThreadDeleteDialog } from "./thread-delete-dialog"; import { VirtualThreadList } from "./thread-list-virtualizer"; import { useThreadArchiveAction } from "./use-thread-archive-action"; @@ -102,14 +101,7 @@ export function ThreadSidebarItem({ const { t } = useI18n(); const { user } = useAuth(); const canDeleteThreads = hasPermission(user, PERMISSIONS.THREADS_DELETE); - const router = useRouter(); - const pathname = usePathname(); - const { thread_id: threadIdFromPath, agent_name: agentNameFromPath } = - useParams<{ - thread_id: string; - agent_name?: string; - }>(); - const { mutate: deleteThread } = useDeleteThread(); + const requestDelete = useThreadDeleteDialog(); const { mutate: renameThread } = useRenameThread(); const { mutate: updatePinnedThread } = usePinThread(); // The move mutation is owned here (not inside `MoveToProjectMenu`) because @@ -140,42 +132,6 @@ export function ThreadSidebarItem({ const [renameValue, setRenameValue] = useState(""); const [newProjectDialogOpen, setNewProjectDialogOpen] = useState(false); - const handleDelete = useCallback(() => { - const currentPathname = - typeof window === "undefined" ? pathname : window.location.pathname; - const threadPath = pathOfThread(thread); - const nextThreadPath = pathOfThread("new", { - agent_name: agentNameFromPath, - }); - const isNewThreadPath = currentPathname === nextThreadPath; - const isCurrentThread = - thread.thread_id === threadIdFromPath || - threadPath === currentPathname || - (isNewThreadPath && recentThreadId === thread.thread_id); - - deleteThread({ - threadId: thread.thread_id, - onRemoteDeleted: isCurrentThread - ? () => { - resetThreadChatAfterDelete({ - deletedThreadId: thread.thread_id, - nextPath: nextThreadPath, - force: true, - }); - void router.replace(nextThreadPath); - } - : undefined, - }); - }, [ - agentNameFromPath, - deleteThread, - pathname, - recentThreadId, - router, - thread, - threadIdFromPath, - ]); - const handleRenameSubmit = useCallback(() => { if (renameValue.trim()) { renameThread( @@ -379,7 +335,9 @@ export function ThreadSidebarItem({ {canDeleteThreads && ( <> - + requestDelete({ thread, recentThreadId })} + > {t.common.delete} diff --git a/frontend/src/components/workspace/thread-delete-dialog.tsx b/frontend/src/components/workspace/thread-delete-dialog.tsx new file mode 100644 index 000000000..88030ac9a --- /dev/null +++ b/frontend/src/components/workspace/thread-delete-dialog.tsx @@ -0,0 +1,173 @@ +"use client"; + +import { useParams, usePathname, useRouter } from "next/navigation"; +import { + createContext, + useCallback, + useContext, + useEffect, + useRef, + useState, + type ReactNode, +} from "react"; +import { toast } from "sonner"; + +import { Button } from "@/components/ui/button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; +import { resetThreadChatAfterDelete } from "@/components/workspace/chats/use-thread-chat"; +import { useI18n } from "@/core/i18n/hooks"; +import { useDeleteThread } from "@/core/threads/hooks"; +import type { AgentThread } from "@/core/threads/types"; +import { pathOfThread, titleOfThread } from "@/core/threads/utils"; + +type DeleteTarget = { + thread: AgentThread; + recentThreadId?: string | undefined; +}; +const ThreadDeleteContext = createContext< + ((target: DeleteTarget) => void) | null +>(null); + +export function useThreadDeleteDialog() { + const requestDelete = useContext(ThreadDeleteContext); + if (!requestDelete) throw new Error("ThreadDeleteDialogProvider is required"); + return requestDelete; +} + +export function ThreadDeleteDialogProvider({ + children, +}: { + children: ReactNode; +}) { + const { t } = useI18n(); + const router = useRouter(); + const pathname = usePathname(); + const { thread_id: threadIdFromPath, agent_name: agentNameFromPath } = + useParams<{ thread_id: string; agent_name?: string }>(); + const { + mutateAsync: deleteThread, + isPending: isDeleting, + isError: deleteFailed, + } = useDeleteThread(); + // A partial deletion can remove the row on a list refresh. Keep its snapshot + // and the retry dialog in this stable host, outside the virtualized lists. + const [target, setTarget] = useState(null); + const deleteInFlight = useRef(false); + const deleteCancelRef = useRef(null); + + useEffect(() => { + // Wait for React to re-enable the button before restoring focus. + if (target && deleteFailed && !isDeleting) deleteCancelRef.current?.focus(); + }, [target, deleteFailed, isDeleting]); + + const handleDelete = useCallback(async () => { + if (!target || deleteInFlight.current) return; + const { thread, recentThreadId } = target; + deleteInFlight.current = true; + const currentPathname = + typeof window === "undefined" ? pathname : window.location.pathname; + const threadPath = pathOfThread(thread); + const nextThreadPath = pathOfThread("new", { + agent_name: agentNameFromPath, + }); + const isNewThreadPath = currentPathname === nextThreadPath; + const isCurrentThread = + thread.thread_id === threadIdFromPath || + threadPath === currentPathname || + (isNewThreadPath && recentThreadId === thread.thread_id); + + try { + await deleteThread({ + threadId: thread.thread_id, + onDeleted: isCurrentThread + ? () => { + resetThreadChatAfterDelete({ + deletedThreadId: thread.thread_id, + nextPath: nextThreadPath, + force: true, + }); + void router.replace(nextThreadPath); + } + : undefined, + }); + setTarget(null); + } catch (error) { + console.error("Failed to delete chat:", error); + toast.error( + error instanceof Error && error.message + ? error.message + : t.chats.deleteFailed, + ); + } finally { + deleteInFlight.current = false; + } + }, [ + agentNameFromPath, + deleteThread, + pathname, + router, + target, + threadIdFromPath, + t.chats.deleteFailed, + ]); + + const requestDelete = useCallback((next: DeleteTarget) => { + if (!deleteInFlight.current) setTarget(next); + }, []); + return ( + + {children} + { + if (!open && !deleteInFlight.current) setTarget(null); + }} + > + { + event.preventDefault(); + deleteCancelRef.current?.focus(); + }} + onEscapeKeyDown={(event) => { + if (deleteInFlight.current) event.preventDefault(); + }} + onInteractOutside={(event) => { + if (deleteInFlight.current) event.preventDefault(); + }} + > + + {t.chats.deleteChat} + + {target && t.chats.deleteConfirm(titleOfThread(target.thread))} + + + + + + + + + + ); +} diff --git a/frontend/src/components/workspace/workspace-sidebar.tsx b/frontend/src/components/workspace/workspace-sidebar.tsx index 25135d82a..32135120c 100644 --- a/frontend/src/components/workspace/workspace-sidebar.tsx +++ b/frontend/src/components/workspace/workspace-sidebar.tsx @@ -12,6 +12,7 @@ import { import { WorkspaceChannelsList } from "./channels/workspace-channels-list"; import { ProjectsSection } from "./projects-section"; import { RecentChatList } from "./recent-chat-list"; +import { ThreadDeleteDialogProvider } from "./thread-delete-dialog"; import { WorkspaceHeader } from "./workspace-header"; import { WorkspaceNavChatList } from "./workspace-nav-chat-list"; import { WorkspaceNavMenu } from "./workspace-nav-menu"; @@ -21,7 +22,7 @@ export function WorkspaceSidebar({ }: React.ComponentProps) { const { open: isSidebarOpen } = useSidebar(); return ( - <> + @@ -41,6 +42,6 @@ export function WorkspaceSidebar({ - + ); } diff --git a/frontend/src/core/i18n/locales/en-US.ts b/frontend/src/core/i18n/locales/en-US.ts index 07d8edd42..28e25611b 100644 --- a/frontend/src/core/i18n/locales/en-US.ts +++ b/frontend/src/core/i18n/locales/en-US.ts @@ -725,6 +725,10 @@ export const enUS: Translations = { // Chats chats: { + deleteChat: "Delete chat", + deleteConfirm: (title) => + `Delete “${title}”? This will delete the conversation and its files. This action cannot be undone.`, + deleteFailed: "Failed to delete chat. Please try again.", noActiveChats: "No recent chats", activeChats: "Recent chats", archivedChats: "Archived", diff --git a/frontend/src/core/i18n/locales/types.ts b/frontend/src/core/i18n/locales/types.ts index b670f36ac..c4c3b57b7 100644 --- a/frontend/src/core/i18n/locales/types.ts +++ b/frontend/src/core/i18n/locales/types.ts @@ -598,6 +598,9 @@ export interface Translations { // Chats chats: { + deleteChat: string; + deleteConfirm: (title: string) => string; + deleteFailed: string; noActiveChats: string; activeChats: string; archivedChats: string; diff --git a/frontend/src/core/i18n/locales/zh-CN.ts b/frontend/src/core/i18n/locales/zh-CN.ts index 243f420c4..efd891cf8 100644 --- a/frontend/src/core/i18n/locales/zh-CN.ts +++ b/frontend/src/core/i18n/locales/zh-CN.ts @@ -686,6 +686,10 @@ export const zhCN: Translations = { // Chats chats: { + deleteChat: "删除对话", + deleteConfirm: (title) => + `确定删除“${title}”吗？这将删除对话及其文件，此操作不可撤销。`, + deleteFailed: "删除对话失败，请重试。", noActiveChats: "暂无近期会话", activeChats: "近期会话", archivedChats: "已归档", diff --git a/frontend/src/core/threads/hooks.ts b/frontend/src/core/threads/hooks.ts index db2e25f08..f6f425cf9 100644 --- a/frontend/src/core/threads/hooks.ts +++ b/frontend/src/core/threads/hooks.ts @@ -3422,7 +3422,13 @@ async function deleteThreadEverywhere( apiClient: ThreadDeleteClient, threadId: string, ) { - await apiClient.threads.delete(threadId); + try { + await apiClient.threads.delete(threadId); + } catch (error) { + // A previous attempt may have deleted the remote thread before local + // cleanup failed. Only 404 is success here; authorization failures are not. + if (getHttpStatus(error) !== 404) throw error; + } await deleteLocalThreadData(threadId); } @@ -3517,18 +3523,17 @@ export function useDeleteThread() { return useMutation({ mutationFn: async ({ threadId, - onRemoteDeleted, + onDeleted, }: { threadId: string; - onRemoteDeleted?: () => void; + onDeleted?: () => void; }) => { const deletedSidecarThreadIds = await deleteSidecarThreadsForParent( apiClient, threadId, ); - await apiClient.threads.delete(threadId); - onRemoteDeleted?.(); - await deleteLocalThreadData(threadId); + await deleteThreadEverywhere(apiClient, threadId); + onDeleted?.(); return deletedSidecarThreadIds; }, onSuccess(deletedSidecarThreadIds, { threadId }) { diff --git a/frontend/tests/e2e/thread-delete-confirm.spec.ts b/frontend/tests/e2e/thread-delete-confirm.spec.ts new file mode 100644 index 000000000..e40d9f5bd --- /dev/null +++ b/frontend/tests/e2e/thread-delete-confirm.spec.ts @@ -0,0 +1,191 @@ +import { expect, test, type Page } from "@playwright/test"; + +import { mockLangGraphAPI } from "./utils/mock-api"; + +const CHAT = "00000000-0000-0000-0000-000000000901"; +const TITLE = "Report to keep"; + +async function openDeleteDialog(page: Page, captureEntry = false) { + const link = page.locator( + `a[data-sidebar="menu-button"][href="/workspace/chats/${CHAT}"]`, + ); + await link.hover(); + await link.locator("xpath=..").getByRole("button", { name: "More" }).click(); + if (captureEntry) { + await page.screenshot({ + path: test.info().outputPath("delete-entry-point.png"), + animations: "disabled", + }); + } + await page.getByRole("menuitem", { name: "Delete", exact: true }).click(); + return page.getByRole("dialog", { name: "Delete chat", exact: true }); +} + +test("opening and dismissing deletion never sends a delete request", async ({ + page, +}) => { + mockLangGraphAPI(page, { threads: [{ thread_id: CHAT, title: TITLE }] }); + const deletes: string[] = []; + page.on("request", (request) => { + if (request.method() === "DELETE") deletes.push(request.url()); + }); + await page.goto(`/workspace/chats/${CHAT}`); + for (const dismissal of ["Cancel", "Escape", "Close"]) { + const dialog = await openDeleteDialog(page); + await expect(dialog).toContainText(TITLE); + await expect(dialog).toContainText("cannot be undone"); + await expect( + dialog.getByRole("button", { name: "Cancel", exact: true }), + ).toBeFocused(); + expect(deletes).toEqual([]); + if (dismissal === "Escape") await page.keyboard.press("Escape"); + else + await dialog + .getByRole("button", { name: dismissal, exact: true }) + .click(); + await expect(dialog).toBeHidden(); + await expect(page).toHaveURL(new RegExp(CHAT)); + expect(deletes).toEqual([]); + } +}); + +test("confirmation waits for deletion, prevents dismissal, and permits retry after failure", async ({ + page, +}, testInfo) => { + mockLangGraphAPI(page, { threads: [{ thread_id: CHAT, title: TITLE }] }); + let releaseDelete!: () => void; + const gate = new Promise((resolve) => { + releaseDelete = resolve; + }); + let attempts = 0; + await page.route(`**/api/langgraph/threads/${CHAT}`, async (route) => { + if (route.request().method() !== "DELETE") return route.fallback(); + attempts++; + if (attempts > 1) return route.fallback(); + await gate; + await route.fulfill({ + status: 403, + contentType: "application/json", + body: JSON.stringify({ detail: "Permission denied" }), + }); + }); + await page.goto(`/workspace/chats/${CHAT}`); + const dialog = await openDeleteDialog(page, true); + await expect(page.getByRole("menu")).toBeHidden(); + await page.screenshot({ + path: testInfo.outputPath("delete-confirm.png"), + animations: "disabled", + }); + await dialog.getByRole("button", { name: "Delete", exact: true }).click(); + await expect.poll(() => attempts).toBe(1); + await expect( + dialog.getByRole("button", { name: "Cancel", exact: true }), + ).toBeDisabled(); + await expect( + dialog.getByRole("button", { name: "Loading", exact: false }), + ).toBeDisabled(); + await expect( + dialog.getByRole("button", { name: "Close", exact: true }), + ).toHaveCount(0); + await page.keyboard.press("Escape"); + await page.mouse.click(5, 5); + await expect(dialog).toBeVisible(); + expect(attempts).toBe(1); + releaseDelete(); + await expect.soft(page.getByText(/Permission denied/)).toBeVisible(); + await expect(dialog).toBeVisible(); + await expect( + dialog.getByRole("button", { name: "Cancel", exact: true }), + ).toBeFocused(); + await page.screenshot({ + path: testInfo.outputPath("delete-error.png"), + animations: "disabled", + }); + await expect(page).toHaveURL(new RegExp(CHAT)); + await page.keyboard.press("Tab"); + await expect( + dialog.getByRole("button", { name: "Delete", exact: true }), + ).toBeFocused(); + await page.keyboard.press("Enter"); + await expect(dialog).toBeHidden(); + await expect(page).toHaveURL(/\/workspace\/chats\/new$/); + await expect( + page.locator( + `a[data-sidebar="menu-button"][href="/workspace/chats/${CHAT}"]`, + ), + ).toHaveCount(0); + expect(attempts).toBe(2); +}); + +for (const active of [true, false]) { + test(`cleanup failure keeps ${active ? "active" : "inactive"} chat deletion retryable after the row disappears`, async ({ + page, + }) => { + const other = "00000000-0000-0000-0000-000000000902"; + mockLangGraphAPI(page, { + threads: [ + { thread_id: CHAT, title: TITLE }, + { thread_id: other, title: "Keep open" }, + ], + }); + let releaseCleanup!: () => void; + const cleanupGate = new Promise((resolve) => { + releaseCleanup = resolve; + }); + let remoteAttempts = 0; + let cleanupAttempts = 0; + await page.route(`**/api/langgraph/threads/${CHAT}`, (route) => { + if (route.request().method() !== "DELETE") return route.fallback(); + remoteAttempts++; + // The real gateway has require_existing=True: after the first successful + // deletion, a retry must accept 404 and continue with local cleanup. + if (remoteAttempts > 1) + return route.fulfill({ + status: 404, + contentType: "application/json", + body: JSON.stringify({ detail: "Thread not found" }), + }); + return route.fallback(); + }); + await page.route(`**/api/threads/${CHAT}`, async (route) => { + if (route.request().method() !== "DELETE") return route.fallback(); + cleanupAttempts++; + if (cleanupAttempts > 1) return route.fallback(); + await cleanupGate; + return route.fulfill({ + status: 500, + contentType: "application/json", + body: JSON.stringify({ detail: "Cleanup failed" }), + }); + }); + const originalPath = `/workspace/chats/${active ? CHAT : other}`; + await page.goto(originalPath); + const dialog = await openDeleteDialog(page); + await dialog.getByRole("button", { name: "Delete", exact: true }).click(); + await expect.poll(() => cleanupAttempts).toBe(1); + await expect(page).toHaveURL(new RegExp(`${originalPath}$`)); + await expect(dialog).toBeVisible(); + releaseCleanup(); + await expect( + page.getByText("Cleanup failed", { exact: true }), + ).toBeVisible(); + // onSettled refetches the list, which no longer contains this thread. + await expect( + page.locator( + `a[data-sidebar="menu-button"][href="/workspace/chats/${CHAT}"]`, + ), + ).toHaveCount(0); + await expect(dialog).toBeVisible(); + await expect(dialog).toContainText(TITLE); + await expect( + dialog.getByRole("button", { name: "Cancel", exact: true }), + ).toBeFocused(); + await dialog.getByRole("button", { name: "Delete", exact: true }).click(); + await expect(dialog).toBeHidden(); + await expect.poll(() => cleanupAttempts).toBe(2); + expect(remoteAttempts).toBe(2); + await expect(page).toHaveURL( + active ? /\/workspace\/chats\/new$/ : new RegExp(`${originalPath}$`), + ); + }); +} diff --git a/frontend/tests/e2e/thread-history.spec.ts b/frontend/tests/e2e/thread-history.spec.ts index 2c8f02406..155098caa 100644 --- a/frontend/tests/e2e/thread-history.spec.ts +++ b/frontend/tests/e2e/thread-history.spec.ts @@ -533,6 +533,10 @@ test.describe("Thread history", () => { await inactiveThreadItem.hover(); await inactiveThreadItem.getByRole("button", { name: /more/i }).click(); await page.getByRole("menuitem", { name: /delete/i }).click(); + await page + .getByRole("dialog") + .getByRole("button", { name: "Delete", exact: true }) + .click(); await expect(page).toHaveURL(new RegExp(MOCK_THREAD_ID)); await expect( @@ -685,12 +689,13 @@ test.describe("Thread history", () => { await expect(textarea).toBeVisible(); }); - test("deleting the active newly created chat returns to the new chat screen", async ({ + test("retrying deletion of the active newly created chat returns to the new chat screen", async ({ page, }) => { mockLangGraphAPI(page); + let cleanupAttempts = 0; await page.route(/\/api\/threads\/[^/]+$/, (route) => { - if (route.request().method() === "DELETE") { + if (route.request().method() === "DELETE" && ++cleanupAttempts === 1) { return route.fulfill({ status: 500, contentType: "application/json", @@ -722,6 +727,25 @@ test.describe("Thread history", () => { await recentThreadItem.hover(); await recentThreadItem.getByRole("button", { name: /more/i }).click(); await page.getByRole("menuitem", { name: /delete/i }).click(); + await page + .getByRole("dialog") + .getByRole("button", { name: "Delete", exact: true }) + .click(); + + // Remote deletion succeeded, but local cleanup failed. Keep the dialog + // and streamed content until the user retries the remaining cleanup. + await expect( + page.getByText("Local cleanup failed", { + exact: true, + }), + ).toBeVisible(); + await expect(page.getByText("Hello from DeerFlow!")).toBeVisible(); + await page + .getByRole("dialog") + .getByRole("button", { name: "Delete", exact: true }) + .click(); + await expect(page.getByRole("dialog")).toBeHidden(); + expect(cleanupAttempts).toBe(2); await expect(page).toHaveURL(/\/workspace\/chats\/new$/); await expect(page.getByText("Previous question")).toHaveCount(0); diff --git a/frontend/tests/unit/components/workspace/thread-delete-dialog.dom.test.tsx b/frontend/tests/unit/components/workspace/thread-delete-dialog.dom.test.tsx new file mode 100644 index 000000000..61ea819a1 --- /dev/null +++ b/frontend/tests/unit/components/workspace/thread-delete-dialog.dom.test.tsx @@ -0,0 +1,104 @@ +import { afterEach, describe, expect, it, rs } from "@rstest/core"; +import { + QueryClient, + QueryClientProvider, + useMutation, +} from "@tanstack/react-query"; +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; +import { toast } from "sonner"; + +import { + ThreadDeleteDialogProvider, + useThreadDeleteDialog, +} from "@/components/workspace/thread-delete-dialog"; +import { DEFAULT_LOCALE } from "@/core/i18n"; +import { I18nProvider } from "@/core/i18n/context"; + +const { deleteThread } = rs.hoisted(() => ({ deleteThread: rs.fn() })); +rs.mock("next/navigation", () => ({ + useRouter: () => ({ replace: rs.fn() }), + usePathname: () => "/workspace/chats/new", + useParams: () => ({}), +})); +rs.mock("sonner", () => ({ toast: { error: rs.fn() } })); +rs.mock("@/components/workspace/chats/use-thread-chat", () => ({ + resetThreadChatAfterDelete: rs.fn(), +})); +rs.mock("@/core/threads/hooks", () => ({ + useDeleteThread: () => useMutation({ mutationFn: deleteThread }), +})); + +function Trigger() { + const requestDelete = useThreadDeleteDialog(); + return ( + + ); +} + +afterEach(() => { + cleanup(); + rs.restoreAllMocks(); +}); + +describe("thread deletion failure feedback", () => { + for (const [name, error, message] of [ + ["specific error", new Error("Permission denied"), "Permission denied"], + ["empty error", new Error(""), "Failed to delete chat. Please try again."], + ["non-Error rejection", null, "Failed to delete chat. Please try again."], + ] as const) { + it(`reports ${name} and restores Cancel focus`, async () => { + deleteThread.mockRejectedValueOnce(error); + const log = rs + .spyOn(console, "error") + .mockImplementation(() => undefined); + const client = new QueryClient({ + defaultOptions: { mutations: { retry: false } }, + }); + render( + + + + + + + , + ); + fireEvent.click(screen.getByRole("button", { name: "Open deletion" })); + const cancel = await screen.findByRole("button", { + name: "Cancel", + }); + const del = screen.getByRole("button", { name: "Delete" }); + del.focus(); + fireEvent.click(del); + await waitFor(() => expect(toast.error).toHaveBeenCalledWith(message)); + await waitFor(() => expect(document.activeElement).toBe(cancel)); + expect(log).toHaveBeenCalledWith("Failed to delete chat:", error); + expect(screen.getByRole("dialog")).not.toBeNull(); + fireEvent.click(cancel); + await waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); + client.clear(); + }); + } +}); diff --git a/frontend/tests/unit/components/workspace/thread-sidebar-item-delete-gating.dom.test.tsx b/frontend/tests/unit/components/workspace/thread-sidebar-item-delete-gating.dom.test.tsx index 68ed68e2d..668255565 100644 --- a/frontend/tests/unit/components/workspace/thread-sidebar-item-delete-gating.dom.test.tsx +++ b/frontend/tests/unit/components/workspace/thread-sidebar-item-delete-gating.dom.test.tsx @@ -11,6 +11,7 @@ import type { ReactNode } from "react"; import { SidebarProvider } from "@/components/ui/sidebar"; import { ThreadSidebarItem } from "@/components/workspace/recent-chat-list"; +import { ThreadDeleteDialogProvider } from "@/components/workspace/thread-delete-dialog"; import { AuthProvider } from "@/core/auth/AuthProvider"; import type { User } from "@/core/auth/types"; import { DEFAULT_LOCALE } from "@/core/i18n"; @@ -49,7 +50,9 @@ function renderItem(user: User): ReturnType { - + + + diff --git a/frontend/tests/unit/core/threads/delete-thread.dom.test.tsx b/frontend/tests/unit/core/threads/delete-thread.dom.test.tsx new file mode 100644 index 000000000..1960e1dd7 --- /dev/null +++ b/frontend/tests/unit/core/threads/delete-thread.dom.test.tsx @@ -0,0 +1,102 @@ +import { afterEach, beforeEach, expect, rs, test } from "@rstest/core"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act, cleanup, renderHook } from "@testing-library/react"; +import type { PropsWithChildren } from "react"; + +const mocks = rs.hoisted(() => ({ + remove: rs.fn(), + search: rs.fn(), + fetch: rs.fn(), +})); +rs.mock("@/core/api", () => ({ + getAPIClient: () => ({ + threads: { delete: mocks.remove, search: mocks.search }, + }), +})); +rs.mock("@/core/api/fetcher", () => ({ fetch: mocks.fetch })); + +import { useDeleteThread } from "@/core/threads/hooks"; + +beforeEach(() => { + mocks.remove.mockReset().mockResolvedValue(undefined); + mocks.search.mockReset().mockResolvedValue([]); + mocks.fetch + .mockReset() + .mockImplementation(async () => new Response(null, { status: 204 })); +}); +afterEach(cleanup); + +function setup() { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + const wrapper = ({ children }: PropsWithChildren) => ( + {children} + ); + return renderHook(() => useDeleteThread(), { wrapper }); +} + +test("cleanup failure does not invoke completion and a remote 404 retry finishes cleanup", async () => { + const onDeleted = rs.fn(); + const { result } = setup(); + mocks.fetch.mockResolvedValueOnce( + new Response(JSON.stringify({ detail: "Cleanup unavailable" }), { + status: 500, + }), + ); + await act(async () => { + await expect( + result.current.mutateAsync({ threadId: "parent", onDeleted }), + ).rejects.toThrow("Cleanup unavailable"); + }); + expect(onDeleted).not.toHaveBeenCalled(); + mocks.remove.mockRejectedValueOnce( + Object.assign(new Error("Thread not found"), { status: 404 }), + ); + await act(async () => { + await result.current.mutateAsync({ threadId: "parent", onDeleted }); + }); + expect(mocks.fetch).toHaveBeenCalledTimes(2); + expect(onDeleted).toHaveBeenCalledTimes(1); +}); + +for (const status of [403, 500]) { + test(`remote ${status} stays an error and never starts local cleanup`, async () => { + const onDeleted = rs.fn(); + mocks.remove.mockRejectedValueOnce({ status }); + const { result } = setup(); + await act(async () => { + await expect( + result.current.mutateAsync({ threadId: "parent", onDeleted }), + ).rejects.toEqual({ status }); + }); + expect(mocks.fetch).not.toHaveBeenCalled(); + expect(onDeleted).not.toHaveBeenCalled(); + }); +} + +test("already-deleted sidecars still receive local cleanup", async () => { + mocks.search.mockResolvedValueOnce([ + { + thread_id: "sidecar", + metadata: { deerflow_sidecar: true, parent_thread_id: "parent" }, + }, + ]); + mocks.remove.mockRejectedValueOnce( + Object.assign(new Error("Thread not found"), { response: { status: 404 } }), + ); + const { result } = setup(); + await act(async () => { + await expect( + result.current.mutateAsync({ threadId: "parent" }), + ).resolves.toEqual(["sidecar"]); + }); + expect(mocks.fetch).toHaveBeenCalledWith( + expect.stringContaining("/api/threads/sidecar"), + { method: "DELETE" }, + ); + expect(mocks.fetch).toHaveBeenCalledWith( + expect.stringContaining("/api/threads/parent"), + { method: "DELETE" }, + ); +});