diff --git a/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx b/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx index 82faefb85b..45ee1221e8 100644 --- a/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.tsx @@ -2,7 +2,7 @@ import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-li import { atom } from 'nanostores' import { afterEach, describe, expect, it, vi } from 'vitest' -import { SessionActionsMenu } from './session-actions-menu' +import { SessionActionsMenu, SessionContextMenu } from './session-actions-menu' afterEach(cleanup) @@ -20,7 +20,16 @@ vi.mock('@/hermes', () => ({ renameSession: vi.fn() })) vi.mock('@/i18n', () => ({ useI18n: () => ({ t: { - common: { cancel: 'Cancel', close: 'Close', delete: 'Delete', save: 'Save' }, + common: { + cancel: 'Cancel', + close: 'Close', + confirm: 'Confirm', + delete: 'Delete', + done: 'Done', + loading: 'Loading…', + save: 'Save' + }, + errors: { genericFailure: 'Something went wrong' }, sidebar: { projects: { menuAppearance: 'Appearance', @@ -35,6 +44,10 @@ vi.mock('@/i18n', () => ({ branchFrom: 'Branch from here', copyId: 'Copy ID', copyIdFailed: 'Failed to copy ID', + deleteDesc: (title: string) => `Delete ${title}?`, + deleteTitle: 'Delete session?', + deleting: 'Deleting…', + deleted: 'Session deleted', export: 'Export', hideTabBar: 'Hide tab bar', pin: 'Pin', @@ -140,4 +153,119 @@ describe('SessionActionsMenu', () => { // eslint-disable-next-line no-restricted-globals -- asserting real focus requires the live document expect(document.activeElement).not.toBe(trigger) }) + + it('confirms before deleting — cancel keeps the session, confirm deletes it', async () => { + const onDelete = vi.fn() + render( + + + + ) + + const trigger = screen.getByRole('button', { name: 'Session actions' }) + fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.pointerUp(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.click(trigger) + + const deleteItem = await screen.findByRole('menuitem', { name: /delete/i }) + fireEvent.click(deleteItem) + + // The confirm dialog is up and names the session being deleted. + expect(await screen.findByRole('dialog')).toBeTruthy() + expect(screen.getByText(/My session/)).toBeTruthy() + + // Cancel: nothing is deleted. + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })) + expect(onDelete).not.toHaveBeenCalled() + + // Re-open the menu and confirm: only now does the delete call fire. + fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.pointerUp(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.click(trigger) + const deleteItemAgain = await screen.findByRole('menuitem', { name: /delete/i }) + fireEvent.click(deleteItemAgain) + + expect(await screen.findByRole('dialog')).toBeTruthy() + fireEvent.click(screen.getByRole('button', { name: 'Delete' })) + // ConfirmDialog shows a done beat before auto-closing (600ms); awaiting it + // also drains the async run() update inside act(). + expect(await screen.findByText('Session deleted')).toBeTruthy() + expect(onDelete).toHaveBeenCalledTimes(1) + }) + + it('disables the delete item when no onDelete is provided', async () => { + render( + + + + ) + + const trigger = screen.getByRole('button', { name: 'Session actions' }) + fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.pointerUp(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.click(trigger) + + const deleteItem = await screen.findByRole('menuitem', { name: /delete/i }) + expect(deleteItem.getAttribute('aria-disabled')).toBe('true') + }) + + it('confirms with the Enter key and cancels with Escape', async () => { + const onDelete = vi.fn() + render( + + + + ) + + const trigger = screen.getByRole('button', { name: 'Session actions' }) + fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.pointerUp(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.click(trigger) + fireEvent.click(await screen.findByRole('menuitem', { name: /delete/i })) + + const dialog = await screen.findByRole('dialog') + expect(dialog).toBeTruthy() + + // Escape cancels: dialog closes, nothing is deleted. + fireEvent.keyDown(window.document, { key: 'Escape' }) + expect(await screen.queryByRole('dialog')).toBeNull() + expect(onDelete).not.toHaveBeenCalled() + + // Re-open and confirm with Enter: the delete call fires. + fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.pointerUp(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.click(trigger) + fireEvent.click(await screen.findByRole('menuitem', { name: /delete/i })) + fireEvent.keyDown(await screen.findByRole('dialog'), { key: 'Enter' }) + + expect(await screen.findByText('Session deleted')).toBeTruthy() + expect(onDelete).toHaveBeenCalledTimes(1) + }) + + it('routes the same confirm guard through the context menu', async () => { + const onDelete = vi.fn() + render( + + + + ) + + const row = screen.getByRole('button', { name: 'Session row' }) + fireEvent.contextMenu(row) + + fireEvent.click(await screen.findByRole('menuitem', { name: /delete/i })) + expect(await screen.findByRole('dialog')).toBeTruthy() + + fireEvent.click(screen.getByRole('button', { name: 'Delete' })) + expect(await screen.findByText('Session deleted')).toBeTruthy() + expect(onDelete).toHaveBeenCalledTimes(1) + }) }) diff --git a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx index 9a0e406f41..3a9e5b8c10 100644 --- a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx @@ -20,8 +20,9 @@ import { import { Button } from '@/components/ui/button' import { Codicon } from '@/components/ui/codicon' import { ColorSwatches } from '@/components/ui/color-swatches' +import { ConfirmDialog } from '@/components/ui/confirm-dialog' import { CopyButton } from '@/components/ui/copy-button' -import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle } from '@/components/ui/dialog' +import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle, preventCloseButtonAutoFocus } from '@/components/ui/dialog' import { Input } from '@/components/ui/input' import { renameSession } from '@/hermes' import { useI18n } from '@/i18n' @@ -198,6 +199,7 @@ function useSessionActions({ // action leaves the restore alone (it's the correct behavior for them). Mirrors // the project menu's appearance-popover guard. const suppressCloseFocusRef = useRef(false) + const [deleteOpen, setDeleteOpen] = useState(false) const tiles = useStore($sessionTiles) const selectedStoredSessionId = useStore($selectedStoredSessionId) const isRemote = useStore($connection)?.mode === 'remote' @@ -399,7 +401,15 @@ function useSessionActions({ label: t.common.delete, onSelect: () => { triggerHaptic('warning') - onDelete?.() + + // Deleting is irreversible (the CLI path asks y/N; the desktop used to + // fire instantly on click). Gate it behind an explicit confirm — see + // #61470. The dialog owns the delete call, so every surface that routes + // through this menu (sidebar rows, tab menus, the chat header) gets the + // guard for free. + if (onDelete) { + setDeleteOpen(true) + } }, variant: 'destructive' } @@ -484,7 +494,50 @@ function useSessionActions({ } } - return { onCloseAutoFocus, renameDialog, renderItems } + const deleteDialog = ( + { + onDelete?.() + }} + onOpenChange={setDeleteOpen} + open={deleteOpen} + sessionTitle={title} + /> + ) + + return { deleteDialog, onCloseAutoFocus, renameDialog, renderItems } +} + +interface DeleteSessionDialogProps { + open: boolean + onOpenChange: (open: boolean) => void + onConfirm: () => void + sessionTitle: string +} + +// Thin wrapper over ConfirmDialog — the single choke point for every session +// delete entry point (sidebar rows, tab menus, the chat header). Deleting a +// session is irreversible and the desktop used to fire it instantly on click +// (#61470); this mirrors the CLI's y/N guard. onConfirm is the fire-and-forget +// delete call; ConfirmDialog owns the busy/done beat and Enter-to-confirm. +function DeleteSessionDialog({ open, onOpenChange, onConfirm, sessionTitle }: DeleteSessionDialogProps) { + const { t } = useI18n() + const r = t.sidebar.row + + return ( + onOpenChange(false)} + onConfirm={onConfirm} + onOpenAutoFocus={preventCloseButtonAutoFocus} + open={open} + title={r.deleteTitle} + /> + ) } interface SessionActionsMenuProps @@ -494,7 +547,7 @@ interface SessionActionsMenuProps export function SessionActionsMenu({ children, align = 'end', sideOffset = 6, ...actions }: SessionActionsMenuProps) { const { t } = useI18n() - const { onCloseAutoFocus, renameDialog, renderItems } = useSessionActions(actions) + const { deleteDialog, onCloseAutoFocus, renameDialog, renderItems } = useSessionActions(actions) return ( <> @@ -509,6 +562,7 @@ export function SessionActionsMenu({ children, align = 'end', sideOffset = 6, .. {children} {renameDialog} + {deleteDialog} ) } @@ -519,7 +573,7 @@ interface SessionContextMenuProps extends SessionActions { export function SessionContextMenu({ children, ...actions }: SessionContextMenuProps) { const { t } = useI18n() - const { onCloseAutoFocus, renameDialog, renderItems } = useSessionActions(actions) + const { deleteDialog, onCloseAutoFocus, renameDialog, renderItems } = useSessionActions(actions) return ( <> @@ -532,6 +586,7 @@ export function SessionContextMenu({ children, ...actions }: SessionContextMenuP {children} {renameDialog} + {deleteDialog} ) } diff --git a/apps/desktop/src/components/ui/confirm-dialog.tsx b/apps/desktop/src/components/ui/confirm-dialog.tsx index becb958a98..d4d5fc327d 100644 --- a/apps/desktop/src/components/ui/confirm-dialog.tsx +++ b/apps/desktop/src/components/ui/confirm-dialog.tsx @@ -28,6 +28,10 @@ interface ConfirmDialogProps { destructive?: boolean /** Close as soon as onConfirm resolves — for optimistic actions that finish in the background. */ dismissOnConfirm?: boolean + /** Focus control for dialogs with no input. Pass `preventCloseButtonAutoFocus` + * so opening doesn't land focus on the close/cancel button (which would pop + * its tooltip with no pointer near it). */ + onOpenAutoFocus?: (event: Event) => void } // Shared confirmation dialog: Enter confirms (from anywhere in the dialog), @@ -44,7 +48,8 @@ export function ConfirmDialog({ doneLabel, cancelLabel, destructive = false, - dismissOnConfirm = false + dismissOnConfirm = false, + onOpenAutoFocus }: ConfirmDialogProps) { const { t } = useI18n() const [status, setStatus] = useState<'done' | 'idle' | 'saving'>('idle') @@ -104,6 +109,7 @@ export function ConfirmDialog({ void run() } }} + onOpenAutoFocus={onOpenAutoFocus} > {title} diff --git a/apps/desktop/src/i18n/ar.ts b/apps/desktop/src/i18n/ar.ts index 1c9db27193..1397b6f4aa 100644 --- a/apps/desktop/src/i18n/ar.ts +++ b/apps/desktop/src/i18n/ar.ts @@ -1668,6 +1668,10 @@ export const ar = defineLocale({ renameTitle: 'إعادة تسمية الجلسة', renameDesc: '', untitledPlaceholder: 'جلسة بلا عنوان', + deleteTitle: 'حذف الجلسة؟', + deleteDesc: title => `سيتم حذف «${title}» نهائيًا. لا يمكن التراجع عن هذا الإجراء.`, + deleting: 'جارٍ الحذف…', + deleted: 'تم حذف الجلسة', ageNow: 'الآن', ageDay: 'يوم', ageHour: 'ساعة', diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index 1e836201ec..f5f021424c 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -2040,6 +2040,10 @@ export const en: Translations = { renameTitle: 'Rename session', renameDesc: 'Leave empty to clear.', untitledPlaceholder: 'Untitled session', + deleteTitle: 'Delete session?', + deleteDesc: title => `This will permanently delete “${title}”. This cannot be undone.`, + deleting: 'Deleting…', + deleted: 'Session deleted', untitledChat: id => `Chat ${id}`, messageCount: count => `${count} ${count === 1 ? 'message' : 'messages'}`, todoProgress: 'Tasks completed', diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index a59bed41c2..db3799c83b 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -1811,6 +1811,10 @@ export const ja = defineLocale({ renameTitle: 'セッションの名前を変更', renameDesc: '空欄にするとクリアされます。', untitledPlaceholder: '無題のセッション', + deleteTitle: 'セッションを削除しますか?', + deleteDesc: title => `「${title}」を完全に削除します。この操作は元に戻せません。`, + deleting: '削除中…', + deleted: 'セッションを削除しました', untitledChat: id => `セッション ${id}`, ageNow: 'たった今', ageDay: '日', diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index ce2066beda..ac803a3782 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -1723,6 +1723,10 @@ export interface Translations { renameTitle: string renameDesc: string untitledPlaceholder: string + deleteTitle: string + deleteDesc: (title: string) => string + deleting: string + deleted: string untitledChat: (id: string) => string messageCount: (count: number) => string todoProgress: string diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index 8efe69eba8..d310074045 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -1753,6 +1753,10 @@ export const zhHant = defineLocale({ renameTitle: '重新命名工作階段', renameDesc: '留空則清除。', untitledPlaceholder: '未命名工作階段', + deleteTitle: '刪除會話?', + deleteDesc: title => `這將永久刪除「${title}」,且無法復原。`, + deleting: '正在刪除…', + deleted: '會話已刪除', untitledChat: id => `工作階段 ${id}`, ageNow: '剛才', ageDay: '天', diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index b133f1c6b3..a1b03f9e45 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -2226,6 +2226,10 @@ export const zh: Translations = { renameTitle: '重命名会话', renameDesc: '留空则清除。', untitledPlaceholder: '无标题会话', + deleteTitle: '删除会话?', + deleteDesc: title => `这将永久删除“${title}”,且无法撤销。`, + deleting: '正在删除…', + deleted: '会话已删除', untitledChat: id => `会话 ${id}`, messageCount: count => `${count} 条消息`, todoProgress: '任务完成度',