fix(desktop): heal type-to-focus onto the visible chat surface

Type-to-focus routes through requestComposerFocus('active'), which resolved
to a module-level activeTarget claim. Inactive tabs stay mounted under
data-pane-hidden, so typing in a session tile then clicking the main tab
left activeTarget on the buried tile: use-keybinds preventDefaults the
keystroke, the buried composer ignores the request (or is filtered out),
and the main composer never sees it. Same class of bug after the inline
edit composer unmounts with activeTarget still 'edit'.

Heal 'active' against the visible data-composer-target stamp (the same
visibility policy as every other document-wide surface lookup), release
the claim on real unmounts (useComposerDraft + user-edit-composer), and
keep getActiveComposer honest so Esc / soft / / voice agree with the
keyboard path.

The unmount release is salvaged from #72625 (@briandevans); this PR adds
the keep-alive tab heal his unmount-only fix couldn't cover.

Co-authored-by: briandevans <252620095+briandevans@users.noreply.github.com>
This commit is contained in:
Brooklyn Nicholson 2026-07-29 00:19:38 -05:00
parent 402286fcff
commit f12e6526a6
5 changed files with 351 additions and 7 deletions

View file

@ -1,6 +1,13 @@
import { afterEach, describe, expect, it } from 'vitest'
import { blurComposerInput } from './focus'
import {
blurComposerInput,
getActiveComposer,
markActiveComposer,
onComposerFocusRequest,
releaseActiveComposer,
requestComposerFocus
} from './focus'
import { RICH_INPUT_SLOT } from './rich-editor'
/**
@ -21,8 +28,23 @@ function mountInput(hidden = false) {
return input
}
/** A chat surface stamp — the same `data-composer-target` ChatView hangs. */
function mountSurface(target: string, hidden = false) {
const layer = document.createElement('div')
layer.toggleAttribute('data-pane-hidden', hidden)
const surface = document.createElement('div')
surface.dataset.composerTarget = target
layer.append(surface)
document.body.append(layer)
return surface
}
afterEach(() => {
document.body.innerHTML = ''
// `activeTarget` is module-level — a case that leaves a stale claim behind
// would otherwise decide the next one.
markActiveComposer('main')
})
describe('blurComposerInput', () => {
@ -48,3 +70,149 @@ describe('blurComposerInput', () => {
expect(document.activeElement).toBe(outside)
})
})
/**
* `markActiveComposer` has four call sites and, unguarded, no counterpart: an
* unmounting or keep-alive-buried composer left `activeTarget` pointing at
* itself, so every `'active'`-routed request was delivered to a target with no
* on-screen subscriber. Type-to-focus preventDefaults the keystroke BEFORE the
* request, so a dead target swallows the character and focuses nothing.
*/
describe('releaseActiveComposer', () => {
it('falls back to the main composer when the claimant releases', () => {
const root = document.createElement('div')
root.dataset.slot = 'aui_edit-composer-root'
document.body.append(root)
markActiveComposer('edit')
expect(getActiveComposer()).toBe('edit')
root.remove()
releaseActiveComposer('edit')
expect(getActiveComposer()).toBe('main')
})
it('leaves the key with the live claimant when a stale composer releases late', () => {
markActiveComposer('edit')
markActiveComposer('tile:abc')
releaseActiveComposer('edit')
expect(getActiveComposer()).toBe('tile:abc')
})
it('prefers the visible chat surface over a hard main default', () => {
const root = document.createElement('div')
root.dataset.slot = 'aui_edit-composer-root'
document.body.append(root)
mountSurface('tile:visible')
markActiveComposer('edit')
root.remove()
releaseActiveComposer('edit')
expect(getActiveComposer()).toBe('tile:visible')
})
it('routes an active-target request to the main composer once the edit composer closes', async () => {
// Mirrors the per-composer filter in use-composer-draft / user-edit-composer:
// a composer ignores any request not addressed to its own target.
const mainComposerSaw: string[] = []
const off = onComposerFocusRequest(({ target }) => {
if (target === 'main') {
mainComposerSaw.push(target)
}
})
const root = document.createElement('div')
root.dataset.slot = 'aui_edit-composer-root'
document.body.append(root)
markActiveComposer('edit')
root.remove()
releaseActiveComposer('edit')
requestComposerFocus('active')
// `dispatch` defers to a macrotask so click/keydown handlers settle first.
await new Promise(resolve => window.setTimeout(resolve, 0))
off()
expect(mainComposerSaw).toEqual(['main'])
})
})
describe('resolveActive / keep-alive tab heal', () => {
it('heals type-to-focus onto the visible main tab when a tile is buried', async () => {
// Repro for the reported main-tab miss: user typed in a session tile, then
// clicked the main/workspace tab without focusing its input. The tile stays
// mounted under data-pane-hidden, so activeTarget still reads tile:… and
// every type-to-focus request is dropped by the visible main composer.
mountSurface('tile:buried', true)
mountSurface('main')
markActiveComposer('tile:buried')
expect(getActiveComposer()).toBe('main')
const mainSaw: string[] = []
const tileSaw: string[] = []
const off = onComposerFocusRequest(({ target }) => {
if (target === 'main') {
mainSaw.push(target)
}
if (target === 'tile:buried') {
tileSaw.push(target)
}
})
requestComposerFocus('active', { typeChar: 'h' })
await new Promise(resolve => window.setTimeout(resolve, 0))
off()
expect(mainSaw).toEqual(['main'])
expect(tileSaw).toEqual([])
// Cache stays honest so dict/insert/Esc path all agree thereafter.
expect(getActiveComposer()).toBe('main')
})
it('keeps a live tile claim while that tile is the visible surface', () => {
mountSurface('main', true)
mountSurface('tile:front')
markActiveComposer('tile:front')
expect(getActiveComposer()).toBe('tile:front')
})
it('heals an edit claim once the edit root is gone (no release site needed)', async () => {
mountSurface('main')
markActiveComposer('edit')
// No edit root in the document → claim is dead. getActiveComposer heals.
expect(getActiveComposer()).toBe('main')
const mainSaw: string[] = []
const off = onComposerFocusRequest(({ target }) => {
if (target === 'main') {
mainSaw.push(target)
}
})
requestComposerFocus('active', { typeChar: 'a' })
await new Promise(resolve => window.setTimeout(resolve, 0))
off()
expect(mainSaw).toEqual(['main'])
})
it('holds an edit claim while the edit composer root is mounted', () => {
const root = document.createElement('div')
root.dataset.slot = 'aui_edit-composer-root'
document.body.append(root)
mountSurface('main')
markActiveComposer('edit')
expect(getActiveComposer()).toBe('edit')
})
})

View file

@ -43,6 +43,21 @@ const INSERT_REFS_EVENT = 'hermes:composer-insert-refs'
const SUBMIT_EVENT = 'hermes:composer-submit'
const VOICE_TOGGLE_EVENT = 'hermes:composer-voice-toggle'
/** Inline edit composer root — mounted only while a user bubble is being edited. */
const EDIT_COMPOSER_ROOT = '[data-slot="aui_edit-composer-root"]'
/** Attribute-safe selector fragment. jsdom (vitest) does not ship `CSS.escape`. */
const cssEscape = (value: string): string => {
if (typeof CSS !== 'undefined' && typeof CSS.escape === 'function') {
return CSS.escape(value)
}
// Our targets are `'main'` / `'edit'` / `'tile:<id>'` — alphanumerics plus `:`
// and `-`. Escape anything outside that set so a weird id cannot break the
// attribute selector.
return value.replace(/[^a-zA-Z0-9_:-]/g, ch => `\\${ch}`)
}
interface SubmitDetail {
target: ComposerTarget
text: string
@ -50,7 +65,76 @@ interface SubmitDetail {
let activeTarget: ComposerTarget = 'main'
const resolve = (target: ComposerTarget | 'active') => (target === 'active' ? activeTarget : target)
/**
* The chat surface currently on screen (`data-composer-target` hung off each
* ChatView). Inactive tabs stay mounted with `data-pane-hidden`, so this uses
* the same visibility policy as every other document-wide surface lookup.
*/
const visibleChatTarget = (): ComposerTarget | null => {
if (typeof document === 'undefined') {
return null
}
const surface = queryVisible<HTMLElement>('[data-composer-target]')
const target = surface?.dataset.composerTarget
return target ? (target as ComposerTarget) : null
}
/** True when `target` still has a live, on-screen subscriber. */
const targetIsReachable = (target: ComposerTarget): boolean => {
if (typeof document === 'undefined') {
return true
}
// The edit composer is an in-thread overlay, not a chat surface — it never
// stamps `data-composer-target`. While its root is mounted it still owns the
// bus; once it tears down the claim is dead.
if (target === 'edit') {
return Boolean(document.querySelector(EDIT_COMPOSER_ROOT))
}
// Exact match on a VISIBLE surface. Background keep-alive tabs carry the same
// `data-composer-target` but sit under `data-pane-hidden`, so queryVisible
// filters them out.
if (queryVisible(`[data-composer-target="${cssEscape(target)}"]`)) {
return true
}
// A different chat surface is on screen → this claim is buried or gone.
// (A claim with zero stamped surfaces yet — first paint, pure-unit tests —
// keeps the marked key until the DOM contradicts it.)
if (queryVisible('[data-composer-target]')) {
return false
}
return true
}
/**
* The composer `'active'` should route to right now.
*
* The cached claim (`activeTarget`) wins while its surface is still on screen.
* Tab stacks keep inactive panes mounted, so focusing a tile then clicking the
* main tab leaves `activeTarget` pointing at a buried composer with no
* subscriber on the visible surface, every type-to-focus keystroke is
* preventDefault'd and dropped. Heal to the visible chat surface (or main)
* whenever the claim is off-screen or gone, and keep the cache honest so Esc /
* voice / soft `/` agree with the keyboard path.
*/
const resolveActive = (): ComposerTarget => {
if (targetIsReachable(activeTarget)) {
return activeTarget
}
const visible = visibleChatTarget() ?? 'main'
activeTarget = visible
return visible
}
const resolve = (target: ComposerTarget | 'active') => (target === 'active' ? resolveActive() : target)
const dispatch = <T>(name: string, detail: T) => {
if (typeof window === 'undefined') {
@ -82,9 +166,33 @@ export const markActiveComposer = (target: ComposerTarget) => {
activeTarget = target
}
/** Hand the routing key back when a composer unmounts, so `'active'` can never
* resolve to a composer that no longer has a subscriber such a request is
* dispatched and then dropped by every mounted composer's target filter, and
* nothing re-marks the active composer on its own.
*
* Guarded on identity: a composer unmounting AFTER another one claimed the key
* (closing a background tile, a deferred edit-close cleanup) must not steal it
* from the live claimant. Falls through to {@link resolveActive} when the
* caller's surface is buried rather than gone, so closing on a tab switch that
* already re-fronted another chat surfaces there immediately. */
export const releaseActiveComposer = (target: ComposerTarget) => {
if (activeTarget !== target) {
return
}
// Prefer the visible chat surface over a hard `'main'` default — releasing a
// closed tile while another tile is fronted should land there, not the
// (possibly buried) workspace tab.
activeTarget = visibleChatTarget() ?? 'main'
}
/** The composer that last held focus the target `'active'` resolves to.
* Used by broadcast listeners (voice, Esc-to-stop) to act on exactly one. */
export const getActiveComposer = (): ComposerTarget => activeTarget
* Used by broadcast listeners (voice, Esc-to-stop) to act on exactly one.
* Heals a stale claim the same way {@link requestComposerFocus} does, so Esc
* and type-to-focus never disagree after a tab switch left the bus pointing at
* a keep-alive-mounted background composer. */
export const getActiveComposer = (): ComposerTarget => resolveActive()
export const requestComposerFocus = (
target: ComposerTarget | 'active' = 'active',

View file

@ -5,6 +5,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest'
import { type ComposerAttachment, mainComposerScope, stashSessionDraft } from '@/store/composer'
import type { QueueEditState } from '../composer-utils'
import { type ComposerTarget, getActiveComposer, markActiveComposer } from '../focus'
import { type ComposerScope, ComposerScopeProvider, MAIN_COMPOSER_SCOPE } from '../scope'
import { useComposerDraft } from './use-composer-draft'
@ -128,3 +130,48 @@ describe('useComposerDraft — rehydrate diagnostic log stays redacted', () => {
})
})
})
describe('useComposerDraft — a closing composer hands the focus-bus key back', () => {
afterEach(() => {
cleanup()
mainComposerScope.clear()
markActiveComposer('main')
})
function renderScoped(target: ComposerTarget) {
const scope: ComposerScope = { ...MAIN_COMPOSER_SCOPE, target }
return render(
<ComposerScopeProvider value={scope}>
<ProbeHarness
activeQueueSessionKey="session-tile"
onLayoutSnapshot={() => undefined}
sessionId="session-tile"
/>
</ComposerScopeProvider>
)
}
it('stops `active` resolving to a session tile once the tile unmounts', () => {
const { unmount } = renderScoped('tile:abc')
// Mounting claims the bus for this tile — the leak precondition.
expect(getActiveComposer()).toBe('tile:abc')
unmount()
expect(getActiveComposer()).toBe('main')
})
it('leaves the key alone when another composer claimed it before this one unmounted', () => {
const { unmount } = renderScoped('tile:abc')
expect(getActiveComposer()).toBe('tile:abc')
// The user clicks into a second tile, which claims the bus.
markActiveComposer('tile:other')
unmount()
expect(getActiveComposer()).toBe('tile:other')
})
})

View file

@ -17,7 +17,8 @@ import {
markActiveComposer,
onComposerFocusRequest,
onComposerInsertRefsRequest,
onComposerInsertRequest
onComposerInsertRequest,
releaseActiveComposer
} from '../focus'
import { type InlineRefInput, insertInlineRefsIntoEditor } from '../inline-refs'
import { composerPlainText, placeCaretEnd, REF_RE, renderComposerContents } from '../rich-editor'
@ -153,6 +154,15 @@ export function useComposerDraft({
}
}, [focusInput, focusKey, focusRequestId, inputDisabled])
// The mirror of the `markActiveComposer` above: give the key back when this
// composer goes away (a session tile closing, a pane unmounting). Covers both
// claim sites for this composer — `focusInput` here and ChatBar's `onFocus` —
// since they mark the same scope target. Without it `'active'` keeps
// resolving to a dead tile and every routed focus/insert request is dropped.
// (Heal-to-visible in focus.ts covers the keep-alive-tab case where the pane
// stays mounted behind the front tab; this covers true unmounts.)
useEffect(() => () => releaseActiveComposer(target), [target])
useEffect(() => {
if (inputDisabled) {
return undefined

View file

@ -19,7 +19,8 @@ import {
focusComposerInput,
markActiveComposer,
onComposerFocusRequest,
onComposerInsertRequest
onComposerInsertRequest,
releaseActiveComposer
} from '@/app/chat/composer/focus'
import { useAtCompletions } from '@/app/chat/composer/hooks/use-at-completions'
import { useComposerUndo } from '@/app/chat/composer/hooks/use-composer-undo'
@ -111,7 +112,17 @@ export const UserEditComposer: FC<UserEditComposerProps> = ({ cwd, gateway, sess
const at = useAtCompletions({ cwd, gateway, sessionId })
const slash = useSlashCompletions({ gateway })
useEffect(() => () => notifyThreadEditClose(), [])
// This is the one composer that routinely unmounts, so it is where the focus
// bus leaks: confirming or cancelling an edit tears the composer down while
// `'edit'` is still the active target. Release it alongside the thread-scroll
// cleanup so keyboard routing falls back to the visible chat composer.
useEffect(
() => () => {
notifyThreadEditClose()
releaseActiveComposer('edit')
},
[]
)
const focusEditor = useCallback(() => {
const editor = editorRef.current