From 6453ccdc5c652b80a91669f5854fef1770a879f9 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 22 Oct 2026 09:36:16 +0000 Subject: [PATCH 1/7] test(server): reproduces issue 23513 Closing a server preview tab skips the page's beforeunload handlers, and an unsaved-changes guard must not leave the tab open behind its dialog. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 79 ++++++++++++++++++- 1 file changed, 76 insertions(+), 3 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index ac4a785f207f3..56480b0a700c0 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -183,11 +183,34 @@ function makeContext(onClose?: (context: BrowserContext) => void) { throw new Error("Unexpected locator action"); }), isClosed: () => closed, - close: vi.fn(async () => { - if (!closed) { + // Models Playwright: unload handlers always run, but `beforeunload` only with + // `runBeforeUnload`, and a guarding page stays open behind a `beforeunload` dialog. + beforeUnload: "none" as "none" | "listen" | "guard", + unloadEvents: [] as string[], + beforeUnloadDialog: null as null | { + accept: ReturnType; + dismiss: ReturnType; + }, + close: vi.fn(async (options?: { runBeforeUnload?: boolean }) => { + if (closed) return; + const finish = () => { + if (closed) return; + page.unloadEvents.push("pagehide", "visibilitychange"); closed = true; events.emit("close"); - } + }; + if (!options?.runBeforeUnload || page.beforeUnload === "none") return finish(); + page.unloadEvents.push("beforeunload"); + if (page.beforeUnload === "listen") return finish(); + const dialog = { + type: () => "beforeunload", + message: () => "", + defaultValue: () => "", + accept: vi.fn(async () => finish()), + dismiss: vi.fn(async () => {}), + }; + page.beforeUnloadDialog = dialog; + events.emit("dialog", dialog); }), }; const context = { @@ -673,6 +696,56 @@ it.live("reports a pending dialog without evaluating the page and resolves it ex ).pipe(Effect.provide(layer)), ); +it.live("closing a tab runs the page's beforeunload handlers", () => + Effect.scoped( + Effect.gen(function* () { + const { broker, tabId } = yield* ready; + const page = contexts[0]!.page; + page.beforeUnload = "listen"; + yield* broker.invoke({ scope, tabId, operation: "close", input: {} }); + expect(page.isClosed()).toBe(true); + expect(page.unloadEvents).toEqual(["beforeunload", "pagehide", "visibilitychange"]); + }), + ).pipe(Effect.provide(layer)), +); + +it.live("closing a tab with an unsaved-changes guard still closes it", () => + Effect.scoped( + Effect.gen(function* () { + const browser = yield* ServerBrowser.ServerBrowser; + const broker = yield* Broker.PreviewAutomationBroker; + yield* Effect.yieldNow; + // A profile context is shared, so closing the tab cannot fall back to closing its context. + yield* browser.reportProfiles({ + profiles: [{ id: "profile-work", name: "Work", kind: "persistent" }], + defaultProfileId: "profile-work", + }); + const opened = yield* broker.invoke({ + scope, + operation: "open", + input: { reuseExistingTab: false, show: false, profileId: "profile-work" }, + }); + const tabId = PreviewTabId.make(opened.tabId!); + expect(contextRequests).toEqual([{ profileId: "profile-work", isolated: false }]); + const page = contexts[0]!.page; + page.beforeUnload = "guard"; + const closed = new Promise((resolve) => page.once("close", () => resolve())); + yield* broker.invoke({ scope, tabId, operation: "close", input: {} }); + expect(page.beforeUnloadDialog).not.toBeNull(); + // Without this bound, a close stuck behind the unanswered dialog only fails at the suite timeout. + yield* Effect.promise(() => closed).pipe( + Effect.timeoutOrElse({ + duration: "2 seconds", + orElse: () => + Effect.die(new Error("The tab stayed open behind its beforeunload dialog.")), + }), + ); + expect(page.beforeUnloadDialog!.accept).toHaveBeenCalledTimes(1); + expect(page.unloadEvents).toEqual(["beforeunload", "pagehide", "visibilitychange"]); + }), + ).pipe(Effect.provide(layer)), +); + it.live("a tab opened to a file the browser cannot show reports the file to download", () => Effect.scoped( Effect.gen(function* () { From 78a63776799e696e6837d12301d05c23ede2667e Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 22 Oct 2026 09:40:25 +0000 Subject: [PATCH 2/7] fix(server): closing a server preview tab runs its beforeunload handlers Closing a headless preview tab skipped the page's beforeunload handlers. It now closes with runBeforeUnload, accepts any beforeunload guard dialog on a page that is already closing, and closes an isolated context only after its page has closed. The guard test now checks the result directly instead of waiting behind a timeout. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 11 ++-------- apps/server/src/preview/ServerBrowser.ts | 20 +++++++++++++++++-- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 56480b0a700c0..d90e4f31aabed 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -729,18 +729,11 @@ it.live("closing a tab with an unsaved-changes guard still closes it", () => expect(contextRequests).toEqual([{ profileId: "profile-work", isolated: false }]); const page = contexts[0]!.page; page.beforeUnload = "guard"; - const closed = new Promise((resolve) => page.once("close", () => resolve())); yield* broker.invoke({ scope, tabId, operation: "close", input: {} }); + // The mock raises the dialog inside `close`, so an unanswered guard leaves the page open here. expect(page.beforeUnloadDialog).not.toBeNull(); - // Without this bound, a close stuck behind the unanswered dialog only fails at the suite timeout. - yield* Effect.promise(() => closed).pipe( - Effect.timeoutOrElse({ - duration: "2 seconds", - orElse: () => - Effect.die(new Error("The tab stayed open behind its beforeunload dialog.")), - }), - ); expect(page.beforeUnloadDialog!.accept).toHaveBeenCalledTimes(1); + expect(page.isClosed()).toBe(true); expect(page.unloadEvents).toEqual(["beforeunload", "pagehide", "visibilitychange"]); }), ).pipe(Effect.provide(layer)), diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 5b386e5f0797a..69dd375425a88 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -672,8 +672,19 @@ const make = Effect.gen(function* () { for (const viewer of tab.viewers) viewer.push({ _tag: end }); void tab.control.close().catch(constVoid); if (tab.desktop) void tab.desktop.close().catch(constVoid); - else void tab.page.close().catch(constVoid); - if (tab.isolatedContext) void tab.page.context().close().catch(constVoid); + else { + const page = tab.page; + // `runBeforeUnload` returns before the page closes, so closing an isolated context + // must wait for it. A pending dialog would block `beforeunload` from running at all. + const closed = page.isClosed() + ? Promise.resolve() + : new Promise((resolve) => page.once("close", () => resolve())); + void page + .close({ runBeforeUnload: tab.dialog === null }) + .catch(() => page.close()) + .catch(constVoid); + if (tab.isolatedContext) void closed.then(() => page.context().close()).catch(constVoid); + } void tab.recording?.encoder.close().catch(constVoid); void NodeFSP.rm(downloadDir(tab), { recursive: true, force: true }).catch(constVoid); reportLiveTabs(); @@ -871,6 +882,11 @@ const make = Effect.gen(function* () { }); }); page.on("dialog", (dialog) => { + // Closing is already decided, so an unsaved-changes guard must not keep the page open. + if (tab.closing && dialog.type() === "beforeunload") { + void dialog.accept().catch(constVoid); + return; + } tab.dialog = dialog; broadcastControl(tab); }); From bbf5c294c914be640fd445681fc4031846aa041c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 22 Oct 2026 09:47:28 +0000 Subject: [PATCH 3/7] test(server): isolated tab context closes only after its page closes Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index d90e4f31aabed..0974e2dcc6484 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -191,6 +191,9 @@ function makeContext(onClose?: (context: BrowserContext) => void) { accept: ReturnType; dismiss: ReturnType; }, + // Set to hold the page open after `beforeunload` until resolved, as Playwright's + // `runBeforeUnload` close resolves before the page actually closes. + unloadGate: null as PromiseWithResolvers | null, close: vi.fn(async (options?: { runBeforeUnload?: boolean }) => { if (closed) return; const finish = () => { @@ -201,6 +204,10 @@ function makeContext(onClose?: (context: BrowserContext) => void) { }; if (!options?.runBeforeUnload || page.beforeUnload === "none") return finish(); page.unloadEvents.push("beforeunload"); + if (page.beforeUnload === "listen" && page.unloadGate) { + void page.unloadGate.promise.then(finish); + return; + } if (page.beforeUnload === "listen") return finish(); const dialog = { type: () => "beforeunload", @@ -225,9 +232,11 @@ function makeContext(onClose?: (context: BrowserContext) => void) { sessions.push(session); return session; }, + closing: Promise.withResolvers(), close: vi.fn(async () => { if (contextClosed) return; contextClosed = true; + context.closing.resolve(); await page.close(); onClose?.(context as unknown as BrowserContext); }), @@ -709,6 +718,25 @@ it.live("closing a tab runs the page's beforeunload handlers", () => ).pipe(Effect.provide(layer)), ); +it.live("closing an isolated tab closes its context only after the page closes", () => + Effect.scoped( + Effect.gen(function* () { + const { broker, tabId } = yield* ready; + const context = contexts[0]!; + const page = context.page; + page.beforeUnload = "listen"; + page.unloadGate = Promise.withResolvers(); + yield* broker.invoke({ scope, tabId, operation: "close", input: {} }); + expect(context.close).not.toHaveBeenCalled(); + expect(page.isClosed()).toBe(false); + page.unloadGate.resolve(); + yield* Effect.promise(() => context.closing.promise); + expect(page.isClosed()).toBe(true); + expect(page.unloadEvents).toEqual(["beforeunload", "pagehide", "visibilitychange"]); + }), + ).pipe(Effect.provide(layer)), +); + it.live("closing a tab with an unsaved-changes guard still closes it", () => Effect.scoped( Effect.gen(function* () { From 2fe77f4048a1295a21f89581691a825e761319d3 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 22 Oct 2026 09:58:54 +0000 Subject: [PATCH 4/7] fix(server): a closing tab whose page never finishes unloading is force-closed Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 21 +++++++++++++++++++ apps/server/src/preview/ServerBrowser.ts | 4 ++++ 2 files changed, 25 insertions(+) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 0974e2dcc6484..e41a80d66f504 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -737,6 +737,27 @@ it.live("closing an isolated tab closes its context only after the page closes", ).pipe(Effect.provide(layer)), ); +it.live("closing a tab force-closes a page that never finishes unloading", () => + Effect.scoped( + Effect.gen(function* () { + const { broker, tabId } = yield* ready; + const context = contexts[0]!; + const page = context.page; + page.beforeUnload = "listen"; + page.unloadGate = Promise.withResolvers(); + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + yield* broker.invoke({ scope, tabId, operation: "close", input: {} }); + vi.advanceTimersByTime(4_999); + expect(page.isClosed()).toBe(false); + expect(context.close).not.toHaveBeenCalled(); + vi.advanceTimersByTime(1); + expect(page.isClosed()).toBe(true); + yield* Effect.promise(() => context.closing.promise); + expect(vi.getTimerCount()).toBe(0); + }).pipe(Effect.ensuring(Effect.sync(() => vi.useRealTimers()))), + ).pipe(Effect.provide(layer)), +); + it.live("closing a tab with an unsaved-changes guard still closes it", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 69dd375425a88..e212e495fd6f3 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -350,6 +350,7 @@ const IDLE_SWEEP_INTERVAL = "1 minute"; const DOWNLOAD_LIMIT = 20; const ABORTED_NAVIGATION_DOWNLOAD_WAIT_MS = 5_000; +const PAGE_CLOSE_GRACE_MS = 5_000; const DOWNLOAD_MAX_BYTES = 1024 * 1024 * 1024; const tabKey = (threadId: string, tabId: string) => `${threadId}\u0000${tabId}`; @@ -679,6 +680,9 @@ const make = Effect.gen(function* () { const closed = page.isClosed() ? Promise.resolve() : new Promise((resolve) => page.once("close", () => resolve())); + // A renderer that never finishes unloading would outlive its tab, so force it closed. + const forceClose = setTimeout(() => void page.close().catch(constVoid), PAGE_CLOSE_GRACE_MS); + void closed.then(() => clearTimeout(forceClose)); void page .close({ runBeforeUnload: tab.dialog === null }) .catch(() => page.close()) From d9a424e258d39138daa679f1ccd8ec2d6a6a8149 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 11 Nov 2026 01:29:31 +0000 Subject: [PATCH 5/7] test(server): drop redundant beforeunload test, follow profileReporter API AI-assisted; verified with the tests and tools listed above. --- apps/server/src/preview/ServerBrowser.test.ts | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index a60b85ad13049..3abc49b41ad15 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -1105,19 +1105,6 @@ it.live("reports a pending dialog without evaluating the page and resolves it ex ).pipe(Effect.provide(layer)), ); -it.live("closing a tab runs the page's beforeunload handlers", () => - Effect.scoped( - Effect.gen(function* () { - const { broker, tabId } = yield* ready; - const page = contexts[0]!.page; - page.beforeUnload = "listen"; - yield* broker.invoke({ scope, tabId, operation: "close", input: {} }); - expect(page.isClosed()).toBe(true); - expect(page.unloadEvents).toEqual(["beforeunload", "pagehide", "visibilitychange"]); - }), - ).pipe(Effect.provide(layer)), -); - it.live("closing an isolated tab closes its context only after the page closes", () => Effect.scoped( Effect.gen(function* () { @@ -1165,7 +1152,8 @@ it.live("closing a tab with an unsaved-changes guard still closes it", () => const broker = yield* Broker.PreviewAutomationBroker; yield* Effect.yieldNow; // A profile context is shared, so closing the tab cannot fall back to closing its context. - yield* browser.reportProfiles({ + const reportProfiles = yield* browser.profileReporter; + yield* reportProfiles({ profiles: [{ id: "profile-work", name: "Work", kind: "persistent" }], defaultProfileId: "profile-work", }); From 8de5a4f5d7073466c3ececdc10b8c4b1fa6382eb Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 11 Nov 2026 01:41:52 +0000 Subject: [PATCH 6/7] fix(server): closing a desktop-rendered preview tab runs its beforeunload handlers AI-assisted; verified with the tests and tools listed above. --- apps/server/src/preview/ServerBrowser.test.ts | 37 +++++++++++++++++-- apps/server/src/preview/ServerBrowser.ts | 33 +++++++++++++---- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 3abc49b41ad15..98a32c04de933 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -67,8 +67,9 @@ vi.mock("./ServerBrowserContexts.ts", () => ({ async connectDesktopPage(endpoint: string) { const context = makeContext(undefined, desktopPageUrl); context.page.viewportSize.mockReturnValue(null); - desktopConnections.push({ endpoint, context }); - return { browser: { close: async () => {} }, page: context.page as unknown as Page }; + const browser = { close: vi.fn(async () => {}) }; + desktopConnections.push({ endpoint, context, browser }); + return { browser, page: context.page as unknown as Page }; } async close() { for (const context of contexts) await context.close(); @@ -359,7 +360,11 @@ let desktopAttachWaiting = Promise.withResolvers(); const shownDesktopTabs = new Set(); const releasedDesktopTabs: Array = []; const desktopTurns = new NodeEvents.EventEmitter(); -const desktopConnections: Array<{ endpoint: string; context: ReturnType }> = []; +const desktopConnections: Array<{ + endpoint: string; + context: ReturnType; + browser: { close: ReturnType }; +}> = []; const testThread = { threadId: ThreadId.make("browser-test-thread"), providerSessionId: "agent-a", @@ -1145,6 +1150,32 @@ it.live("closing a tab force-closes a page that never finishes unloading", () => ).pipe(Effect.provide(layer)), ); +it.live("closing a desktop tab unloads its page before the desktop removes it", () => + Effect.scoped( + Effect.gen(function* () { + const { broker } = yield* ready; + desktopRendersNext = true; + const opened = yield* broker.invoke({ + scope, + operation: "open", + input: { reuseExistingTab: false, open: false }, + }); + const tabId = PreviewTabId.make(opened.tabId!); + const { context, browser } = desktopConnections[0]!; + const page = context.page; + page.beforeUnload = "listen"; + page.unloadGate = Promise.withResolvers(); + yield* broker.invoke({ scope, tabId, operation: "close", input: {} }); + expect(page.unloadEvents).toEqual(["beforeunload"]); + expect(browser.close).not.toHaveBeenCalled(); + page.unloadGate.resolve(); + yield* Effect.promise(() => desktopReleased.promise); + expect(browser.close).toHaveBeenCalledTimes(1); + expect(page.unloadEvents).toEqual(["beforeunload", "pagehide", "visibilitychange"]); + }), + ).pipe(Effect.provide(layer)), +); + it.live("closing a tab with an unsaved-changes guard still closes it", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 2b0782e98de25..da9c29346ea0a 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -753,14 +753,26 @@ const make = Effect.gen(function* () { const end = tab.desktop && !closeSession && sessionOpen(tab) ? "reconnect" : "gone"; for (const viewer of tab.viewers) viewer.push({ _tag: end }); void tab.control.close().catch(constVoid); - if (tab.desktop) void tab.desktop.close().catch(constVoid); - else { - const page = tab.page; - // `runBeforeUnload` returns before the page closes, so closing an isolated context - // must wait for it. A pending dialog would block `beforeunload` from running at all. - const closed = page.isClosed() - ? Promise.resolve() - : new Promise((resolve) => page.once("close", () => resolve())); + const page = tab.page; + // `runBeforeUnload` returns before the page closes, so closing an isolated context + // must wait for it. A pending dialog would block `beforeunload` from running at all. + const closed = page.isClosed() + ? Promise.resolve() + : new Promise((resolve) => page.once("close", () => resolve())); + if (tab.desktop) { + const desktop = tab.desktop; + if (end === "reconnect") void desktop.close().catch(constVoid); + else { + // Stay connected while the departing page unloads, so its close-time guard is accepted. + const grace = new Promise((resolve) => { + const timeout = setTimeout(resolve, PAGE_CLOSE_GRACE_MS); + void closed.then(() => clearTimeout(timeout)); + }); + void Promise.race([closed, grace]) + .then(() => desktop.close()) + .catch(constVoid); + } + } else { // A renderer that never finishes unloading would outlive its tab, so force it closed. const forceClose = setTimeout(() => void page.close().catch(constVoid), PAGE_CLOSE_GRACE_MS); void closed.then(() => clearTimeout(forceClose)); @@ -2174,6 +2186,11 @@ const make = Effect.gen(function* () { reason: "humanControl", }); void tab.control.close().catch(constVoid); + // Closing the session makes the desktop remove its webview without unloading the page. + if (tab.desktop && tab.dialog === null) { + tab.closing = true; + await tab.page.close({ runBeforeUnload: true }).catch(constVoid); + } await runPromise(manager.close({ threadId: tab.threadId, tabId: tab.tabId })); dropTab(tab, false); return {}; From 4332ed468f87a6a5d7074f92227a4e8a6f6410bd Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 11 Nov 2026 02:09:07 +0000 Subject: [PATCH 7/7] fix(server): a closing desktop preview tab finishes unloading before its webview is removed AI-assisted; verified with the tests and tools listed above. --- apps/server/src/preview/ServerBrowser.test.ts | 37 ++++++++++--- apps/server/src/preview/ServerBrowser.ts | 53 +++++++++++-------- 2 files changed, 61 insertions(+), 29 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 98a32c04de933..94d2690a5820f 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -235,11 +235,20 @@ function makeContext(onClose?: (context: BrowserContext) => void, initialUrl = " // Set to hold the page open after `beforeunload` until resolved, as Playwright's // `runBeforeUnload` close resolves before the page actually closes. unloadGate: null as PromiseWithResolvers | null, + removed: false, + pagehideBinding: null as (() => void) | null, + pagehideWatched: Promise.withResolvers(), + exposeFunction: vi.fn(async (_name: string, callback: () => void) => { + page.pagehideBinding = callback; + page.pagehideWatched.resolve(); + }), close: vi.fn(async (options?: { runBeforeUnload?: boolean }) => { if (closed) return; const finish = () => { - if (closed) return; - page.unloadEvents.push("pagehide", "visibilitychange"); + if (closed || page.removed) return; + page.unloadEvents.push("pagehide"); + page.pagehideBinding?.(); + page.unloadEvents.push("visibilitychange"); closed = true; events.emit("close"); }; @@ -1150,10 +1159,11 @@ it.live("closing a tab force-closes a page that never finishes unloading", () => ).pipe(Effect.provide(layer)), ); -it.live("closing a desktop tab unloads its page before the desktop removes it", () => +it.live("closing a desktop tab waits for its page to unload before the desktop removes it", () => Effect.scoped( Effect.gen(function* () { const { broker } = yield* ready; + const manager = yield* Manager.PreviewManager; desktopRendersNext = true; const opened = yield* broker.invoke({ scope, @@ -1163,15 +1173,28 @@ it.live("closing a desktop tab unloads its page before the desktop removes it", const tabId = PreviewTabId.make(opened.tabId!); const { context, browser } = desktopConnections[0]!; const page = context.page; + // The desktop removes the webview once the session closes, dropping an unload in progress. + const events = yield* manager.subscribeEvents; + const sessionClosed = Promise.withResolvers(); + yield* Effect.gen(function* () { + while ((yield* PubSub.take(events)).type !== "closed"); + page.removed = true; + sessionClosed.resolve(); + }).pipe(Effect.forkScoped); page.beforeUnload = "listen"; page.unloadGate = Promise.withResolvers(); - yield* broker.invoke({ scope, tabId, operation: "close", input: {} }); - expect(page.unloadEvents).toEqual(["beforeunload"]); + const closing = yield* broker + .invoke({ scope, tabId, operation: "close", input: {} }) + .pipe(Effect.forkScoped); + // The page finishes unloading once the server listens for it, or too late if it never does. + yield* Effect.promise(() => + Promise.race([page.pagehideWatched.promise, sessionClosed.promise]), + ); expect(browser.close).not.toHaveBeenCalled(); page.unloadGate.resolve(); - yield* Effect.promise(() => desktopReleased.promise); - expect(browser.close).toHaveBeenCalledTimes(1); + yield* Fiber.join(closing); expect(page.unloadEvents).toEqual(["beforeunload", "pagehide", "visibilitychange"]); + expect(browser.close).toHaveBeenCalledTimes(1); }), ).pipe(Effect.provide(layer)), ); diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index da9c29346ea0a..92ac4c6cda24b 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -743,6 +743,28 @@ const make = Effect.gen(function* () { const sessionOpen = (tab: ServerTab) => !closedSessions.has(tabKey(tab.threadId, tab.tabId)); + // Closing the session makes the desktop destroy its webview, which drops an unload still in + // progress, and `runBeforeUnload` resolves before it runs. So wait for the page's `pagehide`. + const unloadDesktopPage = async (page: Page) => { + const unloaded = Promise.withResolvers(); + const timeout = setTimeout(unloaded.resolve, PAGE_CLOSE_GRACE_MS); + const binding = `__t3PreviewUnloaded${NodeCrypto.randomUUID().replaceAll("-", "")}`; + void (async () => { + await page.exposeFunction(binding, unloaded.resolve); + await page.evaluate((name) => { + const target = globalThis as unknown as { + addEventListener: (type: string, listener: () => void) => void; + }; + target.addEventListener("pagehide", () => + Reflect.apply(Reflect.get(target, name), target, []), + ); + }, binding); + await page.close({ runBeforeUnload: true }); + })().catch(unloaded.resolve); + await unloaded.promise; + clearTimeout(timeout); + }; + const dropTab = (tab: ServerTab, closeSession: boolean) => { const key = tabKey(tab.threadId, tab.tabId); if (tabs.get(key) !== tab) return; @@ -753,26 +775,14 @@ const make = Effect.gen(function* () { const end = tab.desktop && !closeSession && sessionOpen(tab) ? "reconnect" : "gone"; for (const viewer of tab.viewers) viewer.push({ _tag: end }); void tab.control.close().catch(constVoid); - const page = tab.page; - // `runBeforeUnload` returns before the page closes, so closing an isolated context - // must wait for it. A pending dialog would block `beforeunload` from running at all. - const closed = page.isClosed() - ? Promise.resolve() - : new Promise((resolve) => page.once("close", () => resolve())); - if (tab.desktop) { - const desktop = tab.desktop; - if (end === "reconnect") void desktop.close().catch(constVoid); - else { - // Stay connected while the departing page unloads, so its close-time guard is accepted. - const grace = new Promise((resolve) => { - const timeout = setTimeout(resolve, PAGE_CLOSE_GRACE_MS); - void closed.then(() => clearTimeout(timeout)); - }); - void Promise.race([closed, grace]) - .then(() => desktop.close()) - .catch(constVoid); - } - } else { + if (tab.desktop) void tab.desktop.close().catch(constVoid); + else { + const page = tab.page; + // `runBeforeUnload` returns before the page closes, so closing an isolated context + // must wait for it. A pending dialog would block `beforeunload` from running at all. + const closed = page.isClosed() + ? Promise.resolve() + : new Promise((resolve) => page.once("close", () => resolve())); // A renderer that never finishes unloading would outlive its tab, so force it closed. const forceClose = setTimeout(() => void page.close().catch(constVoid), PAGE_CLOSE_GRACE_MS); void closed.then(() => clearTimeout(forceClose)); @@ -2186,10 +2196,9 @@ const make = Effect.gen(function* () { reason: "humanControl", }); void tab.control.close().catch(constVoid); - // Closing the session makes the desktop remove its webview without unloading the page. if (tab.desktop && tab.dialog === null) { tab.closing = true; - await tab.page.close({ runBeforeUnload: true }).catch(constVoid); + await unloadDesktopPage(tab.page); } await runPromise(manager.close({ threadId: tab.threadId, tabId: tab.tabId })); dropTab(tab, false);