Merge pull request #71202 from NousResearch/bb/desktop-session-drop-target

fix(desktop): resolve drop targets and focus against the visible tab

apps/desktop/src/app/chat/composer/focus.test.ts

@@ -0,0 +1,50 @@
+import { afterEach, describe, expect, it } from 'vitest'
+
+import { blurComposerInput } from './focus'
+import { RICH_INPUT_SLOT } from './rich-editor'
+
+/**
+ * Inactive tabs keep their composer mounted, so an unscoped lookup can blur a
+ * background input and leave the one the user is typing in focused.
+ */
+
+/** A composer input inside its own pane layer, hidden or not. */
+function mountInput(hidden = false) {
+  const layer = document.createElement('div')
+  const input = document.createElement('div')
+  input.dataset.slot = RICH_INPUT_SLOT
+  input.tabIndex = 0
+  layer.toggleAttribute('data-pane-hidden', hidden)
+  layer.append(input)
+  document.body.append(layer)
+
+  return input
+}
+
+afterEach(() => {
+  document.body.innerHTML = ''
+})
+
+describe('blurComposerInput', () => {
+  it('blurs the foreground composer while a hidden tab matches first', () => {
+    const background = mountInput(true)
+    const foreground = mountInput()
+
+    foreground.focus()
+    blurComposerInput()
+
+    expect(document.activeElement).not.toBe(foreground)
+    expect(document.activeElement).not.toBe(background)
+  })
+
+  it('leaves focus alone when the composer does not hold it', () => {
+    const outside = document.createElement('button')
+    document.body.append(outside)
+    mountInput()
+
+    outside.focus()
+    blurComposerInput()
+
+    expect(document.activeElement).toBe(outside)
+  })
+})

apps/desktop/src/app/chat/composer/focus.ts

@@ -10,6 +10,8 @@
  * steal focus from the composer effect.
  */
 
+import { queryVisible } from '@/components/pane-shell/pane-visibility'
+
 import type { InlineRefInput } from './inline-refs'
 import { RICH_INPUT_SLOT } from './rich-editor'
 
@@ -175,9 +177,11 @@ export const focusComposerInput = (el: HTMLElement | null) => {
   window.setTimeout(focus, 0)
 }
 
-/** Drop focus from the main composer input (status-stack chrome, sidebar, etc.). */
+/** Drop focus from the main composer input (status-stack chrome, sidebar, etc.).
+ *  Skips inactive tabs — they stay mounted, so an unscoped lookup can land on a
+ *  background composer and leave the visible one focused. */
 export const blurComposerInput = () => {
-  const el = document.querySelector(`[data-slot="${RICH_INPUT_SLOT}"]`) as HTMLElement | null
+  const el = queryVisible(`[data-slot="${RICH_INPUT_SLOT}"]`)
 
   if (el && document.activeElement === el) {
     el.blur()

apps/desktop/src/app/chat/session-drag.test.ts

@@ -0,0 +1,113 @@
+import type { PointerEvent as ReactPointerEvent } from 'react'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { group } from '@/components/pane-shell/tree/model'
+import { $layoutTree } from '@/components/pane-shell/tree/store'
+import { openSessionTile } from '@/store/session-states'
+
+import { requestComposerInsertRefs } from './composer/focus'
+import { startSessionDrag } from './session-drag'
+
+/**
+ * A session drop resolves its target by rect-testing the chat surfaces in the
+ * document. A tab group keeps inactive tabs MOUNTED with their layout box
+ * intact, so a background tab's rect is identical to the foreground tab's —
+ * the drop has to land on the tab the user can actually see.
+ */
+
+vi.mock('@/store/session-states', () => ({ openSessionTile: vi.fn() }))
+vi.mock('./composer/focus', () => ({ requestComposerInsertRefs: vi.fn() }))
+
+const ZONE = { left: 0, top: 0, right: 1000, bottom: 800 }
+const COMPOSER = { left: 100, top: 700, right: 900, bottom: 780 }
+
+const stubRect = (el: Element, box: { left: number; top: number; right: number; bottom: number }) => {
+  el.getBoundingClientRect = () =>
+    ({ ...box, width: box.right - box.left, height: box.bottom - box.top, x: box.left, y: box.top }) as DOMRect
+}
+
+/** The workspace tab kept alive behind an active session tile tab. */
+function mountStackedTabs() {
+  document.body.innerHTML = `
+    <div data-tree-group="g1">
+      <div data-pane-hidden>
+        <div data-session-anchor="workspace" data-composer-target="main">
+          <div data-slot="composer-root"></div>
+        </div>
+      </div>
+      <div>
+        <div data-session-anchor="session-tile:visible" data-composer-target="tile:visible">
+          <div data-slot="composer-root"></div>
+        </div>
+      </div>
+    </div>
+    <div id="row"></div>
+  `
+
+  stubRect(document.querySelector('[data-tree-group]')!, ZONE)
+
+  for (const surface of document.querySelectorAll('[data-session-anchor]')) {
+    stubRect(surface, ZONE)
+  }
+
+  for (const composer of document.querySelectorAll('[data-slot="composer-root"]')) {
+    stubRect(composer, COMPOSER)
+  }
+
+  $layoutTree.set(group(['workspace', 'session-tile:visible'], { id: 'g1' }))
+
+  return document.getElementById('row')!
+}
+
+/** Press on `source`, drag to (x, y), release. The drag session flushes its
+ *  pending move synchronously on release, so no frame wait is needed. */
+function dragTo(source: HTMLElement, x: number, y: number) {
+  startSessionDrag({ id: 'dragged', profile: 'default', title: 'Dragged chat' }, {
+    button: 0,
+    clientX: 0,
+    clientY: 0,
+    currentTarget: source,
+    pointerId: 1
+  } as unknown as ReactPointerEvent<HTMLElement>)
+
+  window.dispatchEvent(new MouseEvent('pointermove', { bubbles: true, clientX: x, clientY: y }))
+  window.dispatchEvent(new MouseEvent('pointerup', { bubbles: true, clientX: x, clientY: y }))
+}
+
+beforeEach(() => {
+  vi.clearAllMocks()
+})
+
+afterEach(() => {
+  document.body.innerHTML = ''
+  $layoutTree.set(null)
+})
+
+describe('session drop targeting across stacked tabs', () => {
+  it('links into the visible tab’s composer, not the tab kept alive behind it', () => {
+    const row = mountStackedTabs()
+
+    dragTo(row, 500, 740)
+
+    expect(requestComposerInsertRefs).toHaveBeenCalledWith(expect.anything(), { target: 'tile:visible' })
+  })
+
+  it('docks a split against the visible tab’s pane', () => {
+    const row = mountStackedTabs()
+
+    dragTo(row, 980, 400)
+
+    expect(openSessionTile).toHaveBeenCalledWith('dragged', 'right', 'session-tile:visible', undefined)
+    expect(requestComposerInsertRefs).not.toHaveBeenCalled()
+  })
+
+  it('commits nothing over a zone that hosts no chat surface', () => {
+    mountStackedTabs()
+    $layoutTree.set(group(['terminal'], { id: 'g1' }))
+
+    dragTo(document.getElementById('row')!, 500, 740)
+
+    expect(requestComposerInsertRefs).not.toHaveBeenCalled()
+    expect(openSessionTile).not.toHaveBeenCalled()
+  })
+})

apps/desktop/src/app/chat/session-drag.ts

@@ -27,6 +27,7 @@
 
 import type { PointerEvent as ReactPointerEvent } from 'react'
 
+import { queryAllVisible } from '@/components/pane-shell/pane-visibility'
 import { findGroup } from '@/components/pane-shell/tree/model'
 import {
   type DoubleTapContext,
@@ -66,8 +67,11 @@ const snapRect = (el: HTMLElement): ZoneRect => {
   return { left: r.left, top: r.top, right: r.right, bottom: r.bottom }
 }
 
+/** Chat surfaces the pointer can land on. Inactive tabs are excluded: they stay
+ *  mounted with their layout box intact, so their rect is identical to the
+ *  visible tab's and a hit-test alone would pick whichever came first. */
 function snapshotSurfaces(): SurfaceSnapshot[] {
-  return [...document.querySelectorAll<HTMLElement>('[data-session-anchor]')].map(el => ({
+  return queryAllVisible('[data-session-anchor]').map(el => ({
     anchor: el.dataset.sessionAnchor || 'workspace',
     composerTarget: el.dataset.composerTarget || 'main',
     rect: snapRect(el)
@@ -123,7 +127,7 @@ export function startSessionDrag(
       zones = snapshotZones()
       strips = snapshotStrips()
       surfaces = snapshotSurfaces()
-      composers = [...document.querySelectorAll<HTMLElement>('[data-slot="composer-root"]')].map(snapRect)
+      composers = queryAllVisible('[data-slot="composer-root"]').map(snapRect)
       zoneHost = new Map(zones.map(zone => [zone.id, chatZonePane(zone.id)]))
       source?.style.setProperty('opacity', '0.45')
       // The same sentinel the zone overlay + chat surfaces key off — the

apps/desktop/src/components/assistant-ui/thread/timeline.test.ts

@@ -0,0 +1,42 @@
+import { afterEach, describe, expect, it } from 'vitest'
+
+import { ownViewport } from './timeline'
+
+/**
+ * Several chat surfaces are mounted at once — side by side in a split, and
+ * stacked as kept-alive inactive tabs. A timeline scrolls its OWN thread.
+ */
+
+afterEach(() => {
+  document.body.innerHTML = ''
+})
+
+const surface = (id: string, hidden = false) => `
+  <div ${hidden ? 'data-pane-hidden' : ''}>
+    <div data-session-anchor="${id}">
+      <div data-slot="aui_thread-viewport" id="viewport-${id}"></div>
+      <div data-slot="thread-timeline" id="timeline-${id}"></div>
+    </div>
+  </div>
+`
+
+describe('ownViewport', () => {
+  it('resolves the viewport of the surface the timeline lives in', () => {
+    document.body.innerHTML = surface('workspace') + surface('session-tile:b')
+
+    expect(ownViewport(document.getElementById('timeline-session-tile:b'))?.id).toBe('viewport-session-tile:b')
+    expect(ownViewport(document.getElementById('timeline-workspace'))?.id).toBe('viewport-workspace')
+  })
+
+  it('ignores a kept-alive tab that matches first', () => {
+    document.body.innerHTML = surface('workspace', true) + surface('session-tile:b')
+
+    expect(ownViewport(document.getElementById('timeline-session-tile:b'))?.id).toBe('viewport-session-tile:b')
+  })
+
+  it('falls back to the document when there is no surface around it', () => {
+    document.body.innerHTML = '<div data-slot="aui_thread-viewport" id="viewport-lone"></div>'
+
+    expect(ownViewport(null)?.id).toBe('viewport-lone')
+  })
+})

apps/desktop/src/components/assistant-ui/thread/timeline.tsx

@@ -101,8 +101,15 @@ function jumpScroll(viewport: HTMLElement, top: number, duration = 170): void {
   jumpRaf = requestAnimationFrame(step)
 }
 
-function scrollToPrompt(id: string) {
-  const viewport = document.querySelector<HTMLElement>(VIEWPORT)
+// A timeline belongs to ONE chat surface, and several are mounted at once — side
+// by side in a split, and stacked (hidden but kept alive) as inactive tabs. Walk
+// up to this timeline's own surface before looking for the viewport; a
+// document-wide lookup scrolls somebody else's thread.
+export const ownViewport = (root: HTMLElement | null): HTMLElement | null =>
+  (root?.closest('[data-session-anchor]') ?? document).querySelector<HTMLElement>(VIEWPORT)
+
+function scrollToPrompt(root: HTMLElement | null, id: string) {
+  const viewport = ownViewport(root)
   const node = viewport?.querySelector<HTMLElement>(`[data-message-id="${CSS.escape(id)}"]`)
 
   if (!viewport || !node) {
@@ -139,6 +146,8 @@ export const ThreadTimeline: FC = () => {
   const [activeIndex, setActiveIndex] = useState(0)
   const [open, setOpen] = useState(false)
   const closeTimerRef = useRef<number | undefined>(undefined)
+  const rootRef = useRef<HTMLDivElement | null>(null)
+  const jump = useCallback((id: string) => scrollToPrompt(rootRef.current, id), [])
 
   // Hover sync lives on the DOM, not in React state — the tick and its popover
   // row are siblings in different subtrees, so a shared index-keyed paint() lights
@@ -176,7 +185,7 @@ export const ThreadTimeline: FC = () => {
   useEffect(() => () => window.clearTimeout(closeTimerRef.current), [])
 
   useEffect(() => {
-    const viewport = document.querySelector<HTMLElement>(VIEWPORT)
+    const viewport = ownViewport(rootRef.current)
 
     if (!viewport || entries.length === 0) {
       return
@@ -236,20 +245,15 @@ export const ThreadTimeline: FC = () => {
       data-suppress-pane-reveal=""
       onMouseEnter={keepOpen}
       onMouseLeave={closeSoon}
+      ref={rootRef}
       role="navigation"
     >
-      <TimelineTicks
-        activeIndex={activeIndex}
-        entries={entries}
-        onHover={paint}
-        onJump={scrollToPrompt}
-        tickRefs={tickRefs}
-      />
+      <TimelineTicks activeIndex={activeIndex} entries={entries} onHover={paint} onJump={jump} tickRefs={tickRefs} />
       <TimelinePopover
         activeIndex={activeIndex}
         entries={entries}
         onHover={paint}
-        onJump={scrollToPrompt}
+        onJump={jump}
         open={open}
         rowRefs={rowRefs}
       />

apps/desktop/src/components/pane-shell/pane-visibility.test.ts

@@ -0,0 +1,41 @@
+import { afterEach, describe, expect, it } from 'vitest'
+
+import { hiddenPaneProps, PANE_HIDDEN_ATTR, queryAllVisible, queryVisible } from './pane-visibility'
+
+/**
+ * Inactive tabs stay mounted with their layout box intact, so they answer
+ * document-wide lookups exactly like the visible tab. These helpers are the one
+ * place that difference is decided.
+ */
+
+const COMPOSER = '[data-slot="composer-root"]'
+
+const tab = (id: string, hidden = false) => `
+  <div ${hidden ? PANE_HIDDEN_ATTR : ''}>
+    <section><div data-slot="composer-root" id="${id}"></div></section>
+  </div>
+`
+
+afterEach(() => {
+  document.body.innerHTML = ''
+})
+
+describe('pane visibility lookups', () => {
+  it('resolves the foreground element even when a hidden tab matches first', () => {
+    document.body.innerHTML = tab('background', true) + tab('foreground')
+
+    expect(queryVisible(COMPOSER)?.id).toBe('foreground')
+    expect(queryAllVisible(COMPOSER).map(el => el.id)).toEqual(['foreground'])
+  })
+
+  it('answers normally when nothing is hidden', () => {
+    document.body.innerHTML = tab('only')
+
+    expect(queryVisible(COMPOSER)?.id).toBe('only')
+  })
+
+  it('marks a pane hidden only while it is inactive', () => {
+    expect(hiddenPaneProps(true)).toEqual({ [PANE_HIDDEN_ATTR]: '' })
+    expect(hiddenPaneProps(false)).toEqual({})
+  })
+})

apps/desktop/src/components/pane-shell/pane-visibility.ts

@@ -0,0 +1,27 @@
+/**
+ * Keep-alive visibility — the one policy every document-wide lookup must obey.
+ *
+ * A tab group keeps each ever-active pane MOUNTED and hides the inactive ones
+ * with `visibility: hidden` (see `tree/renderer/tree-group.tsx`), deliberately
+ * preserving their layout box so scroll positions survive a tab round-trip. The
+ * cost is that an inactive tab's rect is IDENTICAL to the visible tab's, so
+ * neither selector order nor a rect hit-test can tell them apart. A lookup that
+ * resolves "the chat surface / composer / viewport" from the document therefore
+ * has to skip hidden panes, or it silently answers with the wrong tab.
+ */
+
+/** Marks a mounted-but-hidden pane layer (an inactive tab in a stack). */
+export const PANE_HIDDEN_ATTR = 'data-pane-hidden'
+
+const HIDDEN_PANE = `[${PANE_HIDDEN_ATTR}]`
+
+/** Spread onto a kept pane layer so the lookups below can skip it. */
+export const hiddenPaneProps = (hidden: boolean): Record<string, string> => (hidden ? { [PANE_HIDDEN_ATTR]: '' } : {})
+
+/** `querySelectorAll` minus anything inside an inactive tab. */
+export const queryAllVisible = <T extends HTMLElement>(selector: string, root: ParentNode = document): T[] =>
+  [...root.querySelectorAll<T>(selector)].filter(el => !el.closest(HIDDEN_PANE))
+
+/** `querySelector` minus anything inside an inactive tab. */
+export const queryVisible = <T extends HTMLElement>(selector: string, root: ParentNode = document): null | T =>
+  queryAllVisible<T>(selector, root)[0] ?? null

apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx

@@ -30,6 +30,7 @@ import { cn } from '@/lib/utils'
 
 import { $layoutEditMode } from '../../edit-mode'
 import { useWindowControlsOverlap } from '../../geometry'
+import { hiddenPaneProps } from '../../pane-visibility'
 import type { DropPosition, GroupNode, RootEdge } from '../model'
 import { adjacentGroup } from '../model'
 import {
@@ -520,7 +521,9 @@ export function TreeGroup({
       {/* Body: the zone's pane content — every kept (ever-active) pane stays
           mounted in an absolute layer; only the active one is visible.
           `visibility` (not display) keeps the hidden pane's layout box, so
-          scroll positions and measurements survive the round-trip. */}
+          scroll positions and measurements survive the round-trip — which also
+          makes a hidden layer's rect identical to the visible one's, hence the
+          marker document-wide lookups filter on (see pane-visibility.ts). */}
       {!node.minimized && (
         <div className="relative min-h-0 min-w-0 flex-1 overflow-hidden">
           {isEmpty ? (
@@ -538,6 +541,7 @@ export function TreeGroup({
                   aria-hidden={!isActive || undefined}
                   className={cn('absolute inset-0 overflow-auto', !isActive && 'pointer-events-none invisible')}
                   key={paneId}
+                  {...hiddenPaneProps(!isActive)}
                 >
                   {pane?.render ? (
                     <ContribBoundary id={pane.id}>{pane.render()}</ContribBoundary>

apps/desktop/src/lib/keybinds/composer-focus-keys.test.ts

@@ -88,6 +88,17 @@ describe('composerFocusBlockedBySurface', () => {
     expect(composerFocusBlockedBySurface()).toBe(true)
   })
 
+  it('ignores a clarify card waiting in a kept-alive background tab', () => {
+    const tab = document.createElement('div')
+    tab.setAttribute('data-pane-hidden', '')
+    const card = document.createElement('div')
+    card.setAttribute('data-clarify-choices', '')
+    tab.append(card)
+    document.body.append(tab)
+
+    expect(composerFocusBlockedBySurface()).toBe(false)
+  })
+
   it('blocks when focus is inside a terminal', () => {
     const term = document.createElement('div')
     term.setAttribute('data-terminal', '')

apps/desktop/src/lib/keybinds/composer-focus-keys.ts

@@ -7,6 +7,7 @@
  */
 
 import { $workspaceIsPage } from '@/app/routes'
+import { queryVisible } from '@/components/pane-shell/pane-visibility'
 import { switcherActive } from '@/store/session-switcher'
 
 import { isEditableTarget, isFocusWithin } from './combo'
@@ -37,8 +38,15 @@ const ENTER_ACTIVATES = [
   '[role="treeitem"]'
 ].join(',')
 
-const BLOCKING_SURFACE =
-  '[role="dialog"],[role="alertdialog"],[role="menu"],[role="listbox"],[data-radix-popper-content-wrapper],[data-overlay-surface],[data-clarify-choices]'
+// Overlays that cover the whole window (portaled to the body, or the overlay
+// shell itself) — one anywhere means the composer is behind it.
+const BLOCKING_OVERLAY =
+  '[role="dialog"],[role="alertdialog"],[role="menu"],[role="listbox"],[data-radix-popper-content-wrapper],[data-overlay-surface]'
+
+// Blockers that live INSIDE a chat surface. Inactive tabs stay mounted, so this
+// one has to be visible-scoped: a clarify card waiting in a background thread
+// must not take the foreground composer's letter keys.
+const BLOCKING_IN_SURFACE = '[data-clarify-choices]'
 
 /** True when the focused control would normally handle Enter itself. */
 export function isActivateOnEnterTarget(target: EventTarget | null): boolean {
@@ -59,7 +67,8 @@ export function composerFocusBlockedBySurface(): boolean {
     switcherActive() ||
     $workspaceIsPage.get() ||
     isFocusWithin('[data-terminal]') ||
-    Boolean(document.querySelector(BLOCKING_SURFACE))
+    Boolean(document.querySelector(BLOCKING_OVERLAY)) ||
+    Boolean(queryVisible(BLOCKING_IN_SURFACE))
   )
 }