From 6c5a55a2c75ca5d9c6d476ef2826c2a5e3a06f8c Mon Sep 17 00:00:00 2001 From: Isaac Mayes Date: Mon, 26 Oct 2026 03:46:05 -0500 Subject: [PATCH 1/2] fix(desktop): keep preview download settings out of app exports --- apps/desktop/src/preview/CdpRelay.test.ts | 58 ++++++++++++++++--- apps/desktop/src/preview/CdpRelay.ts | 13 ++++- .../src/preview/DesktopBrowserHost.test.ts | 4 +- .../desktop/src/preview/DesktopBrowserHost.ts | 2 +- 4 files changed, 65 insertions(+), 12 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index 1c1f9dc078ca0..582b9dcbb7cb4 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -178,7 +178,7 @@ describe("CDP relay extra sessions", () => { }); describe("CDP relay downloads", () => { - it("applies the server's download directory and reports downloads on the root session", async () => { + it("captures preview downloads without changing the default browser context", async () => { const send = vi.fn(async () => ({})); const directories: Array = []; const written: Array> = []; @@ -201,15 +201,59 @@ describe("CDP relay downloads", () => { ); await settle(); expect(directories).toEqual(["/srv/artifacts"]); - expect(send).toHaveBeenCalledWith( - "Browser.setDownloadBehavior", - { behavior: "allowAndName", downloadPath: "/srv/artifacts", eventsEnabled: true }, - undefined, - ); + expect(send.mock.calls).toEqual([ + ["Page.setDownloadBehavior", { behavior: "default" }, undefined], + ]); written.length = 0; - relay.event("Browser.downloadWillBegin", { guid: "g1", suggestedFilename: "r.csv" }, ""); + relay.event("Page.downloadWillBegin", { guid: "g1", suggestedFilename: "r.csv" }, ""); + relay.event("Page.downloadProgress", { guid: "g1", state: "completed" }, ""); expect(written).toEqual([ { method: "Browser.downloadWillBegin", params: { guid: "g1", suggestedFilename: "r.csv" } }, + { method: "Browser.downloadProgress", params: { guid: "g1", state: "completed" } }, + ]); + }); + + it("keeps denial scoped to the preview and restores Electron's save handling when allowed", async () => { + const send = vi.fn(async () => ({})); + const directories: Array = []; + const relay = createCdpRelayConnection( + makeTarget(send, (directory) => directories.push(directory)), + () => {}, + ); + for (const [id, behavior] of ["deny", "allowAndName"].entries()) { + relay.receive( + JSON.stringify({ + id, + method: "Browser.setDownloadBehavior", + params: { behavior, downloadPath: "/srv/artifacts" }, + }), + ); + await settle(); + } + expect(directories).toEqual([null, "/srv/artifacts"]); + expect(send.mock.calls).toEqual([ + ["Page.setDownloadBehavior", { behavior: "deny" }, undefined], + ["Page.setDownloadBehavior", { behavior: "default" }, undefined], ]); }); + + it("does not publish download events when the connection has disabled them", async () => { + const written: Array> = []; + const relay = createCdpRelayConnection(makeTarget(vi.fn(async () => ({}))), (raw) => + written.push(JSON.parse(raw)), + ); + relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach", params: {} })); + relay.receive( + JSON.stringify({ + id: 2, + method: "Browser.setDownloadBehavior", + params: { behavior: "allowAndName", downloadPath: "/srv/artifacts", eventsEnabled: false }, + }), + ); + await settle(); + written.length = 0; + relay.event("Page.downloadWillBegin", { guid: "g1" }, ""); + relay.event("Page.downloadProgress", { guid: "g1", state: "completed" }, ""); + expect(written).toEqual([]); + }); }); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index 948bc361f5d92..5dfef8232003e 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -53,6 +53,7 @@ export function createCdpRelayConnection( ): CdpRelayConnection { const send = (message: Record) => write(JSON.stringify(message)); let attached = false; + let downloadEventsEnabled = false; /** Relay-created page sessions share one real debugger; child frame sessions retain their own ids. */ const sessions = new Map([[PAGE_SESSION_ID, "page"]]); let sessionSequence = 0; @@ -78,8 +79,13 @@ export function createCdpRelayConnection( const directory = command.params?.["downloadPath"]; const allowed = command.params?.["behavior"] !== "deny" && typeof directory === "string"; target.setDownloadDirectory(allowed ? directory : null); - const { browserContextId: _ignored, ...params } = command.params ?? {}; - return target.send("Browser.setDownloadBehavior", params, undefined); + downloadEventsEnabled = command.params?.["eventsEnabled"] === true; + // Browser-scoped behavior targets Electron's app session; preview paths belong to will-download. + return target.send( + "Page.setDownloadBehavior", + { behavior: command.params?.["behavior"] === "deny" ? "deny" : "default" }, + undefined, + ); } case "Browser.getVersion": return { @@ -186,9 +192,10 @@ export function createCdpRelayConnection( }, event: (method, params, sessionId) => { if (!attached) return; + if (method.startsWith("Page.download")) method = method.replace("Page.", "Browser."); // Download events are the browser's; Playwright listens on its root session. if (method.startsWith("Browser.download")) { - send({ method, params }); + if (downloadEventsEnabled) send({ method, params }); return; } // Electron reports the page's own events with an empty session id. diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 08fefd8d733f5..e3470027a45b7 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -934,9 +934,11 @@ describe("DesktopBrowserHost", () => { }), }), ); - debuggee.emit("Browser.downloadWillBegin", { guid: "guid-1", suggestedFilename: "r.csv" }); + debuggee.emit("Page.downloadWillBegin", { guid: "guid-1", suggestedFilename: "r.csv" }); + expect(host.placeDownload(makeDebuggee().tab.webContents, item)).toBe(false); expect(host.placeDownload(debuggee.tab.webContents, item)).toBe(true); expect(paths).toEqual(["/srv/downloads/guid-1"]); + expect(host.placeDownload(debuggee.tab.webContents, item)).toBe(false); }), ); diff --git a/apps/desktop/src/preview/DesktopBrowserHost.ts b/apps/desktop/src/preview/DesktopBrowserHost.ts index 30f252e3a677e..6de2b35cc4a56 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -422,7 +422,7 @@ export const make = Effect.gen(function* () { focusReleasing: false, screencasts: new Set(), onMessage: (_event, method, params, sessionId) => { - if (method === "Browser.downloadWillBegin") { + if (method === "Page.downloadWillBegin") { const guid = (params as { guid?: unknown } | undefined)?.guid; tab.pendingDownloadGuid = typeof guid === "string" ? guid : null; } From 3dabba59bfb8327bd95c972d4857f5eb872aff68 Mon Sep 17 00:00:00 2001 From: Isaac Mayes Date: Mon, 26 Oct 2026 04:55:56 -0500 Subject: [PATCH 2/2] fix(desktop): preserve download event state on rejected commands --- apps/desktop/src/preview/CdpRelay.test.ts | 40 +++++++++++++++++++++++ apps/desktop/src/preview/CdpRelay.ts | 5 +-- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index 582b9dcbb7cb4..9270f5fc84631 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -256,4 +256,44 @@ describe("CDP relay downloads", () => { relay.event("Page.downloadProgress", { guid: "g1", state: "completed" }, ""); expect(written).toEqual([]); }); + + it.each([false, true])( + "keeps the previous download event setting (%s) when configuration fails", + async (enabled) => { + const send = vi + .fn(async () => ({})) + .mockResolvedValueOnce({}) + .mockRejectedValueOnce(new Error("Page command rejected")); + const written: Array> = []; + const relay = createCdpRelayConnection(makeTarget(send), (raw) => + written.push(JSON.parse(raw)), + ); + relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach", params: {} })); + for (const [id, eventsEnabled] of [enabled, !enabled].entries()) { + relay.receive( + JSON.stringify({ + id: id + 2, + method: "Browser.setDownloadBehavior", + params: { behavior: "allowAndName", downloadPath: "/srv/artifacts", eventsEnabled }, + }), + ); + await settle(); + } + expect(written).toContainEqual({ + id: 3, + error: { code: -32000, message: "Page command rejected" }, + }); + written.length = 0; + relay.event("Page.downloadWillBegin", { guid: "g1" }, ""); + relay.event("Page.downloadProgress", { guid: "g1", state: "completed" }, ""); + expect(written).toEqual( + enabled + ? [ + { method: "Browser.downloadWillBegin", params: { guid: "g1" } }, + { method: "Browser.downloadProgress", params: { guid: "g1", state: "completed" } }, + ] + : [], + ); + }, + ); }); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index 5dfef8232003e..70bfcb64f4e0c 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -79,13 +79,14 @@ export function createCdpRelayConnection( const directory = command.params?.["downloadPath"]; const allowed = command.params?.["behavior"] !== "deny" && typeof directory === "string"; target.setDownloadDirectory(allowed ? directory : null); - downloadEventsEnabled = command.params?.["eventsEnabled"] === true; // Browser-scoped behavior targets Electron's app session; preview paths belong to will-download. - return target.send( + const result = await target.send( "Page.setDownloadBehavior", { behavior: command.params?.["behavior"] === "deny" ? "deny" : "default" }, undefined, ); + downloadEventsEnabled = command.params?.["eventsEnabled"] === true; + return result; } case "Browser.getVersion": return {