Files
orca/src
Neil 264c9ed8d2 fix(browser-pane): stop a dead client-hosted guest from killing the workbench (#18334)
* fix(browser): stop a dead client-hosted guest taking down the workbench

ClientHostedBrowserPagePane called raw <webview> methods from two React effects, so a
guest that is gone throws out of a commit phase and unwinds the terminal.workbench
error boundary instead of showing the pane's own unavailable notice.

Two runtime conditions, two guards:
- Guest destroyed in main while the tag is still in the DOM: the tag keeps its
  guestInstanceId, so every read throws 'Invalid guestInstanceId'. The metadata read is
  now total and the attach effect degrades to browser_client_page_guest_unavailable.
- Retained tag removed from the DOM while the pane stays mounted: contentWindow is null,
  so focus() throws a TypeError. The activation-focus hook now goes through the
  BrowserPageGuestFocus wrapper the pane already builds, which has carried that guard
  since STA-3448.

Follow-up, not in this change: the registry's liveness check compares
readBrowserClientPageAttachedGuestId(webview) to page.webContentsId, which still matches
after main destroys the guest, so a stale 'attached' page can linger.

* fix(browser): keep the dead-guest degrade honest — no spinner, no silent swallow

Review remediation for the guest guards.

- The attach bail now writes `loading: false` before setting
  browser_client_page_guest_unavailable, matching what retryGuestRecoveryRef already does on
  the pane's own route into that state. A page that died mid-load carries `loading: true` in
  the store, so without it the unavailable notice rendered beside a spinner nothing would
  ever stop. Uses the existing updatePageStateFromGuest effect event, not a new setter.
- The dead-guest condition is no longer silent: the metadata read logs the caught error under
  the subsystem's `[browser-client-page]` warn convention, and the attach bail records a
  `browser_client_page_guest_unavailable` crash breadcrumb via the existing
  recordRendererCrashBreadcrumb. Renderer diagnostics only capture window error/rejection, so
  the breadcrumb is what puts this on a channel crash reports actually carry — the registry
  liveness defect this change deliberately does not fix stays measurable, and a read failure
  that is not `Invalid guestInstanceId` is no longer indistinguishable from a dead guest.
- onFailLoad no longer pays five sync IPCs per discarded event: resolveBrowserWebviewLoadFailure
  accepts a lazy fallbackUrl and resolves it after the subframe/ERR_ABORTED filter. Covered by
  a new case in browser-webview-load-failure.test.ts.

Correction to the previous commit's narrative: only the metadata half reaches
browser_client_page_guest_unavailable. The focus half reaches no state at all — the guarded
BrowserPageGuestFocus wrapper returns false and the pane stays mounted over a webview the
registry already removed from the DOM, with no notice and no reopen-on-server escape. It stops
the crash; it does not diagnose the page.

Not changed, with reasons:
- The two sibling attach bails (renderer-unavailable, attach threw) omit the same `loading`
  write. That predates this branch and neither is reached by the dead-guest path; fixing them
  is a separate change.
- recordHistoryFromGuest still passes a raw `webview.getTitle()`. Substituting `metadata.title`
  is not behaviour-preserving: metadata.title falls back to the URL, so an untitled page would
  be filed in history under its URL instead of "New Tab". The call runs only after a five-read
  succeeded and sits in a DOM event listener, which cannot unwind a React commit.

* fix(browser): route every client-hosted guest death to the unavailable notice

A dead guest could still leave the pane mute (retained tag fenced on
render-process-gone/destroyed with no signal to the pane), spinning forever
(did-start-loading read bailing after loading:true), or frozen at stale chrome
(navigation reads bailing silently). All of those now go through one watcher
that detaches, releases the webview ref and enters the pane's existing
browser_client_page_guest_unavailable recovery state, so the user always sees
the notice with its reopen-on-server escape.

The total catch in the guest reader now records a
browser_client_page_guest_read_failed breadcrumb with the error name/message,
so a swallowed failure that is not guest death stays distinguishable in
diagnostics; the guest_unavailable breadcrumb carries the loss reason.

* fix(browser): finish dead guest cleanup and guard history reads
2026-09-04 15:38:00 -07:00
..