fix(preview): make deadline retries idempotent

- Bound failed recording-start cleanup and always release its slot
- Retry appearance changes when a replacement rejects the stale command
- Reuse validated artifact keys across desktop save timeout retries

🤖 Co-authored by GPT-5 in Codex via T3 Code

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. Timed appearance persistence re-reads current tab state after CDP settles and reapplies to the current guest if the webview was replaced instead of trusting a pre-await target snapshot. Recording stop bounds desktop capture shutdown, MediaRecorder settlement, blob conversion, and artifact persistence to the remaining deadline; renderer and desktop-originated deadline failures retain captured chunks and the recording slot so finalization can be retried instead of silently losing the artifact. An in-flight artifact-save promise is shared with that retry, preventing duplicate timestamped files when the renderer deadline wins before desktop IPC settles. 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 bounds cleanup to that deadline while always releasing its renderer recording slot. Timed appearance persistence re-reads current tab state after CDP settles and retries against the current guest if replacement rejects the stale guest's command. Recording stop bounds desktop capture shutdown, MediaRecorder settlement, blob conversion, and artifact persistence to the remaining deadline; renderer and desktop-originated deadline failures retain captured chunks and the recording slot so finalization can be retried instead of silently losing the artifact. An in-flight artifact-save promise is shared with that retry, and every retry reuses a validated desktop artifact idempotency key, preventing duplicate files whether the renderer deadline wins before desktop IPC settles or the desktop reports a timeout after writing. 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.
@@ -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 284 tests across 19 files on Windows, including bounded post-overlay and queued resize mutations, retryable and deduplicated recording finalization across renderer and desktop timeouts, recording-start cleanup, replacement-guest 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 branch-focused suite passed 287 tests across 19 files on Windows, including bounded post-overlay and queued resize mutations, retryable and idempotent recording finalization across renderer and desktop timeouts, deadline-bounded recording-start cleanup, replacement-guest appearance retries, 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

@@ -376,10 +376,11 @@ export const saveRecording = DesktopIpc.makeIpcMethod({
     tabId,
     mimeType,
     data,
+    idempotencyKey,
     timeoutMs,
   }) {
     const manager = yield* PreviewManager.PreviewManager;
-    return yield* manager.saveRecording(tabId, mimeType, data, timeoutMs);
+    return yield* manager.saveRecording(tabId, mimeType, data, idempotencyKey, timeoutMs);
   }),
 });
 

apps/desktop/src/preload.ts

@@ -215,11 +215,12 @@ contextBridge.exposeInMainWorld("desktopBridge", {
         ipcRenderer.invoke(IpcChannels.PREVIEW_RECORDING_START_CHANNEL, { tabId, timeoutMs }),
       stopScreencast: (tabId, timeoutMs) =>
         ipcRenderer.invoke(IpcChannels.PREVIEW_RECORDING_STOP_CHANNEL, { tabId, timeoutMs }),
-      save: (tabId, mimeType, data, timeoutMs) =>
+      save: (tabId, mimeType, data, idempotencyKey, timeoutMs) =>
         ipcRenderer.invoke(IpcChannels.PREVIEW_RECORDING_SAVE_CHANNEL, {
           tabId,
           mimeType,
           data,
+          idempotencyKey,
           timeoutMs,
         }),
       onFrame: (listener) => {

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

@@ -1861,18 +1861,18 @@ describe("PreviewManager", () => {
     ),
   );
 
-  effectIt.effect("reapplies a bounded color scheme after the webview is replaced", () =>
+  effectIt.effect("retries a bounded color scheme when replacement rejects the old command", () =>
     withManager((manager) =>
       Effect.gen(function* () {
-        let finishFirstGuest: (() => void) | undefined;
+        let rejectFirstGuest: ((cause: unknown) => void) | undefined;
         const firstSendCommand = vi.fn(
           async (method: string, parameters?: { features?: ReadonlyArray<{ value: string }> }) => {
             if (
               method === "Emulation.setEmulatedMedia" &&
               parameters?.features?.[0]?.value === "dark"
             ) {
-              await new Promise<void>((resolve) => {
-                finishFirstGuest = resolve;
+              await new Promise<void>((_resolve, reject) => {
+                rejectFirstGuest = reject;
               });
             }
             return undefined;
@@ -1907,6 +1907,7 @@ describe("PreviewManager", () => {
               }),
               detach: vi.fn(() => {
                 attached = false;
+                if (id === 42) rejectFirstGuest?.(new Error("old guest detached"));
               }),
               sendCommand,
               on: vi.fn(),
@@ -1933,7 +1934,6 @@ describe("PreviewManager", () => {
         yield* Effect.yieldNow;
         yield* manager.registerWebview("tab_scheme_replacement", 43);
         yield* Effect.yieldNow;
-        finishFirstGuest?.();
         yield* Fiber.join(mutation);
 
         expect(replacementSendCommand).toHaveBeenCalledWith("Emulation.setEmulatedMedia", {
@@ -1944,6 +1944,31 @@ describe("PreviewManager", () => {
     ),
   );
 
+  effectIt.effect("uses an idempotent artifact path for recording save retries", () =>
+    withManager((manager) =>
+      Effect.gen(function* () {
+        const data = new Uint8Array([1, 2, 3]);
+        const first = yield* manager.saveRecording(
+          "tab_recording_save",
+          "video/webm",
+          data,
+          "8edc2f33-7bb4-4a30-97e8-e78f1d84513a",
+        );
+        const retry = yield* manager.saveRecording(
+          "tab_recording_save",
+          "video/webm",
+          data,
+          "8edc2f33-7bb4-4a30-97e8-e78f1d84513a",
+        );
+
+        expect(retry.id).toBe(first.id);
+        expect(retry.path).toBe(first.path);
+        expect(writeFile).toHaveBeenCalledTimes(2);
+        expect(writeFile.mock.calls[1]?.[0]).toBe(writeFile.mock.calls[0]?.[0]);
+      }),
+    ),
+  );
+
   effectIt.effect("blocks late webview and capture starts during tab close", () =>
     withManager((manager) =>
       Effect.gen(function* () {

apps/desktop/src/preview/Manager.ts

@@ -2345,7 +2345,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
         if (remainingTimeoutMs <= 0) {
           return yield* new PreviewAutomationTimeoutError({ tabId, timeoutMs });
         }
-        yield* withControlSession(
+        const commandExit = yield* withControlSession(
           tabId,
           target,
           "set-color-scheme",
@@ -2359,7 +2359,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
               ],
             }),
           remainingTimeoutMs,
-        );
+        ).pipe(Effect.exit);
         const currentTab = (yield* SynchronizedRef.get(tabsRef)).get(tabId);
         if (!currentTab) {
           return yield* new PreviewTabNotFoundError({ tabId });
@@ -2368,6 +2368,9 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
           target = yield* requireWebContents(tabId);
           continue;
         }
+        if (Exit.isFailure(commandExit)) {
+          return yield* Effect.failCause(commandExit.cause);
+        }
         if (currentTab.colorScheme !== colorScheme) {
           yield* update(tabId, { colorScheme });
         }
@@ -2956,9 +2959,10 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
     tabId: string,
     mimeType: string,
     data: Uint8Array,
+    idempotencyKey: string,
   ) {
-    const [createdAt, millis] = yield* Effect.all([currentIso, currentMillis]);
-    const id = `browser-recording-${millis.toString(36)}`;
+    const createdAt = yield* currentIso;
+    const id = `browser-recording-${idempotencyKey}`;
     const extension = mimeType.includes("mp4") ? "mp4" : "webm";
     const artifactPath = path.join(resolvedArtifactDirectory, `${id}.${extension}`);
     yield* fileSystem.makeDirectory(resolvedArtifactDirectory, { recursive: true }).pipe(
@@ -2997,9 +3001,10 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
     tabId: string,
     mimeType: string,
     data: Uint8Array,
+    idempotencyKey: string,
     timeoutMs?: number,
   ) {
-    const save = performSaveRecording(tabId, mimeType, data);
+    const save = performSaveRecording(tabId, mimeType, data, idempotencyKey);
     if (timeoutMs === undefined) return yield* save;
     const result = yield* save.pipe(Effect.timeoutOption(automationExecutionBudget(timeoutMs)));
     if (Option.isSome(result)) return result.value;
@@ -4376,6 +4381,7 @@ export class PreviewManager extends Context.Service<
       tabId: string,
       mimeType: string,
       data: Uint8Array,
+      idempotencyKey: string,
       timeoutMs?: number,
     ) => Effect.Effect<DesktopPreviewRecordingArtifact, PreviewManagerError>;
     readonly automationStatus: (

apps/web/src/browser/browserRecording.test.ts

@@ -39,14 +39,22 @@ const {
         value.tabIds.size === 0 ? "clear" : `publish:${Array.from(value.tabIds).join(",")}`,
       );
     }),
-    save: vi.fn(async (tabId: string) => ({
-      id: "recording-test",
-      tabId,
-      path: "/tmp/recording-test.webm",
-      mimeType: "video/webm" as const,
-      sizeBytes: 0,
-      createdAt: "2026-06-26T00:00:00.000Z",
-    })),
+    save: vi.fn(
+      async (
+        tabId: string,
+        _mimeType?: string,
+        _data?: Uint8Array,
+        _idempotencyKey?: string,
+        _timeoutMs?: number,
+      ) => ({
+        id: "recording-test",
+        tabId,
+        path: "/tmp/recording-test.webm",
+        mimeType: "video/webm" as const,
+        sizeBytes: 0,
+        createdAt: "2026-06-26T00:00:00.000Z",
+      }),
+    ),
     startScreencast: vi.fn(async (tabId: string) => {
       events.push("start-screencast");
       const surface = surfaceState.byTabId[tabId] as
@@ -237,7 +245,30 @@ describe("browser recording", () => {
 
     await rejection;
     expect(startScreencast).toHaveBeenCalledWith("recording-tab", 40);
-    expect(stopScreencast).toHaveBeenCalledWith("recording-tab");
+    expect(stopScreencast).toHaveBeenCalledWith("recording-tab", 1);
+    expect(readActiveBrowserRecordingTabIds()).toEqual(new Set());
+  });
+
+  it("clears a timed-out startup even when screencast cleanup stalls", async () => {
+    vi.useFakeTimers();
+    startScreencast.mockImplementationOnce(async () => {
+      events.push("start-screencast");
+    });
+    stopScreencast.mockImplementationOnce(
+      async () => await new Promise<undefined>(() => undefined),
+    );
+
+    const startPromise = startBrowserRecording("recording-tab", null, "recording-tab", 40);
+    const rejection = expect(startPromise).rejects.toMatchObject({
+      operation: "wait-first-frame",
+      tabId: "recording-tab",
+    });
+    await Promise.resolve();
+    await vi.advanceTimersByTimeAsync(40);
+    await vi.runOnlyPendingTimersAsync();
+
+    await rejection;
+    expect(stopScreencast).toHaveBeenCalledWith("recording-tab", 1);
     expect(readActiveBrowserRecordingTabIds()).toEqual(new Set());
   });
 
@@ -578,6 +609,29 @@ describe("browser recording", () => {
     expect(readActiveBrowserRecordingTabIds()).toEqual(new Set());
   });
 
+  it("reuses the artifact idempotency key after a desktop save timeout", async () => {
+    save.mockRejectedValueOnce({
+      _tag: "PreviewAutomationTimeoutError",
+      tabId: "recording-tab",
+      timeoutMs: 40,
+    });
+    await startBrowserRecording("recording-tab");
+
+    await expect(stopBrowserRecording("recording-tab", 40)).rejects.toMatchObject({
+      operation: "stop-deadline",
+      tabId: "recording-tab",
+    });
+    expect(readActiveBrowserRecordingTabIds()).toEqual(new Set(["recording-tab"]));
+
+    await expect(stopBrowserRecording("recording-tab")).resolves.toMatchObject({
+      tabId: "recording-tab",
+    });
+    expect(save).toHaveBeenCalledTimes(2);
+    expect(save.mock.calls[0]?.[3]).toEqual(expect.any(String));
+    expect(save.mock.calls[1]?.[3]).toBe(save.mock.calls[0]?.[3]);
+    expect(readActiveBrowserRecordingTabIds()).toEqual(new Set());
+  });
+
   it("finishes startup before stopping so an active recording yields an artifact", async () => {
     let finishStartingScreencast: (() => void) | undefined;
     startScreencast.mockImplementationOnce(async () => {