From 7002dd254889dbfbdf54b8016c3c24d851f7e788 Mon Sep 17 00:00:00 2001 From: PR Batch Tester Date: Fri, 23 Oct 2026 10:01:30 +0000 Subject: [PATCH 1/3] fix(desktop): send a preview tab one agent screenshot at a time Chromium below 157 crashes the Electron main process when a webview guest's renderer exits while a Page.captureScreenshot surface copy is in flight (crbug.com/567321049). Snapshots that time out leave their screenshot pending in Chromium, so retries on a stalled tab pile several up, which makes the crash far likelier when that tab's renderer dies. The desktop relay now holds each guest to one screenshot at a time, and lets the queue go when the guest loads a new document, since a pending screenshot never answers after a reload or navigation. Refs #24026 Co-Authored-By: Claude Opus 5.5 --- .../src/preview/DesktopBrowserHost.test.ts | 60 ++++++++++++++++++- .../desktop/src/preview/DesktopBrowserHost.ts | 48 ++++++++++++++- 2 files changed, 103 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 04dcd22cf3b2c..896e49de33f9b 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -15,10 +15,26 @@ const decodeCdpReply = Schema.decodeUnknownSync( Schema.fromJsonString(Schema.Struct({ id: Schema.Number })), ); const key = { threadId: "thread-1", tabId: "tab-1" }; +// The relay sends after a few microtasks; one macrotask lets them all land. +// oxlint-disable-next-line t3code/no-real-time-wait-in-tests -- a zero-delay macrotask boundary, not a wait for time +const settle = () => new Promise((resolve) => setImmediate(resolve)); +const command = ( + host: DesktopBrowserHost.DesktopBrowserHost["Service"], + id: number, + method: string, +) => + host.handleCommandLine( + encodeJson({ + type: "cdp", + ...key, + message: encodeJson({ id, method, sessionId: "t3-preview-page" }), + }), + ); const makeDebuggee = () => { const emitter = new NodeEvents.EventEmitter(); const pending: Array<() => void> = []; + const sent: Array = []; const debuggee = Object.assign(emitter, { sendCommand: (method: string) => new Promise((resolve) => { @@ -26,20 +42,23 @@ const makeDebuggee = () => { resolve({ targetInfo: { targetId: "GUEST" } }); return; } + sent.push(method); pending.push(() => resolve({ method })); }), }); - const webContents = { + const webContents = Object.assign(new NodeEvents.EventEmitter(), { getURL: () => "http://localhost/", getTitle: () => "Page", getUserAgent: () => "Electron", - }; + }); return { tab: { webContents: webContents as unknown as Electron.WebContents, debugger: debuggee as unknown as Electron.Debugger, }, emit: (method: string, params: unknown) => emitter.emit("message", {}, method, params, ""), + navigate: () => webContents.emit("did-navigate"), + sent, release: () => pending.splice(0).forEach((resolve) => resolve()), }; }; @@ -89,6 +108,43 @@ describe("DesktopBrowserHost", () => { }).pipe(Effect.scoped), ); + it.effect("sends a page one screenshot at a time", () => + Effect.gen(function* () { + const host = yield* DesktopBrowserHost.make; + const debuggee = makeDebuggee(); + host.attach(key, debuggee.tab); + const screenshots = () => + debuggee.sent.filter((method) => method === "Page.captureScreenshot").length; + + yield* command(host, 1, "Page.captureScreenshot"); + yield* command(host, 2, "Page.captureScreenshot"); + yield* command(host, 3, "DOM.enable"); + yield* Effect.promise(settle); + expect(screenshots()).toBe(1); + expect(debuggee.sent).toContain("DOM.enable"); + + debuggee.release(); + yield* Effect.promise(settle); + expect(screenshots()).toBe(2); + }), + ); + + it.effect("stops waiting on a screenshot the page navigated away from", () => + Effect.gen(function* () { + const host = yield* DesktopBrowserHost.make; + const debuggee = makeDebuggee(); + host.attach(key, debuggee.tab); + + yield* command(host, 1, "Page.captureScreenshot"); + yield* command(host, 2, "Page.captureScreenshot"); + yield* command(host, 3, "Page.captureScreenshot"); + yield* Effect.promise(settle); + debuggee.navigate(); + yield* Effect.promise(settle); + expect(debuggee.sent).toEqual(["Page.captureScreenshot", "Page.captureScreenshot"]); + }), + ); + it.effect("saves a server tab's download under its CDP guid where the server asked", () => Effect.gen(function* () { const host = yield* DesktopBrowserHost.make; diff --git a/apps/desktop/src/preview/DesktopBrowserHost.ts b/apps/desktop/src/preview/DesktopBrowserHost.ts index cef42d4f35fc3..062248dd82c27 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -43,6 +43,8 @@ interface AttachedTab { readonly key: DesktopBrowserTabKey; readonly debuggee: DesktopBrowserTabDebugger; relay: CdpRelayConnection | null; + screenshotTurn: object | null; + readonly waitingScreenshots: Array<() => void>; downloadDirectory: string | null; pendingDownloadGuid: string | null; agentInputAt: number; @@ -52,6 +54,7 @@ interface AttachedTab { params: unknown, sessionId: string, ) => void; + readonly onNewDocument: () => void; } export class DesktopBrowserHost extends Context.Service< @@ -84,15 +87,45 @@ export const make = Effect.gen(function* () { const tabs = new Map(); const emit = (event: DesktopBrowserEventType) => runFork(PubSub.publish(outbox, event)); + const nextScreenshot = (tab: AttachedTab) => { + tab.screenshotTurn = null; + tab.waitingScreenshots.shift()?.(); + }; + + /** + * Chromium below 157 crashes the whole app when a guest's renderer exits with a + * screenshot copy in flight (crbug.com/567321049). Overlapping screenshots make that + * far likelier, and one the server gave up on stays pending in Chromium, so each guest + * gets one at a time. + */ + const takeScreenshot = (tab: AttachedTab, capture: () => Promise) => + new Promise((resolve, reject) => { + const start = () => { + const turn = {}; + tab.screenshotTurn = turn; + capture() + .finally(() => { + if (tab.screenshotTurn === turn) nextScreenshot(tab); + }) + .then(resolve, reject); + }; + if (tab.screenshotTurn === null) start(); + else tab.waitingScreenshots.push(start); + }); + const relayFor = (tab: AttachedTab) => { if (tab.relay) return tab.relay; const { webContents, debugger: debuggee } = tab.debuggee; + const send = (method: string, params: Record, sessionId?: string) => + sessionId === undefined + ? sendPageCommand(webContents, debuggee, method, params) + : debuggee.sendCommand(method, params, sessionId); const relay: CdpRelayConnection = createCdpRelayConnection( { send: (method, params, sessionId) => - sessionId === undefined - ? sendPageCommand(webContents, debuggee, method, params) - : debuggee.sendCommand(method, params, sessionId), + method === "Page.captureScreenshot" + ? takeScreenshot(tab, () => send(method, params, sessionId)) + : send(method, params, sessionId), targetId: () => debuggee .sendCommand("Target.getTargetInfo") @@ -134,6 +167,7 @@ export const make = Effect.gen(function* () { if (!tab) return; tabs.delete(id); tab.debuggee.debugger.off("message", tab.onMessage); + tab.debuggee.webContents.off("did-navigate", tab.onNewDocument); emit({ type: "detached", ...key }); }; @@ -145,6 +179,8 @@ export const make = Effect.gen(function* () { key, debuggee, relay: null, + screenshotTurn: null, + waitingScreenshots: [], downloadDirectory: null, pendingDownloadGuid: null, agentInputAt: Number.NEGATIVE_INFINITY, @@ -155,9 +191,15 @@ export const make = Effect.gen(function* () { } tab.relay?.event(method, params, sessionId); }, + // A screenshot pending when the page loads a new document, a reload included, never + // answers. A crashed page lands here once it reloads, well after its renderer died. + onNewDocument: () => { + if (tab.screenshotTurn !== null) nextScreenshot(tab); + }, }; tabs.set(id, tab); debuggee.debugger.on("message", tab.onMessage); + debuggee.webContents.on("did-navigate", tab.onNewDocument); emit({ type: "attached", ...key }); }; From 31543c321e44334650c41a8a013aed0a901d92f7 Mon Sep 17 00:00:00 2001 From: PR Batch Tester Date: Fri, 23 Oct 2026 10:18:34 +0000 Subject: [PATCH 2/3] fix(desktop): drop screenshots a released preview relay left queued A screenshot queued by a relay the server has since released would still run once its turn came, holding up the new relay's screenshots for a reply nobody reads. Detaching a tab now clears its relay too, so its queue empties the same way. Also checks that a screenshot superseded by a navigation can't start a second one beside the current one when it answers late. Refs #24026 Co-Authored-By: Claude Opus 5.5 --- .../src/preview/DesktopBrowserHost.test.ts | 29 +++++++++++++++++++ .../desktop/src/preview/DesktopBrowserHost.ts | 20 +++++++++---- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 896e49de33f9b..d8d00a6070cad 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -60,6 +60,7 @@ const makeDebuggee = () => { navigate: () => webContents.emit("did-navigate"), sent, release: () => pending.splice(0).forEach((resolve) => resolve()), + releaseFirst: () => pending.shift()?.(), }; }; @@ -142,6 +143,34 @@ describe("DesktopBrowserHost", () => { debuggee.navigate(); yield* Effect.promise(settle); expect(debuggee.sent).toEqual(["Page.captureScreenshot", "Page.captureScreenshot"]); + + // The superseded screenshot answering late must not start a third beside the second. + debuggee.releaseFirst(); + yield* Effect.promise(settle); + expect(debuggee.sent).toHaveLength(2); + debuggee.release(); + yield* Effect.promise(settle); + expect(debuggee.sent).toHaveLength(3); + }), + ); + + it.effect("drops screenshots a released relay left waiting", () => + Effect.gen(function* () { + const host = yield* DesktopBrowserHost.make; + const debuggee = makeDebuggee(); + host.attach(key, debuggee.tab); + + yield* command(host, 1, "Page.captureScreenshot"); + yield* command(host, 2, "Page.captureScreenshot"); + yield* host.handleCommandLine(encodeJson({ type: "release", ...key })); + yield* command(host, 3, "Page.captureScreenshot"); + yield* Effect.promise(settle); + debuggee.release(); + yield* Effect.promise(settle); + expect(debuggee.sent).toEqual(["Page.captureScreenshot", "Page.captureScreenshot"]); + debuggee.release(); + yield* Effect.promise(settle); + expect(debuggee.sent).toEqual(["Page.captureScreenshot", "Page.captureScreenshot"]); }), ); diff --git a/apps/desktop/src/preview/DesktopBrowserHost.ts b/apps/desktop/src/preview/DesktopBrowserHost.ts index 062248dd82c27..7028d16b4be2d 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -96,11 +96,20 @@ export const make = Effect.gen(function* () { * Chromium below 157 crashes the whole app when a guest's renderer exits with a * screenshot copy in flight (crbug.com/567321049). Overlapping screenshots make that * far likelier, and one the server gave up on stays pending in Chromium, so each guest - * gets one at a time. + * gets one at a time. One queued by a relay released since then is dropped unsent. */ - const takeScreenshot = (tab: AttachedTab, capture: () => Promise) => + const takeScreenshot = ( + tab: AttachedTab, + relay: CdpRelayConnection, + capture: () => Promise, + ) => new Promise((resolve, reject) => { const start = () => { + if (tab.relay !== relay) { + reject(new Error("The preview connection that asked for this screenshot closed.")); + nextScreenshot(tab); + return; + } const turn = {}; tab.screenshotTurn = turn; capture() @@ -124,7 +133,7 @@ export const make = Effect.gen(function* () { { send: (method, params, sessionId) => method === "Page.captureScreenshot" - ? takeScreenshot(tab, () => send(method, params, sessionId)) + ? takeScreenshot(tab, relay, () => send(method, params, sessionId)) : send(method, params, sessionId), targetId: () => debuggee @@ -166,6 +175,7 @@ export const make = Effect.gen(function* () { const tab = tabs.get(id); if (!tab) return; tabs.delete(id); + tab.relay = null; tab.debuggee.debugger.off("message", tab.onMessage); tab.debuggee.webContents.off("did-navigate", tab.onNewDocument); emit({ type: "detached", ...key }); @@ -191,8 +201,8 @@ export const make = Effect.gen(function* () { } tab.relay?.event(method, params, sessionId); }, - // A screenshot pending when the page loads a new document, a reload included, never - // answers. A crashed page lands here once it reloads, well after its renderer died. + // A screenshot pending when the page loads a new document, a reload included, can + // stay unanswered. A crashed page lands here once it reloads, well after its renderer died. onNewDocument: () => { if (tab.screenshotTurn !== null) nextScreenshot(tab); }, From bbc1e49be49f6405cedbe2678f8a4c382cf03725 Mon Sep 17 00:00:00 2001 From: PR Batch Tester Date: Fri, 23 Oct 2026 10:29:05 +0000 Subject: [PATCH 3/3] test(desktop): wait on replies instead of a macrotask in screenshot queue tests The screenshot queue tests now wait on the host's own reply events and on the fake debugger receiving a command, rather than a setImmediate boundary behind a lint exemption. A screenshot's reply leaves after its turn has passed on, so the reply is the point where the next send is known. The replies are watched from before the first command, since a new events subscriber releases the tab's relay. Refs #24026 Co-Authored-By: Claude Opus 5.5 --- .../src/preview/DesktopBrowserHost.test.ts | 80 ++++++++++++------- 1 file changed, 49 insertions(+), 31 deletions(-) diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index d8d00a6070cad..50039c4e4b04e 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "@effect/vitest"; import { DesktopBrowserEvent } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; +import * as Queue from "effect/Queue"; import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import * as NodeEvents from "node:events"; @@ -15,9 +16,7 @@ const decodeCdpReply = Schema.decodeUnknownSync( Schema.fromJsonString(Schema.Struct({ id: Schema.Number })), ); const key = { threadId: "thread-1", tabId: "tab-1" }; -// The relay sends after a few microtasks; one macrotask lets them all land. -// oxlint-disable-next-line t3code/no-real-time-wait-in-tests -- a zero-delay macrotask boundary, not a wait for time -const settle = () => new Promise((resolve) => setImmediate(resolve)); +/** Sends a page command whose id the fake debugger records as it receives it. */ const command = ( host: DesktopBrowserHost.DesktopBrowserHost["Service"], id: number, @@ -27,23 +26,25 @@ const command = ( encodeJson({ type: "cdp", ...key, - message: encodeJson({ id, method, sessionId: "t3-preview-page" }), + message: encodeJson({ id, method, params: { id }, sessionId: "t3-preview-page" }), }), ); const makeDebuggee = () => { const emitter = new NodeEvents.EventEmitter(); const pending: Array<() => void> = []; - const sent: Array = []; + const sent: Array = []; + const sendWaiters: Array<() => void> = []; const debuggee = Object.assign(emitter, { - sendCommand: (method: string) => + sendCommand: (method: string, params?: { id?: number }) => new Promise((resolve) => { if (method === "Target.getTargetInfo") { resolve({ targetInfo: { targetId: "GUEST" } }); return; } - sent.push(method); + sent.push(params?.id); pending.push(() => resolve({ method })); + sendWaiters.shift()?.(); }), }); const webContents = Object.assign(new NodeEvents.EventEmitter(), { @@ -58,7 +59,9 @@ const makeDebuggee = () => { }, emit: (method: string, params: unknown) => emitter.emit("message", {}, method, params, ""), navigate: () => webContents.emit("did-navigate"), + /** Ids of the commands the debugger received, in order. */ sent, + nextSend: () => new Promise((resolve) => sendWaiters.push(resolve)), release: () => pending.splice(0).forEach((resolve) => resolve()), releaseFirst: () => pending.shift()?.(), }; @@ -71,6 +74,25 @@ const takeEvents = (host: DesktopBrowserHost.DesktopBrowserHost["Service"], coun Effect.map((lines) => lines.map((line) => decodeEvent(new TextDecoder().decode(line)))), ); +/** + * Ids of the replies the host sends, in order. A screenshot's reply leaves after its turn + * has passed on, so by then the next waiting screenshot, if any, has been sent. Subscribe + * before sending commands: a new subscriber releases the tab's relay. + */ +const watchReplies = (host: DesktopBrowserHost.DesktopBrowserHost["Service"]) => + Effect.gen(function* () { + const replies = yield* Queue.unbounded(); + yield* host.events.pipe( + Stream.map((line) => decodeEvent(new TextDecoder().decode(line))), + Stream.runForEach((event) => + event.type === "cdp" ? Queue.offer(replies, decodeCdpReply(event.message).id) : Effect.void, + ), + Effect.forkScoped, + ); + yield* Effect.yieldNow; + return replies; + }); + describe("DesktopBrowserHost", () => { it.effect("announces tabs already attached to a backend that starts later", () => Effect.gen(function* () { @@ -114,20 +136,17 @@ describe("DesktopBrowserHost", () => { const host = yield* DesktopBrowserHost.make; const debuggee = makeDebuggee(); host.attach(key, debuggee.tab); - const screenshots = () => - debuggee.sent.filter((method) => method === "Page.captureScreenshot").length; + const replies = yield* watchReplies(host); yield* command(host, 1, "Page.captureScreenshot"); yield* command(host, 2, "Page.captureScreenshot"); yield* command(host, 3, "DOM.enable"); - yield* Effect.promise(settle); - expect(screenshots()).toBe(1); - expect(debuggee.sent).toContain("DOM.enable"); + expect(debuggee.sent).toEqual([1, 3]); - debuggee.release(); - yield* Effect.promise(settle); - expect(screenshots()).toBe(2); - }), + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(1); + expect(debuggee.sent).toEqual([1, 3, 2]); + }).pipe(Effect.scoped), ); it.effect("stops waiting on a screenshot the page navigated away from", () => @@ -135,23 +154,23 @@ describe("DesktopBrowserHost", () => { const host = yield* DesktopBrowserHost.make; const debuggee = makeDebuggee(); host.attach(key, debuggee.tab); + const replies = yield* watchReplies(host); yield* command(host, 1, "Page.captureScreenshot"); yield* command(host, 2, "Page.captureScreenshot"); yield* command(host, 3, "Page.captureScreenshot"); - yield* Effect.promise(settle); debuggee.navigate(); - yield* Effect.promise(settle); - expect(debuggee.sent).toEqual(["Page.captureScreenshot", "Page.captureScreenshot"]); + expect(debuggee.sent).toEqual([1, 2]); // The superseded screenshot answering late must not start a third beside the second. debuggee.releaseFirst(); - yield* Effect.promise(settle); - expect(debuggee.sent).toHaveLength(2); - debuggee.release(); - yield* Effect.promise(settle); - expect(debuggee.sent).toHaveLength(3); - }), + expect(yield* Queue.take(replies)).toBe(1); + expect(debuggee.sent).toEqual([1, 2]); + + debuggee.releaseFirst(); + expect(yield* Queue.take(replies)).toBe(2); + expect(debuggee.sent).toEqual([1, 2, 3]); + }).pipe(Effect.scoped), ); it.effect("drops screenshots a released relay left waiting", () => @@ -164,13 +183,12 @@ describe("DesktopBrowserHost", () => { yield* command(host, 2, "Page.captureScreenshot"); yield* host.handleCommandLine(encodeJson({ type: "release", ...key })); yield* command(host, 3, "Page.captureScreenshot"); - yield* Effect.promise(settle); - debuggee.release(); - yield* Effect.promise(settle); - expect(debuggee.sent).toEqual(["Page.captureScreenshot", "Page.captureScreenshot"]); + expect(debuggee.sent).toEqual([1]); + + const sent = debuggee.nextSend(); debuggee.release(); - yield* Effect.promise(settle); - expect(debuggee.sent).toEqual(["Page.captureScreenshot", "Page.captureScreenshot"]); + yield* Effect.promise(() => sent); + expect(debuggee.sent).toEqual([1, 3]); }), );