From d2eb3c57078cbaf6d890c8f7de0aee65c9fb5fab Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 18 Sep 2026 21:14:58 -0400 Subject: [PATCH] fix(remote): reuse verified browser placement compatibility --- ...-client-host-placement-preparation.test.ts | 20 ++++++++++++++++ ...owser-client-host-placement-preparation.ts | 3 +-- ...onment-browser-client-host-handler.test.ts | 24 +++++++++---------- ...environment-browser-client-host-handler.ts | 9 +++---- ...untime-environment-status-recovery.test.ts | 23 ++++++++++++++++++ .../runtime-environment-transport-routing.ts | 15 ++++++++++++ .../runtime/web-runtime-browser-creation.ts | 10 +++----- 7 files changed, 78 insertions(+), 26 deletions(-) diff --git a/src/main/browser/browser-client-host-placement-preparation.test.ts b/src/main/browser/browser-client-host-placement-preparation.test.ts index 89b17b65b72..97b674ae410 100644 --- a/src/main/browser/browser-client-host-placement-preparation.test.ts +++ b/src/main/browser/browser-client-host-placement-preparation.test.ts @@ -130,6 +130,26 @@ describe('browser client host placement preparation', () => { expect(harness.startHost).not.toHaveBeenCalled() expect(harness.closeHost).not.toHaveBeenCalled() }) + + it('recovers client placement on the next create after a transient status failure', async () => { + const harness = createHarness() + harness.getStatus + .mockResolvedValueOnce({ + id: 'status.get', + ok: false, + error: { code: 'runtime_unavailable', message: 'socket reconnecting' }, + _meta: { runtimeId: 'runtime-a' } + }) + .mockResolvedValueOnce(runtimeStatus()) + + await expect(harness.prepare()).resolves.toEqual({ kind: 'server' }) + await expect(harness.prepare()).resolves.toEqual({ + kind: 'client', + browserHostClientId: 'browser-client-a' + }) + expect(harness.getStatus).toHaveBeenCalledTimes(2) + expect(harness.startHost).toHaveBeenCalledTimes(1) + }) }) function createHarness(options?: { diff --git a/src/main/browser/browser-client-host-placement-preparation.ts b/src/main/browser/browser-client-host-placement-preparation.ts index b92aa9564f2..0d55eac663c 100644 --- a/src/main/browser/browser-client-host-placement-preparation.ts +++ b/src/main/browser/browser-client-host-placement-preparation.ts @@ -39,8 +39,7 @@ export async function prepareBrowserClientHostPlacement( const response = await options.getStatus(initialEnvironment.id) if (!response.ok) { // Why server instead of a throw: an unanswered probe never told us whether this host can - // client-host, and every create probes now that the renderer no longer gates on cached - // capabilities — so rethrowing would turn one flaky `status.get` (its own fresh socket, 15s + // client-host, so rethrowing would turn one flaky `status.get` (its own fresh socket, 15s // ceiling) into a failed create that server placement completes. The tabCreate right behind // this rides the same link and reports a genuinely dead connection itself. return SERVER_PLACEMENT diff --git a/src/main/ipc/runtime-environment-browser-client-host-handler.test.ts b/src/main/ipc/runtime-environment-browser-client-host-handler.test.ts index 6819d8e87a2..1175aa03801 100644 --- a/src/main/ipc/runtime-environment-browser-client-host-handler.test.ts +++ b/src/main/ipc/runtime-environment-browser-client-host-handler.test.ts @@ -11,14 +11,14 @@ import type { KnownRuntimeEnvironment } from '../../shared/runtime-environments' const { handleMock, resolveEnvironmentMock, - getRuntimeEnvironmentStatusMock, + getRuntimeEnvironmentStatusForBrowserPlacementMock, startHostMock, closeHostMock, manuallyDisconnectedMock } = vi.hoisted(() => ({ handleMock: vi.fn(), resolveEnvironmentMock: vi.fn(), - getRuntimeEnvironmentStatusMock: vi.fn(), + getRuntimeEnvironmentStatusForBrowserPlacementMock: vi.fn(), startHostMock: vi.fn(), closeHostMock: vi.fn(), manuallyDisconnectedMock: vi.fn() @@ -29,7 +29,7 @@ vi.mock('../../shared/runtime-environment-store', () => ({ resolveEnvironment: resolveEnvironmentMock })) vi.mock('./runtime-environment-transport-routing', () => ({ - getRuntimeEnvironmentStatus: getRuntimeEnvironmentStatusMock + getRuntimeEnvironmentStatusForBrowserPlacement: getRuntimeEnvironmentStatusForBrowserPlacementMock })) vi.mock('../browser/paired-runtime-browser-client-host-runtime', () => ({ startPairedRuntimeBrowserClientHost: startHostMock, @@ -49,8 +49,8 @@ describe('runtime environment browser client host handler', () => { handleMock.mockReset() resolveEnvironmentMock.mockReset() resolveEnvironmentMock.mockReturnValue(environment()) - getRuntimeEnvironmentStatusMock.mockReset() - getRuntimeEnvironmentStatusMock.mockResolvedValue({ + getRuntimeEnvironmentStatusForBrowserPlacementMock.mockReset() + getRuntimeEnvironmentStatusForBrowserPlacementMock.mockResolvedValue({ id: 'status.get', ok: true, result: { @@ -94,11 +94,9 @@ describe('runtime environment browser client host handler', () => { await expect( prepare(null, { selector: 'environment-a', expectedPairingRevision: 7 }) ).resolves.toEqual({ kind: 'client', browserHostClientId: 'browser-client-a' }) - expect(getRuntimeEnvironmentStatusMock).toHaveBeenCalledWith( + expect(getRuntimeEnvironmentStatusForBrowserPlacementMock).toHaveBeenCalledWith( '/profile', - 'environment-a', - undefined, - { observeOnly: true } + 'environment-a' ) expect(startHostMock).toHaveBeenCalledWith({ environment: expect.objectContaining({ id: 'environment-a', pairingRevision: 7 }), @@ -114,7 +112,7 @@ describe('runtime environment browser client host handler', () => { ) await expect(prepare(null, { selector: 'environment-a' })).resolves.toEqual({ kind: 'server' }) - expect(getRuntimeEnvironmentStatusMock).not.toHaveBeenCalled() + expect(getRuntimeEnvironmentStatusForBrowserPlacementMock).not.toHaveBeenCalled() expect(startHostMock).not.toHaveBeenCalled() expect(closeHostMock).not.toHaveBeenCalled() }) @@ -142,7 +140,7 @@ describe('runtime environment browser client host handler', () => { }) it('answers server placement when the fresh probe never reaches the host', async () => { - getRuntimeEnvironmentStatusMock.mockResolvedValue({ + getRuntimeEnvironmentStatusForBrowserPlacementMock.mockResolvedValue({ id: 'status.get', ok: false, error: { code: 'runtime_unavailable', message: 'socket closed before ready' }, @@ -163,7 +161,7 @@ describe('runtime environment browser client host handler', () => { // placement instead of throwing, so nothing else notices that the user detached the runtime // while it was in flight. it('rejects a manual disconnect that lands while the probe is in flight', async () => { - getRuntimeEnvironmentStatusMock.mockImplementation(async () => { + getRuntimeEnvironmentStatusForBrowserPlacementMock.mockImplementation(async () => { manuallyDisconnectedMock.mockReturnValue(true) return { id: 'status.get', @@ -193,7 +191,7 @@ describe('runtime environment browser client host handler', () => { await expect(prepare(null, { selector: 'environment-a' })).rejects.toThrow( 'runtime_manually_disconnected' ) - expect(getRuntimeEnvironmentStatusMock).not.toHaveBeenCalled() + expect(getRuntimeEnvironmentStatusForBrowserPlacementMock).not.toHaveBeenCalled() expect(startHostMock).not.toHaveBeenCalled() }) diff --git a/src/main/ipc/runtime-environment-browser-client-host-handler.ts b/src/main/ipc/runtime-environment-browser-client-host-handler.ts index fb380c30c3d..742b3214938 100644 --- a/src/main/ipc/runtime-environment-browser-client-host-handler.ts +++ b/src/main/ipc/runtime-environment-browser-client-host-handler.ts @@ -11,7 +11,7 @@ import { } from '../browser/paired-runtime-browser-client-host-runtime' import { prepareBrowserClientHostPlacement } from '../browser/browser-client-host-placement-preparation' import { isRuntimeEnvironmentManuallyDisconnected } from './runtime-environment-connectivity-handlers' -import { getRuntimeEnvironmentStatus } from './runtime-environment-transport-routing' +import { getRuntimeEnvironmentStatusForBrowserPlacement } from './runtime-environment-transport-routing' export function registerRuntimeEnvironmentBrowserClientHostHandler(options: { getUserDataPath: () => string @@ -32,9 +32,10 @@ export function registerRuntimeEnvironmentBrowserClientHostHandler(options: { resolveEnvironment: (selector) => resolveEnvironment(userDataPath, selector), getStatus: async (environmentId) => { requireConnected(environmentId) - const status = await getRuntimeEnvironmentStatus(userDataPath, environmentId, undefined, { - observeOnly: true - }) + const status = await getRuntimeEnvironmentStatusForBrowserPlacement( + userDataPath, + environmentId + ) requireConnected(environmentId) return status }, diff --git a/src/main/ipc/runtime-environment-status-recovery.test.ts b/src/main/ipc/runtime-environment-status-recovery.test.ts index 82e94ea62e0..8b96172fa1e 100644 --- a/src/main/ipc/runtime-environment-status-recovery.test.ts +++ b/src/main/ipc/runtime-environment-status-recovery.test.ts @@ -7,6 +7,7 @@ import { addEnvironmentFromPairingCode } from '../../shared/runtime-environment- import { pairingCode } from './runtime-environments-ipc-test-harness' import { getRuntimeEnvironmentStatus, + getRuntimeEnvironmentStatusForBrowserPlacement, resetSharedControlSupport } from './runtime-environment-transport-routing' @@ -87,3 +88,25 @@ it('a passive capability check does not strand later active bootstrap recovery', await vi.advanceTimersByTimeAsync(3_000) expect(request).toHaveBeenCalledTimes(3) }) + +it('retries a transient placement probe once before falling back to server placement', async () => { + const environment = addEnvironmentFromPairingCode(profile, { + name: 'placement-retry', + pairingCode: pairingCode() + }) + request + .mockRejectedValueOnce( + Object.assign(new Error('host not ready'), { code: 'runtime_unavailable' }) + ) + .mockResolvedValueOnce({ + id: 'status', + ok: true, + result: { runtimeId: 'host-1', graphStatus: 'ready', capabilities: [] }, + _meta: { runtimeId: 'host-1' } + }) + + await expect( + getRuntimeEnvironmentStatusForBrowserPlacement(profile, environment.id) + ).resolves.toMatchObject({ ok: true, result: { runtimeId: 'host-1' } }) + expect(request).toHaveBeenCalledTimes(2) +}) diff --git a/src/main/ipc/runtime-environment-transport-routing.ts b/src/main/ipc/runtime-environment-transport-routing.ts index f39962c20cb..489181d8de0 100644 --- a/src/main/ipc/runtime-environment-transport-routing.ts +++ b/src/main/ipc/runtime-environment-transport-routing.ts @@ -7,6 +7,7 @@ import type { RuntimeRpcResponse } from '../../shared/runtime-rpc-envelope' import type { RuntimeStatus } from '../../shared/runtime-types' +import { isRuntimeHostStatusBlocked } from '../../shared/runtime-host-status' import { subscribeRemoteRuntimeRequest, type RemoteRuntimeSubscription @@ -62,6 +63,20 @@ export async function getRuntimeEnvironmentStatus( ) } +/** Probe live placement state, retrying once when the first status attempt is transient. */ +export async function getRuntimeEnvironmentStatusForBrowserPlacement( + userDataPath: string, + selector: string +): Promise> { + const response = await getRuntimeEnvironmentStatus(userDataPath, selector, undefined, { + observeOnly: true + }) + if (response.ok || isRuntimeHostStatusBlocked(response)) { + return response + } + return getRuntimeEnvironmentStatus(userDataPath, selector, undefined, { reconnect: true }) +} + export async function callRuntimeEnvironment( userDataPath: string, selector: string, diff --git a/src/renderer/src/runtime/web-runtime-browser-creation.ts b/src/renderer/src/runtime/web-runtime-browser-creation.ts index 99ad27d0adb..5e3983c3cbf 100644 --- a/src/renderer/src/runtime/web-runtime-browser-creation.ts +++ b/src/renderer/src/runtime/web-runtime-browser-creation.ts @@ -46,13 +46,9 @@ export async function createWebRuntimeSessionBrowserTab( stageWebRuntimeBrowserCreation(context) const placementPreference = args.placementPreference ?? 'auto' let placement: BrowserPageCreationPlacement = { kind: 'server' } - // Why no cached-capability gate here: the renderer's runtime status can hold a pre-upgrade - // "cannot client-host" verdict for a whole catalog TTL, and skipping the preparation on it - // pinned a capable pair to server placement for that long. The preparation reads live status - // and answers `server` for a runtime that truly cannot host, so staleness now costs a round - // trip against an incapable host instead of the wrong placement. An unreachable host pays for - // that on the failure path too: the probe's 15s ceiling, then the tabCreate behind it and the - // cleanup close its non-definitive failure needs, where before the create never started. + // Why the main process owns this probe: renderer status can lag a runtime replacement. The + // preparation path checks live host state and retries one transient failure before falling + // back to server placement. if (placementPreference !== 'server') { try { await pauseDuringE2eWebRuntimeBrowserClientHostPreparation()