fix(sidecar): panel button deletes the side chat instead of hiding it (#3961)

* fix(sidecar): make panel button delete the side chat instead of hiding it

The side chat panel's top-right button called `sidecar.close()`, which only
hid the panel — duplicating the header `SidecarTrigger` toggle. Repurpose the
button so the panel owns deletion and the header toggle owns hide/show.

- When a conversation exists, the button shows a trash icon and opens a
  confirmation dialog before deleting via `useDeleteThread` (backend cascade).
- In the draft state (references only, no thread yet) the header trigger is not
  rendered, so the button falls back to a plain close (X) that discards the
  draft without a confirm — there is nothing persisted to delete.

Also fix a latent double-delete bug surfaced by the new dialog:
`useDeleteThread` deletes through the LangGraph route (which drops the
thread_meta row) and then calls `deleteLocalThreadData`, which hits the same
gateway handler whose `require_existing=True` guard now 404s. Treat that 404 as
idempotent success. The recent-chat delete used fire-and-forget `mutate`, so it
swallowed this error; the sidecar's `mutateAsync` surfaced it as a toast.

The e2e mock now mirrors the gateway ownership guard (second DELETE 404s once
the thread is gone) so the regression stays covered. Adds tests for delete,
draft-state close, and header-owned hide/show.

* fix(sidecar): lock delete dialog while deletion is in flight

The delete confirmation dialog disabled Cancel during an in-flight
delete but still allowed dismissal via the overlay, Esc, and the
built-in close (X). Those paths could imply the delete was cancelled
when it was actually still running. Ignore dismissals and drop the
close button while `isDeleting` so only the (disabled) Cancel path
controls the dialog.
This commit is contained in:
Ryker_Feng 2026-07-06 22:14:44 +08:00 committed by GitHub
parent 48e5856c24
commit 34f87f6c92
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 435 additions and 19 deletions

View file

@ -8,6 +8,7 @@ import {
MessageSquareTextIcon,
PaperclipIcon,
RocketIcon,
Trash2Icon,
XIcon,
ZapIcon,
} from "lucide-react";
@ -35,6 +36,14 @@ import {
type PromptInputMessage,
} from "@/components/ai-elements/prompt-input";
import { Button } from "@/components/ui/button";
import {
Dialog,
DialogContent,
DialogDescription,
DialogFooter,
DialogHeader,
DialogTitle,
} from "@/components/ui/dialog";
import {
DropdownMenuGroup,
DropdownMenuLabel,
@ -50,6 +59,7 @@ import {
} from "@/core/sidecar";
import { createSidecarThread } from "@/core/sidecar/api";
import {
useDeleteThread,
useThreadStream,
type ThreadStreamOptions,
} from "@/core/threads/hooks";
@ -137,6 +147,9 @@ export function SidecarPanel({ className }: { className?: string }) {
const { models, tokenUsageEnabled } = useModels();
const [modelDialogOpen, setModelDialogOpen] = useState(false);
const [creatingThread, setCreatingThread] = useState(false);
const [deleteDialogOpen, setDeleteDialogOpen] = useState(false);
const { mutateAsync: deleteThread, isPending: isDeleting } =
useDeleteThread();
const [queuedSubmit, setQueuedSubmit] = useState<{
message: PromptInputMessage;
references: SidecarReference[];
@ -437,6 +450,39 @@ export function SidecarPanel({ className }: { className?: string }) {
],
);
const discardDraftAndClose = useCallback(() => {
sidecar.clearActiveReferences();
sidecar.setSidecarThreadId(null);
sidecar.close();
}, [sidecar]);
const handleDelete = useCallback(async () => {
const threadId = sidecar.sidecarThreadId;
// Guard: the trash button only opens this dialog once a thread exists, so a
// missing id here means the draft was cleared underneath us — just close.
if (!threadId) {
discardDraftAndClose();
setDeleteDialogOpen(false);
return;
}
try {
await deleteThread({ threadId });
discardDraftAndClose();
setDeleteDialogOpen(false);
toast.success(t.sidecar.deleteSuccess);
} catch (error) {
toast.error(
error instanceof Error ? error.message : t.sidecar.deleteFailed,
);
}
}, [
deleteThread,
discardDraftAndClose,
sidecar.sidecarThreadId,
t.sidecar.deleteFailed,
t.sidecar.deleteSuccess,
]);
return (
<div
className={cn("flex size-full min-h-0 flex-col", className)}
@ -454,16 +500,35 @@ export function SidecarPanel({ className }: { className?: string }) {
: t.sidecar.noContext}
</div>
</div>
<Tooltip content={t.common.close}>
<Button
aria-label={t.common.close}
size="icon-sm"
variant="ghost"
onClick={() => sidecar.close()}
>
<XIcon />
</Button>
</Tooltip>
{hasSidecarThread ? (
<Tooltip content={t.sidecar.delete}>
<Button
aria-label={t.sidecar.delete}
className="text-muted-foreground hover:text-destructive"
data-testid="sidecar-delete-button"
size="icon-sm"
variant="ghost"
onClick={() => setDeleteDialogOpen(true)}
>
<Trash2Icon />
</Button>
</Tooltip>
) : (
// No conversation yet — nothing to delete, so this just discards the
// draft and closes the panel. A plain X (no confirm) keeps it light.
<Tooltip content={t.common.close}>
<Button
aria-label={t.common.close}
className="text-muted-foreground hover:text-foreground"
data-testid="sidecar-close-button"
size="icon-sm"
variant="ghost"
onClick={() => discardDraftAndClose()}
>
<XIcon />
</Button>
</Tooltip>
)}
</header>
<div className="min-h-0 flex-1">
@ -558,6 +623,55 @@ export function SidecarPanel({ className }: { className?: string }) {
</PromptInput>
</PromptInputProvider>
</div>
<Dialog
open={deleteDialogOpen}
onOpenChange={(open) => {
// While the delete is in flight the only way out is the (disabled)
// Cancel button, so ignore overlay/Esc/close-button dismissals that
// would otherwise hide the dialog and imply the delete was cancelled.
if (!open && isDeleting) {
return;
}
setDeleteDialogOpen(open);
}}
>
<DialogContent
showCloseButton={!isDeleting}
onEscapeKeyDown={(event) => {
if (isDeleting) {
event.preventDefault();
}
}}
onInteractOutside={(event) => {
if (isDeleting) {
event.preventDefault();
}
}}
>
<DialogHeader>
<DialogTitle>{t.sidecar.delete}</DialogTitle>
<DialogDescription>{t.sidecar.deleteConfirm}</DialogDescription>
</DialogHeader>
<DialogFooter>
<Button
variant="outline"
onClick={() => setDeleteDialogOpen(false)}
disabled={isDeleting}
>
{t.common.cancel}
</Button>
<Button
variant="destructive"
data-testid="sidecar-delete-confirm-button"
onClick={() => void handleDelete()}
disabled={isDeleting}
>
{isDeleting ? t.common.loading : t.common.delete}
</Button>
</DialogFooter>
</DialogContent>
</Dialog>
</div>
);
}

View file

@ -452,6 +452,11 @@ export const enUS: Translations = {
title: "Side chat",
open: "Open side chat",
close: "Close side chat",
delete: "Delete side chat",
deleteConfirm:
"Are you sure you want to delete this side chat? This action cannot be undone. To simply hide it, use the side chat toggle in the header instead.",
deleteSuccess: "Side chat deleted",
deleteFailed: "Failed to delete side chat.",
addToConversation: "Add to conversation",
askInSideChat: "Ask in side chat",
reference: "Reference",

View file

@ -362,6 +362,10 @@ export interface Translations {
title: string;
open: string;
close: string;
delete: string;
deleteConfirm: string;
deleteSuccess: string;
deleteFailed: string;
addToConversation: string;
askInSideChat: string;
reference: string;

View file

@ -436,6 +436,11 @@ export const zhCN: Translations = {
title: "侧边对话",
open: "打开侧边对话",
close: "关闭侧边对话",
delete: "删除侧边对话",
deleteConfirm:
"确定要删除该侧边对话吗?此操作不可撤销。如果只是想隐藏,请使用顶部的侧边对话开关。",
deleteSuccess: "侧边对话已删除",
deleteFailed: "删除侧边对话失败。",
addToConversation: "添加到对话",
askInSideChat: "在侧边聊天中提问",
reference: "引用",

View file

@ -2044,7 +2044,12 @@ async function deleteLocalThreadData(threadId: string) {
},
);
if (!response.ok) {
// A 404 means the thread is already gone — the desired end state. The prior
// `apiClient.threads.delete` call hits the same gateway handler (nginx
// rewrites /api/langgraph/threads/* to /api/threads/*) and removes the
// thread_meta row, so this second delete's ownership guard 404s. Treat it as
// success to keep the delete idempotent.
if (!response.ok && response.status !== 404) {
const error = await response
.json()
.catch(() => ({ detail: "Failed to delete local thread data." }));

View file

@ -781,6 +781,10 @@ test.describe("Side chat", () => {
await expect(
page.getByRole("heading", { name: "Ask a follow-up" }),
).toBeVisible();
// Draft state (no thread created yet): the header shows a plain close (X),
// not the destructive delete — there is nothing persisted to delete.
await expect(page.getByTestId("sidecar-close-button")).toBeVisible();
await expect(page.getByTestId("sidecar-delete-button")).toBeHidden();
const sidecarReference = page.getByTestId("sidecar-reference-attachment");
const sidecarInputForm = page.locator("form").filter({
has: page.getByPlaceholder(/deeper follow-up/i),
@ -943,14 +947,8 @@ test.describe("Side chat", () => {
await expectComposerHeightsEqual(page);
await expect(page.getByTestId("sidecar-header-trigger")).toBeVisible();
await page
.getByTestId("sidecar-panel")
.getByRole("button", { name: /close/i })
.click();
await expect(page.getByTestId("sidecar-panel")).toBeHidden();
await expect(page.getByTestId("sidecar-header-trigger")).toBeVisible();
await page.getByTestId("sidecar-header-trigger").click();
await expect(page.getByTestId("sidecar-panel")).toBeVisible();
// Hiding the side chat is owned by the header trigger; the panel's own
// button deletes the side chat instead of hiding it.
await expect(
page.getByTestId("sidecar-header-trigger"),
).toHaveAccessibleName("Close side chat");
@ -1279,4 +1277,273 @@ test.describe("Side chat", () => {
timeout: 10_000,
});
});
test("deletes the side chat from the panel's own button", async ({
page,
}) => {
mockLangGraphAPI(page, {
threads: [
{
thread_id: MOCK_THREAD_ID,
title: "Main conversation",
messages: [
{
type: "human",
id: "parent-human-1",
content: [{ type: "text", text: "Plan the feature." }],
},
{
type: "ai",
id: "parent-ai-1",
content: "Build it as a side conversation.",
},
],
},
{
thread_id: MOCK_SIDECAR_THREAD_ID,
title: "Restored side chat",
updated_at: "2025-01-01T00:00:01Z",
metadata: {
deerflow_sidecar: true,
parent_thread_id: MOCK_THREAD_ID,
sidecar_context_type: "referenced_message",
sidecar_context_label: "Selected assistant text #2",
sidecar_context_count: 1,
referenced_message_id: "parent-ai-1",
referenced_message_ids: ["parent-ai-1"],
referenced_message_role: "assistant",
referenced_message_roles: ["assistant"],
},
messages: [
{
type: "ai",
id: "side-ai-1",
content: "Restored side answer.",
},
],
},
],
});
await page.goto(`/workspace/chats/${MOCK_THREAD_ID}`);
await expect(
page.getByText("Build it as a side conversation."),
).toBeVisible({ timeout: 15_000 });
await expect(page.getByTestId("sidecar-header-trigger")).toBeVisible({
timeout: 10_000,
});
// Open the side chat panel via the header trigger.
await page.getByTestId("sidecar-header-trigger").click();
await expect(page.getByTestId("sidecar-panel")).toBeVisible();
// The panel's own button deletes the side chat (it does not merely hide it).
await page.getByTestId("sidecar-delete-button").click();
await expect(
page.getByText("This action cannot be undone", { exact: false }),
).toBeVisible();
const deleteRequestPromise = page.waitForRequest(
(request) =>
request.method() === "DELETE" &&
request.url().includes(`/threads/${MOCK_SIDECAR_THREAD_ID}`),
);
await page.getByTestId("sidecar-delete-confirm-button").click();
await deleteRequestPromise;
// The panel closes and, because the sidecar thread is gone, the header
// trigger unmounts too — hiding is owned by the trigger, deleting by this.
await expect(page.getByTestId("sidecar-panel")).toBeHidden();
await expect(page.getByTestId("sidecar-header-trigger")).toBeHidden({
timeout: 10_000,
});
});
test("keeps the delete dialog open while the delete is in flight", async ({
page,
}) => {
mockLangGraphAPI(page, {
threads: [
{
thread_id: MOCK_THREAD_ID,
title: "Main conversation",
messages: [
{
type: "human",
id: "parent-human-1",
content: [{ type: "text", text: "Plan the feature." }],
},
{
type: "ai",
id: "parent-ai-1",
content: "Build it as a side conversation.",
},
],
},
{
thread_id: MOCK_SIDECAR_THREAD_ID,
title: "Restored side chat",
updated_at: "2025-01-01T00:00:01Z",
metadata: {
deerflow_sidecar: true,
parent_thread_id: MOCK_THREAD_ID,
sidecar_context_type: "referenced_message",
sidecar_context_label: "Selected assistant text #2",
sidecar_context_count: 1,
referenced_message_id: "parent-ai-1",
referenced_message_ids: ["parent-ai-1"],
referenced_message_role: "assistant",
referenced_message_roles: ["assistant"],
},
messages: [
{
type: "ai",
id: "side-ai-1",
content: "Restored side answer.",
},
],
},
],
});
// Hold the local-delete step open so the mutation stays pending while we
// probe every dismissal path Radix would otherwise honor.
let releaseDelete: (() => void) | undefined;
const deleteGate = new Promise<void>((resolve) => {
releaseDelete = resolve;
});
await page.route(/\/api\/threads\/[^/]+$/, async (route) => {
if (route.request().method() !== "DELETE") {
return route.fallback();
}
await deleteGate;
return route.fallback();
});
await page.goto(`/workspace/chats/${MOCK_THREAD_ID}`);
await expect(
page.getByText("Build it as a side conversation."),
).toBeVisible({ timeout: 15_000 });
await expect(page.getByTestId("sidecar-header-trigger")).toBeVisible({
timeout: 10_000,
});
await page.getByTestId("sidecar-header-trigger").click();
await expect(page.getByTestId("sidecar-panel")).toBeVisible();
await page.getByTestId("sidecar-delete-button").click();
const dialogTitle = page.getByRole("heading", { name: "Delete side chat" });
await expect(dialogTitle).toBeVisible();
// The built-in Radix close (X) is present before the delete starts.
await expect(
page.locator('[data-slot="dialog-content"] [data-slot="dialog-close"]'),
).toHaveCount(1);
await page.getByTestId("sidecar-delete-confirm-button").click();
// Delete is in flight: confirm shows the loading label and Cancel disables.
await expect(
page.getByTestId("sidecar-delete-confirm-button"),
).toBeDisabled();
await expect(page.getByRole("button", { name: "Cancel" })).toBeDisabled();
// The built-in close (X) is removed so it can't imply a cancel.
await expect(
page.locator('[data-slot="dialog-content"] [data-slot="dialog-close"]'),
).toHaveCount(0);
// Esc and overlay clicks must not dismiss the dialog mid-delete.
await page.keyboard.press("Escape");
await expect(dialogTitle).toBeVisible();
await page
.locator('[data-slot="dialog-overlay"]')
.click({ position: { x: 5, y: 5 } });
await expect(dialogTitle).toBeVisible();
// Once the delete resolves the dialog closes and the panel goes away.
releaseDelete?.();
await expect(dialogTitle).toBeHidden({ timeout: 10_000 });
await expect(page.getByTestId("sidecar-panel")).toBeHidden();
});
test("closes the draft side chat without deleting when no conversation exists", async ({
page,
}) => {
mockLangGraphAPI(page, {
threads: [
{
thread_id: MOCK_THREAD_ID,
title: "Main conversation",
messages: [
{
type: "human",
id: "parent-human-1",
content: [{ type: "text", text: "Plan the feature." }],
},
{
type: "ai",
id: "parent-ai-1",
content: "Build it as a side conversation.",
},
],
},
],
});
await page.route("**/api/models", (route) => {
if (route.request().method() !== "GET") {
return route.fallback();
}
return route.fulfill({
status: 200,
contentType: "application/json",
body: JSON.stringify({
models: [
{
id: "deepseek-v4-pro",
name: "deepseek-v4-pro",
model: "deepseek-v4-pro",
display_name: "DeepSeek V4 Pro",
supports_thinking: true,
supports_reasoning_effort: true,
},
],
token_usage: { enabled: false },
}),
});
});
let deleteRequestFired = false;
page.on("request", (request) => {
if (
request.method() === "DELETE" &&
request.url().includes("/threads/")
) {
deleteRequestFired = true;
}
});
await page.goto(`/workspace/chats/${MOCK_THREAD_ID}`);
await expect(
page.getByText("Build it as a side conversation."),
).toBeVisible({ timeout: 15_000 });
// Open the side chat as a draft (references only, no thread created yet).
await selectTextAndClickToolbarButton(
page,
"Build it as a side conversation.",
"Ask in side chat",
);
await expect(page.getByTestId("sidecar-panel")).toBeVisible();
await expect(
page.getByTestId("sidecar-reference-attachment"),
).toBeVisible();
// The draft has no persisted thread, so the header offers a plain close (X)
// instead of the destructive delete button.
await expect(page.getByTestId("sidecar-delete-button")).toBeHidden();
await page.getByTestId("sidecar-close-button").click();
await expect(page.getByTestId("sidecar-panel")).toBeHidden();
// No thread was ever created, so closing must not issue a DELETE request.
expect(deleteRequestFired).toBe(false);
});
});

View file

@ -691,6 +691,22 @@ export function mockLangGraphAPI(page: Page, options?: MockAPIOptions) {
void page.route(/\/api\/threads\/[^/]+$/, (route) => {
if (route.request().method() === "DELETE") {
const threadId = decodeURIComponent(
new URL(route.request().url()).pathname.split("/").at(-1) ?? "",
);
// Mirror the gateway's `require_existing=True` ownership guard: deleting
// an already-removed thread 404s. `useDeleteThread` first deletes via the
// LangGraph route (which drops the thread_meta row) and then hits this
// route, so this reproduces the real double-delete 404 the frontend must
// treat as idempotent success.
if (!threads.some((thread) => thread.thread_id === threadId)) {
return route.fulfill({
status: 404,
contentType: "application/json",
body: JSON.stringify({ detail: `Thread ${threadId} not found` }),
});
}
threads = threads.filter((thread) => thread.thread_id !== threadId);
return route.fulfill({
status: 204,
});