From 394c280efd1a1c43b719065550c84646003eec2a Mon Sep 17 00:00:00 2001 From: T3 Code PR Stack <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2026 12:08:08 +0200 Subject: [PATCH] fix(web): address desktop preview tabs by runtime id in automation A conflict resolution in an upstream merge kept a stale fork copy of PreviewAutomationHosts. Status and the overlay wait used upstream's previewRuntimeTabId, but navigate, snapshot, click, type, press, scroll, evaluate, waitFor, setColorScheme and recordings still passed the bare server tab id. The desktop registers webviews under the runtime id, so every one of those failed with "Preview tab not found: tab_N" while preview_status kept reporting a healthy tab. Restore upstream's file (runtime ids, viewport rollback, presentation settle, shared navigation readiness) and keep only the fork's resolveNavigableUrl for open/navigate. Add a regression test that the snapshot bridge call receives the runtime tab id. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../preview/PreviewAutomationHosts.test.tsx | 65 +++++- .../preview/PreviewAutomationHosts.tsx | 187 +++++++++++------- 2 files changed, 182 insertions(+), 70 deletions(-) diff --git a/apps/web/src/components/preview/PreviewAutomationHosts.test.tsx b/apps/web/src/components/preview/PreviewAutomationHosts.test.tsx index 75ed20cc4fe7..6613bbcd0b50 100644 --- a/apps/web/src/components/preview/PreviewAutomationHosts.test.tsx +++ b/apps/web/src/components/preview/PreviewAutomationHosts.test.tsx @@ -14,7 +14,13 @@ import { create, type ReactTestRenderer } from "react-test-renderer"; import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; import { __resetClientSettingsPersistenceForTests } from "~/hooks/useSettings"; -import { readThreadPreviewState, resetPreviewStateForTests } from "~/previewStateStore"; +import { previewRuntimeTabId } from "~/browser/previewRuntimeTabId"; +import { + applyPreviewDesktopState, + readThreadPreviewState, + reconcilePreviewServerSessions, + resetPreviewStateForTests, +} from "~/previewStateStore"; import { appAtomRegistry, AppAtomRegistryProvider } from "~/rpc/atomRegistry"; import { PreviewAutomationHosts } from "./PreviewAutomationHosts"; @@ -32,6 +38,12 @@ const mocks = vi.hoisted(() => ({ (target: { environmentId: EnvironmentId; input: PreviewAutomationResponse }) => Promise >(), focus: vi.fn(async () => undefined), + bridge: { + automation: { + status: vi.fn(async (_tabId: string) => ({ available: true, loading: false })), + snapshot: vi.fn(async (_tabId: string) => ({ title: "Snapshot" })), + }, + }, })); vi.mock("~/localApi", () => ({ @@ -57,7 +69,7 @@ vi.mock("~/state/use-atom-command", () => ({ vi.mock("~/state/use-atom-query-runner", () => ({ useAtomQueryRunner: () => mocks.list, })); -vi.mock("./previewBridge", () => ({ previewBridge: { automation: {} } })); +vi.mock("./previewBridge", () => ({ previewBridge: mocks.bridge })); const environmentId = EnvironmentId.make("automation-environment"); const threadId = ThreadId.make("automation-thread"); @@ -188,3 +200,52 @@ describe("PreviewAutomationHosts open", () => { expect(mocks.setClientSettings).not.toHaveBeenCalled(); }); }); + +describe("PreviewAutomationHosts desktop operations", () => { + it("addresses the desktop tab by its runtime id, not the server tab id", async () => { + // The desktop registers webviews under previewRuntimeTabId. Passing the + // bare server tab id makes every operation after `status` fail with + // "Preview tab not found" in the desktop preview manager. + reconcilePreviewServerSessions(threadRef, { + sessions: [snapshot], + serverEpoch: "test-server", + revision: 1, + }); + applyPreviewDesktopState(threadRef, snapshot.tabId, {} as never); + const runtimeTabId = previewRuntimeTabId(threadRef, "test-server", snapshot.tabId); + const wrapper = { getAttribute: () => "active" }; + const webview = { + getAttribute: (name: string) => (name === "data-preview-tab" ? runtimeTabId : null), + closest: () => wrapper, + executeJavaScript: async () => null, + }; + vi.stubGlobal("document", { hasFocus: () => false, querySelectorAll: () => [webview] }); + const response = deferred(); + mocks.respond.mockImplementationOnce(async ({ input }) => response.resolve(input)); + + await act(async () => { + appAtomRegistry.set( + requestsAtom, + AsyncResult.success({ + ...requestEvent, + request: { + requestId: "snapshot-request", + threadId, + tabId: snapshot.tabId, + operation: "snapshot", + input: {}, + timeoutMs: 15_000, + }, + }), + ); + await response.promise; + }); + + await expect(response.promise).resolves.toMatchObject({ + requestId: "snapshot-request", + ok: true, + }); + expect(mocks.bridge.automation.snapshot).toHaveBeenCalledExactlyOnceWith(runtimeTabId); + expect(mocks.bridge.automation.status).toHaveBeenCalledWith(runtimeTabId); + }); +}); diff --git a/apps/web/src/components/preview/PreviewAutomationHosts.tsx b/apps/web/src/components/preview/PreviewAutomationHosts.tsx index 47da1e43302f..33a84f66bd95 100644 --- a/apps/web/src/components/preview/PreviewAutomationHosts.tsx +++ b/apps/web/src/components/preview/PreviewAutomationHosts.tsx @@ -62,7 +62,6 @@ import { useAtomCommand } from "~/state/use-atom-command"; import { previewBridge } from "./previewBridge"; import { - PreviewAutomationNavigationTimeoutError, PreviewAutomationOperationError, PreviewAutomationOverlayTimeoutError, PreviewAutomationRecordingNotActiveError, @@ -76,7 +75,10 @@ import { shouldAutoShowPreviewForAutomationUse, shouldOpenPreviewMiniPlayer, } from "./previewAutomationOpenReadiness"; -import { assertPreviewRuntimeCurrent } from "./previewNavigationReadiness"; +import { + assertPreviewRuntimeCurrent, + waitForNavigationReadiness, +} from "./previewNavigationReadiness"; import { createPreviewAutomationRequestConsumerAtom } from "./previewAutomationRequestConsumer"; import { createPreviewAutomationClientId } from "./previewAutomationClientId"; import { @@ -86,6 +88,17 @@ import { } from "./previewAutomationTarget"; import { resolveHostWaitBudgetMs, waitForHostReadiness } from "./previewAutomationHostBudget"; import { isPreviewViewportReady } from "./previewViewportReadiness"; +import { shouldRollbackPreviewViewport } from "./previewViewportRollback"; + +const PREVIEW_PRESENTATION_SETTLE_TIMEOUT_MS = 500; + +const waitForPreviewPresentation = async (runtimeTabId: string): Promise => { + const deadline = Date.now() + PREVIEW_PRESENTATION_SETTLE_TIMEOUT_MS; + while (Date.now() <= deadline) { + if (useBrowserSurfaceStore.getState().byTabId[runtimeTabId]?.visible) return; + await new Promise((resolve) => window.setTimeout(resolve, 16)); + } +}; const waitForDesktopOverlay = async ( threadRef: ScopedThreadRef, @@ -116,38 +129,6 @@ const waitForDesktopOverlay = async ( }); }; -const waitForNavigationReadiness = async ( - threadRef: ScopedThreadRef, - requestId: string, - tabId: string, - readiness: PreviewAutomationNavigateInput["readiness"], - timeoutMs: number, -): Promise => { - const targetReadiness = readiness ?? "load"; - if (!previewBridge || targetReadiness === "none") return; - const deadline = Date.now() + timeoutMs; - while (Date.now() <= deadline) { - if (targetReadiness === "domContentLoaded") { - const readyState = await previewBridge.automation.evaluate(tabId, { - expression: "document.readyState", - }); - if (readyState === "interactive" || readyState === "complete") return; - } else { - const status = await previewBridge.automation.status(tabId); - if (!status.loading) return; - } - await new Promise((resolve) => window.setTimeout(resolve, 50)); - } - throw new PreviewAutomationNavigationTimeoutError({ - requestId, - environmentId: threadRef.environmentId, - threadId: threadRef.threadId, - tabId, - readiness: targetReadiness, - timeoutMs, - }); -}; - interface ExecutablePreviewWebview extends Element { readonly executeJavaScript: (code: string, userGesture?: boolean) => Promise; } @@ -180,8 +161,10 @@ const readWebviewViewport = async ( : null; }; -const readRenderedViewport = async (tabId: string): Promise => { - const webview = findPreviewWebview(tabId); +const readRenderedViewport = async ( + runtimeTabId: string, +): Promise => { + const webview = findPreviewWebview(runtimeTabId); if (!webview) return null; return await readWebviewViewport(webview); }; @@ -197,19 +180,23 @@ const readDeclaredViewport = ( }; const waitForRenderedViewport = async ( + threadRef: ScopedThreadRef, tabId: string, + runtimeTabId: string, setting: PreviewViewportSetting, timeoutMs: number, context: { readonly requestId: PreviewAutomationRequest["requestId"]; + readonly operation: PreviewAutomationRequest["operation"]; readonly environmentId: EnvironmentId; readonly threadId: PreviewAutomationRequest["threadId"]; }, ): Promise => { const deadline = Date.now() + timeoutMs; while (Date.now() <= deadline) { + assertPreviewRuntimeCurrent(threadRef, tabId, runtimeTabId, context); try { - const webview = findPreviewWebview(tabId); + const webview = findPreviewWebview(runtimeTabId); const appliedSettingKey = webview?.getAttribute("data-preview-viewport-key") ?? null; const declaredViewport = readDeclaredViewport(webview); const renderedViewport = webview ? await readWebviewViewport(webview) : null; @@ -530,12 +517,22 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId }) if (activeSnapshot && previewAutomationOpenNeedsOverlay(input, activeSnapshot)) { await requireReadyTab(); } + if (shouldPresentPreview) { + // React commits the thread-bound surface asynchronously. Settle + // briefly so active-thread opens report visible=true, without + // turning a background thread's offscreen mini player into an + // operation failure. + await waitForPreviewPresentation(activeRuntimeTabId); + } if (reusedExistingTab && resolvedInputUrl && previewBridge) { - await previewBridge.navigate(activeTabId, resolvedInputUrl); + assertPreviewRuntimeCurrent(threadRef, activeTabId, activeRuntimeTabId, request); + await previewBridge.navigate(activeRuntimeTabId, resolvedInputUrl); await waitForNavigationReadiness( threadRef, request.requestId, activeTabId, + activeRuntimeTabId, + request.operation, "load", request.timeoutMs, ); @@ -552,11 +549,13 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId }) url: input.url!, }, ); - await ready.bridge.navigate(ready.tabId, resolvedUrl); + await ready.bridge.navigate(ready.runtimeTabId, resolvedUrl); await waitForNavigationReadiness( threadRef, request.requestId, ready.tabId, + ready.runtimeTabId, + request.operation, input.readiness ?? "load", input.timeoutMs ?? request.timeoutMs, ); @@ -566,28 +565,76 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId }) const ready = await requireReadyTab(); const input = request.input as PreviewAutomationResizeInput; const setting = resolvePreviewViewport(input); - const result = await resize({ - environmentId, - input: { - threadId: request.threadId, - tabId: ready.tabId, - viewport: setting, - }, + const applied = await runBrowserViewportMutation(ready.runtimeTabId, async () => { + const operationState = assertPreviewRuntimeCurrent( + threadRef, + ready.tabId, + ready.runtimeTabId, + request, + ); + const previousSetting = + operationState.sessions[ready.tabId]?.viewport ?? FILL_PREVIEW_VIEWPORT; + const result = await resize({ + environmentId, + input: { + threadId: request.threadId, + tabId: ready.tabId, + viewport: setting, + }, + }); + if (result._tag === "Failure") { + return raiseAtomCommandFailure(result); + } + updatePreviewServerSnapshot(threadRef, result.value); + return { + previousSetting, + serverEpoch: operationState.serverEpoch, + }; }); - if (result._tag === "Failure") { - return raiseAtomCommandFailure(result); + let viewport: PreviewRenderedViewportSize; + try { + viewport = await waitForRenderedViewport( + threadRef, + ready.tabId, + ready.runtimeTabId, + setting, + input.timeoutMs ?? request.timeoutMs, + { + requestId: request.requestId, + operation: request.operation, + environmentId, + threadId: request.threadId, + }, + ); + } catch (cause) { + await runBrowserViewportMutation(ready.runtimeTabId, async () => { + const latestState = readThreadPreviewState(threadRef); + const latestSetting = + latestState.sessions[ready.tabId]?.viewport ?? FILL_PREVIEW_VIEWPORT; + if ( + shouldRollbackPreviewViewport( + applied.previousSetting, + setting, + latestSetting, + applied.serverEpoch, + latestState.serverEpoch, + ) + ) { + const rollback = await resize({ + environmentId, + input: { + threadId: request.threadId, + tabId: ready.tabId, + viewport: applied.previousSetting, + }, + }); + if (rollback._tag !== "Failure") { + updatePreviewServerSnapshot(threadRef, rollback.value); + } + } + }); + throw cause; } - updatePreviewServerSnapshot(threadRef, result.value); - const viewport = await waitForRenderedViewport( - ready.tabId, - setting, - input.timeoutMs ?? request.timeoutMs, - { - requestId: request.requestId, - environmentId, - threadId: request.threadId, - }, - ); return { tabId: ready.tabId, setting, @@ -597,7 +644,7 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId }) case "setColorScheme": { const ready = await requireReadyTab(); const input = request.input as PreviewAutomationSetColorSchemeInput; - await ready.bridge.setColorScheme(ready.tabId, input.colorScheme); + await ready.bridge.setColorScheme(ready.runtimeTabId, input.colorScheme); return { tabId: ready.tabId, colorScheme: input.colorScheme, @@ -605,53 +652,57 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId }) } case "snapshot": { const ready = await requireReadyTab(); - return await ready.bridge.automation.snapshot(ready.tabId); + return await ready.bridge.automation.snapshot(ready.runtimeTabId); } case "click": { const ready = await requireReadyTab(); return await ready.bridge.automation.click( - ready.tabId, + ready.runtimeTabId, request.input as Parameters[1], ); } case "type": { const ready = await requireReadyTab(); return await ready.bridge.automation.type( - ready.tabId, + ready.runtimeTabId, request.input as Parameters[1], ); } case "press": { const ready = await requireReadyTab(); return await ready.bridge.automation.press( - ready.tabId, + ready.runtimeTabId, request.input as Parameters[1], ); } case "scroll": { const ready = await requireReadyTab(); return await ready.bridge.automation.scroll( - ready.tabId, + ready.runtimeTabId, request.input as Parameters[1], ); } case "evaluate": { const ready = await requireReadyTab(); return await ready.bridge.automation.evaluate( - ready.tabId, + ready.runtimeTabId, request.input as Parameters[1], ); } case "waitFor": { const ready = await requireReadyTab(); return await ready.bridge.automation.waitFor( - ready.tabId, + ready.runtimeTabId, request.input as Parameters[1], ); } case "recordingStart": { const ready = await requireReadyTab(); - const startedAt = await startBrowserRecording(ready.tabId, threadRef); + const startedAt = await startBrowserRecording( + ready.runtimeTabId, + threadRef, + ready.tabId, + ); return { tabId: ready.tabId, recording: true,