From f6c211dd7ebc37705f0d109f092594c7c2a096fc Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 11:29:11 -0700 Subject: [PATCH 01/12] WIP agent-browser eager launch lifecycle --- docs/specs/dor-browser.md | 47 ++++-- docs/specs/dor-browser.rationale.md | 12 ++ lib/src/components/Wall.tsx | 48 ++++-- lib/src/components/wall/AgentBrowserPanel.tsx | 16 +- .../wall/agent-browser-connection.test.ts | 11 ++ .../wall/agent-browser-connection.ts | 8 + .../agent-browser-surface-controller.test.ts | 112 +++++++++++++ .../wall/agent-browser-surface-controller.ts | 128 +++++++++++---- lib/src/components/wall/use-dor-control.ts | 10 +- lib/src/host/agent-browser-host.test.ts | 134 ++++++++++++++- lib/src/host/agent-browser-host.ts | 152 +++++++++++++----- scripts/spec-word-budgets.json | 2 +- 12 files changed, 567 insertions(+), 113 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index ee535b572..4b24fce4d 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -200,7 +200,7 @@ size landed**, so a resize transient is not read as an external override. | From -> To | Behavior | | --- | --- | -| `iframe` -> `ab-screencast` / `ab-popout` | Host spawns a fresh `gui-` session at the current URL via `agentBrowserOpen`; `ab-popout` spawns headed in one shot, so the surface mounts already popped out. Hidden/inert without that capability. | +| `iframe` -> `ab-screencast` / `ab-popout` | **The pane swaps at once** to a session-less agent-browser pane — inert, so it cannot race the boot (rationale) — while the host spawns a fresh `gui-` session at the current URL via `agentBrowserOpen` and hands over `{session, wsPort, binaryPath}` as **one** params refresh. `ab-popout` spawns headed in one shot, so the surface mounts already popped out. A spawn that yields no session restores the iframe; a pane killed meanwhile closes the session. Hidden/inert without that capability. | | `ab-screencast` <-> `ab-popout` | Same session, headed/headless relaunch in `AgentBrowserPanel`; preserves only the active URL. | | `ab-*` -> `iframe` | Uses canonical `params.url`; with multiple tabs, requires the user to press `c` in the warning overlay, because only the active tab survives. | @@ -281,9 +281,18 @@ its own because clients trigger it. Rules park and recovery must not break: - **Never set `AGENT_BROWSER_IDLE_TIMEOUT_MS`** for Dormouse-managed sessions — daemon self-exit when idle would defeat "alive while parked". - **Never query the daemon mid-relaunch** — see [Pop-Out](#pop-out). - -The stream WebSocket carries frame pulses + status, tab snapshots, and native -`input_mouse` / `input_keyboard` input. +- **A relaunch in flight drops the stream and CDP observer at once**, shows a + relaunching placeholder, and reconnects only to the port the host hands back + (rationale). **One relaunch at a time**: a pop-out or pop-in issued during + one is ignored; a session-less pane has nothing to relaunch. +- **A `{session, wsPort, binaryPath}` refresh reconciles once**, straight to the + port, with no `stream status`. + +The stream WebSocket carries frame pulses + status, tab snapshots, `url`, and +native `input_mouse` / `input_keyboard` input. **`url` names the active tab at +navigation commit; `tabs` refreshes only when the driving command completes** +(for a slow page, after the load; rationale), so header and relaunch target +follow `url`, title cleared until `tabs` names the page. **Two-stage paint.** A changed stream JPEG paints at once as a **provisional frame** — the first image, and 250ms after pointer input (continuous movement @@ -336,18 +345,28 @@ shared by the stream and `tab list --json`). `ab-popout` relaunches the same session headed, because Chrome fixes headed/headless at daemon launch. The pane becomes a stub with Pop back in, plus -Bring to front where a host implements `agentBrowserBringToFront`. **State -carried in v1 is only the active non-blank URL**: other tabs, DOM state, scroll, -form inputs, session storage, cookies/logins do not survive. +Bring to front where a host implements `agentBrowserBringToFront`; while the +window is still opening (a relaunch in flight, or an eager pane without its +session) the stub offers neither. **State carried in v1 is only the active +non-blank URL**: other tabs, DOM state, scroll, form inputs, session storage, +cookies/logins do not survive. Host sequence: run `close`, **then terminate the daemon by its pid file** (`$AGENT_BROWSER_SOCKET_DIR/.pid`, default `~/.agent-browser`) **and -wait for it to exit** (rationale), then reopen and read a new stream port. -**Never query the daemon during the close/reopen gap** (rationale), host and -controller park/recovery paths alike, so **Dormouse supplies the active-tab URL -and the host trusts it**. After reopening, the host best-effort closes any stray -`about:blank` tab the close+reopen race left behind, **but only while a real page -is open**, so it never closes the sole tab (rationale). +wait for it to exit** (rationale), then reopen. **Never wait for the page to +load** (rationale): every launch — pop-out, pop-in, `agentBrowserOpen` — +resolves once the *relaunched* daemon is up (a pid file naming a pid other than +the killed one, and a `.stream` file naming a port that accepts a +connection), asking `stream status` only after `open` returns. **A non-zero +`open` exit with the daemon up is a page still loading, not a failed launch**; +only a launch that never publishes a port fails, and then `agentBrowserOpen` +closes what it spawned. **A headed session is tracked for shutdown before its +launch**, so a window whose page never loads is still closed. **Never query the +daemon during the close/reopen gap** (rationale), host and controller +park/recovery paths alike, so **Dormouse supplies the active-tab URL and the +host trusts it**. Once `open` returns, the host best-effort closes any stray +`about:blank` tab the close+reopen race left behind, **but only while a real +page is open**, so it never closes the sole tab (rationale). While popped out, Dormouse keeps a stream/CDP observer so URL/header state follows same-tab navigation and a headed window close can auto-revert to @@ -371,7 +390,7 @@ sidecar/Rust adapter. | `agentBrowserStreamStatus` | Current stream port, for stale-`wsPort` recovery. | | `agentBrowserEdit` | select-all/copy/cut via fixed host-owned JS plus an OS clipboard write. | | `getAgentBrowserStreamUrl` | Direct stream URL, or the VS Code relay URL. | -| `agentBrowserOpen` | Spawn a GUI-owned session for iframe -> agent-browser. | +| `agentBrowserOpen` | Spawn a GUI-owned session for iframe -> agent-browser; resolves when the daemon is up, not when the page loads ([Pop-Out](#pop-out)). | | `agentBrowserPopOut` / `agentBrowserPopIn` | Headed/headless relaunch. | | `agentBrowserBringToFront` | Optional; no host implements it today. | diff --git a/docs/specs/dor-browser.rationale.md b/docs/specs/dor-browser.rationale.md index 6d8769a7a..883c7d823 100644 --- a/docs/specs/dor-browser.rationale.md +++ b/docs/specs/dor-browser.rationale.md @@ -22,6 +22,10 @@ **Why the handover is a single params refresh.** Setting `session` is what reconciles the controller and connects it, so landing it ahead of `wsPort`/`binaryPath` — or before `agent-browser open` has returned — connects against a daemon that is not up. Handing it over even after a failed `open` lets the placeholder name what it is waiting for; the menu that would have reported the error closed long ago. +## Display Modal And Render Swaps + +**Why the iframe swap is eager.** The same 1–3s daemon boot as the context-menu connect, behind a modal that has already closed; and while the swap awaited `open`, a slow page held the iframe on screen for the whole load and a timed-out `open` dropped the swap silently — leaving an orphan `gui-` browser nobody could see or close. + ## Agent-Browser Renderer **Why one-session-one-surface is not an invariant.** `dor ab` forwards the user's command and then runs `stream status` before it asks the host for a surface, so a surface killed or render-swapped inside that window is gone by the time the trailing request arrives — and the session behind it is still live and needs somewhere to render. @@ -38,12 +42,20 @@ **Why every non-crisp painter must bump the draw generation.** The byte-dedup compares an incoming capture against the last crisp draw. A resting page whose crisp bytes match that draw dedups to a no-op and strands the pane on the blurry provisional frame; a freshly re-attached canvas mounts blank and has the same problem. +**Why the connection is dropped at relaunch start rather than left to fail.** The host closes the browser and kills the daemon, so the old socket's close is certain. Left connected, its three reconnect failures flagged the pane "ended" about three seconds into a pop-in that a slow page could hold open for 25s, and the popped-out CDP observer's `get cdp-url` — issued the moment `poppedOut` flipped — landed in the close→reopen gap, where a daemon command spawns a competing headless daemon that the headed relaunch then reattaches to. + +**Why `url` is tracked separately from `tabs`.** Measured against agent-browser 0.31.1 (2026-09): on `open`, the stream sends `tabs` (about:blank), then `url` naming the target at navigation commit, and refreshes `tabs` only when the CLI command completes — after `load`. During a slow load the tab list still named the previous page, so a pop-out issued then relaunched the page before the one being loaded. + **Whose limitation the CSS-resolution provisional frame is.** Chromium's `Page.startScreencast` captures in DIP and exposes no DPR knob, so the stream is CSS-resolution whatever the client asks for — upstream Chromium, not something agent-browser chose or could fix. ## Pop-Out **The symptom when the daemon is not killed first.** `agent-browser --headed open` against a live headless daemon reattaches to it and exits 0, so the host logs a successful headed open and the mode never changes. The user presses Pop out, gets the pane stub with no OS window anywhere, and nothing in the logs says why. +**Why the host does not wait for `open`.** Measured against agent-browser 0.31.1 (2026-09): `open ` blocks until the page's `load` event, up to the CLI's 25s default action timeout, then exits 1 with "Operation timed out" — with the daemon up, the tab on the URL, and `stream status` answering. Every other daemon command queues behind it: a `stream status` issued mid-`open` returned after 22s. Meanwhile the daemon writes `.pid` and `.stream` within ~100ms of launch, and the stream serves status, tabs and frames from then on. Awaiting `open` therefore made a slow page cost the whole load before the pane showed anything, and turned the timeout into a "failed" relaunch — one whose headed window was never tracked for shutdown, because tracking followed a zero exit. + +**Why the stale state files need the replaced pid.** SIGTERM leaves the dead daemon's `.pid` and `.stream` files in place for the new daemon to overwrite. A port read from the stale file is probed against nothing — unless some other process has since taken it — so the launch also waits for a pid other than the one it killed before it trusts the stream file. + **Why nothing may query the daemon during the close/reopen gap.** With the old daemon dead and the new one not yet up, a `stream status` or tab query spawns a *competing* daemon at `about:blank` — agent-browser's CLI starts one on demand — and the relaunch then races two daemons for the same session. **Why the stray-`about:blank` sweep is guarded.** The close/reopen pair can leave an extra blank tab beside the navigated one. Sweeping blanks unconditionally is the obvious fix and is wrong: a session whose only tab is legitimately blank would lose it, leaving the pane with nothing to show. diff --git a/lib/src/components/Wall.tsx b/lib/src/components/Wall.tsx index 12c004ef6..121920aeb 100644 --- a/lib/src/components/Wall.tsx +++ b/lib/src/components/Wall.tsx @@ -1181,7 +1181,7 @@ export function Wall({ }, [generatePaneId, surfaceRefForId, forgetSurfaceRef, selectPane, enterTerminalMode, showShellSpawnNotice, lath, nav]); // --- dor control plane (the `dor` CLI's webview handler) --- - const { connectPort } = useDorControl({ + const { connectPort, updateSurfaceParams } = useDorControl({ lath, nav, doorsRef, @@ -1318,7 +1318,10 @@ export function Wall({ // Canonical params.url (mirrored from the chrome snapshot) first; fall // back to the live snapshot for a surface that hasn't reported a tab yet. const url = (typeof params?.url === 'string' && params.url) || getAgentBrowserScreenController(id)?.chrome().url; - if (!url) return; + if (!url) { + console.warn(`[dormouse] cannot swap surface '${id}' to iframe: no URL observed yet`); + return; + } replaceSurface(id, { params: { surfaceType: 'browser', renderMode: 'iframe', url }, title: hostPathDisplay(url, true), @@ -1330,6 +1333,12 @@ export function Wall({ // spawn a session for the URL (absent ⇒ inert, like other host-gated // affordances). ab-popout spawns headed directly so the new surface mounts // already popped-out (no headless launch + immediate relaunch flash). + // + // The swap lands NOW: the iframe is replaced by a session-less agent-browser + // pane — inert, so it cannot race the daemon boot — whose placeholder names + // what it is waiting for, and the daemon hands it `{session, wsPort, + // binaryPath}` as one params refresh. Same shape as the pane context menu's + // connect (docs/specs/dor-browser.md → "Pane Context Menu Connect"). if (currentRenderMode === 'iframe' && (mode === 'ab-screencast' || mode === 'ab-popout')) { const chromeUrl = getAgentBrowserScreenController(id)?.chrome().url; const url = (typeof chromeUrl === 'string' && chromeUrl) @@ -1337,26 +1346,33 @@ export function Wall({ const platform = getPlatform(); if (!url || !platform.agentBrowserOpen) return; const headed = mode === 'ab-popout'; + const title = hostPathDisplay(url, true); + const eagerId = replaceSurface(id, { + params: { surfaceType: 'browser', renderMode: mode, url, syncEngaged: true }, + title, + }); + if (!eagerId) return; platform.agentBrowserOpen(url, { headed }, lastAgentBrowserBinaryPathRef.current).then((res) => { - if (!res.ok || !res.session) return; + if (!res.ok || !res.session) { + console.warn(`[dormouse] failed to swap iframe surface '${id}' to agent-browser:`, res.error ?? '(no session)'); + // Nothing came up to bind: give the iframe back, if the eager pane is + // still on screen (a no-op when it was killed or minimized meanwhile). + replaceSurface(eagerId, { params: { surfaceType: 'browser', renderMode: 'iframe', url }, title }); + return; + } if (res.binaryPath) lastAgentBrowserBinaryPathRef.current = res.binaryPath; - const nextParams = { - surfaceType: 'browser', - renderMode: mode, + const bound = { session: res.session, - url, ...(res.wsPort !== undefined ? { wsPort: res.wsPort } : {}), ...(res.binaryPath !== undefined ? { binaryPath: res.binaryPath } : {}), - syncEngaged: true, }; - const nextId = replaceSurface(id, { - params: nextParams, - title: hostPathDisplay(url, true), - }); - if (!nextId) { - closeAgentBrowserSession(nextParams); - console.warn(`[dormouse] failed to replace iframe surface '${id}' with agent-browser surface`); + // The eager pane was killed (or is mid-fade) while the daemon booted: + // the session has no pane to live in, so close it rather than leak it. + if (!lath.store.has(eagerId) || lath.isDying(eagerId)) { + closeAgentBrowserSession({ renderMode: mode, ...bound }); + return; } + updateSurfaceParams(eagerId, bound); }).catch((err) => { console.warn('[dormouse] failed to swap iframe surface to agent-browser:', err); }); @@ -1378,7 +1394,7 @@ export function Wall({ 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]); + }), [addSplitPanel, minimizePane, enterTerminalMode, exitTerminalMode, killPaneImmediately, replaceSurface, buildDorSurfaces, createContentSurface, surfaceRefForId, connectPort, updateSurfaceParams, lath, nav]); const wallActionsRef = useRef(wallActions); wallActionsRef.current = wallActions; diff --git a/lib/src/components/wall/AgentBrowserPanel.tsx b/lib/src/components/wall/AgentBrowserPanel.tsx index 18d2ac95c..3267d5ec9 100644 --- a/lib/src/components/wall/AgentBrowserPanel.tsx +++ b/lib/src/components/wall/AgentBrowserPanel.tsx @@ -65,7 +65,7 @@ export function AgentBrowserPanel({ id, params: rawParams, parked, renderMode: r ); const snapshot = useSyncExternalStore(controller.subscribe, controller.snapshot); - const { tabs, status, connectionLost, hasFrame, poppedOut, streamPort } = snapshot; + const { tabs, status, connectionLost, hasFrame, poppedOut, relaunching, streamPort } = snapshot; const interactive = mode === 'passthrough' && selectedId === id; const interactiveRef = useRef(interactive); @@ -313,6 +313,9 @@ export function AgentBrowserPanel({ id, params: rawParams, parked, renderMode: r // 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…'; + // Mid pop-in: the headed browser is closed by design and the headless one + // is booting — not a session that ended. + if (relaunching) return 'Relaunching browser…'; 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.`; @@ -381,10 +384,13 @@ export function AgentBrowserPanel({ id, params: rawParams, parked, renderMode: r }} /> {poppedOut ? ( - // Popped out to a headed OS window — the pane is a clean stub. + // Popped out to a headed OS window — the pane is a clean stub. While + // the window is still being opened (a relaunch in flight, or an eager + // swap whose daemon has not yet named its session) there is nothing to + // bring to front or pop back in, so the affordances wait with it.
-
This browser is running in a separate window.
-
+
{!session || relaunching ? 'Opening the browser window…' : 'This browser is running in a separate window.'}
+ {session && !relaunching &&
{getPlatform().agentBrowserBringToFront && ( -
+
}
) : placeholder ? (
{placeholder}
diff --git a/lib/src/components/wall/agent-browser-connection.test.ts b/lib/src/components/wall/agent-browser-connection.test.ts index 77c707294..79959eed1 100644 --- a/lib/src/components/wall/agent-browser-connection.test.ts +++ b/lib/src/components/wall/agent-browser-connection.test.ts @@ -43,6 +43,17 @@ afterEach(() => { }); describe('agent-browser connection', () => { + it('forwards the stream\'s url message as a navigation event', async () => { + const connection = createAgentBrowserConnection({ session: 's', streamPort: 1234 }); + const events: unknown[] = []; + connection.subscribe((event) => { if (event.type === 'url') events.push(event); }); + await Promise.resolve(); + WebSocketMock.instances[0].emitMessage(JSON.stringify({ type: 'url', url: 'https://example.com/slow', timestamp: 1 })); + WebSocketMock.instances[0].emitMessage(JSON.stringify({ type: 'url' })); + expect(events).toEqual([{ type: 'url', url: 'https://example.com/slow' }]); + connection.dispose(); + }); + it('closes the stream websocket when disposed', async () => { const connection = createAgentBrowserConnection({ session: 'dormouse.1.default', diff --git a/lib/src/components/wall/agent-browser-connection.ts b/lib/src/components/wall/agent-browser-connection.ts index 168a51a29..c7ed1f218 100644 --- a/lib/src/components/wall/agent-browser-connection.ts +++ b/lib/src/components/wall/agent-browser-connection.ts @@ -63,6 +63,11 @@ export type AgentBrowserConnectionEvent = | { type: 'connection-error'; port: number } | { type: 'status'; status: AgentBrowserStreamStatus } | { type: 'tabs'; tabs: AgentBrowserTab[]; previousTabs: AgentBrowserTab[] } + /** The active tab committed a navigation. Fires at commit; the `tabs` + * snapshot refreshes only when the driving command completes, which for a + * slow page is the whole load (docs/specs/dor-browser.md → "Agent-Browser + * Connection"). */ + | { type: 'url'; url: string } | { type: 'frame-pulse'; metadata?: AgentBrowserFramePulse; @@ -310,6 +315,9 @@ export class AgentBrowserConnection { this.emit({ type: 'status', status }); } else if (msg.type === 'tabs' && Array.isArray(msg.tabs)) { this.handleTabs(parseAgentBrowserTabs(msg.tabs)); + } else if (msg.type === 'url' && typeof msg.url === 'string') { + this.debug('url', { url: msg.url }); + this.emit({ type: 'url', url: msg.url }); } } 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 4310a4bee..8cc01c2b1 100644 --- a/lib/src/components/wall/agent-browser-surface-controller.test.ts +++ b/lib/src/components/wall/agent-browser-surface-controller.test.ts @@ -520,3 +520,115 @@ describe('dispose', () => { expect(getAgentBrowserSurfaceController('id')).toBeNull(); }); }); + +describe('relaunch (pop-out / pop-in)', () => { + type RelaunchPlatform = FakePtyAdapter & Pick; + function relaunchPlatform(): RelaunchPlatform & { resolvePopOut: (res: { ok: boolean; wsPort?: number }) => void } { + const platform = new FakePtyAdapter() as RelaunchPlatform; + let resolvePopOut!: (res: { ok: boolean; wsPort?: number }) => void; + platform.agentBrowserCommand = vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })); + platform.agentBrowserStreamStatus = vi.fn(async () => ({ ok: true, wsPort: 9999 })); + platform.agentBrowserPopOut = vi.fn(() => new Promise<{ ok: boolean; wsPort?: number }>((r) => { resolvePopOut = r; })); + platform.agentBrowserPopIn = vi.fn(async () => ({ ok: true, wsPort: 5555 })); + setPlatform(platform); + return Object.assign(platform, { resolvePopOut: (res: { ok: boolean; wsPort?: number }) => resolvePopOut(res) }); + } + + it('drops the stream up front and connects to the host\'s port only once the relaunch ends', async () => { + const platform = relaunchPlatform(); + const controller = acquireAgentBrowserSurfaceController('id', { session: 'sess', wsPort: 1111 }); + const sink = makeSink(); + controller.attachView(sink); + await flushMicrotasks(); + const old = streamSocket(1111); + expect(old?.readyState).toBe(1); + + getAgentBrowserScreenController('id')?.actions.setRenderMode?.('ab-popout'); + await flushMicrotasks(); + // The host is about to close this browser and kill its daemon: the old + // socket is released now rather than left to fail into "ended"/recovery. + expect(old?.readyState).toBe(3); + expect(controller.snapshot().relaunching).toBe(true); + expect(controller.snapshot().poppedOut).toBe(true); + // No daemon command while the relaunch is in flight: not even the popped-out + // CDP observer's `get cdp-url`. + expect(platform.agentBrowserCommand).not.toHaveBeenCalledWith('sess', ['get', 'cdp-url'], undefined); + + platform.resolvePopOut({ ok: true, wsPort: 3456 }); + await flushMicrotasks(); + expect(controller.snapshot().relaunching).toBe(false); + expect(streamSockets(3456).length).toBe(1); + expect(streamSockets(1111).length).toBe(1); + expect(platform.agentBrowserCommand).toHaveBeenCalledWith('sess', ['get', 'cdp-url'], undefined); + expect(platform.agentBrowserStreamStatus).not.toHaveBeenCalled(); + }); + + it('ignores a second pop-out or pop-in while one is in flight', async () => { + const platform = relaunchPlatform(); + const controller = acquireAgentBrowserSurfaceController('id', { session: 'sess', wsPort: 1111 }); + controller.attachView(makeSink()); + await flushMicrotasks(); + + getAgentBrowserScreenController('id')?.actions.setRenderMode?.('ab-popout'); + controller.popIn(); + getAgentBrowserScreenController('id')?.actions.setRenderMode?.('ab-popout'); + await flushMicrotasks(); + + expect(platform.agentBrowserPopOut).toHaveBeenCalledTimes(1); + expect(platform.agentBrowserPopIn).not.toHaveBeenCalled(); + expect(controller.snapshot().poppedOut).toBe(true); + + platform.resolvePopOut({ ok: true, wsPort: 3456 }); + await flushMicrotasks(); + controller.popIn(); + expect(platform.agentBrowserPopIn).toHaveBeenCalledTimes(1); + }); + + it('pop-in on a session-less popped-out pane is a no-op', async () => { + const platform = relaunchPlatform(); + const controller = acquireAgentBrowserSurfaceController('id', { renderMode: 'ab-popout' }); + const sink = makeSink(); + controller.attachView(sink); + await flushMicrotasks(); + + controller.popIn(); + expect(platform.agentBrowserPopIn).not.toHaveBeenCalled(); + expect(controller.snapshot().poppedOut).toBe(true); + expect(sink.updateParameters).not.toHaveBeenCalledWith({ renderMode: 'ab-screencast' }); + }); + + it('a relaunch carries the URL the stream committed, not the one the last tabs snapshot reported', async () => { + const platform = relaunchPlatform(); + const controller = acquireAgentBrowserSurfaceController('id', { session: 'sess', wsPort: 1111, url: 'https://before.example/' }); + const sink = makeSink(); + controller.attachView(sink); + await flushMicrotasks(); + const socket = streamSocket(1111); + socket?.emitMessage(JSON.stringify({ + type: 'tabs', + tabs: [{ tabId: 't1', title: 'Before', url: 'https://before.example/', active: true }], + })); + // A navigation commits to a page that is still loading: `tabs` will not + // refresh until the load completes. + socket?.emitMessage(JSON.stringify({ type: 'url', url: 'https://slow.example/' })); + expect(getAgentBrowserScreenController('id')?.chrome().url).toBe('https://slow.example/'); + expect(getAgentBrowserScreenController('id')?.chrome().title).toBeNull(); + expect(sink.updateParameters).toHaveBeenCalledWith({ url: 'https://slow.example/' }); + + getAgentBrowserScreenController('id')?.actions.setRenderMode?.('ab-popout'); + expect(platform.agentBrowserPopOut).toHaveBeenCalledWith('sess', expect.objectContaining({ url: 'https://slow.example/' }), undefined); + }); + + it('a single {session, wsPort} handover connects once and asks the daemon nothing', async () => { + const platform = relaunchPlatform(); + const controller = acquireAgentBrowserSurfaceController('id', { renderMode: 'ab-screencast', url: 'https://x.example/' }); + controller.attachView(makeSink()); + await flushMicrotasks(); + expect(WebSocketMock.instances.length).toBe(0); + + controller.updateParams({ session: 'sess', wsPort: 4321, url: 'https://x.example/' }); + await flushMicrotasks(); + expect(streamSockets(4321).length).toBe(1); + expect(platform.agentBrowserStreamStatus).not.toHaveBeenCalled(); + }); +}); diff --git a/lib/src/components/wall/agent-browser-surface-controller.ts b/lib/src/components/wall/agent-browser-surface-controller.ts index 6b3e21486..264ff8269 100644 --- a/lib/src/components/wall/agent-browser-surface-controller.ts +++ b/lib/src/components/wall/agent-browser-surface-controller.ts @@ -168,6 +168,10 @@ export interface AgentBrowserViewSnapshot { connectionLost: boolean; hasFrame: boolean; poppedOut: boolean; + /** A headed↔headless relaunch is in flight: the old stream is gone by design + * and the new one is not yet known, so the view shows neither "ended" nor + * a pop-in/pop-out affordance. */ + relaunching: boolean; streamPort: number | undefined; } @@ -212,10 +216,13 @@ export class AgentBrowserSurfaceController { // Gate auto-revert: only treat a dropped stream as "window closed" once the // headed stream has actually connected (avoids reverting mid-relaunch). private headedConnected = false; - // True while a headed↔headless relaunch is in flight. The relaunch closes the - // current stream before reopening on a new port, so that expected drop must - // not be read as "the headed window closed" (it would auto-revert mid-pop-out) - // nor trigger stale-port recovery (which could spawn a competing daemon). + // True while a headed↔headless relaunch is in flight. The host closes the + // current browser and kills its daemon before reopening on a new port, so the + // connection is dropped up front (reconcileConnection) rather than left to + // fail: its close would otherwise read as "the headed window closed" (an + // auto-revert mid-pop-out) or as a stale port (a recovery `stream status` + // that spawns a competing daemon — the daemon is deliberately not up). The + // CDP observer waits for the same reason. The host hands back the port. private relaunching = false; // --- visibility / parking --- @@ -379,6 +386,7 @@ export class AgentBrowserSurfaceController { connectionLost: this.connectionLost, hasFrame: this.hasFrame, poppedOut: this.poppedOut, + relaunching: this.relaunching, streamPort: this.streamPort, }; } @@ -393,6 +401,7 @@ export class AgentBrowserSurfaceController { prev.connectionLost === this.connectionLost && prev.hasFrame === this.hasFrame && prev.poppedOut === this.poppedOut && + prev.relaunching === this.relaunching && prev.streamPort === this.streamPort ) return; this.viewSnapshot = this.buildViewSnapshot(); @@ -541,23 +550,38 @@ export class AgentBrowserSurfaceController { updateParams(params: AgentBrowserSurfaceParams): void { if (this.disposed) return; + // Mirror every field first, then reconcile once: `{session, wsPort, + // binaryPath}` lands as a single write when a daemon boot hands over a + // session-less pane, and reacting per field would fire a `stream status` + // for the session before its port is mirrored — a CLI spawn that queues + // behind the daemon's in-flight `open` only to be discarded. + let sessionChanged = false; + let binaryPathChanged = false; + let portChanged = false; if (params.session !== this.session) { this.session = params.session; if (params.session) clearAgentBrowserSessionClosed(params.session); - this.reconcile(); - this.emitView(); - this.maybeRecoverStalePort(); + sessionChanged = true; } const nextBinaryPath = allowedBinaryPath(params.binaryPath); if (nextBinaryPath !== this.binaryPath) { this.binaryPath = nextBinaryPath; - this.maybeRecoverStalePort(); + binaryPathChanged = true; } if (params.wsPort !== this.wsPort) { + this.wsPort = params.wsPort; + portChanged = true; + } + if (portChanged) { // Mirrors the old useEffect(() => setStreamPort(wsPort), [wsPort]): a // `dor ab` re-run refreshing wsPort reconnects to the new port. - this.wsPort = params.wsPort; this.setStreamPort(params.wsPort); + } else if (sessionChanged) { + this.reconcile(); + this.emitView(); + this.maybeRecoverStalePort(); + } else if (binaryPathChanged) { + this.maybeRecoverStalePort(); } if (params.url !== this.paramsUrl) { this.paramsUrl = params.url; @@ -630,7 +654,7 @@ export class AgentBrowserSurfaceController { } private reconcileConnection(): void { - const desired = !this.disposed && !!this.streamPort && !!this.session && !this.parked; + const desired = !this.disposed && !!this.streamPort && !!this.session && !this.parked && !this.relaunching; // recoverySeq forces a reconnect at the same session/port (a same-port // `dor ab` refresh); it is part of the identity key so a bump re-creates. const key = desired ? `${this.session}:${this.streamPort}:${this.recoverySeq}` : null; @@ -693,6 +717,12 @@ export class AgentBrowserSurfaceController { this.maybeDisengageSync(); this.publishScreen(); } + } else if (event.type === 'url') { + // A navigation committed. `tabs` catches up only when the driving + // command completes — for a slow page, the whole load — so record it now: + // the header follows, and a relaunch mid-load carries the page being + // loaded rather than the one before it. + this.applyStreamUrl(event.url); } else if (event.type === 'tabs') { const prevActiveId = event.previousTabs.find((t) => t.active)?.tabId; const nextActiveId = event.tabs.find((t) => t.active)?.tabId; @@ -792,7 +822,10 @@ export class AgentBrowserSurfaceController { // headed window without polling. private reconcileCdp(): void { const platform = getPlatform(); - const desired = !this.disposed && this.poppedOut && !!this.session && !!platform.agentBrowserCommand; + // `get cdp-url` is a daemon command: issued mid-relaunch it lands on the + // daemon being killed, or spawns a competing one in the gap before the + // headed relaunch — which then reattaches headless ("--headed ignored"). + const desired = !this.disposed && this.poppedOut && !this.relaunching && !!this.session && !!platform.agentBrowserCommand; const key = desired ? `${this.session}:${this.streamPort}` : null; if (key === this.cdpKey) return; this.cdpTeardown?.(); @@ -1083,6 +1116,21 @@ export class AgentBrowserSurfaceController { : tab)); } + // The stream's `url` message names the active tab's new URL and nothing else; + // the previous page's title no longer describes it, so the tab falls back to + // its URL until the load completes and `tabs` brings the real title. + private applyStreamUrl(url: string): void { + if (!isRestorableUrl(url)) return; + this.rememberRestorableUrl(url); + const active = this.activeTab(); + if (!active) { + this.setTabs([{ tabId: 'stream-active', title: null, url, active: true }]); + return; + } + if (active.url === url) return; + this.setTabs(this.tabs.map((tab) => (tab === active ? { ...tab, url, title: null } : tab))); + } + private currentRelaunchUrl(): string | undefined { return [ this.latestRestorableUrl, @@ -1226,67 +1274,79 @@ export class AgentBrowserSurfaceController { // pane becomes a stub; the stream stays connected to observe tabs/status and // to auto-revert when the window closes. The new Chrome process gets a fresh // stream port, which we write into params so the WS reconnects. + // Enter/leave the relaunch window. Entering drops the stream connection and + // CDP observer (reconcile gates on `relaunching`); leaving reconnects to + // whatever port is current, so callers set the host's port BEFORE leaving. + private setRelaunching(relaunching: boolean): void { + if (this.relaunching === relaunching) return; + this.relaunching = relaunching; + this.emitView(); + this.reconcile(); + } + private popOut(): void { const platform = getPlatform(); const session = this.session; if (!session || !platform.agentBrowserPopOut) return; + // One relaunch at a time: a second pop-out/pop-in while the host is mid + // close→kill→reopen would interleave two relaunches of one session. + if (this.relaunching) { abDebugLog('[ab-panel] popOut ignored: relaunch in flight'); return; } if (this.closeIfSessionMarkedClosed(session)) return; this.headedConnected = false; - this.relaunching = true; + this.setRelaunching(true); this.setPoppedOut(true); this.writeParams({ renderMode: 'ab-popout' }); // Pop-out failed: revert to in-pane unless the stream came back live anyway. const revertUnlessLive = () => this.reconcileStreamPort().then((live) => { - this.relaunching = false; - if (live) return; - this.setPoppedOut(false); - this.writeParams({ renderMode: 'ab-screencast' }); + if (!live) { + this.setPoppedOut(false); + this.writeParams({ renderMode: 'ab-screencast' }); + } + this.setRelaunching(false); }); - // Don't reconcile to the current (headless) port first — it's about to close. // Connect to the headed window's fresh port once the relaunch returns it. const url = this.currentRelaunchUrl(); abDebugLog(`[ab-panel] popOut -> ${JSON.stringify({ session, url })}`); platform.agentBrowserPopOut(session, { rect: paneScreenRect(this.sink?.viewport), url }, this.binaryPath).then((res) => { abDebugLog(`[ab-panel] popOut result ${JSON.stringify(res)}`); - if (this.closeIfSessionMarkedClosed(session)) return; + if (this.closeIfSessionMarkedClosed(session)) { this.setRelaunching(false); return; } if (!res.ok) { void revertUnlessLive(); return; } void this.reconcileStreamPort(res.wsPort); - this.relaunching = false; + this.setRelaunching(false); }).catch((err) => { abDebugLog(`[ab-panel] popOut error ${String(err)}`); - if (this.closeIfSessionMarkedClosed(session)) return; + if (this.closeIfSessionMarkedClosed(session)) { this.setRelaunching(false); return; } void revertUnlessLive(); }); } popIn(): void { const session = this.session; + // A session-less pane (an eager swap whose daemon is still booting) has + // nothing to relaunch yet; the handover lands as a params refresh. + if (!session) return; + if (this.relaunching) { abDebugLog('[ab-panel] popIn ignored: relaunch in flight'); return; } if (this.closeIfSessionMarkedClosed(session)) return; - // Same expected mid-relaunch stream drop as pop-out: suppress screenshot - // pulses so none relaunches the just-closed browser at about:blank. - this.relaunching = true; + this.setRelaunching(true); this.setPoppedOut(false); this.writeParams({ renderMode: 'ab-screencast' }); const platform = getPlatform(); - if (!session || !platform.agentBrowserPopIn) { this.relaunching = false; return; } - // Don't reconcile to the current (headed) port first — the host is about to - // kill that daemon. Querying now would spawn a competing daemon. Connect to - // the fresh port the host returns. + if (!platform.agentBrowserPopIn) { this.setRelaunching(false); return; } + // Connect to the fresh port the host returns; the current (headed) port is + // about to die with its daemon. const url = this.currentRelaunchUrl(); abDebugLog(`[ab-panel] popIn -> ${JSON.stringify({ session, url })}`); platform.agentBrowserPopIn(session, { url }, this.binaryPath).then((res) => { abDebugLog(`[ab-panel] popIn result ${JSON.stringify(res)}`); - if (this.closeIfSessionMarkedClosed(session)) { this.relaunching = false; return; } - if (res.ok) void this.reconcileStreamPort(res.wsPort); - else void this.reconcileStreamPort(); - this.relaunching = false; + if (this.closeIfSessionMarkedClosed(session)) { this.setRelaunching(false); return; } + return (res.ok ? this.reconcileStreamPort(res.wsPort) : this.reconcileStreamPort()) + .then(() => this.setRelaunching(false)); }).catch(() => { - if (this.closeIfSessionMarkedClosed(session)) { this.relaunching = false; return; } - void this.reconcileStreamPort(); - this.relaunching = false; + if (this.closeIfSessionMarkedClosed(session)) { this.setRelaunching(false); return; } + void this.reconcileStreamPort().then(() => this.setRelaunching(false)); }); } diff --git a/lib/src/components/wall/use-dor-control.ts b/lib/src/components/wall/use-dor-control.ts index dbf77b98f..fd3341692 100644 --- a/lib/src/components/wall/use-dor-control.ts +++ b/lib/src/components/wall/use-dor-control.ts @@ -392,7 +392,13 @@ export function useDorControl({ 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 } { +}): { + connectPort: (id: string, url: string) => Promise; + /** Fold a params patch onto a surface (visible pane or minimized door) — the + * one write path a background daemon boot uses to hand a session-less pane + * its `{session, wsPort, binaryPath}`. */ + updateSurfaceParams: (id: string, patch: Record) => void; +} { const resolveVisibleSurface = useCallback(( target: string | undefined, callerSurfaceId: string | undefined, @@ -1096,5 +1102,5 @@ export function useDorControl({ return () => window.removeEventListener('dormouse:control-request', handler); }, [buildDorSurfaces, buildDorSurfaceList, createContentSurface, createSplitSurface, ensureAgentBrowserSurface, findSurfaceIdRunningCommand, killPaneImmediately, requireBrowserSurface, requireListedSurface, requireTerminalSurface, resolveListedSurface, resolveVisibleSurface, surfaceRefForId, lath, nav]); - return { connectPort }; + return { connectPort, updateSurfaceParams }; } diff --git a/lib/src/host/agent-browser-host.test.ts b/lib/src/host/agent-browser-host.test.ts index 23d083647..5f2a229f1 100644 --- a/lib/src/host/agent-browser-host.test.ts +++ b/lib/src/host/agent-browser-host.test.ts @@ -1,4 +1,5 @@ import { existsSync, mkdtempSync, promises as fsp, statSync, writeFileSync } from 'fs'; +import { createServer, type Server } from 'net'; import { tmpdir } from 'os'; import { dirname, join } from 'path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -8,6 +9,59 @@ type SpawnResult = { stdout?: string; stderr?: string; code?: number }; const spawnMock = vi.hoisted(() => vi.fn()); +// A pid no process on this machine can hold (macOS/Linux pid_max is far lower), +// so `process.kill(pid, 0)` answers ESRCH: "the daemon exited". +const DEAD_PID = 2147483000; + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((r) => { resolve = r; }); + return { promise, resolve }; +} + +const spawnResult = (result: SpawnResult) => ({ + ok: true as const, + exitCode: result.code ?? 0, + stdout: result.stdout ?? '', + stderr: result.stderr ?? '', +}); + +/** Dispatch spawns by their subcommand (the args after `--session `), + * for flows where `open` must stay pending while other commands answer. */ +function mockSpawnByCommand(handlers: Record Promise | SpawnResult>) { + const calls: string[][] = []; + spawnMock.mockImplementation(async (_binary: string, args: string[]) => { + calls.push(args); + const rest = args[0] === '--session' ? args.slice(2) : args; + const key = rest[0] === '--headed' ? `--headed ${rest[1]}` : rest[0]; + const handler = handlers[key]; + if (!handler) throw new Error(`unexpected spawn: ${args.join(' ')}`); + return spawnResult(await handler(args)); + }); + return calls; +} + +/** A listener standing in for the daemon's stream server, so a port named in + * `.stream` actually accepts connections. */ +async function listen(): Promise<{ port: number; server: Server }> { + const server = createServer(); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const address = server.address(); + if (!address || typeof address === 'string') throw new Error('no port'); + return { port: address.port, server }; +} + +/** A port nothing listens on: bind, read it, release it. */ +async function closedPort(): Promise { + const { port, server } = await listen(); + await new Promise((resolve) => server.close(() => resolve())); + return port; +} + +function writeState(session: string, ext: 'pid' | 'stream', value: number): void { + writeFileSync(join(process.env.AGENT_BROWSER_SOCKET_DIR!, `${session}.${ext}`), `${value}\n`); +} + // The host spawns through dor-lib-common's spawnAndCapture; mock just that // boundary (not its internal cross-spawn — spawnAndCapture's own behavior is // covered by dor-lib-common's tests), keeping the package's other real exports @@ -45,9 +99,12 @@ describe('agent-browser host relaunch', () => { }); it('closes a stray about:blank tab when tab list reports CLI-style id fields', async () => { + // No pid file here (an older CLI): the port comes from `stream status` once + // `open` has returned, and the sweep runs after that. enqueueSpawnResults([ {}, // close {}, // --headed open + { stdout: JSON.stringify({ port: 61218 }) }, { stdout: JSON.stringify({ tabs: [ @@ -57,17 +114,84 @@ describe('agent-browser host relaunch', () => { }), }, {}, // tab close blank-tab - { stdout: JSON.stringify({ port: 61218 }) }, ]); const host = createAgentBrowserHost({ writeClipboardText: vi.fn() }); const result = await host.popOut('dormouse.1.default', { url: 'https://example.com/' }, '/usr/local/bin/agent-browser'); expect(result).toEqual({ ok: true, wsPort: 61218 }); - expect(spawnMock).toHaveBeenCalledWith( - '/usr/local/bin/agent-browser', - ['--session', 'dormouse.1.default', 'tab', 'close', 'blank-tab'], - ); + await vi.waitFor(() => { + expect(spawnMock).toHaveBeenCalledWith( + '/usr/local/bin/agent-browser', + ['--session', 'dormouse.1.default', 'tab', 'close', 'blank-tab'], + ); + }); + }); + + it('pop-out returns the relaunched daemon\'s port while `open` is still waiting on the page', async () => { + // The killed daemon leaves its state files behind: a dead pid and a port + // nothing listens on. The relaunch must not read those as the new daemon. + const session = 'dormouse.1.default'; + const stale = await closedPort(); + writeState(session, 'pid', DEAD_PID); + writeState(session, 'stream', stale); + const opened = deferred(); + const calls = mockSpawnByCommand({ + close: () => ({}), + '--headed open': () => opened.promise, + tab: (args) => (args.includes('list') + ? { stdout: JSON.stringify({ tabs: [{ tabId: 'blank', url: 'about:blank', active: false }, { tabId: 'real', url: 'https://example.com/', active: true }] }) } + : {}), + }); + const host = createAgentBrowserHost({ writeClipboardText: vi.fn() }); + const popOut = host.popOut(session, { url: 'https://example.com/' }); + + // While the stale files are all there is, the launch waits. + await new Promise((resolve) => setTimeout(resolve, 350)); + expect(calls.some((args) => args.includes('tab'))).toBe(false); + // The new daemon comes up: a fresh pid and a port that accepts connections. + const { port, server } = await listen(); + try { + writeState(session, 'pid', DEAD_PID + 1); + writeState(session, 'stream', port); + expect(await popOut).toEqual({ ok: true, wsPort: port }); + // `open` has not returned, so no daemon command (the blank-tab sweep) has + // been queued behind it. + expect(calls.some((args) => args.includes('tab'))).toBe(false); + expect(calls.some((args) => args.includes('stream'))).toBe(false); + + opened.resolve({ code: 1, stderr: 'Operation timed out. The page may still be loading' }); + await vi.waitFor(() => { + expect(calls).toContainEqual(['--session', session, 'tab', 'close', 'blank']); + }); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } + }); + + it('open() treats a timed-out page load as a live launch, and a launch with no daemon as a failure', async () => { + const host = createAgentBrowserHost({ writeClipboardText: vi.fn() }); + let session = ''; + // Timed out, daemon up (pid file present): the browser is on the page. + mockSpawnByCommand({ + open: (args) => { + session = args[1]; + writeState(session, 'pid', DEAD_PID); + return { code: 1, stderr: 'Operation timed out. The page may still be loading' }; + }, + stream: () => ({ stdout: JSON.stringify({ port: 61219 }) }), + }); + expect(await host.open('https://slow.example/', {})).toEqual({ ok: true, session: expect.stringMatching(/^dormouse\.1\.gui-/), wsPort: 61219 }); + + // Failed with no daemon at all: fail, and close so nothing half-launched + // outlives the swap. + const calls = mockSpawnByCommand({ + open: () => ({ code: 1, stderr: 'boom' }), + close: () => ({}), + }); + expect(await host.open('https://slow.example/', {})).toEqual({ ok: false, error: 'boom' }); + expect(calls.some((args) => args[2] === 'close')).toBe(true); + expect(calls.some((args) => args[2] === 'stream')).toBe(false); }); }); diff --git a/lib/src/host/agent-browser-host.ts b/lib/src/host/agent-browser-host.ts index 115f6ae65..7cf22c2bb 100644 --- a/lib/src/host/agent-browser-host.ts +++ b/lib/src/host/agent-browser-host.ts @@ -36,6 +36,7 @@ * concern (the standalone webview's `tauri://localhost` origin is accepted, so * it connects directly). It stays in the VS Code host. */ +import * as net from 'net'; import * as os from 'os'; import * as path from 'path'; import { promises as fs, rmSync } from 'fs'; @@ -79,6 +80,10 @@ const EDIT_SCRIPTS: Record = { const STREAM_PORT_READ_ATTEMPTS = 4; const STREAM_PORT_READ_DELAY_MS = 150; +// How often a launch re-reads the daemon's state files while `open` is still +// waiting on the page (docs/specs/dor-browser.md → "Pop-Out"). +const LAUNCH_POLL_MS = 100; +const PORT_PROBE_TIMEOUT_MS = 500; const delay = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)); export interface AgentBrowserHostDeps { @@ -223,19 +228,39 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser return process.env.AGENT_BROWSER_SOCKET_DIR || path.join(os.homedir(), '.agent-browser'); } - async function killDaemon(session: string): Promise { - const pidFile = path.join(agentBrowserStateDir(), `${session}.pid`); - let pid: number; + // The daemon's state files beside its socket: `.pid` and + // `.stream` (the stream server's port, written as the daemon comes + // up — ~100ms into a launch, long before the page loads). Neither is cleaned + // up when the daemon is killed, so a reader must know which daemon wrote it. + async function readStateNumber(session: string, ext: 'pid' | 'stream'): Promise { try { - pid = Number.parseInt((await fs.readFile(pidFile, 'utf8')).trim(), 10); + const value = Number.parseInt((await fs.readFile(path.join(agentBrowserStateDir(), `${session}.${ext}`), 'utf8')).trim(), 10); + return Number.isInteger(value) && value > 0 ? value : undefined; } catch { - return; // no pid file — nothing to kill (already gone, or custom dir) + return undefined; // absent (no daemon yet, custom dir, or an older CLI) } - if (!Number.isInteger(pid) || pid <= 0) return; + } + + function portAccepts(port: number): Promise { + return new Promise((resolve) => { + const socket = net.connect({ host: '127.0.0.1', port }); + const settle = (accepted: boolean) => { socket.destroy(); resolve(accepted); }; + socket.once('connect', () => settle(true)); + socket.once('error', () => settle(false)); + socket.setTimeout(PORT_PROBE_TIMEOUT_MS, () => settle(false)); + }); + } + + /** Terminate the session's daemon and wait for it to exit. Returns the pid + * the pid file named (dead or not), so a relaunch can tell the daemon that + * replaces it from the stale state files it leaves behind. */ + async function killDaemon(session: string): Promise { + const pid = await readStateNumber(session, 'pid'); + if (pid === undefined) return undefined; // no pid file — nothing to kill (already gone, or custom dir) try { process.kill(pid, 'SIGTERM'); } catch { - return; // ESRCH: already dead + return pid; // ESRCH: already dead } // Wait for the process to actually exit (signal 0 throws once it's gone), so // the relaunch doesn't race a daemon that's still shutting down. @@ -244,12 +269,58 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser process.kill(pid, 0); } catch { log(`[ab-relaunch] daemon ${pid} for ${session} exited after ${i * 50}ms`); - return; + return pid; } await delay(50); } log(`[ab-relaunch] daemon ${pid} for ${session} still alive after 2s; SIGKILL`); try { process.kill(pid, 'SIGKILL'); } catch { /* ignore */ } + return pid; + } + + // `agent-browser open ` returns when the page's `load` event fires — up to + // the CLI's action timeout (25s in 0.31.1), after which it exits non-zero with + // the browser live on the page — and every other daemon command queues behind + // it. A transition that awaited it would block for the whole page load and + // then read a slow page as a failed launch. So the launch resolves as soon as + // the *daemon* is up: its pid file names a pid other than the one a relaunch + // just killed, and its stream file names a port that accepts a connection. The + // stream serves status/tabs/frames while `open` is still waiting, so the pane + // shows the page loading. Only once `open` has returned does the exit code + // matter, and then only if no daemon came up at all. + type Launch = { + wsPort: number | undefined; + /** Settles when `open` itself returns — possibly long after the launch. */ + opened: Promise; + }; + async function launch(session: string, args: string[], binaryPath: string | undefined, replacedPid?: number): Promise { + let settled: AgentBrowserCommandResult | undefined; + const opened = runWithBinaryFallback(args, binaryPath).then((result) => { + settled = result; + return result; + }); + for (;;) { + const pid = await readStateNumber(session, 'pid'); + const daemonUp = pid !== undefined && pid !== replacedPid; + if (settled) { + // A non-zero exit with the daemon up is a page that has not finished + // loading, not a failed launch. Without a pid file (an older CLI) the + // exit code is all there is. + if (settled.exitCode !== 0 && !daemonUp) return { wsPort: undefined, opened }; + return { wsPort: await readStreamPort(session, binaryPath), opened }; + } + if (daemonUp) { + const port = await readStateNumber(session, 'stream'); + if (port !== undefined && await portAccepts(port)) return { wsPort: port, opened }; + } + await delay(LAUNCH_POLL_MS); + } + } + + function logOpened(label: string, opened: Promise): void { + void opened.then((result) => { + log(`[ab-relaunch] ${label} exit=${result.exitCode}${result.stderr.trim() ? ` stderr=${result.stderr.trim()}` : ''}`); + }); } // After a relaunch, close any stray about:blank tab the close+reopen race can @@ -473,14 +544,21 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser if (typeof url !== 'string' || !url) return { ok: false, error: 'url is required' }; const session = generateGuiSession(); const args = ['--session', session, ...(opts?.headed ? ['--headed'] : []), 'open', url]; - const opened = await runWithBinaryFallback(args, binaryPath); - if (opened.exitCode !== 0) { - return { ok: false, error: opened.stderr.trim() || `open exited ${opened.exitCode}` }; - } - // A headed spawn is a real OS window — track it so shutdown can close it. + // A headed spawn is a real OS window — track it before the launch so a + // window whose page never finishes loading is still closed on shutdown. if (opts?.headed) poppedOutSessions.set(session, binaryPath); - const wsPort = await readStreamPort(session, binaryPath); - return { ok: true, session, ...(wsPort ? { wsPort } : {}), ...(binaryPath ? { binaryPath } : {}) }; + const { wsPort, opened } = await launch(session, args, binaryPath); + logOpened(`open session=${session}`, opened); + if (wsPort === undefined) { + poppedOutSessions.delete(session); + // Nothing came up to bind a surface to. Close whatever did, so a + // half-launched browser does not outlive the swap it was spawned for + // (a no-op when there is no daemon — `close` starts none). + await runWithBinaryFallback(['--session', session, 'close'], binaryPath); + const failed = await opened; + return { ok: false, error: failed.stderr.trim() || `open exited ${failed.exitCode}` }; + } + return { ok: true, session, wsPort, ...(binaryPath ? { binaryPath } : {}) }; } // Pop-out is a relaunch, not a live toggle: Chrome's headed/headless choice is @@ -500,18 +578,28 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser // Close the browser, then fully stop the daemon so the headed relaunch isn't // ignored as "daemon already running" (which would leave it headless). await runWithBinaryFallback(['--session', session, 'close'], binaryPath); - await killDaemon(session); - const opened = await runWithBinaryFallback(['--session', session, '--headed', 'open', url], binaryPath); - log(`[ab-relaunch] popOut headed-open exit=${opened.exitCode}${opened.stderr.trim() ? ` stderr=${opened.stderr.trim()}` : ''}`); - if (opened.exitCode !== 0) { - return { ok: false, error: opened.stderr.trim() || `headed open exited ${opened.exitCode}` }; - } - // Now a real headed OS window — track it so shutdown can close it. + const replacedPid = await killDaemon(session); + // A real headed OS window from here on — track it before the launch so + // shutdown closes it even if its page never finishes loading. poppedOutSessions.set(session, binaryPath); - await closeStrayBlankTabs(session, binaryPath); - const wsPort = await readStreamPort(session, binaryPath); - log(`[ab-relaunch] popOut returning wsPort=${wsPort}`); - return { ok: true, ...(wsPort ? { wsPort } : {}) }; + return relaunch('popOut', session, ['--session', session, '--headed', 'open', url], binaryPath, replacedPid); + } + + // Shared tail of pop-out/pop-in: launch, and once `open` itself returns — + // possibly well after the pane is already streaming — sweep the stray blank + // tab the close+reopen can leave (a daemon command, so it must not run while + // `open` still holds the queue). A launch that never published a port is the + // one failure: the exit code alone is not. + async function relaunch(label: string, session: string, args: string[], binaryPath: string | undefined, replacedPid: number | undefined): Promise { + const { wsPort, opened } = await launch(session, args, binaryPath, replacedPid); + logOpened(`${label} open`, opened); + if (wsPort === undefined) { + const failed = await opened; + return { ok: false, error: failed.stderr.trim() || `${label} open exited ${failed.exitCode}` }; + } + void opened.then(() => closeStrayBlankTabs(session, binaryPath)).catch(() => undefined); + log(`[ab-relaunch] ${label} returning wsPort=${wsPort}`); + return { ok: true, wsPort }; } // The reverse: close the headed session and relaunch it headless at the active @@ -527,18 +615,10 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser // Reverse of pop-out: the daemon is headed, so a plain `open` would reattach // to it and stay headed. Stop the daemon so the relaunch comes up headless. await runWithBinaryFallback(['--session', session, 'close'], binaryPath); - await killDaemon(session); + const replacedPid = await killDaemon(session); // The headed window is gone after the close above; back to headless. poppedOutSessions.delete(session); - const opened = await runWithBinaryFallback(['--session', session, 'open', url], binaryPath); - log(`[ab-relaunch] popIn open exit=${opened.exitCode}${opened.stderr.trim() ? ` stderr=${opened.stderr.trim()}` : ''}`); - if (opened.exitCode !== 0) { - return { ok: false, error: opened.stderr.trim() || `open exited ${opened.exitCode}` }; - } - await closeStrayBlankTabs(session, binaryPath); - const wsPort = await readStreamPort(session, binaryPath); - log(`[ab-relaunch] popIn returning wsPort=${wsPort}`); - return { ok: true, ...(wsPort ? { wsPort } : {}) }; + return relaunch('popIn', session, ['--session', session, 'open', url], binaryPath, replacedPid); } // Close every still-popped-out session's headed window. Called from each diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 01bb83d48..fc836b961 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -4,7 +4,7 @@ "docs/specs/alert.md": 6350, "docs/specs/auto-update.md": 1000, "docs/specs/deploy.md": 1900, - "docs/specs/dor-browser.md": 4200, + "docs/specs/dor-browser.md": 4500, "docs/specs/dor-cli.md": 4800, "docs/specs/dor-tool.md": 2100, "docs/specs/glossary.md": 2500, From 3198f83511f2061064e824d1e3218faf928dc0b4 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 11:31:47 -0700 Subject: [PATCH 02/12] fix(browser): preserve minimized eager swaps --- docs/specs/dor-browser.md | 2 +- lib/src/components/Wall.test.tsx | 94 ++++++++++++++++++++++++++++++++ lib/src/components/Wall.tsx | 31 +++++++++-- 3 files changed, 120 insertions(+), 7 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 4b24fce4d..9f526276c 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -200,7 +200,7 @@ size landed**, so a resize transient is not read as an external override. | From -> To | Behavior | | --- | --- | -| `iframe` -> `ab-screencast` / `ab-popout` | **The pane swaps at once** to a session-less agent-browser pane — inert, so it cannot race the boot (rationale) — while the host spawns a fresh `gui-` session at the current URL via `agentBrowserOpen` and hands over `{session, wsPort, binaryPath}` as **one** params refresh. `ab-popout` spawns headed in one shot, so the surface mounts already popped out. A spawn that yields no session restores the iframe; a pane killed meanwhile closes the session. Hidden/inert without that capability. | +| `iframe` -> `ab-screencast` / `ab-popout` | **The pane swaps at once** to a session-less agent-browser pane — inert, so it cannot race the boot (rationale) — while the host spawns a fresh `gui-` session at the current URL via `agentBrowserOpen` and hands over `{session, wsPort, binaryPath}` as **one** params refresh. `ab-popout` spawns headed in one shot, so the surface mounts already popped out. A spawn that yields no session restores the iframe; a Surface minimized meanwhile receives either outcome through its Door, while one killed meanwhile closes a spawned session. Hidden/inert without that capability. | | `ab-screencast` <-> `ab-popout` | Same session, headed/headless relaunch in `AgentBrowserPanel`; preserves only the active URL. | | `ab-*` -> `iframe` | Uses canonical `params.url`; with multiple tabs, requires the user to press `c` in the warning overlay, because only the active tab survives. | diff --git a/lib/src/components/Wall.test.tsx b/lib/src/components/Wall.test.tsx index 9a96e0c6f..556b306b2 100644 --- a/lib/src/components/Wall.test.tsx +++ b/lib/src/components/Wall.test.tsx @@ -12,6 +12,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { SURFACE_CONTROL_METHODS } from 'dor/protocol'; import { sessionForKey } from 'dor-lib-common/agent-browser'; import { Wall } from './Wall'; +import { getAgentBrowserScreenController } from './wall/agent-browser-screen'; import { setPlatform } from '../lib/platform'; import { FakePtyAdapter } from '../lib/platform/fake-adapter'; import type { PlatformAdapter } from '../lib/platform/types'; @@ -525,6 +526,99 @@ describe('Wall on the Lath engine', () => { } }); + it('hands an eager render-swap session to a Surface minimized during launch', async () => { + const untouchedSpy = vi.spyOn(terminalRegistry, 'isUntouched').mockReturnValue(true); + let resolveOpen!: (result: { ok: boolean; session?: string; wsPort?: number; binaryPath?: string }) => void; + const openResult = new Promise<{ ok: boolean; session?: string; wsPort?: number; binaryPath?: string }>((resolve) => { + resolveOpen = resolve; + }); + (fake as PlatformAdapter).agentBrowserOpen = vi.fn(() => openResult); + const agentBrowserCommand = vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })); + (fake as PlatformAdapter).agentBrowserCommand = agentBrowserCommand; + + try { + await act(async () => { + root.render(); + }); + await flush(); + const iframeId = (await dispatchIframe('http://localhost:5173/')).id; + + await act(async () => { + getAgentBrowserScreenController(iframeId)?.actions.setRenderMode?.('ab-screencast'); + }); + await flush(); + const eagerLeaf = container.querySelector('[data-lath-leaf]')!; + const eagerId = eagerLeaf.dataset.lathLeaf!; + expect(eagerId).not.toBe(iframeId); + + await act(async () => { + eagerLeaf.querySelector('[aria-label="Minimize"]')!.click(); + }); + await flush(); + expect(container.querySelector(`[data-door-id="${eagerId}"]`)).not.toBeNull(); + + await act(async () => { + resolveOpen({ ok: true, session: 'dormouse.1.gui-minimized', wsPort: 4321, binaryPath: '/usr/bin/agent-browser' }); + await openResult; + }); + await flush(); + + expect(await dispatchResolveAgentBrowser(eagerId)).toEqual({ + ok: true, + result: { surfaceId: eagerId, surfaceRef: 'surface:1', session: 'dormouse.1.gui-minimized' }, + }); + expect(agentBrowserCommand).not.toHaveBeenCalledWith( + 'dormouse.1.gui-minimized', + ['close'], + '/usr/bin/agent-browser', + ); + } finally { + untouchedSpy.mockRestore(); + } + }); + + it('restores a minimized eager render swap to iframe when launch returns no session', async () => { + const untouchedSpy = vi.spyOn(terminalRegistry, 'isUntouched').mockReturnValue(true); + let resolveOpen!: (result: { ok: boolean; error?: string }) => void; + const openResult = new Promise<{ ok: boolean; error?: string }>((resolve) => { + resolveOpen = resolve; + }); + (fake as PlatformAdapter).agentBrowserOpen = vi.fn(() => openResult); + + try { + await act(async () => { + root.render(); + }); + await flush(); + const iframeId = (await dispatchIframe('http://localhost:5173/')).id; + + await act(async () => { + getAgentBrowserScreenController(iframeId)?.actions.setRenderMode?.('ab-screencast'); + }); + await flush(); + const eagerLeaf = container.querySelector('[data-lath-leaf]')!; + const eagerId = eagerLeaf.dataset.lathLeaf!; + await act(async () => { + eagerLeaf.querySelector('[aria-label="Minimize"]')!.click(); + }); + await flush(); + + await act(async () => { + resolveOpen({ ok: false, error: 'launch failed' }); + await openResult; + }); + await flush(); + + expect(container.querySelector(`[data-door-id="${eagerId}"]`)).not.toBeNull(); + expect(await dispatchResolveAgentBrowser(eagerId)).toEqual({ + ok: false, + error: "surface 'surface:1' is not agent-browser rendered (render_mode: iframe)", + }); + } finally { + untouchedSpy.mockRestore(); + } + }); + it('resolves a browser surface handle to its agent-browser session, and gates the rest', async () => { const untouchedSpy = vi.spyOn(terminalRegistry, 'isUntouched').mockReturnValue(false); (fake as PlatformAdapter).agentBrowserCommand = vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })); diff --git a/lib/src/components/Wall.tsx b/lib/src/components/Wall.tsx index 121920aeb..14abfe86d 100644 --- a/lib/src/components/Wall.tsx +++ b/lib/src/components/Wall.tsx @@ -1352,12 +1352,30 @@ export function Wall({ title, }); if (!eagerId) return; + const eagerDoorExists = () => doorsRef.current.some((door) => door.id === eagerId); + const eagerSurfaceExists = () => ( + !!lath.getMeta(eagerId) && (lath.store.has(eagerId) || eagerDoorExists()) + ); + const restoreIframe = () => { + if (lath.isDying(eagerId)) return; + if (lath.store.has(eagerId)) { + replaceSurface(eagerId, { params: { surfaceType: 'browser', renderMode: 'iframe', url }, title }); + return; + } + // A minimized Surface is outside the visible tree but its Door + meta + // are still authoritative. Swap the parked body back in place so the + // Door does not remain a session-less "Connecting..." pane. + if (!eagerDoorExists() || !lath.getMeta(eagerId)) return; + disposeAgentBrowserSurfaceController(eagerId); + lath.store.updateParams(eagerId, { surfaceType: 'browser', renderMode: 'iframe', url, syncEngaged: false }); + lath.store.setTitle(eagerId, title); + }; platform.agentBrowserOpen(url, { headed }, lastAgentBrowserBinaryPathRef.current).then((res) => { if (!res.ok || !res.session) { console.warn(`[dormouse] failed to swap iframe surface '${id}' to agent-browser:`, res.error ?? '(no session)'); - // Nothing came up to bind: give the iframe back, if the eager pane is - // still on screen (a no-op when it was killed or minimized meanwhile). - replaceSurface(eagerId, { params: { surfaceType: 'browser', renderMode: 'iframe', url }, title }); + // Nothing came up to bind: give the iframe back if the eager Surface + // still exists, whether it is visible or minimized meanwhile. + restoreIframe(); return; } if (res.binaryPath) lastAgentBrowserBinaryPathRef.current = res.binaryPath; @@ -1366,9 +1384,10 @@ export function Wall({ ...(res.wsPort !== undefined ? { wsPort: res.wsPort } : {}), ...(res.binaryPath !== undefined ? { binaryPath: res.binaryPath } : {}), }; - // The eager pane was killed (or is mid-fade) while the daemon booted: - // the session has no pane to live in, so close it rather than leak it. - if (!lath.store.has(eagerId) || lath.isDying(eagerId)) { + // A Door is a retained Surface even though it is outside the visible + // tree. Close only when the eager Surface was genuinely destroyed (or + // its visible pane is mid-fade); otherwise hand the session to its meta. + if (!eagerSurfaceExists() || lath.isDying(eagerId)) { closeAgentBrowserSession({ renderMode: mode, ...bound }); return; } From daa1e3837355a9d87e66ef753482763c37af9667 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 11:32:22 -0700 Subject: [PATCH 03/12] fix(browser): roll back rejected eager launches --- docs/specs/dor-browser.md | 2 +- lib/src/components/Wall.test.tsx | 30 ++++++++++++++++++++++++++++++ lib/src/components/Wall.tsx | 1 + 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 9f526276c..ea79f6c83 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -200,7 +200,7 @@ size landed**, so a resize transient is not read as an external override. | From -> To | Behavior | | --- | --- | -| `iframe` -> `ab-screencast` / `ab-popout` | **The pane swaps at once** to a session-less agent-browser pane — inert, so it cannot race the boot (rationale) — while the host spawns a fresh `gui-` session at the current URL via `agentBrowserOpen` and hands over `{session, wsPort, binaryPath}` as **one** params refresh. `ab-popout` spawns headed in one shot, so the surface mounts already popped out. A spawn that yields no session restores the iframe; a Surface minimized meanwhile receives either outcome through its Door, while one killed meanwhile closes a spawned session. Hidden/inert without that capability. | +| `iframe` -> `ab-screencast` / `ab-popout` | **The pane swaps at once** to a session-less agent-browser pane — inert, so it cannot race the boot (rationale) — while the host spawns a fresh `gui-` session at the current URL via `agentBrowserOpen` and hands over `{session, wsPort, binaryPath}` as **one** params refresh. `ab-popout` spawns headed in one shot, so the surface mounts already popped out. A spawn that rejects or yields no session restores the iframe; a Surface minimized meanwhile receives either outcome through its Door, while one killed meanwhile closes a spawned session. Hidden/inert without that capability. | | `ab-screencast` <-> `ab-popout` | Same session, headed/headless relaunch in `AgentBrowserPanel`; preserves only the active URL. | | `ab-*` -> `iframe` | Uses canonical `params.url`; with multiple tabs, requires the user to press `c` in the warning overlay, because only the active tab survives. | diff --git a/lib/src/components/Wall.test.tsx b/lib/src/components/Wall.test.tsx index 556b306b2..df6220fe3 100644 --- a/lib/src/components/Wall.test.tsx +++ b/lib/src/components/Wall.test.tsx @@ -619,6 +619,36 @@ describe('Wall on the Lath engine', () => { } }); + it('restores an eager render swap to iframe when launch rejects', async () => { + const untouchedSpy = vi.spyOn(terminalRegistry, 'isUntouched').mockReturnValue(true); + (fake as PlatformAdapter).agentBrowserOpen = vi.fn(async () => { + throw new Error('transport failed'); + }); + + try { + await act(async () => { + root.render(); + }); + await flush(); + const iframeId = (await dispatchIframe('http://localhost:5173/')).id; + + await act(async () => { + getAgentBrowserScreenController(iframeId)?.actions.setRenderMode?.('ab-screencast'); + }); + await flush(); + + const restoredId = container.querySelector('[data-lath-leaf]')!.dataset.lathLeaf!; + expect(restoredId).not.toBe(iframeId); + expect(getAgentBrowserScreenController(restoredId)?.snapshot().renderMode).toBe('iframe'); + expect(await dispatchResolveAgentBrowser('surface:1')).toEqual({ + ok: false, + error: "surface 'surface:1' is not agent-browser rendered (render_mode: iframe)", + }); + } finally { + untouchedSpy.mockRestore(); + } + }); + it('resolves a browser surface handle to its agent-browser session, and gates the rest', async () => { const untouchedSpy = vi.spyOn(terminalRegistry, 'isUntouched').mockReturnValue(false); (fake as PlatformAdapter).agentBrowserCommand = vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })); diff --git a/lib/src/components/Wall.tsx b/lib/src/components/Wall.tsx index 14abfe86d..ade805d8b 100644 --- a/lib/src/components/Wall.tsx +++ b/lib/src/components/Wall.tsx @@ -1394,6 +1394,7 @@ export function Wall({ updateSurfaceParams(eagerId, bound); }).catch((err) => { console.warn('[dormouse] failed to swap iframe surface to agent-browser:', err); + restoreIframe(); }); } }, From 5b236edddecf4ee7a83cb17689cb9f4520ce449e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 11:33:09 -0700 Subject: [PATCH 04/12] fix(browser): dispatch oversized URL messages --- docs/specs/dor-browser.md | 7 ++++--- .../wall/agent-browser-connection.test.ts | 21 +++++++++++++++++++ .../wall/agent-browser-connection.ts | 18 +++++++++------- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index ea79f6c83..728d307f5 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -288,9 +288,10 @@ its own because clients trigger it. Rules park and recovery must not break: - **A `{session, wsPort, binaryPath}` refresh reconciles once**, straight to the port, with no `stream status`. -The stream WebSocket carries frame pulses + status, tab snapshots, `url`, and -native `input_mouse` / `input_keyboard` input. **`url` names the active tab at -navigation commit; `tabs` refreshes only when the driving command completes** +The stream carries frames, status, tab snapshots, `url`, and native +`input_mouse` / `input_keyboard` input. **Control envelopes dispatch at any +size.** **`url` names the active tab at navigation commit; `tabs` +refreshes only when the driving command completes** (for a slow page, after the load; rationale), so header and relaunch target follow `url`, title cleared until `tabs` names the page. diff --git a/lib/src/components/wall/agent-browser-connection.test.ts b/lib/src/components/wall/agent-browser-connection.test.ts index 79959eed1..ab11a5ac9 100644 --- a/lib/src/components/wall/agent-browser-connection.test.ts +++ b/lib/src/components/wall/agent-browser-connection.test.ts @@ -209,6 +209,27 @@ describe('agent-browser connection', () => { expect(connection.snapshot().tabs[0]).toMatchObject({ tabId: 't0', active: true }); }); + it('routes an oversized URL envelope as control data instead of a frame pulse', async () => { + const connection = createAgentBrowserConnection({ + session: 'dormouse.1.default', + streamPort: 1234, + }); + const events: unknown[] = []; + connection.subscribe((event) => { + if (event.type === 'url' || event.type === 'frame-pulse') events.push(event); + }); + + await Promise.resolve(); + const url = `data:text/plain,${'x'.repeat(20_000)}`; + const payload = JSON.stringify({ type: 'url', url, timestamp: 1 }); + expect(payload.length).toBeGreaterThan(16384); + + WebSocketMock.instances[0].emitMessage(payload); + + expect(events).toEqual([{ type: 'url', url }]); + connection.dispose(); + }); + it('re-primes after a reconnect so the first identical frame/tabs still forwards', async () => { vi.useFakeTimers(); try { diff --git a/lib/src/components/wall/agent-browser-connection.ts b/lib/src/components/wall/agent-browser-connection.ts index c7ed1f218..6f2a53a5f 100644 --- a/lib/src/components/wall/agent-browser-connection.ts +++ b/lib/src/components/wall/agent-browser-connection.ts @@ -260,13 +260,17 @@ export class AgentBrowserConnection { if (typeof raw !== 'string') return; if (raw.length > FRAME_PULSE_THRESHOLD) { // Size alone can't discriminate a frame from a control message: a `tabs` - // snapshot with many long URLs/titles crosses the threshold too, and routing - // it as a frame would silently drop the tab update. A frame's bulk is a - // base64 JPEG body, whose alphabet contains no `"` or `:`, so a compact - // `"type":"tabs"`/`"type":"status"` substring can never occur inside a real - // frame — it's a zero-false-positive marker for an oversized control message. - // Only those pay a parse; frames keep the hash+pulse fast path untouched. - if (raw.includes('"type":"tabs"') || raw.includes('"type":"status"')) { + // snapshot or URL with enough text crosses the threshold too, and routing + // it as a frame would silently drop the update. A frame's bulk is a base64 + // JPEG body, whose alphabet contains no `"` or `:`, so these compact type + // substrings cannot occur inside a real frame — they are zero-false-positive + // markers for oversized control messages. Only those pay a parse; frames + // keep the hash+pulse fast path untouched. + if ( + raw.includes('"type":"tabs"') + || raw.includes('"type":"status"') + || raw.includes('"type":"url"') + ) { this.dispatchControl(raw); return; } From 7b91f8ceec27107ef1ebad1ac426d033d340f4bd Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 11:34:10 -0700 Subject: [PATCH 05/12] fix(browser): clear titles on same-URL commits --- docs/specs/dor-browser.md | 4 +-- .../agent-browser-surface-controller.test.ts | 27 +++++++++++++++++++ .../wall/agent-browser-surface-controller.ts | 5 +++- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 728d307f5..c4563a9f0 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -292,8 +292,8 @@ The stream carries frames, status, tab snapshots, `url`, and native `input_mouse` / `input_keyboard` input. **Control envelopes dispatch at any size.** **`url` names the active tab at navigation commit; `tabs` refreshes only when the driving command completes** -(for a slow page, after the load; rationale), so header and relaunch target -follow `url`, title cleared until `tabs` names the page. +(for a slow page, after the load; rationale), so every commit clears the title +until `tabs` refreshes, even at the same URL. **Two-stage paint.** A changed stream JPEG paints at once as a **provisional frame** — the first image, and 250ms after pointer input (continuous movement 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 8cc01c2b1..43952f574 100644 --- a/lib/src/components/wall/agent-browser-surface-controller.test.ts +++ b/lib/src/components/wall/agent-browser-surface-controller.test.ts @@ -619,6 +619,33 @@ describe('relaunch (pop-out / pop-in)', () => { expect(platform.agentBrowserPopOut).toHaveBeenCalledWith('sess', expect.objectContaining({ url: 'https://slow.example/' }), undefined); }); + it('clears a stale title when navigation commits at the same URL', async () => { + const controller = acquireAgentBrowserSurfaceController('id', { + session: 'sess', wsPort: 1111, url: 'https://same.example/', + }); + const sink = makeSink(); + controller.attachView(sink); + await flushMicrotasks(); + const socket = streamSocket(1111); + socket?.emitMessage(JSON.stringify({ + type: 'tabs', + tabs: [{ tabId: 't1', title: 'Before reload', url: 'https://same.example/', active: true }], + })); + expect(getAgentBrowserScreenController('id')?.chrome().title).toBe('Before reload'); + + socket?.emitMessage(JSON.stringify({ type: 'url', url: 'https://same.example/' })); + + expect(getAgentBrowserScreenController('id')?.chrome()).toMatchObject({ + url: 'https://same.example/', + title: null, + }); + expect(sink.setTitle).toHaveBeenLastCalledWith('same.example'); + const titleWrites = sink.setTitle.mock.calls.length; + + socket?.emitMessage(JSON.stringify({ type: 'url', url: 'https://same.example/' })); + expect(sink.setTitle).toHaveBeenCalledTimes(titleWrites); + }); + it('a single {session, wsPort} handover connects once and asks the daemon nothing', async () => { const platform = relaunchPlatform(); const controller = acquireAgentBrowserSurfaceController('id', { renderMode: 'ab-screencast', url: 'https://x.example/' }); diff --git a/lib/src/components/wall/agent-browser-surface-controller.ts b/lib/src/components/wall/agent-browser-surface-controller.ts index 264ff8269..0b813a13c 100644 --- a/lib/src/components/wall/agent-browser-surface-controller.ts +++ b/lib/src/components/wall/agent-browser-surface-controller.ts @@ -1127,7 +1127,10 @@ export class AgentBrowserSurfaceController { this.setTabs([{ tabId: 'stream-active', title: null, url, active: true }]); return; } - if (active.url === url) return; + // A reload commits the same URL but invalidates the old document title just + // as surely as a different-URL navigation. Once cleared, repeat URL events + // are a true no-op until `tabs` supplies the refreshed title. + if (active.url === url && active.title === null) return; this.setTabs(this.tabs.map((tab) => (tab === active ? { ...tab, url, title: null } : tab))); } From e363d768b41d0fc9dfa2f755e64c9b875028dceb Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 13:40:35 -0700 Subject: [PATCH 06/12] fix(browser): cancel superseded relaunch sweeps --- docs/specs/dor-browser.md | 6 ++-- docs/specs/dor-browser.rationale.md | 2 ++ lib/src/host/agent-browser-host.test.ts | 38 ++++++++++++++++++++ lib/src/host/agent-browser-host.ts | 46 ++++++++++++++++++++++--- 4 files changed, 84 insertions(+), 8 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index c4563a9f0..227db63d6 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -365,9 +365,9 @@ closes what it spawned. **A headed session is tracked for shutdown before its launch**, so a window whose page never loads is still closed. **Never query the daemon during the close/reopen gap** (rationale), host and controller park/recovery paths alike, so **Dormouse supplies the active-tab URL and the -host trusts it**. Once `open` returns, the host best-effort closes any stray -`about:blank` tab the close+reopen race left behind, **but only while a real -page is open**, so it never closes the sole tab (rationale). +host trusts it**. Once `open` returns, only the latest relaunch best-effort +closes stray `about:blank` tabs, **and only while a real page is open**, so it +never closes the sole tab (rationale). While popped out, Dormouse keeps a stream/CDP observer so URL/header state follows same-tab navigation and a headed window close can auto-revert to diff --git a/docs/specs/dor-browser.rationale.md b/docs/specs/dor-browser.rationale.md index 883c7d823..3905e5b39 100644 --- a/docs/specs/dor-browser.rationale.md +++ b/docs/specs/dor-browser.rationale.md @@ -58,6 +58,8 @@ **Why nothing may query the daemon during the close/reopen gap.** With the old daemon dead and the new one not yet up, a `stream status` or tab query spawns a *competing* daemon at `about:blank` — agent-browser's CLI starts one on demand — and the relaunch then races two daemons for the same session. +A post-open blank-tab sweep can become such a query when a later relaunch starts before the earlier page finishes loading, so the host invalidates the earlier sweep as soon as the later close begins. + **Why the stray-`about:blank` sweep is guarded.** The close/reopen pair can leave an extra blank tab beside the navigated one. Sweeping blanks unconditionally is the obvious fix and is wrong: a session whose only tab is legitimately blank would lose it, leaving the pane with nothing to show. ## Agent-Browser Host Capabilities diff --git a/lib/src/host/agent-browser-host.test.ts b/lib/src/host/agent-browser-host.test.ts index 5f2a229f1..426cd8c54 100644 --- a/lib/src/host/agent-browser-host.test.ts +++ b/lib/src/host/agent-browser-host.test.ts @@ -169,6 +169,44 @@ describe('agent-browser host relaunch', () => { } }); + it('does not run an earlier relaunch\'s blank-tab sweep after a later relaunch begins', async () => { + const session = 'dormouse.1.default'; + const firstOpened = deferred(); + const secondClose = deferred(); + let closeCount = 0; + const calls = mockSpawnByCommand({ + close: () => (++closeCount === 1 ? {} : secondClose.promise), + '--headed open': () => firstOpened.promise, + tab: () => ({ + stdout: JSON.stringify({ + tabs: [ + { tabId: 'blank', url: 'about:blank', active: false }, + { tabId: 'real', url: 'https://example.com/', active: true }, + ], + }), + }), + }); + const host = createAgentBrowserHost({ writeClipboardText: vi.fn() }); + const popOut = host.popOut(session, { url: 'https://example.com/' }); + const { port, server } = await listen(); + try { + writeState(session, 'pid', DEAD_PID); + writeState(session, 'stream', port); + expect(await popOut).toEqual({ ok: true, wsPort: port }); + + // The second relaunch invalidates the first one's post-open tail before + // its close queues behind that still-pending `open` command. + void host.popIn(session, { url: 'https://example.com/' }); + await vi.waitFor(() => expect(closeCount).toBe(2)); + firstOpened.resolve({ code: 1, stderr: 'Operation timed out' }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(calls.some((args) => args.includes('tab'))).toBe(false); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } + }); + it('open() treats a timed-out page load as a live launch, and a launch with no daemon as a failure', async () => { const host = createAgentBrowserHost({ writeClipboardText: vi.fn() }); let session = ''; diff --git a/lib/src/host/agent-browser-host.ts b/lib/src/host/agent-browser-host.ts index 7cf22c2bb..18bfacf77 100644 --- a/lib/src/host/agent-browser-host.ts +++ b/lib/src/host/agent-browser-host.ts @@ -122,6 +122,17 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser // Headless sessions are deliberately NOT tracked — they're left alive to // reattach across webview reloads (the wsPort/stream-recovery design). const poppedOutSessions = new Map(); + // A relaunch returns once the daemon is streamable, while its `open` command + // may remain pending until page load. Key the post-open blank-tab sweep so a + // later pop-in/pop-out invalidates every command left behind by the previous + // relaunch before starting its own close -> kill -> reopen gap. + const relaunchGenerations = new Map(); + let nextRelaunchGeneration = 0; + function beginRelaunch(session: string): number { + const generation = ++nextRelaunchGeneration; + relaunchGenerations.set(session, generation); + return generation; + } // The host's PATH is often the GUI login PATH (no nvm/volta shims), so prefer // the absolute path `dor ab` resolved in the user's terminal; fall through on @@ -326,12 +337,21 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser // After a relaunch, close any stray about:blank tab the close+reopen race can // leave behind — but only when a real page is open, so we never close the sole // tab. Best-effort: a failure here must not fail the pop-out/pop-in. - async function closeStrayBlankTabs(session: string, binaryPath?: string): Promise { + async function closeStrayBlankTabs( + session: string, + binaryPath?: string, + current: () => boolean = () => true, + ): Promise { + if (!current()) return; const tabs = await listTabs(session, binaryPath); + // The list may have queued behind `open`; a newer relaunch can begin while + // it waits. Never issue a tab close into that relaunch's daemon gap. + if (!current()) return; log(`[ab-relaunch] tabs after open: ${JSON.stringify(tabs)}`); if (tabs.length < 2 || !tabs.some((t) => usableRelaunchUrl(t.url))) return; for (const tab of tabs) { if (!usableRelaunchUrl(tab.url)) { + if (!current()) return; log(`[ab-relaunch] closing stray blank tab ${tab.tabId}`); await runWithBinaryFallback(['--session', session, 'tab', 'close', tab.tabId], binaryPath); } @@ -573,6 +593,7 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser binaryPath?: string, ): Promise { if (typeof session !== 'string' || !session) return { ok: false, error: 'session is required' }; + const generation = beginRelaunch(session); const url = relaunchUrl(opts?.url); log(`[ab-relaunch] popOut session=${session} requestedUrl=${JSON.stringify(opts?.url)} -> open ${url}`); // Close the browser, then fully stop the daemon so the headed relaunch isn't @@ -582,7 +603,7 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser // A real headed OS window from here on — track it before the launch so // shutdown closes it even if its page never finishes loading. poppedOutSessions.set(session, binaryPath); - return relaunch('popOut', session, ['--session', session, '--headed', 'open', url], binaryPath, replacedPid); + return relaunch('popOut', session, ['--session', session, '--headed', 'open', url], binaryPath, replacedPid, generation); } // Shared tail of pop-out/pop-in: launch, and once `open` itself returns — @@ -590,14 +611,28 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser // tab the close+reopen can leave (a daemon command, so it must not run while // `open` still holds the queue). A launch that never published a port is the // one failure: the exit code alone is not. - async function relaunch(label: string, session: string, args: string[], binaryPath: string | undefined, replacedPid: number | undefined): Promise { + async function relaunch( + label: string, + session: string, + args: string[], + binaryPath: string | undefined, + replacedPid: number | undefined, + generation: number, + ): Promise { const { wsPort, opened } = await launch(session, args, binaryPath, replacedPid); logOpened(`${label} open`, opened); if (wsPort === undefined) { + if (relaunchGenerations.get(session) === generation) relaunchGenerations.delete(session); const failed = await opened; return { ok: false, error: failed.stderr.trim() || `${label} open exited ${failed.exitCode}` }; } - void opened.then(() => closeStrayBlankTabs(session, binaryPath)).catch(() => undefined); + const current = () => relaunchGenerations.get(session) === generation; + void opened + .then(() => closeStrayBlankTabs(session, binaryPath, current)) + .catch(() => undefined) + .finally(() => { + if (current()) relaunchGenerations.delete(session); + }); log(`[ab-relaunch] ${label} returning wsPort=${wsPort}`); return { ok: true, wsPort }; } @@ -610,6 +645,7 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser binaryPath?: string, ): Promise { if (typeof session !== 'string' || !session) return { ok: false, error: 'session is required' }; + const generation = beginRelaunch(session); const url = relaunchUrl(opts?.url); log(`[ab-relaunch] popIn session=${session} requestedUrl=${JSON.stringify(opts?.url)} -> open ${url}`); // Reverse of pop-out: the daemon is headed, so a plain `open` would reattach @@ -618,7 +654,7 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser const replacedPid = await killDaemon(session); // The headed window is gone after the close above; back to headless. poppedOutSessions.delete(session); - return relaunch('popIn', session, ['--session', session, 'open', url], binaryPath, replacedPid); + return relaunch('popIn', session, ['--session', session, 'open', url], binaryPath, replacedPid, generation); } // Close every still-popped-out session's headed window. Called from each From 6f52787a09ed1592b57442255932c205c33d18ff Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 13:41:49 -0700 Subject: [PATCH 07/12] fix(browser): report missing launch ports clearly --- docs/specs/dor-browser.md | 8 ++++---- lib/src/host/agent-browser-host.test.ts | 23 +++++++++++++++++++++++ lib/src/host/agent-browser-host.ts | 12 ++++++++++-- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 227db63d6..b6e75c7b0 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -360,10 +360,10 @@ resolves once the *relaunched* daemon is up (a pid file naming a pid other than the killed one, and a `.stream` file naming a port that accepts a connection), asking `stream status` only after `open` returns. **A non-zero `open` exit with the daemon up is a page still loading, not a failed launch**; -only a launch that never publishes a port fails, and then `agentBrowserOpen` -closes what it spawned. **A headed session is tracked for shutdown before its -launch**, so a window whose page never loads is still closed. **Never query the -daemon during the close/reopen gap** (rationale), host and controller +only a launch without a published port fails, including after a zero exit; +`agentBrowserOpen` then closes its spawn. **A headed session is tracked for +shutdown before its launch**, so a window whose page never loads is still +closed. **Never query the daemon during the close/reopen gap** (rationale), host and controller park/recovery paths alike, so **Dormouse supplies the active-tab URL and the host trusts it**. Once `open` returns, only the latest relaunch best-effort closes stray `about:blank` tabs, **and only while a real page is open**, so it diff --git a/lib/src/host/agent-browser-host.test.ts b/lib/src/host/agent-browser-host.test.ts index 426cd8c54..402939903 100644 --- a/lib/src/host/agent-browser-host.test.ts +++ b/lib/src/host/agent-browser-host.test.ts @@ -231,6 +231,29 @@ describe('agent-browser host relaunch', () => { expect(calls.some((args) => args[2] === 'close')).toBe(true); expect(calls.some((args) => args[2] === 'stream')).toBe(false); }); + + it('reports a zero-exit launch that publishes no stream port without claiming it exited unsuccessfully', async () => { + const host = createAgentBrowserHost({ writeClipboardText: vi.fn() }); + mockSpawnByCommand({ + open: () => ({}), + stream: () => ({ stdout: '{}' }), + close: () => ({}), + }); + expect(await host.open('https://example.com/', {})).toEqual({ + ok: false, + error: 'open published no stream port', + }); + + mockSpawnByCommand({ + close: () => ({}), + '--headed open': () => ({}), + stream: () => ({ stdout: '{}' }), + }); + expect(await host.popOut('dormouse.1.default', { url: 'https://example.com/' })).toEqual({ + ok: false, + error: 'popOut open published no stream port', + }); + }); }); describe('agent-browser host screenshot transport', () => { diff --git a/lib/src/host/agent-browser-host.ts b/lib/src/host/agent-browser-host.ts index 18bfacf77..252816357 100644 --- a/lib/src/host/agent-browser-host.ts +++ b/lib/src/host/agent-browser-host.ts @@ -334,6 +334,14 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser }); } + function launchFailure(label: string, result: AgentBrowserCommandResult): string { + const stderr = result.stderr.trim(); + if (stderr) return stderr; + return result.exitCode === 0 + ? `${label} published no stream port` + : `${label} exited ${result.exitCode}`; + } + // After a relaunch, close any stray about:blank tab the close+reopen race can // leave behind — but only when a real page is open, so we never close the sole // tab. Best-effort: a failure here must not fail the pop-out/pop-in. @@ -576,7 +584,7 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser // (a no-op when there is no daemon — `close` starts none). await runWithBinaryFallback(['--session', session, 'close'], binaryPath); const failed = await opened; - return { ok: false, error: failed.stderr.trim() || `open exited ${failed.exitCode}` }; + return { ok: false, error: launchFailure('open', failed) }; } return { ok: true, session, wsPort, ...(binaryPath ? { binaryPath } : {}) }; } @@ -624,7 +632,7 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser if (wsPort === undefined) { if (relaunchGenerations.get(session) === generation) relaunchGenerations.delete(session); const failed = await opened; - return { ok: false, error: failed.stderr.trim() || `${label} open exited ${failed.exitCode}` }; + return { ok: false, error: launchFailure(`${label} open`, failed) }; } const current = () => relaunchGenerations.get(session) === generation; void opened From 27c19321f23e1aa2b46781c1bef571ed4eecfdb1 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 13:43:27 -0700 Subject: [PATCH 08/12] fix(browser): reconcile session changes at live ports --- docs/specs/dor-browser.md | 4 +-- docs/specs/dor-browser.rationale.md | 2 ++ .../agent-browser-surface-controller.test.ts | 28 +++++++++++++++++++ .../wall/agent-browser-surface-controller.ts | 6 +++- 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index b6e75c7b0..05e4319f3 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -285,8 +285,8 @@ its own because clients trigger it. Rules park and recovery must not break: relaunching placeholder, and reconnects only to the port the host hands back (rationale). **One relaunch at a time**: a pop-out or pop-in issued during one is ignored; a session-less pane has nothing to relaunch. -- **A `{session, wsPort, binaryPath}` refresh reconciles once**, straight to the - port, with no `stream status`. +- **A `{session, wsPort, binaryPath}` refresh reconciles its session even at the + live port**, with no `stream status`. The stream carries frames, status, tab snapshots, `url`, and native `input_mouse` / `input_keyboard` input. **Control envelopes dispatch at any diff --git a/docs/specs/dor-browser.rationale.md b/docs/specs/dor-browser.rationale.md index 3905e5b39..d8c198f5e 100644 --- a/docs/specs/dor-browser.rationale.md +++ b/docs/specs/dor-browser.rationale.md @@ -22,6 +22,8 @@ **Why the handover is a single params refresh.** Setting `session` is what reconciles the controller and connects it, so landing it ahead of `wsPort`/`binaryPath` — or before `agent-browser open` has returned — connects against a daemon that is not up. Handing it over even after a failed `open` lets the placeholder name what it is waiting for; the menu that would have reported the error closed long ago. +The persisted `wsPort` mirror can lag the controller's already-live port after a buffered write, so a simultaneous session change still reconciles when setting that port itself is a no-op. + ## Display Modal And Render Swaps **Why the iframe swap is eager.** The same 1–3s daemon boot as the context-menu connect, behind a modal that has already closed; and while the swap awaited `open`, a slow page held the iframe on screen for the whole load and a timed-out `open` dropped the swap silently — leaving an orphan `gui-` browser nobody could see or close. 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 43952f574..0da1755a3 100644 --- a/lib/src/components/wall/agent-browser-surface-controller.test.ts +++ b/lib/src/components/wall/agent-browser-surface-controller.test.ts @@ -375,6 +375,34 @@ describe('updateParams', () => { expect(sink.updateParameters.mock.calls.length).toBe(writesBefore); expect(streamSockets(1111).length).toBe(1); }); + + it('reconciles a changed session when its params port already matches the live port', async () => { + const platform = new FakePtyAdapter() as FakePtyAdapter & Pick; + platform.agentBrowserCommand = vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })); + platform.agentBrowserPopOut = vi.fn(async () => ({ ok: true, wsPort: 2222 })); + setPlatform(platform); + + const controller = acquireAgentBrowserSurfaceController('id', { session: 'old-session', wsPort: 1111 }); + controller.attachView(makeSink()); + await flushMicrotasks(); + + // The relaunch adopts 2222 immediately, but params.wsPort remains 1111 + // until the view's buffered write echoes back through updateParams. + getAgentBrowserScreenController('id')?.actions.setRenderMode?.('ab-popout'); + await flushMicrotasks(); + const oldSessionSocket = streamSocket(2222); + expect(oldSessionSocket?.readyState).toBe(1); + expect(streamSockets(2222)).toHaveLength(1); + + // The echo also hands over a new session. Since 2222 is already the live + // port, setStreamPort no-ops; session reconciliation must still replace the + // old-session connection with one keyed to new-session. + controller.updateParams({ session: 'new-session', wsPort: 2222 }); + await flushMicrotasks(); + + expect(oldSessionSocket?.readyState).toBe(3); + expect(streamSockets(2222)).toHaveLength(2); + }); }); describe('stale-port recovery gating', () => { diff --git a/lib/src/components/wall/agent-browser-surface-controller.ts b/lib/src/components/wall/agent-browser-surface-controller.ts index 0b813a13c..a5a8dbcf6 100644 --- a/lib/src/components/wall/agent-browser-surface-controller.ts +++ b/lib/src/components/wall/agent-browser-surface-controller.ts @@ -576,7 +576,11 @@ export class AgentBrowserSurfaceController { // Mirrors the old useEffect(() => setStreamPort(wsPort), [wsPort]): a // `dor ab` re-run refreshing wsPort reconnects to the new port. this.setStreamPort(params.wsPort); - } else if (sessionChanged) { + } + // Run this even after the port arm: the persisted params mirror (`wsPort`) + // can lag the already-live `streamPort`, making setStreamPort a no-op while + // the session identity still changes underneath that connection. + if (sessionChanged) { this.reconcile(); this.emitView(); this.maybeRecoverStalePort(); From b39fbcec0c80934b30d8239eb3ec4899c8d3e461 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 13:48:38 -0700 Subject: [PATCH 09/12] test(browser): order relaunch daemon fixture --- lib/src/host/agent-browser-host.test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/src/host/agent-browser-host.test.ts b/lib/src/host/agent-browser-host.test.ts index 402939903..b76b76100 100644 --- a/lib/src/host/agent-browser-host.test.ts +++ b/lib/src/host/agent-browser-host.test.ts @@ -186,11 +186,18 @@ describe('agent-browser host relaunch', () => { }), }), }); + // Seed the daemon state that pop-out replaces. Wait until headed open has + // started before publishing the successor, so a slow CI runner cannot make + // killDaemon mistake the successor PID for the one it replaced. + const stale = await closedPort(); + writeState(session, 'pid', DEAD_PID); + writeState(session, 'stream', stale); const host = createAgentBrowserHost({ writeClipboardText: vi.fn() }); const popOut = host.popOut(session, { url: 'https://example.com/' }); + await vi.waitFor(() => expect(calls.some((args) => args.includes('--headed'))).toBe(true)); const { port, server } = await listen(); try { - writeState(session, 'pid', DEAD_PID); + writeState(session, 'pid', DEAD_PID + 1); writeState(session, 'stream', port); expect(await popOut).toEqual({ ok: true, wsPort: port }); From 5bf75179d17ec496857ef5233dea710986633632 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 13:51:34 -0700 Subject: [PATCH 10/12] fix(browser): cancel relaunch sweeps on close --- docs/specs/dor-browser.md | 2 +- docs/specs/dor-browser.rationale.md | 2 +- lib/src/host/agent-browser-host.test.ts | 42 +++++++++++++++++++++++++ lib/src/host/agent-browser-host.ts | 13 +++++--- 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 05e4319f3..d859b2565 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -365,7 +365,7 @@ only a launch without a published port fails, including after a zero exit; shutdown before its launch**, so a window whose page never loads is still closed. **Never query the daemon during the close/reopen gap** (rationale), host and controller park/recovery paths alike, so **Dormouse supplies the active-tab URL and the -host trusts it**. Once `open` returns, only the latest relaunch best-effort +host trusts it**. Once `open` returns, only a still-current relaunch best-effort closes stray `about:blank` tabs, **and only while a real page is open**, so it never closes the sole tab (rationale). diff --git a/docs/specs/dor-browser.rationale.md b/docs/specs/dor-browser.rationale.md index d8c198f5e..03dbed1f9 100644 --- a/docs/specs/dor-browser.rationale.md +++ b/docs/specs/dor-browser.rationale.md @@ -60,7 +60,7 @@ The persisted `wsPort` mirror can lag the controller's already-live port after a **Why nothing may query the daemon during the close/reopen gap.** With the old daemon dead and the new one not yet up, a `stream status` or tab query spawns a *competing* daemon at `about:blank` — agent-browser's CLI starts one on demand — and the relaunch then races two daemons for the same session. -A post-open blank-tab sweep can become such a query when a later relaunch starts before the earlier page finishes loading, so the host invalidates the earlier sweep as soon as the later close begins. +A post-open blank-tab sweep can become such a query when a later relaunch or explicit Surface close starts before the earlier page finishes loading, so the host invalidates the sweep as soon as either close begins. **Why the stray-`about:blank` sweep is guarded.** The close/reopen pair can leave an extra blank tab beside the navigated one. Sweeping blanks unconditionally is the obvious fix and is wrong: a session whose only tab is legitimately blank would lose it, leaving the pane with nothing to show. diff --git a/lib/src/host/agent-browser-host.test.ts b/lib/src/host/agent-browser-host.test.ts index b76b76100..b787a856e 100644 --- a/lib/src/host/agent-browser-host.test.ts +++ b/lib/src/host/agent-browser-host.test.ts @@ -214,6 +214,48 @@ describe('agent-browser host relaunch', () => { } }); + it('does not run a relaunch blank-tab sweep after the session is explicitly closed', async () => { + const session = 'dormouse.1.default'; + const opened = deferred(); + const explicitClose = deferred(); + let closeCount = 0; + const calls = mockSpawnByCommand({ + close: () => (++closeCount === 1 ? {} : explicitClose.promise), + '--headed open': () => opened.promise, + tab: () => ({ + stdout: JSON.stringify({ + tabs: [ + { tabId: 'blank', url: 'about:blank', active: false }, + { tabId: 'real', url: 'https://example.com/', active: true }, + ], + }), + }), + }); + const stale = await closedPort(); + writeState(session, 'pid', DEAD_PID); + writeState(session, 'stream', stale); + const host = createAgentBrowserHost({ writeClipboardText: vi.fn() }); + const popOut = host.popOut(session, { url: 'https://example.com/' }); + await vi.waitFor(() => expect(calls.some((args) => args.includes('--headed'))).toBe(true)); + const { port, server } = await listen(); + try { + writeState(session, 'pid', DEAD_PID + 1); + writeState(session, 'stream', port); + expect(await popOut).toEqual({ ok: true, wsPort: port }); + + // Pane kill/render-swap enters command('close') and invalidates the + // relaunch tail synchronously, before the close queues behind open. + void host.command(session, ['close']); + await vi.waitFor(() => expect(closeCount).toBe(2)); + opened.resolve({ code: 1, stderr: 'Operation timed out' }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(calls.some((args) => args.includes('tab'))).toBe(false); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } + }); + it('open() treats a timed-out page load as a live launch, and a launch with no daemon as a failure', async () => { const host = createAgentBrowserHost({ writeClipboardText: vi.fn() }); let session = ''; diff --git a/lib/src/host/agent-browser-host.ts b/lib/src/host/agent-browser-host.ts index 252816357..c993a5dca 100644 --- a/lib/src/host/agent-browser-host.ts +++ b/lib/src/host/agent-browser-host.ts @@ -347,8 +347,8 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser // tab. Best-effort: a failure here must not fail the pop-out/pop-in. async function closeStrayBlankTabs( session: string, + current: () => boolean, binaryPath?: string, - current: () => boolean = () => true, ): Promise { if (!current()) return; const tabs = await listTabs(session, binaryPath); @@ -442,8 +442,13 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser return { exitCode: 1, stdout: '', stderr: `agent-browser get '${args[1] ?? ''}' is not allowed from the webview` }; } // An explicit close (kill / render-swap) tears the session down itself, so - // it's no longer ours to clean up on shutdown. - if (subcommand === 'close') poppedOutSessions.delete(session); + // it's no longer ours to clean up on shutdown. It also invalidates a + // post-open sweep left by a fast-returning relaunch: once closed, no later + // daemon command may recreate this otherwise-untracked session. + if (subcommand === 'close') { + poppedOutSessions.delete(session); + relaunchGenerations.delete(session); + } return runWithBinaryFallback(['--session', session, ...args], binaryPath); } @@ -636,7 +641,7 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser } const current = () => relaunchGenerations.get(session) === generation; void opened - .then(() => closeStrayBlankTabs(session, binaryPath, current)) + .then(() => closeStrayBlankTabs(session, current, binaryPath)) .catch(() => undefined) .finally(() => { if (current()) relaunchGenerations.delete(session); From 3b2413c2db75f65701edac31e2e9831b4c7602a8 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 13:57:37 -0700 Subject: [PATCH 11/12] fix(browser): cancel relaunch sweeps on shutdown --- docs/specs/dor-browser.md | 4 +-- docs/specs/dor-browser.rationale.md | 2 +- lib/src/host/agent-browser-host.test.ts | 44 +++++++++++++++++++++++++ lib/src/host/agent-browser-host.ts | 4 +++ 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index d859b2565..94df1e202 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -371,8 +371,8 @@ never closes the sole tab (rationale). While popped out, Dormouse keeps a stream/CDP observer so URL/header state follows same-tab navigation and a headed window close can auto-revert to -headless. **Hosts must close tracked popped-out sessions on shutdown** to avoid -orphan headed windows. +headless. **Hosts must cancel pending relaunch sweeps before closing popped-out +sessions on shutdown**. Source of truth: `lib/src/components/wall/agent-browser-surface-controller.ts` (pop-out state, CDP observer, auto-revert), `lib/src/host/agent-browser-host.ts` (`popOut`, `popIn`, diff --git a/docs/specs/dor-browser.rationale.md b/docs/specs/dor-browser.rationale.md index 03dbed1f9..dd440ca92 100644 --- a/docs/specs/dor-browser.rationale.md +++ b/docs/specs/dor-browser.rationale.md @@ -60,7 +60,7 @@ The persisted `wsPort` mirror can lag the controller's already-live port after a **Why nothing may query the daemon during the close/reopen gap.** With the old daemon dead and the new one not yet up, a `stream status` or tab query spawns a *competing* daemon at `about:blank` — agent-browser's CLI starts one on demand — and the relaunch then races two daemons for the same session. -A post-open blank-tab sweep can become such a query when a later relaunch or explicit Surface close starts before the earlier page finishes loading, so the host invalidates the sweep as soon as either close begins. +A post-open blank-tab sweep can become such a query when a later relaunch, explicit Surface close, or host shutdown starts before the earlier page finishes loading, so the host invalidates the sweep before any close can release that pending launch. **Why the stray-`about:blank` sweep is guarded.** The close/reopen pair can leave an extra blank tab beside the navigated one. Sweeping blanks unconditionally is the obvious fix and is wrong: a session whose only tab is legitimately blank would lose it, leaving the pane with nothing to show. diff --git a/lib/src/host/agent-browser-host.test.ts b/lib/src/host/agent-browser-host.test.ts index b787a856e..b08a2e191 100644 --- a/lib/src/host/agent-browser-host.test.ts +++ b/lib/src/host/agent-browser-host.test.ts @@ -256,6 +256,50 @@ describe('agent-browser host relaunch', () => { } }); + it('does not run a relaunch blank-tab sweep during host shutdown', async () => { + const session = 'dormouse.1.default'; + const opened = deferred(); + let closeCount = 0; + const calls = mockSpawnByCommand({ + close: () => { + closeCount += 1; + // The shutdown close releases the still-pending headed open. Its + // continuation must already be invalidated before this can happen. + if (closeCount === 2) opened.resolve({ code: 1, stderr: 'Operation timed out' }); + return {}; + }, + '--headed open': () => opened.promise, + tab: () => ({ + stdout: JSON.stringify({ + tabs: [ + { tabId: 'blank', url: 'about:blank', active: false }, + { tabId: 'real', url: 'https://example.com/', active: true }, + ], + }), + }), + }); + const stale = await closedPort(); + writeState(session, 'pid', DEAD_PID); + writeState(session, 'stream', stale); + const host = createAgentBrowserHost({ writeClipboardText: vi.fn() }); + const popOut = host.popOut(session, { url: 'https://example.com/' }); + await vi.waitFor(() => expect(calls.some((args) => args.includes('--headed'))).toBe(true)); + const { port, server } = await listen(); + try { + writeState(session, 'pid', DEAD_PID + 1); + writeState(session, 'stream', port); + expect(await popOut).toEqual({ ok: true, wsPort: port }); + + await host.closePoppedOut(); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(closeCount).toBe(2); + expect(calls.some((args) => args.includes('tab'))).toBe(false); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } + }); + it('open() treats a timed-out page load as a live launch, and a launch with no daemon as a failure', async () => { const host = createAgentBrowserHost({ writeClipboardText: vi.fn() }); let session = ''; diff --git a/lib/src/host/agent-browser-host.ts b/lib/src/host/agent-browser-host.ts index c993a5dca..2d192979f 100644 --- a/lib/src/host/agent-browser-host.ts +++ b/lib/src/host/agent-browser-host.ts @@ -679,6 +679,10 @@ export function createAgentBrowserHost(deps: AgentBrowserHostDeps): AgentBrowser async function closePoppedOut(): Promise { const entries = [...poppedOutSessions.entries()]; poppedOutSessions.clear(); + // Shutdown owns every session now, including a headless pop-in that has + // already left poppedOutSessions. Invalidate all post-open tails before a + // close can release their pending `open` commands and let them query again. + relaunchGenerations.clear(); await Promise.all([ ...entries.map(([session, binaryPath]) => runWithBinaryFallback(['--session', session, 'close'], binaryPath).catch(() => undefined), From a82c709132b2418327607886aac41274ccb39c63 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 3 Sep 2026 14:04:00 -0700 Subject: [PATCH 12/12] docs(browser): preserve shutdown cleanup invariant --- docs/specs/dor-browser.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/specs/dor-browser.md b/docs/specs/dor-browser.md index 94df1e202..7297de3bf 100644 --- a/docs/specs/dor-browser.md +++ b/docs/specs/dor-browser.md @@ -369,10 +369,10 @@ host trusts it**. Once `open` returns, only a still-current relaunch best-effort closes stray `about:blank` tabs, **and only while a real page is open**, so it never closes the sole tab (rationale). -While popped out, Dormouse keeps a stream/CDP observer so URL/header state -follows same-tab navigation and a headed window close can auto-revert to -headless. **Hosts must cancel pending relaunch sweeps before closing popped-out -sessions on shutdown**. +While popped out, Dormouse keeps a stream/CDP observer for same-tab URL/header +updates and headed-window close auto-revert. **Hosts must cancel pending +relaunch sweeps, then close tracked popped-out sessions on shutdown** so +quitting orphans no headed window. Source of truth: `lib/src/components/wall/agent-browser-surface-controller.ts` (pop-out state, CDP observer, auto-revert), `lib/src/host/agent-browser-host.ts` (`popOut`, `popIn`,