From adb6ee4e958c3791c7db7f2b694f2f3f0006f2ee Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 10 Jul 2026 14:17:22 -0700 Subject: [PATCH 01/13] =?UTF-8?q?Extract=20port=E2=86=92URL=20and=20browse?= =?UTF-8?q?r-surface=20plumbing=20for=20the=20header=20connect=20menu?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Groundwork for a pane-header right-click menu that lists a terminal's bound ports and connects to one like `dor ab open`: - port-url.ts: listenerUrlsByPort grouping shared by surface.resolveOpen and the upcoming menu (loopback wins localhost, IPv6 bracketed) - use-dor-control: reuse-or-create surface logic extracted as ensureAgentBrowserSurface; the split reference resolves lazily so a minimized caller can still refresh an existing surface - connect-port.ts: webview reproduction of `dor ab open ` against the workspace's default agent-browser session - WallActions: resolveSurfaceRef + onConnectPort - dor-lib-common: browser-safe ./agent-browser subpath export Co-Authored-By: Claude Fable 5 --- docs/specs/dor-browser.md | 1 + docs/specs/dor-cli.md | 3 +- dor-lib-common/package.json | 4 + lib/src/components/Wall.tsx | 18 +- .../wall/AgentBrowserPanel.test.tsx | 2 + lib/src/components/wall/IframePanel.test.tsx | 2 + .../wall/SurfacePaneHeader.test.tsx | 2 + lib/src/components/wall/connect-port.test.ts | 85 ++++++++ lib/src/components/wall/connect-port.ts | 61 ++++++ lib/src/components/wall/port-url.test.ts | 100 +++++++++ lib/src/components/wall/port-url.ts | 50 +++++ .../components/wall/use-dev-server-ports.ts | 8 +- lib/src/components/wall/use-dor-control.ts | 193 ++++++++++-------- lib/src/components/wall/wall-context.tsx | 10 + .../stories/BrowserChromeHeader.stories.tsx | 5 + lib/src/stories/MouseHeaderIcon.stories.tsx | 2 + lib/src/stories/ShellCwd.stories.tsx | 2 + .../stories/TerminalPaneHeader.stories.tsx | 2 + 18 files changed, 458 insertions(+), 92 deletions(-) create mode 100644 lib/src/components/wall/connect-port.test.ts create mode 100644 lib/src/components/wall/connect-port.ts create mode 100644 lib/src/components/wall/port-url.test.ts create mode 100644 lib/src/components/wall/port-url.ts diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index fe9b91c4..93ca588a 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -161,6 +161,7 @@ polls only while a wanted port is still unmatched. Reload revalidates optimistically. Source of truth: `lib/src/components/wall/use-dev-server-ports.ts`, +`lib/src/components/wall/port-url.ts` (the `servesLoopback` predicate), `lib/src/components/wall/agent-browser-ports.ts`, `lib/src/components/wall/browser-url.ts`. diff --git a/docs/specs/dor-cli.md b/docs/specs/dor-cli.md index 17b71b87..65d89623 100644 --- a/docs/specs/dor-cli.md +++ b/docs/specs/dor-cli.md @@ -427,7 +427,8 @@ Source of truth: `dor/src/commands/open-target.ts` (classification + `:port` sugar + `surface.resolveOpen` call), `dor/src/commands/iframe.ts` / `dor/src/commands/agent-browser.ts` (the two entry points), `dor/src/protocol.ts` (`resolveOpen`), the `surface.resolveOpen` handler in -`lib/src/components/wall/use-dor-control.ts`. +`lib/src/components/wall/use-dor-control.ts`, and the port→URL grouping/selection +in `lib/src/components/wall/port-url.ts` (`listenerUrlsByPort`). ## Agent Workflows diff --git a/dor-lib-common/package.json b/dor-lib-common/package.json index 4848b9d6..2515c1c2 100644 --- a/dor-lib-common/package.json +++ b/dor-lib-common/package.json @@ -8,6 +8,10 @@ ".": { "types": "./dist/index.d.ts", "default": "./dist/index.js" + }, + "./agent-browser": { + "types": "./dist/agent-browser.d.ts", + "default": "./dist/agent-browser.js" } }, "scripts": { diff --git a/lib/src/components/Wall.tsx b/lib/src/components/Wall.tsx index dc84264e..74b00b53 100644 --- a/lib/src/components/Wall.tsx +++ b/lib/src/components/Wall.tsx @@ -71,6 +71,7 @@ import { useWallKeyboard } from './wall/use-wall-keyboard'; import { useSessionPersistence } from './wall/use-session-persistence'; import { useDevServerPortCorrelation } from './wall/use-dev-server-ports'; import { useDorControl } from './wall/use-dor-control'; +import { connectPortToDefaultBrowser } from './wall/connect-port'; import { useWindowFocused } from './wall/use-window-focused'; import { DialogKeyboardContext, @@ -1106,7 +1107,7 @@ export function Wall({ }, [generatePaneId, surfaceRefForId, forgetSurfaceRef, selectPane, showShellSpawnNotice, lath, nav]); // --- dor control plane (the `dor` CLI's webview handler) --- - useDorControl({ + const { ensureAgentBrowserSurface } = useDorControl({ lath, nav, doorsRef, @@ -1285,7 +1286,20 @@ export function Wall({ title: hostPathDisplay(url, true), }); }, - }), [addSplitPanel, minimizePane, enterTerminalMode, exitTerminalMode, killPaneImmediately, replaceSurface, buildDorSurfaces, createContentSurface, lath, nav]); + resolveSurfaceRef: (id) => surfaceRefForId(id), + onConnectPort: async (id, url) => { + // The pane context menu's "connect a port" action: act like `dor ab open`. + const reference = buildDorSurfaces().find((s) => s.id === id); + if (!reference) return { ok: false, message: 'pane is gone' }; + return connectPortToDefaultBrowser({ + url, + reference, + platform: getPlatform(), + binaryPath: lastAgentBrowserBinaryPathRef.current, + ensureSurface: ensureAgentBrowserSurface, + }); + }, + }), [addSplitPanel, minimizePane, enterTerminalMode, exitTerminalMode, killPaneImmediately, replaceSurface, buildDorSurfaces, createContentSurface, surfaceRefForId, ensureAgentBrowserSurface, lath, nav]); const wallActionsRef = useRef(wallActions); wallActionsRef.current = wallActions; diff --git a/lib/src/components/wall/AgentBrowserPanel.test.tsx b/lib/src/components/wall/AgentBrowserPanel.test.tsx index 86c10301..9aee7801 100644 --- a/lib/src/components/wall/AgentBrowserPanel.test.tsx +++ b/lib/src/components/wall/AgentBrowserPanel.test.tsx @@ -46,6 +46,8 @@ function stubActions(overrides: Partial = {}): WallActions { onFinishRename: vi.fn(() => ({ accepted: true })), onCancelRename: vi.fn(), onSwapRenderMode: vi.fn(), + resolveSurfaceRef: vi.fn((id: string) => id), + onConnectPort: vi.fn(async () => ({ ok: true as const })), ...overrides, }; } diff --git a/lib/src/components/wall/IframePanel.test.tsx b/lib/src/components/wall/IframePanel.test.tsx index 0cde700b..c3e039ba 100644 --- a/lib/src/components/wall/IframePanel.test.tsx +++ b/lib/src/components/wall/IframePanel.test.tsx @@ -28,6 +28,8 @@ function stubActions(overrides: Partial = {}): WallActions { onFinishRename: vi.fn(() => ({ accepted: true })), onCancelRename: vi.fn(), onSwapRenderMode: vi.fn(), + resolveSurfaceRef: vi.fn((id: string) => id), + onConnectPort: vi.fn(async () => ({ ok: true as const })), ...overrides, }; } diff --git a/lib/src/components/wall/SurfacePaneHeader.test.tsx b/lib/src/components/wall/SurfacePaneHeader.test.tsx index 803262bb..5764804f 100644 --- a/lib/src/components/wall/SurfacePaneHeader.test.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.test.tsx @@ -46,6 +46,8 @@ function stubActions(overrides: Partial = {}): WallActions { onFinishRename: vi.fn(() => ({ accepted: true })), onCancelRename: vi.fn(), onSwapRenderMode: vi.fn(), + resolveSurfaceRef: vi.fn((id: string) => id), + onConnectPort: vi.fn(async () => ({ ok: true as const })), ...overrides, }; } diff --git a/lib/src/components/wall/connect-port.test.ts b/lib/src/components/wall/connect-port.test.ts new file mode 100644 index 00000000..7130ee7a --- /dev/null +++ b/lib/src/components/wall/connect-port.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it, vi } from 'vitest'; +import { sessionForKey } from 'dor-lib-common/agent-browser'; +import { connectPortToDefaultBrowser } from './connect-port'; +import type { EnsureAgentBrowserSurfaceResult } from './use-dor-control'; +import type { Surface as DorSurface } from 'dor/commands/types'; + +const URL = 'http://localhost:5173/'; +const SESSION = sessionForKey('default'); +const reference = { id: 'pane-1', ref: 'surface:1' } as DorSurface; + +const okEnsure = (): EnsureAgentBrowserSurfaceResult => + ({ ok: true, status: 'created', surfaceId: 'pane-2', surfaceRef: 'surface:2', minimized: false }); + +// ensureSurface receives the reference as a lazy thunk; unwrap it so the +// call-shape assertions can compare plain values. +function ensureCallArgs(ensureSurface: ReturnType) { + const [args] = ensureSurface.mock.calls.at(-1) ?? []; + const { reference: lazyReference, ...rest } = args as { reference: () => { ok: boolean; value?: DorSurface } } & Record; + return { ...rest, reference: lazyReference().value }; +} + +describe('connectPortToDefaultBrowser', () => { + it('opens the URL in the default session, reads the port, and ensures the surface', async () => { + const ensureSurface = vi.fn(okEnsure); + const platform = { + agentBrowserCommand: vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })), + agentBrowserStreamStatus: vi.fn(async () => ({ ok: true, wsPort: 4321 })), + }; + const result = await connectPortToDefaultBrowser({ url: URL, reference, platform, binaryPath: '/bin/ab', ensureSurface }); + + expect(result).toEqual({ ok: true }); + expect(platform.agentBrowserCommand).toHaveBeenCalledWith(SESSION, ['open', URL], '/bin/ab'); + expect(platform.agentBrowserStreamStatus).toHaveBeenCalledWith(SESSION, '/bin/ab'); + expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: 4321, binaryPath: '/bin/ab', reference }); + }); + + it('fails when the host cannot run agent-browser', async () => { + const ensureSurface = vi.fn(okEnsure); + const result = await connectPortToDefaultBrowser({ url: URL, reference, platform: {}, ensureSurface }); + expect(result).toEqual({ ok: false, message: 'opening a browser surface is not supported on this host' }); + expect(ensureSurface).not.toHaveBeenCalled(); + }); + + it('surfaces trimmed stderr on a non-zero exit', async () => { + const ensureSurface = vi.fn(okEnsure); + const platform = { agentBrowserCommand: vi.fn(async () => ({ exitCode: 3, stdout: '', stderr: ' boom \n' })) }; + const result = await connectPortToDefaultBrowser({ url: URL, reference, platform, ensureSurface }); + expect(result).toEqual({ ok: false, message: 'boom' }); + expect(ensureSurface).not.toHaveBeenCalled(); + }); + + it('reports the exit code when a non-zero exit has no stderr', async () => { + const platform = { agentBrowserCommand: vi.fn(async () => ({ exitCode: 3, stdout: '', stderr: '' })) }; + const result = await connectPortToDefaultBrowser({ url: URL, reference, platform, ensureSurface: vi.fn(okEnsure) }); + expect(result).toEqual({ ok: false, message: 'agent-browser open exited 3' }); + }); + + it('leaves wsPort undefined when the host has no stream-status channel', async () => { + const ensureSurface = vi.fn(okEnsure); + const platform = { agentBrowserCommand: vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })) }; + const result = await connectPortToDefaultBrowser({ url: URL, reference, platform, ensureSurface }); + expect(result).toEqual({ ok: true }); + expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: undefined, binaryPath: undefined, reference }); + }); + + it('leaves wsPort undefined when stream status fails', async () => { + const ensureSurface = vi.fn(okEnsure); + const platform = { + agentBrowserCommand: vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })), + agentBrowserStreamStatus: vi.fn(async () => ({ ok: false, error: 'nope' })), + }; + await connectPortToDefaultBrowser({ url: URL, reference, platform, binaryPath: '/bin/ab', ensureSurface }); + expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: undefined, binaryPath: '/bin/ab', reference }); + }); + + it('propagates an ensureSurface failure', async () => { + const ensureSurface = vi.fn((): EnsureAgentBrowserSurfaceResult => ({ ok: false, message: 'no room' })); + const platform = { + agentBrowserCommand: vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })), + agentBrowserStreamStatus: vi.fn(async () => ({ ok: true, wsPort: 4321 })), + }; + const result = await connectPortToDefaultBrowser({ url: URL, reference, platform, ensureSurface }); + expect(result).toEqual({ ok: false, message: 'no room' }); + }); +}); diff --git a/lib/src/components/wall/connect-port.ts b/lib/src/components/wall/connect-port.ts new file mode 100644 index 00000000..da0f866c --- /dev/null +++ b/lib/src/components/wall/connect-port.ts @@ -0,0 +1,61 @@ +/** + * "Connect a port" — the wall-side reproduction of `dor ab open ` for the + * pane context menu (see `dor/src/commands/agent-browser.ts` for the CLI flow). + * Opens the URL in the workspace's default agent-browser session and reuses or + * creates that session's browser surface. Dependency-injected (platform + + * `ensureSurface`) so it stays unit-testable off the React tree. + */ +import type { PlatformAdapter } from '../../lib/platform/types'; +import type { Surface as DorSurface } from 'dor/commands/types'; +// The browser-safe subpath: dist/agent-browser.js is pure ES, so importing it +// keeps cross-spawn (the package's Node-only default export) out of the webview. +import { sessionForKey } from 'dor-lib-common/agent-browser'; +import type { EnsureAgentBrowserSurface } from './use-dor-control'; + +/** The host capabilities `connectPortToDefaultBrowser` needs — the same two the + * CLI path leans on, narrowed so tests can stub them without a full adapter. */ +type ConnectPlatform = Pick; + +export type ConnectPortResult = { ok: true } | { ok: false; message: string }; + +export async function connectPortToDefaultBrowser({ + url, + reference, + platform, + binaryPath, + ensureSurface, +}: { + url: string; + /** The pane the port belongs to — the split reference for a fresh surface. */ + reference: DorSurface; + platform: ConnectPlatform; + /** Last binary path a `dor ab` surface resolved; undefined ⇒ host falls back + * to PATH / DORMOUSE_AGENT_BROWSER_BIN. */ + binaryPath?: string; + ensureSurface: EnsureAgentBrowserSurface; +}): Promise { + const session = sessionForKey('default'); + // Host-gated: no agent-browser runner ⇒ no browser surface (same spirit as the + // render-swap guard in Wall.tsx). + if (!platform.agentBrowserCommand) { + return { ok: false, message: 'opening a browser surface is not supported on this host' }; + } + // 'open' is on the host's subcommand allowlist; the CLI boots the daemon/browser + // if it isn't already running. + const opened = await platform.agentBrowserCommand(session, ['open', url], binaryPath); + if (opened.exitCode !== 0) { + return { ok: false, message: opened.stderr.trim() || `agent-browser open exited ${opened.exitCode}` }; + } + // Best-effort stream port so the panel connects straight to the live screencast; + // if it's absent or stale the panel recovers it later, so a miss is non-fatal. + let wsPort: number | undefined; + if (platform.agentBrowserStreamStatus) { + const status = await platform.agentBrowserStreamStatus(session, binaryPath); + if (status.ok) wsPort = status.wsPort; + } + // The menu resolved its pane eagerly (it's the visible pane under the cursor), + // so the lazy reference just hands it through. + const ensured = ensureSurface({ key: 'default', session, wsPort, binaryPath, reference: () => ({ ok: true, value: reference }) }); + if (!ensured.ok) return { ok: false, message: ensured.message }; + return { ok: true }; +} diff --git a/lib/src/components/wall/port-url.test.ts b/lib/src/components/wall/port-url.test.ts new file mode 100644 index 00000000..58e016d7 --- /dev/null +++ b/lib/src/components/wall/port-url.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it } from 'vitest'; +import { listenerUrlsByPort, servesLoopback } from './port-url'; +import type { OpenPort } from '../../lib/platform/types'; + +function tcp(o: { + address: string; + port: number; + family?: 'IPv4' | 'IPv6'; + pid?: number; + processName?: string; +}): OpenPort { + return { + protocol: 'tcp', + family: o.family ?? (o.address.includes(':') ? 'IPv6' : 'IPv4'), + address: o.address, + port: o.port, + pid: o.pid ?? 1234, + ...(o.processName !== undefined ? { processName: o.processName } : {}), + }; +} + +// protocol is typed 'tcp'; the filter is still exercised via a cast. +const udp = (port: number): OpenPort => + ({ protocol: 'udp', family: 'IPv4', address: '127.0.0.1', port, pid: 1 } as unknown as OpenPort); + +describe('servesLoopback', () => { + it('is true for loopback and any-interface binds', () => { + for (const addr of ['127.0.0.1', '::1', '0.0.0.0', '::']) expect(servesLoopback(addr)).toBe(true); + }); + it('is false for a specific interface', () => { + for (const addr of ['192.168.1.5', 'fe80::1']) expect(servesLoopback(addr)).toBe(false); + }); +}); + +describe('listenerUrlsByPort', () => { + it.each(['127.0.0.1', '::1', '0.0.0.0', '::'])( + 'prefers a loopback-servable bind (%s) → localhost, even beside a specific one', + (loopbackAddr) => { + const ports = [tcp({ address: '192.168.1.5', port: 5173 }), tcp({ address: loopbackAddr, port: 5173 })]; + expect(listenerUrlsByPort(ports)).toEqual([{ port: 5173, host: 'localhost', url: 'http://localhost:5173/' }]); + }, + ); + + it('uses the specific bound address when no loopback bind exists', () => { + expect(listenerUrlsByPort([tcp({ address: '192.168.1.5', port: 8080 })])).toEqual([ + { port: 8080, host: '192.168.1.5', url: 'http://192.168.1.5:8080/' }, + ]); + }); + + it('brackets a specific IPv6 bind', () => { + expect(listenerUrlsByPort([tcp({ address: 'fe80::1', port: 3000 })])).toEqual([ + { port: 3000, host: '[fe80::1]', url: 'http://[fe80::1]:3000/' }, + ]); + }); + + it('breaks ties IPv4-before-IPv6 among specific binds', () => { + const ports = [tcp({ address: 'fe80::1', port: 4000 }), tcp({ address: '10.0.0.2', port: 4000 })]; + expect(listenerUrlsByPort(ports)[0]).toEqual({ port: 4000, host: '10.0.0.2', url: 'http://10.0.0.2:4000/' }); + }); + + it('breaks same-family ties by address lexicographically', () => { + const ports = [tcp({ address: '10.0.0.9', port: 4100 }), tcp({ address: '10.0.0.2', port: 4100 })]; + expect(listenerUrlsByPort(ports)[0].host).toBe('10.0.0.2'); + }); + + it('emits one entry per port even across families (dedupe)', () => { + const ports = [tcp({ address: '127.0.0.1', port: 5173 }), tcp({ address: '::1', port: 5173 })]; + expect(listenerUrlsByPort(ports)).toEqual([{ port: 5173, host: 'localhost', url: 'http://localhost:5173/' }]); + }); + + it('sorts entries ascending by port', () => { + const ports = [8080, 3000, 5173].map((port) => tcp({ address: '127.0.0.1', port })); + expect(listenerUrlsByPort(ports).map((e) => e.port)).toEqual([3000, 5173, 8080]); + }); + + it('ignores non-tcp listeners', () => { + expect(listenerUrlsByPort([udp(9000), tcp({ address: '127.0.0.1', port: 5173 })]).map((e) => e.port)).toEqual([5173]); + expect(listenerUrlsByPort([udp(9000)])).toEqual([]); + }); + + it('carries the selected listener processName', () => { + expect(listenerUrlsByPort([tcp({ address: '127.0.0.1', port: 5173, processName: 'node' })])[0].processName).toBe('node'); + }); + + it('prefers the selected listener processName over siblings', () => { + const ports = [ + tcp({ address: '192.168.1.5', port: 5173, processName: 'sibling' }), + tcp({ address: '127.0.0.1', port: 5173, processName: 'chosen' }), + ]; + expect(listenerUrlsByPort(ports)[0].processName).toBe('chosen'); + }); + + it('falls back to the first defined processName when the selected listener has none', () => { + const ports = [ + tcp({ address: '127.0.0.1', port: 5173 }), + tcp({ address: '192.168.1.5', port: 5173, processName: 'vite' }), + ]; + expect(listenerUrlsByPort(ports)[0].processName).toBe('vite'); + }); +}); diff --git a/lib/src/components/wall/port-url.ts b/lib/src/components/wall/port-url.ts new file mode 100644 index 00000000..39488985 --- /dev/null +++ b/lib/src/components/wall/port-url.ts @@ -0,0 +1,50 @@ +/** + * Port → dev-server URL selection, shared by the `surface.resolveOpen` control + * method (`dor ab open ` / `dor iframe `) and the pane + * context menu's port list. One entry per distinct TCP port a process tree + * binds; the host/URL choice matches what `dor ab open` opens. + */ +import type { OpenPort } from '../../lib/platform/types'; + +/** One openable dev-server URL for a distinct listening port. */ +export type PortUrlEntry = { port: number; host: string; url: string; processName?: string }; + +// A process bound here answers `localhost:`: loopback (127.0.0.1 / ::1) +// or any-interface (0.0.0.0 / ::). A process bound to one specific non-loopback +// interface is excluded — it isn't reachable as localhost. +export function servesLoopback(address: string): boolean { + return address === '127.0.0.1' || address === '::1' || address === '0.0.0.0' || address === '::'; +} + +/** + * Group TCP listeners into one openable URL per distinct port, sorted ascending + * by port. Per port: a loopback-servable bind wins the host `localhost`; + * otherwise pick a listener IPv4-first then address-lexicographic, bracketing + * IPv6 (`[::1]`). The URL is `http://:/`. `processName` is the + * selected listener's, falling back to the first defined one for that port. + */ +export function listenerUrlsByPort(ports: OpenPort[]): PortUrlEntry[] { + const tcpPorts = ports.filter((entry) => entry.protocol === 'tcp'); + const distinctPorts = [...new Set(tcpPorts.map((entry) => entry.port))].sort((a, b) => a - b); + return distinctPorts.map((port) => { + const portListeners = tcpPorts.filter((entry) => entry.port === port); + const loopbackListener = portListeners.find((entry) => servesLoopback(entry.address)); + const selectedListener = loopbackListener ?? [...portListeners].sort((a, b) => { + if (a.family !== b.family) return a.family === 'IPv4' ? -1 : 1; + return a.address < b.address ? -1 : a.address > b.address ? 1 : 0; + })[0]; + const host = loopbackListener + ? 'localhost' + : selectedListener.family === 'IPv6' + ? `[${selectedListener.address}]` + : selectedListener.address; + const processName = selectedListener.processName + ?? portListeners.find((entry) => entry.processName !== undefined)?.processName; + return { + port, + host, + url: `http://${host}:${port}/`, + ...(processName !== undefined ? { processName } : {}), + }; + }); +} diff --git a/lib/src/components/wall/use-dev-server-ports.ts b/lib/src/components/wall/use-dev-server-ports.ts index 5fa933e2..79019ba9 100644 --- a/lib/src/components/wall/use-dev-server-ports.ts +++ b/lib/src/components/wall/use-dev-server-ports.ts @@ -37,6 +37,7 @@ import { import type { DooredItem } from './wall-types'; import type { LathWallEngine } from './lath-wall-engine'; import { isBrowserParams } from './browser-surface'; +import { servesLoopback } from './port-url'; // Wait this long after interest changes before scanning, so a tab's open + // initial screencast settle first and quick navigation coalesces into one scan. @@ -57,13 +58,6 @@ function isTerminalDoor(door: DooredItem): boolean { return (door.component ?? 'terminal') === 'terminal'; } -// A process bound here answers `localhost:`: loopback (127.0.0.1 / ::1) -// or any-interface (0.0.0.0 / ::). A process bound to one specific non-loopback -// interface is excluded — it isn't reachable as localhost. -export function servesLoopback(address: string): boolean { - return address === '127.0.0.1' || address === '::1' || address === '0.0.0.0' || address === '::'; -} - // requestIdleCallback isn't universal (absent in WKWebView / Tauri on macOS), // so fall back to a short timer. Handles are plain numbers in both paths. function scheduleIdle(cb: () => void): number { diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index 5ffa1255..4fea8f8c 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -20,7 +20,7 @@ import { import { surfaceRunsCommand, type TerminalPaneState } from '../../lib/terminal-state'; import { hostPathDisplay } from './browser-url'; import { isAgentBrowserParams } from './browser-surface'; -import { servesLoopback } from './use-dev-server-ports'; +import { listenerUrlsByPort } from './port-url'; import { dorDirectionForEdge, type LathWallEngine } from './lath-wall-engine'; import type { WallNav } from './keyboard/types'; import type { DooredItem } from './wall-types'; @@ -57,6 +57,29 @@ type DorControlRequest = Omit & { respond: (response: DorControlResult) => void; }; +/** Outcome of {@link EnsureAgentBrowserSurface}: the fields the caller maps onto + * its response, or a failure message. `minimized` is the surface's current + * minimized state (the reused surface's, or the requested value for a fresh one). */ +export type EnsureAgentBrowserSurfaceResult = + | { ok: true; status: 'created' | 'existing' | 'replaced'; surfaceId: string; surfaceRef: string; minimized: boolean } + | { ok: false; message: string }; + +/** Reuse-or-create the browser surface bound to an agent-browser `session`, + * refreshing its stream port / binary path when reused. This is the surface + * half of `dor ab` — shared by the control plane and the pane context menu's + * "connect a port" action (`connect-port.ts`). */ +export type EnsureAgentBrowserSurface = (args: { + key?: string; + session: string; + wsPort?: number; + binaryPath?: string; + /** Resolved lazily, only when a fresh surface must be created: the reuse path + * must succeed without a visible reference (e.g. `dor ab` from a minimized + * terminal refreshing an existing surface). */ + reference: () => ParseResult; + minimized?: boolean; +}) => EnsureAgentBrowserSurfaceResult; + function isSingletonWorkspaceTarget(target: string | undefined): boolean { return !target || target === 'workspace:1' || target === '1'; } @@ -345,7 +368,7 @@ export function useDorControl({ killPaneImmediately: (id: string) => void; /** The last binary path a `dor ab` surface resolved on a terminal's PATH. */ lastAgentBrowserBinaryPathRef: MutableRefObject; -}): void { +}): { ensureAgentBrowserSurface: EnsureAgentBrowserSurface } { const resolveVisibleSurface = useCallback(( target: string | undefined, callerSurfaceId: string | undefined, @@ -416,6 +439,70 @@ export function useDorControl({ return null; }, [lath]); + const ensureAgentBrowserSurface = useCallback(({ + key, + session, + wsPort, + binaryPath, + reference, + minimized = false, + }) => { + // Remember the resolved binary so an embed→screencast swap can spawn one. + if (binaryPath) lastAgentBrowserBinaryPathRef.current = binaryPath; + const refreshedParams = { + ...(wsPort !== undefined ? { wsPort } : {}), + ...(binaryPath !== undefined ? { binaryPath } : {}), + }; + + const existing = findAgentBrowserSurface(session); + if (existing) { + // Reuse: refresh the stream port (OS-assigned, churns across session + // restarts) so the panel reconnects to the live stream, and the + // resolved binary path alongside it. + if (!existing.minimized && Object.keys(refreshedParams).length > 0) { + lath.store.updateParams(existing.id, refreshedParams); + } else if (existing.minimized && Object.keys(refreshedParams).length > 0) { + const nextDoors = doorsRef.current.map((door) => door.id === existing.id + ? { ...door, params: { ...door.params, ...refreshedParams } } + : door); + doorsRef.current = nextDoors; + setDoors(nextDoors); + } + return { + ok: true, + status: 'existing', + surfaceId: existing.id, + surfaceRef: surfaceRefForId(existing.id), + minimized: existing.minimized, + }; + } + + const target = reference(); + if (!target.ok) return { ok: false, message: target.message }; + const result = createContentSurface({ + minimized, + params: { + surfaceType: 'browser', + renderMode: 'ab-screencast', + session, + ...(key !== undefined ? { key } : {}), + ...refreshedParams, + }, + reference: target.value, + title: key ?? session, + // `dor ab` opens the screencast in the background; caller keeps focus. + focusNeutral: true, + }); + if (!result.ok) return { ok: false, message: result.message }; + return { + ok: true, + status: result.value.status, + surfaceId: result.value.id, + surfaceRef: result.value.ref, + minimized, + }; + }, [createContentSurface, findAgentBrowserSurface, lath, setDoors, surfaceRefForId]); + useEffect(() => { const handler = async (event: Event) => { const detail = (event as CustomEvent).detail; @@ -723,61 +810,13 @@ export function useDorControl({ detail.respond({ ok: false, error: 'session is required' }); return; } - const key = stringParam(params.key); - const wsPort = numberParam(params.wsPort); - const binaryPath = stringParam(params.binaryPath); - // Remember the resolved binary so an embed→screencast swap can spawn one. - if (binaryPath) lastAgentBrowserBinaryPathRef.current = binaryPath; - const refreshedParams = { - ...(wsPort !== undefined ? { wsPort } : {}), - ...(binaryPath !== undefined ? { binaryPath } : {}), - }; - - const existing = findAgentBrowserSurface(session); - if (existing) { - // Reuse: refresh the stream port (OS-assigned, churns across session - // restarts) so the panel reconnects to the live stream, and the - // resolved binary path alongside it. - if (!existing.minimized && Object.keys(refreshedParams).length > 0) { - lath.store.updateParams(existing.id, refreshedParams); - } else if (existing.minimized && Object.keys(refreshedParams).length > 0) { - const nextDoors = doorsRef.current.map((door) => door.id === existing.id - ? { ...door, params: { ...door.params, ...refreshedParams } } - : door); - doorsRef.current = nextDoors; - setDoors(nextDoors); - } - detail.respond({ - ok: true, - result: { - status: 'existing', - surfaceId: existing.id, - surfaceRef: surfaceRefForId(existing.id), - session, - minimized: existing.minimized, - }, - }); - return; - } - - const target = resolveVisibleSurface(stringParam(params.surface), detail.surfaceId); - if (!target.ok) { - detail.respond({ ok: false, error: target.message }); - return; - } - const result = createContentSurface({ + const result = ensureAgentBrowserSurface({ + key: stringParam(params.key), + session, + wsPort: numberParam(params.wsPort), + binaryPath: stringParam(params.binaryPath), + reference: () => resolveVisibleSurface(stringParam(params.surface), detail.surfaceId), minimized: booleanParam(params.minimized), - params: { - surfaceType: 'browser', - renderMode: 'ab-screencast', - session, - ...(key !== undefined ? { key } : {}), - ...refreshedParams, - }, - reference: target.value, - title: key ?? session, - // `dor ab` opens the screencast in the background; caller keeps focus. - focusNeutral: true, }); if (!result.ok) { detail.respond({ ok: false, error: result.message }); @@ -786,11 +825,11 @@ export function useDorControl({ detail.respond({ ok: true, result: { - status: result.value.status, - surfaceId: result.value.id, - surfaceRef: result.value.ref, + status: result.status, + surfaceId: result.surfaceId, + surfaceRef: result.surfaceRef, session, - minimized: booleanParam(params.minimized), + minimized: result.minimized, }, }); return; @@ -809,40 +848,28 @@ export function useDorControl({ } catch { ports = []; } - const tcpPorts = ports.filter((entry) => entry.protocol === 'tcp'); - // Group every TCP listener by port. A loopback-reachable binding wins - // for the localhost URL; otherwise use the bound LAN/Tailnet address. - const candidatePorts = [...new Set(tcpPorts.map((entry) => entry.port))].sort((a, b) => a - b); - if (candidatePorts.length === 0) { + // Group every TCP listener into one openable URL per distinct port + // (loopback-reachable bind wins localhost; otherwise the bound + // LAN/Tailnet address). Shared with the pane context menu's port list. + const entries = listenerUrlsByPort(ports); + if (entries.length === 0) { detail.respond({ ok: false, error: `surface '${target.ref}' is not serving any port` }); return; } - if (candidatePorts.length > 1) { + if (entries.length > 1) { detail.respond({ ok: false, - error: `surface '${target.ref}' is serving multiple ports (${candidatePorts.join(', ')}); open one explicitly, e.g. http://localhost:${candidatePorts[0]}`, + error: `surface '${target.ref}' is serving multiple ports (${entries.map((entry) => entry.port).join(', ')}); open one explicitly, e.g. http://localhost:${entries[0].port}`, }); return; } - const port = candidatePorts[0]; - const portListeners = tcpPorts.filter((entry) => entry.port === port); - const loopbackListener = portListeners.find((entry) => servesLoopback(entry.address)); - const selectedListener = loopbackListener ?? [...portListeners].sort((a, b) => { - if (a.family !== b.family) return a.family === 'IPv4' ? -1 : 1; - return a.address < b.address ? -1 : a.address > b.address ? 1 : 0; - })[0]; - const host = loopbackListener - ? 'localhost' - : selectedListener.family === 'IPv6' - ? `[${selectedListener.address}]` - : selectedListener.address; detail.respond({ ok: true, result: { surfaceId: target.id, surfaceRef: target.ref, - port, - url: `http://${host}:${port}/`, + port: entries[0].port, + url: entries[0].url, }, }); return; @@ -853,5 +880,7 @@ export function useDorControl({ window.addEventListener('dormouse:control-request', handler); return () => window.removeEventListener('dormouse:control-request', handler); - }, [buildDorSurfaces, buildDorSurfaceList, createContentSurface, createSplitSurface, findAgentBrowserSurface, findSurfaceIdRunningCommand, killPaneImmediately, requireListedSurface, requireTerminalSurface, resolveListedSurface, resolveVisibleSurface, surfaceRefForId, lath, nav]); + }, [buildDorSurfaces, buildDorSurfaceList, createContentSurface, createSplitSurface, ensureAgentBrowserSurface, findSurfaceIdRunningCommand, killPaneImmediately, requireListedSurface, requireTerminalSurface, resolveListedSurface, resolveVisibleSurface, surfaceRefForId, lath, nav]); + + return { ensureAgentBrowserSurface }; } diff --git a/lib/src/components/wall/wall-context.tsx b/lib/src/components/wall/wall-context.tsx index 2168f021..35800aa0 100644 --- a/lib/src/components/wall/wall-context.tsx +++ b/lib/src/components/wall/wall-context.tsx @@ -49,6 +49,14 @@ export interface WallActions { * window.open, surfaced by the proxy shim) becomes a new pane * (docs/specs/dor-browser.md → "Iframe Shim"). */ onOpenBrowserPane?: (id: string, url: string) => void; + /** The stable `surface:N` ref for a pane/door id (minted lazily, exactly as + * `dor list` assigns refs). Used by the pane context menu to show the handle. */ + resolveSurfaceRef: (id: string) => string; + /** Act like `dor ab open ` for a port the pane's process tree binds: open + * the URL in the workspace's default agent-browser session and reuse-or-create + * its browser surface (`connect-port.ts`). Resolves with a failure message the + * menu can surface. */ + onConnectPort: (id: string, url: string) => Promise<{ ok: true } | { ok: false; message: string }>; } export const WallActionsContext = createContext({ @@ -66,6 +74,8 @@ export const WallActionsContext = createContext({ onCancelRename: () => {}, onSwapRenderMode: () => {}, onOpenBrowserPane: () => {}, + resolveSurfaceRef: (id: string) => id, + onConnectPort: async () => ({ ok: false, message: 'no Wall is mounted' }), }); /** Engine-directed writes from a pane/header (title + params). The read side is diff --git a/lib/src/stories/BrowserChromeHeader.stories.tsx b/lib/src/stories/BrowserChromeHeader.stories.tsx index 4f8ac62e..ba3f4ebb 100644 --- a/lib/src/stories/BrowserChromeHeader.stories.tsx +++ b/lib/src/stories/BrowserChromeHeader.stories.tsx @@ -47,6 +47,11 @@ const loggingActions: WallActions = { onFinishRename: () => ({ accepted: true }), onCancelRename: () => {}, onSwapRenderMode: (id, mode) => console.log('[story] swap render', id, mode), + resolveSurfaceRef: (id) => id, + onConnectPort: async (id, url) => { + console.log('[story] connect port', id, url); + return { ok: true }; + }, }; interface StoryArgs { diff --git a/lib/src/stories/MouseHeaderIcon.stories.tsx b/lib/src/stories/MouseHeaderIcon.stories.tsx index 1d15a7d6..d2d6ac5b 100644 --- a/lib/src/stories/MouseHeaderIcon.stories.tsx +++ b/lib/src/stories/MouseHeaderIcon.stories.tsx @@ -33,6 +33,8 @@ const noopActions: WallActions = { onFinishRename: () => ({ accepted: true }), onCancelRename: () => {}, onSwapRenderMode: () => {}, + resolveSurfaceRef: (id) => id, + onConnectPort: async () => ({ ok: true }), }; function MouseIconStoryFrame({ diff --git a/lib/src/stories/ShellCwd.stories.tsx b/lib/src/stories/ShellCwd.stories.tsx index 8bfc8a90..334f765a 100644 --- a/lib/src/stories/ShellCwd.stories.tsx +++ b/lib/src/stories/ShellCwd.stories.tsx @@ -53,6 +53,8 @@ const noopActions: WallActions = { onFinishRename: () => ({ accepted: true }), onCancelRename: () => {}, onSwapRenderMode: () => {}, + resolveSurfaceRef: (id) => id, + onConnectPort: async () => ({ ok: true }), }; const meta: Meta = { diff --git a/lib/src/stories/TerminalPaneHeader.stories.tsx b/lib/src/stories/TerminalPaneHeader.stories.tsx index c3b57e5d..23876508 100644 --- a/lib/src/stories/TerminalPaneHeader.stories.tsx +++ b/lib/src/stories/TerminalPaneHeader.stories.tsx @@ -29,6 +29,8 @@ const noopActions: WallActions = { onFinishRename: () => ({ accepted: true }), onCancelRename: () => {}, onSwapRenderMode: () => {}, + resolveSurfaceRef: (id) => id, + onConnectPort: async () => ({ ok: true }), }; function actionsRejecting(reason: 'empty' | 'reserved'): WallActions { From e0230c206aeb304076e4fb5a41a874546c95c48d Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 10 Jul 2026 14:27:10 -0700 Subject: [PATCH 02/13] /simplify: bind connectPort inside useDorControl, thread the lazy reference through - useDorControl returns a bound connectPort(id, url) instead of exposing ensureAgentBrowserSurface for Wall.tsx to reassemble; Wall delegates in one line like its sibling actions - connectPortToDefaultBrowser takes the reference as a lazy thunk, so the reuse path survives a pane vanishing between right-click and connect - wall-context reuses ConnectPortResult instead of restating it inline - hoist the duplicated refreshedParams guard; drop an eta-expansion Co-Authored-By: Claude Fable 5 --- lib/src/components/Wall.tsx | 21 +++------- lib/src/components/wall/connect-port.test.ts | 12 +++--- lib/src/components/wall/connect-port.ts | 12 +++--- lib/src/components/wall/use-dor-control.ts | 40 +++++++++++++++----- lib/src/components/wall/wall-context.tsx | 3 +- 5 files changed, 50 insertions(+), 38 deletions(-) diff --git a/lib/src/components/Wall.tsx b/lib/src/components/Wall.tsx index 74b00b53..c75cd747 100644 --- a/lib/src/components/Wall.tsx +++ b/lib/src/components/Wall.tsx @@ -71,7 +71,6 @@ import { useWallKeyboard } from './wall/use-wall-keyboard'; import { useSessionPersistence } from './wall/use-session-persistence'; import { useDevServerPortCorrelation } from './wall/use-dev-server-ports'; import { useDorControl } from './wall/use-dor-control'; -import { connectPortToDefaultBrowser } from './wall/connect-port'; import { useWindowFocused } from './wall/use-window-focused'; import { DialogKeyboardContext, @@ -1107,7 +1106,7 @@ export function Wall({ }, [generatePaneId, surfaceRefForId, forgetSurfaceRef, selectPane, showShellSpawnNotice, lath, nav]); // --- dor control plane (the `dor` CLI's webview handler) --- - const { ensureAgentBrowserSurface } = useDorControl({ + const { connectPort } = useDorControl({ lath, nav, doorsRef, @@ -1286,20 +1285,10 @@ export function Wall({ title: hostPathDisplay(url, true), }); }, - resolveSurfaceRef: (id) => surfaceRefForId(id), - onConnectPort: async (id, url) => { - // The pane context menu's "connect a port" action: act like `dor ab open`. - const reference = buildDorSurfaces().find((s) => s.id === id); - if (!reference) return { ok: false, message: 'pane is gone' }; - return connectPortToDefaultBrowser({ - url, - reference, - platform: getPlatform(), - binaryPath: lastAgentBrowserBinaryPathRef.current, - ensureSurface: ensureAgentBrowserSurface, - }); - }, - }), [addSplitPanel, minimizePane, enterTerminalMode, exitTerminalMode, killPaneImmediately, replaceSurface, buildDorSurfaces, createContentSurface, surfaceRefForId, ensureAgentBrowserSurface, lath, nav]); + resolveSurfaceRef: surfaceRefForId, + // The pane context menu's "connect a port" action: act like `dor ab open`. + onConnectPort: connectPort, + }), [addSplitPanel, minimizePane, enterTerminalMode, exitTerminalMode, killPaneImmediately, replaceSurface, buildDorSurfaces, createContentSurface, surfaceRefForId, connectPort, lath, nav]); const wallActionsRef = useRef(wallActions); wallActionsRef.current = wallActions; diff --git a/lib/src/components/wall/connect-port.test.ts b/lib/src/components/wall/connect-port.test.ts index 7130ee7a..bdd6173f 100644 --- a/lib/src/components/wall/connect-port.test.ts +++ b/lib/src/components/wall/connect-port.test.ts @@ -6,7 +6,9 @@ import type { Surface as DorSurface } from 'dor/commands/types'; const URL = 'http://localhost:5173/'; const SESSION = sessionForKey('default'); -const reference = { id: 'pane-1', ref: 'surface:1' } as DorSurface; +const pane = { id: 'pane-1', ref: 'surface:1' } as DorSurface; +// The reference is handed through lazily (resolved only on the create path). +const reference = () => ({ ok: true as const, value: pane }); const okEnsure = (): EnsureAgentBrowserSurfaceResult => ({ ok: true, status: 'created', surfaceId: 'pane-2', surfaceRef: 'surface:2', minimized: false }); @@ -16,7 +18,7 @@ const okEnsure = (): EnsureAgentBrowserSurfaceResult => function ensureCallArgs(ensureSurface: ReturnType) { const [args] = ensureSurface.mock.calls.at(-1) ?? []; const { reference: lazyReference, ...rest } = args as { reference: () => { ok: boolean; value?: DorSurface } } & Record; - return { ...rest, reference: lazyReference().value }; + return { ...rest, pane: lazyReference().value }; } describe('connectPortToDefaultBrowser', () => { @@ -31,7 +33,7 @@ describe('connectPortToDefaultBrowser', () => { expect(result).toEqual({ ok: true }); expect(platform.agentBrowserCommand).toHaveBeenCalledWith(SESSION, ['open', URL], '/bin/ab'); expect(platform.agentBrowserStreamStatus).toHaveBeenCalledWith(SESSION, '/bin/ab'); - expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: 4321, binaryPath: '/bin/ab', reference }); + expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: 4321, binaryPath: '/bin/ab', pane }); }); it('fails when the host cannot run agent-browser', async () => { @@ -60,7 +62,7 @@ describe('connectPortToDefaultBrowser', () => { const platform = { agentBrowserCommand: vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })) }; const result = await connectPortToDefaultBrowser({ url: URL, reference, platform, ensureSurface }); expect(result).toEqual({ ok: true }); - expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: undefined, binaryPath: undefined, reference }); + expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: undefined, binaryPath: undefined, pane }); }); it('leaves wsPort undefined when stream status fails', async () => { @@ -70,7 +72,7 @@ describe('connectPortToDefaultBrowser', () => { agentBrowserStreamStatus: vi.fn(async () => ({ ok: false, error: 'nope' })), }; await connectPortToDefaultBrowser({ url: URL, reference, platform, binaryPath: '/bin/ab', ensureSurface }); - expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: undefined, binaryPath: '/bin/ab', reference }); + expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: undefined, binaryPath: '/bin/ab', pane }); }); it('propagates an ensureSurface failure', async () => { diff --git a/lib/src/components/wall/connect-port.ts b/lib/src/components/wall/connect-port.ts index da0f866c..f1391313 100644 --- a/lib/src/components/wall/connect-port.ts +++ b/lib/src/components/wall/connect-port.ts @@ -6,7 +6,7 @@ * `ensureSurface`) so it stays unit-testable off the React tree. */ import type { PlatformAdapter } from '../../lib/platform/types'; -import type { Surface as DorSurface } from 'dor/commands/types'; +import type { ParseResult, Surface as DorSurface } from 'dor/commands/types'; // The browser-safe subpath: dist/agent-browser.js is pure ES, so importing it // keeps cross-spawn (the package's Node-only default export) out of the webview. import { sessionForKey } from 'dor-lib-common/agent-browser'; @@ -26,8 +26,10 @@ export async function connectPortToDefaultBrowser({ ensureSurface, }: { url: string; - /** The pane the port belongs to — the split reference for a fresh surface. */ - reference: DorSurface; + /** The pane the port belongs to — the split reference for a fresh surface. + * Lazy for the same reason as `EnsureAgentBrowserSurface`'s: the reuse path + * must succeed even when the pane is no longer visible. */ + reference: () => ParseResult; platform: ConnectPlatform; /** Last binary path a `dor ab` surface resolved; undefined ⇒ host falls back * to PATH / DORMOUSE_AGENT_BROWSER_BIN. */ @@ -53,9 +55,7 @@ export async function connectPortToDefaultBrowser({ const status = await platform.agentBrowserStreamStatus(session, binaryPath); if (status.ok) wsPort = status.wsPort; } - // The menu resolved its pane eagerly (it's the visible pane under the cursor), - // so the lazy reference just hands it through. - const ensured = ensureSurface({ key: 'default', session, wsPort, binaryPath, reference: () => ({ ok: true, value: reference }) }); + const ensured = ensureSurface({ key: 'default', session, wsPort, binaryPath, reference }); if (!ensured.ok) return { ok: false, message: ensured.message }; return { ok: true }; } diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index 4fea8f8c..1aad2a01 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -20,6 +20,8 @@ import { import { surfaceRunsCommand, type TerminalPaneState } from '../../lib/terminal-state'; import { hostPathDisplay } from './browser-url'; import { isAgentBrowserParams } from './browser-surface'; +// Runtime import is one-way (connect-port's use-dor-control import is type-only). +import { connectPortToDefaultBrowser, type ConnectPortResult } from './connect-port'; import { listenerUrlsByPort } from './port-url'; import { dorDirectionForEdge, type LathWallEngine } from './lath-wall-engine'; import type { WallNav } from './keyboard/types'; @@ -368,7 +370,7 @@ export function useDorControl({ killPaneImmediately: (id: string) => void; /** The last binary path a `dor ab` surface resolved on a terminal's PATH. */ lastAgentBrowserBinaryPathRef: MutableRefObject; -}): { ensureAgentBrowserSurface: EnsureAgentBrowserSurface } { +}): { connectPort: (id: string, url: string) => Promise } { const resolveVisibleSurface = useCallback(( target: string | undefined, callerSurfaceId: string | undefined, @@ -459,14 +461,16 @@ export function useDorControl({ // Reuse: refresh the stream port (OS-assigned, churns across session // restarts) so the panel reconnects to the live stream, and the // resolved binary path alongside it. - if (!existing.minimized && Object.keys(refreshedParams).length > 0) { - lath.store.updateParams(existing.id, refreshedParams); - } else if (existing.minimized && Object.keys(refreshedParams).length > 0) { - const nextDoors = doorsRef.current.map((door) => door.id === existing.id - ? { ...door, params: { ...door.params, ...refreshedParams } } - : door); - doorsRef.current = nextDoors; - setDoors(nextDoors); + if (Object.keys(refreshedParams).length > 0) { + if (existing.minimized) { + const nextDoors = doorsRef.current.map((door) => door.id === existing.id + ? { ...door, params: { ...door.params, ...refreshedParams } } + : door); + doorsRef.current = nextDoors; + setDoors(nextDoors); + } else { + lath.store.updateParams(existing.id, refreshedParams); + } } return { ok: true, @@ -503,6 +507,22 @@ export function useDorControl({ }; }, [createContentSurface, findAgentBrowserSurface, lath, setDoors, surfaceRefForId]); + // The pane context menu's "connect a port" action, bound to this hook's + // closure so Wall.tsx delegates in one line instead of re-threading the + // hook's internals (`ensureAgentBrowserSurface`, the binary-path ref). + const connectPort = useCallback((id: string, url: string) => connectPortToDefaultBrowser({ + url, + // Lazy: the reuse path must succeed even if the pane vanished (e.g. was + // minimized) between the right-click and the connect resolving. + reference: () => { + const surface = buildDorSurfaces().find((candidate) => candidate.id === id); + return surface ? { ok: true, value: surface } : { ok: false, message: `surface for pane '${id}' was not found` }; + }, + platform: getPlatform(), + binaryPath: lastAgentBrowserBinaryPathRef.current, + ensureSurface: ensureAgentBrowserSurface, + }), [buildDorSurfaces, ensureAgentBrowserSurface, lastAgentBrowserBinaryPathRef]); + useEffect(() => { const handler = async (event: Event) => { const detail = (event as CustomEvent).detail; @@ -882,5 +902,5 @@ export function useDorControl({ return () => window.removeEventListener('dormouse:control-request', handler); }, [buildDorSurfaces, buildDorSurfaceList, createContentSurface, createSplitSurface, ensureAgentBrowserSurface, findSurfaceIdRunningCommand, killPaneImmediately, requireListedSurface, requireTerminalSurface, resolveListedSurface, resolveVisibleSurface, surfaceRefForId, lath, nav]); - return { ensureAgentBrowserSurface }; + return { connectPort }; } diff --git a/lib/src/components/wall/wall-context.tsx b/lib/src/components/wall/wall-context.tsx index 35800aa0..57a2f7da 100644 --- a/lib/src/components/wall/wall-context.tsx +++ b/lib/src/components/wall/wall-context.tsx @@ -2,6 +2,7 @@ import { createContext } from 'react'; import type { AlertButtonActionResult, SessionStatus, SetTerminalUserTitleResult } from '../../lib/terminal-registry'; import type { WallMode } from './wall-types'; import type { RenderMode } from './agent-browser-screen'; +import type { ConnectPortResult } from './connect-port'; export interface PaneElementsState { elements: Map; @@ -56,7 +57,7 @@ export interface WallActions { * the URL in the workspace's default agent-browser session and reuse-or-create * its browser surface (`connect-port.ts`). Resolves with a failure message the * menu can surface. */ - onConnectPort: (id: string, url: string) => Promise<{ ok: true } | { ok: false; message: string }>; + onConnectPort: (id: string, url: string) => Promise; } export const WallActionsContext = createContext({ From 9489c263d04d77881136910df30c46f2f41720ae Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 10 Jul 2026 14:46:02 -0700 Subject: [PATCH 03/13] Pane-header right-click menu: surface handle + bound ports, click to connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Right-clicking a terminal pane header's bare regions opens a context menu showing the pane's stable dor handle (surface:N) and the TCP ports its process tree binds — spinner while the scan runs, then one host:port row per distinct port (process name muted), or no-ports / scan-failed lines. Clicking a row reproduces `dor ab open `: the URL opens in the default agent-browser session and its browser surface is reused or created, focus-neutrally. Rows are inert labels on hosts that cannot run agent-browser. Title-span and bell right-clicks keep their own popovers. Verified live in the standalone agent-browser harness: menu shows surface:1 + localhost:8123 (Python), click splits in the screencast surface on dormouse.1.default, second connect reuses the same pane. Co-Authored-By: Claude Fable 5 --- docs/specs/dor-browser.md | 24 +++ docs/specs/layout.md | 5 +- .../wall/PaneHeaderContextMenu.test.tsx | 192 ++++++++++++++++++ .../components/wall/PaneHeaderContextMenu.tsx | 170 ++++++++++++++++ .../components/wall/TerminalPaneHeader.tsx | 19 ++ 5 files changed, 409 insertions(+), 1 deletion(-) create mode 100644 lib/src/components/wall/PaneHeaderContextMenu.test.tsx create mode 100644 lib/src/components/wall/PaneHeaderContextMenu.tsx diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 93ca588a..1a45983c 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -165,6 +165,28 @@ Source of truth: `lib/src/components/wall/use-dev-server-ports.ts`, `lib/src/components/wall/agent-browser-ports.ts`, `lib/src/components/wall/browser-url.ts`. +## Pane Context Menu Connect + +The terminal pane header's right-click menu (`docs/specs/layout.md` → Pane +header) lists the ports a pane's process tree binds using the **same** per-port +URL selection as `surface.resolveOpen` (`dor ab open ` / +`dor iframe `): `listenerUrlsByPort` in `port-url.ts` groups TCP +listeners into one openable `http://:/` per distinct port +(loopback-reachable bind wins `localhost`; otherwise the bound LAN/Tailnet +address, IPv6 bracketed). + +Clicking a port row reproduces `dor ab open ` against the **default** +key/session: it runs `agent-browser open ` on that session and +reuses-or-creates the session's browser surface, focus-neutrally (the caller +keeps focus) — the wall-side mirror of the CLI flow, not the control plane. It is +host-gated on `agentBrowserCommand`: absent (e.g. the web demo host), the rows +render as inert labels. + +Source of truth: `lib/src/components/wall/connect-port.ts` +(`connectPortToDefaultBrowser`), the `connectPort` binding in +`use-dor-control.ts` (reusing `ensureAgentBrowserSurface`), +`lib/src/components/wall/PaneHeaderContextMenu.tsx`. + ## Display Modal And Render Swaps The Display modal is the sole GUI for changing render mode and screencast @@ -511,6 +533,8 @@ Source of truth: `lib/src/lib/platform/types.ts`, - Shell/render swap/lifecycle: `lib/src/components/Wall.tsx`, `lib/src/components/wall/BrowserPanel.tsx`, `lib/src/components/wall/browser-surface.ts`. +- Pane context-menu connect: `lib/src/components/wall/PaneHeaderContextMenu.tsx`, + `lib/src/components/wall/connect-port.ts`, `lib/src/components/wall/port-url.ts`. - Chrome/modal: `SurfacePaneHeader.tsx`, `AgentBrowserScreenModal.tsx`, `agent-browser-screen.ts`, `browser-url.ts`. - Agent-browser renderer: `AgentBrowserPanel.tsx`, diff --git a/docs/specs/layout.md b/docs/specs/layout.md index d895e981..7e8bda2a 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -81,6 +81,8 @@ The header label is the `DerivedHeader` returned by `deriveHeader(paneState, vis The diagnostic popup lists the latest entry per `titleCandidates` channel as defined in `docs/specs/terminal-state.md`. Each row shows the channel, latest candidate text, and timestamp. The popup is diagnostic only; it does not change the title priority rules. +Right-clicking the header's bare regions opens a **context menu** at the pointer. The title span and the alert bell own their own right-clicks (both `stopPropagation`, opening the diagnostic popup and the alert dialog respectively), so those take precedence — the context menu only fires on the rest of the header. It is portaled to `document.body`, viewport-clamped, and dismissed like the diagnostic popup (outside `pointerdown`, `Escape`, `resize`, capture-phase `scroll`). It shows the pane's `surface:N` handle (`resolveSurfaceRef`) as a non-interactive monospace row, then the TCP ports the pane's process tree binds: a spinner while `getOpenPorts` runs (once per open — right-click again to rescan), then one `host:port` row per distinct port (the process name muted beside it), or a muted `no listening ports` / `port scan failed` line. Clicking a port row reproduces `dor ab open ` for that port (`docs/specs/dor-browser.md` → Pane Context Menu Connect); the rows are inert labels on hosts that cannot open a browser surface. Only terminal panes have this menu. Source of truth: `PaneHeaderContextMenu.tsx`, `TerminalPaneHeader.tsx`. + Elements from left to right: - Derived session label (click to rename/pin, right-click to inspect title candidates, truncates with ellipsis) @@ -375,7 +377,8 @@ The refill adopts the replacement (`selectPane`) only when the current selection | `lib/src/components/wall/wall-types.ts` / `wall-context.tsx` | Shared Wall types and React contexts used by Wall, pane headers, panels, overlays, and the baseboard | | `lib/src/components/wall/LathHost.tsx` | The tiling engine's HTML adapter: leaf divs, sashes, the pane/door drag gesture, and imperative animator frame application. Engine internals are mapped in `docs/specs/tiling-engine.md`. | | `lib/src/components/wall/TerminalPanel.tsx` | Pane body wrapper; registers the pane's DOM element (`usePaneChrome`) | -| `lib/src/components/wall/TerminalPaneHeader.tsx` | Pane header with rename, alert/TODO, mouse override, split/zoom/minimize/kill controls | +| `lib/src/components/wall/TerminalPaneHeader.tsx` | Pane header with rename, alert/TODO, mouse override, split/zoom/minimize/kill controls, and the right-click context menu | +| `lib/src/components/wall/PaneHeaderContextMenu.tsx` | Pane-header right-click menu: the `surface:N` handle plus the pane's bound TCP ports; a port click connects it to the default browser (`docs/specs/dor-browser.md`) | | `lib/src/components/wall/WorkspaceSelectionOverlay.tsx` | Pane/door focus ring and marching-ants overlay; re-measures on Lath store commits + animator frames | | `lib/src/components/wall/MarchingAntsRect.tsx` | SVG marching-ants border path and dash sizing | | `lib/src/components/wall/MouseOverrideBanner.tsx` | Temporary mouse override banner shown from the header icon | diff --git a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx new file mode 100644 index 00000000..6211f109 --- /dev/null +++ b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx @@ -0,0 +1,192 @@ +/** + * @vitest-environment jsdom + */ +import { act, StrictMode } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { PaneProps } from './pane-props'; +import { TerminalPaneHeader } from './TerminalPaneHeader'; +import { WallActionsContext, type WallActions } from './wall-context'; +import { FakePtyAdapter } from '../../lib/platform/fake-adapter'; +import { setPlatform } from '../../lib/platform'; +import type { OpenPort, PlatformAdapter } from '../../lib/platform/types'; + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +function stubActions(overrides: Partial = {}): WallActions { + return { + onKill: vi.fn(), + onMinimize: vi.fn(), + onAlertButton: vi.fn(() => 'noop'), + onToggleTodo: vi.fn(), + onSplitH: vi.fn(), + onSplitV: vi.fn(), + onZoom: vi.fn(), + onClickPanel: vi.fn(), + onFocusPane: vi.fn(), + onStartRename: vi.fn(), + onFinishRename: vi.fn(() => ({ accepted: true })), + onCancelRename: vi.fn(), + onSwapRenderMode: vi.fn(), + resolveSurfaceRef: vi.fn((id: string) => id), + onConnectPort: vi.fn(async () => ({ ok: true as const })), + ...overrides, + }; +} + +function headerProps(id: string, title: string): PaneProps { + return { id, title, params: undefined }; +} + +function loopbackPort(port: number, processName?: string): OpenPort { + return { protocol: 'tcp', family: 'IPv4', address: '127.0.0.1', port, pid: 100, ...(processName ? { processName } : {}) }; +} + +/** Make the running host able to open a browser surface, so port rows are buttons. */ +function enableConnect(platform: FakePtyAdapter): void { + (platform as PlatformAdapter).agentBrowserCommand = vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })); +} + +let container: HTMLDivElement; +let root: Root; +let platform: FakePtyAdapter; + +beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + platform = new FakePtyAdapter(); + setPlatform(platform); + // jsdom lacks ResizeObserver; the pane header's responsive tier observer needs it. + globalThis.ResizeObserver ??= class { + observe() {} + unobserve() {} + disconnect() {} + } as unknown as typeof ResizeObserver; +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); + platform.reset(); +}); + +function renderHeader(props: PaneProps, actions: WallActions) { + act(() => { + root.render( + + + + + , + ); + }); +} + +function fireContextMenu() { + const header = container.firstElementChild as HTMLElement; + header.dispatchEvent(new MouseEvent('contextmenu', { bubbles: true, clientX: 40, clientY: 12 })); +} + +function menuFor(id: string): HTMLElement | null { + return document.body.querySelector(`[data-pane-context-menu-for="${id}"]`); +} + +describe('PaneHeaderContextMenu — pane header right-click', () => { + it('opens on header contextmenu showing the surface ref from resolveSurfaceRef', async () => { + const resolveSurfaceRef = vi.fn(() => 'surface:3'); + renderHeader(headerProps('term-1', 'title'), stubActions({ resolveSurfaceRef })); + + await act(async () => { fireContextMenu(); }); + + const menu = menuFor('term-1'); + expect(menu).not.toBeNull(); + expect(menu?.textContent).toContain('surface:3'); + expect(resolveSurfaceRef).toHaveBeenCalledWith('term-1'); + }); + + it('shows a spinner while the scan is pending, then port entries after it resolves', async () => { + platform.spawnPty('term-1'); + let resolvePorts!: (ports: OpenPort[]) => void; + platform.getOpenPorts = () => new Promise((res) => { resolvePorts = res; }); + renderHeader(headerProps('term-1', 't'), stubActions()); + + act(() => { fireContextMenu(); }); + const menu = menuFor('term-1'); + expect(menu?.querySelector('.animate-spin')).not.toBeNull(); + expect(menu?.textContent).toContain('scanning ports'); + + await act(async () => { resolvePorts([loopbackPort(5173, 'node')]); }); + expect(menu?.querySelector('.animate-spin')).toBeNull(); + expect(menu?.textContent).toContain('localhost:5173'); + expect(menu?.textContent).toContain('node'); + }); + + it('connects a port (dor ab open) and closes the menu on success', async () => { + enableConnect(platform); + platform.spawnPty('term-1'); + platform.setOpenPorts('term-1', [loopbackPort(5173, 'node')]); + const onConnectPort = vi.fn(async () => ({ ok: true as const })); + renderHeader(headerProps('term-1', 't'), stubActions({ onConnectPort })); + + await act(async () => { fireContextMenu(); }); + const button = menuFor('term-1')?.querySelector('button[data-port-entry="5173"]') as HTMLButtonElement; + expect(button).not.toBeNull(); + + await act(async () => { button.dispatchEvent(new MouseEvent('click', { bubbles: true })); }); + + expect(onConnectPort).toHaveBeenCalledWith('term-1', 'http://localhost:5173/'); + expect(menuFor('term-1')).toBeNull(); + }); + + it('surfaces the failure message and keeps rows enabled when connect fails', async () => { + enableConnect(platform); + platform.spawnPty('term-1'); + platform.setOpenPorts('term-1', [loopbackPort(5173)]); + const onConnectPort = vi.fn(async () => ({ ok: false as const, message: 'agent-browser open exited 1' })); + renderHeader(headerProps('term-1', 't'), stubActions({ onConnectPort })); + + await act(async () => { fireContextMenu(); }); + const button = menuFor('term-1')?.querySelector('button[data-port-entry="5173"]') as HTMLButtonElement; + await act(async () => { button.dispatchEvent(new MouseEvent('click', { bubbles: true })); }); + + const menu = menuFor('term-1'); + expect(menu).not.toBeNull(); + expect(menu?.textContent).toContain('agent-browser open exited 1'); + expect((menu?.querySelector('button[data-port-entry="5173"]') as HTMLButtonElement).disabled).toBe(false); + }); + + it('shows an empty state when the scan finds no listening ports', async () => { + platform.spawnPty('term-1'); + renderHeader(headerProps('term-1', 't'), stubActions()); + + await act(async () => { fireContextMenu(); }); + expect(menuFor('term-1')?.textContent).toContain('no listening ports'); + }); + + it('renders port rows as inert labels (no buttons) when the host cannot connect', async () => { + platform.spawnPty('term-1'); + platform.setOpenPorts('term-1', [loopbackPort(5173, 'node')]); + renderHeader(headerProps('term-1', 't'), stubActions()); + + await act(async () => { fireContextMenu(); }); + const menu = menuFor('term-1'); + expect(menu?.querySelector('button[data-port-entry="5173"]')).toBeNull(); + expect(menu?.querySelector('[data-port-entry="5173"]')).not.toBeNull(); + expect(menu?.textContent).toContain('localhost:5173'); + }); + + it('dismisses on Escape and on an outside pointerdown', async () => { + renderHeader(headerProps('term-1', 't'), stubActions()); + + await act(async () => { fireContextMenu(); }); + expect(menuFor('term-1')).not.toBeNull(); + act(() => { window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); }); + expect(menuFor('term-1')).toBeNull(); + + await act(async () => { fireContextMenu(); }); + expect(menuFor('term-1')).not.toBeNull(); + act(() => { window.dispatchEvent(new Event('pointerdown')); }); + expect(menuFor('term-1')).toBeNull(); + }); +}); diff --git a/lib/src/components/wall/PaneHeaderContextMenu.tsx b/lib/src/components/wall/PaneHeaderContextMenu.tsx new file mode 100644 index 00000000..8499f6ad --- /dev/null +++ b/lib/src/components/wall/PaneHeaderContextMenu.tsx @@ -0,0 +1,170 @@ +import { useEffect, useLayoutEffect, useRef, useState, type CSSProperties } from 'react'; +import { createPortal } from 'react-dom'; +import { CircleNotchIcon } from '@phosphor-icons/react'; +import { clampOverlayPosition } from '../../lib/ui-geometry'; +import { getPlatform } from '../../lib/platform'; +import type { OpenPort } from '../../lib/platform/types'; +import { listenerUrlsByPort, type PortUrlEntry } from './port-url'; +import type { ConnectPortResult } from './connect-port'; + +type ScanState = + | { status: 'scanning' } + | { status: 'loaded'; entries: PortUrlEntry[] } + | { status: 'failed' }; + +/** + * The right-click menu on a terminal pane header (`docs/specs/layout.md` → Pane + * header): the pane's `surface:N` handle, then the TCP ports its process tree + * binds. Clicking a port reproduces `dor ab open ` via `onConnect` + * (`docs/specs/dor-browser.md` → Pane Context Menu Connect). Port rows are only + * clickable when the host can run agent-browser; otherwise the list is an inert + * label (the repo's host-gated-affordance convention). + */ +export function PaneHeaderContextMenu({ + id, + anchor, + surfaceRef, + onConnect, + onClose, +}: { + id: string; + anchor: { x: number; y: number }; + surfaceRef: string; + /** Pre-bound to the pane id: opens `url` in the default agent-browser session. */ + onConnect: (url: string) => Promise; + onClose: () => void; +}) { + const ref = useRef(null); + const aliveRef = useRef(true); + const [style, setStyle] = useState({ position: 'fixed', left: anchor.x, top: anchor.y }); + const [scan, setScan] = useState({ status: 'scanning' }); + const [connectingPort, setConnectingPort] = useState(null); + const [connectError, setConnectError] = useState(null); + + // Absent ⇒ opening a browser surface isn't supported here; port rows render as + // plain labels rather than buttons (the list stays informative). + const canConnect = !!getPlatform().agentBrowserCommand; + + useEffect(() => { + aliveRef.current = true; + return () => { aliveRef.current = false; }; + }, []); + + // Scan once when the menu opens — no polling, no rescan while open (right-click + // again to rescan). The scan may reject on timeout (`OPEN_PORT_TIMEOUT_MS`). + useEffect(() => { + let cancelled = false; + setScan({ status: 'scanning' }); + getPlatform().getOpenPorts(id).then( + (ports: OpenPort[]) => { if (!cancelled) setScan({ status: 'loaded', entries: listenerUrlsByPort(ports) }); }, + () => { if (!cancelled) setScan({ status: 'failed' }); }, + ); + return () => { cancelled = true; }; + }, [id]); + + // Clamp inside the viewport once measured; re-run when the content height + // changes (scanning → loaded, or an error line appears). + useLayoutEffect(() => { + const el = ref.current; + if (!el) return; + const rect = el.getBoundingClientRect(); + setStyle(clampOverlayPosition({ left: anchor.x, top: anchor.y, width: rect.width, height: rect.height })); + }, [anchor.x, anchor.y, scan, connectError]); + + // Dismissal: pointerdown outside (the menu stops its own), Escape, resize, scroll. + useEffect(() => { + const close = () => onClose(); + const closeOnKey = (event: KeyboardEvent) => { + if (event.key === 'Escape') { event.preventDefault(); onClose(); } + }; + window.addEventListener('pointerdown', close); + window.addEventListener('resize', close); + window.addEventListener('scroll', close, true); + window.addEventListener('keydown', closeOnKey); + return () => { + window.removeEventListener('pointerdown', close); + window.removeEventListener('resize', close); + window.removeEventListener('scroll', close, true); + window.removeEventListener('keydown', closeOnKey); + }; + }, [onClose]); + + const connect = async (entry: PortUrlEntry) => { + setConnectError(null); + setConnectingPort(entry.port); + const result = await onConnect(entry.url); + if (!aliveRef.current) return; + if (result.ok) { + onClose(); + } else { + setConnectError(result.message); + setConnectingPort(null); + } + }; + + const connecting = connectingPort !== null; + + return createPortal( +
e.stopPropagation()} + onMouseDown={(e) => e.stopPropagation()} + onContextMenu={(e) => e.preventDefault()} + > +
{surfaceRef}
+
+ {scan.status === 'scanning' && ( +
+ + scanning ports… +
+ )} + {scan.status === 'failed' && ( +
port scan failed
+ )} + {scan.status === 'loaded' && scan.entries.length === 0 && ( +
no listening ports
+ )} + {scan.status === 'loaded' && scan.entries.map((entry) => { + const label = ( + <> + {entry.host}:{entry.port} + {entry.processName && {entry.processName}} + + ); + return canConnect ? ( + + ) : ( +
+ {label} +
+ ); + })} + {connectError && ( + <> +
+
{connectError}
+ + )} +
, + document.body, + ); +} diff --git a/lib/src/components/wall/TerminalPaneHeader.tsx b/lib/src/components/wall/TerminalPaneHeader.tsx index 4f9856f2..1457d5d5 100644 --- a/lib/src/components/wall/TerminalPaneHeader.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.tsx @@ -19,6 +19,7 @@ import { bellIconClass } from '../bell-icon-class'; import { useTodoPillContent } from '../TodoPillBody'; import type { PaneProps } from './pane-props'; import { IllegalRenameWarning, type RenameRejection } from './IllegalRenameWarning'; +import { PaneHeaderContextMenu } from './PaneHeaderContextMenu'; import { DEFAULT_MOUSE_SELECTION_STATE, getMouseSelectionSnapshot, @@ -129,6 +130,7 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { const [dialogTriggerRect, setDialogTriggerRect] = useState(null); const [todoPreviewRect, setTodoPreviewRect] = useState(null); const [titleCandidatesRect, setTitleCandidatesRect] = useState(null); + const [contextMenu, setContextMenu] = useState<{ x: number; y: number; surfaceRef: string } | null>(null); const [renameWarning, setRenameWarning] = useState<{ rect: DOMRect; reason: RenameRejection; value: string } | null>(null); const todoPill = useTodoPillContent(activity.todo); const titleCandidates = useMemo(() => titleCandidatesForDisplay(paneState), [paneState]); @@ -145,6 +147,7 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { const closeDialog = useCallback(() => setDialogTriggerRect(null), []); const closeTodoPreview = useCallback(() => setTodoPreviewRect(null), []); const closeTitleCandidates = useCallback(() => setTitleCandidatesRect(null), []); + const closeContextMenu = useCallback(() => setContextMenu(null), []); const closeRenameWarning = useCallback(() => setRenameWarning(null), []); const submitRename = useCallback((value: string, anchor: HTMLElement) => { const rect = anchor.getBoundingClientRect(); @@ -208,6 +211,13 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { ref={tabRef} className={tabVariant({ state: isActiveHeader ? 'active' : 'inactive' })} onMouseDown={() => actions.onClickPanel(id)} + onContextMenu={(e) => { + // The title span and bell button both stopPropagation their own + // right-clicks, so this only fires on the header's bare regions. + e.preventDefault(); + e.stopPropagation(); + setContextMenu({ x: e.clientX, y: e.clientY, surfaceRef: actions.resolveSurfaceRef(id) }); + }} >
{isRenaming ? ( @@ -406,6 +416,15 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { onClose={closeTitleCandidates} /> )} + {contextMenu && ( + actions.onConnectPort(id, url)} + onClose={closeContextMenu} + /> + )} {renameWarning && ( Date: Fri, 10 Jul 2026 14:59:20 -0700 Subject: [PATCH 04/13] /simplify: shared popover dismissal + chrome, context-fed menu, test-stub dedupe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - useDismissOverlay: one dismissal contract (outside pointerdown, Escape, resize, capture scroll) replacing three verbatim copies in the context menu, TitleCandidatesPopover (now self-owned, matching the menu's placement), and IllegalRenameWarning (auto-dismiss timer stays local) - POPUP_SURFACE_CLASS in design.tsx replaces four copies of the anchored popover chrome recipe - PaneHeaderContextMenu reads WallActions from context: props shrink to {id, anchor, onClose}, the surfaceRef snapshot and pre-bound onConnect disappear; drop the no-op aliveRef and a redundant scanning reset - stubWallActions/ensureResizeObserver in wall-test-utils.ts replace four byte-identical stubActions copies (excluded from the app tsconfig — it imports vitest) Co-Authored-By: Claude Fable 5 --- lib/src/components/design.tsx | 5 ++ .../wall/AgentBrowserPanel.test.tsx | 21 +------- lib/src/components/wall/IframePanel.test.tsx | 22 +------- .../components/wall/IllegalRenameWarning.tsx | 24 +++------ .../wall/PaneHeaderContextMenu.test.tsx | 31 ++--------- .../components/wall/PaneHeaderContextMenu.tsx | 52 +++++-------------- .../wall/SurfacePaneHeader.test.tsx | 22 +------- .../components/wall/TerminalPaneHeader.tsx | 40 ++++---------- .../components/wall/use-dismiss-overlay.ts | 26 ++++++++++ lib/src/components/wall/wall-test-utils.ts | 34 ++++++++++++ lib/tsconfig.app.json | 3 +- 11 files changed, 102 insertions(+), 178 deletions(-) create mode 100644 lib/src/components/wall/use-dismiss-overlay.ts create mode 100644 lib/src/components/wall/wall-test-utils.ts diff --git a/lib/src/components/design.tsx b/lib/src/components/design.tsx index e5576908..b32f8fe0 100644 --- a/lib/src/components/design.tsx +++ b/lib/src/components/design.tsx @@ -24,6 +24,11 @@ export const DOOR_SELECTION_BORDER_RADIUS = `${TERMINAL_BORDER_RADIUS_REM}rem ${ // tiny label legible. Shared so both pill sites stay in sync. export const TODO_PILL_TRACKING_CLASS = 'tracking-[0.08em]'; +// Chrome for small anchored popovers (title candidates, TODO preview, pane +// context menu, rename warning). Text size and padding vary per popover and +// stay at the call site; the surface recipe is shared so they can't drift. +export const POPUP_SURFACE_CLASS = 'z-[1000] rounded border border-border bg-surface-raised font-mono text-foreground shadow-md'; + export function PopupButtonRow({ className, ...props diff --git a/lib/src/components/wall/AgentBrowserPanel.test.tsx b/lib/src/components/wall/AgentBrowserPanel.test.tsx index 9aee7801..030c29a4 100644 --- a/lib/src/components/wall/AgentBrowserPanel.test.tsx +++ b/lib/src/components/wall/AgentBrowserPanel.test.tsx @@ -11,6 +11,7 @@ import { AgentBrowserPanel, HIDDEN_PARK_DELAY_MS } from './AgentBrowserPanel'; import { getAgentBrowserScreenController } from './agent-browser-screen'; import { disposeAllAgentBrowserSurfaceControllers } from './agent-browser-surface-controller'; import { ModeContext, PaneWriteContext, SelectedIdContext, WallActionsContext, type PaneWriteActions, type WallActions } from './wall-context'; +import { stubWallActions as stubActions } from './wall-test-utils'; globalThis.IS_REACT_ACT_ENVIRONMENT = true; @@ -31,26 +32,6 @@ class ResizeObserverMock { disconnect() {} } -function stubActions(overrides: Partial = {}): WallActions { - return { - onKill: vi.fn(), - onMinimize: vi.fn(), - onAlertButton: vi.fn(() => 'noop'), - onToggleTodo: vi.fn(), - onSplitH: vi.fn(), - onSplitV: vi.fn(), - onZoom: vi.fn(), - onClickPanel: vi.fn(), - onFocusPane: vi.fn(), - onStartRename: vi.fn(), - onFinishRename: vi.fn(() => ({ accepted: true })), - onCancelRename: vi.fn(), - onSwapRenderMode: vi.fn(), - resolveSurfaceRef: vi.fn((id: string) => id), - onConnectPort: vi.fn(async () => ({ ok: true as const })), - ...overrides, - }; -} function paneProps(id: string, params: TestPanelParams = DEFAULT_PARAMS): PaneProps { return { id, title: 'Browser', params }; diff --git a/lib/src/components/wall/IframePanel.test.tsx b/lib/src/components/wall/IframePanel.test.tsx index c3e039ba..cd13be04 100644 --- a/lib/src/components/wall/IframePanel.test.tsx +++ b/lib/src/components/wall/IframePanel.test.tsx @@ -10,30 +10,10 @@ import type { PaneProps } from './pane-props'; import { IframePanel } from './IframePanel'; import { getAgentBrowserScreenController } from './agent-browser-screen'; import { PaneWriteContext, WallActionsContext, type PaneWriteActions, type WallActions } from './wall-context'; +import { stubWallActions as stubActions } from './wall-test-utils'; globalThis.IS_REACT_ACT_ENVIRONMENT = true; -function stubActions(overrides: Partial = {}): WallActions { - return { - onKill: vi.fn(), - onMinimize: vi.fn(), - onAlertButton: vi.fn(() => 'noop'), - onToggleTodo: vi.fn(), - onSplitH: vi.fn(), - onSplitV: vi.fn(), - onZoom: vi.fn(), - onClickPanel: vi.fn(), - onFocusPane: vi.fn(), - onStartRename: vi.fn(), - onFinishRename: vi.fn(() => ({ accepted: true })), - onCancelRename: vi.fn(), - onSwapRenderMode: vi.fn(), - resolveSurfaceRef: vi.fn((id: string) => id), - onConnectPort: vi.fn(async () => ({ ok: true as const })), - ...overrides, - }; -} - function paneProps(id: string): PaneProps { return { id, title: 'Raw iframe', params: { url: 'http://example.test/app' } }; } diff --git a/lib/src/components/wall/IllegalRenameWarning.tsx b/lib/src/components/wall/IllegalRenameWarning.tsx index b42f6d0d..c5fc7833 100644 --- a/lib/src/components/wall/IllegalRenameWarning.tsx +++ b/lib/src/components/wall/IllegalRenameWarning.tsx @@ -1,6 +1,8 @@ import { useEffect, useLayoutEffect, useRef, useState, type CSSProperties } from 'react'; import { createPortal } from 'react-dom'; +import { POPUP_SURFACE_CLASS } from '../design'; import type { SetTerminalUserTitleResult } from '../../lib/terminal-registry'; +import { useDismissOverlay } from './use-dismiss-overlay'; export type RenameRejection = Extract['reason']; @@ -36,23 +38,11 @@ export function IllegalRenameWarning({ anchorRect, reason, attemptedValue, onClo }); }, [anchorRect]); + useDismissOverlay(onClose); + useEffect(() => { - const dismiss = () => onClose(); - const onKey = (event: KeyboardEvent) => { - if (event.key === 'Escape') dismiss(); - }; - const timeout = window.setTimeout(dismiss, AUTO_DISMISS_MS); - window.addEventListener('pointerdown', dismiss); - window.addEventListener('resize', dismiss); - window.addEventListener('scroll', dismiss, true); - window.addEventListener('keydown', onKey); - return () => { - window.clearTimeout(timeout); - window.removeEventListener('pointerdown', dismiss); - window.removeEventListener('resize', dismiss); - window.removeEventListener('scroll', dismiss, true); - window.removeEventListener('keydown', onKey); - }; + const timeout = window.setTimeout(onClose, AUTO_DISMISS_MS); + return () => window.clearTimeout(timeout); }, [onClose]); return createPortal( @@ -61,7 +51,7 @@ export function IllegalRenameWarning({ anchorRect, reason, attemptedValue, onClo role="alert" data-testid="illegal-rename-warning" aria-label={`Illegal name: ${describeReason(reason, attemptedValue)}`} - className="z-[1000] max-w-72 rounded border border-border bg-surface-raised px-2.5 py-1.5 font-mono text-xs leading-snug text-foreground shadow-md" + className={`${POPUP_SURFACE_CLASS} max-w-72 px-2.5 py-1.5 text-xs leading-snug`} style={style} onPointerDown={(e) => e.stopPropagation()} > diff --git a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx index 6211f109..5a3308dc 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx @@ -7,39 +7,19 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { PaneProps } from './pane-props'; import { TerminalPaneHeader } from './TerminalPaneHeader'; import { WallActionsContext, type WallActions } from './wall-context'; +import { ensureResizeObserver, stubWallActions as stubActions } from './wall-test-utils'; import { FakePtyAdapter } from '../../lib/platform/fake-adapter'; import { setPlatform } from '../../lib/platform'; import type { OpenPort, PlatformAdapter } from '../../lib/platform/types'; globalThis.IS_REACT_ACT_ENVIRONMENT = true; -function stubActions(overrides: Partial = {}): WallActions { - return { - onKill: vi.fn(), - onMinimize: vi.fn(), - onAlertButton: vi.fn(() => 'noop'), - onToggleTodo: vi.fn(), - onSplitH: vi.fn(), - onSplitV: vi.fn(), - onZoom: vi.fn(), - onClickPanel: vi.fn(), - onFocusPane: vi.fn(), - onStartRename: vi.fn(), - onFinishRename: vi.fn(() => ({ accepted: true })), - onCancelRename: vi.fn(), - onSwapRenderMode: vi.fn(), - resolveSurfaceRef: vi.fn((id: string) => id), - onConnectPort: vi.fn(async () => ({ ok: true as const })), - ...overrides, - }; -} - function headerProps(id: string, title: string): PaneProps { return { id, title, params: undefined }; } function loopbackPort(port: number, processName?: string): OpenPort { - return { protocol: 'tcp', family: 'IPv4', address: '127.0.0.1', port, pid: 100, ...(processName ? { processName } : {}) }; + return { protocol: 'tcp', family: 'IPv4', address: '127.0.0.1', port, pid: 100, processName }; } /** Make the running host able to open a browser surface, so port rows are buttons. */ @@ -57,12 +37,7 @@ beforeEach(() => { root = createRoot(container); platform = new FakePtyAdapter(); setPlatform(platform); - // jsdom lacks ResizeObserver; the pane header's responsive tier observer needs it. - globalThis.ResizeObserver ??= class { - observe() {} - unobserve() {} - disconnect() {} - } as unknown as typeof ResizeObserver; + ensureResizeObserver(); }); afterEach(() => { diff --git a/lib/src/components/wall/PaneHeaderContextMenu.tsx b/lib/src/components/wall/PaneHeaderContextMenu.tsx index 8499f6ad..44488288 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.tsx @@ -1,11 +1,13 @@ -import { useEffect, useLayoutEffect, useRef, useState, type CSSProperties } from 'react'; +import { useContext, useEffect, useLayoutEffect, useRef, useState, type CSSProperties } from 'react'; import { createPortal } from 'react-dom'; import { CircleNotchIcon } from '@phosphor-icons/react'; +import { POPUP_SURFACE_CLASS } from '../design'; import { clampOverlayPosition } from '../../lib/ui-geometry'; import { getPlatform } from '../../lib/platform'; import type { OpenPort } from '../../lib/platform/types'; import { listenerUrlsByPort, type PortUrlEntry } from './port-url'; -import type { ConnectPortResult } from './connect-port'; +import { useDismissOverlay } from './use-dismiss-overlay'; +import { WallActionsContext } from './wall-context'; type ScanState = | { status: 'scanning' } @@ -15,27 +17,22 @@ type ScanState = /** * The right-click menu on a terminal pane header (`docs/specs/layout.md` → Pane * header): the pane's `surface:N` handle, then the TCP ports its process tree - * binds. Clicking a port reproduces `dor ab open ` via `onConnect` - * (`docs/specs/dor-browser.md` → Pane Context Menu Connect). Port rows are only - * clickable when the host can run agent-browser; otherwise the list is an inert - * label (the repo's host-gated-affordance convention). + * binds. Clicking a port reproduces `dor ab open ` via + * `WallActions.onConnectPort` (`docs/specs/dor-browser.md` → Pane Context Menu + * Connect). Port rows are only clickable when the host can run agent-browser; + * otherwise the list is an inert label (the host-gated-affordance convention). */ export function PaneHeaderContextMenu({ id, anchor, - surfaceRef, - onConnect, onClose, }: { id: string; anchor: { x: number; y: number }; - surfaceRef: string; - /** Pre-bound to the pane id: opens `url` in the default agent-browser session. */ - onConnect: (url: string) => Promise; onClose: () => void; }) { + const actions = useContext(WallActionsContext); const ref = useRef(null); - const aliveRef = useRef(true); const [style, setStyle] = useState({ position: 'fixed', left: anchor.x, top: anchor.y }); const [scan, setScan] = useState({ status: 'scanning' }); const [connectingPort, setConnectingPort] = useState(null); @@ -45,16 +42,10 @@ export function PaneHeaderContextMenu({ // plain labels rather than buttons (the list stays informative). const canConnect = !!getPlatform().agentBrowserCommand; - useEffect(() => { - aliveRef.current = true; - return () => { aliveRef.current = false; }; - }, []); - // Scan once when the menu opens — no polling, no rescan while open (right-click // again to rescan). The scan may reject on timeout (`OPEN_PORT_TIMEOUT_MS`). useEffect(() => { let cancelled = false; - setScan({ status: 'scanning' }); getPlatform().getOpenPorts(id).then( (ports: OpenPort[]) => { if (!cancelled) setScan({ status: 'loaded', entries: listenerUrlsByPort(ports) }); }, () => { if (!cancelled) setScan({ status: 'failed' }); }, @@ -71,29 +62,12 @@ export function PaneHeaderContextMenu({ setStyle(clampOverlayPosition({ left: anchor.x, top: anchor.y, width: rect.width, height: rect.height })); }, [anchor.x, anchor.y, scan, connectError]); - // Dismissal: pointerdown outside (the menu stops its own), Escape, resize, scroll. - useEffect(() => { - const close = () => onClose(); - const closeOnKey = (event: KeyboardEvent) => { - if (event.key === 'Escape') { event.preventDefault(); onClose(); } - }; - window.addEventListener('pointerdown', close); - window.addEventListener('resize', close); - window.addEventListener('scroll', close, true); - window.addEventListener('keydown', closeOnKey); - return () => { - window.removeEventListener('pointerdown', close); - window.removeEventListener('resize', close); - window.removeEventListener('scroll', close, true); - window.removeEventListener('keydown', closeOnKey); - }; - }, [onClose]); + useDismissOverlay(onClose); const connect = async (entry: PortUrlEntry) => { setConnectError(null); setConnectingPort(entry.port); - const result = await onConnect(entry.url); - if (!aliveRef.current) return; + const result = await actions.onConnectPort(id, entry.url); if (result.ok) { onClose(); } else { @@ -110,13 +84,13 @@ export function PaneHeaderContextMenu({ role="menu" aria-label="Pane actions" data-pane-context-menu-for={id} - className="z-[1000] max-h-[70vh] w-fit min-w-52 max-w-96 overflow-auto rounded border border-border bg-surface-raised py-1 font-mono text-sm text-foreground shadow-md" + className={`${POPUP_SURFACE_CLASS} max-h-[70vh] w-fit min-w-52 max-w-96 overflow-auto py-1 text-sm`} style={style} onPointerDown={(e) => e.stopPropagation()} onMouseDown={(e) => e.stopPropagation()} onContextMenu={(e) => e.preventDefault()} > -
{surfaceRef}
+
{actions.resolveSurfaceRef(id)}
{scan.status === 'scanning' && (
diff --git a/lib/src/components/wall/SurfacePaneHeader.test.tsx b/lib/src/components/wall/SurfacePaneHeader.test.tsx index 5764804f..1b555a97 100644 --- a/lib/src/components/wall/SurfacePaneHeader.test.tsx +++ b/lib/src/components/wall/SurfacePaneHeader.test.tsx @@ -13,6 +13,7 @@ import { } from './agent-browser-screen'; import { setDevServerResolution } from './agent-browser-ports'; import { WallActionsContext, type WallActions } from './wall-context'; +import { stubWallActions as stubActions } from './wall-test-utils'; globalThis.IS_REACT_ACT_ENVIRONMENT = true; @@ -31,27 +32,6 @@ const CHROME: ChromeSnapshot = { key: 'storybook', }; -function stubActions(overrides: Partial = {}): WallActions { - return { - onKill: vi.fn(), - onMinimize: vi.fn(), - onAlertButton: vi.fn(() => 'noop'), - onToggleTodo: vi.fn(), - onSplitH: vi.fn(), - onSplitV: vi.fn(), - onZoom: vi.fn(), - onClickPanel: vi.fn(), - onFocusPane: vi.fn(), - onStartRename: vi.fn(), - onFinishRename: vi.fn(() => ({ accepted: true })), - onCancelRename: vi.fn(), - onSwapRenderMode: vi.fn(), - resolveSurfaceRef: vi.fn((id: string) => id), - onConnectPort: vi.fn(async () => ({ ok: true as const })), - ...overrides, - }; -} - function register(id: string, chrome: ChromeSnapshot = CHROME) { return registerAgentBrowserScreen(id, { snapshot: SCREEN, diff --git a/lib/src/components/wall/TerminalPaneHeader.tsx b/lib/src/components/wall/TerminalPaneHeader.tsx index 1457d5d5..3601f439 100644 --- a/lib/src/components/wall/TerminalPaneHeader.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.tsx @@ -14,12 +14,13 @@ import { } from '@phosphor-icons/react'; import { HeaderActionButton } from '../HeaderActionButton'; import { TodoAlertDialog } from '../TodoAlertDialog'; -import { TERMINAL_TOP_RADIUS_CLASS, TODO_PILL_TRACKING_CLASS } from '../design'; +import { POPUP_SURFACE_CLASS, TERMINAL_TOP_RADIUS_CLASS, TODO_PILL_TRACKING_CLASS } from '../design'; import { bellIconClass } from '../bell-icon-class'; import { useTodoPillContent } from '../TodoPillBody'; import type { PaneProps } from './pane-props'; import { IllegalRenameWarning, type RenameRejection } from './IllegalRenameWarning'; import { PaneHeaderContextMenu } from './PaneHeaderContextMenu'; +import { useDismissOverlay } from './use-dismiss-overlay'; import { DEFAULT_MOUSE_SELECTION_STATE, getMouseSelectionSnapshot, @@ -130,7 +131,7 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { const [dialogTriggerRect, setDialogTriggerRect] = useState(null); const [todoPreviewRect, setTodoPreviewRect] = useState(null); const [titleCandidatesRect, setTitleCandidatesRect] = useState(null); - const [contextMenu, setContextMenu] = useState<{ x: number; y: number; surfaceRef: string } | null>(null); + const [contextMenu, setContextMenu] = useState<{ x: number; y: number } | null>(null); const [renameWarning, setRenameWarning] = useState<{ rect: DOMRect; reason: RenameRejection; value: string } | null>(null); const todoPill = useTodoPillContent(activity.todo); const titleCandidates = useMemo(() => titleCandidatesForDisplay(paneState), [paneState]); @@ -187,25 +188,6 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { if (!activity.notification) setTodoPreviewRect(null); }, [activity.notification]); - const titleCandidatesOpen = !!titleCandidatesRect; - useEffect(() => { - if (!titleCandidatesOpen) return; - const close = () => setTitleCandidatesRect(null); - const closeOnKey = (event: KeyboardEvent) => { - if (event.key === 'Escape') close(); - }; - window.addEventListener('pointerdown', close); - window.addEventListener('resize', close); - window.addEventListener('scroll', close, true); - window.addEventListener('keydown', closeOnKey); - return () => { - window.removeEventListener('pointerdown', close); - window.removeEventListener('resize', close); - window.removeEventListener('scroll', close, true); - window.removeEventListener('keydown', closeOnKey); - }; - }, [titleCandidatesOpen]); - return (
@@ -417,13 +399,7 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { /> )} {contextMenu && ( - actions.onConnectPort(id, url)} - onClose={closeContextMenu} - /> + )} {renameWarning && ( { const el = ref.current; if (!el) return; @@ -474,7 +452,7 @@ function TitleCandidatesPopover({ ref={ref} role="dialog" aria-label="Title candidates" - className="z-[1000] max-w-96 overflow-auto rounded border border-border bg-surface-raised px-2.5 py-2 font-mono text-sm leading-snug text-foreground shadow-md" + className={`${POPUP_SURFACE_CLASS} max-w-96 overflow-auto px-2.5 py-2 text-sm leading-snug`} style={style} onPointerDown={(e) => e.stopPropagation()} onMouseDown={(e) => e.stopPropagation()} @@ -546,7 +524,7 @@ function TodoNotificationPreview({ ref={ref} id={id} role="tooltip" - className="z-[1000] max-w-80 rounded border border-border bg-surface-raised px-2.5 py-2 font-mono text-sm leading-snug text-foreground shadow-md" + className={`${POPUP_SURFACE_CLASS} max-w-80 px-2.5 py-2 text-sm leading-snug`} style={style} > {notification.title && ( diff --git a/lib/src/components/wall/use-dismiss-overlay.ts b/lib/src/components/wall/use-dismiss-overlay.ts new file mode 100644 index 00000000..afe1f05b --- /dev/null +++ b/lib/src/components/wall/use-dismiss-overlay.ts @@ -0,0 +1,26 @@ +import { useEffect } from 'react'; + +/** + * The pane-header popovers' shared dismissal contract (docs/specs/layout.md): + * any window `pointerdown` (an overlay that should survive its own clicks stops + * propagation on its root), Escape, `resize`, or capture-phase `scroll`. + * Register only while the overlay is mounted — consumers render only when open. + */ +export function useDismissOverlay(onClose: () => void): void { + useEffect(() => { + const close = () => onClose(); + const closeOnKey = (event: KeyboardEvent) => { + if (event.key === 'Escape') close(); + }; + window.addEventListener('pointerdown', close); + window.addEventListener('resize', close); + window.addEventListener('scroll', close, true); + window.addEventListener('keydown', closeOnKey); + return () => { + window.removeEventListener('pointerdown', close); + window.removeEventListener('resize', close); + window.removeEventListener('scroll', close, true); + window.removeEventListener('keydown', closeOnKey); + }; + }, [onClose]); +} diff --git a/lib/src/components/wall/wall-test-utils.ts b/lib/src/components/wall/wall-test-utils.ts new file mode 100644 index 00000000..6e779ab6 --- /dev/null +++ b/lib/src/components/wall/wall-test-utils.ts @@ -0,0 +1,34 @@ +import { vi } from 'vitest'; +import type { WallActions } from './wall-context'; + +/** The full `WallActions` surface as inert vi.fn stubs, so a new member is one + * edit here instead of one per component test. */ +export function stubWallActions(overrides: Partial = {}): WallActions { + return { + onKill: vi.fn(), + onMinimize: vi.fn(), + onAlertButton: vi.fn(() => 'noop'), + onToggleTodo: vi.fn(), + onSplitH: vi.fn(), + onSplitV: vi.fn(), + onZoom: vi.fn(), + onClickPanel: vi.fn(), + onFocusPane: vi.fn(), + onStartRename: vi.fn(), + onFinishRename: vi.fn(() => ({ accepted: true })), + onCancelRename: vi.fn(), + onSwapRenderMode: vi.fn(), + resolveSurfaceRef: vi.fn((id: string) => id), + onConnectPort: vi.fn(async () => ({ ok: true as const })), + ...overrides, + }; +} + +/** jsdom lacks ResizeObserver; the pane headers' responsive-tier observer needs it. */ +export function ensureResizeObserver(): void { + globalThis.ResizeObserver ??= class { + observe() {} + unobserve() {} + disconnect() {} + } as unknown as typeof ResizeObserver; +} diff --git a/lib/tsconfig.app.json b/lib/tsconfig.app.json index cf3a4c0b..2261411a 100644 --- a/lib/tsconfig.app.json +++ b/lib/tsconfig.app.json @@ -23,5 +23,6 @@ "include": ["src"], // The pure rewrite helpers typecheck here (DOM provides URL); the Node proxy // server is esbuild-only (bundled per host), like the rest of our host code. - "exclude": ["src/**/*.test.ts", "src/**/*.test.tsx", "src/host/iframe-proxy.ts", "src/host/agent-browser-host.ts"] + // wall-test-utils imports vitest, so it lives with the tests, not the app. + "exclude": ["src/**/*.test.ts", "src/**/*.test.tsx", "src/components/wall/wall-test-utils.ts", "src/host/iframe-proxy.ts", "src/host/agent-browser-host.ts"] } From 4fabe9481517d403670b2a9655edba390040f2cc Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 10 Jul 2026 17:24:42 -0700 Subject: [PATCH 05/13] One header menu + instant connect: session-less eager pane, background boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes two pieces of feedback on the pane-header context menu: - One right-click menu: the title span no longer owns its own right-click (only the alert bell keeps one); the diagnostic title-candidates popover is now a "title candidates" item at the bottom of the context menu, anchored to the title span. - Instant connect: clicking a port closes the menu and creates the browser pane in the same tick (~16ms measured, was 1-3s of daemon boot). The eager pane is deliberately created without `session` so the controller's stale-port recovery stays inert and cannot race the daemon boot; the panel shows "Connecting to browser session…" while the background `agent-browser open` + `stream status` run, then one params refresh hands it {session, wsPort, binaryPath} and the stream connects. Rapid re-clicks reuse the still-booting pane; failures log to the console. Verified live in the standalone harness: click→pane 16ms, menu closed same-tick, stream connected and navigated; title-span right-click opens the single menu and its item opens the candidates popover. Co-Authored-By: Claude Fable 5 --- docs/specs/dor-browser.md | 23 ++++- docs/specs/layout.md | 6 +- .../wall/PaneHeaderContextMenu.test.tsx | 46 +++++---- .../components/wall/PaneHeaderContextMenu.tsx | 58 ++++++----- .../components/wall/TerminalPaneHeader.tsx | 27 ++++-- lib/src/components/wall/connect-port.test.ts | 92 +++++++++--------- lib/src/components/wall/connect-port.ts | 48 +++++++--- lib/src/components/wall/use-dor-control.ts | 96 ++++++++++++++----- lib/src/components/wall/wall-context.tsx | 8 +- 9 files changed, 255 insertions(+), 149 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 1a45983c..c4de69bd 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -182,9 +182,30 @@ keeps focus) — the wall-side mirror of the CLI flow, not the control plane. It host-gated on `agentBrowserCommand`: absent (e.g. the web demo host), the rows render as inert labels. +**Instant create.** The click is fire-and-forget: the menu closes at once and the +pane appears **before** `agent-browser open` runs (a cold daemon boot is 1–3s). +The eager surface is created **without a `session`** — deliberately: a +session-less `ab-screencast` pane is inert (the controller's `maybeRecoverStalePort` +returns early with no session, so it spawns no CLI and cannot race the daemon +boot), while the panel still shows `Connecting to browser session…`. It carries +`key: 'default'` and the target `url` so the browser chrome shows the destination +immediately. Once `open` succeeds, a best-effort `stream status` runs, then the +pane receives `{session, wsPort, binaryPath}` as **one** params refresh — setting +`session` reconciles the controller and connects it (safe now: the daemon is up). +If `open` fails, the pane is still handed the `session` (so its placeholder names +it) and the failure is logged, not shown in the (already closed) menu. + +The eager lookup reuses before it creates: (a) a surface already bound to the +default session, else (b) a still-booting session-less `key: 'default'` pane from a +rapid earlier click (so a double-click doesn't spawn two panes), else (c) a fresh +session-less pane. Accepted edge: a pane persisted mid-boot restores session-less +and stays a `Connecting…` placeholder — kill it, or connect again (the eager +lookup reuses it via arm (b)). + Source of truth: `lib/src/components/wall/connect-port.ts` (`connectPortToDefaultBrowser`), the `connectPort` binding in -`use-dor-control.ts` (reusing `ensureAgentBrowserSurface`), +`use-dor-control.ts` (its `ensureEagerSurface` + `updateSurfaceParams` seams, +shared with `ensureAgentBrowserSurface`), `lib/src/components/wall/PaneHeaderContextMenu.tsx`. ## Display Modal And Render Swaps diff --git a/docs/specs/layout.md b/docs/specs/layout.md index 7e8bda2a..0dd95a17 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -77,15 +77,15 @@ The content area is a tiling layout of panes rendered by Lath (`docs/specs/tilin Each pane has a 30px header that doubles as a drag handle (a `pointerdown` on the header, past a 5px threshold, begins a Lath pane drag; below the threshold the header's own click behavior stands). The header uses `cursor-grab` / `active:cursor-grabbing`, `select-none`, and the shared terminal top radius from `lib/src/components/design.tsx`. Background and foreground use the `--color-header-active-*` / `--color-header-inactive-*` token pairs, which map to VSCode file-tree list colors. -The header label is the `DerivedHeader` returned by `deriveHeader(paneState, visiblePanes)` in `docs/specs/terminal-state.md` — that spec is the single source of truth for the priority chain (user-pinned title, app-sent overrides, current command title, ` ${LAST_TITLE}` for finished panes, plain `` for fresh panes), the disambiguator rule, and which OSC sources contribute. Layout's job is to render the result: the primary label truncates with ellipsis, the secondary label (when present) is shown muted next to it, click renames/pins, right-click opens the diagnostic popup. +The header label is the `DerivedHeader` returned by `deriveHeader(paneState, visiblePanes)` in `docs/specs/terminal-state.md` — that spec is the single source of truth for the priority chain (user-pinned title, app-sent overrides, current command title, ` ${LAST_TITLE}` for finished panes, plain `` for fresh panes), the disambiguator rule, and which OSC sources contribute. Layout's job is to render the result: the primary label truncates with ellipsis, the secondary label (when present) is shown muted next to it, click renames/pins, right-click opens the header context menu (which offers the diagnostic popup as a `title candidates` item). The diagnostic popup lists the latest entry per `titleCandidates` channel as defined in `docs/specs/terminal-state.md`. Each row shows the channel, latest candidate text, and timestamp. The popup is diagnostic only; it does not change the title priority rules. -Right-clicking the header's bare regions opens a **context menu** at the pointer. The title span and the alert bell own their own right-clicks (both `stopPropagation`, opening the diagnostic popup and the alert dialog respectively), so those take precedence — the context menu only fires on the rest of the header. It is portaled to `document.body`, viewport-clamped, and dismissed like the diagnostic popup (outside `pointerdown`, `Escape`, `resize`, capture-phase `scroll`). It shows the pane's `surface:N` handle (`resolveSurfaceRef`) as a non-interactive monospace row, then the TCP ports the pane's process tree binds: a spinner while `getOpenPorts` runs (once per open — right-click again to rescan), then one `host:port` row per distinct port (the process name muted beside it), or a muted `no listening ports` / `port scan failed` line. Clicking a port row reproduces `dor ab open ` for that port (`docs/specs/dor-browser.md` → Pane Context Menu Connect); the rows are inert labels on hosts that cannot open a browser surface. Only terminal panes have this menu. Source of truth: `PaneHeaderContextMenu.tsx`, `TerminalPaneHeader.tsx`. +Right-clicking anywhere on the header opens a single **context menu** at the pointer. Only the alert bell owns its own right-click (`stopPropagation`, opening the alert dialog); every other region — including the title span — bubbles to this one menu. It is portaled to `document.body`, viewport-clamped, and dismissed like the diagnostic popup (outside `pointerdown`, `Escape`, `resize`, capture-phase `scroll`). It shows the pane's `surface:N` handle (`resolveSurfaceRef`) as a non-interactive monospace row, then the TCP ports the pane's process tree binds: a spinner while `getOpenPorts` runs (once per open — right-click again to rescan), then one `host:port` row per distinct port (the process name muted beside it), or a muted `no listening ports` / `port scan failed` line, then a `title candidates` item that opens the diagnostic popup (anchored to the title span, or the pointer while renaming). Clicking a port row reproduces `dor ab open ` for that port and closes the menu at once (`docs/specs/dor-browser.md` → Pane Context Menu Connect): the browser surface appears immediately, and loading/errors surface in the pane, not the menu; the rows are inert labels on hosts that cannot open a browser surface. Only terminal panes have this menu. Source of truth: `PaneHeaderContextMenu.tsx`, `TerminalPaneHeader.tsx`. Elements from left to right: -- Derived session label (click to rename/pin, right-click to inspect title candidates, truncates with ellipsis) +- Derived session label (click to rename/pin, right-click opens the header context menu whose `title candidates` item inspects the candidates, truncates with ellipsis) - Alert bell button (reflects session activity status) - TODO pill (if todo state is set; hidden in minimal tier) - Flexible gap diff --git a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx index 5a3308dc..9ab6f007 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx @@ -97,7 +97,7 @@ describe('PaneHeaderContextMenu — pane header right-click', () => { expect(menu?.textContent).toContain('node'); }); - it('connects a port (dor ab open) and closes the menu on success', async () => { + it('fires the connect and closes the menu immediately (loading feedback lives in the pane)', async () => { enableConnect(platform); platform.spawnPty('term-1'); platform.setOpenPorts('term-1', [loopbackPort(5173, 'node')]); @@ -114,23 +114,6 @@ describe('PaneHeaderContextMenu — pane header right-click', () => { expect(menuFor('term-1')).toBeNull(); }); - it('surfaces the failure message and keeps rows enabled when connect fails', async () => { - enableConnect(platform); - platform.spawnPty('term-1'); - platform.setOpenPorts('term-1', [loopbackPort(5173)]); - const onConnectPort = vi.fn(async () => ({ ok: false as const, message: 'agent-browser open exited 1' })); - renderHeader(headerProps('term-1', 't'), stubActions({ onConnectPort })); - - await act(async () => { fireContextMenu(); }); - const button = menuFor('term-1')?.querySelector('button[data-port-entry="5173"]') as HTMLButtonElement; - await act(async () => { button.dispatchEvent(new MouseEvent('click', { bubbles: true })); }); - - const menu = menuFor('term-1'); - expect(menu).not.toBeNull(); - expect(menu?.textContent).toContain('agent-browser open exited 1'); - expect((menu?.querySelector('button[data-port-entry="5173"]') as HTMLButtonElement).disabled).toBe(false); - }); - it('shows an empty state when the scan finds no listening ports', async () => { platform.spawnPty('term-1'); renderHeader(headerProps('term-1', 't'), stubActions()); @@ -164,4 +147,31 @@ describe('PaneHeaderContextMenu — pane header right-click', () => { act(() => { window.dispatchEvent(new Event('pointerdown')); }); expect(menuFor('term-1')).toBeNull(); }); + + it('opens the one context menu on a title-span right-click (not a second popover)', async () => { + renderHeader(headerProps('term-1', 'title'), stubActions()); + + const titleSpan = container.querySelector('[data-title-candidates-for="term-1"]') as HTMLElement; + expect(titleSpan).not.toBeNull(); + await act(async () => { + titleSpan.dispatchEvent(new MouseEvent('contextmenu', { bubbles: true, clientX: 40, clientY: 12 })); + }); + + expect(menuFor('term-1')).not.toBeNull(); + // The title span no longer opens its own popover — only the context menu. + expect(document.body.querySelector('[aria-label="Title candidates"]')).toBeNull(); + }); + + it('opens the title-candidates popover from the menu item and closes the menu', async () => { + renderHeader(headerProps('term-1', 'title'), stubActions()); + + await act(async () => { fireContextMenu(); }); + const item = menuFor('term-1')?.querySelector('[data-title-candidates-item]') as HTMLButtonElement; + expect(item).not.toBeNull(); + + await act(async () => { item.dispatchEvent(new MouseEvent('click', { bubbles: true })); }); + + expect(menuFor('term-1')).toBeNull(); + expect(document.body.querySelector('[role="dialog"][aria-label="Title candidates"]')).not.toBeNull(); + }); }); diff --git a/lib/src/components/wall/PaneHeaderContextMenu.tsx b/lib/src/components/wall/PaneHeaderContextMenu.tsx index 44488288..66b842e9 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.tsx @@ -17,26 +17,29 @@ type ScanState = /** * The right-click menu on a terminal pane header (`docs/specs/layout.md` → Pane * header): the pane's `surface:N` handle, then the TCP ports its process tree - * binds. Clicking a port reproduces `dor ab open ` via - * `WallActions.onConnectPort` (`docs/specs/dor-browser.md` → Pane Context Menu - * Connect). Port rows are only clickable when the host can run agent-browser; - * otherwise the list is an inert label (the host-gated-affordance convention). + * binds, then a "title candidates" row that opens the diagnostic popover. + * Clicking a port fires `dor ab open ` via `WallActions.onConnectPort` + * (`docs/specs/dor-browser.md` → Pane Context Menu Connect) and closes the menu + * immediately — the new pane's own "Connecting…" placeholder is the loading + * feedback, and a failure is logged, not shown here. Port rows are only clickable + * when the host can run agent-browser; otherwise the list is an inert label (the + * host-gated-affordance convention). */ export function PaneHeaderContextMenu({ id, anchor, onClose, + onShowTitleCandidates, }: { id: string; anchor: { x: number; y: number }; onClose: () => void; + onShowTitleCandidates: () => void; }) { const actions = useContext(WallActionsContext); const ref = useRef(null); const [style, setStyle] = useState({ position: 'fixed', left: anchor.x, top: anchor.y }); const [scan, setScan] = useState({ status: 'scanning' }); - const [connectingPort, setConnectingPort] = useState(null); - const [connectError, setConnectError] = useState(null); // Absent ⇒ opening a browser surface isn't supported here; port rows render as // plain labels rather than buttons (the list stays informative). @@ -54,30 +57,23 @@ export function PaneHeaderContextMenu({ }, [id]); // Clamp inside the viewport once measured; re-run when the content height - // changes (scanning → loaded, or an error line appears). + // changes (scanning → loaded). useLayoutEffect(() => { const el = ref.current; if (!el) return; const rect = el.getBoundingClientRect(); setStyle(clampOverlayPosition({ left: anchor.x, top: anchor.y, width: rect.width, height: rect.height })); - }, [anchor.x, anchor.y, scan, connectError]); + }, [anchor.x, anchor.y, scan]); useDismissOverlay(onClose); - const connect = async (entry: PortUrlEntry) => { - setConnectError(null); - setConnectingPort(entry.port); - const result = await actions.onConnectPort(id, entry.url); - if (result.ok) { - onClose(); - } else { - setConnectError(result.message); - setConnectingPort(null); - } + // Fire-and-forget: the pane appears immediately and reports its own progress, so + // the menu closes at once rather than waiting on the daemon boot. + const connect = (entry: PortUrlEntry) => { + void actions.onConnectPort(id, entry.url); + onClose(); }; - const connecting = connectingPort !== null; - return createPortal(
connect(entry)} > {label} - {connectingPort === entry.port && ( - - )} ) : (
@@ -132,12 +124,16 @@ export function PaneHeaderContextMenu({
); })} - {connectError && ( - <> -
-
{connectError}
- - )} +
+
, document.body, ); diff --git a/lib/src/components/wall/TerminalPaneHeader.tsx b/lib/src/components/wall/TerminalPaneHeader.tsx index 3601f439..f26ac409 100644 --- a/lib/src/components/wall/TerminalPaneHeader.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.tsx @@ -126,6 +126,7 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { const isActiveHeader = mode === 'passthrough' && isSelected && windowFocused; const isRenaming = renamingId === id; const tabRef = useRef(null); + const titleSpanRef = useRef(null); const suppressAlertClickRef = useRef(false); const [tier, setTier] = useState('full'); const [dialogTriggerRect, setDialogTriggerRect] = useState(null); @@ -149,6 +150,14 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { const closeTodoPreview = useCallback(() => setTodoPreviewRect(null), []); const closeTitleCandidates = useCallback(() => setTitleCandidatesRect(null), []); const closeContextMenu = useCallback(() => setContextMenu(null), []); + // Reachable from the context menu now that the title span owns no right-click. + // Anchor to the span; fall back to the menu's pointer position when the span is + // absent (e.g. mid-rename, when an input replaces it). + const showTitleCandidates = useCallback(() => { + const rect = titleSpanRef.current?.getBoundingClientRect() + ?? (contextMenu ? new DOMRect(contextMenu.x, contextMenu.y, 0, 0) : null); + if (rect) setTitleCandidatesRect(rect); + }, [contextMenu]); const closeRenameWarning = useCallback(() => setRenameWarning(null), []); const submitRename = useCallback((value: string, anchor: HTMLElement) => { const rect = anchor.getBoundingClientRect(); @@ -194,8 +203,9 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { className={tabVariant({ state: isActiveHeader ? 'active' : 'inactive' })} onMouseDown={() => actions.onClickPanel(id)} onContextMenu={(e) => { - // The title span and bell button both stopPropagation their own - // right-clicks, so this only fires on the header's bare regions. + // The whole header opens this one menu; only the bell button + // stopPropagations its own right-click (the alert dialog). Right-clicks + // on the title now bubble here — the menu offers "title candidates". e.preventDefault(); e.stopPropagation(); setContextMenu({ x: e.clientX, y: e.clientY }); @@ -222,15 +232,11 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { /> ) : ( e.stopPropagation()} onClick={(e) => { e.stopPropagation(); actions.onStartRename(id); }} - onContextMenu={(e) => { - e.preventDefault(); - e.stopPropagation(); - setTitleCandidatesRect(e.currentTarget.getBoundingClientRect()); - }} > {displayTitleBase} {showsFailGlyph && ( @@ -399,7 +405,12 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { /> )} {contextMenu && ( - + )} {renameWarning && ( ({ ok: true as const, value: pane }); -const okEnsure = (): EnsureAgentBrowserSurfaceResult => - ({ ok: true, status: 'created', surfaceId: 'pane-2', surfaceRef: 'surface:2', minimized: false }); - -// ensureSurface receives the reference as a lazy thunk; unwrap it so the -// call-shape assertions can compare plain values. -function ensureCallArgs(ensureSurface: ReturnType) { - const [args] = ensureSurface.mock.calls.at(-1) ?? []; - const { reference: lazyReference, ...rest } = args as { reference: () => { ok: boolean; value?: DorSurface } } & Record; - return { ...rest, pane: lazyReference().value }; -} +// The eager surface is created before the daemon boots; a plain ok result stands +// in for the real reuse-or-create seam. +const okEager = () => ({ ok: true as const, value: { surfaceId: 'pane-2' } }); describe('connectPortToDefaultBrowser', () => { - it('opens the URL in the default session, reads the port, and ensures the surface', async () => { - const ensureSurface = vi.fn(okEnsure); + it('creates the eager surface before opening, reads the port, then refreshes with session/wsPort/binaryPath', async () => { + const calls: string[] = []; + const ensureEagerSurface = vi.fn(() => { calls.push('ensure'); return okEager(); }); + const refreshSurface = vi.fn(); const platform = { - agentBrowserCommand: vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })), + agentBrowserCommand: vi.fn(async () => { calls.push('open'); return { exitCode: 0, stdout: '', stderr: '' }; }), agentBrowserStreamStatus: vi.fn(async () => ({ ok: true, wsPort: 4321 })), }; - const result = await connectPortToDefaultBrowser({ url: URL, reference, platform, binaryPath: '/bin/ab', ensureSurface }); + const result = await connectPortToDefaultBrowser({ url: URL, platform, binaryPath: '/bin/ab', ensureEagerSurface, refreshSurface }); expect(result).toEqual({ ok: true }); + // The pane is created (session-less) before the slow daemon boot. + expect(calls).toEqual(['ensure', 'open']); + expect(ensureEagerSurface).toHaveBeenCalledWith(SESSION); expect(platform.agentBrowserCommand).toHaveBeenCalledWith(SESSION, ['open', URL], '/bin/ab'); expect(platform.agentBrowserStreamStatus).toHaveBeenCalledWith(SESSION, '/bin/ab'); - expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: 4321, binaryPath: '/bin/ab', pane }); + expect(refreshSurface).toHaveBeenCalledTimes(1); + expect(refreshSurface).toHaveBeenCalledWith('pane-2', { session: SESSION, wsPort: 4321, binaryPath: '/bin/ab' }); }); - it('fails when the host cannot run agent-browser', async () => { - const ensureSurface = vi.fn(okEnsure); - const result = await connectPortToDefaultBrowser({ url: URL, reference, platform: {}, ensureSurface }); + it('short-circuits before creating a surface when the host cannot run agent-browser', async () => { + const ensureEagerSurface = vi.fn(okEager); + const refreshSurface = vi.fn(); + const result = await connectPortToDefaultBrowser({ url: URL, platform: {}, ensureEagerSurface, refreshSurface }); expect(result).toEqual({ ok: false, message: 'opening a browser surface is not supported on this host' }); - expect(ensureSurface).not.toHaveBeenCalled(); + expect(ensureEagerSurface).not.toHaveBeenCalled(); + expect(refreshSurface).not.toHaveBeenCalled(); }); - it('surfaces trimmed stderr on a non-zero exit', async () => { - const ensureSurface = vi.fn(okEnsure); + it('propagates an ensureEagerSurface failure without opening', async () => { + const ensureEagerSurface = vi.fn(() => ({ ok: false as const, message: 'no room' })); + const refreshSurface = vi.fn(); + const platform = { agentBrowserCommand: vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })) }; + const result = await connectPortToDefaultBrowser({ url: URL, platform, ensureEagerSurface, refreshSurface }); + expect(result).toEqual({ ok: false, message: 'no room' }); + expect(platform.agentBrowserCommand).not.toHaveBeenCalled(); + expect(refreshSurface).not.toHaveBeenCalled(); + }); + + it('keeps the pane (refreshing just the session) and surfaces trimmed stderr on a non-zero exit', async () => { + const ensureEagerSurface = vi.fn(okEager); + const refreshSurface = vi.fn(); const platform = { agentBrowserCommand: vi.fn(async () => ({ exitCode: 3, stdout: '', stderr: ' boom \n' })) }; - const result = await connectPortToDefaultBrowser({ url: URL, reference, platform, ensureSurface }); + const result = await connectPortToDefaultBrowser({ url: URL, platform, ensureEagerSurface, refreshSurface }); expect(result).toEqual({ ok: false, message: 'boom' }); - expect(ensureSurface).not.toHaveBeenCalled(); + // The pane stays; the session lets its placeholder name the session. + expect(refreshSurface).toHaveBeenCalledWith('pane-2', { session: SESSION }); }); it('reports the exit code when a non-zero exit has no stderr', async () => { const platform = { agentBrowserCommand: vi.fn(async () => ({ exitCode: 3, stdout: '', stderr: '' })) }; - const result = await connectPortToDefaultBrowser({ url: URL, reference, platform, ensureSurface: vi.fn(okEnsure) }); + const result = await connectPortToDefaultBrowser({ url: URL, platform, ensureEagerSurface: vi.fn(okEager), refreshSurface: vi.fn() }); expect(result).toEqual({ ok: false, message: 'agent-browser open exited 3' }); }); - it('leaves wsPort undefined when the host has no stream-status channel', async () => { - const ensureSurface = vi.fn(okEnsure); + it('omits wsPort in the refresh when the host has no stream-status channel', async () => { + const refreshSurface = vi.fn(); const platform = { agentBrowserCommand: vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })) }; - const result = await connectPortToDefaultBrowser({ url: URL, reference, platform, ensureSurface }); + const result = await connectPortToDefaultBrowser({ url: URL, platform, ensureEagerSurface: vi.fn(okEager), refreshSurface }); expect(result).toEqual({ ok: true }); - expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: undefined, binaryPath: undefined, pane }); + expect(refreshSurface).toHaveBeenCalledWith('pane-2', { session: SESSION }); }); - it('leaves wsPort undefined when stream status fails', async () => { - const ensureSurface = vi.fn(okEnsure); + it('omits wsPort in the refresh when stream status fails', async () => { + const refreshSurface = vi.fn(); const platform = { agentBrowserCommand: vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })), agentBrowserStreamStatus: vi.fn(async () => ({ ok: false, error: 'nope' })), }; - await connectPortToDefaultBrowser({ url: URL, reference, platform, binaryPath: '/bin/ab', ensureSurface }); - expect(ensureCallArgs(ensureSurface)).toEqual({ key: 'default', session: SESSION, wsPort: undefined, binaryPath: '/bin/ab', pane }); - }); - - it('propagates an ensureSurface failure', async () => { - const ensureSurface = vi.fn((): EnsureAgentBrowserSurfaceResult => ({ ok: false, message: 'no room' })); - const platform = { - agentBrowserCommand: vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })), - agentBrowserStreamStatus: vi.fn(async () => ({ ok: true, wsPort: 4321 })), - }; - const result = await connectPortToDefaultBrowser({ url: URL, reference, platform, ensureSurface }); - expect(result).toEqual({ ok: false, message: 'no room' }); + await connectPortToDefaultBrowser({ url: URL, platform, binaryPath: '/bin/ab', ensureEagerSurface: vi.fn(okEager), refreshSurface }); + expect(refreshSurface).toHaveBeenCalledWith('pane-2', { session: SESSION, binaryPath: '/bin/ab' }); }); }); diff --git a/lib/src/components/wall/connect-port.ts b/lib/src/components/wall/connect-port.ts index f1391313..c4501a4b 100644 --- a/lib/src/components/wall/connect-port.ts +++ b/lib/src/components/wall/connect-port.ts @@ -3,14 +3,20 @@ * pane context menu (see `dor/src/commands/agent-browser.ts` for the CLI flow). * Opens the URL in the workspace's default agent-browser session and reuses or * creates that session's browser surface. Dependency-injected (platform + - * `ensureSurface`) so it stays unit-testable off the React tree. + * eager-surface/refresh seams) so it stays unit-testable off the React tree. + * + * Sequencing (`docs/specs/dor-browser.md` → Pane Context Menu Connect): the pane + * is created SYNCHRONOUSLY and WITHOUT a `session`, so it appears the instant the + * port is clicked and the controller's stale-port recovery stays inert until the + * daemon is up (a session-less agent-browser pane spawns no CLI). The background + * `open` + `stream status` then deliver `{session, wsPort, binaryPath}` as one + * params refresh. */ import type { PlatformAdapter } from '../../lib/platform/types'; -import type { ParseResult, Surface as DorSurface } from 'dor/commands/types'; +import type { ParseResult } from 'dor/commands/types'; // The browser-safe subpath: dist/agent-browser.js is pure ES, so importing it // keeps cross-spawn (the package's Node-only default export) out of the webview. import { sessionForKey } from 'dor-lib-common/agent-browser'; -import type { EnsureAgentBrowserSurface } from './use-dor-control'; /** The host capabilities `connectPortToDefaultBrowser` needs — the same two the * CLI path leans on, narrowed so tests can stub them without a full adapter. */ @@ -20,32 +26,41 @@ export type ConnectPortResult = { ok: true } | { ok: false; message: string }; export async function connectPortToDefaultBrowser({ url, - reference, platform, binaryPath, - ensureSurface, + ensureEagerSurface, + refreshSurface, }: { url: string; - /** The pane the port belongs to — the split reference for a fresh surface. - * Lazy for the same reason as `EnsureAgentBrowserSurface`'s: the reuse path - * must succeed even when the pane is no longer visible. */ - reference: () => ParseResult; platform: ConnectPlatform; /** Last binary path a `dor ab` surface resolved; undefined ⇒ host falls back * to PATH / DORMOUSE_AGENT_BROWSER_BIN. */ binaryPath?: string; - ensureSurface: EnsureAgentBrowserSurface; + /** Reuse-or-create the default browser pane synchronously, before the daemon + * boots, so the pane is on screen the instant the port is clicked. Created + * session-less on purpose (see file header). */ + ensureEagerSurface: (session: string) => ParseResult<{ surfaceId: string }>; + /** Fold a params patch onto the eager surface (visible pane or minimized door) + * — used to hand the pane its `session` and refreshed stream port. */ + refreshSurface: (surfaceId: string, patch: Record) => void; }): Promise { - const session = sessionForKey('default'); // Host-gated: no agent-browser runner ⇒ no browser surface (same spirit as the - // render-swap guard in Wall.tsx). + // render-swap guard in Wall.tsx). Short-circuit before touching the layout. if (!platform.agentBrowserCommand) { return { ok: false, message: 'opening a browser surface is not supported on this host' }; } + const session = sessionForKey('default'); + // Pane appears NOW, session-less — the controller can't race the daemon boot. + const eager = ensureEagerSurface(session); + if (!eager.ok) return { ok: false, message: eager.message }; + // 'open' is on the host's subcommand allowlist; the CLI boots the daemon/browser // if it isn't already running. const opened = await platform.agentBrowserCommand(session, ['open', url], binaryPath); if (opened.exitCode !== 0) { + // The pane stays; hand it the session so its placeholder names the session + // instead of sitting sessionless. + refreshSurface(eager.value.surfaceId, { session }); return { ok: false, message: opened.stderr.trim() || `agent-browser open exited ${opened.exitCode}` }; } // Best-effort stream port so the panel connects straight to the live screencast; @@ -55,7 +70,12 @@ export async function connectPortToDefaultBrowser({ const status = await platform.agentBrowserStreamStatus(session, binaryPath); if (status.ok) wsPort = status.wsPort; } - const ensured = ensureSurface({ key: 'default', session, wsPort, binaryPath, reference }); - if (!ensured.ok) return { ok: false, message: ensured.message }; + // One params write reconciles the session-less pane: setting `session` connects + // the controller (the daemon is up now, so its recovery is safe to run). + refreshSurface(eager.value.surfaceId, { + session, + ...(wsPort !== undefined ? { wsPort } : {}), + ...(binaryPath !== undefined ? { binaryPath } : {}), + }); return { ok: true }; } diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index 1aad2a01..21530e3e 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -20,7 +20,8 @@ import { import { surfaceRunsCommand, type TerminalPaneState } from '../../lib/terminal-state'; import { hostPathDisplay } from './browser-url'; import { isAgentBrowserParams } from './browser-surface'; -// Runtime import is one-way (connect-port's use-dor-control import is type-only). +// One-way import: connect-port no longer depends on this module (its eager-surface +// and refresh seams are injected as plain functions). import { connectPortToDefaultBrowser, type ConnectPortResult } from './connect-port'; import { listenerUrlsByPort } from './port-url'; import { dorDirectionForEdge, type LathWallEngine } from './lath-wall-engine'; @@ -441,6 +442,24 @@ export function useDorControl({ return null; }, [lath]); + // Fold a params patch onto a surface whether it's a visible pane (engine + // metadata) or a minimized door (the doorsRef map) — the reuse/refresh + // mechanics shared by `ensureAgentBrowserSurface`'s reuse arm and the + // connect-port refresh seam. A no-op on an empty patch. + const updateSurfaceParams = useCallback((id: string, patch: Record) => { + if (Object.keys(patch).length === 0) return; + const door = doorsRef.current.find((candidate) => candidate.id === id); + if (door) { + const nextDoors = doorsRef.current.map((d) => d.id === id + ? { ...d, params: { ...d.params, ...patch } } + : d); + doorsRef.current = nextDoors; + setDoors(nextDoors); + } else { + lath.store.updateParams(id, patch); + } + }, [doorsRef, lath, setDoors]); + const ensureAgentBrowserSurface = useCallback(({ key, session, @@ -461,17 +480,7 @@ export function useDorControl({ // Reuse: refresh the stream port (OS-assigned, churns across session // restarts) so the panel reconnects to the live stream, and the // resolved binary path alongside it. - if (Object.keys(refreshedParams).length > 0) { - if (existing.minimized) { - const nextDoors = doorsRef.current.map((door) => door.id === existing.id - ? { ...door, params: { ...door.params, ...refreshedParams } } - : door); - doorsRef.current = nextDoors; - setDoors(nextDoors); - } else { - lath.store.updateParams(existing.id, refreshedParams); - } - } + updateSurfaceParams(existing.id, refreshedParams); return { ok: true, status: 'existing', @@ -505,23 +514,64 @@ export function useDorControl({ surfaceRef: result.value.ref, minimized, }; - }, [createContentSurface, findAgentBrowserSurface, lath, setDoors, surfaceRefForId]); + }, [createContentSurface, findAgentBrowserSurface, updateSurfaceParams, surfaceRefForId]); // The pane context menu's "connect a port" action, bound to this hook's // closure so Wall.tsx delegates in one line instead of re-threading the - // hook's internals (`ensureAgentBrowserSurface`, the binary-path ref). - const connectPort = useCallback((id: string, url: string) => connectPortToDefaultBrowser({ - url, - // Lazy: the reuse path must succeed even if the pane vanished (e.g. was + // hook's internals. The pane is created eagerly and session-less so it appears + // instantly; `connectPortToDefaultBrowser` then hands it its session + + // stream port (docs/specs/dor-browser.md → Pane Context Menu Connect). + const connectPort = useCallback((id: string, url: string): Promise => { + // Lazy: the create path must succeed even if the pane vanished (e.g. was // minimized) between the right-click and the connect resolving. - reference: () => { + const reference = (): ParseResult => { const surface = buildDorSurfaces().find((candidate) => candidate.id === id); return surface ? { ok: true, value: surface } : { ok: false, message: `surface for pane '${id}' was not found` }; - }, - platform: getPlatform(), - binaryPath: lastAgentBrowserBinaryPathRef.current, - ensureSurface: ensureAgentBrowserSurface, - }), [buildDorSurfaces, ensureAgentBrowserSurface, lastAgentBrowserBinaryPathRef]); + }; + const ensureEagerSurface = (session: string): ParseResult<{ surfaceId: string }> => { + // (a) A surface already bound to this session — reuse; params untouched + // (the navigation + final refresh handle the rest). + const existing = findAgentBrowserSurface(session); + if (existing) return { ok: true, value: { surfaceId: existing.id } }; + // (b) A still-booting default pane from a rapid earlier connect (created + // but not yet handed its session) — reuse it so a second click during the + // daemon boot doesn't spawn a duplicate. + const isBooting = (params: unknown) => + isAgentBrowserParams(params) + && (params as { key?: unknown }).key === 'default' + && (params as { session?: unknown }).session === undefined; + const bootingId = lath.listPanes().find((candidate) => isBooting(candidate.params))?.id + ?? doorsRef.current.find((candidate) => isBooting(candidate.params))?.id; + if (bootingId) return { ok: true, value: { surfaceId: bootingId } }; + // (c) Create it now: NO `session` (keeps the controller's stale-port + // recovery inert until the daemon is up), but carry the target `url` so + // the browser chrome shows it immediately. + const target = reference(); + if (!target.ok) return { ok: false, message: target.message }; + const created = createContentSurface({ + minimized: false, + params: { surfaceType: 'browser', renderMode: 'ab-screencast', key: 'default', url }, + reference: target.value, + title: 'default', + focusNeutral: true, + }); + if (!created.ok) return { ok: false, message: created.message }; + return { ok: true, value: { surfaceId: created.value.id } }; + }; + const result = connectPortToDefaultBrowser({ + url, + platform: getPlatform(), + binaryPath: lastAgentBrowserBinaryPathRef.current, + ensureEagerSurface, + refreshSurface: updateSurfaceParams, + }); + // The menu no longer surfaces errors (it closes instantly); log a failure the + // way the render-swap path in Wall.tsx does. + void result.then((outcome) => { + if (!outcome.ok) console.warn('[dormouse] connect port failed:', outcome.message); + }); + return result; + }, [buildDorSurfaces, createContentSurface, findAgentBrowserSurface, lath, updateSurfaceParams, lastAgentBrowserBinaryPathRef]); useEffect(() => { const handler = async (event: Event) => { diff --git a/lib/src/components/wall/wall-context.tsx b/lib/src/components/wall/wall-context.tsx index 57a2f7da..b46227a1 100644 --- a/lib/src/components/wall/wall-context.tsx +++ b/lib/src/components/wall/wall-context.tsx @@ -53,10 +53,10 @@ export interface WallActions { /** The stable `surface:N` ref for a pane/door id (minted lazily, exactly as * `dor list` assigns refs). Used by the pane context menu to show the handle. */ resolveSurfaceRef: (id: string) => string; - /** Act like `dor ab open ` for a port the pane's process tree binds: open - * the URL in the workspace's default agent-browser session and reuse-or-create - * its browser surface (`connect-port.ts`). Resolves with a failure message the - * menu can surface. */ + /** Act like `dor ab open ` for a port the pane's process tree binds: create + * the default agent-browser surface immediately, then open the URL on its + * session and connect the pane (`connect-port.ts`). The menu fires-and-forgets; + * the promise (resolving with any failure message) is tapped/logged, not shown. */ onConnectPort: (id: string, url: string) => Promise; } From 4feaa9bda47c12ee8b877e6318eadf6fd3f65a1f Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 10 Jul 2026 17:37:46 -0700 Subject: [PATCH 06/13] /simplify: consolidate the eager create into ensure, pin the inertness invariant - ensureAgentBrowserSurface takes optional session/url; the connect flow's eager session-less create is now its create arm instead of a second createContentSurface literal (the two ab-screencast creators can't drift) - findSurfaceByParams generalizes the panes-then-doors scan; the session lookup and the booting-pane lookup are two calls of one function - onConnectPort is void: nothing read the promised result since the menu stopped displaying errors; five stub sites shrink to no-ops - the ensure types are module-private again (their last external importer left in the previous commit) and the title-candidates handler is inlined where TypeScript narrows contextMenu - the session-less-is-inert guarantee the instant connect hangs on is now stated at its enforcement point (maybeRecoverStalePort), pinned by a controller test (no CLI spawn, no socket until params deliver the session), and cross-referenced in the Agent-Browser Connection spec Co-Authored-By: Claude Fable 5 --- docs/specs/dor-browser.md | 5 + .../wall/PaneHeaderContextMenu.test.tsx | 2 +- .../components/wall/PaneHeaderContextMenu.tsx | 9 +- .../components/wall/TerminalPaneHeader.tsx | 14 +-- .../agent-browser-surface-controller.test.ts | 24 +++++ .../wall/agent-browser-surface-controller.ts | 5 + lib/src/components/wall/use-dor-control.ts | 98 ++++++++++--------- lib/src/components/wall/wall-context.tsx | 9 +- lib/src/components/wall/wall-test-utils.ts | 2 +- .../stories/BrowserChromeHeader.stories.tsx | 5 +- lib/src/stories/MouseHeaderIcon.stories.tsx | 2 +- lib/src/stories/ShellCwd.stories.tsx | 2 +- .../stories/TerminalPaneHeader.stories.tsx | 2 +- 13 files changed, 105 insertions(+), 74 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index c4de69bd..1b176035 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -301,6 +301,11 @@ the same zero-resource end state with less thrash. The agent-browser daemon/session stays alive throughout and reattaches from persisted params. The controller's client resources are released only at pane kill or a render swap away from the renderer (`disposeAgentBrowserSurfaceController` in `Wall.tsx`). +A controller whose params carry no `session` is deliberately inert — no +connection, no `stream status` query: the instant connect flow +([Pane Context Menu Connect](#pane-context-menu-connect)) depends on that +inertness to keep the eager pane from racing the daemon boot, so the session +must never be derived from `key` here. Hidden-but-mounted panes park too. A Lath leaf is always mounted (no active-tab gating), so a backgrounded window would keep its ~20Hz stream plus per-pulse diff --git a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx index 9ab6f007..f572efeb 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx @@ -101,7 +101,7 @@ describe('PaneHeaderContextMenu — pane header right-click', () => { enableConnect(platform); platform.spawnPty('term-1'); platform.setOpenPorts('term-1', [loopbackPort(5173, 'node')]); - const onConnectPort = vi.fn(async () => ({ ok: true as const })); + const onConnectPort = vi.fn(); renderHeader(headerProps('term-1', 't'), stubActions({ onConnectPort })); await act(async () => { fireContextMenu(); }); diff --git a/lib/src/components/wall/PaneHeaderContextMenu.tsx b/lib/src/components/wall/PaneHeaderContextMenu.tsx index 66b842e9..b3ff539e 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.tsx @@ -14,6 +14,9 @@ type ScanState = | { status: 'loaded'; entries: PortUrlEntry[] } | { status: 'failed' }; +// One recipe for both interactive rows (port entries, title candidates). +const MENU_ROW_CLASS = 'flex w-full items-baseline gap-2 px-2.5 py-1 text-left hover:bg-foreground/10'; + /** * The right-click menu on a terminal pane header (`docs/specs/layout.md` → Pane * header): the pane's `surface:N` handle, then the TCP ports its process tree @@ -70,7 +73,7 @@ export function PaneHeaderContextMenu({ // Fire-and-forget: the pane appears immediately and reports its own progress, so // the menu closes at once rather than waiting on the daemon boot. const connect = (entry: PortUrlEntry) => { - void actions.onConnectPort(id, entry.url); + actions.onConnectPort(id, entry.url); onClose(); }; @@ -113,7 +116,7 @@ export function PaneHeaderContextMenu({ type="button" role="menuitem" data-port-entry={entry.port} - className="flex w-full items-baseline gap-2 px-2.5 py-1 text-left hover:bg-foreground/10" + className={MENU_ROW_CLASS} onClick={() => connect(entry)} > {label} @@ -129,7 +132,7 @@ export function PaneHeaderContextMenu({ type="button" role="menuitem" data-title-candidates-item - className="flex w-full items-baseline gap-2 px-2.5 py-1 text-left text-muted hover:bg-foreground/10 hover:text-foreground" + className={`${MENU_ROW_CLASS} text-muted hover:text-foreground`} onClick={() => { onClose(); onShowTitleCandidates(); }} > title candidates diff --git a/lib/src/components/wall/TerminalPaneHeader.tsx b/lib/src/components/wall/TerminalPaneHeader.tsx index f26ac409..bbc0dae2 100644 --- a/lib/src/components/wall/TerminalPaneHeader.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.tsx @@ -150,14 +150,6 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { const closeTodoPreview = useCallback(() => setTodoPreviewRect(null), []); const closeTitleCandidates = useCallback(() => setTitleCandidatesRect(null), []); const closeContextMenu = useCallback(() => setContextMenu(null), []); - // Reachable from the context menu now that the title span owns no right-click. - // Anchor to the span; fall back to the menu's pointer position when the span is - // absent (e.g. mid-rename, when an input replaces it). - const showTitleCandidates = useCallback(() => { - const rect = titleSpanRef.current?.getBoundingClientRect() - ?? (contextMenu ? new DOMRect(contextMenu.x, contextMenu.y, 0, 0) : null); - if (rect) setTitleCandidatesRect(rect); - }, [contextMenu]); const closeRenameWarning = useCallback(() => setRenameWarning(null), []); const submitRename = useCallback((value: string, anchor: HTMLElement) => { const rect = anchor.getBoundingClientRect(); @@ -409,7 +401,11 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { id={id} anchor={contextMenu} onClose={closeContextMenu} - onShowTitleCandidates={showTitleCandidates} + // Anchor to the title span; fall back to the menu's pointer position + // when the span is absent (mid-rename, when an input replaces it). + onShowTitleCandidates={() => setTitleCandidatesRect( + titleSpanRef.current?.getBoundingClientRect() ?? new DOMRect(contextMenu.x, contextMenu.y, 0, 0), + )} /> )} {renameWarning && ( diff --git a/lib/src/components/wall/agent-browser-surface-controller.test.ts b/lib/src/components/wall/agent-browser-surface-controller.test.ts index dd2ebcc8..c14dc3e9 100644 --- a/lib/src/components/wall/agent-browser-surface-controller.test.ts +++ b/lib/src/components/wall/agent-browser-surface-controller.test.ts @@ -279,6 +279,30 @@ describe('updateParams', () => { }); describe('stale-port recovery gating', () => { + it('stays fully inert for a session-less pane until params deliver the session', async () => { + const streamStatus = vi.fn(async () => ({ ok: true, wsPort: 2222 })); + const platform = new FakePtyAdapter() as FakePtyAdapter & Pick; + platform.agentBrowserStreamStatus = streamStatus; + setPlatform(platform); + + // The pane context menu's instant connect mounts its surface WITHOUT a + // session precisely so nothing here can race the daemon boot — no recovery + // query, no socket (docs/specs/dor-browser.md → Pane Context Menu Connect). + // Deriving the session from `key` would silently reintroduce the race. + const controller = acquireAgentBrowserSurfaceController('id', { key: 'default', url: 'http://localhost:5173/' }); + const sink = makeSink(); + controller.attachView(sink); + await flushMicrotasks(); + expect(streamStatus).not.toHaveBeenCalled(); + expect(WebSocketMock.instances).toHaveLength(0); + + // The background boot hands over {session, wsPort} in one params write, + // which is what brings the stream up. + controller.updateParams({ key: 'default', url: 'http://localhost:5173/', session: 'sess', wsPort: 1111 }); + await flushMicrotasks(); + expect(streamSocket(1111)?.readyState).toBe(1); + }); + it('never queries stream status while parked', async () => { vi.useFakeTimers(); try { diff --git a/lib/src/components/wall/agent-browser-surface-controller.ts b/lib/src/components/wall/agent-browser-surface-controller.ts index f0f14649..1c8bd869 100644 --- a/lib/src/components/wall/agent-browser-surface-controller.ts +++ b/lib/src/components/wall/agent-browser-surface-controller.ts @@ -815,6 +815,11 @@ export class AgentBrowserSurfaceController { // query (mirrors the old effect's cleanup running on every dep change). const gen = ++this.recoveryGen; const session = this.session; + // Session-less is deliberate inertness, not just a null-guard: the pane + // context menu's eager connect creates its surface WITHOUT a session + // precisely so no recovery/CLI spawn can race the daemon boot + // (docs/specs/dor-browser.md → Pane Context Menu Connect). Never derive the + // session from `key` here — that would silently reintroduce the race. if (!session) return; // A parked pane must never query the daemon: a `stream status` at the wrong // moment can spawn a competing daemon, and it's a pointless CLI spawn per diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index 21530e3e..e54fd7da 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -22,7 +22,7 @@ import { hostPathDisplay } from './browser-url'; import { isAgentBrowserParams } from './browser-surface'; // One-way import: connect-port no longer depends on this module (its eager-surface // and refresh seams are injected as plain functions). -import { connectPortToDefaultBrowser, type ConnectPortResult } from './connect-port'; +import { connectPortToDefaultBrowser } from './connect-port'; import { listenerUrlsByPort } from './port-url'; import { dorDirectionForEdge, type LathWallEngine } from './lath-wall-engine'; import type { WallNav } from './keyboard/types'; @@ -63,17 +63,22 @@ type DorControlRequest = Omit & { /** Outcome of {@link EnsureAgentBrowserSurface}: the fields the caller maps onto * its response, or a failure message. `minimized` is the surface's current * minimized state (the reused surface's, or the requested value for a fresh one). */ -export type EnsureAgentBrowserSurfaceResult = +type EnsureAgentBrowserSurfaceResult = | { ok: true; status: 'created' | 'existing' | 'replaced'; surfaceId: string; surfaceRef: string; minimized: boolean } | { ok: false; message: string }; -/** Reuse-or-create the browser surface bound to an agent-browser `session`, - * refreshing its stream port / binary path when reused. This is the surface - * half of `dor ab` — shared by the control plane and the pane context menu's - * "connect a port" action (`connect-port.ts`). */ -export type EnsureAgentBrowserSurface = (args: { +/** Reuse-or-create an agent-browser browser surface — the surface half of + * `dor ab` (the control plane), and, with `session` omitted, the pane context + * menu's eager session-less create (docs/specs/dor-browser.md → Pane Context + * Menu Connect). At least one of `key` / `session` is required (it names the + * surface). */ +type EnsureAgentBrowserSurface = (args: { key?: string; - session: string; + /** Omitted for the eager connect pane, which is created session-less on + * purpose so the controller stays inert until the daemon is up; the reuse + * arm is skipped (there is no session to match). */ + session?: string; + url?: string; wsPort?: number; binaryPath?: string; /** Resolved lazily, only when a fresh surface must be created: the reuse path @@ -371,7 +376,7 @@ export function useDorControl({ killPaneImmediately: (id: string) => void; /** The last binary path a `dor ab` surface resolved on a terminal's PATH. */ lastAgentBrowserBinaryPathRef: MutableRefObject; -}): { connectPort: (id: string, url: string) => Promise } { +}): { connectPort: (id: string, url: string) => Promise } { const resolveVisibleSurface = useCallback(( target: string | undefined, callerSurfaceId: string | undefined, @@ -427,14 +432,11 @@ export function useDorControl({ }, [lath]); /** - * The agent-browser session ↔ surface registry, derived from panel/door - * params rather than kept as separate state so it survives webview reloads. - * Returns the surface bound to `session`, or null if none exists. + * The surface (visible pane or minimized door — panes win) whose params match, + * derived from panel/door params rather than kept as separate state so it + * survives webview reloads. Null when nothing matches. */ - const findAgentBrowserSurface = useCallback((session: string): { id: string; minimized: boolean } | null => { - const isMatch = (params: unknown) => - isAgentBrowserParams(params) && (params as { session?: unknown }).session === session; - + const findSurfaceByParams = useCallback((isMatch: (params: unknown) => boolean): { id: string; minimized: boolean } | null => { const panel = lath.listPanes().find((candidate) => isMatch(candidate.params)); if (panel) return { id: panel.id, minimized: false }; const door = doorsRef.current.find((candidate) => isMatch(candidate.params)); @@ -442,6 +444,12 @@ export function useDorControl({ return null; }, [lath]); + /** The agent-browser session ↔ surface registry: the surface bound to + * `session`, or null if none exists. */ + const findAgentBrowserSurface = useCallback((session: string) => findSurfaceByParams((params) => + isAgentBrowserParams(params) && (params as { session?: unknown }).session === session, + ), [findSurfaceByParams]); + // Fold a params patch onto a surface whether it's a visible pane (engine // metadata) or a minimized door (the doorsRef map) — the reuse/refresh // mechanics shared by `ensureAgentBrowserSurface`'s reuse arm and the @@ -463,6 +471,7 @@ export function useDorControl({ const ensureAgentBrowserSurface = useCallback(({ key, session, + url, wsPort, binaryPath, reference, @@ -475,7 +484,7 @@ export function useDorControl({ ...(binaryPath !== undefined ? { binaryPath } : {}), }; - const existing = findAgentBrowserSurface(session); + const existing = session === undefined ? null : findAgentBrowserSurface(session); if (existing) { // Reuse: refresh the stream port (OS-assigned, churns across session // restarts) so the panel reconnects to the live stream, and the @@ -490,6 +499,8 @@ export function useDorControl({ }; } + const title = key ?? session; + if (title === undefined) return { ok: false, message: 'an agent-browser surface needs a key or a session' }; const target = reference(); if (!target.ok) return { ok: false, message: target.message }; const result = createContentSurface({ @@ -497,12 +508,13 @@ export function useDorControl({ params: { surfaceType: 'browser', renderMode: 'ab-screencast', - session, + ...(session !== undefined ? { session } : {}), ...(key !== undefined ? { key } : {}), + ...(url !== undefined ? { url } : {}), ...refreshedParams, }, reference: target.value, - title: key ?? session, + title, // `dor ab` opens the screencast in the background; caller keeps focus. focusNeutral: true, }); @@ -521,13 +533,8 @@ export function useDorControl({ // hook's internals. The pane is created eagerly and session-less so it appears // instantly; `connectPortToDefaultBrowser` then hands it its session + // stream port (docs/specs/dor-browser.md → Pane Context Menu Connect). - const connectPort = useCallback((id: string, url: string): Promise => { - // Lazy: the create path must succeed even if the pane vanished (e.g. was - // minimized) between the right-click and the connect resolving. - const reference = (): ParseResult => { - const surface = buildDorSurfaces().find((candidate) => candidate.id === id); - return surface ? { ok: true, value: surface } : { ok: false, message: `surface for pane '${id}' was not found` }; - }; + // Failures are logged, not returned — the menu closes before one can exist. + const connectPort = useCallback((id: string, url: string): Promise => { const ensureEagerSurface = (session: string): ParseResult<{ surfaceId: string }> => { // (a) A surface already bound to this session — reuse; params untouched // (the navigation + final refresh handle the rest). @@ -536,42 +543,37 @@ export function useDorControl({ // (b) A still-booting default pane from a rapid earlier connect (created // but not yet handed its session) — reuse it so a second click during the // daemon boot doesn't spawn a duplicate. - const isBooting = (params: unknown) => + const booting = findSurfaceByParams((params) => isAgentBrowserParams(params) && (params as { key?: unknown }).key === 'default' - && (params as { session?: unknown }).session === undefined; - const bootingId = lath.listPanes().find((candidate) => isBooting(candidate.params))?.id - ?? doorsRef.current.find((candidate) => isBooting(candidate.params))?.id; - if (bootingId) return { ok: true, value: { surfaceId: bootingId } }; + && (params as { session?: unknown }).session === undefined); + if (booting) return { ok: true, value: { surfaceId: booting.id } }; // (c) Create it now: NO `session` (keeps the controller's stale-port // recovery inert until the daemon is up), but carry the target `url` so // the browser chrome shows it immediately. - const target = reference(); - if (!target.ok) return { ok: false, message: target.message }; - const created = createContentSurface({ - minimized: false, - params: { surfaceType: 'browser', renderMode: 'ab-screencast', key: 'default', url }, - reference: target.value, - title: 'default', - focusNeutral: true, + const created = ensureAgentBrowserSurface({ + key: 'default', + url, + reference: () => { + const surface = buildDorSurfaces().find((candidate) => candidate.id === id); + return surface ? { ok: true, value: surface } : { ok: false, message: `surface for pane '${id}' was not found` }; + }, }); - if (!created.ok) return { ok: false, message: created.message }; - return { ok: true, value: { surfaceId: created.value.id } }; + if (!created.ok) return created; + return { ok: true, value: { surfaceId: created.surfaceId } }; }; - const result = connectPortToDefaultBrowser({ + return connectPortToDefaultBrowser({ url, platform: getPlatform(), binaryPath: lastAgentBrowserBinaryPathRef.current, ensureEagerSurface, refreshSurface: updateSurfaceParams, - }); - // The menu no longer surfaces errors (it closes instantly); log a failure the - // way the render-swap path in Wall.tsx does. - void result.then((outcome) => { + }).then((outcome) => { + // The menu no longer surfaces errors (it closes instantly); log a failure + // the way the render-swap path in Wall.tsx does. if (!outcome.ok) console.warn('[dormouse] connect port failed:', outcome.message); }); - return result; - }, [buildDorSurfaces, createContentSurface, findAgentBrowserSurface, lath, updateSurfaceParams, lastAgentBrowserBinaryPathRef]); + }, [buildDorSurfaces, ensureAgentBrowserSurface, findAgentBrowserSurface, findSurfaceByParams, updateSurfaceParams, lastAgentBrowserBinaryPathRef]); useEffect(() => { const handler = async (event: Event) => { diff --git a/lib/src/components/wall/wall-context.tsx b/lib/src/components/wall/wall-context.tsx index b46227a1..08d4a4a6 100644 --- a/lib/src/components/wall/wall-context.tsx +++ b/lib/src/components/wall/wall-context.tsx @@ -2,7 +2,6 @@ import { createContext } from 'react'; import type { AlertButtonActionResult, SessionStatus, SetTerminalUserTitleResult } from '../../lib/terminal-registry'; import type { WallMode } from './wall-types'; import type { RenderMode } from './agent-browser-screen'; -import type { ConnectPortResult } from './connect-port'; export interface PaneElementsState { elements: Map; @@ -55,9 +54,9 @@ export interface WallActions { resolveSurfaceRef: (id: string) => string; /** Act like `dor ab open ` for a port the pane's process tree binds: create * the default agent-browser surface immediately, then open the URL on its - * session and connect the pane (`connect-port.ts`). The menu fires-and-forgets; - * the promise (resolving with any failure message) is tapped/logged, not shown. */ - onConnectPort: (id: string, url: string) => Promise; + * session and connect the pane (`connect-port.ts`). Fire-and-forget — failures + * are logged, and the pane itself shows loading state. */ + onConnectPort: (id: string, url: string) => void; } export const WallActionsContext = createContext({ @@ -76,7 +75,7 @@ export const WallActionsContext = createContext({ onSwapRenderMode: () => {}, onOpenBrowserPane: () => {}, resolveSurfaceRef: (id: string) => id, - onConnectPort: async () => ({ ok: false, message: 'no Wall is mounted' }), + onConnectPort: () => {}, }); /** Engine-directed writes from a pane/header (title + params). The read side is diff --git a/lib/src/components/wall/wall-test-utils.ts b/lib/src/components/wall/wall-test-utils.ts index 6e779ab6..1b3e9cb1 100644 --- a/lib/src/components/wall/wall-test-utils.ts +++ b/lib/src/components/wall/wall-test-utils.ts @@ -19,7 +19,7 @@ export function stubWallActions(overrides: Partial = {}): WallActio onCancelRename: vi.fn(), onSwapRenderMode: vi.fn(), resolveSurfaceRef: vi.fn((id: string) => id), - onConnectPort: vi.fn(async () => ({ ok: true as const })), + onConnectPort: vi.fn(), ...overrides, }; } diff --git a/lib/src/stories/BrowserChromeHeader.stories.tsx b/lib/src/stories/BrowserChromeHeader.stories.tsx index ba3f4ebb..6eb9c554 100644 --- a/lib/src/stories/BrowserChromeHeader.stories.tsx +++ b/lib/src/stories/BrowserChromeHeader.stories.tsx @@ -48,10 +48,7 @@ const loggingActions: WallActions = { onCancelRename: () => {}, onSwapRenderMode: (id, mode) => console.log('[story] swap render', id, mode), resolveSurfaceRef: (id) => id, - onConnectPort: async (id, url) => { - console.log('[story] connect port', id, url); - return { ok: true }; - }, + onConnectPort: (id, url) => console.log('[story] connect port', id, url), }; interface StoryArgs { diff --git a/lib/src/stories/MouseHeaderIcon.stories.tsx b/lib/src/stories/MouseHeaderIcon.stories.tsx index d2d6ac5b..aa92e8c0 100644 --- a/lib/src/stories/MouseHeaderIcon.stories.tsx +++ b/lib/src/stories/MouseHeaderIcon.stories.tsx @@ -34,7 +34,7 @@ const noopActions: WallActions = { onCancelRename: () => {}, onSwapRenderMode: () => {}, resolveSurfaceRef: (id) => id, - onConnectPort: async () => ({ ok: true }), + onConnectPort: () => {}, }; function MouseIconStoryFrame({ diff --git a/lib/src/stories/ShellCwd.stories.tsx b/lib/src/stories/ShellCwd.stories.tsx index 334f765a..ad770bdd 100644 --- a/lib/src/stories/ShellCwd.stories.tsx +++ b/lib/src/stories/ShellCwd.stories.tsx @@ -54,7 +54,7 @@ const noopActions: WallActions = { onCancelRename: () => {}, onSwapRenderMode: () => {}, resolveSurfaceRef: (id) => id, - onConnectPort: async () => ({ ok: true }), + onConnectPort: () => {}, }; const meta: Meta = { diff --git a/lib/src/stories/TerminalPaneHeader.stories.tsx b/lib/src/stories/TerminalPaneHeader.stories.tsx index 23876508..c509cd7a 100644 --- a/lib/src/stories/TerminalPaneHeader.stories.tsx +++ b/lib/src/stories/TerminalPaneHeader.stories.tsx @@ -30,7 +30,7 @@ const noopActions: WallActions = { onCancelRename: () => {}, onSwapRenderMode: () => {}, resolveSurfaceRef: (id) => id, - onConnectPort: async () => ({ ok: true }), + onConnectPort: () => {}, }; function actionsRejecting(reason: 'empty' | 'reserved'): WallActions { From 0a4b348539069eaba2d355b8d0e4271a827a6264 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 22 Jul 2026 13:17:13 -0700 Subject: [PATCH 07/13] Connect reveals its surface; name the session-less pane's state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pieces of feedback on the pane-header connect menu: - A port click now selects the browser surface it connected (reattaching it first when minimized, like clicking its Door chip). The create path had inherited `dor ab`'s `focusNeutral: true`, so `settleAddSelection` never selected the new pane, and the reuse arm never touched selection at all — making a repeat click on an already-connected port look like a no-op (the session just re-navigates to the URL it is already on, so the selection move is the only feedback there is). Selection only: passthrough stays on the terminal that was right-clicked. - The eager session-less pane rendered `Waiting for browser session — run dor ab open `: an empty session name and instructions to redo the click that created it. It now shows `Connecting to browser session…`, which is what the spec already promised. `revealSurface` lives in Wall.tsx (the selection authority) and is threaded into useDorControl; the `dor ab` control path stays focus-neutral. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/specs/dor-browser.md | 22 ++++++++++++++----- docs/specs/layout.md | 2 +- lib/src/components/Wall.tsx | 17 ++++++++++++++ lib/src/components/wall/AgentBrowserPanel.tsx | 7 +++++- lib/src/components/wall/use-dor-control.ts | 20 +++++++++++++---- 5 files changed, 57 insertions(+), 11 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 1b176035..33a84f86 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -177,17 +177,29 @@ address, IPv6 bracketed). Clicking a port row reproduces `dor ab open ` against the **default** key/session: it runs `agent-browser open ` on that session and -reuses-or-creates the session's browser surface, focus-neutrally (the caller -keeps focus) — the wall-side mirror of the CLI flow, not the control plane. It is -host-gated on `agentBrowserCommand`: absent (e.g. the web demo host), the rows -render as inert labels. +reuses-or-creates the session's browser surface — the wall-side mirror of the CLI +flow, not the control plane. It is host-gated on `agentBrowserCommand`: absent +(e.g. the web demo host), the rows render as inert labels. + +**The click reveals its surface.** Unlike `dor ab` (focus-neutral: an agent must +not steal focus from the human), a menu click *is* the human asking to see that +browser, so every arm of the eager lookup below ends by selecting the surface — +reattaching it first when it is minimized, on the same terms as clicking its Door +chip. Selection only: passthrough stays on the terminal that was right-clicked, so +connecting a port never redirects the keyboard. This is why a repeat click on an +already-connected port is visible at all — the session merely re-navigates to the +URL it is already on, so the selection move is the only feedback. +Source of truth: `revealSurface` in `Wall.tsx`, threaded into `useDorControl`. **Instant create.** The click is fire-and-forget: the menu closes at once and the pane appears **before** `agent-browser open` runs (a cold daemon boot is 1–3s). The eager surface is created **without a `session`** — deliberately: a session-less `ab-screencast` pane is inert (the controller's `maybeRecoverStalePort` returns early with no session, so it spawns no CLI and cannot race the daemon -boot), while the panel still shows `Connecting to browser session…`. It carries +boot), while the panel still shows `Connecting to browser session…` — the +session-less placeholder branch exists for exactly this pane, and deliberately +does *not* fall through to the idle `run dor ab open ` line, which would ask +the user to redo the click they just made. It carries `key: 'default'` and the target `url` so the browser chrome shows the destination immediately. Once `open` succeeds, a best-effort `stream status` runs, then the pane receives `{session, wsPort, binaryPath}` as **one** params refresh — setting diff --git a/docs/specs/layout.md b/docs/specs/layout.md index 0dd95a17..6e2519ae 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -81,7 +81,7 @@ The header label is the `DerivedHeader` returned by `deriveHeader(paneState, vis The diagnostic popup lists the latest entry per `titleCandidates` channel as defined in `docs/specs/terminal-state.md`. Each row shows the channel, latest candidate text, and timestamp. The popup is diagnostic only; it does not change the title priority rules. -Right-clicking anywhere on the header opens a single **context menu** at the pointer. Only the alert bell owns its own right-click (`stopPropagation`, opening the alert dialog); every other region — including the title span — bubbles to this one menu. It is portaled to `document.body`, viewport-clamped, and dismissed like the diagnostic popup (outside `pointerdown`, `Escape`, `resize`, capture-phase `scroll`). It shows the pane's `surface:N` handle (`resolveSurfaceRef`) as a non-interactive monospace row, then the TCP ports the pane's process tree binds: a spinner while `getOpenPorts` runs (once per open — right-click again to rescan), then one `host:port` row per distinct port (the process name muted beside it), or a muted `no listening ports` / `port scan failed` line, then a `title candidates` item that opens the diagnostic popup (anchored to the title span, or the pointer while renaming). Clicking a port row reproduces `dor ab open ` for that port and closes the menu at once (`docs/specs/dor-browser.md` → Pane Context Menu Connect): the browser surface appears immediately, and loading/errors surface in the pane, not the menu; the rows are inert labels on hosts that cannot open a browser surface. Only terminal panes have this menu. Source of truth: `PaneHeaderContextMenu.tsx`, `TerminalPaneHeader.tsx`. +Right-clicking anywhere on the header opens a single **context menu** at the pointer. Only the alert bell owns its own right-click (`stopPropagation`, opening the alert dialog); every other region — including the title span — bubbles to this one menu. It is portaled to `document.body`, viewport-clamped, and dismissed like the diagnostic popup (outside `pointerdown`, `Escape`, `resize`, capture-phase `scroll`). It shows the pane's `surface:N` handle (`resolveSurfaceRef`) as a non-interactive monospace row, then the TCP ports the pane's process tree binds: a spinner while `getOpenPorts` runs (once per open — right-click again to rescan), then one `host:port` row per distinct port (the process name muted beside it), or a muted `no listening ports` / `port scan failed` line, then a `title candidates` item that opens the diagnostic popup (anchored to the title span, or the pointer while renaming). Clicking a port row reproduces `dor ab open ` for that port and closes the menu at once (`docs/specs/dor-browser.md` → Pane Context Menu Connect): the browser surface appears immediately and becomes the selection (reattaching first if it was minimized), and loading/errors surface in the pane, not the menu; the rows are inert labels on hosts that cannot open a browser surface. Only terminal panes have this menu. Source of truth: `PaneHeaderContextMenu.tsx`, `TerminalPaneHeader.tsx`. Elements from left to right: diff --git a/lib/src/components/Wall.tsx b/lib/src/components/Wall.tsx index c75cd747..16f168fd 100644 --- a/lib/src/components/Wall.tsx +++ b/lib/src/components/Wall.tsx @@ -772,6 +772,22 @@ export function Wall({ const handleReattachRef = useRef(handleReattach); handleReattachRef.current = handleReattach; + /** Put the selection on a surface so the user can see it — the human half of + * `connectPort` (clicking a port row is a request to look at that browser). + * A visible pane is selected in place; a minimized one reattaches first, on + * the same terms as clicking its Door chip. Distinct from `onFocusPane`, + * which also enters passthrough on an already-visible pane: connecting a port + * should highlight the browser, not steal the keyboard from the terminal the + * user right-clicked. */ + const revealSurface = useCallback((id: string) => { + if (nav.hasPane(id)) { + selectPane(id); + return; + } + const door = doorsRef.current.find((item) => item.id === id); + if (door) handleReattachRef.current(door); + }, [nav, selectPane]); + // The Surfaces of the current Workspace. `buildDorSurfaces` is the visible-pane // projection used for geometry/placement; `buildDorSurfaceList` additionally // includes minimized (doored) Surfaces for `dor list` and direct operations. @@ -1117,6 +1133,7 @@ export function Wall({ createSplitSurface, createContentSurface, killPaneImmediately, + revealSurface, lastAgentBrowserBinaryPathRef, }); diff --git a/lib/src/components/wall/AgentBrowserPanel.tsx b/lib/src/components/wall/AgentBrowserPanel.tsx index a70c89e9..dc9e5e0c 100644 --- a/lib/src/components/wall/AgentBrowserPanel.tsx +++ b/lib/src/components/wall/AgentBrowserPanel.tsx @@ -317,7 +317,12 @@ export function AgentBrowserPanel({ id, params: rawParams, renderMode: renderMod // --- placeholder state (derived from the snapshot) --- const placeholder = (() => { - if (!streamPort) return `Waiting for browser session ${session ?? ''} — run dor ab open `; + // Session-less: the pane context menu's eager connect pane, on screen before + // the daemon boots (docs/specs/dor-browser.md → Pane Context Menu Connect). + // It is mid-boot, not idle — telling the user to run `dor ab open` here would + // ask them to redo the click they just made. + if (!session) return 'Connecting to browser session…'; + if (!streamPort) return `Waiting for browser session ${session} — run dor ab open `; if (connectionLost || status?.connected === false) { return `Browser session ${session ?? ''} ended — run dor ab open to restart it, or close this surface.`; } diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index e54fd7da..9a064db3 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -341,6 +341,7 @@ export function useDorControl({ createSplitSurface, createContentSurface, killPaneImmediately, + revealSurface, lastAgentBrowserBinaryPathRef, }: { /** The Lath engine — visible-pane projection (`lath.listPanes()`), aspect-ratio @@ -374,6 +375,10 @@ export function useDorControl({ focusNeutral?: boolean; }) => ParseResult<{ id: string; ref: string; status: 'created' | 'replaced' }>; killPaneImmediately: (id: string) => void; + /** Put the selection on a surface, reattaching it first when it is minimized. + * Used by the human-initiated `connectPort` (a menu click is a request to see + * that surface); the `dor ab` control path stays focus-neutral. */ + revealSurface: (id: string) => void; /** The last binary path a `dor ab` surface resolved on a terminal's PATH. */ lastAgentBrowserBinaryPathRef: MutableRefObject; }): { connectPort: (id: string, url: string) => Promise } { @@ -536,10 +541,17 @@ export function useDorControl({ // Failures are logged, not returned — the menu closes before one can exist. const connectPort = useCallback((id: string, url: string): Promise => { const ensureEagerSurface = (session: string): ParseResult<{ surfaceId: string }> => { + // Every arm below ends on the same surface id, and a menu click is a human + // asking to *see* that surface — so reveal it (reattach if minimized, then + // select). `dor ab`'s control path stays focus-neutral; this one does not. + const reveal = (surfaceId: string): ParseResult<{ surfaceId: string }> => { + revealSurface(surfaceId); + return { ok: true, value: { surfaceId } }; + }; // (a) A surface already bound to this session — reuse; params untouched // (the navigation + final refresh handle the rest). const existing = findAgentBrowserSurface(session); - if (existing) return { ok: true, value: { surfaceId: existing.id } }; + if (existing) return reveal(existing.id); // (b) A still-booting default pane from a rapid earlier connect (created // but not yet handed its session) — reuse it so a second click during the // daemon boot doesn't spawn a duplicate. @@ -547,7 +559,7 @@ export function useDorControl({ isAgentBrowserParams(params) && (params as { key?: unknown }).key === 'default' && (params as { session?: unknown }).session === undefined); - if (booting) return { ok: true, value: { surfaceId: booting.id } }; + if (booting) return reveal(booting.id); // (c) Create it now: NO `session` (keeps the controller's stale-port // recovery inert until the daemon is up), but carry the target `url` so // the browser chrome shows it immediately. @@ -560,7 +572,7 @@ export function useDorControl({ }, }); if (!created.ok) return created; - return { ok: true, value: { surfaceId: created.surfaceId } }; + return reveal(created.surfaceId); }; return connectPortToDefaultBrowser({ url, @@ -573,7 +585,7 @@ export function useDorControl({ // the way the render-swap path in Wall.tsx does. if (!outcome.ok) console.warn('[dormouse] connect port failed:', outcome.message); }); - }, [buildDorSurfaces, ensureAgentBrowserSurface, findAgentBrowserSurface, findSurfaceByParams, updateSurfaceParams, lastAgentBrowserBinaryPathRef]); + }, [buildDorSurfaces, ensureAgentBrowserSurface, findAgentBrowserSurface, findSurfaceByParams, updateSurfaceParams, revealSurface, lastAgentBrowserBinaryPathRef]); useEffect(() => { const handler = async (event: Event) => { From 694b3768a48659bc1686df1ab17709ec0b052801 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 22 Jul 2026 13:19:49 -0700 Subject: [PATCH 08/13] Run blocking sidecar commands off the main thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pane-header connect created its browser pane in 1ms but nothing painted for ~2.9s, so the pane looked like it appeared only after the daemon booted. The eager create was never at fault: `agent_browser_command` was a plain `#[tauri::command]`, which Tauri runs on the main thread, and its body blocks on `recv_timeout` until the sidecar answers. The webview cannot paint during that, so every frame between the click and the daemon coming up was dropped. Declares `(async)` on the twelve commands that reach the blocking sidecar helpers, moving the same body onto a runtime worker. Fixes the connect freeze and a class of smaller ones with it — including `pty_get_open_ports`, whose 3s budget could freeze the very "scanning ports…" spinner that covers it. The three clipboard readers stay sync on purpose: their Windows branches call the Win32 clipboard rather than the sidecar, and that threading is not something this change can test. The invariant is stated at the helpers themselves, where a new command author will see it, and in the standalone spec's bridge section. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/specs/standalone.md | 10 ++++++++++ standalone/src-tauri/src/lib.rs | 33 +++++++++++++++++++++------------ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/docs/specs/standalone.md b/docs/specs/standalone.md index d8a74e93..a4d66371 100644 --- a/docs/specs/standalone.md +++ b/docs/specs/standalone.md @@ -74,6 +74,16 @@ and reads the bytes in Rust so images never ride the JSON-lines pipe shared with PTY traffic (`docs/specs/dor-browser.md`). Request/response commands block on the sidecar's reply with a timeout; `OPEN_PORT_TIMEOUT_MS` in `lib.rs` mirrors the constant in `lib/src/lib/platform/types.ts` and the two must stay in sync. + +**Blocking commands must be `#[tauri::command(async)]`.** `request_from_sidecar` +and `request_from_sidecar_timeout` block the calling thread on a `recv_timeout`, +and Tauri runs a *plain* sync command on the main thread — where that block stops +the webview from painting for the whole round trip (up to `AGENT_BROWSER_TIMEOUT` += 30s for a hung agent-browser; a cold `agent-browser open` froze the UI ~3s, +long enough that a pane created instantly before it looked like it never +appeared). `(async)` runs the same blocking body on a runtime worker instead. The +three clipboard readers are knowingly still sync: their Windows branches call the +Win32 clipboard directly rather than the sidecar. `pty_graceful_kill_all` (`TauriAdapter.gracefulKillAllPtys`) SIGTERMs every live PTY and awaits the sidecar's `gracefulKillDone` (echoing the request's `requestId`; bounded at `timeout + 1.5s`). `gracefulKillDone` fires early once diff --git a/standalone/src-tauri/src/lib.rs b/standalone/src-tauri/src/lib.rs index ff859e05..650e67e5 100644 --- a/standalone/src-tauri/src/lib.rs +++ b/standalone/src-tauri/src/lib.rs @@ -315,6 +315,15 @@ fn request_from_sidecar( request_from_sidecar_timeout(state, event, data, Duration::from_secs(1)) } +/// INVARIANT: every `#[tauri::command]` that reaches these two blocking helpers +/// must be declared `#[tauri::command(async)]` (or be an `async fn`). Tauri runs +/// a plain sync command on the **main thread**, where the `recv_timeout` below +/// stops the webview from painting for the whole round trip — up to +/// `AGENT_BROWSER_TIMEOUT` (30s) for a hung agent-browser, and a visible ~3s +/// freeze on a cold `agent-browser open`, which is long enough to look like a +/// pane that never appeared. `(async)` moves the same blocking body onto a +/// runtime worker, so the UI keeps rendering while the sidecar works. + fn request_from_sidecar_timeout( state: &SidecarState, event: &str, @@ -417,7 +426,7 @@ fn dor_control_response(state: tauri::State<'_, SidecarState>, response: DorCont send_to_sidecar(&state, msg.to_string()); } -#[tauri::command] +#[tauri::command(async)] fn pty_get_cwd( state: tauri::State<'_, SidecarState>, id: String, @@ -431,7 +440,7 @@ fn pty_get_cwd( // Mirrors `OPEN_PORT_TIMEOUT_MS` in `lib/src/lib/platform/types.ts` — keep in sync. const OPEN_PORT_TIMEOUT_MS: u64 = 3000; -#[tauri::command] +#[tauri::command(async)] fn pty_get_open_ports( state: tauri::State<'_, SidecarState>, id: String, @@ -448,7 +457,7 @@ fn pty_get_open_ports( .unwrap_or_else(|| JsonValue::Array(Vec::new()))) } -#[tauri::command] +#[tauri::command(async)] fn pty_get_scrollback( state: tauri::State<'_, SidecarState>, id: String, @@ -481,7 +490,7 @@ async fn pty_graceful_kill_all( // Stands up the loopback iframe proxy in the sidecar and returns the // IframeProxyResult JSON the webview's IframePanel expects. The proxy server is // the shared lib/src/host/iframe-proxy.ts; this only bridges the request. -#[tauri::command] +#[tauri::command(async)] fn iframe_create_proxy_url( state: tauri::State<'_, SidecarState>, target: String, @@ -514,7 +523,7 @@ fn agent_browser_forward( Ok(response.get("result").cloned().unwrap_or(JsonValue::Null)) } -#[tauri::command] +#[tauri::command(async)] fn agent_browser_command( state: tauri::State<'_, SidecarState>, session: String, @@ -528,7 +537,7 @@ fn agent_browser_command( ) } -#[tauri::command] +#[tauri::command(async)] fn agent_browser_edit( state: tauri::State<'_, SidecarState>, session: String, @@ -542,7 +551,7 @@ fn agent_browser_edit( ) } -#[tauri::command] +#[tauri::command(async)] fn agent_browser_stream_status( state: tauri::State<'_, SidecarState>, session: String, @@ -555,7 +564,7 @@ fn agent_browser_stream_status( ) } -#[tauri::command] +#[tauri::command(async)] fn agent_browser_open( state: tauri::State<'_, SidecarState>, url: String, @@ -570,7 +579,7 @@ fn agent_browser_open( } // `rect` is accepted by the adapter but unused — no window positioning today. -#[tauri::command] +#[tauri::command(async)] fn agent_browser_pop_out( state: tauri::State<'_, SidecarState>, session: String, @@ -584,7 +593,7 @@ fn agent_browser_pop_out( ) } -#[tauri::command] +#[tauri::command(async)] fn agent_browser_pop_in( state: tauri::State<'_, SidecarState>, session: String, @@ -604,7 +613,7 @@ fn agent_browser_pop_in( // decodes with createImageBitmap). A base64 `bytesBase64` field is kept as a // fallback for a stale sidecar bundle (dev-time version skew), but the path // branch is preferred. -#[tauri::command] +#[tauri::command(async)] fn agent_browser_screenshot( state: tauri::State<'_, SidecarState>, session: String, @@ -923,7 +932,7 @@ struct ShellInfo { args: Vec, } -#[tauri::command] +#[tauri::command(async)] fn get_available_shells(state: tauri::State<'_, SidecarState>) -> Result, String> { let response = request_from_sidecar_timeout(&state, "pty:getShells", serde_json::json!({}), Duration::from_secs(10))?; let shells: Vec = response From 0a9278e232b1e97f8528b569b34d76610b8dc5e9 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 22 Jul 2026 14:09:23 -0700 Subject: [PATCH 09/13] Alias dor-lib-common to source in the bundling configs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `connect-port.ts` imports `dor-lib-common/agent-browser`, and `connect-port` is reachable from the webview `Wall` graph — making `dor-lib-common` the first package pulled into the webview bundle/typecheck through its package `exports`, which resolve to a `dist/` that three CI jobs never build: - Standalone Smoketest — `npx tsc --noEmit` with no `dor-lib-common` build and no `paths` mapping: TS2307 on the import. - Visual Regression — Storybook build can't resolve the specifier. - Cloudflare Pages — the website build, same unresolved import. Build & Test stayed green only because `pnpm -r run test` builds the `dist` in topo order before lib typechecks; the other three jobs skip that step. This is the `server-lib-common` situation exactly, so it takes the same treatment: alias the bare specifier to source in every bundler config, map it in the standalone tsconfig `paths`. One directory alias covers both the `.` and `/agent-browser` subpaths (tsconfig needs the two entries spelled out). Added to lib's own vite config and the Pocket build too, so neither depends on a pre-built `dist`. Verified with `dor-lib-common/dist` deleted — reproducing the CI condition: the standalone typecheck reproduces TS2307 without the paths entries and passes with them, and the website, Storybook, and Pocket builds all succeed. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/.storybook/main.ts | 5 +++++ lib/vite.config.ts | 4 ++++ lib/vite.pocket.config.ts | 3 +++ standalone/tsconfig.json | 4 +++- website/vite.config.ts | 4 ++++ 5 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/.storybook/main.ts b/lib/.storybook/main.ts index c460fe9e..5f42e176 100644 --- a/lib/.storybook/main.ts +++ b/lib/.storybook/main.ts @@ -30,6 +30,11 @@ const config: StorybookConfig = { // at a `dist` the Storybook/Chromatic job never builds, so alias the bare // specifier to source too. 'server-lib-common': path.resolve(here, '..', '..', 'server-lib-common', 'src'), + // And `Wall` → `useDorControl` → `connect-port` imports + // `dor-lib-common/agent-browser`, whose `exports` point at the same kind of + // unbuilt `dist`. The directory alias covers the subpath and the bare + // specifier both. + 'dor-lib-common': path.resolve(here, '..', '..', 'dor-lib-common', 'src'), }; return config; }, diff --git a/lib/vite.config.ts b/lib/vite.config.ts index 4b2173d8..33ccc141 100644 --- a/lib/vite.config.ts +++ b/lib/vite.config.ts @@ -12,6 +12,10 @@ export default defineConfig({ // path; Vite (and vitest) do not read tsconfig paths, and `dor` has no // package exports, so resolve it to source — the same alias standalone uses. dor: path.resolve(__dirname, "../dor/src"), + // `connect-port.ts` imports `dor-lib-common/agent-browser`; that package's + // `exports` resolve to a `dist` a vitest run has no reason to have built. + // Alias to source so the tests never depend on build order. + "dor-lib-common": path.resolve(__dirname, "../dor-lib-common/src"), }, }, }); diff --git a/lib/vite.pocket.config.ts b/lib/vite.pocket.config.ts index 2760637e..2e3fb2ba 100644 --- a/lib/vite.pocket.config.ts +++ b/lib/vite.pocket.config.ts @@ -22,6 +22,9 @@ export default defineConfig({ // step to generate it). Alias to source, same as the website and // Storybook configs. "server-lib-common": fileURLToPath(new URL("../server-lib-common/src", import.meta.url)), + // `dor-lib-common` has the same unbuilt-`dist` exports problem, reached via + // `Wall` → `useDorControl` → `connect-port`. + "dor-lib-common": fileURLToPath(new URL("../dor-lib-common/src", import.meta.url)), }, }, build: { diff --git a/standalone/tsconfig.json b/standalone/tsconfig.json index 29485e94..b84ec2b4 100644 --- a/standalone/tsconfig.json +++ b/standalone/tsconfig.json @@ -18,7 +18,9 @@ "paths": { "dormouse-lib/*": ["../lib/src/*"], "dor/*": ["../dor/src/*"], - "server-lib-common": ["../server-lib-common/src/index.ts"] + "server-lib-common": ["../server-lib-common/src/index.ts"], + "dor-lib-common": ["../dor-lib-common/src/index.ts"], + "dor-lib-common/agent-browser": ["../dor-lib-common/src/agent-browser.ts"] } }, "include": ["src"] diff --git a/website/vite.config.ts b/website/vite.config.ts index a0e7e70d..fc05662f 100644 --- a/website/vite.config.ts +++ b/website/vite.config.ts @@ -16,6 +16,10 @@ export default defineConfig(({ mode }) => ({ // `server-lib-common`, whose package `exports` resolve to a `dist` this // build never compiles. Alias it to source, exactly like `dormouse-lib`. "server-lib-common": path.resolve(__dirname, "../server-lib-common/src"), + // Same story for `dor-lib-common`: `Wall` → `useDorControl` → `connect-port` + // imports its `./agent-browser` subpath. The directory alias covers both + // that subpath and the bare specifier. + "dor-lib-common": path.resolve(__dirname, "../dor-lib-common/src"), // Wall also imports `dor/*` (protocol + command types); `dor` has no // package `exports`, and vite does not read tsconfig paths, so resolve it // to source — the same alias lib and standalone use. From b37f6e521ca0289b1e006a3d766fd80f446e491d Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 22 Jul 2026 18:21:09 -0700 Subject: [PATCH 10/13] Spec the header context menu's keyboard path; fold title candidates in `>` in command mode opens the selected terminal pane's header context menu, which now embeds the title-candidates table (header row + candidate rows on top, port rows below) instead of offering it as a separate popover. Digit accelerators 1-9 connect ports; arrows rove, Tab cycles, Escape closes; the menu registers as dialog-keyboard-active and restores focus on close. Co-Authored-By: Claude Fable 5 --- docs/specs/dor-browser.md | 37 ++++++++++++++++++---------------- docs/specs/layout.md | 16 +++++++++++---- docs/specs/shortcuts.md | 3 +++ docs/specs/terminal-escapes.md | 2 +- docs/specs/terminal-state.md | 2 +- 5 files changed, 37 insertions(+), 23 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index ed76d652..c4acadb7 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -167,28 +167,31 @@ Source of truth: `lib/src/components/wall/use-dev-server-ports.ts`, ## Pane Context Menu Connect -The terminal pane header's right-click menu (`docs/specs/layout.md` → Pane -header) lists the ports a pane's process tree binds using the **same** per-port -URL selection as `surface.resolveOpen` (`dor ab open ` / -`dor iframe `): `listenerUrlsByPort` in `port-url.ts` groups TCP -listeners into one openable `http://:/` per distinct port -(loopback-reachable bind wins `localhost`; otherwise the bound LAN/Tailnet -address, IPv6 bracketed). - -Clicking a port row reproduces `dor ab open ` against the **default** +The terminal pane header's context menu (`docs/specs/layout.md` → Pane +header — right-click, or `>` in command mode; that spec owns the menu's +layout and keyboard contract) lists the ports a pane's process tree binds +using the **same** per-port URL selection as `surface.resolveOpen` +(`dor ab open ` / `dor iframe `): `listenerUrlsByPort` in +`port-url.ts` groups TCP listeners into one openable `http://:/` +per distinct port (loopback-reachable bind wins `localhost`; otherwise the +bound LAN/Tailnet address, IPv6 bracketed). + +Activating a port row — click, its `1`–`9` digit accelerator, or `Enter` on +the focused row — reproduces `dor ab open ` against the **default** key/session: it runs `agent-browser open ` on that session and reuses-or-creates the session's browser surface — the wall-side mirror of the CLI flow, not the control plane. It is host-gated on `agentBrowserCommand`: absent (e.g. the web demo host), the rows render as inert labels. -**The click reveals its surface.** Unlike `dor ab` (focus-neutral: an agent must -not steal focus from the human), a menu click *is* the human asking to see that -browser, so every arm of the eager lookup below ends by selecting the surface — -reattaching it first when it is minimized, on the same terms as clicking its Door -chip. Selection only: passthrough stays on the terminal that was right-clicked, so -connecting a port never redirects the keyboard. This is why a repeat click on an -already-connected port is visible at all — the session merely re-navigates to the -URL it is already on, so the selection move is the only feedback. +**Activation reveals its surface.** Unlike `dor ab` (focus-neutral: an agent must +not steal focus from the human), activating a menu row — by mouse or keyboard — +*is* the human asking to see that browser, so every arm of the eager lookup below +ends by selecting the surface — reattaching it first when it is minimized, on the +same terms as clicking its Door chip. Selection only: passthrough stays on the +terminal the menu was opened from, so connecting a port never redirects the +keyboard. This is why a repeat activation on an already-connected port is visible +at all — the session merely re-navigates to the URL it is already on, so the +selection move is the only feedback. Source of truth: `revealSurface` in `Wall.tsx`, threaded into `useDorControl`. **Instant create.** The click is fire-and-forget: the menu closes at once and the diff --git a/docs/specs/layout.md b/docs/specs/layout.md index 0419b709..d4158e3a 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -77,15 +77,23 @@ The content area is a tiling layout of panes rendered by Lath (`docs/specs/tilin Each pane has a 30px header that doubles as a drag handle (a `pointerdown` on the header, past a 5px threshold, begins a Lath pane drag; below the threshold the header's own click behavior stands). The header uses `cursor-grab` / `active:cursor-grabbing`, `select-none`, and the shared terminal top radius from `lib/src/components/design.tsx`. Background and foreground use the `--color-header-active-*` / `--color-header-inactive-*` token pairs, which map to VSCode file-tree list colors. -The header label is the `DerivedHeader` returned by `deriveHeader(paneState, visiblePanes)` in `docs/specs/terminal-state.md` — that spec is the single source of truth for the priority chain (user-pinned title, app-sent overrides, current command title, ` ${LAST_TITLE}` for finished panes, plain `` for fresh panes), the disambiguator rule, and which OSC sources contribute. Layout's job is to render the result: the primary label truncates with ellipsis, the secondary label (when present) is shown muted next to it, click renames/pins, right-click opens the header context menu (which offers the diagnostic popup as a `title candidates` item). +The header label is the `DerivedHeader` returned by `deriveHeader(paneState, visiblePanes)` in `docs/specs/terminal-state.md` — that spec is the single source of truth for the priority chain (user-pinned title, app-sent overrides, current command title, ` ${LAST_TITLE}` for finished panes, plain `` for fresh panes), the disambiguator rule, and which OSC sources contribute. Layout's job is to render the result: the primary label truncates with ellipsis, the secondary label (when present) is shown muted next to it, click renames/pins, right-click — or `>` in command mode — opens the header context menu, which leads with the title-candidates table. -The diagnostic popup lists the latest entry per `titleCandidates` channel as defined in `docs/specs/terminal-state.md`. Each row shows the channel, latest candidate text, and timestamp. The popup is diagnostic only; it does not change the title priority rules. +Right-clicking anywhere on the header opens the pane's single **context menu** at the pointer; pressing `>` in command mode opens the same menu for the selected pane, anchored under the header's left edge (the handler finds the header via `data-pane-header-for` and dispatches a synthetic `contextmenu` at that corner, so both paths share one code path; browser surfaces and Doors have no such header, so `>` no-ops there). Only the alert bell owns its own right-click (`stopPropagation`, opening the alert dialog); every other region — including the title span — bubbles to this one menu. It is portaled to `document.body`, viewport-clamped, and dismissed by outside `pointerdown`, `Escape`, `resize`, or capture-phase `scroll` — except scrolls originating inside the menu itself, which must not dismiss it (arrow-key focus moves auto-scroll the overflowing list). -Right-clicking anywhere on the header opens a single **context menu** at the pointer. Only the alert bell owns its own right-click (`stopPropagation`, opening the alert dialog); every other region — including the title span — bubbles to this one menu. It is portaled to `document.body`, viewport-clamped, and dismissed like the diagnostic popup (outside `pointerdown`, `Escape`, `resize`, capture-phase `scroll`). It shows the pane's `surface:N` handle (`resolveSurfaceRef`) as a non-interactive monospace row, then the TCP ports the pane's process tree binds: a spinner while `getOpenPorts` runs (once per open — right-click again to rescan), then one `host:port` row per distinct port (the process name muted beside it), or a muted `no listening ports` / `port scan failed` line, then a `title candidates` item that opens the diagnostic popup (anchored to the title span, or the pointer while renaming). Clicking a port row reproduces `dor ab open ` for that port and closes the menu at once (`docs/specs/dor-browser.md` → Pane Context Menu Connect): the browser surface appears immediately and becomes the selection (reattaching first if it was minimized), and loading/errors surface in the pane, not the menu; the rows are inert labels on hosts that cannot open a browser surface. Only terminal panes have this menu. Source of truth: `PaneHeaderContextMenu.tsx`, `TerminalPaneHeader.tsx`. +Content, top to bottom: + +- **Header row** — the current display title, the pane's `surface:N` handle (`resolveSurfaceRef`, muted), and a close button. +- **Title-candidates table** — the latest entry per `titleCandidates` channel as defined in `docs/specs/terminal-state.md`: channel, latest candidate text, and timestamp per row, or a muted `No title candidates` line. Diagnostic only; it does not change the title priority rules. +- **Port rows** — the TCP ports the pane's process tree binds: a spinner while `getOpenPorts` runs (once per open — reopen to rescan), then one `host:port` row per distinct port (digit accelerator chip first, process name muted beside it), or a muted `no listening ports` / `port scan failed` line. + +The menu owns the keyboard while open: it takes DOM focus on mount and restores the previously focused element on close (so a passthrough terminal gets its keys back), and it registers as dialog-keyboard-active so command-mode keys don't fire underneath (`use-wall-keyboard.ts`). `1`–`9` activate the corresponding port row; presses while the scan is still running are dropped, not buffered — the spinner explains why nothing happened. `↑`/`↓` rove focus across the port rows (wrapping), `Enter`/`Space` activate the focused row, `Tab`/`Shift+Tab` cycle every focusable element, and `Escape` closes. + +Activating a port row (click, digit, or `Enter` on the focused row) reproduces `dor ab open ` for that port and closes the menu at once (`docs/specs/dor-browser.md` → Pane Context Menu Connect): the browser surface appears immediately and becomes the selection (reattaching first if it was minimized) — the one command-mode gesture whose side effect moves selection off the pane it targeted — and loading/errors surface in the pane, not the menu. On hosts that cannot open a browser surface the rows are inert labels with no digit chips, and digits do nothing. Only terminal panes have this menu. Source of truth: `PaneHeaderContextMenu.tsx`, `TerminalPaneHeader.tsx`, `handle-pane-shortcuts.ts`. Elements from left to right: -- Derived session label (click to rename/pin, right-click opens the header context menu whose `title candidates` item inspects the candidates, truncates with ellipsis) +- Derived session label (click to rename/pin, right-click opens the header context menu with the title-candidates table, truncates with ellipsis) - Alert bell button (reflects session activity status) - TODO pill (if todo state is set; hidden in minimal tier) - Flexible gap diff --git a/docs/specs/shortcuts.md b/docs/specs/shortcuts.md index 967a917f..1a4f9f67 100644 --- a/docs/specs/shortcuts.md +++ b/docs/specs/shortcuts.md @@ -29,6 +29,7 @@ In the VS Code extension host, selected workbench chords are mirrored: the termi | `,` | Rename | Enter rename mode for the selected pane's title. | | `a` | Toggle alert | Dismiss or toggle the bell alert for the selected pane. Meaningful only for a terminal Surface — a browser surface has no bell to ring (`docs/specs/glossary.md`). | | `t` | Toggle todo | Toggle the TODO marker on or off for the selected pane's Surface. Works on any Surface — a terminal Session or a browser surface. | +| `>` | Header context menu | Open the selected pane's header context menu — current title + `surface:N`, title candidates, and bound ports with digit-to-connect (mirrors tmux's pane `display-menu` binding). Terminal panes only; no-op on browser surfaces and doors. | ## Navigation (command mode) @@ -65,6 +66,8 @@ On macOS, `Ctrl+C` passes through to the running program (only `⌘C` copies); ` | Prompted character | Confirm kill | Type the character shown in the kill prompt to confirm termination. | | `a` (alert dialog open) | Toggle alert | Same as command-mode `a`. | | `t` (alert dialog open) | Toggle todo | Same as command-mode `t`. | +| `1`–`9` (header context menu open) | Connect port | Open a browser surface on the nth port row. Dropped while the port scan is running; inert on hosts that can't open a browser surface. | +| `↑` / `↓` (header context menu open) | Move row focus | Rove focus across port rows, wrapping; `Enter` or `Space` activates the focused row. | ## Implementation references diff --git a/docs/specs/terminal-escapes.md b/docs/specs/terminal-escapes.md index 7a62a30f..0eea3400 100644 --- a/docs/specs/terminal-escapes.md +++ b/docs/specs/terminal-escapes.md @@ -63,7 +63,7 @@ Unknown non-iTerm2 OSC families pass through to xterm.js unchanged so xterm.js c (`BEL` is not itself an OSC; it has a row here because a standalone `BEL` is parsed and stripped at the same PTY data boundary as the OSCs.) -Some sequences are dual-purpose. The notification rows for `OSC 9 ; ST`, `OSC 99` (`p=title`/`p=body`), and `OSC 777 ; notify` also feed the title-candidate channel in `terminal-state.md` — see its [Title candidate diagnostics](terminal-state.md#supported-osc-inputs) table. Only the OSC 9 *message* form can become a header/door label; OSC 99 and OSC 777 candidates are stored for the diagnostic popup only. The OSC 9 *progress* form (`OSC 9 ; 4`) carries no text and never contributes a title candidate. +Some sequences are dual-purpose. The notification rows for `OSC 9 ; ST`, `OSC 99` (`p=title`/`p=body`), and `OSC 777 ; notify` also feed the title-candidate channel in `terminal-state.md` — see its [Title candidate diagnostics](terminal-state.md#supported-osc-inputs) table. Only the OSC 9 *message* form can become a header/door label; OSC 99 and OSC 777 candidates are stored only for the diagnostic title-candidates table in the header context menu. The OSC 9 *progress* form (`OSC 9 ; 4`) carries no text and never contributes a title candidate. #### OSC color queries on Windows require the bundled ConPTY diff --git a/docs/specs/terminal-state.md b/docs/specs/terminal-state.md index e692435e..81a8642d 100644 --- a/docs/specs/terminal-state.md +++ b/docs/specs/terminal-state.md @@ -238,7 +238,7 @@ Header priority — first match wins: When the finished command exited non-zero, a trailing fail glyph (`✗`) is appended — ` ${LAST_TITLE} ✗` — and `lastCommandFailed` is set on the result. "Failed" requires a real non-zero `exitCode`: the keystroke fallback never records one, so it shows no glyph either way. The glyph rides in `primary` so plain-text title consumers (OS/tab titles) carry it, while the pane header reads `lastCommandFailed` to color it red without re-parsing the string. 4. Otherwise (no running command and no last command): ``. -Rich notification titles from `OSC 99` and `OSC 777` are stored in `titleCandidates` for the diagnostic popup but never become header/door labels. Older shell titles (terminal titles emitted before the current command started, or after the last command finished) remain fallback-only and do not replace `` or pollute `LAST_TITLE`. +Rich notification titles from `OSC 99` and `OSC 777` are stored in `titleCandidates` for the diagnostic title-candidates table (in the header context menu) but never become header/door labels. Older shell titles (terminal titles emitted before the current command started, or after the last command finished) remain fallback-only and do not replace `` or pollute `LAST_TITLE`. ` ${LAST_TITLE}` keeps the just-finished context visible so the user can see at a glance which program just exited. The header surfaces failure minimally — the trailing `✗` glyph for a non-zero exit, nothing more; output and TODO notification are still surfaced via the alert/TODO machinery (`docs/specs/alert.md`). ` ${LAST_TITLE}` persists across subsequent prompt/editing transitions until a new `commandStart` replaces it; only a fresh pane (no `lastCommand` at all) shows plain ``. From a7c92b667d68bb228b4605d6029db2a6f7e946f4 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 22 Jul 2026 18:30:50 -0700 Subject: [PATCH 11/13] Fold the title-candidates table into the header context menu The menu now leads with a header row (display title, surface:N, close) and the diagnostic title-candidates table, with the port rows below; the separate TitleCandidatesPopover and the menu's "title candidates" item are gone. Co-Authored-By: Claude Fable 5 --- .../wall/PaneHeaderContextMenu.test.tsx | 55 +++++++-- .../components/wall/PaneHeaderContextMenu.tsx | 72 ++++++++--- .../components/wall/TerminalPaneHeader.tsx | 114 +----------------- lib/src/stories/ShellCwd.stories.tsx | 10 +- 4 files changed, 107 insertions(+), 144 deletions(-) diff --git a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx index f572efeb..04024de2 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx @@ -11,6 +11,8 @@ import { ensureResizeObserver, stubWallActions as stubActions } from './wall-tes import { FakePtyAdapter } from '../../lib/platform/fake-adapter'; import { setPlatform } from '../../lib/platform'; import type { OpenPort, PlatformAdapter } from '../../lib/platform/types'; +import { removeTerminalPaneState, resetTerminalPaneState } from '../../lib/terminal-registry'; +import type { TerminalTitle, TerminalTitleSource } from '../../lib/terminal-state'; globalThis.IS_REACT_ACT_ENVIRONMENT = true; @@ -18,6 +20,10 @@ function headerProps(id: string, title: string): PaneProps { return { id, title, params: undefined }; } +function candidate(title: string, source: TerminalTitleSource, updatedAt: number): TerminalTitle { + return { title, source, updatedAt }; +} + function loopbackPort(port: number, processName?: string): OpenPort { return { protocol: 'tcp', family: 'IPv4', address: '127.0.0.1', port, pid: 100, processName }; } @@ -44,6 +50,7 @@ afterEach(() => { act(() => root.unmount()); container.remove(); platform.reset(); + removeTerminalPaneState('term-1'); }); function renderHeader(props: PaneProps, actions: WallActions) { @@ -148,7 +155,7 @@ describe('PaneHeaderContextMenu — pane header right-click', () => { expect(menuFor('term-1')).toBeNull(); }); - it('opens the one context menu on a title-span right-click (not a second popover)', async () => { + it('opens the one context menu when a title-span right-click bubbles up', async () => { renderHeader(headerProps('term-1', 'title'), stubActions()); const titleSpan = container.querySelector('[data-title-candidates-for="term-1"]') as HTMLElement; @@ -158,20 +165,50 @@ describe('PaneHeaderContextMenu — pane header right-click', () => { }); expect(menuFor('term-1')).not.toBeNull(); - // The title span no longer opens its own popover — only the context menu. - expect(document.body.querySelector('[aria-label="Title candidates"]')).toBeNull(); }); - it('opens the title-candidates popover from the menu item and closes the menu', async () => { - renderHeader(headerProps('term-1', 'title'), stubActions()); + it('renders the header row (display title + surface ref) and the title-candidates table inline', async () => { + const resolveSurfaceRef = vi.fn(() => 'surface:7'); + resetTerminalPaneState('term-1', { + title: candidate('Pinned API', 'user', 6_000), + titleCandidates: { + user: candidate('Pinned API', 'user', 6_000), + osc0: candidate('dev server', 'osc0', 1_000), + osc99: candidate('Codex waiting', 'osc99', 4_000), + }, + }); + renderHeader(headerProps('term-1', 't'), stubActions({ resolveSurfaceRef })); + + await act(async () => { fireContextMenu(); }); + + const menu = menuFor('term-1'); + expect(menu).not.toBeNull(); + // Header row: current display title + surface ref. + expect(menu?.textContent).toContain('Pinned API'); + expect(menu?.textContent).toContain('surface:7'); + // Candidates table: one row per channel (source label + candidate title). + expect(menu?.textContent).toContain('OSC 0'); + expect(menu?.textContent).toContain('dev server'); + expect(menu?.textContent).toContain('OSC 99'); + expect(menu?.textContent).toContain('Codex waiting'); + expect(menu?.textContent).not.toContain('No title candidates'); + }); + + it('shows the empty title-candidates line when the pane has no candidates', async () => { + renderHeader(headerProps('term-1', 't'), stubActions()); await act(async () => { fireContextMenu(); }); - const item = menuFor('term-1')?.querySelector('[data-title-candidates-item]') as HTMLButtonElement; - expect(item).not.toBeNull(); + expect(menuFor('term-1')?.textContent).toContain('No title candidates'); + }); + + it('closes the menu when the header close button is clicked', async () => { + renderHeader(headerProps('term-1', 't'), stubActions()); - await act(async () => { item.dispatchEvent(new MouseEvent('click', { bubbles: true })); }); + await act(async () => { fireContextMenu(); }); + const closeButton = menuFor('term-1')?.querySelector('button[aria-label="Close menu"]') as HTMLButtonElement; + expect(closeButton).not.toBeNull(); + await act(async () => { closeButton.dispatchEvent(new MouseEvent('click', { bubbles: true })); }); expect(menuFor('term-1')).toBeNull(); - expect(document.body.querySelector('[role="dialog"][aria-label="Title candidates"]')).not.toBeNull(); }); }); diff --git a/lib/src/components/wall/PaneHeaderContextMenu.tsx b/lib/src/components/wall/PaneHeaderContextMenu.tsx index b3ff539e..5b69a85f 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.tsx @@ -1,10 +1,11 @@ import { useContext, useEffect, useLayoutEffect, useRef, useState, type CSSProperties } from 'react'; import { createPortal } from 'react-dom'; -import { CircleNotchIcon } from '@phosphor-icons/react'; +import { CircleNotchIcon, XIcon } from '@phosphor-icons/react'; import { POPUP_SURFACE_CLASS } from '../design'; import { clampOverlayPosition } from '../../lib/ui-geometry'; import { getPlatform } from '../../lib/platform'; import type { OpenPort } from '../../lib/platform/types'; +import { titleSourceLabel, type TerminalTitle } from '../../lib/terminal-state'; import { listenerUrlsByPort, type PortUrlEntry } from './port-url'; import { useDismissOverlay } from './use-dismiss-overlay'; import { WallActionsContext } from './wall-context'; @@ -14,14 +15,15 @@ type ScanState = | { status: 'loaded'; entries: PortUrlEntry[] } | { status: 'failed' }; -// One recipe for both interactive rows (port entries, title candidates). +// One recipe for the interactive port-entry rows. const MENU_ROW_CLASS = 'flex w-full items-baseline gap-2 px-2.5 py-1 text-left hover:bg-foreground/10'; /** * The right-click menu on a terminal pane header (`docs/specs/layout.md` → Pane - * header): the pane's `surface:N` handle, then the TCP ports its process tree - * binds, then a "title candidates" row that opens the diagnostic popover. - * Clicking a port fires `dor ab open ` via `WallActions.onConnectPort` + * header): a header row with the current display title, the pane's `surface:N` + * handle, and a close button; then the diagnostic title-candidates table (latest + * entry per channel); then the TCP ports the pane's process tree binds. Clicking a + * port fires `dor ab open ` via `WallActions.onConnectPort` * (`docs/specs/dor-browser.md` → Pane Context Menu Connect) and closes the menu * immediately — the new pane's own "Connecting…" placeholder is the loading * feedback, and a failure is logged, not shown here. Port rows are only clickable @@ -32,12 +34,14 @@ export function PaneHeaderContextMenu({ id, anchor, onClose, - onShowTitleCandidates, + candidates, + currentTitle, }: { id: string; anchor: { x: number; y: number }; onClose: () => void; - onShowTitleCandidates: () => void; + candidates: TerminalTitle[]; + currentTitle: string; }) { const actions = useContext(WallActionsContext); const ref = useRef(null); @@ -89,7 +93,35 @@ export function PaneHeaderContextMenu({ onMouseDown={(e) => e.stopPropagation()} onContextMenu={(e) => e.preventDefault()} > -
{actions.resolveSurfaceRef(id)}
+
+ {currentTitle} + {actions.resolveSurfaceRef(id)} + +
+
+ {candidates.length === 0 ? ( +
No title candidates
+ ) : ( +
+ {candidates.map((candidate) => ( +
+ {titleSourceLabel(candidate.source)} + {candidate.title} + +
+ ))} +
+ )} +
{scan.status === 'scanning' && (
@@ -127,17 +159,21 @@ export function PaneHeaderContextMenu({
); })} -
-
, document.body, ); } + +function formatTitleCandidateTime(timestamp: number): string { + if (!Number.isFinite(timestamp)) return 'unknown'; + return new Date(timestamp).toLocaleTimeString([], { + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + }); +} + +function formatTitleCandidateDateTime(timestamp: number): string | undefined { + if (!Number.isFinite(timestamp)) return undefined; + return new Date(timestamp).toISOString(); +} diff --git a/lib/src/components/wall/TerminalPaneHeader.tsx b/lib/src/components/wall/TerminalPaneHeader.tsx index 851b15fa..239649b5 100644 --- a/lib/src/components/wall/TerminalPaneHeader.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.tsx @@ -20,7 +20,6 @@ import { useTodoPillContent } from '../TodoPillBody'; import type { PaneProps } from './pane-props'; import { IllegalRenameWarning, type RenameRejection } from './IllegalRenameWarning'; import { PaneHeaderContextMenu } from './PaneHeaderContextMenu'; -import { useDismissOverlay } from './use-dismiss-overlay'; import { DEFAULT_MOUSE_SELECTION_STATE, getMouseSelectionSnapshot, @@ -43,8 +42,6 @@ import { deriveHeader, resolveDisplayPrimary, titleCandidatesForDisplay, - titleSourceLabel, - type TerminalTitle, } from '../../lib/terminal-state'; import { DialogKeyboardContext, @@ -81,8 +78,6 @@ const ALERT_BUTTON_LABELS: Record(null); - const titleSpanRef = useRef(null); const suppressAlertClickRef = useRef(false); const [tier, setTier] = useState('full'); const [dialogTriggerRect, setDialogTriggerRect] = useState(null); const [todoPreviewRect, setTodoPreviewRect] = useState(null); - const [titleCandidatesRect, setTitleCandidatesRect] = useState(null); const [contextMenu, setContextMenu] = useState<{ x: number; y: number } | null>(null); const [renameWarning, setRenameWarning] = useState<{ rect: DOMRect; reason: RenameRejection; value: string } | null>(null); const todoPill = useTodoPillContent(activity.todo); @@ -148,7 +141,6 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { const closeDialog = useCallback(() => setDialogTriggerRect(null), []); const closeTodoPreview = useCallback(() => setTodoPreviewRect(null), []); - const closeTitleCandidates = useCallback(() => setTitleCandidatesRect(null), []); const closeContextMenu = useCallback(() => setContextMenu(null), []); const closeRenameWarning = useCallback(() => setRenameWarning(null), []); const submitRename = useCallback((value: string, anchor: HTMLElement) => { @@ -224,7 +216,6 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { /> ) : ( e.stopPropagation()} @@ -388,24 +379,13 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { anchorRect={todoPreviewRect} /> )} - {titleCandidatesRect && !dialogTriggerRect && ( - - )} {contextMenu && ( setTitleCandidatesRect( - titleSpanRef.current?.getBoundingClientRect() ?? new DOMRect(contextMenu.x, contextMenu.y, 0, 0), - )} + candidates={titleCandidates} + currentTitle={displayTitleBase} /> )} {renameWarning && ( @@ -420,82 +400,6 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { ); } -function TitleCandidatesPopover({ - anchorRect, - candidates, - currentTitle, - onClose, -}: { - anchorRect: DOMRect; - candidates: TerminalTitle[]; - currentTitle: string; - onClose: () => void; -}) { - const ref = useRef(null); - const [style, setStyle] = useState({ - position: 'fixed', - left: anchorRect.left, - top: anchorRect.bottom + TITLE_CANDIDATES_GAP, - }); - - useDismissOverlay(onClose); - - useLayoutEffect(() => { - const el = ref.current; - if (!el) return; - const rect = el.getBoundingClientRect(); - const top = anchorRect.bottom + TITLE_CANDIDATES_GAP; - const maxLeft = Math.max(TITLE_CANDIDATES_MARGIN, window.innerWidth - rect.width - TITLE_CANDIDATES_MARGIN); - setStyle({ - position: 'fixed', - left: Math.min(Math.max(anchorRect.left, TITLE_CANDIDATES_MARGIN), maxLeft), - top, - maxHeight: Math.max(80, window.innerHeight - top - TITLE_CANDIDATES_MARGIN), - }); - }, [anchorRect]); - - return createPortal( -
e.stopPropagation()} - onMouseDown={(e) => e.stopPropagation()} - onContextMenu={(e) => e.preventDefault()} - > -
-
{currentTitle}
- -
- {candidates.length === 0 ? ( -
No title candidates
- ) : ( -
- {candidates.map((candidate) => ( -
- {titleSourceLabel(candidate.source)} - {candidate.title} - -
- ))} -
- )} -
, - document.body, - ); -} - function TodoNotificationPreview({ id, notification, @@ -562,17 +466,3 @@ function formatNotificationPreview(notification: { title: string | null; body: s const preview = parts.join('\n'); return preview.length > 512 ? `${preview.slice(0, 509)}...` : preview; } - -function formatTitleCandidateTime(timestamp: number): string { - if (!Number.isFinite(timestamp)) return 'unknown'; - return new Date(timestamp).toLocaleTimeString([], { - hour: '2-digit', - minute: '2-digit', - second: '2-digit', - }); -} - -function formatTitleCandidateDateTime(timestamp: number): string | undefined { - if (!Number.isFinite(timestamp)) return undefined; - return new Date(timestamp).toISOString(); -} diff --git a/lib/src/stories/ShellCwd.stories.tsx b/lib/src/stories/ShellCwd.stories.tsx index ad770bdd..0f0bac17 100644 --- a/lib/src/stories/ShellCwd.stories.tsx +++ b/lib/src/stories/ShellCwd.stories.tsx @@ -123,16 +123,16 @@ export const TitleFallbacksAndPinnedTitles: Story = storyFor([ caseState('title-long-user', 'Long user title', idle({ cwd: manual('/repo/app'), title: terminalTitle('my-extremely-long-running-background-process-with-a-very-descriptive-name', 'user') }), 'Truncates before controls'), ]); -export const TitleCandidatePopup: Story = { +export const TitleCandidatesInHeaderMenu: Story = { ...storyFor([ caseState( 'title-candidates-popup', - 'Title candidates popup', + 'Title candidates in header menu', titleCandidateState(), - 'Right-click popup lists each latest title channel', + 'Right-click menu embeds the latest title per channel above the port rows', ), ]), - play: openTitleCandidatesPopup, + play: openHeaderContextMenu, }; export const GroupingKeys: Story = storyFor([ @@ -389,7 +389,7 @@ function wait(ms: number) { return new Promise((resolve) => setTimeout(resolve, ms)); } -async function openTitleCandidatesPopup() { +async function openHeaderContextMenu() { await wait(100); const title = document.querySelector('[data-title-candidates-for="title-candidates-popup"]'); if (!title) return; From 3f55319833ed20f6897fe1cb832f374c5623d585 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 22 Jul 2026 18:43:47 -0700 Subject: [PATCH 12/13] Keyboard path into the header context menu `>` in command mode opens the selected terminal pane's menu (synthetic contextmenu at the header's bottom-left; browser surfaces and doors no-op). While open the menu owns the keyboard: it takes DOM focus (restored on close), reports dialog-keyboard-active, roves ArrowUp/Down across port rows, connects with digit accelerators 1-9 (shown as chips, dropped while the scan runs), and traps Tab/Escape via the focus trap extracted from TodoAlertDialog. Scrolls inside the menu no longer dismiss it. Co-Authored-By: Claude Fable 5 --- lib/src/components/TodoAlertDialog.tsx | 53 +----- lib/src/components/use-popover-focus-trap.ts | 53 ++++++ .../wall/PaneHeaderContextMenu.test.tsx | 154 +++++++++++++++++- .../components/wall/PaneHeaderContextMenu.tsx | 81 ++++++++- .../components/wall/TerminalPaneHeader.tsx | 2 + .../keyboard/handle-pane-shortcuts.test.ts | 45 ++++- .../wall/keyboard/handle-pane-shortcuts.ts | 28 +++- .../components/wall/use-dismiss-overlay.ts | 23 ++- 8 files changed, 369 insertions(+), 70 deletions(-) create mode 100644 lib/src/components/use-popover-focus-trap.ts diff --git a/lib/src/components/TodoAlertDialog.tsx b/lib/src/components/TodoAlertDialog.tsx index 0e3b516a..178b44f6 100644 --- a/lib/src/components/TodoAlertDialog.tsx +++ b/lib/src/components/TodoAlertDialog.tsx @@ -2,6 +2,7 @@ import { useLayoutEffect, useEffect, useRef, useState, useSyncExternalStore, typ import { createPortal } from 'react-dom'; import { XIcon } from '@phosphor-icons/react'; import { Shortcut } from './design'; +import { usePopoverFocusTrap } from './use-popover-focus-trap'; import { clampOverlayPosition, pointInConvexPolygon } from '../lib/ui-geometry'; import { clearSessionTodo, @@ -16,58 +17,6 @@ import { toggleSessionTodo, } from '../lib/terminal-registry'; -/** - * Manages focus trapping, Escape-to-close, and click-outside-to-close for - * portal-based popovers. Scopes keyboard handling to the popover's DOM subtree - * so Tab/Escape don't leak to the rest of the app. - */ -function usePopoverFocusTrap( - ref: React.RefObject, - onClose: () => void, -) { - useEffect(() => { - const el = ref.current; - if (!el) return; - - const handleMouseDown = (e: MouseEvent) => { - if (!el.contains(e.target as Node)) onClose(); - }; - - const handleKeyDown = (e: KeyboardEvent) => { - // Only handle keys when focus is inside the popover - if (!el.contains(document.activeElement)) return; - - if (e.key === 'Escape') { - e.preventDefault(); - e.stopPropagation(); - onClose(); - return; - } - if (e.key !== 'Tab') return; - - const focusables = Array.from( - el.querySelectorAll('button:not([disabled]), [tabindex]:not([tabindex="-1"])'), - ); - if (focusables.length === 0) return; - - const currentIndex = focusables.findIndex((f) => f === document.activeElement); - const nextIndex = currentIndex === -1 - ? 0 - : (currentIndex + (e.shiftKey ? -1 : 1) + focusables.length) % focusables.length; - - e.preventDefault(); - focusables[nextIndex]?.focus(); - }; - - window.addEventListener('mousedown', handleMouseDown); - window.addEventListener('keydown', handleKeyDown, true); - return () => { - window.removeEventListener('mousedown', handleMouseDown); - window.removeEventListener('keydown', handleKeyDown, true); - }; - }, [ref, onClose]); -} - export function TodoAlertDialog({ triggerRect, sessionId, diff --git a/lib/src/components/use-popover-focus-trap.ts b/lib/src/components/use-popover-focus-trap.ts new file mode 100644 index 00000000..b0177bd5 --- /dev/null +++ b/lib/src/components/use-popover-focus-trap.ts @@ -0,0 +1,53 @@ +import { useEffect } from 'react'; + +/** + * Manages focus trapping, Escape-to-close, and click-outside-to-close for + * portal-based popovers. Scopes keyboard handling to the popover's DOM subtree + * so Tab/Escape don't leak to the rest of the app. + */ +export function usePopoverFocusTrap( + ref: React.RefObject, + onClose: () => void, +) { + useEffect(() => { + const el = ref.current; + if (!el) return; + + const handleMouseDown = (e: MouseEvent) => { + if (!el.contains(e.target as Node)) onClose(); + }; + + const handleKeyDown = (e: KeyboardEvent) => { + // Only handle keys when focus is inside the popover + if (!el.contains(document.activeElement)) return; + + if (e.key === 'Escape') { + e.preventDefault(); + e.stopPropagation(); + onClose(); + return; + } + if (e.key !== 'Tab') return; + + const focusables = Array.from( + el.querySelectorAll('button:not([disabled]), [tabindex]:not([tabindex="-1"])'), + ); + if (focusables.length === 0) return; + + const currentIndex = focusables.findIndex((f) => f === document.activeElement); + const nextIndex = currentIndex === -1 + ? 0 + : (currentIndex + (e.shiftKey ? -1 : 1) + focusables.length) % focusables.length; + + e.preventDefault(); + focusables[nextIndex]?.focus(); + }; + + window.addEventListener('mousedown', handleMouseDown); + window.addEventListener('keydown', handleKeyDown, true); + return () => { + window.removeEventListener('mousedown', handleMouseDown); + window.removeEventListener('keydown', handleKeyDown, true); + }; + }, [ref, onClose]); +} diff --git a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx index 04024de2..42796b32 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx @@ -6,7 +6,7 @@ import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { PaneProps } from './pane-props'; import { TerminalPaneHeader } from './TerminalPaneHeader'; -import { WallActionsContext, type WallActions } from './wall-context'; +import { DialogKeyboardContext, WallActionsContext, type WallActions } from './wall-context'; import { ensureResizeObserver, stubWallActions as stubActions } from './wall-test-utils'; import { FakePtyAdapter } from '../../lib/platform/fake-adapter'; import { setPlatform } from '../../lib/platform'; @@ -36,6 +36,7 @@ function enableConnect(platform: FakePtyAdapter): void { let container: HTMLDivElement; let root: Root; let platform: FakePtyAdapter; +let keyboardActiveSpy: ReturnType; beforeEach(() => { container = document.createElement('div'); @@ -44,6 +45,7 @@ beforeEach(() => { platform = new FakePtyAdapter(); setPlatform(platform); ensureResizeObserver(); + keyboardActiveSpy = vi.fn(); }); afterEach(() => { @@ -57,9 +59,11 @@ function renderHeader(props: PaneProps, actions: WallActions) { act(() => { root.render( - - - + + + + + , ); }); @@ -212,3 +216,145 @@ describe('PaneHeaderContextMenu — pane header right-click', () => { expect(menuFor('term-1')).toBeNull(); }); }); + +const DEFAULT_KB_PORTS: OpenPort[] = [loopbackPort(3000, 'alpha'), loopbackPort(5173, 'beta')]; + +function keydown(key: string, init: KeyboardEventInit = {}): KeyboardEvent { + return new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...init }); +} + +/** Enable connect, spawn a pty with `ports`, render, and open the menu. */ +async function openConnectableMenu( + onConnectPort: ReturnType = vi.fn(), + ports: OpenPort[] = DEFAULT_KB_PORTS, +): Promise> { + enableConnect(platform); + platform.spawnPty('term-1'); + platform.setOpenPorts('term-1', ports); + renderHeader(headerProps('term-1', 't'), stubActions({ onConnectPort })); + await act(async () => { fireContextMenu(); }); + return onConnectPort; +} + +describe('PaneHeaderContextMenu — keyboard access', () => { + it('takes DOM focus on the menu container when it opens', async () => { + await openConnectableMenu(); + expect(document.activeElement).toBe(menuFor('term-1')); + }); + + it('reports dialog-keyboard-active while open and inactive once closed', async () => { + await openConnectableMenu(); + expect(keyboardActiveSpy).toHaveBeenLastCalledWith(true); + + act(() => { window.dispatchEvent(keydown('Escape')); }); + expect(menuFor('term-1')).toBeNull(); + expect(keyboardActiveSpy).toHaveBeenLastCalledWith(false); + }); + + it('renders digit chips and connects the nth port row when its digit is pressed, then closes', async () => { + const onConnectPort = vi.fn(); + await openConnectableMenu(onConnectPort); + // First 9 connectable rows carry a digit accelerator chip. + expect(menuFor('term-1')?.textContent).toContain('[1]'); + expect(menuFor('term-1')?.textContent).toContain('[2]'); + + act(() => { window.dispatchEvent(keydown('2')); }); + // Ports sort ascending: 3000 → [1], 5173 → [2]. + expect(onConnectPort).toHaveBeenCalledWith('term-1', 'http://localhost:5173/'); + expect(menuFor('term-1')).toBeNull(); + }); + + it('drops a digit press while the port scan is still running', async () => { + enableConnect(platform); + platform.spawnPty('term-1'); + platform.getOpenPorts = () => new Promise(() => {}); // never resolves + const onConnectPort = vi.fn(); + renderHeader(headerProps('term-1', 't'), stubActions({ onConnectPort })); + + act(() => { fireContextMenu(); }); + expect(menuFor('term-1')?.textContent).toContain('scanning ports'); + + act(() => { window.dispatchEvent(keydown('1')); }); + expect(onConnectPort).not.toHaveBeenCalled(); + expect(menuFor('term-1')).not.toBeNull(); + }); + + it('ignores an out-of-range digit', async () => { + const onConnectPort = vi.fn(); + await openConnectableMenu(onConnectPort); + + act(() => { window.dispatchEvent(keydown('5')); }); + expect(onConnectPort).not.toHaveBeenCalled(); + expect(menuFor('term-1')).not.toBeNull(); + }); + + it('renders no digit chips and ignores digits on an inert host', async () => { + platform.spawnPty('term-1'); + platform.setOpenPorts('term-1', DEFAULT_KB_PORTS); + const onConnectPort = vi.fn(); + renderHeader(headerProps('term-1', 't'), stubActions({ onConnectPort })); + + await act(async () => { fireContextMenu(); }); + expect(menuFor('term-1')?.textContent).not.toContain('[1]'); + + act(() => { window.dispatchEvent(keydown('1')); }); + expect(onConnectPort).not.toHaveBeenCalled(); + expect(menuFor('term-1')).not.toBeNull(); + }); + + it('roves focus across port rows with the arrow keys, wrapping at the ends', async () => { + await openConnectableMenu(); + const menu = menuFor('term-1')!; + const rows = menu.querySelectorAll('button[role="menuitem"]'); + expect(rows.length).toBe(2); + + act(() => { window.dispatchEvent(keydown('ArrowDown')); }); + expect(document.activeElement).toBe(rows[0]); + act(() => { window.dispatchEvent(keydown('ArrowDown')); }); + expect(document.activeElement).toBe(rows[1]); + act(() => { window.dispatchEvent(keydown('ArrowDown')); }); + expect(document.activeElement).toBe(rows[0]); // wraps past the last + act(() => { window.dispatchEvent(keydown('ArrowUp')); }); + expect(document.activeElement).toBe(rows[1]); // wraps before the first + }); + + it('cycles focus through every focusable with Tab, including the close button', async () => { + await openConnectableMenu(); + const menu = menuFor('term-1')!; + const closeBtn = menu.querySelector('button[aria-label="Close menu"]')!; + const rows = Array.from(menu.querySelectorAll('button[role="menuitem"]')); + const order = [closeBtn, rows[0], rows[1]]; + + // From the container, Tab lands on the first focusable and wraps through all. + for (const expected of [...order, order[0]]) { + act(() => { window.dispatchEvent(keydown('Tab')); }); + expect(document.activeElement).toBe(expected); + } + }); + + it('restores focus to the previously focused element when it closes', async () => { + const outside = document.createElement('button'); + document.body.appendChild(outside); + outside.focus(); + expect(document.activeElement).toBe(outside); + + await openConnectableMenu(); + expect(document.activeElement).toBe(menuFor('term-1')); + + act(() => { window.dispatchEvent(keydown('Escape')); }); + expect(menuFor('term-1')).toBeNull(); + expect(document.activeElement).toBe(outside); + outside.remove(); + }); + + it('survives a scroll originating inside the menu but dismisses on an outside scroll', async () => { + await openConnectableMenu(); + const menu = menuFor('term-1')!; + + act(() => { menu.dispatchEvent(new Event('scroll', { bubbles: false })); }); + expect(menuFor('term-1')).not.toBeNull(); + + act(() => { window.dispatchEvent(new Event('scroll')); }); + expect(menuFor('term-1')).toBeNull(); + }); +}); diff --git a/lib/src/components/wall/PaneHeaderContextMenu.tsx b/lib/src/components/wall/PaneHeaderContextMenu.tsx index 5b69a85f..d4996921 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.tsx @@ -1,11 +1,12 @@ -import { useContext, useEffect, useLayoutEffect, useRef, useState, type CSSProperties } from 'react'; +import { useCallback, useContext, useEffect, useLayoutEffect, useRef, useState, type CSSProperties } from 'react'; import { createPortal } from 'react-dom'; import { CircleNotchIcon, XIcon } from '@phosphor-icons/react'; -import { POPUP_SURFACE_CLASS } from '../design'; +import { POPUP_SURFACE_CLASS, Shortcut } from '../design'; import { clampOverlayPosition } from '../../lib/ui-geometry'; import { getPlatform } from '../../lib/platform'; import type { OpenPort } from '../../lib/platform/types'; import { titleSourceLabel, type TerminalTitle } from '../../lib/terminal-state'; +import { usePopoverFocusTrap } from '../use-popover-focus-trap'; import { listenerUrlsByPort, type PortUrlEntry } from './port-url'; import { useDismissOverlay } from './use-dismiss-overlay'; import { WallActionsContext } from './wall-context'; @@ -29,17 +30,25 @@ const MENU_ROW_CLASS = 'flex w-full items-baseline gap-2 px-2.5 py-1 text-left h * feedback, and a failure is logged, not shown here. Port rows are only clickable * when the host can run agent-browser; otherwise the list is an inert label (the * host-gated-affordance convention). + * + * The menu owns the keyboard while open (`docs/specs/layout.md` → Pane header): + * it takes DOM focus on mount (restoring the prior focus on close so a + * passthrough terminal gets its keys back), reports dialog-keyboard-active so + * command-mode keys don't fire underneath, wires `1`–`9` to the port rows, + * roves `↑`/`↓` across them, and traps Tab/Escape via `usePopoverFocusTrap`. */ export function PaneHeaderContextMenu({ id, anchor, onClose, + onKeyboardActiveChange, candidates, currentTitle, }: { id: string; anchor: { x: number; y: number }; onClose: () => void; + onKeyboardActiveChange: (active: boolean) => void; candidates: TerminalTitle[]; currentTitle: string; }) { @@ -72,22 +81,79 @@ export function PaneHeaderContextMenu({ setStyle(clampOverlayPosition({ left: anchor.x, top: anchor.y, width: rect.width, height: rect.height })); }, [anchor.x, anchor.y, scan]); - useDismissOverlay(onClose); + // Scrolls inside the menu (arrow-key focus moves auto-scroll the overflowing + // list) must not dismiss it; every other scroll still does. + useDismissOverlay(onClose, ref); + usePopoverFocusTrap(ref, onClose); + + // Report dialog-keyboard-active so command-mode shortcuts stay dormant while + // the menu is open (matches TodoAlertDialog). + useEffect(() => { + onKeyboardActiveChange(true); + return () => onKeyboardActiveChange(false); + }, [onKeyboardActiveChange]); + + // Take DOM focus on the menu container (tabIndex=-1) so our keyboard handlers + // fire via `el.contains(document.activeElement)`; restore the prior focus on + // close so a passthrough terminal gets its keys back. + useEffect(() => { + const previouslyFocused = document.activeElement as HTMLElement | null; + ref.current?.focus({ preventScroll: true }); + return () => { + if (previouslyFocused?.isConnected) previouslyFocused.focus({ preventScroll: true }); + }; + }, []); // Fire-and-forget: the pane appears immediately and reports its own progress, so // the menu closes at once rather than waiting on the daemon boot. - const connect = (entry: PortUrlEntry) => { + const connect = useCallback((entry: PortUrlEntry) => { actions.onConnectPort(id, entry.url); onClose(); - }; + }, [actions, id, onClose]); + + // Keyboard rove + digit accelerators. Capture phase, scoped to the menu, and + // active only while mounted. Enter/Space are left to the focused native button. + useEffect(() => { + const el = ref.current; + if (!el) return; + const handler = (e: KeyboardEvent) => { + if (!el.contains(document.activeElement)) return; + if (e.key === 'ArrowDown' || e.key === 'ArrowUp') { + const items = Array.from(el.querySelectorAll('[role="menuitem"]')); + if (items.length === 0) return; + e.preventDefault(); + e.stopPropagation(); + const currentIndex = items.findIndex((item) => item === document.activeElement); + const nextIndex = currentIndex === -1 + ? (e.key === 'ArrowDown' ? 0 : items.length - 1) + : (currentIndex + (e.key === 'ArrowDown' ? 1 : -1) + items.length) % items.length; + items[nextIndex]?.focus(); + return; + } + // Digits connect the nth port row — but only when there is a loaded list to + // index into. Scanning/failed/out-of-range/inert-host presses are dropped, + // not buffered. + if (canConnect && scan.status === 'loaded' && /^[1-9]$/.test(e.key)) { + const entry = scan.entries[Number(e.key) - 1]; + if (entry) { + e.preventDefault(); + e.stopPropagation(); + connect(entry); + } + } + }; + window.addEventListener('keydown', handler, true); + return () => window.removeEventListener('keydown', handler, true); + }, [canConnect, scan, connect]); return createPortal(
e.stopPropagation()} onMouseDown={(e) => e.stopPropagation()} @@ -135,7 +201,7 @@ export function PaneHeaderContextMenu({ {scan.status === 'loaded' && scan.entries.length === 0 && (
no listening ports
)} - {scan.status === 'loaded' && scan.entries.map((entry) => { + {scan.status === 'loaded' && scan.entries.map((entry, index) => { const label = ( <> {entry.host}:{entry.port} @@ -151,6 +217,7 @@ export function PaneHeaderContextMenu({ className={MENU_ROW_CLASS} onClick={() => connect(entry)} > + {index < 9 && {index + 1}} {label} ) : ( diff --git a/lib/src/components/wall/TerminalPaneHeader.tsx b/lib/src/components/wall/TerminalPaneHeader.tsx index 239649b5..8ada124b 100644 --- a/lib/src/components/wall/TerminalPaneHeader.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.tsx @@ -184,6 +184,7 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { return (
actions.onClickPanel(id)} onContextMenu={(e) => { @@ -384,6 +385,7 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { id={id} anchor={contextMenu} onClose={closeContextMenu} + onKeyboardActiveChange={setDialogKeyboardActive} candidates={titleCandidates} currentTitle={displayTitleBase} /> diff --git a/lib/src/components/wall/keyboard/handle-pane-shortcuts.test.ts b/lib/src/components/wall/keyboard/handle-pane-shortcuts.test.ts index 128731fb..a1a359b6 100644 --- a/lib/src/components/wall/keyboard/handle-pane-shortcuts.test.ts +++ b/lib/src/components/wall/keyboard/handle-pane-shortcuts.test.ts @@ -1,7 +1,7 @@ /** * @vitest-environment jsdom */ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { handlePaneShortcuts } from './handle-pane-shortcuts'; import type { WallKeyboardCtx } from './types'; @@ -23,6 +23,11 @@ vi.mock('../../KillConfirm', () => ({ randomKillChar: () => 'Q', })); +// jsdom here ships no `CSS` global; the header lookup escapes ids via CSS.escape. +globalThis.CSS ??= { + escape: (value: string) => value.replace(/[^a-zA-Z0-9_-]/g, (c) => `\\${c}`), +} as unknown as typeof CSS; + function makeNav(overrides: Partial = {}): WallKeyboardCtx['nav'] { return { findInDirection: () => null, @@ -173,3 +178,41 @@ describe('handlePaneShortcuts Cmd-Arrow swap (nav seam)', () => { expect(ctx.selectPane).not.toHaveBeenCalled(); }); }); + +describe('handlePaneShortcuts `>` header context menu', () => { + beforeEach(() => vi.clearAllMocks()); + afterEach(() => { document.body.innerHTML = ''; }); + + function makeHeaderEl(id: string): ReturnType { + const el = document.createElement('div'); + el.setAttribute('data-pane-header-for', id); + document.body.appendChild(el); + const onContextMenu = vi.fn(); + el.addEventListener('contextmenu', onContextMenu); + return onContextMenu; + } + + it('dispatches contextmenu on the selected pane header element', () => { + const onContextMenu = makeHeaderEl('pane-a'); + + expect(handlePaneShortcuts(keydown('>'), makeCtx(), { current: null })).toBe(true); + + expect(onContextMenu).toHaveBeenCalledTimes(1); + expect(onContextMenu.mock.calls[0][0].type).toBe('contextmenu'); + }); + + it('consumes `>` without throwing when no header element matches', () => { + const ctx = makeCtx(); + expect(document.querySelector('[data-pane-header-for="pane-a"]')).toBeNull(); + expect(() => handlePaneShortcuts(keydown('>'), ctx, { current: null })).not.toThrow(); + expect(handlePaneShortcuts(keydown('>'), ctx, { current: null })).toBe(true); + }); + + it('does nothing when a door is selected', () => { + const onContextMenu = makeHeaderEl('pane-a'); + const ctx = makeCtx({ selectedTypeRef: { current: 'door' } }); + + expect(handlePaneShortcuts(keydown('>'), ctx, { current: null })).toBe(false); + expect(onContextMenu).not.toHaveBeenCalled(); + }); +}); diff --git a/lib/src/components/wall/keyboard/handle-pane-shortcuts.ts b/lib/src/components/wall/keyboard/handle-pane-shortcuts.ts index 5b390fb6..63a4c3bf 100644 --- a/lib/src/components/wall/keyboard/handle-pane-shortcuts.ts +++ b/lib/src/components/wall/keyboard/handle-pane-shortcuts.ts @@ -11,10 +11,15 @@ function findAlertButtonForSession(id: string): HTMLButtonElement | null { return document.querySelector(`[data-alert-button-for="${CSS.escape(id)}"]`); } +function findPaneHeaderForSession(id: string): HTMLElement | null { + return document.querySelector(`[data-pane-header-for="${CSS.escape(id)}"]`); +} + /** * Single-pane shortcuts: Enter (focus/reattach), `|`/`%`/`-`/`"` (split), * Cmd-Arrow (swap with neighbor), k/x (kill confirm), `,` (rename), - * m/d (minimize), t/a (todo/alert toggle), z (zoom + passthrough focus). + * m/d (minimize), t/a (todo/alert toggle), z (zoom + passthrough focus), + * `>` (open the selected pane's header context menu). */ export function handlePaneShortcuts( e: KeyboardEvent, @@ -143,5 +148,26 @@ export function handlePaneShortcuts( return true; } + if (e.key === '>' && sid && ctx.selectedTypeRef.current === 'pane') { + e.preventDefault(); + e.stopPropagation(); + // Reuse the header's own onContextMenu path: dispatch a synthetic + // contextmenu at the header's bottom-left corner so the menu opens anchored + // under the header's left edge. Browser-surface panes carry no + // `data-pane-header-for`, so the lookup misses and the key is a consumed + // no-op — the spec'd behavior for surfaces with no header context menu. + const header = findPaneHeaderForSession(sid); + if (header) { + const rect = header.getBoundingClientRect(); + header.dispatchEvent(new MouseEvent('contextmenu', { + bubbles: true, + cancelable: true, + clientX: rect.left, + clientY: rect.bottom, + })); + } + return true; + } + return false; } diff --git a/lib/src/components/wall/use-dismiss-overlay.ts b/lib/src/components/wall/use-dismiss-overlay.ts index afe1f05b..b680c773 100644 --- a/lib/src/components/wall/use-dismiss-overlay.ts +++ b/lib/src/components/wall/use-dismiss-overlay.ts @@ -1,26 +1,39 @@ -import { useEffect } from 'react'; +import { useEffect, type RefObject } from 'react'; /** * The pane-header popovers' shared dismissal contract (docs/specs/layout.md): * any window `pointerdown` (an overlay that should survive its own clicks stops * propagation on its root), Escape, `resize`, or capture-phase `scroll`. * Register only while the overlay is mounted — consumers render only when open. + * + * When `insideRef` is provided, a capture-phase `scroll` whose `event.target` + * is a Node inside `insideRef.current` does NOT dismiss (arrow-key focus moves + * auto-scroll the menu's own overflow container, which must not close it). + * Scrolls from anywhere else still dismiss. */ -export function useDismissOverlay(onClose: () => void): void { +export function useDismissOverlay( + onClose: () => void, + insideRef?: RefObject, +): void { useEffect(() => { const close = () => onClose(); const closeOnKey = (event: KeyboardEvent) => { if (event.key === 'Escape') close(); }; + const closeOnScroll = (event: Event) => { + const inside = insideRef?.current; + if (inside && event.target instanceof Node && inside.contains(event.target)) return; + close(); + }; window.addEventListener('pointerdown', close); window.addEventListener('resize', close); - window.addEventListener('scroll', close, true); + window.addEventListener('scroll', closeOnScroll, true); window.addEventListener('keydown', closeOnKey); return () => { window.removeEventListener('pointerdown', close); window.removeEventListener('resize', close); - window.removeEventListener('scroll', close, true); + window.removeEventListener('scroll', closeOnScroll, true); window.removeEventListener('keydown', closeOnKey); }; - }, [onClose]); + }, [onClose, insideRef]); } From 0e750de7c01920747ceb11715e6c4ddbd6db933c Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 22 Jul 2026 18:53:33 -0700 Subject: [PATCH 13/13] Simplify the menu's keyboard/dismissal plumbing Four-angle review cleanups: extract stepFocus so the wrap arithmetic exists once (popover trap, modal trap, menu arrows); make the menu's own keydown handler cover Tab and drop usePopoverFocusTrap there, leaving useDismissOverlay the single dismissal owner; make the overlay root a required useDismissOverlay argument so self-originated scrolls never dismiss any overlay; rename the stale data-title-candidates-for locator to data-pane-title-for. Co-Authored-By: Claude Fable 5 --- lib/src/components/design.tsx | 14 ++++----- lib/src/components/focus-step.ts | 15 ++++++++++ lib/src/components/use-popover-focus-trap.ts | 19 +++++------- .../components/wall/IllegalRenameWarning.tsx | 2 +- .../wall/PaneHeaderContextMenu.test.tsx | 2 +- .../components/wall/PaneHeaderContextMenu.tsx | 29 +++++++++---------- .../components/wall/TerminalPaneHeader.tsx | 2 +- .../components/wall/use-dismiss-overlay.ts | 18 +++++------- lib/src/stories/ShellCwd.stories.tsx | 2 +- 9 files changed, 54 insertions(+), 49 deletions(-) create mode 100644 lib/src/components/focus-step.ts diff --git a/lib/src/components/design.tsx b/lib/src/components/design.tsx index 727e6172..ee559d46 100644 --- a/lib/src/components/design.tsx +++ b/lib/src/components/design.tsx @@ -3,6 +3,7 @@ import { tv, type VariantProps } from 'tailwind-variants'; import { XIcon } from '@phosphor-icons/react'; import { forwardRef, useEffect, useLayoutEffect, useRef, useState } from 'react'; import type { ButtonHTMLAttributes, CSSProperties, HTMLAttributes, ReactNode, RefObject } from 'react'; +import { stepFocus } from './focus-step'; // App-wide type scale, color strategy, and chrome conventions: see // docs/specs/theme.md and AGENTS.md. @@ -385,16 +386,11 @@ function useModalFocusTrap(MODAL_FOCUSABLE_SELECTOR)); - if (focusables.length === 0) return; - - const currentIndex = focusables.findIndex((item) => item === document.activeElement); - const nextIndex = currentIndex === -1 - ? 0 - : (currentIndex + (event.shiftKey ? -1 : 1) + focusables.length) % focusables.length; - event.preventDefault(); - focusables[nextIndex]?.focus(); + stepFocus( + Array.from(modal.querySelectorAll(MODAL_FOCUSABLE_SELECTOR)), + event.shiftKey ? -1 : 1, + ); }; window.addEventListener('keydown', handleKeyDown, true); diff --git a/lib/src/components/focus-step.ts b/lib/src/components/focus-step.ts new file mode 100644 index 00000000..2604ff29 --- /dev/null +++ b/lib/src/components/focus-step.ts @@ -0,0 +1,15 @@ +/** + * Move DOM focus within an ordered candidate list: step from the currently + * focused entry by `delta`, wrapping at both ends. When focus is outside the + * list, seed at the first entry for a forward step and the last for a backward + * one. Shared by the popover/modal focus traps (Tab cycling) and the pane + * header context menu (arrow roving) so the wrap arithmetic exists once. + */ +export function stepFocus(items: HTMLElement[], delta: number): void { + if (items.length === 0) return; + const currentIndex = items.findIndex((item) => item === document.activeElement); + const nextIndex = currentIndex === -1 + ? (delta > 0 ? 0 : items.length - 1) + : (currentIndex + delta + items.length) % items.length; + items[nextIndex]?.focus(); +} diff --git a/lib/src/components/use-popover-focus-trap.ts b/lib/src/components/use-popover-focus-trap.ts index b0177bd5..4903bf32 100644 --- a/lib/src/components/use-popover-focus-trap.ts +++ b/lib/src/components/use-popover-focus-trap.ts @@ -1,4 +1,8 @@ import { useEffect } from 'react'; +import { stepFocus } from './focus-step'; + +/** What Tab reaches inside a popover: real buttons plus explicit tab stops. */ +export const POPOVER_FOCUSABLE_SELECTOR = 'button:not([disabled]), [tabindex]:not([tabindex="-1"])'; /** * Manages focus trapping, Escape-to-close, and click-outside-to-close for @@ -29,18 +33,11 @@ export function usePopoverFocusTrap( } if (e.key !== 'Tab') return; - const focusables = Array.from( - el.querySelectorAll('button:not([disabled]), [tabindex]:not([tabindex="-1"])'), - ); - if (focusables.length === 0) return; - - const currentIndex = focusables.findIndex((f) => f === document.activeElement); - const nextIndex = currentIndex === -1 - ? 0 - : (currentIndex + (e.shiftKey ? -1 : 1) + focusables.length) % focusables.length; - e.preventDefault(); - focusables[nextIndex]?.focus(); + stepFocus( + Array.from(el.querySelectorAll(POPOVER_FOCUSABLE_SELECTOR)), + e.shiftKey ? -1 : 1, + ); }; window.addEventListener('mousedown', handleMouseDown); diff --git a/lib/src/components/wall/IllegalRenameWarning.tsx b/lib/src/components/wall/IllegalRenameWarning.tsx index c5fc7833..ab07b6f1 100644 --- a/lib/src/components/wall/IllegalRenameWarning.tsx +++ b/lib/src/components/wall/IllegalRenameWarning.tsx @@ -38,7 +38,7 @@ export function IllegalRenameWarning({ anchorRect, reason, attemptedValue, onClo }); }, [anchorRect]); - useDismissOverlay(onClose); + useDismissOverlay(onClose, ref); useEffect(() => { const timeout = window.setTimeout(onClose, AUTO_DISMISS_MS); diff --git a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx index 42796b32..b23b8428 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.test.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.test.tsx @@ -162,7 +162,7 @@ describe('PaneHeaderContextMenu — pane header right-click', () => { it('opens the one context menu when a title-span right-click bubbles up', async () => { renderHeader(headerProps('term-1', 'title'), stubActions()); - const titleSpan = container.querySelector('[data-title-candidates-for="term-1"]') as HTMLElement; + const titleSpan = container.querySelector('[data-pane-title-for="term-1"]') as HTMLElement; expect(titleSpan).not.toBeNull(); await act(async () => { titleSpan.dispatchEvent(new MouseEvent('contextmenu', { bubbles: true, clientX: 40, clientY: 12 })); diff --git a/lib/src/components/wall/PaneHeaderContextMenu.tsx b/lib/src/components/wall/PaneHeaderContextMenu.tsx index d4996921..597891c3 100644 --- a/lib/src/components/wall/PaneHeaderContextMenu.tsx +++ b/lib/src/components/wall/PaneHeaderContextMenu.tsx @@ -6,7 +6,8 @@ import { clampOverlayPosition } from '../../lib/ui-geometry'; import { getPlatform } from '../../lib/platform'; import type { OpenPort } from '../../lib/platform/types'; import { titleSourceLabel, type TerminalTitle } from '../../lib/terminal-state'; -import { usePopoverFocusTrap } from '../use-popover-focus-trap'; +import { stepFocus } from '../focus-step'; +import { POPOVER_FOCUSABLE_SELECTOR } from '../use-popover-focus-trap'; import { listenerUrlsByPort, type PortUrlEntry } from './port-url'; import { useDismissOverlay } from './use-dismiss-overlay'; import { WallActionsContext } from './wall-context'; @@ -34,8 +35,9 @@ const MENU_ROW_CLASS = 'flex w-full items-baseline gap-2 px-2.5 py-1 text-left h * The menu owns the keyboard while open (`docs/specs/layout.md` → Pane header): * it takes DOM focus on mount (restoring the prior focus on close so a * passthrough terminal gets its keys back), reports dialog-keyboard-active so - * command-mode keys don't fire underneath, wires `1`–`9` to the port rows, - * roves `↑`/`↓` across them, and traps Tab/Escape via `usePopoverFocusTrap`. + * command-mode keys don't fire underneath, and handles Tab cycling, `↑`/`↓` + * roving, and the `1`–`9` port accelerators in one keydown handler. Dismissal + * (Escape, outside press, resize, scroll) is owned solely by `useDismissOverlay`. */ export function PaneHeaderContextMenu({ id, @@ -81,10 +83,7 @@ export function PaneHeaderContextMenu({ setStyle(clampOverlayPosition({ left: anchor.x, top: anchor.y, width: rect.width, height: rect.height })); }, [anchor.x, anchor.y, scan]); - // Scrolls inside the menu (arrow-key focus moves auto-scroll the overflowing - // list) must not dismiss it; every other scroll still does. useDismissOverlay(onClose, ref); - usePopoverFocusTrap(ref, onClose); // Report dialog-keyboard-active so command-mode shortcuts stay dormant while // the menu is open (matches TodoAlertDialog). @@ -111,23 +110,23 @@ export function PaneHeaderContextMenu({ onClose(); }, [actions, id, onClose]); - // Keyboard rove + digit accelerators. Capture phase, scoped to the menu, and - // active only while mounted. Enter/Space are left to the focused native button. + // Tab cycling + arrow rove + digit accelerators, one handler. Capture phase, + // scoped to the menu, and active only while mounted. Enter/Space are left to + // the focused native button. useEffect(() => { const el = ref.current; if (!el) return; const handler = (e: KeyboardEvent) => { if (!el.contains(document.activeElement)) return; + if (e.key === 'Tab') { + e.preventDefault(); + stepFocus(Array.from(el.querySelectorAll(POPOVER_FOCUSABLE_SELECTOR)), e.shiftKey ? -1 : 1); + return; + } if (e.key === 'ArrowDown' || e.key === 'ArrowUp') { - const items = Array.from(el.querySelectorAll('[role="menuitem"]')); - if (items.length === 0) return; e.preventDefault(); e.stopPropagation(); - const currentIndex = items.findIndex((item) => item === document.activeElement); - const nextIndex = currentIndex === -1 - ? (e.key === 'ArrowDown' ? 0 : items.length - 1) - : (currentIndex + (e.key === 'ArrowDown' ? 1 : -1) + items.length) % items.length; - items[nextIndex]?.focus(); + stepFocus(Array.from(el.querySelectorAll('[role="menuitem"]')), e.key === 'ArrowDown' ? 1 : -1); return; } // Digits connect the nth port row — but only when there is a loaded list to diff --git a/lib/src/components/wall/TerminalPaneHeader.tsx b/lib/src/components/wall/TerminalPaneHeader.tsx index 8ada124b..293e28e3 100644 --- a/lib/src/components/wall/TerminalPaneHeader.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.tsx @@ -217,7 +217,7 @@ export function TerminalPaneHeader({ id, title }: PaneProps) { /> ) : ( e.stopPropagation()} onClick={(e) => { e.stopPropagation(); actions.onStartRename(id); }} diff --git a/lib/src/components/wall/use-dismiss-overlay.ts b/lib/src/components/wall/use-dismiss-overlay.ts index b680c773..2124d126 100644 --- a/lib/src/components/wall/use-dismiss-overlay.ts +++ b/lib/src/components/wall/use-dismiss-overlay.ts @@ -3,17 +3,15 @@ import { useEffect, type RefObject } from 'react'; /** * The pane-header popovers' shared dismissal contract (docs/specs/layout.md): * any window `pointerdown` (an overlay that should survive its own clicks stops - * propagation on its root), Escape, `resize`, or capture-phase `scroll`. + * propagation on its root), Escape, `resize`, or capture-phase `scroll` — + * except scrolls originating inside `overlayRef` itself, which never dismiss: + * internal overflow scrolling (e.g. arrow-key focus moves auto-scrolling an + * overflowing list) doesn't move the overlay's anchor, so it must not close it. * Register only while the overlay is mounted — consumers render only when open. - * - * When `insideRef` is provided, a capture-phase `scroll` whose `event.target` - * is a Node inside `insideRef.current` does NOT dismiss (arrow-key focus moves - * auto-scroll the menu's own overflow container, which must not close it). - * Scrolls from anywhere else still dismiss. */ export function useDismissOverlay( onClose: () => void, - insideRef?: RefObject, + overlayRef: RefObject, ): void { useEffect(() => { const close = () => onClose(); @@ -21,8 +19,8 @@ export function useDismissOverlay( if (event.key === 'Escape') close(); }; const closeOnScroll = (event: Event) => { - const inside = insideRef?.current; - if (inside && event.target instanceof Node && inside.contains(event.target)) return; + const overlay = overlayRef.current; + if (overlay && event.target instanceof Node && overlay.contains(event.target)) return; close(); }; window.addEventListener('pointerdown', close); @@ -35,5 +33,5 @@ export function useDismissOverlay( window.removeEventListener('scroll', closeOnScroll, true); window.removeEventListener('keydown', closeOnKey); }; - }, [onClose, insideRef]); + }, [onClose, overlayRef]); } diff --git a/lib/src/stories/ShellCwd.stories.tsx b/lib/src/stories/ShellCwd.stories.tsx index 0f0bac17..6fe816af 100644 --- a/lib/src/stories/ShellCwd.stories.tsx +++ b/lib/src/stories/ShellCwd.stories.tsx @@ -391,7 +391,7 @@ function wait(ms: number) { async function openHeaderContextMenu() { await wait(100); - const title = document.querySelector('[data-title-candidates-for="title-candidates-popup"]'); + const title = document.querySelector('[data-pane-title-for="title-candidates-popup"]'); if (!title) return; const rect = title.getBoundingClientRect();