Repository navigation
fix(remote-browser): a client-hosted tab survives a network outage instead of staying unavailable - #25799
Closed
Jinwoo-H wants to merge 12 commits into
Closed
fix(remote-browser): a client-hosted tab survives a network outage instead of staying unavailable#25799Jinwoo-H wants to merge 12 commits into
Jinwoo-H wants to merge 12 commits into
Conversation
…ailable after a network outage A paired desktop that loses its host for longer than the browser-host lease grace (25s reset cut, or a 100s silent cut the host's heartbeat reaps) tears down its client-hosted pages and never hosts them again, leaving 'Client-hosted browser unavailable'. Red on main. Extends the freezable TCP proxy with a full silent or reset link cut.
…gh a fenced lease When a paired desktop is unreachable longer than the browser-host lease grace, the host fences its lease but keeps the page records. Until now a desktop coming back under a new lease had no way to hand the still-live pages back: the host saw records that matched the reported inventory, treated them as healthy, and left them placed nowhere. Commands then failed with browser_client_page_placement_required and the page could not load anything. A desktop can now ask for this with a new optional attach field, returningHostReclaimProtocolVersion. A host that echoes it, on attach: - rekeys each kept guest whose record still names the fenced placement (reclaimPage), so the page keeps its DOM and moves to the new lease; - closes a kept guest whose page it released meanwhile (closePage), so its answer decides what is still this desktop's; - restores at the last URL any kept guest the desktop could not vouch for. Reclaim intents take generations above what the runtime already issued. The reclaim command and planner now accept an earlier lease of the same epoch as "older", which is only ever sent to desktops that negotiated it. Older desktops never send the field, so nothing changes for them.
…e-attach on real events A paired desktop that lost its host for longer than the 15s lease reconnect grace treated the expired grace as final: it closed the composition, destroyed every live page and dropped the registry record, and nothing ever registered again. The tab sat on "Client-hosted browser unavailable" for good. Losing contact is not evidence the pages are gone, so the composition now parks instead. BrowserClientHostParking is the one state machine: - the lease marks reconnect failures as lost contact; that, a replaced runtime, and a route that stayed dark all park rather than tear down; - parked keeps the executor and its guests, keeps routes suspended (page traffic fails instead of leaving from this desktop) and closes the dead lease; - a real event asks for exactly one re-attach, shared by concurrent asks: the environment's control connection becoming ready again, the existing online/wake retry IPC, and the user opening the tab; - the re-attach reuses the runtime-replacement path: fresh lease, same executor, inventory sent so the host rekeys the same guests, and each reclaimed guest moves its partition onto the new lease's route; - a host that does not echo returningHostReclaimProtocolVersion gets the guests freed first, so its existing recovery reloads them at their last URL; - after an hour parked the guests are freed to bound memory, keeping the tabs; this timer decides nothing about whether a page still exists; - unpair, re-pair and quit still close the composition. This deletes the grace-expiry teardown and the 45s authority-replacement wait, which existed only to bound how long a dead lease kept guests.
…d of "another desktop" While its host was unreachable a client-hosted tab either showed nothing or, once the guest was gone, "attached to a different desktop or is no longer available" with only a reopen-on-server escape. Now, while the environment's connection is down, a live page gets a "<host> is offline — reconnecting" strip with Reconnect now, and a page whose guest is gone gets the same message as a notice rather than an error. Opening the tab, or clicking Reconnect now, asks main for one re-attach. "Open on a different desktop" is only said when the page's placement names another desktop's host id; otherwise the copy says the page is no longer available on this desktop.
…e and comes back Strengthens the outage repro from "a page came back" to "the same page came back": a token stamped into the live document must read back after the host returns, the placement must be new, nothing may say "unavailable", and the page must load again. Adds the remaining journeys against a real paired host: - a 5s outage keeps today's lease generation and page; - while parked, a fetch from the page fails and the fixture server sees no request from any other route; - a host that quits and stays down past the grace gets the tab back on relaunch, showing "is offline" meanwhile; - a host that does not negotiate the reclaim (ORCA_E2E_DISABLE_RETURNING_ HOST_RECLAIM) gets the page back by reloading it; - a long absence (ORCA_E2E_BROWSER_CLIENT_HOST_PARKED_DISCARD_MS) frees the guest but keeps the tab, which reloads when the host returns; - a page closed on the host while the desktop was away is closed on the desktop when it returns.
…econnect paths The first cut added its own trigger wiring: a shared-control "ready" listener module, a dedicated resume IPC with its own preload method, and a registry park-by-environment entry point. Each duplicated a path that already exists, so they are gone: - the host coming back reaches the renderer's existing same-runtime "contact regained" edge, which now runs the restarted-runtime hook (renamed ensureBrowserClientHostForReturningRuntime); its prepare IPC reaches the registry, which resumes a parked composition; - online, wake, opening the tab and Reconnect now all use the existing retryConnectionsNow IPC, which now also resumes parked hosts, skipping environments the user disconnected by hand; - a route that stays dark parks its own composition directly.
…clear-answer rule - A desktop asks for returning-host reclaim only alongside page reconciliation; a host echoes it only to a desktop that asked, so an older desktop sees exactly the ready it always did. - A host that still holds the page under a placement the desktop does not report leaves the kept guest alone; only a page the host no longer has at all is closed.
Contributor
📝 WalkthroughWalkthroughThe change adds a negotiated protocol for reclaiming pages when a browser host returns. Browser-host compositions can park during contact loss, retain guests, and resume; page reclaim rebinds network routes and updates page placement. The renderer now reports host availability and can request reattachment. New end-to-end tests cover network outages, host restarts, reclaim compatibility, guest disposal, and remote tab closure. Priority: ➖ Normal Merge Risk: 🔵 Low · up to The change is mergeable with bounded follow-up: an outage-test cleanup failure may leave a host process running, and users of non-English locales will see two recovery messages in English. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The recovery design retains important identity and ownership checks. The main concern is failure containment: changing a shared browser proxy can partially succeed, while recovery failure closes only the affected page rather than fencing the shared routing state. No unauthorized page takeover or direct-network leak was established, but this transition needs a stronger failure guarantee. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
efce48a2-4fdc-4437-a50e-df0b471ab2fe
📒 Files selected for processing (61)
src/main/browser/browser-client-host-attach-request.test.tssrc/main/browser/browser-client-host-attach-request.tssrc/main/browser/browser-client-host-authority-replacement-wait.tssrc/main/browser/browser-client-host-authority-replacement.test.tssrc/main/browser/browser-client-host-authority-replacement.tssrc/main/browser/browser-client-host-parking.test.tssrc/main/browser/browser-client-host-parking.tssrc/main/browser/browser-client-page-command-executor-dependencies.tssrc/main/browser/browser-client-page-command-executor-test-harness.tssrc/main/browser/browser-client-page-command-executor.tssrc/main/browser/browser-client-page-command-integration.test.tssrc/main/browser/browser-client-page-reconciliation-adapters.test.tssrc/main/browser/browser-client-page-reconciliation.tssrc/main/browser/browser-client-page-unavailability.test.tssrc/main/browser/browser-host-lease-contact-loss.tssrc/main/browser/browser-route-partition-storage-retirement.test.tssrc/main/browser/browser-route-session-policy.tssrc/main/browser/browser-route-session-registry.tssrc/main/browser/paired-runtime-browser-client-host-composition-contract.tssrc/main/browser/paired-runtime-browser-client-host-composition.test.tssrc/main/browser/paired-runtime-browser-client-host-composition.tssrc/main/browser/paired-runtime-browser-client-host-registry.test.tssrc/main/browser/paired-runtime-browser-client-host-registry.tssrc/main/browser/paired-runtime-browser-client-host-runtime.tssrc/main/browser/paired-runtime-browser-client-host.tssrc/main/browser/paired-runtime-browser-host-lease-connection.tssrc/main/browser/paired-runtime-browser-host-lease-options.tssrc/main/browser/paired-runtime-browser-host-lease-reconnect.test.tssrc/main/browser/paired-runtime-browser-host-lease.tssrc/main/ipc/runtime-environment-recovery-handler.tssrc/main/runtime/browser-host-client-page-adoption.tssrc/main/runtime/browser-host-lease-registry.tssrc/main/runtime/browser-host-page-placement.tssrc/main/runtime/browser-host-page-reconciliation-orchestration.tssrc/main/runtime/browser-host-page-reconciliation-plan.tssrc/main/runtime/browser-host-returning-client-pages.tssrc/main/runtime/rpc/methods/browser-client-host.test.tssrc/main/runtime/rpc/methods/browser-client-host.tssrc/main/runtime/runtime-browser-client-page-adoption.tssrc/main/runtime/runtime-browser-client-page-restored-recovery.test.tssrc/main/runtime/runtime-browser-client-page-returning-host.test.tssrc/renderer/src/components/browser-pane/ClientHostedBrowserPagePane.tsxsrc/renderer/src/components/browser-pane/client-hosted-browser-unavailable-notice.test.tsxsrc/renderer/src/components/browser-pane/client-hosted-browser-unavailable-notice.tsxsrc/renderer/src/components/browser-pane/use-client-hosted-browser-host-reachability.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/fr.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/runtime/restored-client-hosted-browser-host-attach.tssrc/renderer/src/runtime/restored-client-hosted-browser-host-restart-attach.test.tssrc/renderer/src/store/slices/runtime-status-snapshot.test.tssrc/renderer/src/store/slices/runtime-status.tssrc/shared/browser-client-host-protocol.tssrc/shared/browser-client-host-reconciliation-protocol.test.tstests/e2e/helpers/client-hosted-browser-fixture.tstests/e2e/helpers/freezable-tcp-proxy.tstests/e2e/helpers/headless-paired-runtime-host.tstests/e2e/paired-client-hosted-browser-network-outage.spec.ts
💤 Files with no reviewable changes (6)
- src/renderer/src/i18n/locales/es.json
- src/renderer/src/i18n/locales/fr.json
- src/renderer/src/i18n/locales/ja.json
- src/renderer/src/i18n/locales/ko.json
- src/renderer/src/i18n/locales/zh.json
- src/main/browser/browser-client-host-authority-replacement-wait.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| this.reconnectPromise = reconnecting | ||
| void reconnecting | ||
| .catch((reconnectError) => this.failTerminal(asError(reconnectError))) | ||
| .catch((reconnectError) => this.failTerminal(browserHostLeaseContactLost(reconnectError))) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
fd -t f 'reconnect' src/main/browser --exec rg -n -C6 'reconnectBrowserHostLeaseUntil|canReconnect|throw' {}
rg -nP -C4 'function isRecoverableBrowserHostLeaseError' src/main/browserRepository: stablyai/orca
Length of output: 3621
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- lease host ---'
nl -ba src/main/browser/paired-runtime-browser-host-lease.ts | sed -n '1,250p'
printf '%s\n' '--- reconnect helper and delay ---'
rg -n -l 'function reconnectBrowserHostLeaseUntil|function isRecoverableBrowserHostLeaseError|class BrowserHostLeaseContactLostError|browserHostLeaseContactLost' src/main/browser
nl -ba src/main/browser/browser-host-lease-reconnect.ts | sed -n '1,130p' 2>/dev/null || true
printf '%s\n' '--- recoverability classifier ---'
nl -ba src/main/browser/browser-host-admission-recovery.ts | sed -n '1,115p'
printf '%s\n' '--- consumers ---'
rg -n -C5 'isBrowserHostLeaseContactLost|BrowserHostLeaseContactLostError|browserHostLeaseContactLost' src
printf '%s\n' '--- PR diff ---'
git diff --no-ext-diff --unified=8 4e64fa9940f5a4a9bfb7b021a3c405fd76643ca5 0ca9b02c1c8226cc4eb31039cf91d43f89d60499 -- src/main/browser/paired-runtime-browser-host-lease.ts
printf '%s\n' '--- reconnect declarations/callers ---'
rg -n -C8 'reconnectBrowserHostLeaseUntil\\(' src/main/browserRepository: stablyai/orca
Length of output: 28191
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- reconnect helper ---'
nl -ba src/main/browser/browser-host-lease-reconnect-attempts.ts
printf '%s\n' '--- remote error classification ---'
rg -n -C5 'isRecoverableRemoteRuntimeConnectionError|runtime_timeout' src/shared/remote-runtime-client-error-classification.ts src/shared/remote-runtime-client-error.ts
printf '%s\n' '--- composition failure branch ---'
nl -ba src/main/browser/paired-runtime-browser-client-host-composition.ts | sed -n '170,205p'
printf '%s\n' '--- connection errors and authority ---'
rg -n -C4 'expectedAuthority|authority|onFailure|RemoteRuntimeClientError|throw new' src/main/browser/paired-runtime-browser-host-lease-connection.tsRepository: stablyai/orca
Length of output: 10749
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- authority-change producer and classifiers ---'
rg -n -C5 'Browser host lease authority changed in place|isBrowserClientHostAuthorityReplaced|BrowserClientHostAuthorityReplaced' src/main/browser src/shared
printf '%s\n' '--- contact-loss contract ---'
nl -ba src/main/browser/browser-host-lease-contact-loss.ts
printf '%s\n' '--- local error normalization ---'
rg -n -C2 '^function asError|^const asError' src/main/browser/paired-runtime-browser-host-lease.tsRepository: stablyai/orca
Length of output: 15891
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- composition host error and teardown paths ---'
rg -n 'handleHostError|park\\(|close\\(' src/main/browser/paired-runtime-browser-client-host-composition.ts
nl -ba src/main/browser/paired-runtime-browser-client-host-composition.ts | sed -n '200,340p'
printf '%s\n' '--- relevant composition teardown implementation ---'
rg -n -C4 'function closeBrowserClientHostComposition|export .*closeBrowserClientHostComposition|handleHostError' src/main/browserRepository: stablyai/orca
Length of output: 318
🏁 Script executed:
#!/bin/bash
set -e
file=src/main/browser/paired-runtime-browser-client-host-composition.ts
rg -n -F 'handleHostError' "$file"
nl -ba "$file" | sed -n '200,340p'
printf '%s\n' '--- teardown implementation ---'
rg -n -F 'closeBrowserClientHostComposition' src/main/browserRepository: stablyai/orca
Length of output: 4667
Wrap only reconnect expiry and an authority change as lost contact.
When a remote attach response carries a nonrecoverable error, canReconnect is false and the reconnect helper throws it. This catch marks that rejection as contact loss, so the composition parks instead of closing on the definitive error. Preserve parking for grace-period expiry and authority changes. Give the locally detected authority change an explicit type so other nonrecoverable errors can pass through unchanged.
🐛 Suggested fix
--- a/src/main/browser/paired-runtime-browser-host-lease.ts
+++ b/src/main/browser/paired-runtime-browser-host-lease.ts
@@
-import { PairedRuntimeBrowserHostLeaseConnection } from './paired-runtime-browser-host-lease-connection'
+import {
+ BrowserHostLeaseAuthorityChangedError,
+ PairedRuntimeBrowserHostLeaseConnection
+} from './paired-runtime-browser-host-lease-connection'
@@
- .catch((reconnectError) => this.failTerminal(browserHostLeaseContactLost(reconnectError)))
+ .catch((reconnectError) => {
+ const failure = asError(reconnectError)
+ this.failTerminal(
+ isRecoverableBrowserHostLeaseError(failure) ||
+ failure instanceof BrowserHostLeaseAuthorityChangedError
+ ? browserHostLeaseContactLost(failure)
+ : failure
+ )
+ })--- a/src/main/browser/paired-runtime-browser-host-lease-connection.ts
+++ b/src/main/browser/paired-runtime-browser-host-lease-connection.ts
@@
-export class PairedRuntimeBrowserHostLeaseConnection {
+export class BrowserHostLeaseAuthorityChangedError extends Error {
+ constructor() {
+ super('Browser host lease authority changed in place')
+ this.name = 'BrowserHostLeaseAuthorityChangedError'
+ }
+}
+
+export class PairedRuntimeBrowserHostLeaseConnection {
@@
- this.fail(new Error('Browser host lease authority changed in place'))
+ this.fail(new BrowserHostLeaseAuthorityChangedError())
Comment on lines
+102
to
+104
| "hostRow": "Hosted on another device", | ||
| "hostOfflineDescription": "Reconnecting. This page comes back when the host is reachable again.", | ||
| "reconnectNow": "Reconnect now", |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Translate the new offline and reconnect strings.
hostRow, hostOfflineDescription, and reconnectNow are English-only. Users of other supported locales will see untranslated text in the recovery flow. Add these keys to the supported locale catalogs. The PR objectives identify these strings as English-only.
Comment on lines
+98
to
+106
| } finally { | ||
| if (client) { | ||
| await cleanupE2EDaemons(client.userDataDir).catch(() => undefined) | ||
| await client.dispose() | ||
| } | ||
| await proxy?.close() | ||
| await host.dispose() | ||
| await fixture.close() | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Make teardown run every step when one step fails.
The finally block awaits client.dispose(), proxy?.close(), host.dispose(), and fixture.close() one after another. If client.dispose() or proxy.close() throws, the remaining steps do not run. The headless host Electron process and its user-data directory then stay behind and affect later tests. The thrown cleanup error also hides the original assertion failure. Run each step on its own, for example with cleanupHeadlessHostResources-style aggregation or Promise.allSettled in order.
Proposed fix
} finally {
- if (client) {
- await cleanupE2EDaemons(client.userDataDir).catch(() => undefined)
- await client.dispose()
- }
- await proxy?.close()
- await host.dispose()
- await fixture.close()
+ const failures: unknown[] = []
+ const steps: (() => Promise<unknown> | undefined)[] = [
+ () => (client ? cleanupE2EDaemons(client.userDataDir).catch(() => undefined) : undefined),
+ () => client?.dispose(),
+ () => proxy?.close(),
+ () => host.dispose(),
+ () => fixture.close()
+ ]
+ for (const step of steps) {
+ try {
+ await step()
+ } catch (error) {
+ failures.push(error)
+ }
+ }
+ if (failures.length > 0) {
+ throw new AggregateError(failures, 'outage rig cleanup failed')
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } finally { | |
| if (client) { | |
| await cleanupE2EDaemons(client.userDataDir).catch(() => undefined) | |
| await client.dispose() | |
| } | |
| await proxy?.close() | |
| await host.dispose() | |
| await fixture.close() | |
| } | |
| } finally { | |
| const failures: unknown[] = [] | |
| const steps: (() => Promise<unknown> | undefined)[] = [ | |
| () => (client ? cleanupE2EDaemons(client.userDataDir).catch(() => undefined) : undefined), | |
| () => client?.dispose(), | |
| () => proxy?.close(), | |
| () => host.dispose(), | |
| () => fixture.close() | |
| ] | |
| for (const step of steps) { | |
| try { | |
| await step() | |
| } catch (error) { | |
| failures.push(error) | |
| } | |
| } | |
| if (failures.length > 0) { | |
| throw new AggregateError(failures, 'outage rig cleanup failed') | |
| } | |
| } |
… re-attach per environment One state machine: park fences routes and closes the lease; resume re-attaches a parked composition and otherwise answers with the live lease. A refusal now closes instead of parking; only lost contact, a replaced runtime or no answer park. Guests are freed for an older runtime only after its attach shows it cannot rekey them, and the discard timer pauses during an attempt. Tab activation and Reconnect go through the per-environment prepare path, a dark route parks and immediately asks for one resume, and the global resume on retryConnectionsNow is gone. releasedPageIds moves into the adoption options.
…s own refusal Unknown, local and transport errors now park instead of closing, so an unclassified failure is never read as the pages being gone. The lease marks the RPC failures the host actually sent; only a marked, non-recoverable, non-capacity answer (or the reconciliation-unsupported verdict read from the host's ready answer) closes the composition.
… type, not a side table BrowserHostAnswerError subclasses RemoteRuntimeClientError, so every existing code/instanceof consumer is unchanged; parkOrFail checks the type directly.
9 tasks done
- Free only the guest generations published in the attach inventory, so a page the older runtime's recovery already placed survives the release. - A dark route swaps the lease through one queued registry re-attach instead of parking and kicking a resume. - Main publishes each environment's parked state; the offline strip shows whenever the browser host is parked, even with the control link up. - Tab activation and Reconnect use a thin resume IPC instead of a status round-trip through placement preparation. - One unpark() clears the discard timer, and a re-attach superseded by a new park during its await does not unpark. - isFinalBrowserHostRefusal is the one predicate; reconciliation_unsupported is built by browserHostRefusal(); lastAuthority becomes attached.
…back Drops the older-runtime fallback that freed guests after attach and asked the runtime to reload them. Whether a runtime can rekey kept guests is known from the lease's own ready answer, so a runtime without that capability fails lost contact and a dark route exactly as before; only a replaced runtime is still waited for. The e2e older-host case now pins that nothing is kept.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/e2e/paired-client-hosted-browser-network-outage.spec.ts (1)
311-316: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCheck the guest count before restoring the proxy.
readGuestStampreturnsnullfor a closed guest and for a reloaded document withoutwindow.__outageToken. The current sequence restores the proxy before asserting, so it does not establish that the guest was absent during the outage. PollcountGuests(rig)while the proxy is still cut.🐛 Suggested fix
rig.proxy!.cut('reset') await new Promise((resolve) => setTimeout(resolve, OUTAGE_MS.reset)) + await expect + .poll(() => countGuests(rig), { + timeout: 90_000, + message: 'the older host must close the guest during the outage' + }) + .toBe(0) rig.proxy!.restore()
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
20ea3506-9765-42c0-9bad-30544d0d3336
📒 Files selected for processing (20)
src/main/browser/browser-client-host-parked-publication.tssrc/main/browser/browser-host-admission-recovery.test.tssrc/main/browser/browser-host-admission-recovery.tssrc/main/browser/paired-runtime-browser-client-host-composition-contract.tssrc/main/browser/paired-runtime-browser-client-host-composition.test.tssrc/main/browser/paired-runtime-browser-client-host-composition.tssrc/main/browser/paired-runtime-browser-client-host-registry.test.tssrc/main/browser/paired-runtime-browser-client-host-registry.tssrc/main/browser/paired-runtime-browser-client-host-runtime.tssrc/main/browser/paired-runtime-browser-client-host-teardown.tssrc/main/ipc/runtime-environment-browser-client-host-handler.tssrc/main/ipc/runtime-environment-handler-channels.tssrc/main/ipc/runtime-environments-pairing.test.tssrc/preload/api/runtime-api.tssrc/preload/api/runtime-environments-bridge.tssrc/renderer/src/components/browser-pane/client-hosted-browser-unavailable-notice.test.tsxsrc/renderer/src/components/browser-pane/use-client-hosted-browser-host-reachability.tssrc/renderer/src/runtime/browser-client-host-parked-environments.tssrc/shared/browser-client-host-parked.tstests/e2e/paired-client-hosted-browser-network-outage.spec.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
Jinwoo-H
marked this pull request as draft
October 6, 2026 07:23
6 of 9 tasks
Contributor
Author
|
Superseded by #25827. We chose the simpler fix: re-host the pages when contact returns, so they reload at their last address. This approach (keeping live pages through the outage) needed a protocol change and about 1,000 more lines, and kept hitting timing races in review. The branch stays for reference. |
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
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.
ELI5
A remote browser tab in a paired workspace is drawn on your desktop, and the remote Orca lets the desktop host that page. When the network went away for longer than about 15 seconds (Wi‑Fi drop, laptop sleep), the desktop threw the page away and never tried again. The tab said "Client-hosted browser unavailable … attached to a different desktop" forever, even after the network came back. Now the desktop keeps the page through the outage and takes it back when the remote is reachable again, so the page is exactly as you left it.
What Changed
Before: the desktop retried for 15 seconds, then closed every client-hosted page and dropped its registration with that remote. Nothing ever registered again, so the tab stayed broken until it was reopened. The remote, meanwhile, kept the page on file under the old, voided registration and had no way to give it back to the same desktop.
After:
parkedfield) instead of tearing down. It keeps the live pages and its intent to host, with their network routes suspended. It re-attaches only on real events, one attempt at a time through the existing per-remote queue, with no polling: contact with the remote regained (which also covers coming back online or waking), opening the tab, or Reconnect now. It closes only when the remote actually refuses (a host-sent error, typedBrowserHostAnswerError). Timeouts, dropped connections and unknown errors keep it parked. After an hour parked, the live pages are freed to bound memory, but the tabs stay and reload when the remote returns. Unpair, re-pair and quit still close everything.returningHostReclaimProtocolVersion, is echoed inready. When it's negotiated, the remote takes back pages the same desktop kept through a voided registration, using the existing in-placereclaimPagepath, so there's no reload. The remote closes a kept page only when it has a clear answer that the page is gone: no record of that page, and the inventory names this runtime, this desktop and an older registration.Why
The root cause belongs to a family of bugs where losing contact with the remote is treated as a final answer ("closed", "gone"). The rule we follow (docs/reference/ssh-execution-boundary.md, and #25736 for terminals) is that no answer means wait and re-check; only a real answer from the remote closes something.
Alternatives considered:
Compatibility:
Linked Issue
N/A (internal report).
Visual Proof
To follow in a comment (before: the "unavailable" notice after a 25s outage; after: the same page still live, and the offline strip while disconnected).
Testing
E2E,
tests/e2e/paired-client-hosted-browser-network-outage.spec.ts: a real paired host and desktop with isolated profiles, and a TCP proxy that cuts the link silently (100s) or by reset (25s).The other client-hosted browser specs pass, 11/11.
Unit tests cover the parking state machine, both version skews, and the close-only-on-a-clear-answer rule.
pnpm tc, oxlint,check:code-quality:changedand the localization checks pass.Platforms: macOS desktop and a headless macOS remote. Windows host not exercised.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Known gaps:
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)