fix(preview): bound recording finalization

Author: Quicksaver

BRANCH_DETAILS.md

@@ -20,7 +20,7 @@ This branch does not add a second recording/PiP capture lifecycle or another hid
 Expected behavior:
 
 - Every Electron automation operation has a bounded control-session lifetime. The desktop manager reserves response grace inside the requested timeout without making the execution budget shrink when the caller increases a short timeout, always finalizes controller and action-timeline state, and detaches a timed-out debugger session while still holding an acquired control permit when a CDP command may be pending. Session removal and debugger teardown are atomic with respect to new session acquisition and bound to the exact acquired session, so late interruption or snapshot cleanup cannot detach a healthy replacement. Operations already queued on the retired semaphore detect that stale session and retry against its replacement. A request that times out while queued behind another action does not detach that action's shared debugger session.
-- Click, type, press, scroll, and wait operations clamp their caller-supplied timeout to the remaining renderer host budget before entering the desktop control-session boundary. Color-scheme changes and recording startup likewise receive the remaining deadline after overlay readiness; a timed-out color-scheme command does not persist a late preference, and timed-out recording startup tears down its frame-capture session and renderer recording state. Operations without a caller timeout use the remaining bounded request budget rather than restarting the desktop default after renderer readiness work.
+- Click, type, press, scroll, and wait operations clamp their caller-supplied timeout to the remaining renderer host budget before entering the desktop control-session boundary. Color-scheme changes and recording startup likewise receive the remaining deadline after overlay readiness; a timed-out color-scheme command does not persist a late preference, and timed-out recording startup tears down its frame-capture session and renderer recording state. Timed appearance persistence re-reads current tab state after CDP settles instead of relying on a pre-await snapshot. Recording stop bounds desktop capture shutdown, MediaRecorder settlement, blob conversion, and artifact persistence to the remaining deadline; a deadline failure retains captured chunks and the recording slot so finalization can be retried instead of silently losing the artifact. Operations without a caller timeout use the remaining bounded request budget rather than restarting the desktop default after renderer readiness work.
 - Snapshot collection keeps active-tab capture on CDP `Page.captureScreenshot` from the compositor surface. For an unselected tab, the renderer stages the still-mounted guest at effectively transparent opacity for two compositor frames, but only for the snapshot itself. The desktop manager captures that compositor surface without focusing the guest or calling `Page.bringToFront`; either activation call can make Electron promote the native guest over the host window and keep the T3 interface covered after staging ends. A separately bounded `webContents.capturePage` attempt provides a fallback, using `stayHidden: true` for background guests and normal visible-page capture for the foreground. Primary and fallback screenshot waits are clamped to the remaining control-session deadline, with budget reserved for fallback and result settlement, so a tight caller deadline can still return semantic data instead of being preempted by the outer session timeout. Every returned PNG, including resized output, is validated and bounded. Final screenshot failure or timeout is logged, an actually timed-out CDP capture resets the session before releasing its control permit, queued work reattaches before issuing its first command, and a capture skipped before CDP runs leaves the healthy session attached. The semantic page state, interactive elements, accessibility tree, diagnostics, and action timeline still return with `screenshot: null` instead of failing the complete snapshot.
 - Desktop preview guests following the system color scheme create their CDP debugger session lazily, with initialization included in the automation operation deadline. This prevents an offscreen Chromium guest from leaving `Runtime.enable` pending while holding the synchronized session lock, which previously made every later evaluation or snapshot against that tab time out even after it became presentable. `apps/web/src/browser/desktopTabLifetime.ts` passes the upstream browser appearance default through `DesktopPreviewCreateTabInputSchema` in `packages/contracts/src/ipc.ts`; `apps/desktop/src/preview/Manager.ts` normalizes that value. A non-system color-scheme override is restored after webview registration or detached DevTools closes through a separately bounded recovery path, while tabs following the system scheme stay detached until the next automation operation.
 - Building on upstream's retained hidden guest, automation background snapshot presentation is reference-counted independently from the normal surface lease and composes with upstream's fitted-source content and corner-radius presentation. `PreviewAutomationHosts.tsx` passes the epoch-scoped runtime tab id into `previewAutomationPresentation.ts`; every surface lookup, staging marker, readiness check, diagnostic read, lease, and desktop capture targets that exact runtime guest, while selection and errors retain the stable server tab id. The presentation helper API has no state-derived or server-id compatibility fallback. Only a one-shot automation snapshot acquires this lease; upstream recording and picture-in-picture continue to use their shared frame-capture lifecycle, while navigation, color-scheme changes, evaluation, waits, and input operations do not acquire an automation presentation lease. Staging always restores the offscreen position and does not change the human-selected surface. The entire lease, including compositor-frame staging and desktop IPC, is bounded by the operation's remaining response budget and reports a typed timeout if it stalls. If the server epoch replaces the runtime guest while staging is pending, the snapshot fails immediately with `PreviewAutomationTargetUnavailableError` instead of waiting on the stale staging marker. If the user foregrounds the target in either surface while staging is pending, that visible presentation satisfies readiness. A never-presented tab does not depend on another browser surface having supplied a panel rectangle: automation staging falls back to a deterministic rectangle fitted inside the renderer viewport.
@@ -30,7 +30,7 @@ Expected behavior:
 - `PreviewAutomationHosts.tsx` resolves `browserDefaults.ts` once at the start of each automation-open request, before taking the session snapshot that pins a reused runtime guest. That single settings snapshot supplies the new-tab viewport and the automatic floating-preview preference, so creation and presentation cannot observe different settings during one request, while cold settings hydration cannot mix a pre-await session snapshot with a post-await server epoch. Explicit `open` or its deprecated `show` alias remains authoritative; when both are omitted, `autoShowFloatingPreview` decides presentation. The resulting `shouldPresentPreview` value is passed unchanged to `previewAutomationOpenReadiness.ts`, so a reused rendered tab left in the background does not wait for visibility while an explicitly shown tab does. After settings and session synchronization, the host rechecks its remaining deadline immediately before tab creation and every later irreversible open-side mutation, so an already expired request cannot create, resize, reveal, or navigate a preview. A newly created tab applies its server snapshot and assigned tab id, uses the configured viewport or the branch's deterministic 1280×800 fallback when the snapshot remains `fill`, initiates any requested selection, and acknowledges server-side creation without depending on cold React panel rendering, Electron overlay registration, or page readiness. Its initial URL continues loading exactly once in that same tab; later wait, snapshot, or interaction operations own attachment and page readiness. Reopening an existing shown tab selects both the preview-state tab and its matching inline mini-player surface, then waits for stable presentation and reasserts that selection across same-server route hydration or session reconciliation. A server-epoch change aborts the pending open with `PreviewAutomationTargetUnavailableError`; the old request never adopts the replacement runtime guest.
 - The standard dev runner keeps local navigation and direct backend URLs on `127.0.0.1`. Browser modes use a single origin: they leave client HTTP/WebSocket URLs and generic `HOST` unset so remote sharing and origin-derived HMR keep working, while Vite's default listener and its default backend proxy use explicit IPv4 loopback. Explicit IPv6 backend binds proxy through IPv6 loopback. Desktop mode pins `HOST` and its renderer/backend URLs to `127.0.0.1`; server-only mode keeps direct HTTP/WebSocket URLs on the same IPv4 loopback.
 - Retained browser guests subscribe only to render state for their own epoch-scoped runtime tab, plus their thread's active panel and mini-player selection. Presentation or background-capture updates for another tab do not rerender every mounted `HostedBrowserWebview`; selection changes override a stale surface-visible flag so background staging remains nearly transparent and exposes its readiness marker, and staging derives its viewport-fitted rectangle from the target tab's own stable rectangle. Only the active guest is exposed through the host accessibility tree; a nearly transparent background-capture guest stays `aria-hidden` without affecting CDP accessibility-tree collection against the guest target.
-- Rendered viewport readiness revalidates the epoch-scoped runtime guest after each awaited guest measurement and clamps every polling sleep to the remaining resize deadline, preserving typed target-replacement and viewport-timeout failures for short budgets.
+- Rendered viewport readiness revalidates the epoch-scoped runtime guest after each awaited guest measurement and clamps every polling sleep to the remaining resize deadline, preserving typed target-replacement and viewport-timeout failures for short budgets. The serialized viewport mutation also rechecks that deadline after acquiring its mutation queue, before sending the server resize.
 - The primary pairing route watches for later URL-fragment changes while it remains mounted. Navigating an already-loaded `/pair` document to `/pair#token=...` claims each new token once, removes the secret fragment, and runs the normal pairing exchange without requiring a reload or a second desktop window. Multiple tokens received while an exchange is pending are serialized, and the submitting state remains active until every queued exchange settles.
 - `dev:desktop` derives `T3CODE_DESKTOP_USER_DATA_DIR=<resolved base dir>/userdata/electron` whenever the runner has an explicit base directory. Desktop configuration resolves that override to an absolute path, and app identity uses it before legacy migration or the normal Electron user-data default. This keeps an isolated worktree dev desktop from reusing an installed or earlier development profile whose incompatible IndexedDB schema can prevent the renderer from starting. Packaged/default startup remains unchanged when no override is supplied.
 
@@ -76,7 +76,7 @@ vp test run scripts/dev-runner.test.ts apps/desktop/src/app/DesktopAppIdentity.t
 
 Current verification:
 
-- The branch-focused suite passed 278 tests across 19 files on Windows, including the new bounded post-overlay mutations, recording-start cleanup, short-deadline viewport polling, post-read runtime-replacement, and background-guest accessibility regressions. The two unchanged desktop path-fixture files in the command above currently produce eight POSIX-versus-Windows path assertion failures; the affected runtime behavior passes in the remaining focused suite.
+- The branch-focused suite passed 281 tests across 19 files on Windows, including bounded post-overlay and queued resize mutations, retryable recording finalization, recording-start cleanup, current-state appearance persistence, short-deadline viewport polling, post-read runtime-replacement, and background-guest accessibility regressions. The two unchanged desktop path-fixture files in the command above currently produce eight POSIX-versus-Windows path assertion failures; the affected runtime behavior passes in the remaining focused suite.
 - The incoming browser-default, provider-access, pull-request budget, tooltip, update-copy, and settings coverage passed 552 tests across 26 files. The focused open-policy and open-session subset passes 18 tests, including a shared explicit-over-default presentation decision and a reused rendered tab that remains in the background without a visibility wait.
 - Desktop, server, contracts, and mobile typechecks pass. Web typecheck currently reports 16 existing errors in `BranchToolbarBranchSelector.tsx`, `ModelPickerContent.tsx`, `PreviewAutomationHosts.tsx` at the pre-existing registry access, `FontFamilyPicker.tsx`, `use-atom-command.ts`, and `use-atom-query-runner.ts`; none are in the browser-default integration changed here.
 - An isolated web client on ports `5744`/`13784` paired and loaded successfully. Settings → Integrations exposed the browser defaults and agent-access setting; agent browser access could be disabled and restored. The non-Electron client correctly disabled desktop-only viewport, zoom, and appearance controls.

apps/desktop/src/ipc/methods/preview.ts

@@ -12,6 +12,7 @@ import {
   DesktopPreviewNavigateInputSchema,
   DesktopPreviewRecordingArtifactSchema,
   DesktopPreviewRecordingStartInputSchema,
+  DesktopPreviewRecordingStopInputSchema,
   DesktopPreviewRecordingSaveInputSchema,
   DesktopPreviewRegisterWebviewInputSchema,
   DesktopPreviewScreenshotArtifactSchema,
@@ -178,11 +179,15 @@ export const startRecording = DesktopIpc.makeIpcMethod({
     yield* manager.startRecording(tabId, timeoutMs);
   }),
 });
-export const stopRecording = tabMethod(
-  IpcChannels.PREVIEW_RECORDING_STOP_CHANNEL,
-  "desktop.ipc.preview.stopRecording",
-  (manager, tabId) => manager.stopRecording(tabId),
-);
+export const stopRecording = DesktopIpc.makeIpcMethod({
+  channel: IpcChannels.PREVIEW_RECORDING_STOP_CHANNEL,
+  payload: DesktopPreviewRecordingStopInputSchema,
+  result: Schema.Void,
+  handler: Effect.fn("desktop.ipc.preview.stopRecording")(function* ({ tabId, timeoutMs }) {
+    const manager = yield* PreviewManager.PreviewManager;
+    yield* manager.stopRecording(tabId, timeoutMs);
+  }),
+});
 export const openPictureInPicture = tabMethod(
   IpcChannels.PREVIEW_PICTURE_IN_PICTURE_OPEN_CHANNEL,
   "desktop.ipc.preview.openPictureInPicture",
@@ -367,9 +372,14 @@ export const saveRecording = DesktopIpc.makeIpcMethod({
   channel: IpcChannels.PREVIEW_RECORDING_SAVE_CHANNEL,
   payload: DesktopPreviewRecordingSaveInputSchema,
   result: DesktopPreviewRecordingArtifactSchema,
-  handler: Effect.fn("desktop.ipc.preview.saveRecording")(function* ({ tabId, mimeType, data }) {
+  handler: Effect.fn("desktop.ipc.preview.saveRecording")(function* ({
+    tabId,
+    mimeType,
+    data,
+    timeoutMs,
+  }) {
     const manager = yield* PreviewManager.PreviewManager;
-    return yield* manager.saveRecording(tabId, mimeType, data);
+    return yield* manager.saveRecording(tabId, mimeType, data, timeoutMs);
   }),
 });
 

apps/desktop/src/preload.ts

@@ -213,13 +213,14 @@ contextBridge.exposeInMainWorld("desktopBridge", {
     recording: {
       startScreencast: (tabId, timeoutMs) =>
         ipcRenderer.invoke(IpcChannels.PREVIEW_RECORDING_START_CHANNEL, { tabId, timeoutMs }),
-      stopScreencast: (tabId) =>
-        ipcRenderer.invoke(IpcChannels.PREVIEW_RECORDING_STOP_CHANNEL, { tabId }),
-      save: (tabId, mimeType, data) =>
+      stopScreencast: (tabId, timeoutMs) =>
+        ipcRenderer.invoke(IpcChannels.PREVIEW_RECORDING_STOP_CHANNEL, { tabId, timeoutMs }),
+      save: (tabId, mimeType, data, timeoutMs) =>
         ipcRenderer.invoke(IpcChannels.PREVIEW_RECORDING_SAVE_CHANNEL, {
           tabId,
           mimeType,
           data,
+          timeoutMs,
         }),
       onFrame: (listener) => {
         const wrappedListener = (_event: Electron.IpcRendererEvent, frame: unknown) => {

apps/desktop/src/preview/Manager.test.ts

@@ -1788,6 +1788,79 @@ describe("PreviewManager", () => {
     ),
   );
 
+  effectIt.effect("re-reads color-scheme state after a bounded CDP mutation settles", () =>
+    withManager((manager) =>
+      Effect.gen(function* () {
+        let attached = false;
+        let darkCommands = 0;
+        let finishBoundedDark: (() => void) | undefined;
+        const sendCommand = vi.fn(
+          async (method: string, parameters?: { features?: ReadonlyArray<{ value: string }> }) => {
+            if (method !== "Emulation.setEmulatedMedia") return undefined;
+            if (parameters?.features?.[0]?.value !== "dark") return undefined;
+            darkCommands += 1;
+            if (darkCommands !== 2) return undefined;
+            await new Promise<void>((resolve) => {
+              finishBoundedDark = resolve;
+            });
+            return undefined;
+          },
+        );
+        fromId.mockReturnValue({
+          id: 42,
+          isDestroyed: () => false,
+          isDevToolsOpened: () => false,
+          getType: () => "webview",
+          getURL: () => "https://example.com",
+          getTitle: () => "Example",
+          isLoading: () => false,
+          getZoomFactor: () => 1,
+          setZoomFactor: vi.fn(),
+          on: vi.fn(),
+          off: vi.fn(),
+          ipc: { on: vi.fn(), off: vi.fn() },
+          send: webviewSend,
+          navigationHistory: { canGoBack: () => false, canGoForward: () => false },
+          setWindowOpenHandler: vi.fn(),
+          debugger: {
+            isAttached: () => attached,
+            attach: vi.fn(() => {
+              attached = true;
+            }),
+            detach: vi.fn(() => {
+              attached = false;
+            }),
+            sendCommand,
+            on: vi.fn(),
+            off: vi.fn(),
+          },
+        } as never);
+        const states: PreviewManager.PreviewTabState[] = [];
+        yield* manager.subscribeStateChanges((_tabId, state) =>
+          Effect.sync(() => {
+            states.push(state);
+          }),
+        );
+        yield* manager.createTab("tab_scheme_reread");
+        yield* manager.registerWebview("tab_scheme_reread", 42);
+        yield* Effect.yieldNow;
+        yield* manager.setColorScheme("tab_scheme_reread", "dark");
+
+        const boundedDark = yield* manager
+          .setColorScheme("tab_scheme_reread", "dark", 1_000)
+          .pipe(Effect.forkChild({ startImmediately: true }));
+        yield* Effect.yieldNow;
+        yield* manager.setColorScheme("tab_scheme_reread", "light");
+        expect(states.at(-1)?.colorScheme).toBe("light");
+
+        finishBoundedDark?.();
+        yield* Fiber.join(boundedDark);
+
+        expect(states.at(-1)?.colorScheme).toBe("dark");
+      }),
+    ),
+  );
+
   effectIt.effect("blocks late webview and capture starts during tab close", () =>
     withManager((manager) =>
       Effect.gen(function* () {