diff --git a/apps/desktop/src/app/chat/sidebar/gateway-groups.tsx b/apps/desktop/src/app/chat/sidebar/gateway-groups.tsx index c108523841..659b2f394a 100644 --- a/apps/desktop/src/app/chat/sidebar/gateway-groups.tsx +++ b/apps/desktop/src/app/chat/sidebar/gateway-groups.tsx @@ -40,7 +40,7 @@ import { rankSessions } from './order' import { SIDEBAR_GROUP_PAGE } from './projects/model' import type { SidebarSessionGroup } from './projects/workspace-groups' import { WorkspaceAddButton, WorkspaceShowMoreButton } from './projects/workspace-header' -import { ReorderableList, useSortableBindings } from './reorderable-list' +import { ReorderableList, shellOwnsPress, useSortableBindings } from './reorderable-list' interface GatewayProfileGroupsProps { groups: SidebarSessionGroup[] @@ -277,6 +277,12 @@ function GatewayProfileGroup({ } onPointerDown={event => { + // The group's ⋯ menu portals out of this row's React subtree: gate the + // shell on a press that actually started inside it. + if (!shellOwnsPress(event)) { + return + } + if ((event.target as HTMLElement).closest('[data-reorder-handle], [data-row-actions]')) { return } diff --git a/apps/desktop/src/app/chat/sidebar/projects/overview-row.tsx b/apps/desktop/src/app/chat/sidebar/projects/overview-row.tsx index ba4a6197f5..e005d00b00 100644 --- a/apps/desktop/src/app/chat/sidebar/projects/overview-row.tsx +++ b/apps/desktop/src/app/chat/sidebar/projects/overview-row.tsx @@ -23,6 +23,7 @@ import { SidebarRowNest, SidebarRowShell } from '../chrome' +import { shellOwnsPress } from '../reorderable-list' import { expandedProjectSessions, latestProjectSessions, PROJECT_PREVIEW_COUNT, useWorkspaceNodeOpen } from './model' import { ProjectContextMenu, ProjectMenu } from './project-menu' @@ -228,6 +229,13 @@ export function ProjectOverviewRow({ // A project row has no rival drag (its title navigates on CLICK), so the // sortable owns the press outright. onPointerDown={event => { + // The project row's ⋯ menu and its confirm dialog portal out of this + // row's React subtree — a press on either arrives with a target outside + // the row, so gate the shell on presses that started inside it. + if (!shellOwnsPress(event)) { + return + } + if ((event.target as HTMLElement).closest('[data-reorder-handle], [data-row-actions]')) { return } diff --git a/apps/desktop/src/app/chat/sidebar/reorderable-list.tsx b/apps/desktop/src/app/chat/sidebar/reorderable-list.tsx index 29c25ca783..07df290335 100644 --- a/apps/desktop/src/app/chat/sidebar/reorderable-list.tsx +++ b/apps/desktop/src/app/chat/sidebar/reorderable-list.tsx @@ -86,3 +86,21 @@ export function useSortableBindings(id: string) { } } } + +/** + * A row shell owns the presses that STARTED inside its own DOM, and nothing + * else. React re-dispatches an event fired in a PORTAL along the REACT tree, + * so a pointerdown on a dialog's input — `DialogContent` portals into `` + * — still arrives at the row shell that rendered the dialog, carrying a + * `target` outside the row. Those presses belong to the dialog: selecting a + * session title in the rename input must not arm a reorder or lift the row onto + * the shared drag session (the pointer-side sibling of the Space leak #83617 + * fixed on the keyboard side). Gate the shell's own `onPointerDown` with this + * BEFORE its `[data-reorder-handle], [data-row-actions]` exclusion — that + * selector walks the DOM, where a portal's content has neither marker. + */ +export function shellOwnsPress(event: React.PointerEvent) { + const target = event.target + + return target instanceof Node && event.currentTarget.contains(target) +} diff --git a/apps/desktop/src/app/chat/sidebar/session-row-rename-drag.test.tsx b/apps/desktop/src/app/chat/sidebar/session-row-rename-drag.test.tsx new file mode 100644 index 0000000000..ea425fd958 --- /dev/null +++ b/apps/desktop/src/app/chat/sidebar/session-row-rename-drag.test.tsx @@ -0,0 +1,393 @@ +import { KeyboardSensor, PointerSensor, useSensor, useSensors } from '@dnd-kit/core' +import { sortableKeyboardCoordinates } from '@dnd-kit/sortable' +import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react' +import { atom } from 'nanostores' +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest' + +import { startSessionDrag } from '@/app/chat/session-drag' +import type * as SessionDrag from '@/app/chat/session-drag' +import type { SessionInfo } from '@/hermes' +import type * as ChatRuntime from '@/lib/chat-runtime' +import type * as GatewayStore from '@/store/gateway' +import type * as ProjectsStore from '@/store/projects' +import type * as SessionStore from '@/store/session' +import type * as SessionColorStore from '@/store/session-color' +import type * as SessionStatesStore from '@/store/session-states' +import type * as WindowsStore from '@/store/windows' + +import { ReorderableList, useSortableBindings } from './reorderable-list' +import { SidebarSessionRow } from './session-row' + +afterEach(() => { + cleanup() + vi.clearAllMocks() +}) + +// The MOUSE-side sibling of the #83617 regression covered in +// session-row.test.tsx. The row renders its ⋯ menu (and through it the rename +// dialog) INSIDE its own React subtree, and DialogContent portals into . +// React re-dispatches an event fired in a portal along the REACT tree, so a +// pointerdown inside the dialog's input still reaches the row shell's own +// onPointerDown with a `target` that is NOT a DOM descendant of the row — +// selecting the title with the mouse then lifted the row onto the shared drag +// session (ghost = the session title, every zone lit as a drop target, and a +// release over the composer inserts an @session chip) while also arming the +// dnd-kit reorder. +// +// The menu and the dialog are the REAL components here (no stub): the press +// really does travel through a portal. Only the drag sink and the sinks the +// row/menu reach for on open are mocked. +vi.mock('@/app/chat/session-drag', async importOriginal => { + const actual = await importOriginal() + + // Spy-WRAPPER, not a stub: the assertions count the calls, while the real + // session still runs so the engage chrome a user sees (the grabbing cursor, + // the row's lifted look) is exercised rather than assumed. + return { ...actual, startSessionDrag: vi.fn(actual.startSessionDrag) } +}) +vi.mock('@/components/pane-shell/tree/store', async importOriginal => { + const actual = await importOriginal>() + + return { + ...actual, + closeAllTreeTabs: vi.fn(), + closeOtherTreeTabs: vi.fn(), + closeTreeTabsToRight: vi.fn(), + treeTabCloseTargets: vi.fn(() => null) + } +}) +vi.mock('@/hermes', async importOriginal => { + const actual = await importOriginal>() + + return { ...actual, renameSession: vi.fn() } +}) +vi.mock('@/i18n', () => ({ + useI18n: () => ({ + t: { + assistant: { + thread: { + today: (time: string) => `Today at ${time}`, + yesterday: (time: string) => `Yesterday at ${time}` + } + }, + common: { + cancel: 'Cancel', + close: 'Close', + confirm: 'Confirm', + delete: 'Delete', + done: 'Done', + loading: 'Loading…', + save: 'Save' + }, + errors: { genericFailure: 'Something went wrong' }, + sidebar: { + messageCount: (count: number) => `${count} messages`, + projects: { + home: 'Home', + menuAppearance: 'Appearance', + moveFailed: 'Could not move session', + moveNoProjects: 'No other projects', + movedTo: (name: string) => `Moved to ${name}`, + moveToProject: 'Move to project', + noColor: 'No color' + }, + row: { + ageMin: 'm', + ageNow: 'now', + archive: 'Archive', + backgroundRunning: 'Running in background', + branchFrom: 'Branch from here', + copyId: 'Copy ID', + copyIdFailed: 'Failed to copy ID', + deleteDesc: (title: string) => `Delete ${title}?`, + deleteTitle: 'Delete session?', + deleted: 'Session deleted', + deleting: 'Deleting…', + export: 'Export', + finishedUnread: 'Finished', + handoffOrigin: (platform: string) => `Started on ${platform}`, + hideTabBar: 'Hide tab bar', + markRead: 'Mark as read', + messageCount: (count: number) => `${count} messages`, + needsInput: 'Needs input', + pin: 'Pin', + rename: 'Rename', + renamed: 'Renamed', + renameFailed: 'Rename failed', + renameTitle: 'Rename session', + sessionActions: 'Session actions', + sessionRunning: 'Running', + todoProgress: 'Tasks completed', + unpin: 'Unpin', + untitledPlaceholder: 'Untitled', + waitingForAnswer: 'Waiting for answer' + }, + toolCallCount: (count: number) => `${count} tool calls` + }, + zones: { closeAll: 'Close all', closeOthers: 'Close others', closeToRight: 'Close to the right' } + } + }) +})) +vi.mock('@/lib/haptics', async importOriginal => { + const actual = await importOriginal>() + + return { ...actual, triggerHaptic: vi.fn() } +}) +vi.mock('@/lib/profile-color', async importOriginal => { + const actual = await importOriginal>() + + return { ...actual, PROFILE_SWATCHES: [] } +}) +vi.mock('@/lib/session-export', async importOriginal => { + const actual = await importOriginal>() + + return { ...actual, exportSession: vi.fn() } +}) +vi.mock('@/lib/session-source', async importOriginal => { + const actual = await importOriginal>() + + return { + ...actual, + handoffOriginSource: (state?: string, platform?: string) => (state && platform ? platform : null), + sessionSourceLabel: (source: string) => source + } +}) +vi.mock('@/lib/time', async importOriginal => { + const actual = await importOriginal>() + + return { ...actual, coarseElapsed: () => ({ unit: 'minute' as const, value: 5 }) } +}) + +// importOriginal + named overrides (never a wholesale replacement): both the +// row and the menu read more of these stores than this file names, and an +// unlisted export silently becomes `undefined` and crashes a nanostores +// `computed()` downstream. +vi.mock('@/lib/chat-runtime', async importOriginal => { + const actual = await importOriginal() + + return { ...actual, sessionTitle: (s: SessionInfo) => (s as unknown as { title: string }).title } +}) +vi.mock('@/store/gateway', async importOriginal => { + const actual = await importOriginal() + + return { ...actual, activeGateway: vi.fn(() => null) } +}) +vi.mock('@/store/notifications', async importOriginal => { + const actual = await importOriginal>() + + return { ...actual, notify: vi.fn(), notifyError: vi.fn() } +}) +vi.mock('@/store/projects', async importOriginal => { + const actual = await importOriginal() + + return { ...actual, $projectTree: atom([]) } +}) +vi.mock('@/store/session', async importOriginal => { + const actual = await importOriginal() + + return { ...actual, $unreadFinishedSessionIds: atom([]) } +}) +vi.mock('@/store/session-color', async importOriginal => { + const actual = await importOriginal() + + return { ...actual, $sessionColorOverrides: atom>({}) } +}) +vi.mock('@/store/session-states', async importOriginal => { + const actual = await importOriginal() + + return { + ...actual, + $attentionSessionIds: atom([]), + $sessionTiles: atom([]), + $stalledSessionIds: atom([]), + openSessionTile: vi.fn() + } +}) +vi.mock('@/store/windows', async importOriginal => { + const actual = await importOriginal() + + return { + ...actual, + canOpenSessionInTerminal: () => false, + canOpenSessionWindow: () => false, + openSessionInNewWindow: vi.fn(), + openSessionInTerminal: vi.fn() + } +}) + +function makeSession(overrides: Partial & { title: string }): SessionInfo { + return { + handoff_platform: null, + handoff_state: null, + id: 's1', + last_active: 0, + profile: 'default', + started_at: 0, + ...overrides + } as unknown as SessionInfo +} + +const noop = vi.fn() + +function SortableRow({ session }: { session: SessionInfo }) { + const { dragHandleProps, dragging, ref, reorderable, style } = useSortableBindings(session.id) + + return ( + + ) +} + +function Host({ session }: { session: SessionInfo }) { + // The sidebar's own sensor set (index.tsx dndSensors). + const sensors = useSensors( + useSensor(PointerSensor, { activationConstraint: { distance: 6 } }), + useSensor(KeyboardSensor, { coordinateGetter: sortableKeyboardCoordinates }) + ) + + return ( + + + + ) +} + +/** Open the row's ⋯ menu and click Rename — the real dialog, opened the real way. */ +async function openRenameDialog() { + const trigger = screen.getByRole('button', { name: 'Session actions' }) + + // Radix's dropdown trigger opens on pointerdown, not on a bare click. + fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.pointerUp(trigger, { button: 0, pointerType: 'mouse' }) + fireEvent.click(trigger) + + fireEvent.click(await screen.findByRole('menuitem', { name: /rename/i })) + + const dialog = await screen.findByRole('dialog') + + return within(dialog).getByRole('textbox') +} + +// drag session coalesces its moves into that frame (drag-session.ts: onMove -> +// rAF -> processMove -> engage). Stub it with a 16ms timer so the engage the +// user actually sees (the grabbing cursor, the lifted row) happens here too. +beforeAll(() => { + vi.stubGlobal( + 'requestAnimationFrame', + (callback: FrameRequestCallback) => setTimeout(() => callback(Date.now()), 16) as unknown as number + ) + vi.stubGlobal('cancelAnimationFrame', (handle: number) => clearTimeout(handle)) +}) + +afterAll(() => vi.unstubAllGlobals()) + +/** Both rAF (now a timer) and the drag session's own bookkeeping have run. */ +const nextFrame = () => new Promise(resolve => setTimeout(resolve, 48)) + +const rowShell = (container: HTMLElement) => container.querySelector('.row-hover')! + +/** What the drag session paints on engage — the row's lifted look. */ +const LIFTED_OPACITY = '0.45' + +describe('SidebarSessionRow drag and the rename dialog (portal press)', () => { + it("ignores the dialog's own presses, so selecting the title cannot lift the row", async () => { + const { container } = render() + const input = await openRenameDialog() + const row = rowShell(container) + const doc = container.ownerDocument + + // Opening the menu and the dialog is itself clean: the ⋯ press lands on the + // row's own data-row-actions cluster, and the menu-item click starts nothing. + expect(startSessionDrag).not.toHaveBeenCalled() + + // A mouse text-selection drag: press inside the dialog's input (a portal in + // ) and move past both thresholds — the drag session's own 4px and + // dnd-kit's 6px activation distance. + fireEvent.pointerDown(input, { + button: 0, + clientX: 20, + clientY: 20, + isPrimary: true, + pointerId: 1, + pointerType: 'mouse' + }) + await act(async () => { + fireEvent.pointerMove(doc, { clientX: 80, clientY: 20, isPrimary: true, pointerId: 1 }) + await nextFrame() + }) + + expect(startSessionDrag).not.toHaveBeenCalled() + expect(row.style.opacity).not.toBe(LIFTED_OPACITY) + expect(container.querySelector('[data-glass-opaque]')).toBeNull() + + fireEvent.pointerUp(doc, { clientX: 80, clientY: 20, isPrimary: true, pointerId: 1 }) + + // The modal backdrop portals out of the same React subtree: pressing it and + // moving must not lift the row either. + const overlay = doc.querySelector('[data-slot="dialog-overlay"]')! + fireEvent.pointerDown(overlay, { + button: 0, + clientX: 10, + clientY: 10, + isPrimary: true, + pointerId: 2, + pointerType: 'mouse' + }) + await act(async () => { + fireEvent.pointerMove(doc, { clientX: 90, clientY: 10, isPrimary: true, pointerId: 2 }) + await nextFrame() + }) + + expect(startSessionDrag).not.toHaveBeenCalled() + expect(row.style.opacity).not.toBe(LIFTED_OPACITY) + + fireEvent.pointerUp(doc, { clientX: 90, clientY: 10, isPrimary: true, pointerId: 2 }) + + // Leave no modal behind in the document for the next test. + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })) + }) + + it('still lifts the row from a press on its own body', async () => { + const { container } = render() + const row = rowShell(container) + const doc = container.ownerDocument + const body = screen.getByText('Draggable').closest('button')! + + fireEvent.pointerDown(body, { + button: 0, + clientX: 20, + clientY: 100, + isPrimary: true, + pointerId: 3, + pointerType: 'mouse' + }) + await act(async () => { + fireEvent.pointerMove(doc, { clientX: 20, clientY: 160, isPrimary: true, pointerId: 3 }) + await nextFrame() + }) + + // The row's OWN press still runs BOTH drags off one gesture: the shared + // session engages (the row takes the lifted look the user sees) and the + // dnd-kit reorder arms (the row goes opaque, the grabber reports pressed). + expect(startSessionDrag).toHaveBeenCalledTimes(1) + expect(row.style.opacity).toBe(LIFTED_OPACITY) + expect(row.getAttribute('data-glass-opaque')).not.toBeNull() + expect(container.querySelector('[data-reorder-handle][aria-pressed="true"]')).not.toBeNull() + + fireEvent.pointerUp(doc, { clientX: 20, clientY: 160, isPrimary: true, pointerId: 3 }) + }) +}) diff --git a/apps/desktop/src/app/chat/sidebar/session-row.tsx b/apps/desktop/src/app/chat/sidebar/session-row.tsx index c9c483f091..63919055e0 100644 --- a/apps/desktop/src/app/chat/sidebar/session-row.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-row.tsx @@ -46,6 +46,7 @@ import { SidebarRowLeadGlyph, SidebarRowShell } from './chrome' +import { shellOwnsPress } from './reorderable-list' import { SessionActionsMenu, SessionContextMenu } from './session-actions-menu' import { sessionRowDetails } from './session-row-details' import { resolveSessionRowClick } from './session-row-gesture' @@ -380,6 +381,13 @@ function SidebarSessionRowImpl({ // activator only; the full handle stays on the grabber (see // useSortableBindings). onPointerDown={event => { + // The rename dialog and the ⋯ menu portal out of this row's React + // subtree, so their presses land here with a target outside the row — + // select the title in the dialog's input and the row would lift. + if (!shellOwnsPress(event)) { + return + } + // The grabber already carries these same listeners, and the ⋯ // cluster keeps its own gestures. if ((event.target as HTMLElement).closest('[data-reorder-handle], [data-row-actions]')) {