hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 29 Packages Projects Releases Wiki Activity

fix(desktop): a sidebar row only owns presses that started inside it

Browse Source 
React re-dispatches an event fired in a portal along the REACT tree, not the DOM
tree. DialogContent portals into <body>, but in the React tree the session
rename dialog is a child of the session row (SessionActionsMenu renders inside
the row's actions slot), so a pointerdown inside the dialog's input still reached
the row shell's own onPointerDown with a target outside the row. The shell's only
guard walks the DOM (target.closest('[data-reorder-handle], [data-row-actions]')),
which cannot match anything mounted at <body>, so the handler fell through to
startSessionDrag(...) — the shared drag session's threshold is 4px, and any mouse
text selection crosses it — and to the forwarded dnd-kit pointer activator:
selecting the session title with the mouse lifted the row, lit every drop target
and armed the reorder.

Gate each shell's own onPointerDown on shellOwnsPress() (a press that started
inside the row's own DOM) before the existing marker exclusion: the session row,
the project row and the gateway group header. The keyboard side of the same leak
was already fixed in #115333.

Coordinates are untouched; the grabber keeps the full dnd-kit handle, so a press
on the row itself still runs both drags off one gesture.

Fixes #116080
This commit is contained in:
zqy1-1
2026-09-19 20:46:44 +08:00
committed by Teknium
parent e1ffc7bad3
commit c143e04498
5 changed files with 434 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
apps/desktop/src/app/chat/sidebar/gateway-groups.tsx
View File

@@ -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({
</SidebarRowGrab>
}
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
}

8
apps/desktop/src/app/chat/sidebar/projects/overview-row.tsx
View File

@@ -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
}

18
apps/desktop/src/app/chat/sidebar/reorderable-list.tsx
View File

@@ -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 `<body>`
* — 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<HTMLElement>) {
const target = event.target
return target instanceof Node && event.currentTarget.contains(target)
}

393
apps/desktop/src/app/chat/sidebar/session-row-rename-drag.test.tsx Normal file
View File

@@ -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 <body>.
// 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<typeof SessionDrag>()
// 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<Record<string, unknown>>()
return {
...actual,
closeAllTreeTabs: vi.fn(),
closeOtherTreeTabs: vi.fn(),
closeTreeTabsToRight: vi.fn(),
treeTabCloseTargets: vi.fn(() => null)
}
})
vi.mock('@/hermes', async importOriginal => {
const actual = await importOriginal<Record<string, unknown>>()
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<Record<string, unknown>>()
return { ...actual, triggerHaptic: vi.fn() }
})
vi.mock('@/lib/profile-color', async importOriginal => {
const actual = await importOriginal<Record<string, unknown>>()
return { ...actual, PROFILE_SWATCHES: [] }
})
vi.mock('@/lib/session-export', async importOriginal => {
const actual = await importOriginal<Record<string, unknown>>()
return { ...actual, exportSession: vi.fn() }
})
vi.mock('@/lib/session-source', async importOriginal => {
const actual = await importOriginal<Record<string, unknown>>()
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<Record<string, unknown>>()
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<typeof ChatRuntime>()
return { ...actual, sessionTitle: (s: SessionInfo) => (s as unknown as { title: string }).title }
})
vi.mock('@/store/gateway', async importOriginal => {
const actual = await importOriginal<typeof GatewayStore>()
return { ...actual, activeGateway: vi.fn(() => null) }
})
vi.mock('@/store/notifications', async importOriginal => {
const actual = await importOriginal<Record<string, unknown>>()
return { ...actual, notify: vi.fn(), notifyError: vi.fn() }
})
vi.mock('@/store/projects', async importOriginal => {
const actual = await importOriginal<typeof ProjectsStore>()
return { ...actual, $projectTree: atom<unknown[]>([]) }
})
vi.mock('@/store/session', async importOriginal => {
const actual = await importOriginal<typeof SessionStore>()
return { ...actual, $unreadFinishedSessionIds: atom<string[]>([]) }
})
vi.mock('@/store/session-color', async importOriginal => {
const actual = await importOriginal<typeof SessionColorStore>()
return { ...actual, $sessionColorOverrides: atom<Record<string, string>>({}) }
})
vi.mock('@/store/session-states', async importOriginal => {
const actual = await importOriginal<typeof SessionStatesStore>()
return {
...actual,
$attentionSessionIds: atom<string[]>([]),
$sessionTiles: atom<unknown[]>([]),
$stalledSessionIds: atom<string[]>([]),
openSessionTile: vi.fn()
}
})
vi.mock('@/store/windows', async importOriginal => {
const actual = await importOriginal<typeof WindowsStore>()
return {
...actual,
canOpenSessionInTerminal: () => false,
canOpenSessionWindow: () => false,
openSessionInNewWindow: vi.fn(),
openSessionInTerminal: vi.fn()
}
})
function makeSession(overrides: Partial<SessionInfo> & { 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 (
<SidebarSessionRow
dragging={dragging}
dragHandleProps={dragHandleProps}
isPinned={false}
isSelected={false}
onArchive={noop}
onDelete={noop}
onPin={noop}
onResume={noop}
onToggleUnread={noop}
ref={ref}
reorderable={reorderable}
session={session}
style={style}
unread={false}
/>
)
}
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 (
<ReorderableList ids={[session.id]} onReorder={noop} sensors={sensors}>
<SortableRow session={session} />
</ReorderableList>
)
}
/** 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<HTMLElement>('.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(<Host session={makeSession({ title: 'Renamable' })} />)
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
// <body>) 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<HTMLElement>('[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(<Host session={makeSession({ title: 'Draggable' })} />)
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 })
})
})

8
apps/desktop/src/app/chat/sidebar/session-row.tsx
View File

@@ -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]')) {