From ed33adad994e51f981e675a06aac679a4af27618 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 3 Sep 2026 14:03:19 -0700 Subject: [PATCH 1/5] docs(workspace): plan retaining terminals across Reader, Spotlight and Settings Refs #752 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Kk16MNVJtWAnCeGxGRuHqa --- ...2026-09-03-focus-modes-retain-terminals.md | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-03-focus-modes-retain-terminals.md diff --git a/docs/superpowers/plans/2026-09-03-focus-modes-retain-terminals.md b/docs/superpowers/plans/2026-09-03-focus-modes-retain-terminals.md new file mode 100644 index 000000000..aa252fe27 --- /dev/null +++ b/docs/superpowers/plans/2026-09-03-focus-modes-retain-terminals.md @@ -0,0 +1,56 @@ +# Workspace: keep terminals alive across Reader, Spotlight and Settings + +Fixes #752. Refs #745, #749, #103, #390. + +## Problem + +`MainSurface` renders Settings, `ReaderView` and `SpotlightView` *instead of* +the `GlobalEditorShell → TileTree` subtree. Entering any of them unmounts +every pane; with `agentViewMode = 'terminal'` that disposes every xterm and +detaches every PTY, and leaving re-mounts the tree from scratch: N × +(attach + up to 512 KiB replay + parse + fit/resize handshake) on the +renderer thread, a multi-second freeze, lost scrollback, and a burst of +`agent-pty-attach` / `resize` / `session:screen` IPC. The app already +solves this for Global Editor fullscreen by RETAINING the workspace under +`display:none` and telling the terminal ownership boundary that "mounted" +no longer means "visible" (`GlobalEditorWorkspaceSlot`, +`AgentTerminalOwnerVisibilityProvider`). + +## Design + +- `RetainedWorkspaceSurface` (app/shell) wraps the shell subtree: when a + takeover surface is active it sets `display:none` on a layout-neutral + wrapper (`display:contents` otherwise), provides + `AgentTerminalOwnerVisibilityProvider visible={false}` so hidden panes + neither claim dimension ownership nor measure, and provides a new + `WorkspaceSurfaceHiddenContext` for anything that must know the subtree + is retained-but-hidden. +- `MainSurface` always renders the retained workspace and renders the + takeover surface (Settings / Reader / Spotlight) alongside it. The + takeover surfaces keep bypassing `GlobalEditorShell` for the layout + reasons documented there; only the shell's mount lifetime changes. +- `TileLeaf` folds `WorkspaceSurfaceHiddenContext` into its Tail-All + visibility mask next to the editor-fullscreen term, so un-hiding is a + genuine false→true transition that re-pins (same reasoning as the + existing mask comment). +- `GlobalEditorShell`'s fullscreen Escape listener ignores keys while the + surface is hidden, so Escape in Reader Mode exits Reader (owned by + `useKeybinds`) instead of also collapsing a fullscreen editor underneath. + +Spotlight mounts its own leaf for the focused session; the hidden tile- +tree leaf keeps its PTY attachment (refcounted in main) and its xterm +keeps parsing while hidden. That parse cost is far below a replay and is +exactly what editor fullscreen already accepts. Hidden feeds keep +rendering; freezing them is a separate structural item. + +Not in scope: hoisting xterm instances out of React (the structural fix +tracked in #752's second half and the architecture audit). + +## Verification + +- `TerminalDimensionOwnership.renderer.test.tsx`: Settings/Reader/ + Spotlight keep the pane terminal MOUNTED (mount count stays 1 across + enter/exit) and hidden; dimension ownership still hands to the inline + debug terminal while hidden and returns afterwards. +- Existing Spotlight target-guard test adapted to the retained tree. +- `npx tsc -b`; renderer suites for app/shell, tile-tree, global-editor. From 0f37c7bc5c334fa978b908382f3a98579de7906e Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 3 Sep 2026 16:05:26 -0700 Subject: [PATCH 2/5] perf(workspace): retain the workspace under Reader, Spotlight and Settings instead of unmounting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MainSurface rendered the focus-takeover surfaces instead of the GlobalEditorShell subtree, so entering Reader Mode with terminal panes disposed every xterm and detached every PTY, and leaving re-attached and replayed up to 512 KiB into each brand-new terminal — a multi-second freeze and lost scrollback per toggle. Global Editor fullscreen already retains the workspace under display:none and tells the terminal ownership boundary that mounted no longer means visible; RetainedWorkspaceSurface applies the same pattern one level up for the surfaces that bypass the shell, with a layout-neutral display:contents wrapper when visible. TileLeaf folds the new hidden flag into its Tail-All mask so un-hiding is a genuine false→true transition that re-pins, and the shell's fullscreen Escape listener stands down while hidden so Escape in Reader Mode exits Reader rather than also collapsing a fullscreen editor underneath. Fixes #752 Refs #745, #749 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Kk16MNVJtWAnCeGxGRuHqa --- src/renderer/src/app/shell/MainSurface.tsx | 31 ++++++-- .../app/shell/RetainedWorkspaceSurface.tsx | 60 ++++++++++++++++ ...rminalDimensionOwnership.renderer.test.tsx | 72 ++++++++++++++++--- .../global-editor/ui/GlobalEditorShell.tsx | 11 ++- .../src/workspace/tile-tree/TileLeaf.tsx | 18 +++-- 5 files changed, 170 insertions(+), 22 deletions(-) create mode 100644 src/renderer/src/app/shell/RetainedWorkspaceSurface.tsx diff --git a/src/renderer/src/app/shell/MainSurface.tsx b/src/renderer/src/app/shell/MainSurface.tsx index d2fc735b0..c15ebbf75 100644 --- a/src/renderer/src/app/shell/MainSurface.tsx +++ b/src/renderer/src/app/shell/MainSurface.tsx @@ -9,6 +9,7 @@ import { DispatchLayout } from '@renderer/workspace/dispatch/DispatchLayout' import { TileTree } from '@renderer/workspace/tile-tree/TileTree' import { NewAgentPlacementOverlay } from '@renderer/features/workspace/ui/NewAgentPlacementOverlay' import { usePlacementOverlay } from '@renderer/features/workspace/surfaces/usePlacementOverlay' +import { RetainedWorkspaceSurface } from './RetainedWorkspaceSurface' import { WelcomeEmpty } from './WelcomeEmpty' // The main-area mode routing, extracted verbatim from App.tsx (#494). @@ -37,6 +38,16 @@ export function MainSurface({ onNewTabRequest }: { onNewTabRequest: () => void } ? state.tabs.some(tab => tab.id === spotlightTabId) : false + // Exactly one takeover surface at a time, in the priority order the old + // if/else ladder had. Settings wins over Reader wins over Spotlight. + const takeover = settingsPageOpen + ? 'settings' + : workspace.readerMode && readerModeTabExists + ? 'reader' + : workspace.spotlight && spotlightTabExists + ? 'spotlight' + : null + return ( <> {/* @@ -61,6 +72,15 @@ export function MainSurface({ onNewTabRequest }: { onNewTabRequest: () => void } the desired behaviour: focus modes are a temporary context, not a "close the editor" instruction. + WHY the workspace is RETAINED, not replaced, under a takeover (#752): + the takeover surfaces used to render instead of the shell, which + unmounted every pane — every xterm disposed, every PTY detached, + and on exit every terminal re-attached and replayed its whole + buffer. RetainedWorkspaceSurface keeps the shell mounted under + display:none while a takeover is active, the same way Global + Editor fullscreen retains it, so leaving Reader Mode is a style + change rather than a rebuild. + WHY TileTabs / Dispatch / TileTree stay inside the shell: These ARE the workspace. The point of the editor overlay is to be alongside them, so reading code does not require @@ -72,7 +92,7 @@ export function MainSurface({ onNewTabRequest }: { onNewTabRequest: () => void } positions — survives toggling between Dispatch / TileTree and Welcome. */} - {settingsPageOpen ? ( + {takeover === 'settings' ? ( void } onChange={setSettings} onReset={resetSettings} /> - ) : workspace.readerMode && readerModeTabExists ? ( + ) : takeover === 'reader' ? ( - ) : workspace.spotlight && spotlightTabExists ? ( + ) : takeover === 'spotlight' ? ( - ) : ( + ) : null} + ) } diff --git a/src/renderer/src/app/shell/RetainedWorkspaceSurface.tsx b/src/renderer/src/app/shell/RetainedWorkspaceSurface.tsx new file mode 100644 index 000000000..9339fae97 --- /dev/null +++ b/src/renderer/src/app/shell/RetainedWorkspaceSurface.tsx @@ -0,0 +1,60 @@ +import { createContext, useContext } from 'react' +import type { ReactNode } from 'react' + +import { AgentTerminalOwnerVisibilityProvider } from '@renderer/workspace/terminal/AgentTerminalOwnership' + +// The workspace subtree that survives focus-takeover surfaces (#752). +// +// WHY the workspace is hidden rather than unmounted while Settings, Reader +// Mode or Spotlight own the screen: every pane in the tile tree owns live +// state that is expensive or impossible to rebuild — an xterm instance with +// its PTY attachment and scrollback, a feed's scroll position, Monaco +// buffers. MainSurface used to render the takeover surface INSTEAD of the +// shell, so entering Reader Mode with terminal panes disposed every xterm +// and leaving re-attached every PTY and replayed up to 512 KiB into each +// brand-new terminal: a multi-second freeze and lost scrollback per toggle. +// Global Editor fullscreen had the same problem and solved it by retaining +// the subtree under display:none (GlobalEditorWorkspaceSlot); this is the +// same pattern one level up, for the surfaces that bypass the shell. +// +// WHY `display: contents` when visible: the wrapper must be layout-neutral so +// the shell's own flex sizing is unchanged; a hidden wrapper is `display: +// none`, which removes the subtree from layout (scrollHeight 0, no paint) but +// keeps every component mounted. +// +// WHY the terminal visibility provider sits here: "mounted" no longer means +// "can measure a viewport". MountedAgentTerminalOwner reads this to withhold +// the dimension claim while hidden, exactly as it does for editor fullscreen, +// so a hidden pane neither fights the debug inline terminal nor sends a +// zero-sized resize to the PTY. +// +// WHY a separate hidden context: some consumers need "is my subtree retained +// but off-screen" for reasons unrelated to terminals — TileLeaf's Tail-All +// mask must turn false while hidden so that un-hiding is a real false→true +// transition that re-pins the feed (see the mask comment in TileLeaf). +export const WorkspaceSurfaceHiddenContext = createContext(false) + +export function useWorkspaceSurfaceHidden(): boolean { + return useContext(WorkspaceSurfaceHiddenContext) +} + +export function RetainedWorkspaceSurface({ + hidden, + children, +}: { + hidden: boolean + children: ReactNode +}) { + return ( + + +
+ {children} +
+ + + ) +} diff --git a/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx b/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx index 11cbb6bd2..a51ba3724 100644 --- a/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx +++ b/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx @@ -10,6 +10,11 @@ import { MainSurface } from './MainSurface' const harness = vi.hoisted(() => ({ appState: {} as Record, workspace: {} as Record, + // Pane terminal lifecycle, keyed by session: a takeover must HIDE the pane, + // never unmount it (#752) — an unmount is a disposed xterm plus a full PTY + // replay on the way back. + paneMounts: {} as Record, + paneUnmounts: {} as Record, })) vi.mock('@renderer/app-state/hooks', () => ({ @@ -55,11 +60,20 @@ vi.mock('@renderer/features/workspace/ui/NewAgentPlacementOverlay', () => ({ NewAgentPlacementOverlay: () => null, })) -vi.mock('@renderer/workspace/tile-tree/AgentTerminalLeaf', () => ({ - AgentTerminalLeaf: ({ sessionId }: { sessionId: string }) => ( -
- ), -})) +vi.mock('@renderer/workspace/tile-tree/AgentTerminalLeaf', async () => { + const { useEffect } = await import('react') + return { + AgentTerminalLeaf: ({ sessionId }: { sessionId: string }) => { + useEffect(() => { + harness.paneMounts[sessionId] = (harness.paneMounts[sessionId] ?? 0) + 1 + return () => { + harness.paneUnmounts[sessionId] = (harness.paneUnmounts[sessionId] ?? 0) + 1 + } + }, [sessionId]) + return
+ }, + } +}) vi.mock('@renderer/features/debug/ui/AgentInlineTerminal', () => ({ AgentInlineTerminal: () =>
, @@ -72,6 +86,8 @@ vi.mock('@renderer/features/debug/devDebugConfig', () => ({ describe('terminal dimension ownership across main-surface takeovers', () => { beforeEach(() => { + harness.paneMounts = {} + harness.paneUnmounts = {} const runtime = emptyRuntime() const activeTab = { id: 'tab-1', @@ -127,7 +143,7 @@ describe('terminal dimension ownership across main-surface takeovers', () => { cleanup() }) - it('hands dimension ownership to the inline terminal only while Settings unmounts the pane terminal', async () => { + it('hands dimension ownership to the inline terminal while Settings hides the retained pane terminal', async () => { const tree = () => ( @@ -146,7 +162,11 @@ describe('terminal dimension ownership across main-surface takeovers', () => { view.rerender(tree()) expect(screen.getByTestId('settings-page')).toBeTruthy() - expect(screen.queryByTestId('pane-agent-terminal-session-1')).toBeNull() + // Retained, not unmounted: the pane is still in the tree, hidden with + // its whole workspace, and has released its dimension claim. + expect(screen.getByTestId('pane-agent-terminal-session-1')).toBeTruthy() + expect(screen.getByTestId('retained-workspace-surface').style.display).toBe('none') + expect(harness.paneUnmounts['session-1'] ?? 0).toBe(0) const openInline = await screen.findByTitle('Open inline raw PTY terminal') fireEvent.click(openInline) expect(screen.getByTestId('inline-agent-terminal')).toBeTruthy() @@ -155,12 +175,43 @@ describe('terminal dimension ownership across main-surface takeovers', () => { view.rerender(tree()) expect(screen.getByTestId('pane-agent-terminal-session-1')).toBeTruthy() + expect(screen.getByTestId('retained-workspace-surface').style.display).toBe('contents') + expect(harness.paneMounts['session-1']).toBe(1) + expect(harness.paneUnmounts['session-1'] ?? 0).toBe(0) await waitFor(() => { expect(screen.queryByTestId('inline-agent-terminal')).toBeNull() expect(screen.getByText('raw screen (last 20 lines)')).toBeTruthy() }) }) + it('keeps every pane terminal mounted across entering and leaving Reader Mode', () => { + const tree = () => ( + + + + ) + const view = render(tree()) + expect(screen.getByTestId('pane-agent-terminal-session-1')).toBeTruthy() + expect(screen.getByTestId('retained-workspace-surface').style.display).toBe('contents') + + harness.workspace = { + ...harness.workspace, + readerMode: { tabId: 'tab-1', focusedSessionId: 'session-1' }, + } + view.rerender(tree()) + expect(screen.getByTestId('reader-view')).toBeTruthy() + expect(screen.getByTestId('pane-agent-terminal-session-1')).toBeTruthy() + expect(screen.getByTestId('retained-workspace-surface').style.display).toBe('none') + + harness.workspace = { ...harness.workspace, readerMode: null } + view.rerender(tree()) + expect(screen.queryByTestId('reader-view')).toBeNull() + expect(screen.getByTestId('retained-workspace-surface').style.display).toBe('contents') + // One mount for the whole round trip: no disposed xterm, no PTY replay. + expect(harness.paneMounts['session-1']).toBe(1) + expect(harness.paneUnmounts['session-1'] ?? 0).toBe(0) + }) + it('guards the debug target by the terminal Spotlight actually mounted', async () => { const splitTab = { ...(harness.workspace.activeTab as Record), @@ -197,8 +248,11 @@ describe('terminal dimension ownership across main-surface takeovers', () => { , ) - expect(screen.getByTestId('pane-agent-terminal-session-2')).toBeTruthy() - expect(screen.queryByTestId('pane-agent-terminal-session-1')).toBeNull() + // Spotlight mounts its own leaf for session-2 on top of the retained + // (hidden) tile tree, which still holds both panes. + expect(screen.getAllByTestId('pane-agent-terminal-session-2')).toHaveLength(2) + expect(screen.getByTestId('pane-agent-terminal-session-1')).toBeTruthy() + expect(screen.getByTestId('retained-workspace-surface').style.display).toBe('none') // The command/debug target intentionally remains session-1 while // Spotlight renders session-2. Both are Terminal-configured, but they own // different PTYs, so suppressing session-1's inline recovery terminal diff --git a/src/renderer/src/features/global-editor/ui/GlobalEditorShell.tsx b/src/renderer/src/features/global-editor/ui/GlobalEditorShell.tsx index 246a773b2..0699dee3c 100644 --- a/src/renderer/src/features/global-editor/ui/GlobalEditorShell.tsx +++ b/src/renderer/src/features/global-editor/ui/GlobalEditorShell.tsx @@ -37,6 +37,7 @@ import { import { useFocusedAgentCwd } from '@renderer/features/global-editor/useFocusedAgentCwd' import { SplitHandle } from '@renderer/features/shared/SplitHandle' import { useResizableSplitter } from '@renderer/features/shared/useResizableSplitter' +import { useWorkspaceSurfaceHidden } from '@renderer/app/shell/RetainedWorkspaceSurface' // Splitter geometry. SPLITTER_PX is the visual width of the // draggable bar between the editor and the workspace. We avoid @@ -997,8 +998,14 @@ export function GlobalEditorShell({ children, workspace }: Props) { // Escape exits fullscreen — but only when no editor overlay owns the // key (Quick Open / content search close themselves on Escape and must // not ALSO drop the user out of fullscreen with the same press). + // WHY the retained-surface flag is part of the gate (#752): the workspace, + // this shell included, now stays mounted under display:none while Reader + // Mode / Spotlight / Settings own the screen. Escape there belongs to the + // takeover surface (useKeybinds exits Reader on Escape); a fullscreen + // editor hiding underneath must not ALSO collapse on the same press. + const surfaceHidden = useWorkspaceSurfaceHidden() useEffect(() => { - if (!open || !editorFullscreen) return + if (!open || !editorFullscreen || surfaceHidden) return const onKeyDown = (event: KeyboardEvent) => { if (event.key !== 'Escape') return if (event.defaultPrevented) return @@ -1009,7 +1016,7 @@ export function GlobalEditorShell({ children, workspace }: Props) { } window.addEventListener('keydown', onKeyDown) return () => window.removeEventListener('keydown', onKeyDown) - }, [open, editorFullscreen, setEditorFullscreen]) + }, [open, editorFullscreen, setEditorFullscreen, surfaceHidden]) // When open without a focused cwd (rare boot edge), still show // the split so the user sees the overlay engaged — but the diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx index d3b41f0ef..7ff69e358 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx @@ -7,6 +7,7 @@ import type { KeyboardEvent } from 'react' import { useAppStore } from '@renderer/app-state/hooks' import { useGlobalEditorStore } from '@renderer/features/global-editor/store' +import { useWorkspaceSurfaceHidden } from '@renderer/app/shell/RetainedWorkspaceSurface' import { useGlobalToast } from '@renderer/ui/GlobalToast' import { Feed } from '@renderer/features/feed/ui/Feed' import type { ScrollInfo } from '@renderer/features/feed/ui/Feed' @@ -141,6 +142,9 @@ export function TileLeaf({ // survives. Reading the flag here is what turns "mounted" back into // "visible" — see the mask below. const workspaceHiddenByEditor = useGlobalEditorStore(state => state.editorFullscreen) + // Same shape one level up (#752): Reader Mode, Spotlight and Settings now + // retain the whole workspace under display:none instead of unmounting it. + const workspaceHiddenBySurface = useWorkspaceSurfaceHidden() // This one OR is the ENTIRE implementation of "Tail All" scoping, and it is // load-bearing in a way that is easy to mistake for a shortcut. // @@ -186,11 +190,12 @@ export function TileLeaf({ // // The converse is not true either — a mounted TileLeaf does not always render // a Feed to tail. Two known cases: a pane showing a workflow run swaps Feed - // for WorkflowRunView below, and Reader Mode is a full takeover (MainSurface - // renders ReaderView *instead of* the workspace shell) so no TileLeaf exists - // at all there; ReaderView owns an independent stickToBottom. The remote - // client mounts Feed directly (remote-client/src/ui/SessionView.tsx) and - // deliberately passes no tail props, so Tail All is desktop-only. + // for WorkflowRunView below, and Reader Mode / Spotlight / Settings retain + // the workspace hidden (RetainedWorkspaceSurface, #752 — the second mask + // term below, same reasoning as the editor one); ReaderView owns an + // independent stickToBottom. The remote client mounts Feed directly + // (remote-client/src/ui/SessionView.tsx) and deliberately passes no tail + // props, so Tail All is desktop-only. // In all of these Tail All is inert, not wrong — but the palette still reports // "On", which is the honest cost of a workspace-level stance. // @@ -199,7 +204,8 @@ export function TileLeaf({ // Note that the *flag* restoring is not the same as the *scroll position* // restoring — see the tail-mode guard in Feed's scroll listener for why the // pre-tail position has to be protected for that promise to hold. - const effectiveTailMode = (runtime.tailMode || tailAllMode) && !workspaceHiddenByEditor + const effectiveTailMode = + (runtime.tailMode || tailAllMode) && !workspaceHiddenByEditor && !workspaceHiddenBySurface const dictationEnabled = useAppStore(state => state.settings.dictationEnabled) const dictationProvider = useAppStore(state => state.settings.dictationProvider) const dictationShortcut = useAppStore(state => state.settings.dictationShortcut) From a2ea689e09a1f4c49717337321677a7adb4de5de Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 3 Sep 2026 16:48:05 -0700 Subject: [PATCH 3/5] fix(workspace): make hidden panes give up input, size and focus ownership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #757 found the retention design shadowed in the production tree: the Global Editor slot nests inside the retained surface and provided its own terminal-visibility value, so React's nearest-provider rule hid the "Reader/Spotlight/Settings took the screen" signal from every pane whenever the editor was not fullscreen. Retained panes kept their dimension claim, measured a display:none box, left the PTY at Spotlight's size on exit and kept the inline debug terminal disabled. Visibility now composes: a subtree is visible only if every enclosing shell says so. The retained focused TileLeaf also kept every document-level input router — type-to-focus, paste-to-focus, the bare-Enter submit target, the dictation hotkey, the condition outlet — so typing "yes" + Enter in Reader Mode submitted a prompt through an invisible composer. Input ownership is now focused AND visible, and the caret/xterm refocus effects re-run on reveal, which the remount used to do for free. Shell panes (TerminalLeaf) had no ownership hook at all and would have kept Spotlight's PTY size after exit; they now reset their sent size when hidden and refit on reveal. Feeds skip the sticky-bottom pin while hidden and re-pin on reveal. Comments that described the takeovers as unmounting the workspace are corrected. Refs #752, #760 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Kk16MNVJtWAnCeGxGRuHqa --- ...RetainedWorkspaceSurface.renderer.test.tsx | 55 +++++++++++++++++++ .../debug/surfaces/DebugSurfacesImpl.tsx | 7 ++- src/renderer/src/features/feed/ui/Feed.tsx | 9 ++- .../src/features/reader/ui/ReaderView.tsx | 5 +- .../terminal/AgentTerminalOwnership.tsx | 19 ++++++- ...lLeaf.dimensionOwnership.renderer.test.tsx | 48 ++++++++++++++++ .../workspace/tile-tree/AgentTerminalLeaf.tsx | 18 +++--- .../src/workspace/tile-tree/TerminalLeaf.tsx | 43 ++++++++++++++- .../src/workspace/tile-tree/TileLeaf.tsx | 46 ++++++++++------ 9 files changed, 217 insertions(+), 33 deletions(-) create mode 100644 src/renderer/src/app/shell/RetainedWorkspaceSurface.renderer.test.tsx diff --git a/src/renderer/src/app/shell/RetainedWorkspaceSurface.renderer.test.tsx b/src/renderer/src/app/shell/RetainedWorkspaceSurface.renderer.test.tsx new file mode 100644 index 000000000..132086337 --- /dev/null +++ b/src/renderer/src/app/shell/RetainedWorkspaceSurface.renderer.test.tsx @@ -0,0 +1,55 @@ +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it } from 'vitest' + +import { GlobalEditorWorkspaceSlot } from '@renderer/features/global-editor/ui/GlobalEditorWorkspaceSlot' +import { + AgentTerminalOwnershipProvider, + MountedAgentTerminalOwner, + useAgentTerminalDimensionActive, + useHasAgentTerminalDimensionClaim, +} from '@renderer/workspace/terminal/AgentTerminalOwnership' + +import { RetainedWorkspaceSurface } from './RetainedWorkspaceSurface' + +// #752 review: the editor slot nests INSIDE the retained surface and provides +// its own visibility value. If the two did not compose, a hidden workspace +// would keep every pane's dimension claim and the whole retention design +// would be dead in the production tree. This renders the real slot. + +afterEach(() => cleanup()) + +function Probe() { + const claimed = useHasAgentTerminalDimensionClaim('s1') + const active = useAgentTerminalDimensionActive() + return
+} + +function tree(hidden: boolean) { + return ( + + + + ) +} + +describe('RetainedWorkspaceSurface visibility composition', () => { + it('releases the dimension claim through the editor slot while hidden and restores it on reveal', () => { + const view = render(tree(true)) + expect(screen.getByTestId('probe').dataset.claimed).toBe('false') + expect(screen.getByTestId('probe').dataset.active).toBe('false') + + view.rerender(tree(false)) + expect(screen.getByTestId('probe').dataset.claimed).toBe('true') + }) + + it('keeps the claim while visible even though the slot provides its own value', () => { + render(tree(false)) + expect(screen.getByTestId('probe').dataset.claimed).toBe('true') + }) +}) diff --git a/src/renderer/src/features/debug/surfaces/DebugSurfacesImpl.tsx b/src/renderer/src/features/debug/surfaces/DebugSurfacesImpl.tsx index cb03efd4c..81ba31b40 100644 --- a/src/renderer/src/features/debug/surfaces/DebugSurfacesImpl.tsx +++ b/src/renderer/src/features/debug/surfaces/DebugSurfacesImpl.tsx @@ -85,9 +85,10 @@ export function DebugSurfacesImpl() { sessionId={targetId} runtime={runtime} kind={kind} - // WHY policy is not enough here: Settings and Reader unmount the - // workspace even though the target remains configured for Terminal, - // while Spotlight can mount a different session. The interactive + // WHY policy is not enough here: Settings and Reader hide the + // retained workspace (#752) so its pane releases its dimension claim + // even though the target remains configured for Terminal, while + // Spotlight can mount a different session. The interactive // debug xterm conflicts only with a real, mounted AgentTerminalLeaf // that can currently resize this exact PTY. inlineRawTerminalDisabled={paneTerminalClaimsDimensions} diff --git a/src/renderer/src/features/feed/ui/Feed.tsx b/src/renderer/src/features/feed/ui/Feed.tsx index ea280aefa..12524aa1d 100644 --- a/src/renderer/src/features/feed/ui/Feed.tsx +++ b/src/renderer/src/features/feed/ui/Feed.tsx @@ -1,4 +1,5 @@ import { TaskNotificationsContext } from '@renderer/features/feed/context' +import { useAgentTerminalOwnerVisible } from '@renderer/workspace/terminal/AgentTerminalOwnership' import { RenderShapeCaptureProvider } from '@renderer/features/feed/evidence/RenderShapeCaptureContext' import { getRendererProviderCapabilities } from '@providers/registry.renderer.capabilities' import { @@ -570,6 +571,7 @@ function FeedImpl({ const semanticHistorySignal = semanticHistory .map(semanticTurnScrollSignal) .join('|') + const workspaceVisible = useAgentTerminalOwnerVisible() useEffect(() => { // During a bulk bootstrap burst we skip per-append auto-scroll. // The pin-once-on-transition effect below lands us at the bottom @@ -578,13 +580,18 @@ function FeedImpl({ // the LazyEntry observer cascade. See docs/superpowers/plans/ // 2026-04-15-bootstrap-replay-perf.md. if (bootstrapping) return + // While the workspace is retained under display:none (editor fullscreen, + // Reader/Spotlight/Settings — #752) scrollHeight is 0 and a pin would be + // a no-op; the flag in the deps makes the reveal re-run this effect, so + // a sticky-bottom feed that grew while hidden comes back at the bottom. + if (!workspaceVisible) return if (!tailMode && !stickyBottomRef.current) return // scrollTop = scrollHeight pins to bottom without the smooth-scroll // overshoot scrollIntoView sometimes produces. Direct, instant, // no animation frames. const el = scrollerRef.current if (el) el.scrollTop = el.scrollHeight - }, [entries.length, tailMode, semanticTurnSignal, semanticHistorySignal, bootstrapping]) + }, [entries.length, tailMode, semanticTurnSignal, semanticHistorySignal, bootstrapping, workspaceVisible]) // Pin-once on the bootstrap → live transition. Runs exactly once per // transition thanks to the previous-value ref: we read the prior diff --git a/src/renderer/src/features/reader/ui/ReaderView.tsx b/src/renderer/src/features/reader/ui/ReaderView.tsx index 4a7bb9e09..17c69fb64 100644 --- a/src/renderer/src/features/reader/ui/ReaderView.tsx +++ b/src/renderer/src/features/reader/ui/ReaderView.tsx @@ -384,8 +384,9 @@ function ReaderBody({
)} {/* Pane toast, rendered here as well as in TileLeaf. - WHY: Reader Mode is a full takeover — no TileLeaf is mounted, so - the toast TileLeaf normally paints never reaches the screen. That + WHY: Reader Mode takes over the screen — the workspace and its + TileLeaf stay mounted but hidden (display:none, #752), so the + toast TileLeaf normally paints never reaches the screen. That left "Reply to Selection" with no feedback whatsoever in Reader Mode: the draft is written to a composer the reader does not show, the palette closes, and nothing visibly happens. Any diff --git a/src/renderer/src/workspace/terminal/AgentTerminalOwnership.tsx b/src/renderer/src/workspace/terminal/AgentTerminalOwnership.tsx index dbc665c0a..99322ca85 100644 --- a/src/renderer/src/workspace/terminal/AgentTerminalOwnership.tsx +++ b/src/renderer/src/workspace/terminal/AgentTerminalOwnership.tsx @@ -33,13 +33,30 @@ export function AgentTerminalOwnerVisibilityProvider({ visible: boolean children: ReactNode }) { + // WHY visibility COMPOSES with the enclosing provider instead of replacing + // it (#752 review): the Global Editor slot provides its own value for the + // editor-fullscreen case and sits INSIDE the retained workspace surface. + // React resolves the nearest provider, so a plain override made the + // outer "hidden by Reader/Spotlight/Settings" signal invisible to every + // pane whenever the editor was not fullscreen — the retained pane kept its + // dimension claim, kept measuring a display:none box, and the inline + // debug terminal stayed disabled. A subtree is visible only if every + // enclosing shell says so. + const parentVisible = useContext(AgentTerminalOwnerVisibilityContext) return ( - + {children} ) } +/** True when no enclosing shell has hidden this subtree (editor fullscreen + * or a Reader/Spotlight/Settings takeover). Panes use it to decide whether + * "focused" may mean "owns keyboard input" and whether a fit is real. */ +export function useAgentTerminalOwnerVisible(): boolean { + return useContext(AgentTerminalOwnerVisibilityContext) +} + export function AgentTerminalOwnershipProvider({ children }: { children: ReactNode }) { const ownerCountsRef = useRef(new Map()) const [claimedSessionIds, setClaimedSessionIds] = useState>(() => new Set()) diff --git a/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.dimensionOwnership.renderer.test.tsx b/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.dimensionOwnership.renderer.test.tsx index f5271e2f5..3b1b610f9 100644 --- a/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.dimensionOwnership.renderer.test.tsx +++ b/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.dimensionOwnership.renderer.test.tsx @@ -1,6 +1,7 @@ import { act, cleanup, render } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { RetainedWorkspaceSurface } from '@renderer/app/shell/RetainedWorkspaceSurface' import { GlobalEditorWorkspaceSlot } from '@renderer/features/global-editor/ui/GlobalEditorWorkspaceSlot' import { emptyRuntime } from '@renderer/session-runtime/state' import { @@ -224,4 +225,51 @@ describe('AgentTerminalLeaf dimension ownership', () => { expect(api.sendInput).toHaveBeenCalledWith('session-1', 'x') expect(workspace.acknowledgeSession).toHaveBeenCalledWith('session-1') }) + + it('sends exactly one resize when a retained pane is revealed after a takeover', async () => { + // #752: Reader/Spotlight/Settings hide the workspace instead of + // unmounting it. While hidden the pane must not measure a display:none + // box; on reveal it must remeasure and resize the PTY even if its size + // equals what it sent before (Spotlight's copy may have changed the PTY). + const runtime = { + ...emptyRuntime(), + processStatus: 'started' as const, + } + const tree = (hidden: boolean) => ( + + + + ) + const view = render(tree(false)) + await act(async () => { + attach.resolve('') + await attach.promise + }) + act(() => flushAnimationFrames()) + expect(resize).toHaveBeenCalledTimes(1) + expect(resize).toHaveBeenLastCalledWith('session-1', 120, 40) + + view.rerender(tree(true)) + act(() => flushAnimationFrames()) + expect(resize).toHaveBeenCalledTimes(1) + + view.rerender(tree(false)) + act(() => flushAnimationFrames()) + expect(resize).toHaveBeenCalledTimes(2) + expect(resize).toHaveBeenLastCalledWith('session-1', 120, 40) + }) }) diff --git a/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx b/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx index 9e79ca4c2..def40d340 100644 --- a/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx @@ -93,7 +93,9 @@ export function AgentTerminalLeaf({ useComposerDictation({ enabled: dictationEnabled, - focused, + // A retained, hidden pane (Reader/Spotlight/Settings, #752) is still the + // workspace-focused one; it must not be the dictation target. + focused: focused && dimensionActive, provider: dictationProvider, shortcut: dictationShortcut, sink: { kind: 'terminal', sessionId }, @@ -277,10 +279,10 @@ export function AgentTerminalLeaf({ // callbacks current without making them part of xterm's mount contract. // // WHY the wake is conditional (#596): this effect runs on every MOUNT, - // and this component is unmounted and remounted constantly — entering - // and leaving Spotlight/Reader/Settings (which render outside - // GlobalEditorShell, so the whole tile tree goes away), and on every tab - // switch. Waking unconditionally meant each of those round-tripped + // and this component is still remounted on every tab switch and layout + // change (Reader/Spotlight/Settings stopped unmounting it in #752, but + // Spotlight mounts a second leaf). Waking unconditionally meant each of + // those round-tripped // through recoverSession for an agent that was already running, flapped // its runtime status spawning→started, and — until the companion fix in // ensureSessionLive — armed a 30-second timer that KILLED the live @@ -412,9 +414,11 @@ export function AgentTerminalLeaf({ } }, [sessionId]) + // Also re-runs when the pane is revealed after a takeover (#752): the + // remount used to refocus xterm for free; retained panes need it here. useEffect(() => { - if (focused) termRef.current?.focus() - }, [focused]) + if (focused && dimensionActive) termRef.current?.focus() + }, [focused, dimensionActive]) const focusTerminal = () => { termRef.current?.focus() diff --git a/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx b/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx index f464f4ee5..2cb90f92e 100644 --- a/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx @@ -1,4 +1,5 @@ import { useEffect, useRef } from 'react' +import { useAgentTerminalOwnerVisible } from '@renderer/workspace/terminal/AgentTerminalOwnership' import { Terminal } from '@xterm/xterm' import { FitAddon } from '@xterm/addon-fit' @@ -106,6 +107,20 @@ export function TerminalLeaf({ const termRef = useRef(null) const focusedRef = useRef(focused) focusedRef.current = focused + // WHY a shell pane tracks visibility itself (#752 review): agent panes get + // their "may I measure / who owns the PTY size" answer from + // MountedAgentTerminalOwner, which TerminalLeaf never had — a shell has no + // inline debug twin to arbitrate with. But retention under display:none + // gives it the same stale-size problem: Spotlight's copy resizes the PTY + // to full width, and on reveal the retained leaf's fit equals its old + // cols/rows, so the de-dupe swallows the resize the shell now needs. + const ownerVisible = useAgentTerminalOwnerVisible() + const ownerVisibleRef = useRef(ownerVisible) + ownerVisibleRef.current = ownerVisible + const onVisibilityChangeRef = useRef<((visible: boolean) => void) | null>(null) + useEffect(() => { + onVisibilityChangeRef.current?.(ownerVisible) + }, [ownerVisible]) // FitAddon instance — held for resize callbacks. const fitRef = useRef(null) @@ -151,6 +166,10 @@ export function TerminalLeaf({ const fitAndNotifyResize = () => { resizeFrame = null if (!term || !fit) return + // A display:none box measures as nothing; FitAddon's NaN guard would + // catch it, but never even asking keeps the guard from being the only + // thing between a hidden pane and a 2×1 resize on a live shell. + if (!ownerVisibleRef.current) return try { fit.fit() const { cols, rows } = term @@ -187,6 +206,23 @@ export function TerminalLeaf({ resizeFrame = requestAnimationFrame(fitAndNotifyResize) } + onVisibilityChangeRef.current = visible => { + if (!visible) { + // Mirror AgentTerminalLeaf's ownership loss: whatever size we last + // sent no longer describes the PTY once another leaf (Spotlight's) + // may have resized it, and a queued measurement must not replay. + pendingResize = null + lastCols = 0 + lastRows = 0 + if (resizeFrame !== null) { + cancelAnimationFrame(resizeFrame) + resizeFrame = null + } + return + } + scheduleFitAndNotifyResize() + } + try { // WHY theme xterm explicitly: xterm renders its own canvas-ish DOM tree // and does not inherit the surrounding Tailwind tokens. Leaving the @@ -340,7 +376,7 @@ export function TerminalLeaf({ // focus might have drifted to somewhere else — this // re-focus closes the gap if the pane is still the // workspace-focused one. - if (focusedRef.current) liveTerm.focus() + if (focusedRef.current && ownerVisibleRef.current) liveTerm.focus() }) .catch(err => { showPaneToastRef.current( @@ -402,6 +438,7 @@ export function TerminalLeaf({ } term?.dispose() termRef.current = null + onVisibilityChangeRef.current = null fitRef.current = null } // sessionId is the identity of the session we're attached to — @@ -422,8 +459,8 @@ export function TerminalLeaf({ // synchronous `focusTerminal()` handler below, wired into the // outer div's onMouseDown. useEffect(() => { - if (focused) termRef.current?.focus() - }, [focused]) + if (focused && ownerVisible) termRef.current?.focus() + }, [focused, ownerVisible]) // Imperative re-focus. Called from outer-div mousedown so that // ANY click inside the terminal pane re-focuses xterm's helper diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx index 7ff69e358..851f228e2 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx @@ -145,6 +145,16 @@ export function TileLeaf({ // Same shape one level up (#752): Reader Mode, Spotlight and Settings now // retain the whole workspace under display:none instead of unmounting it. const workspaceHiddenBySurface = useWorkspaceSurfaceHidden() + // WHY "focused" is not enough for input ownership (#752 review): the + // retained tree keeps this leaf mounted AND focused (activeTab's focused + // session) while Reader/Spotlight/Settings own the screen. Every document- + // level router below — type-to-focus, paste-to-focus, the bare-Enter + // submit target, the dictation hotkey, the condition outlet — used to be + // unreachable because the leaf did not exist; now it must be told that a + // hidden focused pane owns nothing. Typing "yes" + Enter in Reader Mode + // must not submit a prompt through an invisible composer. + const workspaceHidden = workspaceHiddenByEditor || workspaceHiddenBySurface + const interactive = focused && !workspaceHidden // This one OR is the ENTIRE implementation of "Tail All" scoping, and it is // load-bearing in a way that is easy to mistake for a shortcut. // @@ -204,8 +214,7 @@ export function TileLeaf({ // Note that the *flag* restoring is not the same as the *scroll position* // restoring — see the tail-mode guard in Feed's scroll listener for why the // pre-tail position has to be protected for that promise to hold. - const effectiveTailMode = - (runtime.tailMode || tailAllMode) && !workspaceHiddenByEditor && !workspaceHiddenBySurface + const effectiveTailMode = (runtime.tailMode || tailAllMode) && !workspaceHidden const dictationEnabled = useAppStore(state => state.settings.dictationEnabled) const dictationProvider = useAppStore(state => state.settings.dictationProvider) const dictationShortcut = useAppStore(state => state.settings.dictationShortcut) @@ -283,17 +292,18 @@ export function TileLeaf({ endHistoryCycle, } = usePromptHistory({ entries: runtime.entries, sessionKind }) - // When focus flips to this pane, move the DOM caret into its input. + // When focus flips to this pane — or the pane is revealed again after a + // takeover while still focused — move the DOM caret into its input. useEffect(() => { - if (focused) inputRef.current?.focus() - }, [focused]) + if (interactive) inputRef.current?.focus() + }, [interactive]) // Type-to-focus — document-level key listener that routes printable // keys into the composer when the pane is focused but DOM focus // drifted elsewhere. Hook in ./TileLeaf/useTypeToFocus.ts owns // the full filter/injection logic. useTypeToFocus({ - focused, + focused: interactive, sessionId, inputRef, setDraftInput, @@ -543,7 +553,7 @@ export function TileLeaf({ // second renderer. A selected WorkflowRunView replaces Feed entirely; // treating a hidden ledger candidate as painted would manufacture the very // visibility proof this observation is supposed to test. - if (feedIsMounted && !workspaceHiddenByEditor) { + if (feedIsMounted && !workspaceHidden) { for (const item of ledgerFeedPlan.items) { if (item.type !== 'entry') continue const submissionId = optimisticEntrySubmissionId(item.entry) @@ -568,7 +578,7 @@ export function TileLeaf({ // WorkflowRunView owns the central cell. Recording from the exact array // handed to QueueStrip closes that observational blind spot without // pretending the queue has a provider or rollout identity. - if (!workspaceHiddenByEditor) { + if (!workspaceHidden) { for (const message of runtime.queuedMessages) { const submissionId = queuedMessageSubmissionId(message) if (!submissionId) continue @@ -604,7 +614,7 @@ export function TileLeaf({ sessionId, feedIsMounted, visibleSubmitSurfaceOwner, - workspaceHiddenByEditor, + workspaceHidden, ]) // Claude image-paste flow — three clipboard ingress paths, media- @@ -624,7 +634,7 @@ export function TileLeaf({ // ./TileLeaf/usePasteToFocus.ts. Declared here (not next to // useTypeToFocus) because it depends on `handlePaste`. usePasteToFocus({ - focused, + focused: interactive, sessionId, inputRef, setDraftInput, @@ -656,7 +666,7 @@ export function TileLeaf({ const dictation = useComposerDictation({ enabled: dictationEnabled, - focused, + focused: interactive, provider: dictationProvider, shortcut: dictationShortcut, sink: { @@ -675,7 +685,7 @@ export function TileLeaf({ useEffect(() => { return registerComposerEnterTarget({ - focused, + focused: interactive, hovered: composerHovered, hasSubmittableDraft: () => { // Slash mode is PTY-owned: Enter commits Claude Code's highlighted @@ -692,7 +702,7 @@ export function TileLeaf({ void submitCurrentDraft('global-enter') }, }) - }, [focused, composerHovered, input, runtime.draftImages.length, slashMode, submitCurrentDraft]) + }, [interactive, composerHovered, input, runtime.draftImages.length, slashMode, submitCurrentDraft]) // Auto-send a clicked prompt suggestion. onApplySuggestion prefills the draft // and stashes the text in autoSendPendingRef; this effect waits until the @@ -777,8 +787,12 @@ export function TileLeaf({ // needing a ref forwarded out of this component. The existing // debug panels are stateless about the DOM and read from runtime // props instead, so a data attribute keeps that boundary intact. - // Session UUIDs are unique across the app, so there's no collision - // risk with multiple panes mounted simultaneously. + // Session UUIDs are unique across the app, so ordinarily one element per + // session. The exception (#752): while Spotlight shows a session, the + // retained (display:none) tile tree still holds that session's leaf, so + // two elements share the id and `querySelector` returns whichever comes + // first in DOM order — the Spotlight one, because MainSurface renders the + // takeover surface BEFORE the retained workspace. Keep that order.
feed.resolveCondition(sessionId, action)} - interactionActive={focused} + interactionActive={interactive} /> From 07521e00da9ab2f3851ebacd8369133cd5075885 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 3 Sep 2026 17:07:59 -0700 Subject: [PATCH 4/5] fix(workspace): scope pane interactivity to the composed visibility context and keep Monaco's focus on reveal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-review of #757: deriving "hidden" from the editor store's global editorFullscreen flag made Spotlight's own visible leaf — rendered beside the shell — refuse Enter/y/n on an approval strip whenever the editor was fullscreen underneath. useInteractiveOwnership now derives interactive and hidden from the composed visibility context, which is scoped to the subtree, and TileLeaf's store read is gone. Reveal refocus (caret, xterm) now only takes an unowned focus: a takeover surface unmounting leaves focus on , which the pane reclaims as the remount used to, but editor-fullscreen exit leaves it in Monaco, which must keep it. Hidden panes also unregister as dictation fallback targets (enabled, not just focused). TerminalLeaf gains the same retention round- trip test as the agent leaf, and the ownership derivation is pinned across the four shell combinations. Refs #752, #760 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Kk16MNVJtWAnCeGxGRuHqa --- ...2026-09-03-focus-modes-retain-terminals.md | 32 +++- .../workspace/tile-tree/AgentTerminalLeaf.tsx | 19 ++- .../TerminalLeaf.retention.renderer.test.tsx | 156 ++++++++++++++++++ .../src/workspace/tile-tree/TerminalLeaf.tsx | 9 +- .../src/workspace/tile-tree/TileLeaf.tsx | 48 +++--- .../useInteractiveOwnership.renderer.test.tsx | 52 ++++++ .../TileLeaf/useInteractiveOwnership.ts | 37 +++++ 7 files changed, 321 insertions(+), 32 deletions(-) create mode 100644 src/renderer/src/workspace/tile-tree/TerminalLeaf.retention.renderer.test.tsx create mode 100644 src/renderer/src/workspace/tile-tree/TileLeaf/useInteractiveOwnership.renderer.test.tsx create mode 100644 src/renderer/src/workspace/tile-tree/TileLeaf/useInteractiveOwnership.ts diff --git a/docs/superpowers/plans/2026-09-03-focus-modes-retain-terminals.md b/docs/superpowers/plans/2026-09-03-focus-modes-retain-terminals.md index aa252fe27..482827257 100644 --- a/docs/superpowers/plans/2026-09-03-focus-modes-retain-terminals.md +++ b/docs/superpowers/plans/2026-09-03-focus-modes-retain-terminals.md @@ -37,6 +37,28 @@ no longer means "visible" (`GlobalEditorWorkspaceSlot`, surface is hidden, so Escape in Reader Mode exits Reader (owned by `useKeybinds`) instead of also collapsing a fullscreen editor underneath. +- **Visibility composes across shells** (review). `AgentTerminalOwner- + VisibilityProvider` ANDs with its enclosing provider: the editor slot + nests inside the retained surface and provides its own value, and React's + nearest-provider rule would otherwise hide the takeover signal from every + pane whenever the editor is not fullscreen — retained panes kept their + dimension claim, measured a display:none box, left the PTY at Spotlight's + size on exit and kept the inline debug terminal disabled. +- **Hidden panes own nothing** (review). `useInteractiveOwnership(focused)` + derives `interactive = focused && visible` and `hidden` from that composed + context — never from the editor store's global flag, which made + Spotlight's own visible leaf refuse Enter/y/n under a fullscreen editor. + TileLeaf feeds it to type-to-focus, paste-to-focus, the bare-Enter submit + target, the dictation hotkey (`enabled` too, so a hidden pane is not even + a fallback target) and the condition outlet; AgentTerminalLeaf gates + dictation and xterm focus the same way; TerminalLeaf (no ownership hook of + its own) resets its sent size when hidden and refits on reveal. +- **Reveal refocus only takes an unowned focus** (`focusIsUnowned`): a + takeover unmounting leaves focus on , which the pane reclaims as the + remount used to; editor-fullscreen exit leaves it in Monaco, which must + keep it. +- Feeds skip the sticky-bottom pin while hidden and re-pin on reveal. + Spotlight mounts its own leaf for the focused session; the hidden tile- tree leaf keeps its PTY attachment (refcounted in main) and its xterm keeps parsing while hidden. That parse cost is far below a replay and is @@ -52,5 +74,13 @@ tracked in #752's second half and the architecture audit). Spotlight keep the pane terminal MOUNTED (mount count stays 1 across enter/exit) and hidden; dimension ownership still hands to the inline debug terminal while hidden and returns afterwards. +- `RetainedWorkspaceSurface.renderer.test.tsx`: the real editor slot inside + a hidden surface releases the dimension claim and restores it on reveal. +- `useInteractiveOwnership.renderer.test.tsx`: no shell → interactive; + hidden surface, fullscreen slot, visible slot inside a hidden surface → + not; both visible → interactive, unfocused → not. +- `AgentTerminalLeaf.dimensionOwnership` and `TerminalLeaf.retention` + round trips: one resize on attach, none while hidden, one on reveal. - Existing Spotlight target-guard test adapted to the retained tree. -- `npx tsc -b`; renderer suites for app/shell, tile-tree, global-editor. +- `npx tsc -b`; renderer suites for app/shell, tile-tree, global-editor, + reader, spotlight, feed. diff --git a/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx b/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx index def40d340..3418f2753 100644 --- a/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx @@ -1,4 +1,5 @@ import { useEffect, useLayoutEffect, useRef } from 'react' +import { focusIsUnowned } from '@renderer/workspace/tile-tree/TileLeaf/useInteractiveOwnership' import { Terminal } from '@xterm/xterm' import { FitAddon } from '@xterm/addon-fit' @@ -15,7 +16,7 @@ import { isSessionExited } from '@renderer/workspace/providerSessionIdentity' import { shortenCwd } from '@renderer/workspace/tile-tree/TileLeaf/labels' import { PaneToast } from '@renderer/workspace/tile-tree/TileLeaf/PaneToast' import { useComposerDictation } from '@renderer/workspace/tile-tree/TileLeaf/useComposerDictation' -import { useAgentTerminalDimensionActive } from '@renderer/workspace/terminal/AgentTerminalOwnership' +import { useAgentTerminalDimensionActive , useAgentTerminalOwnerVisible } from '@renderer/workspace/terminal/AgentTerminalOwnership' import { AgentTitleHeader } from '@renderer/workspace/tile-tree/AgentTitleHeader' import { createTerminalInputForwarder } from '@renderer/workspace/tile-tree/terminalInputForwarder' @@ -75,6 +76,7 @@ export function AgentTerminalLeaf({ const focusedRef = useRef(focused) focusedRef.current = focused const dimensionActive = useAgentTerminalDimensionActive() + const ownerVisible = useAgentTerminalOwnerVisible() const dimensionActiveRef = useRef(false) const dimensionOwnershipEpochRef = useRef(0) const onDimensionOwnershipChangeRef = useRef<((active: boolean) => void) | null>(null) @@ -92,9 +94,10 @@ export function AgentTerminalLeaf({ }, [dimensionActive]) useComposerDictation({ - enabled: dictationEnabled, // A retained, hidden pane (Reader/Spotlight/Settings, #752) is still the - // workspace-focused one; it must not be the dictation target. + // workspace-focused one; it must be neither the dictation target nor a + // fallback (`enabled`), while an unfocused visible pane stays a fallback. + enabled: dictationEnabled && ownerVisible, focused: focused && dimensionActive, provider: dictationProvider, shortcut: dictationShortcut, @@ -415,9 +418,15 @@ export function AgentTerminalLeaf({ }, [sessionId]) // Also re-runs when the pane is revealed after a takeover (#752): the - // remount used to refocus xterm for free; retained panes need it here. + // remount used to refocus xterm for free; retained panes need it here — + // but only when focus is unowned, so editor-fullscreen exit leaves the + // caret in Monaco (see focusIsUnowned). + const prevFocusedRef = useRef(focused) useEffect(() => { - if (focused && dimensionActive) termRef.current?.focus() + const focusChanged = prevFocusedRef.current !== focused + prevFocusedRef.current = focused + if (!focused || !dimensionActive) return + if (focusChanged || focusIsUnowned()) termRef.current?.focus() }, [focused, dimensionActive]) const focusTerminal = () => { diff --git a/src/renderer/src/workspace/tile-tree/TerminalLeaf.retention.renderer.test.tsx b/src/renderer/src/workspace/tile-tree/TerminalLeaf.retention.renderer.test.tsx new file mode 100644 index 000000000..d87690efc --- /dev/null +++ b/src/renderer/src/workspace/tile-tree/TerminalLeaf.retention.renderer.test.tsx @@ -0,0 +1,156 @@ +import { act, cleanup, render } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { RetainedWorkspaceSurface } from '@renderer/app/shell/RetainedWorkspaceSurface' +import type { Workspace } from '@renderer/workspace/workspaceStore' +import { TerminalLeaf } from './TerminalLeaf' + +const xtermHarness = vi.hoisted(() => ({ + cols: 120, + rows: 40, + onData: null as ((data: string) => void) | null, +})) + +vi.mock('@xterm/xterm', () => ({ + Terminal: class { + cols = xtermHarness.cols + rows = xtermHarness.rows + options: Record = {} + loadAddon() {} + open() {} + onData(listener: (data: string) => void) { + xtermHarness.onData = listener + return { dispose() {} } + } + // Real xterm reports each write parsed via the callback; the input + // forwarder (#745) holds its replay latch until then, so a mock that + // never calls back would model a pane that is deaf forever. + write(_data: string, callback?: () => void) { callback?.() } + focus() {} + dispose() {} + }, +})) + +vi.mock('@xterm/addon-fit', () => ({ + FitAddon: class { + fit() {} + }, +})) + +vi.mock('@renderer/app-state/hooks', () => ({ + useAppStore: (selector: (state: Record) => unknown) => + selector({ + settings: { + dictationEnabled: false, + dictationProvider: 'local', + dictationShortcut: 'off', + }, + }), +})) + +vi.mock('@renderer/app-state/settings/theme', async importOriginal => ({ + ...(await importOriginal>()), + THEME_CHANGED_EVENT: 'agent-code:test-theme-change', + getActiveAppFontFamily: () => 'monospace', +})) + +vi.mock('@renderer/workspace/tile-tree/xtermTheme', () => ({ + readXtermTheme: () => ({}), + syncXtermTheme: () => {}, +})) + +vi.mock('@renderer/workspace/tile-tree/TileLeaf/useComposerDictation', () => ({ + useComposerDictation: () => {}, +})) + +type Deferred = { + promise: Promise + resolve: (value: T) => void +} + +function deferred(): Deferred { + let resolve!: (value: T) => void + const promise = new Promise(done => { + resolve = done + }) + return { promise, resolve } +} + +describe('TerminalLeaf retention', () => { + let attach: Deferred + let resize: ReturnType + let nextFrameId: number + let frames: Map + + const workspace = { + acknowledgeSession: vi.fn(), + ensureSessionLive: vi.fn().mockResolvedValue(undefined), + showPaneToast: vi.fn(), + } as unknown as Workspace + + function flushAnimationFrames() { + const pending = [...frames.values()] + frames.clear() + for (const callback of pending) callback(performance.now()) + } + + beforeEach(() => { + attach = deferred() + resize = vi.fn().mockResolvedValue(undefined) + nextFrameId = 0 + frames = new Map() + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + const id = ++nextFrameId + frames.set(id, callback) + return id + }) + vi.stubGlobal('cancelAnimationFrame', (id: number) => { + frames.delete(id) + }) + Object.defineProperty(window, 'api', { + configurable: true, + value: { + attachTerminal: vi.fn(() => attach.promise), + detachTerminal: vi.fn().mockResolvedValue(undefined), + onSessionTerminalData: vi.fn(() => () => {}), + resize, + sendInput: vi.fn().mockResolvedValue(undefined), + }, + }) + }) + + afterEach(() => { + cleanup() + Reflect.deleteProperty(window, 'api') + vi.unstubAllGlobals() + }) + + // #752 review B3: shell panes have no dimension-ownership hook, so on a + // takeover they must reset the size they last sent and refit on reveal — + // Spotlight's copy may have resized the PTY meanwhile, and the de-dupe + // would otherwise swallow the resize the shell now needs. + it('sends exactly one resize when revealed after a takeover, none while hidden', async () => { + const tree = (hidden: boolean) => ( + + ) + const view = render(tree(false)) + await act(async () => { + attach.resolve('') + await attach.promise + }) + act(() => flushAnimationFrames()) + expect(resize).toHaveBeenCalledTimes(1) + expect(resize).toHaveBeenLastCalledWith('shell-1', 120, 40) + + view.rerender(tree(true)) + act(() => flushAnimationFrames()) + expect(resize).toHaveBeenCalledTimes(1) + + view.rerender(tree(false)) + act(() => flushAnimationFrames()) + expect(resize).toHaveBeenCalledTimes(2) + expect(resize).toHaveBeenLastCalledWith('shell-1', 120, 40) + }) +}) diff --git a/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx b/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx index 2cb90f92e..8e2308e32 100644 --- a/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx @@ -1,4 +1,5 @@ import { useEffect, useRef } from 'react' +import { focusIsUnowned } from '@renderer/workspace/tile-tree/TileLeaf/useInteractiveOwnership' import { useAgentTerminalOwnerVisible } from '@renderer/workspace/terminal/AgentTerminalOwnership' import { Terminal } from '@xterm/xterm' import { FitAddon } from '@xterm/addon-fit' @@ -458,8 +459,14 @@ export function TerminalLeaf({ // focused at the workspace level. For that we have the // synchronous `focusTerminal()` handler below, wired into the // outer div's onMouseDown. + const prevFocusedRef = useRef(focused) useEffect(() => { - if (focused && ownerVisible) termRef.current?.focus() + const focusChanged = prevFocusedRef.current !== focused + prevFocusedRef.current = focused + if (!focused || !ownerVisible) return + // On a reveal only take an unowned focus (see focusIsUnowned): editor- + // fullscreen exit must leave the caret in Monaco. + if (focusChanged || focusIsUnowned()) termRef.current?.focus() }, [focused, ownerVisible]) // Imperative re-focus. Called from outer-div mousedown so that diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx index 851f228e2..7176c6b4e 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx @@ -6,8 +6,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import type { KeyboardEvent } from 'react' import { useAppStore } from '@renderer/app-state/hooks' -import { useGlobalEditorStore } from '@renderer/features/global-editor/store' -import { useWorkspaceSurfaceHidden } from '@renderer/app/shell/RetainedWorkspaceSurface' +import { focusIsUnowned, useInteractiveOwnership } from '@renderer/workspace/tile-tree/TileLeaf/useInteractiveOwnership' import { useGlobalToast } from '@renderer/ui/GlobalToast' import { Feed } from '@renderer/features/feed/ui/Feed' import type { ScrollInfo } from '@renderer/features/feed/ui/Feed' @@ -137,24 +136,14 @@ export function TileLeaf({ const htmlDebugPanelOpen = useAppStore(state => state.htmlDebugPanelOpen) const tailAllMode = useAppStore(state => state.tailAllMode) // The one place the "mounted ⇒ visible" shortcut genuinely breaks: Global - // Editor fullscreen hides the whole workspace subtree with `display: 'none'` - // (GlobalEditorShell) while deliberately keeping it mounted so editor state - // survives. Reading the flag here is what turns "mounted" back into - // "visible" — see the mask below. - const workspaceHiddenByEditor = useGlobalEditorStore(state => state.editorFullscreen) - // Same shape one level up (#752): Reader Mode, Spotlight and Settings now - // retain the whole workspace under display:none instead of unmounting it. - const workspaceHiddenBySurface = useWorkspaceSurfaceHidden() - // WHY "focused" is not enough for input ownership (#752 review): the - // retained tree keeps this leaf mounted AND focused (activeTab's focused - // session) while Reader/Spotlight/Settings own the screen. Every document- - // level router below — type-to-focus, paste-to-focus, the bare-Enter - // submit target, the dictation hotkey, the condition outlet — used to be - // unreachable because the leaf did not exist; now it must be told that a - // hidden focused pane owns nothing. Typing "yes" + Enter in Reader Mode - // must not submit a prompt through an invisible composer. - const workspaceHidden = workspaceHiddenByEditor || workspaceHiddenBySurface - const interactive = focused && !workspaceHidden + // Editor fullscreen (GlobalEditorWorkspaceSlot) and the Reader/Spotlight/ + // Settings takeover (RetainedWorkspaceSurface, #752) both hide the whole + // workspace subtree with `display: 'none'` while deliberately keeping it + // mounted. `hidden` is the composed answer from that context — scoped to + // this subtree, never a global flag — and it is what turns "mounted" back + // into "visible" for the mask below and into "owns keyboard input" for + // every document-level router. See useInteractiveOwnership for why. + const { interactive, hidden: workspaceHidden } = useInteractiveOwnership(focused) // This one OR is the ENTIRE implementation of "Tail All" scoping, and it is // load-bearing in a way that is easy to mistake for a shortcut. // @@ -292,11 +281,17 @@ export function TileLeaf({ endHistoryCycle, } = usePromptHistory({ entries: runtime.entries, sessionKind }) - // When focus flips to this pane — or the pane is revealed again after a - // takeover while still focused — move the DOM caret into its input. + // When focus flips to this pane, move the DOM caret into its input. On a + // reveal (interactive turning true with `focused` unchanged) only take an + // unowned focus: a takeover surface unmounting leaves focus on , but + // editor-fullscreen exit leaves it in Monaco, which must keep it. + const prevFocusedRef = useRef(focused) useEffect(() => { - if (interactive) inputRef.current?.focus() - }, [interactive]) + const focusChanged = prevFocusedRef.current !== focused + prevFocusedRef.current = focused + if (!interactive) return + if (focusChanged || focusIsUnowned()) inputRef.current?.focus() + }, [interactive, focused]) // Type-to-focus — document-level key listener that routes printable // keys into the composer when the pane is focused but DOM focus @@ -665,7 +660,10 @@ export function TileLeaf({ }) const dictation = useComposerDictation({ - enabled: dictationEnabled, + // A hidden pane must not even be a fallback dictation target; an + // unfocused VISIBLE pane must stay registered as one, so this is + // `hidden`, not `interactive`. + enabled: dictationEnabled && !workspaceHidden, focused: interactive, provider: dictationProvider, shortcut: dictationShortcut, diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf/useInteractiveOwnership.renderer.test.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf/useInteractiveOwnership.renderer.test.tsx new file mode 100644 index 000000000..a9d62b75a --- /dev/null +++ b/src/renderer/src/workspace/tile-tree/TileLeaf/useInteractiveOwnership.renderer.test.tsx @@ -0,0 +1,52 @@ +import { renderHook } from '@testing-library/react' +import type { ReactNode } from 'react' +import { describe, expect, it } from 'vitest' + +import { RetainedWorkspaceSurface } from '@renderer/app/shell/RetainedWorkspaceSurface' +import { GlobalEditorWorkspaceSlot } from '@renderer/features/global-editor/ui/GlobalEditorWorkspaceSlot' + +import { useInteractiveOwnership } from './useInteractiveOwnership' + +// #752 review: the source of truth for "may this pane own input" must be the +// composed visibility CONTEXT, scoped to the subtree — not a global store +// flag. A store flag made Spotlight's own (visible) leaf refuse Enter/y/n +// whenever the editor was fullscreen underneath. + +function hook(wrapper?: (props: { children: ReactNode }) => JSX.Element) { + return renderHook(() => useInteractiveOwnership(true), { wrapper }).result.current +} + +describe('useInteractiveOwnership', () => { + it('is interactive with no enclosing shell (Spotlight renders beside the shell)', () => { + expect(hook()).toEqual({ interactive: true, hidden: false }) + }) + + it('is not interactive under a hidden takeover surface', () => { + expect(hook(({ children }) => )) + .toEqual({ interactive: false, hidden: true }) + }) + + it('is not interactive under a fullscreen editor slot', () => { + expect(hook(({ children }) => ( + {children} + ))).toEqual({ interactive: false, hidden: true }) + }) + + it('is not interactive when a visible editor slot sits inside a hidden surface', () => { + expect(hook(({ children }) => ( + + ))).toEqual({ interactive: false, hidden: true }) + }) + + it('is interactive when both shells are visible, and never when unfocused', () => { + const wrapper = ({ children }: { children: ReactNode }) => ( + + ) + expect(renderHook(() => useInteractiveOwnership(true), { wrapper }).result.current).toEqual({ interactive: true, hidden: false }) + expect(renderHook(() => useInteractiveOwnership(false), { wrapper }).result.current).toEqual({ interactive: false, hidden: false }) + }) +}) diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf/useInteractiveOwnership.ts b/src/renderer/src/workspace/tile-tree/TileLeaf/useInteractiveOwnership.ts new file mode 100644 index 000000000..fee13668f --- /dev/null +++ b/src/renderer/src/workspace/tile-tree/TileLeaf/useInteractiveOwnership.ts @@ -0,0 +1,37 @@ +import { useAgentTerminalOwnerVisible } from '@renderer/workspace/terminal/AgentTerminalOwnership' + +// "May this pane own keyboard input right now?" (#752) +// +// WHY this is derived from the composed visibility CONTEXT and not from a +// store flag: the workspace subtree is retained under display:none by two +// different shells — Global Editor fullscreen (GlobalEditorWorkspaceSlot) and +// the Reader/Spotlight/Settings takeover (RetainedWorkspaceSurface) — and +// each one provides the same context, ANDed with its parent. A pane inside +// either shell therefore sees `visible === false`; a pane rendered OUTSIDE +// them (Spotlight's own leaf, which lives beside the shell) sees the default +// `true` even while the editor is fullscreen. Reading the editor store's +// `editorFullscreen` flag instead would be global, not scoped: the first cut +// of this did that and made Spotlight's visible leaf refuse Enter/y/n on an +// approval strip whenever the editor was fullscreen underneath. +// +// WHY "focused" alone is not enough: the retained tree keeps the active +// tab's focused leaf mounted AND focused while another surface owns the +// screen. Every document-level router — type-to-focus, paste-to-focus, the +// bare-Enter submit target, the dictation hotkey, the condition outlet — +// used to be unreachable because the leaf did not exist; now it must be told +// that a hidden focused pane owns nothing. +export function useInteractiveOwnership(focused: boolean): { interactive: boolean; hidden: boolean } { + const visible = useAgentTerminalOwnerVisible() + return { interactive: focused && visible, hidden: !visible } +} + +// WHY refocus-on-reveal must check where focus currently is: a takeover +// surface unmounting leaves DOM focus on , so pulling it back into the +// pane restores what the remount used to do for free. Editor fullscreen +// exit is different — the user is in Monaco, presses Escape (or "Split"), +// and the pane becoming visible again must NOT steal the caret out of the +// file they are editing. Only an unowned focus is ours to take. +export function focusIsUnowned(): boolean { + const active = document.activeElement + return active === null || active === document.body +} From 7f6bb763774fbc379a1d839e694c4a944df71929 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 3 Sep 2026 18:10:02 -0700 Subject: [PATCH 5/5] fix(workspace): gate shell-pane dictation on visibility during takeovers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TerminalLeaf registered useComposerDictation with raw enabled/focused, so a focused shell pane retained under display:none kept claiming the global dictation hotkey while Reader, Spotlight or Settings owned the screen — the transcript went straight into the invisible pane's PTY. Both sibling leaves already gate on visibility; this was the one leaf kind that didn't, the last B2-class hole from round 3. Addresses the round-4 review blocker. --- ...lLeaf.dictationOwnership.renderer.test.tsx | 141 ++++++++++++++++++ .../src/workspace/tile-tree/TerminalLeaf.tsx | 15 +- 2 files changed, 153 insertions(+), 3 deletions(-) create mode 100644 src/renderer/src/workspace/tile-tree/TerminalLeaf.dictationOwnership.renderer.test.tsx diff --git a/src/renderer/src/workspace/tile-tree/TerminalLeaf.dictationOwnership.renderer.test.tsx b/src/renderer/src/workspace/tile-tree/TerminalLeaf.dictationOwnership.renderer.test.tsx new file mode 100644 index 000000000..f736076ba --- /dev/null +++ b/src/renderer/src/workspace/tile-tree/TerminalLeaf.dictationOwnership.renderer.test.tsx @@ -0,0 +1,141 @@ +import { act, cleanup, render } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { RetainedWorkspaceSurface } from '@renderer/app/shell/RetainedWorkspaceSurface' +import type { Workspace } from '@renderer/workspace/workspaceStore' +import { TerminalLeaf } from './TerminalLeaf' + +// #757 round-4 review blocker: a hidden focused shell pane must not own the +// dictation hotkey. TerminalLeaf stays mounted under display:none while +// Reader/Spotlight/Settings own the screen and the retained tree keeps +// focusedSessionId on it; an ungated useComposerDictation registration meant +// the registry's preferred focused target transcribed straight into the +// invisible pane's PTY. This pins the LEAF's contract — both registration +// flags visibility-gated, matching the TileLeaf/AgentTerminalLeaf pattern. +// The registry's own preference behavior is covered by useComposerDictation's +// tests; mocking the hook keeps audio/WebStack out of this file. + +const xtermHarness = vi.hoisted(() => ({ + onData: null as ((data: string) => void) | null, +})) + +vi.mock('@xterm/xterm', () => ({ + Terminal: class { + cols = 120 + rows = 40 + options: Record = {} + loadAddon() {} + open() {} + onData(listener: (data: string) => void) { + xtermHarness.onData = listener + return { dispose() {} } + } + write(_data: string, callback?: () => void) { callback?.() } + focus() {} + dispose() {} + }, +})) + +vi.mock('@xterm/addon-fit', () => ({ + FitAddon: class { + fit() {} + }, +})) + +vi.mock('@renderer/app-state/hooks', () => ({ + useAppStore: (selector: (state: Record) => unknown) => + selector({ + settings: { + dictationEnabled: true, + dictationProvider: 'local', + dictationShortcut: 'off', + }, + }), +})) + +vi.mock('@renderer/app-state/settings/theme', async importOriginal => ({ + ...(await importOriginal>()), + THEME_CHANGED_EVENT: 'agent-code:test-theme-change', + getActiveAppFontFamily: () => 'monospace', +})) + +vi.mock('@renderer/workspace/tile-tree/xtermTheme', () => ({ + readXtermTheme: () => ({}), + syncXtermTheme: () => {}, +})) + +const dictationSpy = vi.hoisted(() => vi.fn()) + +vi.mock('@renderer/workspace/tile-tree/TileLeaf/useComposerDictation', () => ({ + useComposerDictation: dictationSpy, +})) + +function registrationFlags(): { enabled: boolean; focused: boolean } { + const last = dictationSpy.mock.calls[dictationSpy.mock.calls.length - 1]?.[0] as { + enabled: boolean + focused: boolean + } + return { enabled: last?.enabled === true, focused: last?.focused === true } +} + +describe('TerminalLeaf dictation ownership', () => { + let attach: { promise: Promise; resolve: (value: string) => void } + + const workspace = { + acknowledgeSession: vi.fn(), + ensureSessionLive: vi.fn().mockResolvedValue(undefined), + showPaneToast: vi.fn(), + } as unknown as Workspace + + beforeEach(() => { + dictationSpy.mockClear() + let resolveAttach!: (value: string) => void + attach = { + promise: new Promise(done => { resolveAttach = done }), + resolve: resolveAttach, + } + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + callback(performance.now()) + return 0 + }) + vi.stubGlobal('cancelAnimationFrame', () => {}) + Object.defineProperty(window, 'api', { + configurable: true, + value: { + attachTerminal: vi.fn(() => attach.promise), + detachTerminal: vi.fn().mockResolvedValue(undefined), + onSessionTerminalData: vi.fn(() => () => {}), + resize: vi.fn().mockResolvedValue(undefined), + sendInput: vi.fn().mockResolvedValue(undefined), + }, + }) + }) + + afterEach(() => { + cleanup() + Reflect.deleteProperty(window, 'api') + vi.unstubAllGlobals() + }) + + it('registers dictation only while the pane is visible, even when focused', async () => { + const tree = (hidden: boolean) => ( + + ) + const view = render(tree(false)) + await act(async () => { + attach.resolve('') + await attach.promise + }) + expect(registrationFlags()).toEqual({ enabled: true, focused: true }) + + // Takeover: the pane keeps `focused` in the retained tree but must stop + // claiming the dictation hotkey the reader/spotlight surface now owns. + view.rerender(tree(true)) + expect(registrationFlags()).toEqual({ enabled: false, focused: false }) + + view.rerender(tree(false)) + expect(registrationFlags()).toEqual({ enabled: true, focused: true }) + }) +}) diff --git a/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx b/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx index 8e2308e32..6c59796a7 100644 --- a/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx @@ -77,9 +77,19 @@ export function TerminalLeaf({ const dictationEnabled = useAppStore(state => state.settings.dictationEnabled) const dictationProvider = useAppStore(state => state.settings.dictationProvider) const dictationShortcut = useAppStore(state => state.settings.dictationShortcut) + // Declared before the dictation hook so the registration below can gate on + // it (hook order is call order; the visibility answer must exist first). + const ownerVisible = useAgentTerminalOwnerVisible() useComposerDictation({ - enabled: dictationEnabled, - focused, + // WHY both flags are visibility-gated (#757 round-4 review): this leaf + // stays mounted (display:none) while Reader/Spotlight/Settings own the + // screen, and the retained tree keeps focusedSessionId pointing at it. + // The dictation registry prefers the focused target, so an ungated + // registration meant the global Fn hotkey transcribed straight into + // this INVISIBLE pane's PTY. Both siblings (TileLeaf, AgentTerminalLeaf) + // already gate the same way; this was the one leaf kind that didn't. + enabled: dictationEnabled && ownerVisible, + focused: focused && ownerVisible, provider: dictationProvider, shortcut: dictationShortcut, sink: { kind: 'terminal', sessionId }, @@ -115,7 +125,6 @@ export function TerminalLeaf({ // gives it the same stale-size problem: Spotlight's copy resizes the PTY // to full width, and on reveal the retained leaf's fit equals its old // cols/rows, so the de-dupe swallows the resize the shell now needs. - const ownerVisible = useAgentTerminalOwnerVisible() const ownerVisibleRef = useRef(ownerVisible) ownerVisibleRef.current = ownerVisible const onVisibilityChangeRef = useRef<((visible: boolean) => void) | null>(null)