refactor(browser): extract surface-scoped agent-browser controller + streaming perf#204
Merged
Conversation
…o them Browser panels use dockview renderer:'always', so a pane hidden behind another tab (or a backgrounded window) stayed mounted with its ~20Hz frame stream, per-pulse screenshot spawns, and canvas draws running for nothing. A new useSurfaceVisibility hook (dockview onDidVisibilityChange + document.visibilitychange) parks such panes after a 1s debounce: the stream connection and screenshot loop are disposed while the daemon and session stay alive, and daemon-side screencasting stops on its own since clients trigger it. Unparking reconnects, keeps the last frame on canvas instead of blanking to the placeholder, and re-primes from the stream's re-broadcast — including state that changed while hidden. Popped-out panes are exempt: their stream observer is what detects a headed-window close and drives auto-revert. Parked panes also never query stream status (no competing-daemon risk, no idle CLI spawns) and never drive the viewport from a hidden rect. Live-verified in the dev:standalone:ab harness: zero stream/screenshot activity while hidden, daemon alive, reconnect picked up a navigation performed while parked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rPanel AgentBrowserPanel owned every piece of non-React lifecycle — stream connection + screenshot loop, stale-port recovery, viewport-sync state machine, pop-out/pop-in orchestration + auto-revert, CDP observer, canonical-URL tracking, input bridging, params persistence, and the screen/chrome registration — coordinated through ~20 refs and effects with StrictMode workarounds. All of it now lives in agent-browser-surface-controller.ts, a module-level registry keyed by surface id that mirrors terminal-lifecycle.ts; the panel is a thin view (1,345 → 446 lines) that mounts a canvas, feeds params/visibility, and subscribes to one snapshot. The controller is surface-scoped: it survives unmount (minimize, layout churn, StrictMode) and is disposed by kill/render-swap in Wall.tsx — the first piece of the per-surface teardown hook the spec's Future Work calls for. Minimize now parks via view-detach (same 1s debounce and zero-resource end state as hidden tabs) instead of synchronously disposing, and a minimized popped-out pane keeps its observer so window-close auto-revert works while minimized. Live-verified in the dev:standalone:ab harness: render, canvas click navigation, park/unpark, minimize/reattach with URL preserved, kill teardown, and fresh session after kill. The dockview "invalid operation" error logged on browser-pane kill was A/B-confirmed pre-existing on the parent commit (identical stack via kill-animation.ts finalize), not a regression from this refactor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-frame main-thread work Standalone transport: the sidecar's agentBrowser:screenshot response carried ~100-700KB of base64 image per frame as a JSON line on the same stdio pipe as all PTY traffic, head-of-line blocking terminal output and paying a full serde parse + base64 round-trip per shot. The shared host now exposes screenshotToFile (capture only); the sidecar returns the temp-file path and Rust reads the bytes itself into the raw ipc Response. VS Code keeps the byte-returning screenshot(); the dev harness bridge reads the path in Node. A stale-bundle bytesBase64 fallback stays in Rust for dev-time version skew. Webview trims, all in the surface controller and screenshot loop: - computeScreenSnapshot/maybeDisengageSync ran getBoundingClientRect on every non-duplicate stream frame (~20Hz forced-layout hazard); they now read a pane-size cache fed by an always-on ResizeObserver. - High-rate [ab-panel]/[agent-browser] console diagnostics sit behind a new dormouse.flags.abDebugLogs localStorage flag (reload to apply); warns stay unconditional and the connection debug ring is untouched. - Byte-identical screenshot captures skip decode+draw (djb2 dedup keyed by a draw-target generation, bumped per attach so a remounted canvas still repaints), and a re-attach with a live connection pulses one capture so a view remounted within the park debounce doesn't sit blank. CDP capture (Page.captureScreenshot over a persistent connection) was prototyped and measured byte-identical at DPR 1 and ~35% faster warm, but returns CSS-resolution frames at DPR>1 unless the client re-applies emulation — only safe while sync-to-pane owns the values. Findings and the daemon-crash gotcha are recorded in the spec's Future Work; the CLI path stays. Live-verified in the dev:standalone:ab harness (sidecar bundle rebuilt): path-over-stdio renders end-to-end, logs correctly gated off/on, no park/unpark or sync-to-pane regressions. cargo check clean; 751 lib + 17 standalone + 49 website tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two confirm-char keydowns processed before a React re-render both passed acceptKill's guard: it reads confirmKillRef.current.exit, but the ref only syncs during the next render. killPaneImmediately then ran twice, and the two orchestrateKill invocations raced one animationend — the first finalize removed the panel, the second called removePanel on the stale reference and threw dockview's unhandled 'invalid operation'. The throw also stranded killInProgressRef at true, permanently skipping the auto-spawn delay. The bug looked browser-pane-specific in testing only because browser panes always go through the typed-confirm flow while fresh terminals kill directly. acceptKill now writes the staged exit into the ref synchronously, so a second keydown in the same task is swallowed. bareRemove re-resolves the panel by id and removes only the exact object it captured — safe against any competing remover (a doubled kill, a dor-CLI kill, a render swap) — and resets killInProgressRef in a finally. Reproduced deterministically (two synchronous confirm keydowns) and re-verified fixed in the dev:standalone:ab harness: 0 errors across 5 kills including the exact repro twice; last-pane auto-spawn intact. Four regression tests pin the fix and fail with the guard reverted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…into one, trim view snapshot Post-extraction cleanup of the surface-scoped controller: - Move the duplicated tabDisplayTitle into browser-url.ts (beside hostPathDisplay, which it calls) and import it in both the panel and the controller. - Merge the sync-only debounced ResizeObserver into the always-on pane-size observer, so there is one observer per pane instead of two. A pending sync debounce is dropped if sync disengages mid-window, matching the old teardown. - Drop session (redundant with params) and parked (tests-only) from the view snapshot; expose parked via isParked(). This also removes a wasted panel re-render on every park/unpark. - Stop feeding the never-read renderMode into updateParams (and drop the stale effect dep that re-ran it on every render-mode flip). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deploying mouseterm with
|
| Latest commit: |
575456c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://83b3bb29.mouseterm.pages.dev |
| Branch Preview URL: | https://ab-dogfood.mouseterm.pages.dev |
dormouse-bot
reviewed
Jul 3, 2026
dormouse-bot
left a comment
dormouse-bot
left a comment
Collaborator
There was a problem hiding this comment.
Read the whole diff and the new controller in full. This is a clean extraction — the controller faithfully mirrors the old panel's state machine (I traced the deleted effects to their imperative equivalents and the guards line up), the tests are thorough, and the screenshot-transport split is consistently threaded through host / sidecar / Rust / dev-bridge with a base64 fallback for version skew.
One observation inline about liveStreamPort surviving a park/unpark cycle — a narrow edge case, not a blocker.
dormouse-bot
approved these changes
Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extracts all the non-React lifecycle of the agent-browser surface out of
AgentBrowserPanel.tsxinto a surface-id-keyed controller registry (agent-browser-surface-controller.ts, mirroringterminal-lifecycle.ts), and lands several streaming performance and robustness fixes on top. The panel is now a thin view: it mounts a canvas, feeds params/visibility, forwards DOM input, and subscribes to one snapshot viauseSyncExternalStore.Commits
orchestrateKillre-resolves the panel by id and wraps the in-progress flag in try/finally so it is idempotent against any competing remover (double confirm, CLI kill, render swap)./simplifycleanup pass: one sharedtabDisplayTitle, a single pane observer instead of two, and a leaner view snapshot (drops a wasted re-render per park).Testing
tsc -bcleanvitest run— 715/715 lib tests pass (includes new controller, screenshot-loop, and kill-animation suites)🤖 Generated with Claude Code