Skip to content

Commit 6716ed7

Browse files
committed
fix(preview): preserve recording retries
1 parent c87599e commit 6716ed7

5 files changed

Lines changed: 221 additions & 38 deletions

File tree

BRANCH_DETAILS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ This branch does not add a second recording/PiP capture lifecycle or another hid
2020
Expected behavior:
2121

2222
- 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.
23-
- 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.
23+
- 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.
2424
- 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.
2525
- 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.
2626
- 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
7676

7777
Current verification:
7878

79-
- 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.
79+
- 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.
8080
- 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.
8181
- 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.
8282
- 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/preview/Manager.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1861,6 +1861,89 @@ describe("PreviewManager", () => {
18611861
),
18621862
);
18631863

1864+
effectIt.effect("reapplies a bounded color scheme after the webview is replaced", () =>
1865+
withManager((manager) =>
1866+
Effect.gen(function* () {
1867+
let finishFirstGuest: (() => void) | undefined;
1868+
const firstSendCommand = vi.fn(
1869+
async (method: string, parameters?: { features?: ReadonlyArray<{ value: string }> }) => {
1870+
if (
1871+
method === "Emulation.setEmulatedMedia" &&
1872+
parameters?.features?.[0]?.value === "dark"
1873+
) {
1874+
await new Promise<void>((resolve) => {
1875+
finishFirstGuest = resolve;
1876+
});
1877+
}
1878+
return undefined;
1879+
},
1880+
);
1881+
const replacementSendCommand = vi.fn(async () => undefined);
1882+
const makeWebContents = (
1883+
id: number,
1884+
sendCommand: typeof firstSendCommand | typeof replacementSendCommand,
1885+
) => {
1886+
let attached = false;
1887+
return {
1888+
id,
1889+
isDestroyed: () => false,
1890+
isDevToolsOpened: () => false,
1891+
getType: () => "webview",
1892+
getURL: () => "https://example.com",
1893+
getTitle: () => "Example",
1894+
isLoading: () => false,
1895+
getZoomFactor: () => 1,
1896+
setZoomFactor: vi.fn(),
1897+
on: vi.fn(),
1898+
off: vi.fn(),
1899+
ipc: { on: vi.fn(), off: vi.fn() },
1900+
send: webviewSend,
1901+
navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1902+
setWindowOpenHandler: vi.fn(),
1903+
debugger: {
1904+
isAttached: () => attached,
1905+
attach: vi.fn(() => {
1906+
attached = true;
1907+
}),
1908+
detach: vi.fn(() => {
1909+
attached = false;
1910+
}),
1911+
sendCommand,
1912+
on: vi.fn(),
1913+
off: vi.fn(),
1914+
},
1915+
} as never;
1916+
};
1917+
const first = makeWebContents(42, firstSendCommand);
1918+
const replacement = makeWebContents(43, replacementSendCommand);
1919+
fromId.mockImplementation((id) => (id === 42 ? first : id === 43 ? replacement : null));
1920+
const states: PreviewManager.PreviewTabState[] = [];
1921+
yield* manager.subscribeStateChanges((_tabId, state) =>
1922+
Effect.sync(() => {
1923+
states.push(state);
1924+
}),
1925+
);
1926+
yield* manager.createTab("tab_scheme_replacement");
1927+
yield* manager.registerWebview("tab_scheme_replacement", 42);
1928+
yield* Effect.yieldNow;
1929+
1930+
const mutation = yield* manager
1931+
.setColorScheme("tab_scheme_replacement", "dark", 1_000)
1932+
.pipe(Effect.forkChild({ startImmediately: true }));
1933+
yield* Effect.yieldNow;
1934+
yield* manager.registerWebview("tab_scheme_replacement", 43);
1935+
yield* Effect.yieldNow;
1936+
finishFirstGuest?.();
1937+
yield* Fiber.join(mutation);
1938+
1939+
expect(replacementSendCommand).toHaveBeenCalledWith("Emulation.setEmulatedMedia", {
1940+
features: [{ name: "prefers-color-scheme", value: "dark" }],
1941+
});
1942+
expect(states.at(-1)?.colorScheme).toBe("dark");
1943+
}),
1944+
),
1945+
);
1946+
18641947
effectIt.effect("blocks late webview and capture starts during tab close", () =>
18651948
withManager((manager) =>
18661949
Effect.gen(function* () {

apps/desktop/src/preview/Manager.ts

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2338,30 +2338,46 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
23382338
return yield* new PreviewTabNotFoundError({ tabId });
23392339
}
23402340
if (timeoutMs !== undefined) {
2341-
const wc = yield* requireWebContents(tabId);
2342-
yield* withControlSession(
2343-
tabId,
2344-
wc,
2345-
"set-color-scheme",
2346-
(send) =>
2347-
send("Emulation.setEmulatedMedia", {
2348-
features: [
2349-
{
2350-
name: "prefers-color-scheme",
2351-
value: colorScheme === "system" ? "" : colorScheme,
2352-
},
2353-
],
2354-
}),
2355-
timeoutMs,
2356-
);
2357-
const currentTab = (yield* SynchronizedRef.get(tabsRef)).get(tabId);
2358-
if (!currentTab) {
2359-
return yield* new PreviewTabNotFoundError({ tabId });
2360-
}
2361-
if (currentTab.colorScheme !== colorScheme) {
2362-
yield* update(tabId, { colorScheme });
2341+
const deadline = (yield* currentMillis) + timeoutMs;
2342+
let target = yield* requireWebContents(tabId);
2343+
while (true) {
2344+
const remainingTimeoutMs = deadline - (yield* currentMillis);
2345+
if (remainingTimeoutMs <= 0) {
2346+
return yield* new PreviewAutomationTimeoutError({ tabId, timeoutMs });
2347+
}
2348+
yield* withControlSession(
2349+
tabId,
2350+
target,
2351+
"set-color-scheme",
2352+
(send) =>
2353+
send("Emulation.setEmulatedMedia", {
2354+
features: [
2355+
{
2356+
name: "prefers-color-scheme",
2357+
value: colorScheme === "system" ? "" : colorScheme,
2358+
},
2359+
],
2360+
}),
2361+
remainingTimeoutMs,
2362+
);
2363+
const currentTab = (yield* SynchronizedRef.get(tabsRef)).get(tabId);
2364+
if (!currentTab) {
2365+
return yield* new PreviewTabNotFoundError({ tabId });
2366+
}
2367+
if (currentTab.webContentsId !== target.id) {
2368+
target = yield* requireWebContents(tabId);
2369+
continue;
2370+
}
2371+
if (currentTab.colorScheme !== colorScheme) {
2372+
yield* update(tabId, { colorScheme });
2373+
}
2374+
const appliedTab = (yield* SynchronizedRef.get(tabsRef)).get(tabId);
2375+
if (!appliedTab) {
2376+
return yield* new PreviewTabNotFoundError({ tabId });
2377+
}
2378+
if (appliedTab.webContentsId === target.id) return;
2379+
target = yield* requireWebContents(tabId);
23632380
}
2364-
return;
23652381
}
23662382
if (tab.colorScheme !== colorScheme) {
23672383
// Record the choice even when the CDP call below can't run yet (no

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,58 @@ describe("browser recording", () => {
526526
expect(readActiveBrowserRecordingTabIds()).toEqual(new Set());
527527
});
528528

529+
it("shares an in-flight artifact save with a retry after the renderer deadline", async () => {
530+
vi.useFakeTimers();
531+
let finishSaving: ((artifact: Awaited<ReturnType<typeof save>>) => void) | undefined;
532+
save.mockImplementationOnce(
533+
async () =>
534+
await new Promise<Awaited<ReturnType<typeof save>>>((resolve) => {
535+
finishSaving = resolve;
536+
}),
537+
);
538+
await startBrowserRecording("recording-tab");
539+
540+
const firstStop = stopBrowserRecording("recording-tab", 40);
541+
const rejection = expect(firstStop).rejects.toMatchObject({ operation: "stop-deadline" });
542+
await vi.advanceTimersByTimeAsync(40);
543+
await rejection;
544+
545+
const retry = stopBrowserRecording("recording-tab");
546+
expect(save).toHaveBeenCalledOnce();
547+
finishSaving?.({
548+
id: "recording-test",
549+
tabId: "recording-tab",
550+
path: "/tmp/recording-test.webm",
551+
mimeType: "video/webm",
552+
sizeBytes: 0,
553+
createdAt: "2026-06-26T00:00:00.000Z",
554+
});
555+
556+
await expect(retry).resolves.toMatchObject({ id: "recording-test" });
557+
expect(save).toHaveBeenCalledOnce();
558+
expect(readActiveBrowserRecordingTabIds()).toEqual(new Set());
559+
});
560+
561+
it("keeps recording state retryable when the desktop stop deadline rejects first", async () => {
562+
stopScreencast.mockRejectedValueOnce({
563+
_tag: "PreviewAutomationTimeoutError",
564+
tabId: "recording-tab",
565+
timeoutMs: 40,
566+
});
567+
await startBrowserRecording("recording-tab");
568+
569+
await expect(stopBrowserRecording("recording-tab", 40)).rejects.toMatchObject({
570+
operation: "stop-deadline",
571+
tabId: "recording-tab",
572+
});
573+
expect(readActiveBrowserRecordingTabIds()).toEqual(new Set(["recording-tab"]));
574+
575+
await expect(stopBrowserRecording("recording-tab")).resolves.toMatchObject({
576+
tabId: "recording-tab",
577+
});
578+
expect(readActiveBrowserRecordingTabIds()).toEqual(new Set());
579+
});
580+
529581
it("finishes startup before stopping so an active recording yields an artifact", async () => {
530582
let finishStartingScreencast: (() => void) | undefined;
531583
startScreencast.mockImplementationOnce(async () => {

0 commit comments

Comments
 (0)