diff --git a/apps/desktop/src/app/chat/composer/focus.test.ts b/apps/desktop/src/app/chat/composer/focus.test.ts index f3987982274..bc92932bb8f 100644 --- a/apps/desktop/src/app/chat/composer/focus.test.ts +++ b/apps/desktop/src/app/chat/composer/focus.test.ts @@ -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') + }) +}) diff --git a/apps/desktop/src/app/chat/composer/focus.ts b/apps/desktop/src/app/chat/composer/focus.ts index 7120fde2b1a..3302d031078 100644 --- a/apps/desktop/src/app/chat/composer/focus.ts +++ b/apps/desktop/src/app/chat/composer/focus.ts @@ -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:'` — 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('[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 = (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', diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx b/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx index 81401482f69..7707c8ae753 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx @@ -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( + + undefined} + sessionId="session-tile" + /> + + ) + } + + 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') + }) +}) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.ts index 43b8514cc35..d981d6435d8 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.ts @@ -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 diff --git a/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx b/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx index bf8df4ea01e..8791ac3b75e 100644 --- a/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx @@ -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 = ({ 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