From b76cd78eec5e59de934c637fbde69e95c7f4f3d1 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Tue, 3 Nov 2026 22:08:26 +0000 Subject: [PATCH 01/24] fix(desktop): agents can drive the popup windows their desktop tabs open A window.open popup from an agent's desktop-rendered preview tab opened as a native window the agent could not see: the CDP relay exposed only the tab's own page, so Playwright never reported the popup and the server never adopted it. The relay now carries the tab's popup windows as extra targets, announced after the page with it as their opener, each routed to its own debugger. The desktop host attaches a popup's debugger only once the server asks for popups, which it does for agent-owned tabs alone, and keeps per-popup focus, screenshot, download and stream state. The server adopts these popups through adoptPopup, including ones already open when it connects, as native tabs that share the opener's connection, and publishes them streamed from their first snapshot so the desktop never loads them a second time. Refs https://github.com/pingdotgg/t3code/issues/30384 Implemented by Claude Opus 5.5 in Claude Code. Co-Authored-By: Claude Opus 5.5 --- apps/desktop/src/preview/CdpRelay.test.ts | 158 ++++++ apps/desktop/src/preview/CdpRelay.ts | 256 +++++++-- .../src/preview/DesktopBrowserHost.test.ts | 126 ++++- .../desktop/src/preview/DesktopBrowserHost.ts | 512 ++++++++++++------ apps/desktop/src/preview/Manager.ts | 1 + .../src/preview/DesktopBrowserChannel.ts | 13 +- apps/server/src/preview/Manager.ts | 5 +- apps/server/src/preview/ServerBrowser.test.ts | 92 +++- apps/server/src/preview/ServerBrowser.ts | 36 +- packages/contracts/src/desktopBrowser.ts | 2 + packages/contracts/src/preview.ts | 5 +- 11 files changed, 954 insertions(+), 252 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index d409ad8b42e09..85095b5dac270 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -170,6 +170,164 @@ describe("CDP relay extra sessions", () => { }); }); +describe("CDP relay popups", () => { + const makePopup = ( + targetId: string, + send: CdpRelayTarget["send"], + close: () => void = () => {}, + setDownloadDirectory: CdpRelayTarget["setDownloadDirectory"] = () => {}, + ): CdpRelayTarget => ({ + ...makeTarget(send, setDownloadDirectory), + targetId: async () => targetId, + url: () => `http://localhost:4719/${targetId}`, + close, + }); + const resultOf = (written: Array>, id: number) => + written.find((message) => message["id"] === id)!; + + it("announces the page's popups after it, as its popups, and routes them separately", async () => { + const pageSend = vi.fn(async () => ({})); + const earlySend = vi.fn(async () => ({})); + const lateSend = vi.fn(async () => ({})); + const closeLate = vi.fn(); + const directories: Array = []; + const written: Array> = []; + const relay = makeRelay(makeTarget(pageSend), (raw) => written.push(JSON.parse(raw))); + const early = relay.addPopup(makePopup("EARLY", earlySend)); + relay.receive( + JSON.stringify({ + id: 1, + method: "Browser.setDownloadBehavior", + params: { behavior: "allowAndName", downloadPath: "/srv/artifacts", eventsEnabled: true }, + }), + ); + relay.receive(JSON.stringify({ id: 2, method: "Target.setAutoAttach", params: {} })); + await settle(); + const late = relay.addPopup( + makePopup("LATE", lateSend, closeLate, (directory) => directories.push(directory)), + ); + await settle(); + + const announced = written.filter((m) => m["method"] === "Target.attachedToTarget"); + expect(announced.map((m) => m["params"])).toEqual([ + expect.objectContaining({ + sessionId: "t3-preview-page", + targetInfo: expect.not.objectContaining({ openerId: expect.anything() }), + }), + expect.objectContaining({ + targetInfo: expect.objectContaining({ targetId: "EARLY", openerId: "GUEST-TARGET" }), + }), + expect.objectContaining({ + targetInfo: expect.objectContaining({ targetId: "LATE", openerId: "GUEST-TARGET" }), + }), + ]); + const [, earlySession, lateSession] = announced.map( + (m) => (m["params"] as { sessionId: string }).sessionId, + ); + // A popup opened after the server chose where downloads go saves there too. + expect(directories).toEqual(["/srv/artifacts"]); + expect(lateSend).toHaveBeenCalledWith( + "Page.setDownloadBehavior", + { behavior: "default" }, + undefined, + ); + + relay.receive(JSON.stringify({ id: 3, method: "Target.getTargets" })); + relay.receive(JSON.stringify({ id: 4, method: "Target.attachToBrowserTarget" })); + await settle(); + expect( + (resultOf(written, 3)["result"] as { targetInfos: Array<{ targetId: string }> }).targetInfos, + ).toEqual([ + expect.objectContaining({ targetId: "GUEST-TARGET" }), + expect.objectContaining({ targetId: "EARLY" }), + expect.objectContaining({ targetId: "LATE" }), + ]); + const browserSession = (resultOf(written, 4)["result"] as { sessionId: string }).sessionId; + relay.receive( + JSON.stringify({ + id: 5, + method: "Target.attachToTarget", + params: { targetId: "LATE", flatten: true }, + sessionId: browserSession, + }), + ); + relay.receive( + JSON.stringify({ id: 6, method: "Target.closeTarget", params: { targetId: "GUEST-TARGET" } }), + ); + relay.receive( + JSON.stringify({ id: 7, method: "Target.attachToTarget", params: { targetId: "OTHER" } }), + ); + await settle(); + const extra = (resultOf(written, 5)["result"] as { sessionId: string }).sessionId; + expect(resultOf(written, 6)).toHaveProperty("error"); + expect(resultOf(written, 7)).toHaveProperty("error"); + + relay.receive(JSON.stringify({ id: 8, method: "Page.enable", sessionId: earlySession })); + relay.receive(JSON.stringify({ id: 9, method: "Page.startScreencast", sessionId: extra })); + relay.receive(JSON.stringify({ id: 10, method: "Page.enable", sessionId: "t3-preview-page" })); + await settle(); + expect(earlySend.mock.calls.at(-1)).toEqual(["Page.enable", {}, undefined, undefined]); + expect(lateSend.mock.calls.at(-1)).toEqual(["Page.startScreencast", {}, undefined, extra]); + expect(pageSend.mock.calls.at(-1)).toEqual(["Page.enable", {}, undefined, undefined]); + + // A frame's session belongs to the popup whose debugger attached it. + late.event( + "Target.attachedToTarget", + { sessionId: "late-frame", targetInfo: { type: "iframe" } }, + "", + ); + relay.receive(JSON.stringify({ id: 11, method: "DOM.enable", sessionId: "late-frame" })); + await settle(); + expect(lateSend.mock.calls.at(-1)).toEqual(["DOM.enable", {}, "late-frame", undefined]); + + written.length = 0; + early.event("Page.loadEventFired", { timestamp: 1 }, ""); + late.event("Page.downloadWillBegin", { guid: "g1", frameId: "LATE" }, ""); + expect(written).toEqual([ + { method: "Page.loadEventFired", params: { timestamp: 1 }, sessionId: earlySession }, + { method: "Browser.downloadWillBegin", params: { guid: "g1", frameId: "LATE" } }, + ]); + + relay.receive( + JSON.stringify({ id: 12, method: "Target.closeTarget", params: { targetId: "LATE" } }), + ); + await settle(); + expect(closeLate).toHaveBeenCalledOnce(); + written.length = 0; + late.remove(); + late.event("Page.loadEventFired", {}, ""); + expect(written).toEqual([ + { + method: "Target.detachedFromTarget", + params: { sessionId: lateSession, targetId: "LATE" }, + }, + { + method: "Target.detachedFromTarget", + params: { sessionId: extra, targetId: "LATE" }, + sessionId: browserSession, + }, + ]); + relay.receive(JSON.stringify({ id: 13, method: "Target.getTargets" })); + await settle(); + expect( + (resultOf(written, 13)["result"] as { targetInfos: Array<{ targetId: string }> }).targetInfos, + ).toHaveLength(2); + }); + + it("never announces a popup that closed before the page was", async () => { + const written: Array> = []; + const relay = makeRelay(makeTarget(vi.fn()), (raw) => written.push(JSON.parse(raw))); + relay.addPopup(makePopup("GONE", vi.fn())).remove(); + relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach", params: {} })); + await settle(); + expect(written.filter((m) => m["method"] !== undefined)).toEqual([ + expect.objectContaining({ + params: expect.objectContaining({ sessionId: "t3-preview-page" }), + }), + ]); + }); +}); + describe("CDP relay downloads", () => { it("captures preview downloads without changing the default browser context", async () => { const send = vi.fn(async () => ({})); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index 9a8e4e4c68346..a58a16d5e6b7f 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -16,11 +16,23 @@ export interface CdpRelayTarget { readonly userAgent: () => string; /** The desktop and its server share a disk; download paths need no translation. */ readonly setDownloadDirectory: (directory: string | null) => void; + /** Only a popup closes this way; the tab's own page closes with its preview tab. */ + readonly close?: () => void; +} + +type CdpRelayEvent = (method: string, params: unknown, sessionId: string | undefined) => void; + +export interface CdpRelayPopup { + readonly event: CdpRelayEvent; + /** Its window closed, or its debugger went away. */ + readonly remove: () => void; } export interface CdpRelayConnection { readonly receive: (raw: string) => void; - readonly event: (method: string, params: unknown, sessionId: string | undefined) => void; + readonly event: CdpRelayEvent; + /** A window the page opened; Playwright sees it as the page's popup. */ + readonly addPopup: (target: CdpRelayTarget) => CdpRelayPopup; } interface CdpCommand { @@ -70,6 +82,20 @@ const ACKNOWLEDGED = new Set([ "Browser.cancelDownload", ]); +interface RelayedTarget { + readonly target: CdpRelayTarget; + /** The session it is announced on. */ + readonly sessionId: string; + /** Known once announced; a closed target cannot be asked for it. */ + targetId: string | null; + removed: boolean; +} + +type RelaySession = + | { readonly kind: "browser" } + /** Relay-created page sessions share one real debugger; `parent` is the session that attached it. */ + | { readonly kind: "page"; readonly owner: RelayedTarget; readonly parent: string | undefined }; + export function createCdpRelayConnection( target: CdpRelayTarget, write: (message: string) => void, @@ -97,25 +123,71 @@ export function createCdpRelayConnection( closed = true; close(); }; - let attached = false; + let autoAttached = 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 downloads: { + readonly behavior: "deny" | "default"; + readonly directory: string | null; + } | null = null; + const page: RelayedTarget = { + target, + sessionId: PAGE_SESSION_ID, + targetId: null, + removed: false, + }; + /** The page first, then its popups in the order they opened. */ + const targets = new Set([page]); + const sessions = new Map([ + [PAGE_SESSION_ID, { kind: "page", owner: page, parent: undefined }], + ]); + /** Child frame and worker sessions, by the debugger that attached them; others are the page's. */ + const children = new Map(); let sessionSequence = 0; - const newSession = (kind: "browser" | "page") => { - const sessionId = `t3-preview-${kind}-${++sessionSequence}`; - sessions.set(sessionId, kind); + const newSession = (session: RelaySession) => { + const sessionId = `t3-preview-${session.kind}-${++sessionSequence}`; + sessions.set(sessionId, session); return sessionId; }; - const targetInfo = async () => ({ - targetId: await target.targetId(), + const announced = (relayed: RelayedTarget) => relayed.targetId !== null && !relayed.removed; + const targetInfo = async (relayed: RelayedTarget) => ({ + targetId: await relayed.target.targetId(), type: "page", - title: target.title(), - url: target.url(), + title: relayed.target.title(), + url: relayed.target.url(), attached: true, - canAccessOpener: false, + ...(relayed === page + ? { canAccessOpener: false } + : { canAccessOpener: true, openerId: await target.targetId() }), browserContextId: BROWSER_CONTEXT_ID, }); + const attachedEvent = (relayed: RelayedTarget, info: { readonly targetId: string }) => { + relayed.targetId = info.targetId; + sessions.set(relayed.sessionId, { kind: "page", owner: relayed, parent: undefined }); + return { + method: "Target.attachedToTarget", + params: { sessionId: relayed.sessionId, targetInfo: info, waitingForDebugger: false }, + }; + }; + /** Popups follow the page, one at a time, so Playwright already knows their opener. */ + let announcing = Promise.resolve(); + const announcePopup = (relayed: RelayedTarget) => { + announcing = announcing.then(() => + targetInfo(relayed).then( + (info) => { + if (!relayed.removed && relayed.targetId === null) send(attachedEvent(relayed, info)); + }, + () => {}, + ), + ); + }; + const findTarget = async (targetId: unknown) => { + for (const relayed of targets) { + if (relayed === page || announced(relayed)) { + if ((await relayed.target.targetId()) === targetId) return relayed; + } + } + return undefined; + }; const browserCommand = async (command: CdpCommand): Promise => { if (ACKNOWLEDGED.has(command.method)) return {}; @@ -123,13 +195,15 @@ export function createCdpRelayConnection( case "Browser.setDownloadBehavior": { const directory = command.params?.["downloadPath"]; const allowed = command.params?.["behavior"] !== "deny" && typeof directory === "string"; + const behavior = command.params?.["behavior"] === "deny" ? "deny" : "default"; // Browser-scoped behavior targets Electron's app session; preview paths belong to will-download. - const result = await target.send( - "Page.setDownloadBehavior", - { behavior: command.params?.["behavior"] === "deny" ? "deny" : "default" }, - undefined, + const [result] = await Promise.all( + [...targets].map((relayed) => + relayed.target.send("Page.setDownloadBehavior", { behavior }, undefined), + ), ); - target.setDownloadDirectory(allowed ? directory : null); + downloads = { behavior, directory: allowed ? directory : null }; + for (const relayed of targets) relayed.target.setDownloadDirectory(downloads.directory); downloadEventsEnabled = command.params?.["eventsEnabled"] === true; return result; } @@ -155,33 +229,43 @@ export function createCdpRelayConnection( case "Target.getBrowserContexts": return { browserContextIds: [] }; case "Target.getTargets": - return { targetInfos: [await targetInfo()] }; + return { + targetInfos: await Promise.all( + [...targets] + .filter((relayed) => relayed === page || announced(relayed)) + .map(targetInfo), + ), + }; case "Target.attachToBrowserTarget": - return { sessionId: newSession("browser") }; + return { sessionId: newSession({ kind: "browser" }) }; case "Target.attachToTarget": { - const targetId = command.params?.["targetId"]; - if (targetId !== (await target.targetId())) - throw new Error("Only this preview tab's page can be attached."); - return { sessionId: newSession("page") }; + const owner = await findTarget(command.params?.["targetId"]); + if (!owner) throw new Error("Only this preview tab's page and its popups can be attached."); + return { sessionId: newSession({ kind: "page", owner, parent: command.sessionId }) }; } case "Target.detachFromTarget": { const sessionId = command.params?.["sessionId"]; - if (typeof sessionId === "string" && sessionId !== PAGE_SESSION_ID) { - sessions.delete(sessionId); - target.sessionDetached?.(sessionId); + const session = typeof sessionId === "string" ? sessions.get(sessionId) : undefined; + if (session?.kind === "page" && sessionId !== session.owner.sessionId) { + sessions.delete(sessionId as string); + session.owner.target.sessionDetached?.(sessionId as string); + } else if (session?.kind === "browser") { + sessions.delete(sessionId as string); } return {}; } + case "Target.closeTarget": { + const owner = await findTarget(command.params?.["targetId"]); + if (owner === undefined || owner === page || !owner.target.close) + throw new Error(`Not supported for a desktop preview tab: ${command.method}`); + owner.target.close(); + return { success: true }; + } case "Target.setAutoAttach": { - if (attached) return {}; - attached = true; - const info = await targetInfo(); - return { - [ANNOUNCE]: { - method: "Target.attachedToTarget", - params: { sessionId: PAGE_SESSION_ID, targetInfo: info, waitingForDebugger: false }, - }, - }; + if (autoAttached) return {}; + autoAttached = true; + const info = await targetInfo(page); + return { [ANNOUNCE]: info }; } default: throw new Error(`Not supported for a desktop preview tab: ${command.method}`); @@ -189,16 +273,66 @@ export function createCdpRelayConnection( }; const pageCommand = (command: CdpCommand): Promise => { - const child = sessions.get(command.sessionId!) === "page" ? undefined : command.sessionId; - const owner = - child === undefined && command.sessionId !== PAGE_SESSION_ID ? command.sessionId : undefined; - // Child frames/workers pause on auto-attach until resumed; only the main page can skip this command. + const session = sessions.get(command.sessionId!); + const child = session?.kind === "page" ? undefined : command.sessionId; + const owner = session?.kind === "page" ? session.owner : (children.get(child!) ?? page); + const extra = child === undefined && command.sessionId !== owner.sessionId; + // Child frames/workers pause on auto-attach until resumed; only a main page can skip this command. if (command.method === "Runtime.runIfWaitingForDebugger" && child === undefined) { return Promise.resolve({}); } // Playwright asks for this on every page; the host emulates focus only while it is in use. if (command.method === "Emulation.setFocusEmulationEnabled") return Promise.resolve({}); - return target.send(command.method, command.params ?? {}, child, owner); + return owner.target.send( + command.method, + command.params ?? {}, + child, + extra ? command.sessionId : undefined, + ); + }; + + const eventFor = + (relayed: RelayedTarget): CdpRelayEvent => + (method, params, sessionId) => { + const child = (params as { sessionId?: unknown } | null | undefined)?.sessionId; + if (method === "Target.attachedToTarget" && typeof child === "string") { + children.set(child, relayed); + } else if (method === "Target.detachedFromTarget" && typeof child === "string") { + children.delete(child); + } + if (!announced(relayed)) 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")) { + if (downloadEventsEnabled) send({ method, params }); + return; + } + // Electron reports the page's own events with an empty session id. + if (sessionId) { + send({ method, params, sessionId }); + return; + } + for (const [id, session] of sessions) + if (session.kind === "page" && session.owner === relayed) + send({ method, params, sessionId: id }); + }; + + const removePopup = (relayed: RelayedTarget) => { + if (relayed.removed) return; + relayed.removed = true; + targets.delete(relayed); + for (const [id, owner] of children) if (owner === relayed) children.delete(id); + const targetId = relayed.targetId; + if (targetId === null) return; + for (const [id, session] of sessions) { + if (session.kind !== "page" || session.owner !== relayed) continue; + sessions.delete(id); + send({ + method: "Target.detachedFromTarget", + params: { sessionId: id, targetId }, + ...(session.parent === undefined ? {} : { sessionId: session.parent }), + }); + } }; return { @@ -212,19 +346,21 @@ export function createCdpRelayConnection( } if (!isCommand(command)) return; const result = - command.sessionId === undefined || sessions.get(command.sessionId) === "browser" + command.sessionId === undefined || sessions.get(command.sessionId)?.kind === "browser" ? browserCommand(command) : pageCommand(command); const route = command.sessionId === undefined ? {} : { sessionId: command.sessionId }; // Reply as commands finish so slow commands cannot block later replies. void result.then( (value) => { - const announce = + const info = typeof value === "object" && value !== null && ANNOUNCE in value - ? (value as { [ANNOUNCE]: Record })[ANNOUNCE] + ? (value as { [ANNOUNCE]: { readonly targetId: string } })[ANNOUNCE] : undefined; - send({ id: command.id, result: announce ? {} : (value ?? {}), ...route }); - if (announce) send(announce); + send({ id: command.id, result: info ? {} : (value ?? {}), ...route }); + if (!info) return; + send(attachedEvent(page, info)); + for (const relayed of targets) if (relayed !== page) announcePopup(relayed); }, (cause: unknown) => send({ @@ -237,21 +373,23 @@ 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")) { - if (downloadEventsEnabled) send({ method, params }); - return; - } - // Electron reports the page's own events with an empty session id. - if (sessionId) { - send({ method, params, sessionId }); - return; + event: eventFor(page), + addPopup: (popupTarget) => { + const relayed: RelayedTarget = { + target: popupTarget, + sessionId: `t3-preview-popup-${++sessionSequence}`, + targetId: null, + removed: false, + }; + targets.add(relayed); + if (downloads !== null) { + popupTarget.setDownloadDirectory(downloads.directory); + void popupTarget + .send("Page.setDownloadBehavior", { behavior: downloads.behavior }, undefined) + .catch(() => {}); } - for (const [id, kind] of sessions) - if (kind === "page") send({ method, params, sessionId: id }); + if (announced(page)) announcePopup(relayed); + return { event: eventFor(relayed), remove: () => removePopup(relayed) }; }, }; } diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 2e6e54a5293a7..e27d4fc256ed1 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -33,6 +33,7 @@ const command = ( const makeDebuggee = ( options: { + readonly targetId?: string; readonly deferFocus?: boolean; readonly refuse?: (method: string, sessionId: string | undefined) => boolean; } = {}, @@ -55,7 +56,7 @@ const makeDebuggee = ( return; } if (method === "Target.getTargetInfo") { - resolve({ targetInfo: { targetId: "GUEST" } }); + resolve({ targetInfo: { targetId: options.targetId ?? "GUEST" } }); return; } if (method === "Emulation.setFocusEmulationEnabled") { @@ -68,11 +69,23 @@ const makeDebuggee = ( pending.push(() => resolve({ method })); sendWaiters.shift()?.(); }), + attached: false, + isAttached: () => debuggee.attached, + attach: () => { + debuggee.attached = true; + }, + detach: vi.fn(() => { + debuggee.attached = false; + }), }); + let destroyed = false; const webContents = Object.assign(new NodeEvents.EventEmitter(), { + debugger: debuggee, getURL: () => "http://localhost/", getTitle: () => "Page", getUserAgent: () => "Electron", + isDestroyed: () => destroyed, + close: vi.fn(), }); return { tab: { @@ -80,6 +93,11 @@ const makeDebuggee = ( debugger: debuggee as unknown as Electron.Debugger, }, emit: (method: string, params: unknown) => emitter.emit("message", {}, method, params, ""), + debuggee, + destroy: () => { + destroyed = true; + webContents.emit("destroyed"); + }, navigate: () => webContents.emit("did-navigate"), /** Ids of the commands the debugger received, in order. */ sent, @@ -969,6 +987,7 @@ describe("DesktopBrowserHost", () => { id: 2, method: "Input.dispatchMouseEvent", params: { type: "mousePressed", x: 1, y: 1 }, + sessionId: "t3-preview-page", }), }), ); @@ -976,6 +995,111 @@ describe("DesktopBrowserHost", () => { }), ); + it.effect("drives an agent tab's popup windows through its relay, each on its own", () => + Effect.gen(function* () { + const host = yield* DesktopBrowserHost.make; + const page = makeDebuggee(); + const early = makeDebuggee({ targetId: "EARLY" }); + const late = makeDebuggee({ targetId: "LATE" }); + const human = makeDebuggee(); + const humanPopup = makeDebuggee({ targetId: "HUMAN-POPUP" }); + const humanKey = { threadId: "thread-1", tabId: "tab-human" }; + const events = yield* Queue.unbounded(); + yield* host.events.pipe( + Stream.runForEach((line) => + Queue.offer(events, decodeEvent(new TextDecoder().decode(line))), + ), + Effect.forkScoped, + ); + const cdp = (tabKey: typeof key, message: Record) => + host.handleCommandLine( + encodeJson({ type: "cdp", ...tabKey, message: encodeJson(message) }), + ); + const nextCdp = Effect.gen(function* () { + let event = yield* Queue.take(events); + while (event.type !== "cdp") event = yield* Queue.take(events); + return JSON.parse(event.message) as Record; + }); + host.attach(key, page.tab); + host.attach(humanKey, human.tab); + yield* Queue.takeN(events, 2); + host.openedPopup(page.tab.webContents, early.tab.webContents); + host.openedPopup(human.tab.webContents, humanPopup.tab.webContents); + + yield* host.handleCommandLine(encodeJson({ type: "popups", ...key })); + yield* cdp(key, { id: 1, method: "Target.setAutoAttach" }); + yield* cdp(humanKey, { id: 1, method: "Target.setAutoAttach" }); + const announced: Array> = []; + while (announced.length < 3) { + const message = yield* nextCdp; + if (message["method"] === "Target.attachedToTarget") announced.push(message); + } + host.openedPopup(page.tab.webContents, late.tab.webContents); + let lateAnnounced = yield* nextCdp; + while (lateAnnounced["method"] !== "Target.attachedToTarget") lateAnnounced = yield* nextCdp; + announced.push(lateAnnounced); + const popups = announced + .map((message) => message["params"] as { sessionId: string; targetInfo: object }) + .filter((params) => params.sessionId !== "t3-preview-page"); + expect(popups.map((params) => params.targetInfo)).toEqual([ + expect.objectContaining({ targetId: "EARLY", openerId: "GUEST" }), + expect.objectContaining({ targetId: "LATE", openerId: "GUEST" }), + ]); + // A person's tab keeps its popups to itself. + expect(humanPopup.debuggee.attached).toBe(false); + expect(early.debuggee.attached).toBe(true); + + const [earlySession, lateSession] = popups.map((params) => params.sessionId); + yield* cdp(key, { + id: 2, + method: "Page.captureScreenshot", + params: { id: 2 }, + sessionId: earlySession, + }); + yield* cdp(key, { + id: 3, + method: "Page.captureScreenshot", + params: { id: 3 }, + sessionId: lateSession, + }); + yield* settle; + // The early popup's pending screenshot neither blocks the late one's nor focuses the page. + expect([early.sent, late.sent, page.sent]).toEqual([[2], [3], []]); + expect([early.focusEmulation, late.focusEmulation, page.focusEmulation]).toEqual([ + [true], + [true], + [], + ]); + + yield* cdp(key, { + id: 4, + method: "Browser.setDownloadBehavior", + params: { behavior: "allowAndName", downloadPath: "/srv/downloads" }, + }); + yield* settle; + for (const debuggee of [page, early, late]) debuggee.release(); + let reply = yield* nextCdp; + while (reply["id"] !== 4) reply = yield* nextCdp; + const paths: Array = []; + const item = { + setSavePath: (path: string) => void paths.push(path), + } as unknown as Electron.DownloadItem; + late.emit("Page.downloadWillBegin", { guid: "guid-late" }); + expect(host.placeDownload(page.tab.webContents, item)).toBe(false); + expect(host.placeDownload(late.tab.webContents, item)).toBe(true); + expect(paths).toEqual(["/srv/downloads/guid-late"]); + + late.destroy(); + let closed = yield* nextCdp; + while (closed["method"] !== "Target.detachedFromTarget") closed = yield* nextCdp; + expect(closed["params"]).toEqual({ sessionId: lateSession, targetId: "LATE" }); + + yield* host.handleCommandLine(encodeJson({ type: "release", ...key })); + expect(early.debuggee.detach).toHaveBeenCalledOnce(); + expect(late.debuggee.detach).not.toHaveBeenCalled(); + }).pipe(Effect.scoped), + ); + it.effect("detaches only the tab whose CDP event is too large to relay", () => 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 3ac6c6d73caec..5323b5230da49 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -14,15 +14,17 @@ import * as PubSub from "effect/PubSub"; import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; -import { createCdpRelayConnection, type CdpRelayConnection } from "./CdpRelay.ts"; +import { + createCdpRelayConnection, + type CdpRelayConnection, + type CdpRelayPopup, + type CdpRelayTarget, +} from "./CdpRelay.ts"; import { sendPageCommand } from "./GuestInput.ts"; const encodeEvent = Schema.encodeSync(Schema.fromJsonString(DesktopBrowserEvent)); const decodeCommand = Schema.decodeUnknownOption(Schema.fromJsonString(DesktopBrowserCommand)); const lineEncoder = new TextEncoder(); -const AGENT_INPUT_COMMAND = - /"method":"(?:Input\.|Page\.navigate"|Runtime\.(?:evaluate|callFunctionOn)")/; -/** The same commands as above, by method, for the relay's sends. */ const AGENT_INPUT_METHOD = /^(?:Input\.|Page\.navigate$|Runtime\.(?:evaluate|callFunctionOn)$)/; const AGENT_DOWNLOAD_WINDOW_MS = 5_000; const FOCUS_IDLE = "5 seconds"; @@ -39,10 +41,10 @@ export interface DesktopBrowserTabDebugger { const keyOf = ({ threadId, tabId }: DesktopBrowserTabKey) => `${threadId}\u0000${tabId}`; -interface AttachedTab { - readonly key: DesktopBrowserTabKey; +/** A page the relay drives: the tab's own, or a popup it opened. */ +interface AttachedTarget { + readonly tab: AttachedTab; readonly debuggee: DesktopBrowserTabDebugger; - relay: CdpRelayConnection | null; screenshotTurn: object | null; /** Each holds focus and painting while `held` and whoever queued it still waits on it. */ readonly waitingScreenshots: Array<{ @@ -61,8 +63,6 @@ interface AttachedTab { object, { readonly owner: string | undefined; readonly paints: boolean } >; - /** Whether the client keeps the guest painting, as published on `painting`. */ - painting: boolean; /** * Driven by the agent since its thread's last turn ended or the guest last throttled. A * turn's start can reach us after its first command, so this holds focus once it does. @@ -80,6 +80,29 @@ interface AttachedTab { readonly onNewDocument: () => void; } +/** Sessions of one relay connection, which its page and popups share. */ +interface RelayOwners { + readonly detached: Set; + /** Viewers and recordings stream; the agent's own sessions never do. */ + readonly streaming: Set; +} + +interface AttachedTab extends AttachedTarget { + readonly key: DesktopBrowserTabKey; + relay: CdpRelayConnection | null; + owners: RelayOwners; + /** Whether the client keeps the guest painting, as published on `painting`. */ + painting: boolean; + /** Set while the server drives the tab for an agent, whose popups become its tabs too. */ + showsPopups: boolean; + readonly popups: Map; +} + +interface AttachedPopup extends AttachedTarget { + relayed: CdpRelayPopup | null; + readonly onDetach: () => void; +} + export class DesktopBrowserHost extends Context.Service< DesktopBrowserHost, { @@ -87,6 +110,8 @@ export class DesktopBrowserHost extends Context.Service< readonly handleCommandLine: (line: string) => Effect.Effect; readonly attach: (key: DesktopBrowserTabKey, debuggee: DesktopBrowserTabDebugger) => void; readonly detach: (key: DesktopBrowserTabKey) => void; + /** A guest opened a popup window; a tab the server drives for an agent shows it as a tab too. */ + readonly openedPopup: (opener: Electron.WebContents, popup: Electron.WebContents) => void; readonly humanStartedDownload: (source: Electron.WebContents) => boolean; readonly placeDownload: (source: Electron.WebContents, item: Electron.DownloadItem) => boolean; readonly pointers: Stream.Stream<{ @@ -117,15 +142,21 @@ export const make = Effect.gen(function* () { }>(); const runFork = Effect.runForkWith(yield* Effect.context()); const tabs = new Map(); + /** Every guest's open popups, so a tab the server starts driving later still shows them. */ + const openedPopups = new Map>(); /** Threads whose agent turn runs, by the server's account. */ const agentTurns = new Set(); const emit = (event: DesktopBrowserEventType) => runFork(PubSub.publish(outbox, event)); - const nextScreenshot = (tab: AttachedTab) => { - tab.screenshotTurn = null; - tab.waitingScreenshots.shift()?.start(); - releaseFocusWhenIdle(tab); - syncPainting(tab); + const isAttached = (target: AttachedTarget) => + tabs.get(keyOf(target.tab.key)) === target.tab && + (target === target.tab || target.tab.popups.get(target.debuggee.webContents) === target); + + const nextScreenshot = (target: AttachedTarget) => { + target.screenshotTurn = null; + target.waitingScreenshots.shift()?.start(); + releaseFocusWhenIdle(target); + syncPainting(target); }; /** @@ -135,38 +166,38 @@ export const make = Effect.gen(function* () { * gets one at a time. One whose relay or session has since gone is dropped unsent. */ const takeScreenshot = ( - tab: AttachedTab, + target: AttachedTarget, relay: CdpRelayConnection, ownerLive: () => boolean, /** Sends only while `current` holds; holds focus and painting only while `held`. */ capture: (current: () => boolean, held: boolean) => Promise, ) => new Promise((resolve, reject) => { - const live = () => tab.relay === relay && ownerLive(); + const live = () => target.tab.relay === relay && isAttached(target) && ownerLive(); const waiting = { start: () => { if (!live()) { reject(new Error("The preview connection that asked for this screenshot closed.")); - nextScreenshot(tab); + nextScreenshot(target); return; } const turn = {}; - tab.screenshotTurn = turn; - capture(() => tab.screenshotTurn === turn && live(), waiting.held) + target.screenshotTurn = turn; + capture(() => target.screenshotTurn === turn && live(), waiting.held) .finally(() => { - if (tab.screenshotTurn === turn) nextScreenshot(tab); + if (target.screenshotTurn === turn) nextScreenshot(target); }) .then(resolve, reject); }, live, held: true, }; - if (tab.screenshotTurn === null) waiting.start(); + if (target.screenshotTurn === null) waiting.start(); else { // The one ahead may be abandoned mid-capture; it needs frames to finish and free its turn. - tab.waitingScreenshots.push(waiting); - setFocusEmulation(tab, true); - syncPainting(tab); + target.waitingScreenshots.push(waiting); + setFocusEmulation(target, true); + syncPainting(target); } }); @@ -175,44 +206,44 @@ export const make = Effect.gen(function* () { * never throttle. Without it, a hidden guest paints no frames and focus() fires no focus * events, so the guest is emulated focused while the server drives it or streams it. */ - const setFocusEmulation = (tab: AttachedTab, enabled: boolean) => { - if (tab.focusEmulated === enabled) return; - tab.focusEmulated = enabled; + const setFocusEmulation = (target: AttachedTarget, enabled: boolean) => { + if (target.focusEmulated === enabled) return; + target.focusEmulated = enabled; // A guest that refuses it, or a debugger already gone, behaves as unfocused. - const change: Promise = tab.debuggee.debugger + const change: Promise = target.debuggee.debugger .sendCommand("Emulation.setFocusEmulationEnabled", { enabled }) .then( () => undefined, () => undefined, ) .finally(() => { - if (tab.focusChange === change) tab.focusChange = null; + if (target.focusChange === change) target.focusChange = null; }); - tab.focusChange = change; + target.focusChange = change; }; - const waitingHolds = (tab: AttachedTab) => - tab.waitingScreenshots.some((waiting) => waiting.held && waiting.live()); + const waitingHolds = (target: AttachedTarget) => + target.waitingScreenshots.some((waiting) => waiting.held && waiting.live()); - const releaseFocusWhenIdle = (tab: AttachedTab) => { - if (tab.focusReleasing) return; - tab.focusReleasing = true; + const releaseFocusWhenIdle = (target: AttachedTarget) => { + if (target.focusReleasing) return; + target.focusReleasing = true; runFork( Effect.gen(function* () { let uses: number; do { - uses = tab.focusUses; + uses = target.focusUses; yield* Effect.sleep(FOCUS_IDLE); - } while (uses !== tab.focusUses); - tab.focusReleasing = false; + } while (uses !== target.focusUses); + target.focusReleasing = false; // The last pending command to settle starts another idle wait. - if (tab.focusPending.size > 0 || tab.screencasts.size > 0 || waitingHolds(tab)) { + if (target.focusPending.size > 0 || target.screencasts.size > 0 || waitingHolds(target)) { return; } // The agent pauses to think between steps; its tab stays focused until the turn ends. - if (tab.agentUsed && agentTurns.has(tab.key.threadId)) return; - tab.agentUsed = false; - if (tabs.get(keyOf(tab.key)) === tab) setFocusEmulation(tab, false); + if (target.agentUsed && agentTurns.has(target.tab.key.threadId)) return; + target.agentUsed = false; + if (isAttached(target)) setFocusEmulation(target, false); }), ); }; @@ -221,8 +252,11 @@ export const make = Effect.gen(function* () { * Electron composites no offscreen guest, so its screenshot or screencast waits for a frame * that never comes. The client paints the guest behind the app while one is pending; the * capture then finishes on its own, and the guest goes back offscreen once none is. + * A popup is a window of its own, which the client does not paint. */ - const syncPainting = (tab: AttachedTab) => { + const syncPainting = (target: AttachedTarget) => { + const tab = target.tab; + if (target !== tab) return; const active = tabs.get(keyOf(tab.key)) === tab && (tab.screencasts.size > 0 || @@ -235,22 +269,22 @@ export const make = Effect.gen(function* () { /** A command may wait on the page's next frame, so the guest stays focused until it settles. */ const holdFocus = ( - tab: AttachedTab, + target: AttachedTarget, method: string, owner: string | undefined, work: () => Promise, ) => { const pending = {}; - tab.focusPending.set(pending, { owner, paints: method === "Page.captureScreenshot" }); - tab.focusUses += 1; - setFocusEmulation(tab, true); - releaseFocusWhenIdle(tab); - syncPainting(tab); + target.focusPending.set(pending, { owner, paints: method === "Page.captureScreenshot" }); + target.focusUses += 1; + setFocusEmulation(target, true); + releaseFocusWhenIdle(target); + syncPainting(target); return work().finally(() => { - if (!tab.focusPending.delete(pending)) return; - tab.focusUses += 1; - releaseFocusWhenIdle(tab); - syncPainting(tab); + if (!target.focusPending.delete(pending)) return; + target.focusUses += 1; + releaseFocusWhenIdle(target); + syncPainting(target); }); }; @@ -259,27 +293,29 @@ export const make = Effect.gen(function* () { * also end every hold: a released relay has no one waiting, and a new document answers * nothing pending from the old one. A released relay's screencast has no viewer to stop it. */ - const dropFocusHolds = (tab: AttachedTab) => { - tab.focusPending.clear(); - releaseFocusWhenIdle(tab); - syncPainting(tab); + const dropFocusHolds = (target: AttachedTarget) => { + target.focusPending.clear(); + releaseFocusWhenIdle(target); + syncPainting(target); }; const dropRelay = (tab: AttachedTab) => { tab.relay = null; + tab.showsPopups = false; + for (const popup of tab.popups.values()) hidePopup(popup); tab.screencasts.clear(); tab.agentUsed = false; dropFocusHolds(tab); }; /** A detached session, such as a closed viewer, waits on nothing it sent. */ - const dropSessionHolds = (tab: AttachedTab, owner: string) => { - tab.screencasts.delete(owner); - for (const [pending, hold] of tab.focusPending) { - if (hold.owner === owner) tab.focusPending.delete(pending); + const dropSessionHolds = (target: AttachedTarget, owner: string) => { + target.screencasts.delete(owner); + for (const [pending, hold] of target.focusPending) { + if (hold.owner === owner) target.focusPending.delete(pending); } - releaseFocusWhenIdle(tab); - syncPainting(tab); + releaseFocusWhenIdle(target); + syncPainting(target); }; /** @@ -291,80 +327,96 @@ export const make = Effect.gen(function* () { agentTurns.delete(threadId); for (const tab of tabs.values()) { if (tab.key.threadId !== threadId) continue; - tab.agentUsed = false; - // Still sent in turn, so the queue keeps moving, but no longer kept painting. - for (const waiting of tab.waitingScreenshots) waiting.held = false; - dropFocusHolds(tab); + for (const target of [tab, ...tab.popups.values()]) { + target.agentUsed = false; + // Still sent in turn, so the queue keeps moving, but no longer kept painting. + for (const waiting of target.waitingScreenshots) waiting.held = false; + dropFocusHolds(target); + } } }; - const relayFor = (tab: AttachedTab) => { - if (tab.relay) return tab.relay; - const { webContents, debugger: debuggee } = tab.debuggee; + /** One relay drives the tab's page and its popups; each keeps its own focus and screenshots. */ + const relayTarget = ( + target: AttachedTarget, + relay: () => CdpRelayConnection, + owners: RelayOwners, + ): CdpRelayTarget => { + const { webContents, debugger: debuggee } = target.debuggee; + const live = () => target.tab.relay === relay() && isAttached(target); const send = (method: string, params: Record, sessionId?: string) => sendPageCommand(webContents, debuggee, method, params, sessionId); - const detachedOwners = new Set(); - /** Viewers and recordings stream; the agent's own sessions never do. */ - const streamingOwners = new Set(); - const relay: CdpRelayConnection = createCdpRelayConnection( - { - send: (method, params, sessionId, owner) => { - // A child session's screencast is not the page's, such as a late one on a detached session. - if (sessionId === undefined && method === "Page.startScreencast") { - tab.screencasts.add(owner); - if (owner !== undefined) streamingOwners.add(owner); - } else if (sessionId === undefined && method === "Page.stopScreencast") { - tab.screencasts.delete(owner); - } + return { + send: (method, params, sessionId, owner) => { + // A child session's screencast is not the page's, such as a late one on a detached session. + if (sessionId === undefined && method === "Page.startScreencast") { + target.screencasts.add(owner); + if (owner !== undefined) owners.streaming.add(owner); + } else if (sessionId === undefined && method === "Page.stopScreencast") { + target.screencasts.delete(owner); + } + if (AGENT_INPUT_METHOD.test(method)) { + target.agentInputAt = performance.now(); // A person's input through a viewer, or Playwright's own housekeeping, is not the agent's. - if ( - AGENT_INPUT_METHOD.test(method) && - (owner === undefined || !streamingOwners.has(owner)) - ) { - tab.agentUsed = true; - } - // Waits for focus emulation; insertText and other non-CDP input would not queue behind it. - const forward = (current?: () => boolean) => { - const change = tab.focusChange; - const run = () => - tab.relay === relay && (current === undefined || current()) - ? send(method, params, sessionId) - : Promise.reject(new Error("The page this screenshot was for has moved on.")); - return change === null ? run() : change.then(run); - }; - return method === "Page.captureScreenshot" - ? takeScreenshot( - tab, - relay, - () => owner === undefined || !detachedOwners.has(owner), - (current, held) => - held ? holdFocus(tab, method, owner, () => forward(current)) : forward(current), - ) - : method === "Page.startScreencast" - ? holdFocus(tab, method, owner, forward).catch((cause: unknown) => { - // A viewer can outlive a refused stream; nothing is streaming for it. - if (sessionId === undefined && tab.relay === relay) tab.screencasts.delete(owner); - releaseFocusWhenIdle(tab); - syncPainting(tab); - throw cause; - }) - : holdFocus(tab, method, owner, forward); - }, - sessionDetached: (owner) => { - detachedOwners.add(owner); - dropSessionHolds(tab, owner); - }, - targetId: () => - debuggee - .sendCommand("Target.getTargetInfo") - .then((result: { targetInfo: { targetId: string } }) => result.targetInfo.targetId), - url: () => webContents.getURL(), - title: () => webContents.getTitle(), - userAgent: () => webContents.getUserAgent(), - setDownloadDirectory: (directory) => { - tab.downloadDirectory = directory; - }, + if (owner === undefined || !owners.streaming.has(owner)) target.agentUsed = true; + } + // Waits for focus emulation; insertText and other non-CDP input would not queue behind it. + const forward = (current?: () => boolean) => { + const change = target.focusChange; + const run = () => + live() && (current === undefined || current()) + ? send(method, params, sessionId) + : Promise.reject(new Error("The page this screenshot was for has moved on.")); + return change === null ? run() : change.then(run); + }; + return method === "Page.captureScreenshot" + ? takeScreenshot( + target, + relay(), + () => owner === undefined || !owners.detached.has(owner), + (current, held) => + held ? holdFocus(target, method, owner, () => forward(current)) : forward(current), + ) + : method === "Page.startScreencast" + ? holdFocus(target, method, owner, forward).catch((cause: unknown) => { + // A viewer can outlive a refused stream; nothing is streaming for it. + if (sessionId === undefined && live()) target.screencasts.delete(owner); + releaseFocusWhenIdle(target); + syncPainting(target); + throw cause; + }) + : holdFocus(target, method, owner, forward); + }, + sessionDetached: (owner) => { + owners.detached.add(owner); + dropSessionHolds(target, owner); }, + targetId: () => + debuggee + .sendCommand("Target.getTargetInfo") + .then((result: { targetInfo: { targetId: string } }) => result.targetInfo.targetId), + url: () => webContents.getURL(), + title: () => webContents.getTitle(), + userAgent: () => webContents.getUserAgent(), + setDownloadDirectory: (directory) => { + target.downloadDirectory = directory; + }, + ...(target === target.tab + ? {} + : { + close: () => { + if (!webContents.isDestroyed()) webContents.close(); + }, + }), + }; + }; + + const relayFor = (tab: AttachedTab) => { + if (tab.relay) return tab.relay; + const { debugger: debuggee } = tab.debuggee; + tab.owners = { detached: new Set(), streaming: new Set() }; + const relay: CdpRelayConnection = createCdpRelayConnection( + relayTarget(tab, () => relay, tab.owners), (message) => { if (tab.relay === relay && tabs.get(keyOf(tab.key)) === tab) { emit({ type: "cdp", ...tab.key, message }); @@ -390,17 +442,127 @@ export const make = Effect.gen(function* () { }, ); tab.relay = relay; + showPopups(tab); return relay; }; + const freshTarget = (debuggee: DesktopBrowserTabDebugger) => ({ + debuggee, + screenshotTurn: null, + waitingScreenshots: [], + downloadDirectory: null, + pendingDownloadGuid: null, + agentInputAt: Number.NEGATIVE_INFINITY, + focusEmulated: false, + focusChange: null, + focusUses: 0, + focusPending: new Map(), + agentUsed: false, + focusReleasing: false, + screencasts: new Set(), + }); + + const listen = >( + target: T, + relayed: () => { readonly event: CdpRelayConnection["event"] } | null, + ): T & Pick => + Object.assign(target, { + onMessage: (_event: Electron.Event, method: string, params: unknown, sessionId: string) => { + if (method === "Page.downloadWillBegin") { + const guid = (params as { guid?: unknown } | undefined)?.guid; + target.pendingDownloadGuid = typeof guid === "string" ? guid : null; + } + relayed()?.event(method, params, sessionId); + }, + // 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: () => { + const listening = target as T & AttachedTarget; + // Before the next screenshot starts, so its own hold survives. + dropFocusHolds(listening); + if (listening.screenshotTurn !== null) nextScreenshot(listening); + }, + }); + + const showPopup = (tab: AttachedTab, webContents: Electron.WebContents) => { + const relay = tab.relay; + if (!relay || !tab.showsPopups || tab.popups.has(webContents) || webContents.isDestroyed()) + return; + // Kept for the popup's lifetime; `webContents.debugger` throws once it is destroyed. + const debuggee = webContents.debugger; + // DevTools, or another client, already holds its debugger. + if (debuggee.isAttached()) return; + try { + debuggee.attach("1.3"); + } catch { + return; + } + const popup: AttachedPopup = listen( + { + ...freshTarget({ webContents, debugger: debuggee }), + tab, + relayed: null, + onDetach: () => hidePopup(popup, true), + }, + () => popup.relayed, + ); + tab.popups.set(webContents, popup); + debuggee.on("message", popup.onMessage); + debuggee.on("detach", popup.onDetach); + webContents.on("did-navigate", popup.onNewDocument); + popup.relayed = relay.addPopup(relayTarget(popup, () => relay, tab.owners)); + }; + + /** Detaching the debugger also ends the popup's streams, emulation and paused children. */ + const hidePopup = (popup: AttachedPopup, debuggerGone = false) => { + const { tab, debuggee } = popup; + if (tab.popups.get(debuggee.webContents) !== popup) return; + tab.popups.delete(debuggee.webContents); + debuggee.debugger.off("message", popup.onMessage); + debuggee.debugger.off("detach", popup.onDetach); + debuggee.webContents.off("did-navigate", popup.onNewDocument); + popup.relayed?.remove(); + popup.screencasts.clear(); + dropFocusHolds(popup); + if (!debuggerGone && !debuggee.webContents.isDestroyed() && debuggee.debugger.isAttached()) { + debuggee.debugger.detach(); + } + }; + + const showPopups = (tab: AttachedTab) => { + for (const popup of openedPopups.get(tab.debuggee.webContents) ?? []) showPopup(tab, popup); + }; + + const openedPopup = (opener: Electron.WebContents, popup: Electron.WebContents) => { + const popups = openedPopups.get(opener) ?? new Set(); + openedPopups.set(opener, popups); + popups.add(popup); + popup.once("destroyed", () => { + popups.delete(popup); + if (popups.size === 0 && openedPopups.get(opener) === popups) openedPopups.delete(opener); + for (const tab of tabs.values()) { + const shown = tab.popups.get(popup); + if (shown) hidePopup(shown, true); + } + }); + for (const tab of tabs.values()) if (tab.debuggee.webContents === opener) showPopup(tab, popup); + }; + + const targetOf = (source: Electron.WebContents) => { + for (const tab of tabs.values()) { + if (tab.debuggee.webContents === source) return tab; + const popup = tab.popups.get(source); + if (popup) return popup; + } + return undefined; + }; + // CDP supplies the path before this runs; otherwise Electron opens a Save dialog. const placeDownload = (source: Electron.WebContents, item: Electron.DownloadItem) => { - const tab = [...tabs.values()].find( - (candidate) => candidate.debuggee.webContents === source && candidate.downloadDirectory, - ); - if (!tab?.downloadDirectory || !tab.pendingDownloadGuid) return false; - item.setSavePath(NodePath.join(tab.downloadDirectory, tab.pendingDownloadGuid)); - tab.pendingDownloadGuid = null; + const target = targetOf(source); + if (!target?.downloadDirectory || !target.pendingDownloadGuid) return false; + item.setSavePath(NodePath.join(target.downloadDirectory, target.pendingDownloadGuid)); + target.pendingDownloadGuid = null; return true; }; @@ -408,6 +570,7 @@ export const make = Effect.gen(function* () { const id = keyOf(key); const tab = tabs.get(id); if (!tab) return; + for (const popup of tab.popups.values()) hidePopup(popup); tabs.delete(id); tab.relay = null; tab.debuggee.debugger.off("message", tab.onMessage); @@ -420,38 +583,21 @@ export const make = Effect.gen(function* () { const id = keyOf(key); if (tabs.get(id)?.debuggee.webContents === debuggee.webContents) return; detach(key); - const tab: AttachedTab = { - key, - debuggee, - relay: null, - screenshotTurn: null, - waitingScreenshots: [], - downloadDirectory: null, - pendingDownloadGuid: null, - agentInputAt: Number.NEGATIVE_INFINITY, - focusEmulated: false, - focusChange: null, - focusUses: 0, - focusPending: new Map(), - painting: false, - agentUsed: false, - focusReleasing: false, - screencasts: new Set(), - onMessage: (_event, method, params, sessionId) => { - if (method === "Page.downloadWillBegin") { - const guid = (params as { guid?: unknown } | undefined)?.guid; - tab.pendingDownloadGuid = typeof guid === "string" ? guid : null; - } - tab.relay?.event(method, params, sessionId); - }, - // 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: () => { - // Before the next screenshot starts, so its own hold survives. - dropFocusHolds(tab); - if (tab.screenshotTurn !== null) nextScreenshot(tab); + const tab: AttachedTab = listen( + { + ...freshTarget(debuggee), + get tab() { + return tab; + }, + key, + relay: null, + owners: { detached: new Set(), streaming: new Set() }, + painting: false, + showsPopups: false, + popups: new Map(), }, - }; + () => tab.relay, + ); tabs.set(id, tab); debuggee.debugger.on("message", tab.onMessage); debuggee.webContents.on("did-navigate", tab.onNewDocument); @@ -469,22 +615,29 @@ export const make = Effect.gen(function* () { } const tab = tabs.get(keyOf(command.value)); if (!tab) return; - if (command.value.type === "pointer") { - const { threadId, tabId, phase, x, y } = command.value; - runFork(PubSub.publish(pointers, { key: { threadId, tabId }, phase, x, y })); - return; - } - if (command.value.type === "release") { - dropRelay(tab); - return; + switch (command.value.type) { + case "pointer": { + const { threadId, tabId, phase, x, y } = command.value; + runFork(PubSub.publish(pointers, { key: { threadId, tabId }, phase, x, y })); + return; + } + case "release": + dropRelay(tab); + return; + case "popups": + tab.showsPopups = true; + if (tab.relay) showPopups(tab); + return; + case "cdp": + relayFor(tab).receive(command.value.message); } - if (AGENT_INPUT_COMMAND.test(command.value.message)) tab.agentInputAt = performance.now(); - relayFor(tab).receive(command.value.message); }); const humanStartedDownload = (source: Electron.WebContents) => { - const tab = [...tabs.values()].find((candidate) => candidate.debuggee.webContents === source); - return tab !== undefined && performance.now() - tab.agentInputAt > AGENT_DOWNLOAD_WINDOW_MS; + const target = targetOf(source); + return ( + target !== undefined && performance.now() - target.agentInputAt > AGENT_DOWNLOAD_WINDOW_MS + ); }; const announceAll = Effect.suspend(() => { @@ -514,6 +667,7 @@ export const make = Effect.gen(function* () { handleCommandLine, attach, detach, + openedPopup, placeDownload, humanStartedDownload, }); diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index 18eb4b2b75968..7f50e688474ee 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -1443,6 +1443,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function window.webContents.on("before-input-event", (_event, input) => { syncMenuShortcuts(window.webContents, input); }); + browserHost.openedPopup(wc, window.webContents); }; const beforeInput = (event: Electron.Event, input: Electron.Input): void => { syncMenuShortcuts(wc, input); diff --git a/apps/server/src/preview/DesktopBrowserChannel.ts b/apps/server/src/preview/DesktopBrowserChannel.ts index d407fc78faf57..ecc4fe76705b8 100644 --- a/apps/server/src/preview/DesktopBrowserChannel.ts +++ b/apps/server/src/preview/DesktopBrowserChannel.ts @@ -44,7 +44,11 @@ export class DesktopBrowserChannel extends Context.Service< readonly detached: Stream.Stream; readonly attached: Stream.Stream; readonly isAttached: (key: DesktopTabKey) => Effect.Effect; - readonly endpoint: (key: DesktopTabKey) => Effect.Effect; + /** With `popups`, the tab's popup windows become pages of the same connection. */ + readonly endpoint: ( + key: DesktopTabKey, + options?: { readonly popups: boolean }, + ) => Effect.Effect; readonly pointer: ( key: DesktopTabKey, pointer: { readonly phase: "move" | "click"; readonly x: number; readonly y: number }, @@ -134,7 +138,10 @@ const make = Effect.gen(function* () { Effect.forkScoped, ); - const endpoint = Effect.fn("DesktopBrowserChannel.endpoint")(function* (key: DesktopTabKey) { + const endpoint = Effect.fn("DesktopBrowserChannel.endpoint")(function* ( + key: DesktopTabKey, + options?: { readonly popups: boolean }, + ) { const id = keyOf(key); const secret = NodeCrypto.randomBytes(24).toString("base64url"); const server = yield* NodeSocketServer.makeWebSocket({ @@ -156,6 +163,8 @@ const make = Effect.gen(function* () { yield* command({ type: "release", ...key }); }), ); + // Written before any CDP, so popups already open are there when Playwright connects. + if (options?.popups) yield* command({ type: "popups", ...key }); // The relay serves one Playwright connection; a second would see the first's sessions. let connected = false; yield* server diff --git a/apps/server/src/preview/Manager.ts b/apps/server/src/preview/Manager.ts index f5db1d2fefc1e..dbae559053a35 100644 --- a/apps/server/src/preview/Manager.ts +++ b/apps/server/src/preview/Manager.ts @@ -35,6 +35,8 @@ export class PreviewManager extends Context.Service< readonly open: ( input: PreviewOpenInput & { readonly automationOwner?: string; + /** The desktop streams the tab from its first snapshot instead of hosting a webview. */ + readonly headless?: boolean; /** Runs before the `opened` event publishes, so subscribers find state keyed by the tab. */ readonly beforePublish?: (snapshot: PreviewSessionSnapshot) => void; }, @@ -42,7 +44,7 @@ export class PreviewManager extends Context.Service< readonly reportStatus: ( input: PreviewReportStatusInput & { readonly serverControlled?: boolean }, ) => Effect.Effect; - /** Records that a server tab runs headless. It stays headless until it closes. */ + /** Records that the desktop streams a server tab. It stays streamed until it closes. */ readonly markHeadless: ( input: PreviewCloseInput & { readonly tabId: string }, ) => Effect.Effect; @@ -193,6 +195,7 @@ export const make = Effect.gen(function* PreviewManagerMake() { ? { automationOwner: input.automationOwner } : {}), ...(input.reveal === undefined ? {} : { reveal: input.reveal }), + ...(runtime === "server" && input.headless ? { headless: true } : {}), updatedAt, }; yield* SynchronizedRef.modifyEffect(stateRef, (state) => diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index c558631fd2a51..e2c60391970bd 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -67,6 +67,9 @@ vi.mock("./ServerBrowserContexts.ts", () => ({ async connectDesktopPage(endpoint: string) { const context = makeContext(undefined, desktopPageUrl); context.page.viewportSize.mockReturnValue(null); + const popups = desktopPopupsNext.splice(0); + for (const popup of popups) popup.page.opener = async () => context.page; + context.pages = () => [context.page, ...popups.map((popup) => popup.page)]; desktopConnections.push({ endpoint, context }); return { browser: { close: async () => {} }, page: context.page as unknown as Page }; } @@ -206,6 +209,7 @@ function makeContext(onClose?: (context: BrowserContext) => void, initialUrl = " initialNavigation = null; await pending?.(); }), + opener: async (): Promise => null, goBack: vi.fn(async () => {}), goForward: vi.fn(async () => {}), reload: vi.fn(async () => {}), @@ -235,6 +239,7 @@ function makeContext(onClose?: (context: BrowserContext) => void, initialUrl = " sessions, cssViewport, newPage: async () => page as unknown as Page, + pages: (): ReadonlyArray => [page], grantPermissions: vi.fn(async () => {}), newCDPSession: async (target: unknown) => { const session = makeSession( @@ -308,6 +313,9 @@ let pageSetupFailure: Error | null = null; let initialNavigation: (() => Promise) | null = null; let desktopRendersNext = false; let desktopPageUrl = "about:blank"; +/** Popup windows already open when the next desktop page connects. */ +const desktopPopupsNext: Array> = []; +const desktopEndpoints: Array<{ tabId: string; popups: boolean | undefined }> = []; /** Replaces the answer to the desktop page's size reads. */ let desktopSizeAnswer: (() => Promise>) | null = null; const desktopDetaches = new NodeEvents.EventEmitter(); @@ -379,12 +387,17 @@ const dependencies = Layer.mergeAll( ), ), isAttached: (key) => Effect.sync(() => desktopRenders(key.tabId)), - endpoint: (key) => - Effect.acquireRelease(Effect.succeed(`ws://desktop/${key.tabId}`), () => + endpoint: (key, options) => + Effect.acquireRelease( Effect.sync(() => { - releasedDesktopTabs.push(key.tabId); - desktopReleased.resolve(); + desktopEndpoints.push({ tabId: key.tabId, popups: options?.popups }); + return `ws://desktop/${key.tabId}`; }), + () => + Effect.sync(() => { + releasedDesktopTabs.push(key.tabId); + desktopReleased.resolve(); + }), ), pointer: () => Effect.void, turn: (threadId, active) => Effect.sync(() => desktopTurns.emit("turn", threadId, active)), @@ -457,6 +470,8 @@ beforeEach(() => { desktopTabs.clear(); desktopRendersNext = false; desktopPageUrl = "about:blank"; + desktopPopupsNext.length = 0; + desktopEndpoints.length = 0; desktopSizeAnswer = null; desktopDetachReady = Promise.withResolvers(); desktopReleased = Promise.withResolvers(); @@ -623,6 +638,75 @@ it.effect("a tab the desktop does not draw is marked headless, so the desktop st ).pipe(Effect.provide(layer)), ); +it.effect("an agent's desktop tab's popup windows become its tabs, drawn by the desktop", () => + Effect.scoped( + Effect.gen(function* () { + const { broker } = yield* ready; + const manager = yield* Manager.PreviewManager; + const events = yield* manager.subscribeEvents; + const nextOpened = Effect.gen(function* () { + let event = yield* PubSub.take(events); + while (event.type !== "opened") event = yield* PubSub.take(events); + return event.snapshot; + }); + const existing = makeContext(undefined, "https://example.com/popout"); + desktopPopupsNext.push(existing); + desktopRendersNext = true; + const opener = yield* broker.invoke({ + scope, + operation: "open", + input: { reuseExistingTab: false, show: false }, + }); + yield* nextOpened; + const existingTab = yield* nextOpened; + const later = makeContext(undefined, "https://example.com/auth"); + const openerPage = desktopConnections[0]!.context.page; + openerPage.emit("popup", later.page); + openerPage.emit("popup", existing.page); + const laterTab = yield* nextOpened; + + // Streamed from its first snapshot: the desktop never loads the popup in a webview of its own. + expect([existingTab.headless, laterTab.headless]).toEqual([true, true]); + expect(desktopEndpoints).toEqual([{ tabId: opener.tabId, popups: true }]); + const status = yield* broker.invoke({ + scope, + tabId: existingTab.tabId, + operation: "status", + input: {}, + }); + expect(status.tabs).toEqual( + expect.arrayContaining( + [existingTab, laterTab].map((popup) => + expect.objectContaining({ + tabId: popup.tabId, + openerTabId: opener.tabId, + ownedByCaller: true, + }), + ), + ), + ); + expect(status.tabs).toHaveLength(4); + // A native window keeps its own size and the person's file picker. + expect(existing.page.setViewportSize).not.toHaveBeenCalled(); + existing.page.openFileChooser(); + expect(existing.page.nativeFileChooser).toHaveBeenCalledOnce(); + + yield* broker.invoke({ scope, tabId: existingTab.tabId, operation: "close", input: {} }); + expect(existing.page.close).toHaveBeenCalled(); + expect(releasedDesktopTabs).toEqual([]); + const { sessions } = yield* manager.list({ threadId: scope.thread.threadId }); + expect(sessions.map((session) => session.tabId)).toContain(laterTab.tabId); + expect(sessions.map((session) => session.tabId)).not.toContain(existingTab.tabId); + + // A person's desktop tab keeps its popup windows to itself. + desktopRendersNext = true; + const human = yield* manager.open({ threadId: scope.thread.threadId, runtime: "server" }); + yield* broker.invoke({ scope, tabId: human.tabId, operation: "status", input: {} }); + expect(desktopEndpoints.at(-1)).toEqual({ tabId: human.tabId, popups: false }); + }), + ).pipe(Effect.provide(layer)), +); + it.effect("a desktop page that loaded before the server attached reports its loaded title", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index e03d7e3b35db1..549aed8961daa 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -484,7 +484,12 @@ const make = Effect.gen(function* () { >(); const closedPendingTabs = new Set(); - const adoptedPages = new Map(); + const adoptedPages = new Map< + string, + { readonly page: Page; readonly openerTabId: string; readonly native: boolean } + >(); + /** A desktop popup can reach adoption from both its `popup` event and the connect sweep. */ + const adoptedPopups = new WeakSet(); const closedSessions = new Set(); let hostConnectionId: string | null = null; let viewerResizeOrder = 0; @@ -766,7 +771,10 @@ const make = Effect.gen(function* () { try { const endpoint = await runPromise( desktopChannel - .endpoint({ threadId: snapshot.threadId, tabId: snapshot.tabId }) + .endpoint( + { threadId: snapshot.threadId, tabId: snapshot.tabId }, + { popups: snapshot.automationOwner !== undefined }, + ) .pipe(Scope.provide(scope)), ); const connected = await contexts.connectDesktopPage(endpoint); @@ -800,13 +808,17 @@ const make = Effect.gen(function* () { ): Promise => { const adopted = adoptedPages.get(tabKey(snapshot.threadId, snapshot.tabId)); adoptedPages.delete(tabKey(snapshot.threadId, snapshot.tabId)); - const desktop = + const connection = adopted === undefined && (await desktopRenders(snapshot)) ? await connectDesktop(snapshot) : null; + // A desktop popup window shares its opener's connection, so closing it closes only its page. + const desktop = + connection ?? + (adopted?.native ? { page: adopted.page, close: () => adopted.page.close() } : null); if (desktop) opened.push(desktop.close); // Otherwise a desktop that attaches late shows its own page, and the agent drives another. - if (!desktop && desktopChannel.available) { + if (!connection && desktopChannel.available) { await runPromise( manager .markHeadless({ threadId: ThreadId.make(snapshot.threadId), tabId: snapshot.tabId }) @@ -934,7 +946,7 @@ const make = Effect.gen(function* () { if (!desktop) void recordFrameCopies(tab, frame); }); // An adopted popup's frames may have loaded before the tab listened. - if (adopted) for (const frame of page.frames()) void recordFrameCopies(tab, frame); + if (adopted && !desktop) for (const frame of page.frames()) void recordFrameCopies(tab, frame); page.on("requestfailed", (request) => { const errorText = request.failure()?.errorText ?? ""; pushBounded(tab.networkEntries, { @@ -1011,6 +1023,15 @@ const make = Effect.gen(function* () { reportLiveTabs(); // A desktop page navigates on its own and can finish loading before these listeners attach. if (desktop) void page.waitForLoadState("load").then(() => reportLoaded(tab), constVoid); + // Popups opened before the connection fired their `popup` event before the listener above. + if (connection) { + for (const candidate of page.context().pages()) { + if (candidate === page) continue; + void candidate.opener().then((opener) => { + if (opener === page) return adoptPopup(tab, candidate); + }, constVoid); + } + } // Existing desktop and popup PDFs can start loading before interception attaches. if ( snapshot.navStatus._tag === "Loading" && @@ -1321,6 +1342,8 @@ const make = Effect.gen(function* () { }; const adoptPopup = async (opener: ServerTab, popup: Page) => { + if (adoptedPopups.has(popup)) return; + adoptedPopups.add(popup); if (opener.closing) { await popup.close().catch(constVoid); return; @@ -1339,10 +1362,13 @@ const make = Effect.gen(function* () { ...(opener.control.agentId === null ? {} : { automationOwner: opener.control.agentId, reveal: false }), + // The popup's page already exists; a desktop hosting its own webview would load it again. + headless: desktopChannel.available, beforePublish: (snapshot) => adoptedPages.set(tabKey(snapshot.threadId, snapshot.tabId), { page: popup, openerTabId: opener.tabId, + native: opener.desktop !== null, }), }), ).then( diff --git a/packages/contracts/src/desktopBrowser.ts b/packages/contracts/src/desktopBrowser.ts index 17d256f0d8d3e..3484acb32960d 100644 --- a/packages/contracts/src/desktopBrowser.ts +++ b/packages/contracts/src/desktopBrowser.ts @@ -18,6 +18,8 @@ export type DesktopBrowserEvent = typeof DesktopBrowserEvent.Type; export const DesktopBrowserCommand = Schema.Union([ Schema.Struct({ type: Schema.Literal("cdp"), ...TabKey, message: Schema.String }), Schema.Struct({ type: Schema.Literal("release"), ...TabKey }), + /** The server drives this tab for an agent, so the popup windows it opens become tabs too. */ + Schema.Struct({ type: Schema.Literal("popups"), ...TabKey }), Schema.Struct({ type: Schema.Literal("pointer"), ...TabKey, diff --git a/packages/contracts/src/preview.ts b/packages/contracts/src/preview.ts index 1790a4797db29..7094c0ab66370 100644 --- a/packages/contracts/src/preview.ts +++ b/packages/contracts/src/preview.ts @@ -160,7 +160,10 @@ export const PreviewSessionSnapshot = Schema.Struct({ /** Fixed at open; Electron cannot change a webview partition after attach without losing page state. */ profileId: Schema.optional(BrowserProfileId), runtime: Schema.optional(PreviewRuntime), - /** The server runs this tab in headless Chromium, so the desktop streams it like other clients. */ + /** + * The desktop streams this tab like other clients: the server runs it in headless Chromium, + * or it is a popup window another desktop tab opened. + */ headless: Schema.optional(Schema.Boolean), automationOwner: Schema.optional(Schema.String), reveal: Schema.optional(Schema.Boolean), From 0ae0d339bc9d57fc24b0b7e3cbfd1e96a0bbb2c1 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:38:53 +0000 Subject: [PATCH 02/24] fix(desktop): retain popups rejected by tab limits --- apps/server/src/preview/ServerBrowser.test.ts | 19 ++++++++++++------- apps/server/src/preview/ServerBrowser.ts | 2 +- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index e2c60391970bd..47302fcf228ad 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -2244,12 +2244,17 @@ it.live("profiles reported by one client survive another client's report", () => ); it.live.each([ - { budget: "agent", limit: 8 }, - { budget: "server", limit: 32 }, -])("opens and popups reserve the $budget budget before session creation", ({ budget, limit }) => + { budget: "agent", limit: 8, host: "server" }, + { budget: "server", limit: 32, host: "server" }, + { budget: "agent", limit: 8, host: "desktop" }, + { budget: "server", limit: 32, host: "desktop" }, +])("reserves the $budget budget for opens and $host popups", ({ budget, limit, host }) => Effect.scoped( Effect.gen(function* () { + desktopRendersNext = host === "desktop"; const { broker, browser, tabId } = yield* ready; + const openerPage = + host === "desktop" ? desktopConnections[0]!.context.page : contexts[0]!.page; const manager = yield* Manager.PreviewManager; const open = (target = scope) => broker.invoke({ @@ -2293,7 +2298,7 @@ it.live.each([ await launchGate.promise; return newCDPSession(target); }; - contexts[0]!.page.emit("popup", popup.page); + openerPage.emit("popup", popup.page); const last = yield* open().pipe(Effect.result, Effect.forkScoped); const overflow = yield* open().pipe(Effect.result, Effect.forkScoped); // The host processes this after the opens, while manager creation is blocked. @@ -2321,7 +2326,7 @@ it.live.each([ // The same slots must stay reserved after publication, while pages are still opening. const duringLaunch = yield* open().pipe(Effect.result, Effect.forkScoped); const rejectedPopup = makeContext(); - contexts[0]!.page.emit("popup", rejectedPopup.page); + openerPage.emit("popup", rejectedPopup.page); yield* broker.invoke({ scope, tabId, operation: "status", input: {} }); launchGate.resolve(); @@ -2333,13 +2338,13 @@ it.live.each([ failure: { _tag: "PreviewAutomationControlInterruptedError", reason: "tabLimit" }, }); } - yield* Effect.promise(() => resized.promise); + if (host === "server") yield* Effect.promise(() => resized.promise); const { sessions } = yield* manager.list({ threadId: scope.thread.threadId }); for (const session of sessions) yield* browser.attachViewer(viewerInput(session.tabId, false)); expect(sessions).toHaveLength(limit); expect(popup.page.isClosed()).toBe(false); - expect(rejectedPopup.page.isClosed()).toBe(true); + expect(rejectedPopup.page.isClosed()).toBe(host === "server"); const status = yield* broker.invoke({ scope, tabId, diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 549aed8961daa..57cf8337604b0 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -1376,7 +1376,7 @@ const make = Effect.gen(function* () { if (opener.control.agentId === null) controller?.push({ _tag: "popup", tabId: tab.tabId }); }, async () => { - await popup.close().catch(constVoid); + if (opener.desktop === null) await popup.close().catch(constVoid); }, ); }; From f34f0a90c4244f489a47929268d7b91005545277 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:56:11 +0000 Subject: [PATCH 03/24] fix(desktop): preserve native popups when setup fails --- apps/server/src/preview/ServerBrowser.test.ts | 12 ++++++++++++ apps/server/src/preview/ServerBrowser.ts | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 47302fcf228ad..f45301466db38 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -698,6 +698,18 @@ it.effect("an agent's desktop tab's popup windows become its tabs, drawn by the expect(sessions.map((session) => session.tabId)).toContain(laterTab.tabId); expect(sessions.map((session) => session.tabId)).not.toContain(existingTab.tabId); + const failed = makeContext(undefined, "https://example.com/failed-setup"); + failed.newCDPSession = async () => { + throw new Error("Popup debugger setup failed"); + }; + openerPage.emit("popup", failed.page); + const failedTab = yield* nextOpened; + let closed = yield* PubSub.take(events); + while (closed.type !== "closed" || closed.tabId !== failedTab.tabId) + closed = yield* PubSub.take(events); + expect(failed.page.isClosed()).toBe(false); + expect(releasedDesktopTabs).toEqual([]); + // A person's desktop tab keeps its popup windows to itself. desktopRendersNext = true; const human = yield* manager.open({ threadId: scope.thread.threadId, runtime: "server" }); diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 57cf8337604b0..bc3af66b7de0d 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -816,7 +816,7 @@ const make = Effect.gen(function* () { const desktop = connection ?? (adopted?.native ? { page: adopted.page, close: () => adopted.page.close() } : null); - if (desktop) opened.push(desktop.close); + if (connection) opened.push(connection.close); // Otherwise a desktop that attaches late shows its own page, and the agent drives another. if (!connection && desktopChannel.available) { await runPromise( @@ -1015,6 +1015,7 @@ const make = Effect.gen(function* () { const key = tabKey(tab.threadId, tab.tabId); if (closedPendingTabs.delete(key)) { await control.close().catch(constVoid); + if (adopted?.native) await page.close().catch(constVoid); throw new ServerBrowserTabNotFoundError({ threadId: tab.threadId, tabId: tab.tabId }); } page.on("close", () => dropTab(tab, true)); From 6a4ba28c072075e587a0232c3c5b34015301a06c Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Wed, 4 Nov 2026 11:21:50 +0000 Subject: [PATCH 04/24] fix(desktop): detach popup child debugger sessions --- apps/desktop/src/preview/CdpRelay.test.ts | 55 +++++++++++++++++++ apps/desktop/src/preview/CdpRelay.ts | 25 +++++++-- .../src/preview/DesktopBrowserHost.test.ts | 5 +- 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index 85095b5dac270..65373add12ab3 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -53,6 +53,7 @@ describe("CDP relay", () => { ); const written: Array> = []; const relay = makeRelay(makeTarget(send), (raw) => written.push(JSON.parse(raw))); + relay.event("Target.attachedToTarget", { sessionId: "iframe-session" }, ""); relay.receive( JSON.stringify({ id: 5, method: "Page.captureScreenshot", sessionId: "t3-preview-page" }), ); @@ -76,6 +77,7 @@ describe("CDP relay", () => { it("resumes paused child frames, so cross-site iframes run", async () => { const send = vi.fn(async () => ({})); const relay = makeRelay(makeTarget(send), () => {}); + relay.event("Target.attachedToTarget", { sessionId: "iframe-session" }, ""); relay.receive( JSON.stringify({ id: 1, @@ -185,6 +187,59 @@ describe("CDP relay popups", () => { const resultOf = (written: Array>, id: number) => written.find((message) => message["id"] === id)!; + it.each(["browser", "popup", "child"])( + "detaches a popup child through its %s session without affecting the other sessions", + async (source) => { + const pageSend = vi.fn(async () => ({})); + const popupSend = vi.fn(async () => ({})); + const written: Array> = []; + const relay = makeRelay(makeTarget(pageSend), (raw) => written.push(JSON.parse(raw))); + const popup = relay.addPopup(makePopup("POPUP", popupSend)); + relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach" })); + await settle(); + const popupSession = "t3-preview-popup-1"; + popup.event("Target.attachedToTarget", { sessionId: "parent-frame" }, ""); + popup.event("Target.attachedToTarget", { sessionId: "popup-child" }, "parent-frame"); + popup.event("Target.attachedToTarget", { sessionId: "sibling-frame" }, ""); + relay.receive(JSON.stringify({ id: 2, method: "DOM.enable", sessionId: "popup-child" })); + await settle(); + expect(popupSend).toHaveBeenLastCalledWith("DOM.enable", {}, "popup-child", undefined); + + const sessionId = + source === "browser" ? undefined : source === "popup" ? popupSession : "parent-frame"; + relay.receive( + JSON.stringify({ + id: 3, + method: "Target.detachFromTarget", + params: { sessionId: "popup-child" }, + sessionId, + }), + ); + await settle(); + expect(popupSend).toHaveBeenLastCalledWith( + "Target.detachFromTarget", + { sessionId: "popup-child" }, + "parent-frame", + undefined, + ); + expect(resultOf(written, 3)).toHaveProperty("result"); + + popupSend.mockClear(); + relay.receive(JSON.stringify({ id: 4, method: "DOM.enable", sessionId: "popup-child" })); + relay.receive(JSON.stringify({ id: 5, method: "Page.enable", sessionId: popupSession })); + relay.receive(JSON.stringify({ id: 6, method: "DOM.enable", sessionId: "sibling-frame" })); + relay.receive(JSON.stringify({ id: 7, method: "Page.enable", sessionId: "t3-preview-page" })); + await settle(); + expect(resultOf(written, 4)).toHaveProperty("error"); + expect(popupSend.mock.calls).toEqual([ + ["Page.enable", {}, undefined, undefined], + ["DOM.enable", {}, "sibling-frame", undefined], + ]); + expect(pageSend.mock.calls).toEqual([["Page.enable", {}, undefined, undefined]]); + for (const id of [5, 6, 7]) expect(resultOf(written, id)).toHaveProperty("result"); + }, + ); + it("announces the page's popups after it, as its popups, and routes them separately", async () => { const pageSend = vi.fn(async () => ({})); const earlySend = vi.fn(async () => ({})); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index a58a16d5e6b7f..f1f4b463ba6fe 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -140,8 +140,10 @@ export function createCdpRelayConnection( const sessions = new Map([ [PAGE_SESSION_ID, { kind: "page", owner: page, parent: undefined }], ]); - /** Child frame and worker sessions, by the debugger that attached them; others are the page's. */ - const children = new Map(); + const children = new Map< + string, + { readonly owner: RelayedTarget; readonly parent: string | undefined } + >(); let sessionSequence = 0; const newSession = (session: RelaySession) => { const sessionId = `t3-preview-${session.kind}-${++sessionSequence}`; @@ -246,7 +248,16 @@ export function createCdpRelayConnection( case "Target.detachFromTarget": { const sessionId = command.params?.["sessionId"]; const session = typeof sessionId === "string" ? sessions.get(sessionId) : undefined; - if (session?.kind === "page" && sessionId !== session.owner.sessionId) { + const child = typeof sessionId === "string" ? children.get(sessionId) : undefined; + if (child !== undefined) { + await child.owner.target.send( + command.method, + command.params ?? {}, + child.parent, + undefined, + ); + children.delete(sessionId as string); + } else if (session?.kind === "page" && sessionId !== session.owner.sessionId) { sessions.delete(sessionId as string); session.owner.target.sessionDetached?.(sessionId as string); } else if (session?.kind === "browser") { @@ -275,7 +286,9 @@ export function createCdpRelayConnection( const pageCommand = (command: CdpCommand): Promise => { const session = sessions.get(command.sessionId!); const child = session?.kind === "page" ? undefined : command.sessionId; - const owner = session?.kind === "page" ? session.owner : (children.get(child!) ?? page); + const owner = session?.kind === "page" ? session.owner : children.get(child!)?.owner; + if (owner === undefined) return Promise.reject(new Error("Unknown or detached CDP session")); + if (command.method === "Target.detachFromTarget") return browserCommand(command); const extra = child === undefined && command.sessionId !== owner.sessionId; // Child frames/workers pause on auto-attach until resumed; only a main page can skip this command. if (command.method === "Runtime.runIfWaitingForDebugger" && child === undefined) { @@ -296,7 +309,7 @@ export function createCdpRelayConnection( (method, params, sessionId) => { const child = (params as { sessionId?: unknown } | null | undefined)?.sessionId; if (method === "Target.attachedToTarget" && typeof child === "string") { - children.set(child, relayed); + children.set(child, { owner: relayed, parent: sessionId || undefined }); } else if (method === "Target.detachedFromTarget" && typeof child === "string") { children.delete(child); } @@ -321,7 +334,7 @@ export function createCdpRelayConnection( if (relayed.removed) return; relayed.removed = true; targets.delete(relayed); - for (const [id, owner] of children) if (owner === relayed) children.delete(id); + for (const [id, child] of children) if (child.owner === relayed) children.delete(id); const targetId = relayed.targetId; if (targetId === null) return; for (const [id, session] of sessions) { diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index e27d4fc256ed1..810695df59545 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -678,6 +678,7 @@ describe("DesktopBrowserHost", () => { yield* settle; debuggee.release(); expect(yield* Queue.take(replies)).toBe(1); + debuggee.emit("Target.attachedToTarget", { sessionId: "child-frame" }); yield* raw({ id: 2, method: "Page.startScreencast", sessionId: "child-frame" }); expect(yield* Queue.take(replies)).toBe(2); yield* TestClock.adjust("1 minute"); @@ -703,11 +704,13 @@ describe("DesktopBrowserHost", () => { }); expect(yield* Queue.take(replies)).toBe(2); yield* raw({ id: 3, method: "Page.startScreencast", sessionId: "t3-preview-page-1" }); + expect(yield* Queue.take(replies)).toBe(3); yield* settle; debuggee.release(); yield* settle; yield* TestClock.adjust("1 minute"); - expect(debuggee.focusEmulation).toEqual([true, false]); + expect(debuggee.focusEmulation).toEqual([]); + expect(debuggee.sent).toEqual([]); }).pipe(Effect.scoped), ); From 65b0b5904801dbe37f0b24fd36ba450917e7fdf8 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Wed, 4 Nov 2026 12:04:45 +0000 Subject: [PATCH 05/24] fix(desktop): detach announced popup relay sessions --- apps/desktop/src/preview/CdpRelay.test.ts | 37 +++++++++++++++++++++++ apps/desktop/src/preview/CdpRelay.ts | 2 +- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index 65373add12ab3..c9a731866e9bd 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -187,6 +187,43 @@ describe("CDP relay popups", () => { const resultOf = (written: Array>, id: number) => written.find((message) => message["id"] === id)!; + it("detaches an announced popup session without closing its native window or detaching the opener", async () => { + const pageSend = vi.fn(async () => ({})); + const popupSend = vi.fn(async () => ({})); + const closePopup = vi.fn(); + const written: Array> = []; + const relay = makeRelay(makeTarget(pageSend), (raw) => written.push(JSON.parse(raw))); + relay.addPopup(makePopup("POPUP", popupSend, closePopup)); + relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach" })); + await settle(); + relay.receive( + JSON.stringify({ + id: 2, + method: "Target.detachFromTarget", + params: { sessionId: "t3-preview-popup-1" }, + }), + ); + relay.receive( + JSON.stringify({ + id: 3, + method: "Target.detachFromTarget", + params: { sessionId: "t3-preview-page" }, + }), + ); + await settle(); + expect(resultOf(written, 2)).toHaveProperty("result"); + relay.receive( + JSON.stringify({ id: 4, method: "Runtime.evaluate", sessionId: "t3-preview-popup-1" }), + ); + relay.receive(JSON.stringify({ id: 5, method: "Page.enable", sessionId: "t3-preview-page" })); + await settle(); + expect(resultOf(written, 4)).toHaveProperty("error"); + expect(popupSend).not.toHaveBeenCalled(); + expect(closePopup).not.toHaveBeenCalled(); + expect(pageSend).toHaveBeenCalledExactlyOnceWith("Page.enable", {}, undefined, undefined); + expect(resultOf(written, 5)).toHaveProperty("result"); + }); + it.each(["browser", "popup", "child"])( "detaches a popup child through its %s session without affecting the other sessions", async (source) => { diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index f1f4b463ba6fe..141e43e55984f 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -257,7 +257,7 @@ export function createCdpRelayConnection( undefined, ); children.delete(sessionId as string); - } else if (session?.kind === "page" && sessionId !== session.owner.sessionId) { + } else if (session?.kind === "page" && sessionId !== PAGE_SESSION_ID) { sessions.delete(sessionId as string); session.owner.target.sessionDetached?.(sessionId as string); } else if (session?.kind === "browser") { From b73700ef7a8d98b5f2a8a39e8e198acfe9741423 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Wed, 4 Nov 2026 12:24:20 +0000 Subject: [PATCH 06/24] fix(desktop): release detached popup capture holds --- apps/desktop/src/preview/CdpRelay.test.ts | 4 +-- apps/desktop/src/preview/CdpRelay.ts | 2 +- .../src/preview/DesktopBrowserHost.test.ts | 25 +++++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index c9a731866e9bd..6e23eea331dd9 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -269,7 +269,7 @@ describe("CDP relay popups", () => { await settle(); expect(resultOf(written, 4)).toHaveProperty("error"); expect(popupSend.mock.calls).toEqual([ - ["Page.enable", {}, undefined, undefined], + ["Page.enable", {}, undefined, popupSession], ["DOM.enable", {}, "sibling-frame", undefined], ]); expect(pageSend.mock.calls).toEqual([["Page.enable", {}, undefined, undefined]]); @@ -358,7 +358,7 @@ describe("CDP relay popups", () => { relay.receive(JSON.stringify({ id: 9, method: "Page.startScreencast", sessionId: extra })); relay.receive(JSON.stringify({ id: 10, method: "Page.enable", sessionId: "t3-preview-page" })); await settle(); - expect(earlySend.mock.calls.at(-1)).toEqual(["Page.enable", {}, undefined, undefined]); + expect(earlySend.mock.calls.at(-1)).toEqual(["Page.enable", {}, undefined, earlySession]); expect(lateSend.mock.calls.at(-1)).toEqual(["Page.startScreencast", {}, undefined, extra]); expect(pageSend.mock.calls.at(-1)).toEqual(["Page.enable", {}, undefined, undefined]); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index 141e43e55984f..3cead3218793a 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -289,7 +289,7 @@ export function createCdpRelayConnection( const owner = session?.kind === "page" ? session.owner : children.get(child!)?.owner; if (owner === undefined) return Promise.reject(new Error("Unknown or detached CDP session")); if (command.method === "Target.detachFromTarget") return browserCommand(command); - const extra = child === undefined && command.sessionId !== owner.sessionId; + const extra = child === undefined && command.sessionId !== PAGE_SESSION_ID; // Child frames/workers pause on auto-attach until resumed; only a main page can skip this command. if (command.method === "Runtime.runIfWaitingForDebugger" && child === undefined) { return Promise.resolve({}); diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 810695df59545..3248c9f2ea974 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -1092,6 +1092,31 @@ describe("DesktopBrowserHost", () => { expect(host.placeDownload(late.tab.webContents, item)).toBe(true); expect(paths).toEqual(["/srv/downloads/guid-late"]); + for (const id of [5, 6, 7]) { + yield* cdp(key, { + id, + method: id === 7 ? "Page.startScreencast" : "Page.captureScreenshot", + params: { id }, + sessionId: earlySession, + }); + } + yield* settle; + yield* cdp(key, { + id: 8, + method: "Target.detachFromTarget", + params: { sessionId: earlySession }, + }); + let detached = yield* nextCdp; + while (detached["id"] !== 8) detached = yield* nextCdp; + yield* TestClock.adjust("1 minute"); + expect(early.focusEmulation).toEqual([true, false]); + early.release(); + let queued = yield* nextCdp; + while (queued["id"] !== 6) queued = yield* nextCdp; + expect(queued).toHaveProperty("error"); + expect(early.sent.filter((id) => id === 5 || id === 6)).toEqual([5]); + expect(early.tab.webContents.close).not.toHaveBeenCalled(); + late.destroy(); let closed = yield* nextCdp; while (closed["method"] !== "Target.detachedFromTarget") closed = yield* nextCdp; From d9272c7fc37465b46f1cbfa79e5c0ecf582b37a4 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Wed, 4 Nov 2026 12:32:51 +0000 Subject: [PATCH 07/24] fix(desktop): notify clients when popup sessions detach --- apps/desktop/src/preview/CdpRelay.test.ts | 32 ++++++++++++++++++- apps/desktop/src/preview/CdpRelay.ts | 16 ++++++++-- .../src/preview/DesktopBrowserHost.test.ts | 10 +++--- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index 6e23eea331dd9..bbf92ef2e0e28 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -169,6 +169,20 @@ describe("CDP relay extra sessions", () => { written.length = 0; relay.event("Page.screencastFrame", { data: "x" }, undefined); expect(written.map((m) => m["sessionId"])).toEqual(["t3-preview-page", extra]); + relay.receive( + JSON.stringify({ + id: 6, + method: "Target.detachFromTarget", + params: { sessionId: extra }, + sessionId: browserSession, + }), + ); + await settle(); + expect(written).toContainEqual({ + method: "Target.detachedFromTarget", + params: { sessionId: extra, targetId: "GUEST-TARGET" }, + sessionId: browserSession, + }); }); }); @@ -193,7 +207,7 @@ describe("CDP relay popups", () => { const closePopup = vi.fn(); const written: Array> = []; const relay = makeRelay(makeTarget(pageSend), (raw) => written.push(JSON.parse(raw))); - relay.addPopup(makePopup("POPUP", popupSend, closePopup)); + const popup = relay.addPopup(makePopup("POPUP", popupSend, closePopup)); relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach" })); await settle(); relay.receive( @@ -212,6 +226,10 @@ describe("CDP relay popups", () => { ); await settle(); expect(resultOf(written, 2)).toHaveProperty("result"); + expect(written).toContainEqual({ + method: "Target.detachedFromTarget", + params: { sessionId: "t3-preview-popup-1", targetId: "POPUP" }, + }); relay.receive( JSON.stringify({ id: 4, method: "Runtime.evaluate", sessionId: "t3-preview-popup-1" }), ); @@ -222,6 +240,18 @@ describe("CDP relay popups", () => { expect(closePopup).not.toHaveBeenCalled(); expect(pageSend).toHaveBeenCalledExactlyOnceWith("Page.enable", {}, undefined, undefined); expect(resultOf(written, 5)).toHaveProperty("result"); + relay.receive(JSON.stringify({ id: 6, method: "Target.getTargets" })); + await settle(); + expect(resultOf(written, 6)["result"]).toMatchObject({ + targetInfos: [ + { targetId: "GUEST-TARGET", attached: true }, + { targetId: "POPUP", attached: false }, + ], + }); + popup.remove(); + expect( + written.filter((message) => message["method"] === "Target.detachedFromTarget"), + ).toHaveLength(1); }); it.each(["browser", "popup", "child"])( diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index 3cead3218793a..c0f21d7873189 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -156,7 +156,9 @@ export function createCdpRelayConnection( type: "page", title: relayed.target.title(), url: relayed.target.url(), - attached: true, + attached: [...sessions.values()].some( + (session) => session.kind === "page" && session.owner === relayed, + ), ...(relayed === page ? { canAccessOpener: false } : { canAccessOpener: true, openerId: await target.targetId() }), @@ -167,7 +169,11 @@ export function createCdpRelayConnection( sessions.set(relayed.sessionId, { kind: "page", owner: relayed, parent: undefined }); return { method: "Target.attachedToTarget", - params: { sessionId: relayed.sessionId, targetInfo: info, waitingForDebugger: false }, + params: { + sessionId: relayed.sessionId, + targetInfo: { ...info, attached: true }, + waitingForDebugger: false, + }, }; }; /** Popups follow the page, one at a time, so Playwright already knows their opener. */ @@ -258,8 +264,14 @@ export function createCdpRelayConnection( ); children.delete(sessionId as string); } else if (session?.kind === "page" && sessionId !== PAGE_SESSION_ID) { + const targetId = session.owner.targetId ?? (await session.owner.target.targetId()); sessions.delete(sessionId as string); session.owner.target.sessionDetached?.(sessionId as string); + send({ + method: "Target.detachedFromTarget", + params: { sessionId, targetId }, + ...(session.parent === undefined ? {} : { sessionId: session.parent }), + }); } else if (session?.kind === "browser") { sessions.delete(sessionId as string); } diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 3248c9f2ea974..0c6faba0970be 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -14,7 +14,7 @@ import * as DesktopBrowserHost from "./DesktopBrowserHost.ts"; const decodeEvent = Schema.decodeUnknownSync(Schema.fromJsonString(DesktopBrowserEvent)); const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); const decodeCdpReply = Schema.decodeUnknownSync( - Schema.fromJsonString(Schema.Struct({ id: Schema.Number })), + Schema.fromJsonString(Schema.Struct({ id: Schema.optionalKey(Schema.Number) })), ); const key = { threadId: "thread-1", tabId: "tab-1" }; /** Sends a page command whose id the fake debugger records as it receives it. */ @@ -130,9 +130,11 @@ const watchReplies = (host: DesktopBrowserHost.DesktopBrowserHost["Service"]) => 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, - ), + Stream.runForEach((event) => { + if (event.type !== "cdp") return Effect.void; + const { id } = decodeCdpReply(event.message); + return id === undefined ? Effect.void : Queue.offer(replies, id); + }), Effect.forkScoped, ); yield* Effect.yieldNow; From 57eeb3f4407ef33e93a665cbd54426be24e57b25 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Wed, 4 Nov 2026 13:11:07 +0000 Subject: [PATCH 08/24] fix(desktop): revoke detached popup child routes --- apps/desktop/src/preview/CdpRelay.test.ts | 179 +++++++++++++++------- apps/desktop/src/preview/CdpRelay.ts | 22 ++- 2 files changed, 144 insertions(+), 57 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index bbf92ef2e0e28..666018a8e33ab 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -124,6 +124,7 @@ describe("CDP relay", () => { expect(written).toEqual([]); relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach", params: {} })); await settle(); + relay.event("Target.attachedToTarget", { sessionId: "iframe-session" }, ""); relay.event("Page.loadEventFired", { timestamp: 1 }, ""); relay.event("Runtime.consoleAPICalled", {}, "iframe-session"); expect(written.slice(-2)).toEqual([ @@ -201,58 +202,124 @@ describe("CDP relay popups", () => { const resultOf = (written: Array>, id: number) => written.find((message) => message["id"] === id)!; - it("detaches an announced popup session without closing its native window or detaching the opener", async () => { - const pageSend = vi.fn(async () => ({})); - const popupSend = vi.fn(async () => ({})); - const closePopup = vi.fn(); - const written: Array> = []; - const relay = makeRelay(makeTarget(pageSend), (raw) => written.push(JSON.parse(raw))); - const popup = relay.addPopup(makePopup("POPUP", popupSend, closePopup)); - relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach" })); - await settle(); - relay.receive( - JSON.stringify({ - id: 2, - method: "Target.detachFromTarget", - params: { sessionId: "t3-preview-popup-1" }, - }), - ); - relay.receive( - JSON.stringify({ - id: 3, - method: "Target.detachFromTarget", - params: { sessionId: "t3-preview-page" }, - }), - ); - await settle(); - expect(resultOf(written, 2)).toHaveProperty("result"); - expect(written).toContainEqual({ - method: "Target.detachedFromTarget", - params: { sessionId: "t3-preview-popup-1", targetId: "POPUP" }, - }); - relay.receive( - JSON.stringify({ id: 4, method: "Runtime.evaluate", sessionId: "t3-preview-popup-1" }), - ); - relay.receive(JSON.stringify({ id: 5, method: "Page.enable", sessionId: "t3-preview-page" })); - await settle(); - expect(resultOf(written, 4)).toHaveProperty("error"); - expect(popupSend).not.toHaveBeenCalled(); - expect(closePopup).not.toHaveBeenCalled(); - expect(pageSend).toHaveBeenCalledExactlyOnceWith("Page.enable", {}, undefined, undefined); - expect(resultOf(written, 5)).toHaveProperty("result"); - relay.receive(JSON.stringify({ id: 6, method: "Target.getTargets" })); - await settle(); - expect(resultOf(written, 6)["result"]).toMatchObject({ - targetInfos: [ - { targetId: "GUEST-TARGET", attached: true }, - { targetId: "POPUP", attached: false }, - ], - }); - popup.remove(); - expect( - written.filter((message) => message["method"] === "Target.detachedFromTarget"), - ).toHaveLength(1); - }); + it.each(["announced", "extra"])( + "revokes popup children when its last %s page session detaches without affecting other targets", + async (last) => { + const pageSend = vi.fn(async () => ({})); + const popupSend = vi.fn(async () => ({})); + const siblingSend = vi.fn(async () => ({})); + const closePopup = vi.fn(); + const written: Array> = []; + const relay = makeRelay(makeTarget(pageSend), (raw) => written.push(JSON.parse(raw))); + const popup = relay.addPopup(makePopup("POPUP", popupSend, closePopup)); + const sibling = relay.addPopup(makePopup("SIBLING", siblingSend)); + relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach" })); + await settle(); + relay.receive( + JSON.stringify({ id: 2, method: "Target.attachToTarget", params: { targetId: "POPUP" } }), + ); + await settle(); + const extra = (resultOf(written, 2)["result"] as { sessionId: string }).sessionId; + const firstSession = last === "announced" ? extra : "t3-preview-popup-1"; + const lastSession = last === "announced" ? "t3-preview-popup-1" : extra; + relay.event("Target.attachedToTarget", { sessionId: "opener-child" }, ""); + sibling.event("Target.attachedToTarget", { sessionId: "sibling-child" }, ""); + popup.event("Target.attachedToTarget", { sessionId: "popup-child" }, ""); + popup.event("Target.attachedToTarget", { sessionId: "nested-child" }, "popup-child"); + relay.receive( + JSON.stringify({ + id: 3, + method: "Target.detachFromTarget", + params: { sessionId: firstSession }, + }), + ); + await settle(); + relay.receive(JSON.stringify({ id: 4, method: "DOM.enable", sessionId: "popup-child" })); + await settle(); + expect(resultOf(written, 4)).toHaveProperty("result"); + expect(popupSend).toHaveBeenCalledExactlyOnceWith("DOM.enable", {}, "popup-child", undefined); + const childEvent = { + method: "Runtime.consoleAPICalled", + params: {}, + sessionId: "popup-child", + }; + popup.event(childEvent.method, childEvent.params, childEvent.sessionId); + expect(written.at(-1)).toEqual(childEvent); + popupSend.mockClear(); + + relay.receive( + JSON.stringify({ + id: 5, + method: "Target.detachFromTarget", + params: { sessionId: lastSession }, + }), + ); + relay.receive( + JSON.stringify({ + id: 6, + method: "Target.detachFromTarget", + params: { sessionId: "t3-preview-page" }, + }), + ); + await settle(); + for (const sessionId of [firstSession, lastSession]) { + expect(written).toContainEqual({ + method: "Target.detachedFromTarget", + params: { sessionId, targetId: "POPUP" }, + }); + } + const eventCount = written.length; + popup.event(childEvent.method, childEvent.params, childEvent.sessionId); + popup.event("Runtime.consoleAPICalled", {}, "nested-child"); + popup.event("Target.attachedToTarget", { sessionId: "late-child" }, ""); + popup.event("Target.attachedToTarget", { sessionId: "late-nested-child" }, "popup-child"); + popup.event("Page.loadEventFired", {}, ""); + expect(written).toHaveLength(eventCount); + const detached = [ + lastSession, + "popup-child", + "nested-child", + "late-child", + "late-nested-child", + ]; + detached.forEach((sessionId, index) => + relay.receive(JSON.stringify({ id: 10 + index, method: "Runtime.evaluate", sessionId })), + ); + for (const [id, sessionId] of [ + [20, "t3-preview-page"], + [21, "opener-child"], + [22, "sibling-child"], + ]) + relay.receive(JSON.stringify({ id, method: "Page.enable", sessionId })); + relay.receive(JSON.stringify({ id: 23, method: "Target.getTargets" })); + await settle(); + detached.forEach((_, index) => expect(resultOf(written, 10 + index)).toHaveProperty("error")); + expect(popupSend).not.toHaveBeenCalled(); + expect(closePopup).not.toHaveBeenCalled(); + expect(pageSend.mock.calls).toEqual([ + ["Page.enable", {}, undefined, undefined], + ["Page.enable", {}, "opener-child", undefined], + ]); + expect(siblingSend).toHaveBeenCalledExactlyOnceWith( + "Page.enable", + {}, + "sibling-child", + undefined, + ); + for (const id of [20, 21, 22]) expect(resultOf(written, id)).toHaveProperty("result"); + expect(resultOf(written, 23)["result"]).toMatchObject({ + targetInfos: [ + { targetId: "GUEST-TARGET", attached: true }, + { targetId: "POPUP", attached: false }, + { targetId: "SIBLING", attached: true }, + ], + }); + popup.remove(); + expect( + written.filter((message) => message["method"] === "Target.detachedFromTarget"), + ).toHaveLength(2); + }, + ); it.each(["browser", "popup", "child"])( "detaches a popup child through its %s session without affecting the other sessions", @@ -267,6 +334,7 @@ describe("CDP relay popups", () => { const popupSession = "t3-preview-popup-1"; popup.event("Target.attachedToTarget", { sessionId: "parent-frame" }, ""); popup.event("Target.attachedToTarget", { sessionId: "popup-child" }, "parent-frame"); + popup.event("Target.attachedToTarget", { sessionId: "nested-child" }, "popup-child"); popup.event("Target.attachedToTarget", { sessionId: "sibling-frame" }, ""); relay.receive(JSON.stringify({ id: 2, method: "DOM.enable", sessionId: "popup-child" })); await settle(); @@ -296,8 +364,15 @@ describe("CDP relay popups", () => { relay.receive(JSON.stringify({ id: 5, method: "Page.enable", sessionId: popupSession })); relay.receive(JSON.stringify({ id: 6, method: "DOM.enable", sessionId: "sibling-frame" })); relay.receive(JSON.stringify({ id: 7, method: "Page.enable", sessionId: "t3-preview-page" })); + relay.receive(JSON.stringify({ id: 8, method: "DOM.enable", sessionId: "nested-child" })); await settle(); expect(resultOf(written, 4)).toHaveProperty("error"); + expect(resultOf(written, 8)).toHaveProperty("error"); + const eventCount = written.length; + popup.event("Runtime.consoleAPICalled", {}, "popup-child"); + popup.event("Runtime.consoleAPICalled", {}, "nested-child"); + popup.event("Target.attachedToTarget", { sessionId: "late-child" }, "popup-child"); + expect(written).toHaveLength(eventCount); expect(popupSend.mock.calls).toEqual([ ["Page.enable", {}, undefined, popupSession], ["DOM.enable", {}, "sibling-frame", undefined], diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index c0f21d7873189..75009e012583f 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -144,6 +144,10 @@ export function createCdpRelayConnection( string, { readonly owner: RelayedTarget; readonly parent: string | undefined } >(); + const removeChild = (sessionId: string) => { + children.delete(sessionId); + for (const [id, child] of children) if (child.parent === sessionId) removeChild(id); + }; let sessionSequence = 0; const newSession = (session: RelaySession) => { const sessionId = `t3-preview-${session.kind}-${++sessionSequence}`; @@ -151,14 +155,17 @@ export function createCdpRelayConnection( return sessionId; }; const announced = (relayed: RelayedTarget) => relayed.targetId !== null && !relayed.removed; + const hasPageSession = (relayed: RelayedTarget) => { + for (const session of sessions.values()) + if (session.kind === "page" && session.owner === relayed) return true; + return false; + }; const targetInfo = async (relayed: RelayedTarget) => ({ targetId: await relayed.target.targetId(), type: "page", title: relayed.target.title(), url: relayed.target.url(), - attached: [...sessions.values()].some( - (session) => session.kind === "page" && session.owner === relayed, - ), + attached: hasPageSession(relayed), ...(relayed === page ? { canAccessOpener: false } : { canAccessOpener: true, openerId: await target.targetId() }), @@ -262,10 +269,13 @@ export function createCdpRelayConnection( child.parent, undefined, ); - children.delete(sessionId as string); + removeChild(sessionId as string); } else if (session?.kind === "page" && sessionId !== PAGE_SESSION_ID) { const targetId = session.owner.targetId ?? (await session.owner.target.targetId()); sessions.delete(sessionId as string); + if (!hasPageSession(session.owner)) + for (const [id, child] of children) + if (child.owner === session.owner) children.delete(id); session.owner.target.sessionDetached?.(sessionId as string); send({ method: "Target.detachedFromTarget", @@ -319,11 +329,13 @@ export function createCdpRelayConnection( const eventFor = (relayed: RelayedTarget): CdpRelayEvent => (method, params, sessionId) => { + if (relayed.removed || (announced(relayed) && !hasPageSession(relayed))) return; + if (sessionId && children.get(sessionId)?.owner !== relayed) return; const child = (params as { sessionId?: unknown } | null | undefined)?.sessionId; if (method === "Target.attachedToTarget" && typeof child === "string") { children.set(child, { owner: relayed, parent: sessionId || undefined }); } else if (method === "Target.detachedFromTarget" && typeof child === "string") { - children.delete(child); + removeChild(child); } if (!announced(relayed)) return; if (method.startsWith("Page.download")) method = method.replace("Page.", "Browser."); From 9f6d46bda100488f7dce6cebcd3ea4698536c3f2 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Wed, 4 Nov 2026 14:44:45 +0000 Subject: [PATCH 09/24] fix(server): keep desktop popups across debugger recovery and their dialogs native When the desktop recovered a dropped opener debugger, the reconnect's endpoint sent `popups` before the dropped endpoint finished closing, and the dropped endpoint's unconditional `release` then dropped the new relay and hid every popup. A closing endpoint now releases only while it still owns the tab, checked under the write lock at write time. A native popup window has no page dialog listener until its tab is set up, and never if capacity rejects it, so Playwright dismissed its dialogs while the native sheet stayed up. Native popups now subscribe before any async adoption. Co-Authored-By: Claude Opus 5.5 --- .../src/preview/DesktopBrowserChannel.test.ts | 109 ++++++++++++++++++ .../src/preview/DesktopBrowserChannel.ts | 23 ++-- apps/server/src/preview/ServerBrowser.test.ts | 16 +++ apps/server/src/preview/ServerBrowser.ts | 2 + 4 files changed, 140 insertions(+), 10 deletions(-) create mode 100644 apps/server/src/preview/DesktopBrowserChannel.test.ts diff --git a/apps/server/src/preview/DesktopBrowserChannel.test.ts b/apps/server/src/preview/DesktopBrowserChannel.test.ts new file mode 100644 index 0000000000000..8f51d04c83985 --- /dev/null +++ b/apps/server/src/preview/DesktopBrowserChannel.test.ts @@ -0,0 +1,109 @@ +// @effect-diagnostics nodeBuiltinImport:off - The channel reads and writes real desktop fds. +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { it } from "@effect/vitest"; +import * as Context from "effect/Context"; +import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; +import * as Fiber from "effect/Fiber"; +import * as Layer from "effect/Layer"; +import * as Queue from "effect/Queue"; +import * as Scope from "effect/Scope"; +import * as Stream from "effect/Stream"; +import * as NodeChildProcess from "node:child_process"; +import * as NodeFS from "node:fs"; +import * as NodeNet from "node:net"; +import * as NodeOS from "node:os"; +import * as NodePath from "node:path"; +import { expect } from "vite-plus/test"; + +import * as ServerConfig from "../config.ts"; +import * as DesktopBrowserChannel from "./DesktopBrowserChannel.ts"; + +const key = { threadId: "thread-1", tabId: "tab-1" }; + +/** FIFOs stand in for the bootstrap fds the desktop passes the backend. */ +const desktopPipes = Effect.acquireRelease( + Effect.sync(() => { + const dir = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-desktop-browser-")); + const eventsPath = NodePath.join(dir, "events"); + const controlPath = NodePath.join(dir, "control"); + NodeChildProcess.execFileSync("mkfifo", [eventsPath, controlPath]); + // Read-write opens never block waiting for the other end. + const eventsFd = NodeFS.openSync(eventsPath, NodeFS.constants.O_RDWR); + const controlFd = NodeFS.openSync(controlPath, NodeFS.constants.O_RDWR); + const control = new NodeNet.Socket({ + fd: NodeFS.openSync(controlPath, NodeFS.constants.O_RDONLY | NodeFS.constants.O_NONBLOCK), + readable: true, + writable: false, + }); + return { dir, eventsFd, controlFd, control }; + }), + ({ dir, controlFd, control }) => + Effect.sync(() => { + control.destroy(); + NodeFS.closeSync(controlFd); + NodeFS.rmSync(dir, { recursive: true, force: true }); + }), +); + +it.live("a replaced endpoint's shutdown does not release the endpoint that replaced it", () => + Effect.gen(function* () { + const pipes = yield* desktopPipes; + const commands = yield* Queue.unbounded<{ readonly type: string }>(); + let buffered = ""; + pipes.control.on("data", (chunk: Buffer) => { + const lines = (buffered + chunk.toString()).split("\n"); + buffered = lines.pop() ?? ""; + for (const line of lines) Queue.offerUnsafe(commands, JSON.parse(line)); + }); + const emit = (event: object) => NodeFS.writeSync(pipes.eventsFd, `${JSON.stringify(event)}\n`); + + const config = yield* ServerConfig.ServerConfig.pipe( + Effect.provide( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-desktop-browser-channel-" }), + ), + ); + const channel = Context.get( + yield* Layer.build( + DesktopBrowserChannel.layer.pipe( + Layer.provide( + ServerConfig.layer({ + ...config, + desktopBrowserFd: pipes.eventsFd, + desktopBrowserControlFd: pipes.controlFd, + }), + ), + ), + ), + DesktopBrowserChannel.DesktopBrowserChannel, + ); + + emit({ type: "attached", ...key }); + expect(yield* channel.awaitAttached(key, "5 seconds")).toBe(true); + const first = yield* Scope.make(); + yield* channel.endpoint(key, { popups: true }).pipe(Scope.provide(first)); + expect((yield* Queue.take(commands)).type).toBe("popups"); + + // The desktop recovers a dropped debugger: the tab detaches and attaches again, and the + // reconnect opens its endpoint before the dropped connection finishes closing. + const detached = yield* channel.detached.pipe( + Stream.runHead, + Effect.forkChild({ startImmediately: true }), + ); + emit({ type: "detached", ...key }); + yield* Fiber.join(detached); + emit({ type: "attached", ...key }); + expect(yield* channel.awaitAttached(key, "5 seconds")).toBe(true); + const second = yield* Scope.make(); + yield* channel.endpoint(key, { popups: true }).pipe(Scope.provide(second)); + expect((yield* Queue.take(commands)).type).toBe("popups"); + + yield* Scope.close(first, Exit.void); + yield* Scope.close(second, Exit.void); + // Commands are written in order, so everything the two closes sent precedes this one. + yield* channel.turn(key.threadId, false); + const sent: Array = []; + while (sent.at(-1) !== "turn") sent.push((yield* Queue.take(commands)).type); + expect(sent).toEqual(["release", "turn"]); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), +); diff --git a/apps/server/src/preview/DesktopBrowserChannel.ts b/apps/server/src/preview/DesktopBrowserChannel.ts index ecc4fe76705b8..8b36c18ba4713 100644 --- a/apps/server/src/preview/DesktopBrowserChannel.ts +++ b/apps/server/src/preview/DesktopBrowserChannel.ts @@ -90,15 +90,12 @@ const make = Effect.gen(function* () { }); } - const command = (message: DesktopBrowserCommandType) => - writeLock.withPermits(1)( - encodeCommand(message).pipe( - Effect.flatMap((line) => writeAllToFileDescriptor(controlFd, Buffer.from(`${line}\n`))), - Effect.catchCause((cause) => - Effect.logWarning("desktop browser command failed", { cause }), - ), - ), + const write = (message: DesktopBrowserCommandType) => + encodeCommand(message).pipe( + Effect.flatMap((line) => writeAllToFileDescriptor(controlFd, Buffer.from(`${line}\n`))), + Effect.catchCause((cause) => Effect.logWarning("desktop browser command failed", { cause })), ); + const command = (message: DesktopBrowserCommandType) => writeLock.withPermits(1)(write(message)); // Use cancellable socket reads; filesystem pipe reads keep Node alive after scope shutdown. const readable = yield* Effect.acquireRelease( @@ -158,9 +155,15 @@ const make = Effect.gen(function* () { } yield* Effect.addFinalizer(() => Effect.gen(function* () { - if (inbound.get(id) === queue) inbound.delete(id); yield* Queue.shutdown(queue); - yield* command({ type: "release", ...key }); + // A reconnect's endpoint may already hold the tab; its release would drop the new relay. + yield* writeLock.withPermits(1)( + Effect.suspend(() => { + if (inbound.get(id) !== queue) return Effect.void; + inbound.delete(id); + return write({ type: "release", ...key }); + }), + ); }), ); // Written before any CDP, so popups already open are there when Playwright connects. diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index f45301466db38..124eaee50d872 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -164,6 +164,7 @@ function fileChooserElement(accept: string) { function makeContext(onClose?: (context: BrowserContext) => void, initialUrl = "about:blank") { const events = new NodeEvents.EventEmitter(); const nativeFileChooser = vi.fn(); + const dialogAutoDismissed = vi.fn(); const setChooserFiles = vi.fn(async (_files: unknown) => {}); const sessions: ReturnType[] = []; let url = initialUrl; @@ -178,6 +179,13 @@ function makeContext(onClose?: (context: BrowserContext) => void, initialUrl = " emit: (name: string, ...args: unknown[]) => events.emit(name, ...args), nativeFileChooser, setChooserFiles, + dialogAutoDismissed, + // Playwright dismisses a dialog nothing listens for before the page shows it. + openDialog: () => { + if (!events.emit("dialog", { type: () => "alert", message: () => "hello" })) { + dialogAutoDismissed(); + } + }, openFileChooser: () => { if ( !events.emit("filechooser", { @@ -2311,6 +2319,9 @@ it.live.each([ return newCDPSession(target); }; openerPage.emit("popup", popup.page); + // A native window's dialog waits for the person while its tab is still being set up. + popup.page.openDialog(); + expect(popup.page.dialogAutoDismissed).toHaveBeenCalledTimes(host === "desktop" ? 0 : 1); const last = yield* open().pipe(Effect.result, Effect.forkScoped); const overflow = yield* open().pipe(Effect.result, Effect.forkScoped); // The host processes this after the opens, while manager creation is blocked. @@ -2357,6 +2368,11 @@ it.live.each([ expect(sessions).toHaveLength(limit); expect(popup.page.isClosed()).toBe(false); expect(rejectedPopup.page.isClosed()).toBe(host === "server"); + if (host === "desktop") { + // No tab answers the window it could not adopt, so its dialogs stay the person's. + rejectedPopup.page.openDialog(); + expect(rejectedPopup.page.dialogAutoDismissed).not.toHaveBeenCalled(); + } const status = yield* broker.invoke({ scope, tabId, diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index bc3af66b7de0d..cf0bf54f4bb5b 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -1345,6 +1345,8 @@ const make = Effect.gen(function* () { const adoptPopup = async (opener: ServerTab, popup: Page) => { if (adoptedPopups.has(popup)) return; adoptedPopups.add(popup); + // Playwright dismisses dialogs nothing listens for; a native window's are the person's to answer. + if (opener.desktop !== null) popup.on("dialog", constVoid); if (opener.closing) { await popup.close().catch(constVoid); return; From 2888cbedb1aa394cb4ce4eaa0f482534a5821276 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Thu, 5 Nov 2026 01:32:05 +0000 Subject: [PATCH 10/24] fix(server): idle reclamation keeps desktop popup windows and their openers A full server reclaims hidden idle agent tabs. A desktop popup is the person's own native window, and closing its opener destroys it, so neither gives up its slot while the popup is open. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 25 ++++++++++++++++--- apps/server/src/preview/ServerBrowser.ts | 15 ++++++++++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index dcba9f3a3b0e1..1f1985f37a759 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -2465,11 +2465,25 @@ it.live("a full server takes back the slot of the longest idle agent tab nobody input: { reuseExistingTab: false, open: false }, }); }; + const popupEvents = yield* manager.subscribeEvents; + const popup = makeContext(undefined, "https://example.com/popout"); const opened: Array = []; - for (let index = 0; index < 32; index++) { + // The 32nd slot holds the native popup window of agent-3's first tab. + for (let index = 0; index < 31; index++) { now.mockReturnValue(index); + if (index === 24) desktopPopupsNext.push(popup); opened.push((yield* open(`agent-${Math.floor(index / 8)}`)).tabId!); } + let popupOpened = yield* PubSub.take(popupEvents); + while (popupOpened.type !== "opened" || !popupOpened.snapshot.headless) + popupOpened = yield* PubSub.take(popupEvents); + const popupTabId = popupOpened.snapshot.tabId; + yield* broker.invoke({ + scope: asSession("agent-3"), + operation: "status", + tabId: PreviewTabId.make(popupTabId), + input: {}, + }); now.mockReturnValue(100); yield* broker.invoke({ @@ -2486,12 +2500,17 @@ it.live("a full server takes back the slot of the longest idle agent tab nobody now.mockReturnValue(31 * 60 * 1000); // A session at its own limit couldn't use a freed slot, so nothing closes for it. - expect(yield* open("agent-3").pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); + expect(yield* open("agent-2").pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); expect(releasedDesktopTabs).toEqual([]); // Tabs a desktop window shows keep their slots however long they sit idle. for (const tabId of opened) shownDesktopTabs.add(tabId); expect(yield* open("agent-new").pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); expect(releasedDesktopTabs).toEqual([]); + // So does a hidden tab whose popup window is still open on the desktop. + shownDesktopTabs.delete(opened[24]!); + expect(yield* open("agent-new").pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); + expect(releasedDesktopTabs).toEqual([]); + expect(popup.page.close).not.toHaveBeenCalled(); shownDesktopTabs.clear(); shownDesktopTabs.add(opened[1]!); const events = yield* manager.subscribeEvents; @@ -2511,7 +2530,7 @@ it.live("a full server takes back the slot of the longest idle agent tab nobody expect(releasedDesktopTabs).toEqual([opened[2]]); const { sessions } = yield* manager.list({ threadId: scope.thread.threadId }); expect(sessions.map((session) => session.tabId).toSorted()).toEqual( - [...opened.filter((tabId) => tabId !== opened[2]), fresh.tabId!].toSorted(), + [...opened.filter((tabId) => tabId !== opened[2]), popupTabId, fresh.tabId!].toSorted(), ); }), ).pipe(Effect.provide(layer)), diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 40178e667941d..bd548866bb36b 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -1445,10 +1445,23 @@ const make = Effect.gen(function* () { const isVisible = (tab: ServerTab) => tab.viewers.size > 0 || (tab.desktop !== null && desktopChannel.isShown(tab)); + // A desktop popup is the person's own window, and closing its opener destroys it too. + const isNativePopup = (tab: ServerTab) => tab.desktop !== null && tab.openerTabId !== undefined; + const holdsNativePopup = (tab: ServerTab) => + isNativePopup(tab) || + [...tabs.values()].some( + (other) => + isNativePopup(other) && other.threadId === tab.threadId && other.openerTabId === tab.tabId, + ); + const idleAgentTabs = () => { const cutoff = Date.now() - AGENT_TAB_IDLE_MS; return [...tabs.values()].filter( - (tab) => tab.control.agentId !== null && !isVisible(tab) && tab.usedAt < cutoff, + (tab) => + tab.control.agentId !== null && + !isVisible(tab) && + tab.usedAt < cutoff && + !holdsNativePopup(tab), ); }; From a737bccb5017f970124a573c403442ce0dd59bf5 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Thu, 5 Nov 2026 05:24:39 +0000 Subject: [PATCH 11/24] fix(server): retain popup opener protection after failed adoption --- apps/server/src/preview/ServerBrowser.test.ts | 12 ++++++++++++ apps/server/src/preview/ServerBrowser.ts | 19 ++++++++++--------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 1f1985f37a759..e78d408c6cc75 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -2484,6 +2484,8 @@ it.live("a full server takes back the slot of the longest idle agent tab nobody tabId: PreviewTabId.make(popupTabId), input: {}, }); + const refused = makeContext(undefined, "https://example.com/retained"); + desktopConnections[8]!.context.page.emit("popup", refused.page); now.mockReturnValue(100); yield* broker.invoke({ @@ -2511,6 +2513,11 @@ it.live("a full server takes back the slot of the longest idle agent tab nobody expect(yield* open("agent-new").pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); expect(releasedDesktopTabs).toEqual([]); expect(popup.page.close).not.toHaveBeenCalled(); + shownDesktopTabs.add(opened[24]!); + shownDesktopTabs.delete(opened[8]!); + expect(yield* open("agent-new").pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); + expect(releasedDesktopTabs).toEqual([]); + expect(refused.page.close).not.toHaveBeenCalled(); shownDesktopTabs.clear(); shownDesktopTabs.add(opened[1]!); const events = yield* manager.subscribeEvents; @@ -2532,6 +2539,11 @@ it.live("a full server takes back the slot of the longest idle agent tab nobody expect(sessions.map((session) => session.tabId).toSorted()).toEqual( [...opened.filter((tabId) => tabId !== opened[2]), popupTabId, fresh.tabId!].toSorted(), ); + yield* Effect.promise(() => refused.page.close()); + for (const session of sessions) shownDesktopTabs.add(session.tabId); + shownDesktopTabs.delete(opened[8]!); + yield* open("agent-new"); + expect(yield* Queue.take(releases)).toBe(opened[8]); }), ).pipe(Effect.provide(layer)), ); diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index bd548866bb36b..6cf986cf78081 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -299,6 +299,7 @@ interface ServerTab { readonly isolatedContext: boolean; readonly desktop: { readonly close: () => Promise } | null; + readonly nativePopups: Set; readonly profileId: string | undefined; readonly openerTabId: string | undefined; @@ -877,6 +878,7 @@ const make = Effect.gen(function* () { }, isolatedContext, desktop: desktop === null ? null : { close: desktop.close }, + nativePopups: new Set(), profileId: snapshot.profileId, openerTabId: adopted?.openerTabId, downloads: [], @@ -1344,10 +1346,14 @@ const make = Effect.gen(function* () { }; const adoptPopup = async (opener: ServerTab, popup: Page) => { - if (adoptedPopups.has(popup)) return; + if (adoptedPopups.has(popup) || popup.isClosed()) return; adoptedPopups.add(popup); - // Playwright dismisses dialogs nothing listens for; a native window's are the person's to answer. - if (opener.desktop !== null) popup.on("dialog", constVoid); + if (opener.desktop !== null) { + // Keep native dialogs pending even when this popup never becomes an automation tab. + popup.on("dialog", constVoid); + opener.nativePopups.add(popup); + popup.once("close", () => opener.nativePopups.delete(popup)); + } if (opener.closing) { await popup.close().catch(constVoid); return; @@ -1447,12 +1453,7 @@ const make = Effect.gen(function* () { // A desktop popup is the person's own window, and closing its opener destroys it too. const isNativePopup = (tab: ServerTab) => tab.desktop !== null && tab.openerTabId !== undefined; - const holdsNativePopup = (tab: ServerTab) => - isNativePopup(tab) || - [...tabs.values()].some( - (other) => - isNativePopup(other) && other.threadId === tab.threadId && other.openerTabId === tab.tabId, - ); + const holdsNativePopup = (tab: ServerTab) => isNativePopup(tab) || tab.nativePopups.size > 0; const idleAgentTabs = () => { const cutoff = Date.now() - AGENT_TAB_IDLE_MS; From cf696c17b27eb2fe278a7fd4b04d24feba4a61ee Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Thu, 5 Nov 2026 05:43:10 +0000 Subject: [PATCH 12/24] fix(desktop): recover native popup access after DevTools closes --- .../src/preview/DesktopBrowserHost.test.ts | 124 ++++++++++++++++++ .../desktop/src/preview/DesktopBrowserHost.ts | 12 +- 2 files changed, 134 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 9f4db2cd67a42..9834c0fed2240 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -72,6 +72,7 @@ const makeDebuggee = ( attached: false, isAttached: () => debuggee.attached, attach: () => { + if (devToolsOpened) throw new Error("DevTools is open"); debuggee.attached = true; }, detach: vi.fn(() => { @@ -79,12 +80,14 @@ const makeDebuggee = ( }), }); let destroyed = false; + let devToolsOpened = false; const webContents = Object.assign(new NodeEvents.EventEmitter(), { debugger: debuggee, getURL: () => "http://localhost/", getTitle: () => "Page", getUserAgent: () => "Electron", isDestroyed: () => destroyed, + isDevToolsOpened: () => devToolsOpened, close: vi.fn(), }); return { @@ -94,6 +97,15 @@ const makeDebuggee = ( }, emit: (method: string, params: unknown) => emitter.emit("message", {}, method, params, ""), debuggee, + openDevTools: () => { + devToolsOpened = true; + debuggee.attached = false; + debuggee.emit("detach", {}, "replaced_with_devtools"); + }, + closeDevTools: () => { + devToolsOpened = false; + webContents.emit("devtools-closed"); + }, destroy: () => { destroyed = true; webContents.emit("destroyed"); @@ -1168,6 +1180,118 @@ describe("DesktopBrowserHost", () => { }).pipe(Effect.scoped), ); + it.effect.each(["before", "after"] as const)( + "recovers the same popup when DevTools opened %s relay attachment closes", + (when) => + Effect.gen(function* () { + const host = yield* DesktopBrowserHost.make; + const page = makeDebuggee(); + const popup = makeDebuggee({ targetId: "POPUP" }); + const sibling = makeDebuggee({ targetId: "SIBLING" }); + const attach = vi.spyOn(popup.debuggee, "attach"); + const events = yield* Queue.unbounded(); + yield* host.events.pipe( + Stream.runForEach((line) => + Queue.offer(events, decodeEvent(new TextDecoder().decode(line))), + ), + Effect.forkScoped, + ); + const cdp = (message: Record) => + host.handleCommandLine(encodeJson({ type: "cdp", ...key, message: encodeJson(message) })); + const nextCdp = Effect.gen(function* () { + let event = yield* Queue.take(events); + while (event.type !== "cdp") event = yield* Queue.take(events); + return JSON.parse(event.message) as Record; + }); + const reply = (id: number) => + Effect.gen(function* () { + let message = yield* nextCdp; + while (message["id"] !== id) message = yield* nextCdp; + return message; + }); + host.attach(key, page.tab); + expect(yield* Queue.take(events)).toEqual({ type: "attached", ...key }); + if (when === "before") popup.openDevTools(); + host.openedPopup(page.tab.webContents, popup.tab.webContents); + host.openedPopup(page.tab.webContents, sibling.tab.webContents); + yield* host.handleCommandLine(encodeJson({ type: "popups", ...key })); + yield* cdp({ id: 1, method: "Target.setAutoAttach" }); + const sessions = new Map(); + while (sessions.size < (when === "before" ? 2 : 3)) { + const message = yield* nextCdp; + if (message["method"] !== "Target.attachedToTarget") continue; + const params = message["params"] as { + sessionId: string; + targetInfo: { targetId: string }; + }; + sessions.set(params.targetInfo.targetId, params.sessionId); + } + if (when === "after") { + popup.openDevTools(); + const message = yield* nextCdp; + expect(message).toMatchObject({ + method: "Target.detachedFromTarget", + params: { sessionId: sessions.get("POPUP"), targetId: "POPUP" }, + }); + } + expect(popup.debuggee.attached).toBe(false); + attach.mockClear(); + popup.closeDevTools(); + expect(popup.debuggee.attached).toBe(true); + let restored = yield* nextCdp; + while (restored["method"] !== "Target.attachedToTarget") restored = yield* nextCdp; + const restoredParams = restored["params"] as { sessionId: string }; + expect(restored).toMatchObject({ + params: { targetInfo: { targetId: "POPUP", openerId: "GUEST" } }, + }); + expect(restoredParams.sessionId).not.toBe(sessions.get("POPUP")); + popup.closeDevTools(); + expect(attach).toHaveBeenCalledOnce(); + if (when === "after") { + yield* cdp({ id: 2, method: "Runtime.evaluate", sessionId: sessions.get("POPUP") }); + expect(yield* reply(2)).toHaveProperty("error"); + } + for (const [index, sessionId] of [ + restoredParams.sessionId, + sessions.get("SIBLING"), + sessions.get("GUEST"), + ].entries()) { + yield* cdp({ + id: index + 3, + method: "Runtime.evaluate", + params: { id: index + 3 }, + sessionId, + }); + } + yield* settle; + expect([popup.sent, sibling.sent, page.sent]).toEqual([[3], [4], [5]]); + popup.release(); + sibling.release(); + page.release(); + yield* reply(5); + yield* cdp({ id: 6, method: "Target.getTargets" }); + expect(yield* reply(6)).toMatchObject({ + result: { + targetInfos: ["GUEST", "SIBLING", "POPUP"].map((targetId) => + expect.objectContaining({ targetId, attached: true }), + ), + }, + }); + const siblingAttach = vi.spyOn(sibling.debuggee, "attach"); + sibling.openDevTools(); + sibling.destroy(); + sibling.closeDevTools(); + expect(siblingAttach).not.toHaveBeenCalled(); + yield* host.handleCommandLine(encodeJson({ type: "release", ...key })); + popup.closeDevTools(); + expect(attach).toHaveBeenCalledOnce(); + popup.destroy(); + popup.closeDevTools(); + expect(attach).toHaveBeenCalledOnce(); + expect(popup.tab.webContents.close).not.toHaveBeenCalled(); + }).pipe(Effect.scoped), + ); + it.effect("detaches only the tab whose CDP event is too large to relay", () => 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 857c29ecad1c3..bf4074d8624ed 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -495,7 +495,7 @@ export const make = Effect.gen(function* () { // Kept for the popup's lifetime; `webContents.debugger` throws once it is destroyed. const debuggee = webContents.debugger; // DevTools, or another client, already holds its debugger. - if (debuggee.isAttached()) return; + if (webContents.isDevToolsOpened() || debuggee.isAttached()) return; try { debuggee.attach("1.3"); } catch { @@ -538,10 +538,18 @@ export const make = Effect.gen(function* () { }; const openedPopup = (opener: Electron.WebContents, popup: Electron.WebContents) => { + if (popup.isDestroyed()) return; const popups = openedPopups.get(opener) ?? new Set(); + if (popups.has(popup)) return; openedPopups.set(opener, popups); popups.add(popup); + const show = () => { + for (const tab of tabs.values()) + if (tab.debuggee.webContents === opener) showPopup(tab, popup); + }; + popup.on("devtools-closed", show); popup.once("destroyed", () => { + popup.off("devtools-closed", show); popups.delete(popup); if (popups.size === 0 && openedPopups.get(opener) === popups) openedPopups.delete(opener); for (const tab of tabs.values()) { @@ -549,7 +557,7 @@ export const make = Effect.gen(function* () { if (shown) hidePopup(shown, true); } }); - for (const tab of tabs.values()) if (tab.debuggee.webContents === opener) showPopup(tab, popup); + show(); }; const targetOf = (source: Electron.WebContents) => { From 4efd43a7eada32dd54c4a4636b032fdf8ddab81c Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Thu, 5 Nov 2026 06:16:02 +0000 Subject: [PATCH 13/24] fix(desktop): retain popup opener protection through debugger detachment --- .../src/preview/DesktopBrowserHost.test.ts | 32 ++++++++++++++----- .../desktop/src/preview/DesktopBrowserHost.ts | 15 +++++++++ .../src/preview/DesktopBrowserChannel.test.ts | 7 ++++ .../src/preview/DesktopBrowserChannel.ts | 16 +++++++++- apps/server/src/preview/ServerBrowser.test.ts | 21 ++++++++++++ apps/server/src/preview/ServerBrowser.ts | 7 ++-- packages/contracts/src/desktopBrowser.ts | 2 ++ 7 files changed, 86 insertions(+), 14 deletions(-) diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index 9834c0fed2240..4242710a1ebcc 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -179,19 +179,25 @@ describe("DesktopBrowserHost", () => { it.effect("re-sends shown to a backend that starts later and when a tab attaches again", () => Effect.gen(function* () { const host = yield* DesktopBrowserHost.make; - host.attach(key, makeDebuggee().tab); + const page = makeDebuggee(); + const popup = makeDebuggee(); + popup.openDevTools(); + host.openedPopup(page.tab.webContents, popup.tab.webContents); + host.attach(key, page.tab); host.setShown(key, true); - const reader = yield* takeEvents(host, 5).pipe(Effect.forkScoped); + const reader = yield* takeEvents(host, 7).pipe(Effect.forkScoped); yield* Effect.yieldNow; host.detach(key); - host.attach(key, makeDebuggee().tab); + host.attach(key, page.tab); expect(yield* Fiber.join(reader)).toEqual([ { type: "attached", ...key }, + { type: "popupWindows", ...key, open: true }, { type: "shown", ...key, shown: true }, { type: "detached", ...key }, { type: "attached", ...key }, + { type: "popupWindows", ...key, open: true }, { type: "shown", ...key, shown: true }, ]); }), @@ -201,13 +207,16 @@ describe("DesktopBrowserHost", () => { Effect.gen(function* () { const host = yield* DesktopBrowserHost.make; const other = { ...key, tabId: "tab-2" }; + const page = makeDebuggee(); + host.openedPopup(page.tab.webContents, makeDebuggee().tab.webContents); host.setShown(key, false); - host.attach(key, makeDebuggee().tab); + host.attach(key, page.tab); host.setShown(other, true); host.attach(other, makeDebuggee().tab); - expect(yield* takeEvents(host, 3)).toEqual([ + expect(yield* takeEvents(host, 4)).toEqual([ { type: "attached", ...key }, + { type: "popupWindows", ...key, open: true }, { type: "shown", ...key, shown: false }, { type: "attached", ...other }, ]); @@ -1189,11 +1198,14 @@ describe("DesktopBrowserHost", () => { const popup = makeDebuggee({ targetId: "POPUP" }); const sibling = makeDebuggee({ targetId: "SIBLING" }); const attach = vi.spyOn(popup.debuggee, "attach"); + const nativeStates: Array = []; const events = yield* Queue.unbounded(); yield* host.events.pipe( - Stream.runForEach((line) => - Queue.offer(events, decodeEvent(new TextDecoder().decode(line))), - ), + Stream.runForEach((line) => { + const event = decodeEvent(new TextDecoder().decode(line)); + if (event.type === "popupWindows") nativeStates.push(event.open); + return Queue.offer(events, event); + }), Effect.forkScoped, ); const cdp = (message: Record) => @@ -1235,6 +1247,7 @@ describe("DesktopBrowserHost", () => { }); } expect(popup.debuggee.attached).toBe(false); + expect(nativeStates).toEqual([true]); attach.mockClear(); popup.closeDevTools(); expect(popup.debuggee.attached).toBe(true); @@ -1287,6 +1300,9 @@ describe("DesktopBrowserHost", () => { expect(attach).toHaveBeenCalledOnce(); popup.destroy(); popup.closeDevTools(); + let closed = yield* Queue.take(events); + while (closed.type !== "popupWindows" || closed.open) closed = yield* Queue.take(events); + expect(nativeStates).toEqual([true, false]); expect(attach).toHaveBeenCalledOnce(); expect(popup.tab.webContents.close).not.toHaveBeenCalled(); }).pipe(Effect.scoped), diff --git a/apps/desktop/src/preview/DesktopBrowserHost.ts b/apps/desktop/src/preview/DesktopBrowserHost.ts index bf4074d8624ed..7dbd61329c840 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -547,11 +547,18 @@ export const make = Effect.gen(function* () { for (const tab of tabs.values()) if (tab.debuggee.webContents === opener) showPopup(tab, popup); }; + const report = (open: boolean) => { + for (const tab of tabs.values()) { + if (tab.debuggee.webContents === opener) emit({ type: "popupWindows", ...tab.key, open }); + } + }; popup.on("devtools-closed", show); + if (popups.size === 1) report(true); popup.once("destroyed", () => { popup.off("devtools-closed", show); popups.delete(popup); if (popups.size === 0 && openedPopups.get(opener) === popups) openedPopups.delete(opener); + if (popups.size === 0) report(false); for (const tab of tabs.values()) { const shown = tab.popups.get(popup); if (shown) hidePopup(shown, true); @@ -614,6 +621,9 @@ export const make = Effect.gen(function* () { debuggee.debugger.on("message", tab.onMessage); debuggee.webContents.on("did-navigate", tab.onNewDocument); emit({ type: "attached", ...key }); + if (openedPopups.get(debuggee.webContents)?.size) { + emit({ type: "popupWindows", ...key, open: true }); + } const shown = shownTabs.get(id); if (shown !== undefined) emit({ type: "shown", ...key, shown }); }; @@ -674,6 +684,11 @@ export const make = Effect.gen(function* () { dropRelay(tab); const shown = shownTabs.get(keyOf(tab.key)); return PubSub.publish(outbox, { type: "attached", ...tab.key }).pipe( + Effect.andThen( + openedPopups.get(tab.debuggee.webContents)?.size + ? PubSub.publish(outbox, { type: "popupWindows", ...tab.key, open: true }) + : Effect.void, + ), Effect.andThen( shown !== undefined ? PubSub.publish(outbox, { type: "shown", ...tab.key, shown }) diff --git a/apps/server/src/preview/DesktopBrowserChannel.test.ts b/apps/server/src/preview/DesktopBrowserChannel.test.ts index 58513cea36010..8fb508198aab8 100644 --- a/apps/server/src/preview/DesktopBrowserChannel.test.ts +++ b/apps/server/src/preview/DesktopBrowserChannel.test.ts @@ -66,22 +66,29 @@ it.effect("protects unknown desktop visibility until an explicit hidden report a yield* send({ type: "attached", ...key }); yield* channel.awaitAttached(key, "5 seconds"); expect(channel.isShown(key)).toBe(true); + expect(channel.hasNativePopups(key)).toBe(false); yield* send({ type: "shown", ...key, shown: false }); + yield* send({ type: "popupWindows", ...key, open: true }); yield* send({ type: "attached", threadId: key.threadId, tabId: "sentinel-1" }); yield* channel.awaitAttached({ threadId: key.threadId, tabId: "sentinel-1" }, "5 seconds"); expect(channel.isShown(key)).toBe(false); + expect(channel.hasNativePopups(key)).toBe(true); yield* send({ type: "shown", ...key, shown: true }); + yield* send({ type: "popupWindows", ...key, open: false }); yield* send({ type: "attached", threadId: key.threadId, tabId: "sentinel-2" }); yield* channel.awaitAttached({ threadId: key.threadId, tabId: "sentinel-2" }, "5 seconds"); expect(channel.isShown(key)).toBe(true); + expect(channel.hasNativePopups(key)).toBe(false); yield* send({ type: "shown", ...key, shown: false }); + yield* send({ type: "popupWindows", ...key, open: true }); yield* send({ type: "detached", ...key }); yield* send({ type: "attached", threadId: key.threadId, tabId: "sentinel-3" }); yield* channel.awaitAttached({ threadId: key.threadId, tabId: "sentinel-3" }, "5 seconds"); expect(channel.isShown(key)).toBe(true); + expect(channel.hasNativePopups(key)).toBe(false); }), ), ); diff --git a/apps/server/src/preview/DesktopBrowserChannel.ts b/apps/server/src/preview/DesktopBrowserChannel.ts index bdfd75df85746..98067da5a81b9 100644 --- a/apps/server/src/preview/DesktopBrowserChannel.ts +++ b/apps/server/src/preview/DesktopBrowserChannel.ts @@ -46,6 +46,7 @@ export class DesktopBrowserChannel extends Context.Service< readonly isAttached: (key: DesktopTabKey) => Effect.Effect; /** Whether a desktop window draws the tab on screen, which no streamed viewer counts. */ readonly isShown: (key: DesktopTabKey) => boolean; + readonly hasNativePopups: (key: DesktopTabKey) => boolean; /** With `popups`, the tab's popup windows become pages of the same connection. */ readonly endpoint: ( key: DesktopTabKey, @@ -68,6 +69,7 @@ const make = Effect.gen(function* () { const attachedTabs = new Set(); // Missing reports protect desktop tabs until the renderer explicitly marks them hidden. const shownTabs = new Map(); + const popupOpeners = new Set(); const inbound = new Map>(); const writeLock = yield* Semaphore.make(1); @@ -92,6 +94,7 @@ const make = Effect.gen(function* () { return Effect.void; }), isShown: () => false, + hasNativePopups: () => false, }); } @@ -130,6 +133,7 @@ const make = Effect.gen(function* () { case "detached": { attachedTabs.delete(id); shownTabs.delete(id); + popupOpeners.delete(id); const queue = inbound.get(id); return (queue ? Queue.shutdown(queue) : Effect.void).pipe( Effect.andThen(PubSub.publish(changes, { key, attached: false })), @@ -138,10 +142,19 @@ const make = Effect.gen(function* () { case "shown": shownTabs.set(id, event.shown); return Effect.void; + case "popupWindows": + if (event.open) popupOpeners.add(id); + else popupOpeners.delete(id); + return Effect.void; } }), Effect.catchCause((cause) => Effect.logWarning("desktop browser channel stopped", { cause })), - Effect.ensuring(Effect.sync(() => shownTabs.clear())), + Effect.ensuring( + Effect.sync(() => { + shownTabs.clear(); + popupOpeners.clear(); + }), + ), Effect.forkScoped, ); @@ -244,6 +257,7 @@ const make = Effect.gen(function* () { return Effect.sync(() => attachedTabs.has(keyOf(key))); }), isShown: (key) => shownTabs.get(keyOf(key)) ?? true, + hasNativePopups: (key) => popupOpeners.has(keyOf(key)), endpoint, turn: Effect.fn("DesktopBrowserChannel.turn")(function (threadId, active) { return command({ type: "turn", threadId, active }); diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index e78d408c6cc75..221ab0bcb8ec2 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -338,6 +338,7 @@ const desktopRenders = (tabId: string) => { return desktopTabs.has(tabId); }; const shownDesktopTabs = new Set(); +const nativePopupOpeners = new Set(); const releasedDesktopTabs: Array = []; const desktopTurns = new NodeEvents.EventEmitter(); const desktopConnections: Array<{ endpoint: string; context: ReturnType }> = []; @@ -397,6 +398,7 @@ const dependencies = Layer.mergeAll( ), isAttached: (key) => Effect.sync(() => desktopRenders(key.tabId)), isShown: (key) => shownDesktopTabs.has(key.tabId), + hasNativePopups: (key) => nativePopupOpeners.has(key.tabId), endpoint: (key, options) => Effect.acquireRelease( Effect.sync(() => { @@ -480,6 +482,7 @@ beforeEach(() => { focusedFrame = null; desktopTabs.clear(); shownDesktopTabs.clear(); + nativePopupOpeners.clear(); desktopRendersNext = false; desktopPageUrl = "about:blank"; desktopPopupsNext.length = 0; @@ -2486,6 +2489,10 @@ it.live("a full server takes back the slot of the longest idle agent tab nobody }); const refused = makeContext(undefined, "https://example.com/retained"); desktopConnections[8]!.context.page.emit("popup", refused.page); + nativePopupOpeners.add(opened[24]!); + nativePopupOpeners.add(opened[8]!); + // DevTools can already hold a native window before Playwright ever sees its page. + nativePopupOpeners.add(opened[9]!); now.mockReturnValue(100); yield* broker.invoke({ @@ -2518,6 +2525,14 @@ it.live("a full server takes back the slot of the longest idle agent tab nobody expect(yield* open("agent-new").pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); expect(releasedDesktopTabs).toEqual([]); expect(refused.page.close).not.toHaveBeenCalled(); + // Debugger detachment closes Playwright's page, leaving the native window open. + refused.page.emit("close"); + expect(yield* open("agent-new").pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); + expect(releasedDesktopTabs).toEqual([]); + shownDesktopTabs.add(opened[8]!); + shownDesktopTabs.delete(opened[9]!); + expect(yield* open("agent-new").pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); + expect(releasedDesktopTabs).toEqual([]); shownDesktopTabs.clear(); shownDesktopTabs.add(opened[1]!); const events = yield* manager.subscribeEvents; @@ -2540,10 +2555,16 @@ it.live("a full server takes back the slot of the longest idle agent tab nobody [...opened.filter((tabId) => tabId !== opened[2]), popupTabId, fresh.tabId!].toSorted(), ); yield* Effect.promise(() => refused.page.close()); + nativePopupOpeners.delete(opened[8]!); for (const session of sessions) shownDesktopTabs.add(session.tabId); shownDesktopTabs.delete(opened[8]!); yield* open("agent-new"); expect(yield* Queue.take(releases)).toBe(opened[8]); + nativePopupOpeners.delete(opened[9]!); + shownDesktopTabs.add(opened[8]!); + shownDesktopTabs.delete(opened[9]!); + yield* open("agent-new"); + expect(yield* Queue.take(releases)).toBe(opened[9]); }), ).pipe(Effect.provide(layer)), ); diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 3b009cc73b1d0..2347602e49f01 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -308,7 +308,6 @@ interface ServerTab { readonly isolatedContext: boolean; readonly desktop: { readonly close: () => Promise } | null; - readonly nativePopups: Set; readonly profileId: string | undefined; readonly openerTabId: string | undefined; @@ -887,7 +886,6 @@ const make = Effect.gen(function* () { }, isolatedContext, desktop: desktop === null ? null : { close: desktop.close }, - nativePopups: new Set(), profileId: snapshot.profileId, openerTabId: adopted?.openerTabId, downloads: [], @@ -1364,8 +1362,6 @@ const make = Effect.gen(function* () { if (opener.desktop !== null) { // Keep native dialogs pending even when this popup never becomes an automation tab. popup.on("dialog", constVoid); - opener.nativePopups.add(popup); - popup.once("close", () => opener.nativePopups.delete(popup)); } if (opener.closing) { await popup.close().catch(constVoid); @@ -1466,7 +1462,8 @@ const make = Effect.gen(function* () { // A desktop popup is the person's own window, and closing its opener destroys it too. const isNativePopup = (tab: ServerTab) => tab.desktop !== null && tab.openerTabId !== undefined; - const holdsNativePopup = (tab: ServerTab) => isNativePopup(tab) || tab.nativePopups.size > 0; + const holdsNativePopup = (tab: ServerTab) => + isNativePopup(tab) || (tab.desktop !== null && desktopChannel.hasNativePopups(tab)); const idleAgentTabs = () => { const cutoff = Date.now() - AGENT_TAB_IDLE_MS; diff --git a/packages/contracts/src/desktopBrowser.ts b/packages/contracts/src/desktopBrowser.ts index 53281cdb902ed..10f5463039441 100644 --- a/packages/contracts/src/desktopBrowser.ts +++ b/packages/contracts/src/desktopBrowser.ts @@ -14,6 +14,8 @@ export const DesktopBrowserEvent = Schema.Union([ Schema.Struct({ type: Schema.Literal("cdp"), ...TabKey, message: Schema.String }), /** Its `` started or stopped being on screen in a desktop window. */ Schema.Struct({ type: Schema.Literal("shown"), ...TabKey, shown: Schema.Boolean }), + /** Native window lifetime, independent of popup debugger sessions. */ + Schema.Struct({ type: Schema.Literal("popupWindows"), ...TabKey, open: Schema.Boolean }), ]); export type DesktopBrowserEvent = typeof DesktopBrowserEvent.Type; From 25cb5f478be7215830a3878858302d4c2f71c9d1 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Thu, 5 Nov 2026 06:36:25 +0000 Subject: [PATCH 14/24] fix(server): keep tabs with active agent actions out of idle reclamation --- .../desktop/src/preview/DesktopBrowserHost.ts | 1 - apps/server/src/preview/ServerBrowser.test.ts | 25 ++++++++++++++++++- apps/server/src/preview/ServerBrowser.ts | 1 + 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/preview/DesktopBrowserHost.ts b/apps/desktop/src/preview/DesktopBrowserHost.ts index 7dbd61329c840..b2df87363f6f3 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -494,7 +494,6 @@ export const make = Effect.gen(function* () { return; // Kept for the popup's lifetime; `webContents.debugger` throws once it is destroyed. const debuggee = webContents.debugger; - // DevTools, or another client, already holds its debugger. if (webContents.isDevToolsOpened() || debuggee.isAttached()) return; try { debuggee.attach("1.3"); diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 221ab0bcb8ec2..6a98758111670 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -2561,8 +2561,31 @@ it.live("a full server takes back the slot of the longest idle agent tab nobody yield* open("agent-new"); expect(yield* Queue.take(releases)).toBe(opened[8]); nativePopupOpeners.delete(opened[9]!); - shownDesktopTabs.add(opened[8]!); + for (const session of (yield* manager.list({ threadId: scope.thread.threadId })).sessions) + shownDesktopTabs.add(session.tabId); shownDesktopTabs.delete(opened[9]!); + const started = Promise.withResolvers(); + const finish = Promise.withResolvers(); + desktopConnections[9]!.context.page.goto.mockImplementationOnce(async () => { + started.resolve(); + await finish.promise; + }); + yield* Effect.addFinalizer(() => Effect.sync(() => finish.resolve())); + const running = yield* broker + .invoke({ + scope: asSession("agent-1"), + tabId: PreviewTabId.make(opened[9]!), + operation: "navigate", + input: { url: "https://example.com/held" }, + }) + .pipe(Effect.forkScoped); + yield* Effect.promise(() => started.promise); + expect(yield* open("agent-new").pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); + expect(releasedDesktopTabs).not.toContain(opened[9]); + finish.resolve(); + yield* Fiber.join(running); + expect(yield* open("agent-new").pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); + now.mockReturnValue(62 * 60 * 1000); yield* open("agent-new"); expect(yield* Queue.take(releases)).toBe(opened[9]); }), diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 2347602e49f01..134f6c29e0a8a 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -1470,6 +1470,7 @@ const make = Effect.gen(function* () { return [...tabs.values()].filter( (tab) => tab.control.agentId !== null && + !tab.agentOperating && !isVisible(tab) && tab.usedAt < cutoff && !holdsNativePopup(tab), From 3c9fed1ca274179a704a7399be12cd5e8cf5035b Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Thu, 5 Nov 2026 06:53:08 +0000 Subject: [PATCH 15/24] fix(server): retain native popup dialogs through adoption --- apps/server/src/preview/ServerBrowser.test.ts | 285 +++++++++++------- apps/server/src/preview/ServerBrowser.ts | 40 ++- 2 files changed, 198 insertions(+), 127 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 6a98758111670..76632473e374a 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -182,9 +182,17 @@ function makeContext(onClose?: (context: BrowserContext) => void, initialUrl = " dialogAutoDismissed, // Playwright dismisses a dialog nothing listens for before the page shows it. openDialog: () => { - if (!events.emit("dialog", { type: () => "alert", message: () => "hello" })) { + const dialog = { + type: () => "alert", + message: () => "hello", + defaultValue: () => "", + accept: vi.fn(async () => {}), + dismiss: vi.fn(async () => {}), + }; + if (!events.emit("dialog", dialog)) { dialogAutoDismissed(); } + return dialog; }, openFileChooser: () => { if ( @@ -2271,124 +2279,171 @@ it.live("profiles reported by one client survive another client's report", () => ); it.live.each([ - { budget: "agent", limit: 8, host: "server" }, - { budget: "server", limit: 32, host: "server" }, - { budget: "agent", limit: 8, host: "desktop" }, - { budget: "server", limit: 32, host: "desktop" }, -])("reserves the $budget budget for opens and $host popups", ({ budget, limit, host }) => - Effect.scoped( - Effect.gen(function* () { - desktopRendersNext = host === "desktop"; - const { broker, browser, tabId } = yield* ready; - const openerPage = - host === "desktop" ? desktopConnections[0]!.context.page : contexts[0]!.page; - const manager = yield* Manager.PreviewManager; - const open = (target = scope) => - broker.invoke({ - scope: target, + { budget: "agent", limit: 8, host: "server", dialogState: "pending" }, + { budget: "server", limit: 32, host: "server", dialogState: "pending" }, + { budget: "agent", limit: 8, host: "desktop", dialogState: "pending" }, + { budget: "server", limit: 32, host: "desktop", dialogState: "pending" }, + { budget: "agent", limit: 8, host: "desktop", dialogState: "closedBeforeTab" }, + { budget: "agent", limit: 8, host: "desktop", dialogState: "closedDuringSetup" }, + { budget: "agent", limit: 8, host: "desktop", dialogState: "openedDuringSetup" }, +])( + "reserves the $budget budget for opens and $host popups ($dialogState dialog)", + ({ budget, limit, host, dialogState }) => + Effect.scoped( + Effect.gen(function* () { + desktopRendersNext = host === "desktop"; + const { broker, browser, tabId } = yield* ready; + const openerPage = + host === "desktop" ? desktopConnections[0]!.context.page : contexts[0]!.page; + const manager = yield* Manager.PreviewManager; + const open = (target = scope) => + broker.invoke({ + scope: target, + operation: "open", + input: { reuseExistingTab: false, show: false }, + }); + for (let index = 1; index < limit - 3; index++) { + yield* open(budget === "agent" ? scope : asSession(`other-${Math.floor(index / 8)}`)); + } + + const sessionGate = Promise.withResolvers(); + const launchGate = (contextGate = Promise.withResolvers()); + const sessionStarted = Promise.withResolvers(); + const realOpen = manager.open; + const delayedOpen = vi.spyOn(manager, "open").mockImplementation((input) => + Effect.promise(() => { + sessionStarted.resolve(); + return sessionGate.promise; + }).pipe(Effect.andThen(realOpen(input))), + ); + yield* Effect.addFinalizer(() => + Effect.sync(() => { + sessionGate.resolve(); + launchGate.resolve(); + delayedOpen.mockRestore(); + }), + ); + const events = yield* manager.subscribeEvents; + const first = yield* open().pipe(Effect.result, Effect.forkScoped); + yield* Effect.promise(() => sessionStarted.promise); + const popup = makeContext(); + const resized = Promise.withResolvers(); + const setViewportSize = popup.page.setViewportSize; + popup.page.setViewportSize = vi.fn(async (size) => { + await setViewportSize(size); + if (size?.width === 900) resized.resolve(); + }); + const newCDPSession = popup.newCDPSession; + let pendingDialog: ReturnType; + let changedDialog = false; + popup.newCDPSession = async (target) => { + await launchGate.promise; + const session = await newCDPSession(target); + const send = session.send.getMockImplementation()!; + session.send.mockImplementation(async (method, input) => { + if (!changedDialog) { + if (method === "Runtime.evaluate" && dialogState === "closedBeforeTab") { + changedDialog = true; + session.emit("Page.javascriptDialogClosed"); + } + if (method === "Page.enable") { + changedDialog = true; + if (dialogState === "closedDuringSetup") + session.emit("Page.javascriptDialogClosed"); + if (dialogState === "openedDuringSetup") pendingDialog = popup.page.openDialog(); + } + } + return send(method, input); + }); + return session; + }; + openerPage.emit("popup", popup.page); + // A native window's dialog waits for the person while its tab is still being set up. + pendingDialog = popup.page.openDialog(); + expect(popup.page.dialogAutoDismissed).toHaveBeenCalledTimes(host === "desktop" ? 0 : 1); + const last = yield* open().pipe(Effect.result, Effect.forkScoped); + const overflow = yield* open().pipe(Effect.result, Effect.forkScoped); + // The host processes this after the opens, while manager creation is blocked. + const reused = yield* broker.invoke({ + scope, + tabId, operation: "open", - input: { reuseExistingTab: false, show: false }, + input: { show: false }, }); - for (let index = 1; index < limit - 3; index++) { - yield* open(budget === "agent" ? scope : asSession(`other-${Math.floor(index / 8)}`)); - } - - const sessionGate = Promise.withResolvers(); - const launchGate = (contextGate = Promise.withResolvers()); - const sessionStarted = Promise.withResolvers(); - const realOpen = manager.open; - const delayedOpen = vi.spyOn(manager, "open").mockImplementation((input) => - Effect.promise(() => { - sessionStarted.resolve(); - return sessionGate.promise; - }).pipe(Effect.andThen(realOpen(input))), - ); - yield* Effect.addFinalizer(() => - Effect.sync(() => { - sessionGate.resolve(); - launchGate.resolve(); - delayedOpen.mockRestore(); - }), - ); - const events = yield* manager.subscribeEvents; - const first = yield* open().pipe(Effect.result, Effect.forkScoped); - yield* Effect.promise(() => sessionStarted.promise); - const popup = makeContext(); - const resized = Promise.withResolvers(); - const setViewportSize = popup.page.setViewportSize; - popup.page.setViewportSize = vi.fn(async (size) => { - await setViewportSize(size); - if (size?.width === 900) resized.resolve(); - }); - const newCDPSession = popup.newCDPSession; - popup.newCDPSession = async (target) => { - await launchGate.promise; - return newCDPSession(target); - }; - openerPage.emit("popup", popup.page); - // A native window's dialog waits for the person while its tab is still being set up. - popup.page.openDialog(); - expect(popup.page.dialogAutoDismissed).toHaveBeenCalledTimes(host === "desktop" ? 0 : 1); - const last = yield* open().pipe(Effect.result, Effect.forkScoped); - const overflow = yield* open().pipe(Effect.result, Effect.forkScoped); - // The host processes this after the opens, while manager creation is blocked. - const reused = yield* broker.invoke({ - scope, - tabId, - operation: "open", - input: { show: false }, - }); - expect(reused.tabId).toBe(tabId); - expect((yield* manager.list({ threadId: scope.thread.threadId })).sessions).toHaveLength( - limit - 3, - ); + expect(reused.tabId).toBe(tabId); + expect((yield* manager.list({ threadId: scope.thread.threadId })).sessions).toHaveLength( + limit - 3, + ); - sessionGate.resolve(); - for (let count = 0; count < 3; count++) { - let event = yield* PubSub.take(events); - while (event.type !== "opened") event = yield* PubSub.take(events); - yield* manager.resize({ - threadId: scope.thread.threadId, - tabId: event.tabId, - viewport: { _tag: "freeform", width: 900, height: 700 }, - }); - } - // The same slots must stay reserved after publication, while pages are still opening. - const duringLaunch = yield* open().pipe(Effect.result, Effect.forkScoped); - const rejectedPopup = makeContext(); - openerPage.emit("popup", rejectedPopup.page); - yield* broker.invoke({ scope, tabId, operation: "status", input: {} }); - launchGate.resolve(); - - expect((yield* Fiber.join(first))._tag).toBe("Success"); - expect((yield* Fiber.join(last))._tag).toBe("Success"); - for (const fiber of [overflow, duringLaunch]) { - expect(yield* Fiber.join(fiber)).toMatchObject({ - _tag: "Failure", - failure: { _tag: "PreviewAutomationControlInterruptedError", reason: "tabLimit" }, + sessionGate.resolve(); + for (let count = 0; count < 3; count++) { + let event = yield* PubSub.take(events); + while (event.type !== "opened") event = yield* PubSub.take(events); + yield* manager.resize({ + threadId: scope.thread.threadId, + tabId: event.tabId, + viewport: { _tag: "freeform", width: 900, height: 700 }, + }); + } + // The same slots must stay reserved after publication, while pages are still opening. + const duringLaunch = yield* open().pipe(Effect.result, Effect.forkScoped); + const rejectedPopup = makeContext(); + openerPage.emit("popup", rejectedPopup.page); + yield* broker.invoke({ scope, tabId, operation: "status", input: {} }); + launchGate.resolve(); + + expect((yield* Fiber.join(first))._tag).toBe("Success"); + expect((yield* Fiber.join(last))._tag).toBe("Success"); + for (const fiber of [overflow, duringLaunch]) { + expect(yield* Fiber.join(fiber)).toMatchObject({ + _tag: "Failure", + failure: { _tag: "PreviewAutomationControlInterruptedError", reason: "tabLimit" }, + }); + } + if (host === "server") yield* Effect.promise(() => resized.promise); + const { sessions } = yield* manager.list({ threadId: scope.thread.threadId }); + for (const session of sessions) + yield* browser.attachViewer(viewerInput(session.tabId, false)); + expect(sessions).toHaveLength(limit); + expect(popup.page.isClosed()).toBe(false); + expect(rejectedPopup.page.isClosed()).toBe(host === "server"); + if (host === "desktop") { + // No tab answers the window it could not adopt, so its dialogs stay the person's. + rejectedPopup.page.openDialog(); + expect(rejectedPopup.page.dialogAutoDismissed).not.toHaveBeenCalled(); + } + const status = yield* broker.invoke({ + scope, + tabId, + operation: "status", + input: {}, }); - } - if (host === "server") yield* Effect.promise(() => resized.promise); - const { sessions } = yield* manager.list({ threadId: scope.thread.threadId }); - for (const session of sessions) - yield* browser.attachViewer(viewerInput(session.tabId, false)); - expect(sessions).toHaveLength(limit); - expect(popup.page.isClosed()).toBe(false); - expect(rejectedPopup.page.isClosed()).toBe(host === "server"); - if (host === "desktop") { - // No tab answers the window it could not adopt, so its dialogs stay the person's. - rejectedPopup.page.openDialog(); - expect(rejectedPopup.page.dialogAutoDismissed).not.toHaveBeenCalled(); - } - const status = yield* broker.invoke({ - scope, - tabId, - operation: "status", - input: {}, - }); - expect(status.tabs).toHaveLength(limit); - }), - ).pipe(Effect.provide(layer)), + expect(status.tabs).toHaveLength(limit); + if (host === "desktop") { + const adopted = status.tabs!.find((tab) => tab.openerTabId === tabId)!; + const popupStatus = yield* broker.invoke({ + scope, + tabId: adopted.tabId, + operation: "status", + input: {}, + }); + if (dialogState.startsWith("closed")) { + expect(popupStatus.dialog).toBeNull(); + expect(pendingDialog.accept).not.toHaveBeenCalled(); + } else { + expect(popupStatus.dialog).toMatchObject({ type: "alert", message: "hello" }); + yield* broker.invoke({ + scope, + tabId: adopted.tabId, + operation: "dialog", + input: { accept: true }, + }); + expect(pendingDialog.accept).toHaveBeenCalledTimes(1); + } + expect(pendingDialog.dismiss).not.toHaveBeenCalled(); + } + }), + ).pipe(Effect.provide(layer)), ); it.live("failed session and browser launches release capacity, as does closing a tab", () => diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 134f6c29e0a8a..d1ac2ba6525f0 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -496,7 +496,12 @@ const make = Effect.gen(function* () { const closedPendingTabs = new Set(); const adoptedPages = new Map< string, - { readonly page: Page; readonly openerTabId: string; readonly native: boolean } + { + readonly page: Page; + readonly openerTabId: string; + readonly native: boolean; + readonly pendingDialog?: { current: Dialog | null }; + } >(); /** A desktop popup can reach adoption from both its `popup` event and the connect sweep. */ const adoptedPopups = new WeakSet(); @@ -853,6 +858,12 @@ const make = Effect.gen(function* () { const page = adopted?.page ?? desktop?.page ?? (await context.newPage()); if (!desktop) opened.push(() => page.close()); const cdp = await context.newCDPSession(page); + const pendingDialog = adopted?.pendingDialog; + if (pendingDialog) { + cdp.on("Page.javascriptDialogClosed", () => { + pendingDialog.current = null; + }); + } await installPdfDocumentNavigation(cdp); // The desktop's page is already a normal browser. A popup's first request // went out before it became a tab; everything after presents as Chrome. @@ -890,7 +901,7 @@ const make = Effect.gen(function* () { openerTabId: adopted?.openerTabId, downloads: [], fileChooser: null, - dialog: null, + dialog: adopted?.pendingDialog?.current ?? null, setting: snapshot.viewport ?? FILL_PREVIEW_VIEWPORT, colorScheme: "system", zoomFactor: 1, @@ -907,6 +918,16 @@ const make = Effect.gen(function* () { captureLock: Promise.resolve(), capturing: 0, }; + page.on("dialog", (dialog) => { + tab.dialog = dialog; + broadcastControl(tab); + }); + if (desktop) { + cdp.on("Page.javascriptDialogClosed", () => { + tab.dialog = null; + if (tab.viewers.size > 0) return broadcastViewport(tab).catch(constVoid); + }); + } if (!desktop) { await page.setViewportSize(fixedViewportSize(tab.setting) ?? UNATTACHED_FILL_VIEWPORT); } @@ -919,11 +940,6 @@ const make = Effect.gen(function* () { cdp.on("Page.frameResized", remeasure); // A read a navigation broke has a fresh page to measure. cdp.on("Page.loadEventFired", remeasure); - // Closing a dialog resizes nothing, and the host can close it without us. - cdp.on("Page.javascriptDialogClosed", () => { - tab.dialog = null; - return remeasure(); - }); await cdp.send("Page.enable"); } const isMainNavigation = (request: { isNavigationRequest(): boolean; frame(): unknown }) => @@ -1013,10 +1029,6 @@ const make = Effect.gen(function* () { timestamp: new Date().toISOString(), }); }); - page.on("dialog", (dialog) => { - tab.dialog = dialog; - broadcastControl(tab); - }); page.on("download", (download) => void saveDownload(tab, download)); updateFileChooserInterception(tab); // Popups become tabs and keep `window.opener`, so sign-in popups can report back. @@ -1359,9 +1371,12 @@ const make = Effect.gen(function* () { const adoptPopup = async (opener: ServerTab, popup: Page) => { if (adoptedPopups.has(popup) || popup.isClosed()) return; adoptedPopups.add(popup); + const pendingDialog: { current: Dialog | null } = { current: null }; if (opener.desktop !== null) { // Keep native dialogs pending even when this popup never becomes an automation tab. - popup.on("dialog", constVoid); + popup.on("dialog", (dialog) => { + pendingDialog.current = dialog; + }); } if (opener.closing) { await popup.close().catch(constVoid); @@ -1388,6 +1403,7 @@ const make = Effect.gen(function* () { page: popup, openerTabId: opener.tabId, native: opener.desktop !== null, + ...(opener.desktop === null ? {} : { pendingDialog }), }), }), ).then( From e3340568e896cfe86e20a30ff2686a19119aa820 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Thu, 5 Nov 2026 08:03:04 +0000 Subject: [PATCH 16/24] fix(desktop): retain native dialog responses before popup adoption --- apps/desktop/src/preview/CdpRelay.test.ts | 39 +++++++++++++++++++++++ apps/desktop/src/preview/CdpRelay.ts | 24 ++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index 666018a8e33ab..6a7f97ea37992 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -440,6 +440,9 @@ describe("CDP relay popups", () => { expect.objectContaining({ targetId: "LATE" }), ]); const browserSession = (resultOf(written, 4)["result"] as { sessionId: string }).sessionId; + const closedDialog = { result: true, userInput: "" }; + late.event("Page.javascriptDialogOpening", { type: "alert", message: "hello" }, ""); + late.event("Page.javascriptDialogClosed", closedDialog, ""); relay.receive( JSON.stringify({ id: 5, @@ -467,12 +470,48 @@ describe("CDP relay popups", () => { expect(lateSend.mock.calls.at(-1)).toEqual(["Page.startScreencast", {}, undefined, extra]); expect(pageSend.mock.calls.at(-1)).toEqual(["Page.enable", {}, undefined, undefined]); + // The native response predates this CDP session, while popup adoption is waiting. + written.length = 0; + relay.receive(JSON.stringify({ id: 14, method: "Page.enable", sessionId: extra })); + await settle(); + expect(written).toEqual([ + { method: "Page.javascriptDialogClosed", params: closedDialog, sessionId: extra }, + { id: 14, result: {}, sessionId: extra }, + ]); + + const enabling = Promise.withResolvers(); + lateSend.mockImplementationOnce(() => enabling.promise); + written.length = 0; + relay.receive(JSON.stringify({ id: 15, method: "Page.enable", sessionId: extra })); + late.event("Page.javascriptDialogOpening", { type: "confirm", message: "still open" }, ""); + enabling.resolve({}); + await settle(); + expect( + written.filter((message) => message["method"] === "Page.javascriptDialogClosed"), + ).toEqual([]); + expect(resultOf(written, 15)).toMatchObject({ result: {} }); + // A frame's session belongs to the popup whose debugger attached it. late.event( "Target.attachedToTarget", { sessionId: "late-frame", targetInfo: { type: "iframe" } }, "", ); + late.event("Page.javascriptDialogClosed", closedDialog, ""); + late.event("Page.javascriptDialogOpening", { type: "alert", message: "iframe" }, "late-frame"); + written.length = 0; + relay.receive(JSON.stringify({ id: 16, method: "Page.enable", sessionId: extra })); + await settle(); + expect(written).toEqual([{ id: 16, result: {}, sessionId: extra }]); + const closedFrameDialog = { result: false, userInput: "" }; + late.event("Page.javascriptDialogClosed", closedFrameDialog, "late-frame"); + written.length = 0; + relay.receive(JSON.stringify({ id: 17, method: "Page.enable", sessionId: extra })); + await settle(); + expect(written).toEqual([ + { method: "Page.javascriptDialogClosed", params: closedFrameDialog, sessionId: extra }, + { id: 17, result: {}, sessionId: extra }, + ]); relay.receive(JSON.stringify({ id: 11, method: "DOM.enable", sessionId: "late-frame" })); await settle(); expect(lateSend.mock.calls.at(-1)).toEqual(["DOM.enable", {}, "late-frame", undefined]); diff --git a/apps/desktop/src/preview/CdpRelay.ts b/apps/desktop/src/preview/CdpRelay.ts index 75009e012583f..cd33a9ec1aaa6 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -89,6 +89,7 @@ interface RelayedTarget { /** Known once announced; a closed target cannot be asked for it. */ targetId: string | null; removed: boolean; + closedDialog?: unknown; } type RelaySession = @@ -318,19 +319,38 @@ export function createCdpRelayConnection( } // Playwright asks for this on every page; the host emulates focus only while it is in use. if (command.method === "Emulation.setFocusEmulationEnabled") return Promise.resolve({}); - return owner.target.send( + const result = owner.target.send( command.method, command.params ?? {}, child, extra ? command.sessionId : undefined, ); + if (command.method !== "Page.enable" || child !== undefined) return result; + return result.then((result) => { + // Popup adoption may have buffered the dialog before this session existed. + if ( + !owner.removed && + sessions.get(command.sessionId!) === session && + owner.closedDialog !== undefined + ) { + send({ + method: "Page.javascriptDialogClosed", + params: owner.closedDialog, + sessionId: command.sessionId, + }); + } + return result; + }); }; const eventFor = (relayed: RelayedTarget): CdpRelayEvent => (method, params, sessionId) => { - if (relayed.removed || (announced(relayed) && !hasPageSession(relayed))) return; + if (relayed.removed) return; if (sessionId && children.get(sessionId)?.owner !== relayed) return; + if (method === "Page.javascriptDialogOpening") relayed.closedDialog = undefined; + else if (method === "Page.javascriptDialogClosed") relayed.closedDialog = params; + if (announced(relayed) && !hasPageSession(relayed)) return; const child = (params as { sessionId?: unknown } | null | undefined)?.sessionId; if (method === "Target.attachedToTarget" && typeof child === "string") { children.set(child, { owner: relayed, parent: sessionId || undefined }); From ebe647582a122aeb8f14cf7d78367008a69454eb Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Thu, 5 Nov 2026 08:42:51 +0000 Subject: [PATCH 17/24] fix(server): answer native popup dialogs that hold adoption setup A native popup can already show an alert or confirm when its tab is adopted. The modal holds page commands (the PDF viewer probe, then Page.enable) until someone answers it, but the tab only entered the tab list after those commands finished, so preview_status reported it unavailable and preview_dialog waited on setup forever. An adopted native popup's tab is now listed as soon as it can see its dialogs, and the blockable setup runs as its first queued action. Status, preview_dialog, close and viewer control work outside that queue, so the owner can answer the modal and setup finishes. If setup fails after listing, the tab is removed without closing the person's window. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 83 ++++++++++++++---- apps/server/src/preview/ServerBrowser.ts | 85 +++++++++++-------- 2 files changed, 119 insertions(+), 49 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 76632473e374a..ee20c07c5d0cf 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -733,6 +733,32 @@ it.effect("an agent's desktop tab's popup windows become its tabs, drawn by the expect(failed.page.isClosed()).toBe(false); expect(releasedDesktopTabs).toEqual([]); + // Setup that fails after the tab could answer dialogs also leaves the person's window open. + const failedLate = makeContext(undefined, "https://example.com/failed-enable"); + const newLateSession = failedLate.newCDPSession; + failedLate.newCDPSession = async (target) => { + const session = await newLateSession(target); + const send = session.send.getMockImplementation()!; + session.send.mockImplementation(async (method, input) => { + if (method === "Page.enable") throw new Error("Popup debugger detached"); + return send(method, input); + }); + return session; + }; + openerPage.emit("popup", failedLate.page); + const failedLateTab = yield* nextOpened; + closed = yield* PubSub.take(events); + while (closed.type !== "closed" || closed.tabId !== failedLateTab.tabId) + closed = yield* PubSub.take(events); + expect(failedLate.page.isClosed()).toBe(false); + const afterFailure = yield* broker.invoke({ + scope, + tabId: opener.tabId!, + operation: "status", + input: {}, + }); + expect(afterFailure.tabs?.map((tab) => tab.tabId)).not.toContain(failedLateTab.tabId); + // A person's desktop tab keeps its popup windows to itself. desktopRendersNext = true; const human = yield* manager.open({ threadId: scope.thread.threadId, runtime: "server" }); @@ -2286,6 +2312,9 @@ it.live.each([ { budget: "agent", limit: 8, host: "desktop", dialogState: "closedBeforeTab" }, { budget: "agent", limit: 8, host: "desktop", dialogState: "closedDuringSetup" }, { budget: "agent", limit: 8, host: "desktop", dialogState: "openedDuringSetup" }, + // A modal holds page commands until it is answered, as a native sheet does. + { budget: "agent", limit: 8, host: "desktop", dialogState: "holdsPdfProbe" }, + { budget: "server", limit: 32, host: "desktop", dialogState: "holdsPageEnable" }, ])( "reserves the $budget budget for opens and $host popups ($dialogState dialog)", ({ budget, limit, host, dialogState }) => @@ -2336,11 +2365,24 @@ it.live.each([ const newCDPSession = popup.newCDPSession; let pendingDialog: ReturnType; let changedDialog = false; + const answered = Promise.withResolvers(); + const held = Promise.withResolvers(); + const registered = Promise.withResolvers(); + popup.page.waitForLoadState = vi.fn(async () => registered.resolve()); + const holds = (method: string, input: unknown) => + dialogState === "holdsPdfProbe" + ? method === "Runtime.evaluate" && + (input as { expression?: string }).expression === "navigator.pdfViewerEnabled" + : dialogState === "holdsPageEnable" && method === "Page.enable"; popup.newCDPSession = async (target) => { await launchGate.promise; const session = await newCDPSession(target); const send = session.send.getMockImplementation()!; session.send.mockImplementation(async (method, input) => { + if (holds(method, input)) { + held.resolve(); + await answered.promise; + } if (!changedDialog) { if (method === "Runtime.evaluate" && dialogState === "closedBeforeTab") { changedDialog = true; @@ -2360,6 +2402,8 @@ it.live.each([ openerPage.emit("popup", popup.page); // A native window's dialog waits for the person while its tab is still being set up. pendingDialog = popup.page.openDialog(); + pendingDialog.accept.mockImplementation(async () => answered.resolve()); + pendingDialog.dismiss.mockImplementation(async () => answered.resolve()); expect(popup.page.dialogAutoDismissed).toHaveBeenCalledTimes(host === "desktop" ? 0 : 1); const last = yield* open().pipe(Effect.result, Effect.forkScoped); const overflow = yield* open().pipe(Effect.result, Effect.forkScoped); @@ -2401,17 +2445,9 @@ it.live.each([ }); } if (host === "server") yield* Effect.promise(() => resized.promise); - const { sessions } = yield* manager.list({ threadId: scope.thread.threadId }); - for (const session of sessions) - yield* browser.attachViewer(viewerInput(session.tabId, false)); - expect(sessions).toHaveLength(limit); - expect(popup.page.isClosed()).toBe(false); - expect(rejectedPopup.page.isClosed()).toBe(host === "server"); - if (host === "desktop") { - // No tab answers the window it could not adopt, so its dialogs stay the person's. - rejectedPopup.page.openDialog(); - expect(rejectedPopup.page.dialogAutoDismissed).not.toHaveBeenCalled(); - } + const holding = dialogState.startsWith("holds"); + // A held setup waits for its dialog's answer; the others finish on their own. + if (host === "desktop") yield* Effect.promise(() => (holding ? held : registered).promise); const status = yield* broker.invoke({ scope, tabId, @@ -2430,17 +2466,34 @@ it.live.each([ if (dialogState.startsWith("closed")) { expect(popupStatus.dialog).toBeNull(); expect(pendingDialog.accept).not.toHaveBeenCalled(); + expect(pendingDialog.dismiss).not.toHaveBeenCalled(); } else { expect(popupStatus.dialog).toMatchObject({ type: "alert", message: "hello" }); - yield* broker.invoke({ + // The held setup has not finished, yet the tab's owner can answer its dialog. + if (holding) expect(popup.page.waitForLoadState).not.toHaveBeenCalled(); + const accept = dialogState !== "holdsPageEnable"; + const answer = yield* broker.invoke({ scope, tabId: adopted.tabId, operation: "dialog", - input: { accept: true }, + input: { accept }, }); - expect(pendingDialog.accept).toHaveBeenCalledTimes(1); + expect(answer.dialog).toBeNull(); + expect(pendingDialog.accept).toHaveBeenCalledTimes(accept ? 1 : 0); + expect(pendingDialog.dismiss).toHaveBeenCalledTimes(accept ? 0 : 1); + if (holding) yield* Effect.promise(() => registered.promise); } - expect(pendingDialog.dismiss).not.toHaveBeenCalled(); + } + const { sessions } = yield* manager.list({ threadId: scope.thread.threadId }); + for (const session of sessions) + yield* browser.attachViewer(viewerInput(session.tabId, false)); + expect(sessions).toHaveLength(limit); + expect(popup.page.isClosed()).toBe(false); + expect(rejectedPopup.page.isClosed()).toBe(host === "server"); + if (host === "desktop") { + // No tab answers the window it could not adopt, so its dialogs stay the person's. + rejectedPopup.page.openDialog(); + expect(rejectedPopup.page.dialogAutoDismissed).not.toHaveBeenCalled(); } }), ).pipe(Effect.provide(layer)), diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index d1ac2ba6525f0..3682709f205ae 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -750,7 +750,8 @@ const make = Effect.gen(function* () { const sessionOpen = (tab: ServerTab) => !closedSessions.has(tabKey(tab.threadId, tab.tabId)); - const dropTab = (tab: ServerTab, closeSession: boolean) => { + /** `closePage: false` leaves a person's popup window open after its tab failed to adopt it. */ + const dropTab = (tab: ServerTab, closeSession: boolean, closePage = true) => { const key = tabKey(tab.threadId, tab.tabId); if (tabs.get(key) !== tab) return; tabs.delete(key); @@ -760,8 +761,10 @@ 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 void tab.page.close().catch(constVoid); + if (closePage) { + 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); void tab.recording?.encoder.close().catch(constVoid); void NodeFSP.rm(downloadDir(tab), { recursive: true, force: true }).catch(constVoid); @@ -858,13 +861,9 @@ const make = Effect.gen(function* () { const page = adopted?.page ?? desktop?.page ?? (await context.newPage()); if (!desktop) opened.push(() => page.close()); const cdp = await context.newCDPSession(page); - const pendingDialog = adopted?.pendingDialog; - if (pendingDialog) { - cdp.on("Page.javascriptDialogClosed", () => { - pendingDialog.current = null; - }); - } - await installPdfDocumentNavigation(cdp); + // A native popup can already show a modal, which holds page commands until it is answered. + const native = adopted?.native === true; + if (!native) await installPdfDocumentNavigation(cdp); // The desktop's page is already a normal browser. A popup's first request // went out before it became a tab; everything after presents as Chrome. if (!desktop) await presentAsChrome(cdp, host); @@ -928,19 +927,47 @@ const make = Effect.gen(function* () { if (tab.viewers.size > 0) return broadcastViewport(tab).catch(constVoid); }); } - if (!desktop) { - await page.setViewportSize(fixedViewportSize(tab.setting) ?? UNATTACHED_FILL_VIEWPORT); - } - await applyRendering(tab, snapshot); - if (desktop) { - // The desktop resizes and zooms its webview itself; viewers map input with the new size. - const remeasure = () => { - if (tab.viewers.size > 0) return broadcastViewport(tab).catch(constVoid); - }; - cdp.on("Page.frameResized", remeasure); - // A read a navigation broke has a fresh page to measure. - cdp.on("Page.loadEventFired", remeasure); - await cdp.send("Page.enable"); + const finishSetUp = async () => { + if (native) await installPdfDocumentNavigation(cdp); + if (!desktop) { + await page.setViewportSize(fixedViewportSize(tab.setting) ?? UNATTACHED_FILL_VIEWPORT); + } + await applyRendering(tab, snapshot); + if (desktop) { + // The desktop resizes and zooms its webview itself; viewers map input with the new size. + const remeasure = () => { + if (tab.viewers.size > 0) return broadcastViewport(tab).catch(constVoid); + }; + cdp.on("Page.frameResized", remeasure); + // A read a navigation broke has a fresh page to measure. + cdp.on("Page.loadEventFired", remeasure); + await cdp.send("Page.enable"); + } + }; + const key = tabKey(tab.threadId, tab.tabId); + const register = async () => { + if (closedPendingTabs.delete(key)) { + await control.close().catch(constVoid); + if (native) await page.close().catch(constVoid); + throw new ServerBrowserTabNotFoundError({ threadId: tab.threadId, tabId: tab.tabId }); + } + // Playwright cannot reload a crashed page, so it must leave the tab list. + page.on("close", () => dropTab(tab, true)); + page.on("crash", () => dropTab(tab, true)); + tabs.set(key, tab); + reportLiveTabs(); + }; + if (native) { + // Listed first so its owner can answer that modal; later actions queue behind the setup. + await register(); + await control.system(finishSetUp).catch((cause: unknown) => { + dropTab(tab, true, false); + throw cause; + }); + if (tab.closing) + throw new ServerBrowserTabNotFoundError({ threadId: tab.threadId, tabId: tab.tabId }); + } else { + await finishSetUp(); } const isMainNavigation = (request: { isNavigationRequest(): boolean; frame(): unknown }) => request.isNavigationRequest() && request.frame() === page.mainFrame(); @@ -1033,17 +1060,7 @@ const make = Effect.gen(function* () { updateFileChooserInterception(tab); // Popups become tabs and keep `window.opener`, so sign-in popups can report back. page.on("popup", (popup) => void adoptPopup(tab, popup)); - // Playwright cannot reload a crashed page, so it must leave the tab list. - const key = tabKey(tab.threadId, tab.tabId); - if (closedPendingTabs.delete(key)) { - await control.close().catch(constVoid); - if (adopted?.native) await page.close().catch(constVoid); - throw new ServerBrowserTabNotFoundError({ threadId: tab.threadId, tabId: tab.tabId }); - } - page.on("close", () => dropTab(tab, true)); - page.on("crash", () => dropTab(tab, true)); - tabs.set(key, tab); - reportLiveTabs(); + if (!native) await register(); // A desktop page navigates on its own and can finish loading before these listeners attach. if (desktop) void page.waitForLoadState("load").then(() => reportLoaded(tab), constVoid); // Popups opened before the connection fired their `popup` event before the listener above. From 2ce507e2c968f6405abc6be96c70f635d7e22bb5 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Thu, 5 Nov 2026 08:48:23 +0000 Subject: [PATCH 18/24] test(server): held popup setup still denies other agents and human control While a native popup's adoption setup is held by its open dialog, the regression now also shows another agent session is refused (agentMismatch), the owner is refused while a person controls the tab (humanControl), neither attempt answers the dialog, and the opener keeps running actions. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 55 ++++++++++++++++--- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index ee20c07c5d0cf..3ff628956b849 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -2377,9 +2377,10 @@ it.live.each([ popup.newCDPSession = async (target) => { await launchGate.promise; const session = await newCDPSession(target); + const setUp = popup.sessions.length === 1; const send = session.send.getMockImplementation()!; session.send.mockImplementation(async (method, input) => { - if (holds(method, input)) { + if (setUp && holds(method, input)) { held.resolve(); await answered.promise; } @@ -2469,19 +2470,55 @@ it.live.each([ expect(pendingDialog.dismiss).not.toHaveBeenCalled(); } else { expect(popupStatus.dialog).toMatchObject({ type: "alert", message: "hello" }); - // The held setup has not finished, yet the tab's owner can answer its dialog. - if (holding) expect(popup.page.waitForLoadState).not.toHaveBeenCalled(); const accept = dialogState !== "holdsPageEnable"; - const answer = yield* broker.invoke({ - scope, - tabId: adopted.tabId, - operation: "dialog", - input: { accept }, - }); + const answerAs = (target: typeof scope) => + broker.invoke({ + scope: target, + tabId: adopted.tabId, + operation: "dialog", + input: { accept }, + }); + const handoffs: Array> = []; + if (holding) { + // The held setup has not finished, yet only the tab's owner can answer its dialog. + expect(popup.page.waitForLoadState).not.toHaveBeenCalled(); + expect(yield* answerAs(asSession("intruder")).pipe(Effect.flip)).toMatchObject({ + reason: "agentMismatch", + }); + const person = yield* browser.attachViewer(viewerInput(adopted.tabId, true)); + const controlled = (byPerson: boolean) => + Effect.gen(function* () { + let control = yield* Queue.take(person.output); + while (control._tag !== "control" || (control.controller === "you") !== byPerson) + control = yield* Queue.take(person.output); + }); + handoffs.push(yield* person.input({ type: "takeControl" }).pipe(Effect.forkScoped)); + yield* controlled(true); + expect(yield* answerAs(scope).pipe(Effect.flip)).toMatchObject({ + reason: "humanControl", + }); + handoffs.push( + yield* person.input({ type: "releaseControl" }).pipe(Effect.forkScoped), + ); + yield* controlled(false); + expect(pendingDialog.accept).not.toHaveBeenCalled(); + expect(pendingDialog.dismiss).not.toHaveBeenCalled(); + // Its opener keeps working while the popup's setup is held. + expect( + yield* broker.invoke({ + scope, + tabId, + operation: "evaluate", + input: { expression: "document.title" }, + }), + ).toBe("evaluated"); + } + const answer = yield* answerAs(scope); expect(answer.dialog).toBeNull(); expect(pendingDialog.accept).toHaveBeenCalledTimes(accept ? 1 : 0); expect(pendingDialog.dismiss).toHaveBeenCalledTimes(accept ? 0 : 1); if (holding) yield* Effect.promise(() => registered.promise); + for (const handoff of handoffs) yield* Fiber.join(handoff); } } const { sessions } = yield* manager.list({ threadId: scope.thread.threadId }); From 6ec4c56df6786be5ec7282d471a9911fd4728220 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Thu, 5 Nov 2026 09:30:03 +0000 Subject: [PATCH 19/24] fix(server): client closes no longer wait for a dialog holding popup setup The server applied every preview manager event in one serial loop that awaited each tab's setup and queued settings. A native popup whose setup a still-open dialog held therefore stopped all later events: a client's close removed the session but left that popup and any sibling listed and open until someone answered the dialog. Events are now applied without waiting. A close drops its tab immediately, and opens and settings run in a per-tab chain, so a tab's settings keep their order behind its own setup without delaying other tabs. Setup still starts while the open event is handled, so a close that arrives first finds it pending and the tab never registers. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 129 ++++++++++++++++++ apps/server/src/preview/ServerBrowser.ts | 77 +++++++---- 2 files changed, 179 insertions(+), 27 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 3ff628956b849..f591247941cca 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -59,6 +59,7 @@ vi.mock("./ServerBrowserContexts.ts", () => ({ }; } contexts.push(context); + contextCreated?.resolve(context); return context as unknown as BrowserContext; } async scratchPage() { @@ -321,6 +322,7 @@ const contexts: ReturnType[] = []; const encoderStreams: Array<{ frameRate: number | undefined; bitsPerSecond: number }> = []; const contextRequests: Array<{ profileId: string; isolated: boolean }> = []; let contextGate: PromiseWithResolvers | null = null; +let contextCreated: PromiseWithResolvers> | null = null; let pageCopy = ""; let keyCopy = ""; let focusedFrame: ReturnType | null = null; @@ -482,6 +484,7 @@ beforeEach(() => { encoderStreams.length = 0; contextRequests.length = 0; contextGate = null; + contextCreated = null; contextFailure = null; pageSetupFailure = null; initialNavigation = null; @@ -759,6 +762,108 @@ it.effect("an agent's desktop tab's popup windows become its tabs, drawn by the }); expect(afterFailure.tabs?.map((tab) => tab.tabId)).not.toContain(failedLateTab.tabId); + // A dialog holding a popup's setup holds neither a client's close nor other tabs' events. + const answered = Promise.withResolvers(); + yield* Effect.addFinalizer(() => Effect.sync(() => answered.resolve())); + const holdSetUp = (url: string, holds: (method: string, input: unknown) => boolean) => { + const popup = makeContext(undefined, url); + const held = Promise.withResolvers(); + const reloaded = Promise.withResolvers(); + const newHeldSession = popup.newCDPSession; + popup.newCDPSession = async (target) => { + const session = await newHeldSession(target); + const send = session.send.getMockImplementation()!; + session.send.mockImplementation(async (method, input) => { + if (holds(method, input)) { + held.resolve(); + await answered.promise; + } + if (method === "Page.reload") reloaded.resolve(); + return send(method, input); + }); + return session; + }; + openerPage.emit("popup", popup.page); + const dialog = popup.page.openDialog(); + dialog.accept.mockImplementation(async () => answered.resolve()); + return { popup, dialog, held: held.promise, reloaded: reloaded.promise }; + }; + const heldProbe = holdSetUp( + "https://example.com/held-probe", + (method, input) => + method === "Runtime.evaluate" && + (input as { expression?: string }).expression === "navigator.pdfViewerEnabled", + ); + const heldProbeTab = yield* nextOpened; + const heldEnable = holdSetUp( + "https://example.com/held-enable", + (method) => method === "Page.enable", + ); + const heldEnableTab = yield* nextOpened; + yield* Effect.promise(() => Promise.all([heldProbe.held, heldEnable.held])); + const threadId = scope.thread.threadId; + for (const { tabId } of [heldProbeTab, heldEnableTab]) { + yield* manager.resize({ + threadId, + tabId, + viewport: { _tag: "freeform", width: 900, height: 700 }, + }); + yield* manager.adjust({ threadId, tabId, colorScheme: "dark" }); + yield* manager.adjust({ threadId, tabId, clear: "cache" }); + yield* manager.adjust({ threadId, tabId, hardReload: true }); + } + const closing = [heldProbe.popup.page, later.page].map( + (page) => new Promise((resolve) => page.once("close", () => resolve())), + ); + yield* manager.close({ threadId, tabId: heldProbeTab.tabId }); + yield* manager.close({ threadId, tabId: laterTab.tabId }); + yield* Effect.promise(() => Promise.all(closing)); + const whileHeld = yield* broker.invoke({ + scope, + tabId: opener.tabId!, + operation: "status", + input: {}, + }); + const listed = whileHeld.tabs?.map((tab) => tab.tabId); + expect(listed).toContain(heldEnableTab.tabId); + expect(listed).not.toContain(heldProbeTab.tabId); + expect(listed).not.toContain(laterTab.tabId); + expect(heldProbe.dialog.accept).not.toHaveBeenCalled(); + expect(heldProbe.dialog.dismiss).not.toHaveBeenCalled(); + + // Its setup refuses the closed tab once the dialog lets it finish. + const refused = Promise.withResolvers(); + const realClose = manager.close; + const closes = vi.spyOn(manager, "close").mockImplementation((input) => { + if (input.tabId === heldProbeTab.tabId) refused.resolve(); + return realClose(input); + }); + yield* Effect.addFinalizer(() => Effect.sync(() => closes.mockRestore())); + yield* broker.invoke({ + scope, + tabId: heldEnableTab.tabId, + operation: "dialog", + input: { accept: true }, + }); + yield* Effect.promise(() => refused.promise); + const afterSetUp = yield* broker.invoke({ + scope, + tabId: opener.tabId!, + operation: "status", + input: {}, + }); + expect(afterSetUp.tabs?.map((tab) => tab.tabId)).not.toContain(heldProbeTab.tabId); + // The open popup's settings, queued behind its setup, apply in the order they were made. + yield* Effect.promise(() => heldEnable.reloaded); + const sent = heldEnable.popup.sessions[0]!.send.mock; + const order = (method: string) => + sent.invocationCallOrder[sent.calls.findIndex(([name]) => name === method)]!; + expect(heldEnable.popup.page.emulateMedia).toHaveBeenCalledWith({ colorScheme: "dark" }); + expect(heldEnable.popup.page.emulateMedia.mock.invocationCallOrder[0]).toBeLessThan( + order("Network.clearBrowserCache"), + ); + expect(order("Network.clearBrowserCache")).toBeLessThan(order("Page.reload")); + // A person's desktop tab keeps its popup windows to itself. desktopRendersNext = true; const human = yield* manager.open({ threadId: scope.thread.threadId, runtime: "server" }); @@ -2592,6 +2697,30 @@ it.live("failed session and browser launches release capacity, as does closing a input: {}, }); expect(status.tabs).toHaveLength(8); + + // A person's tab closed while its browser launches is never listed or left open. + const launch = (contextGate = Promise.withResolvers()); + const created = (contextCreated = Promise.withResolvers>()); + yield* Effect.addFinalizer(() => Effect.sync(() => launch.resolve())); + const closedEarly = yield* manager.open({ + threadId: scope.thread.threadId, + runtime: "server", + }); + yield* manager.close({ threadId: scope.thread.threadId, tabId: closedEarly.tabId }); + launch.resolve(); + const { page } = yield* Effect.promise(() => created.promise); + yield* Effect.promise( + () => + new Promise((resolve) => + page.isClosed() ? resolve() : page.once("close", () => resolve()), + ), + ); + const afterClose = yield* broker.invoke({ + scope, + operation: "status", + input: {}, + }); + expect(afterClose.tabs?.map((tab) => tab.tabId)).not.toContain(closedEarly.tabId); }), ).pipe(Effect.provide(layer)), ); diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 3682709f205ae..a50aa9971fb57 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -2397,44 +2397,67 @@ const make = Effect.gen(function* () { Effect.ignore, ); + // A dialog can hold a tab's setup for as long as a person leaves it open, so each tab's + // events wait only for that tab's earlier ones, and closing waits for none. + const tabEvents = new Map>(); + const afterTabEvents = (key: string, work: () => Promise) => { + const next = (tabEvents.get(key) ?? Promise.resolve()).then(work).then(constVoid, constVoid); + tabEvents.set(key, next); + void next.then(() => { + if (tabEvents.get(key) === next) tabEvents.delete(key); + }); + }; + const mirrorManagerEvent = (event: PreviewEvent) => - Effect.promise(async () => { - if (event.type === "opened" && event.snapshot.runtime === "server") { - const key = tabKey(event.snapshot.threadId, event.snapshot.tabId); - if (event.snapshot.automationOwner === undefined || pendingTabs.has(key) || tabs.has(key)) - await ensureTab(event.snapshot).catch(constVoid); + Effect.sync(() => { + const key = tabKey(event.threadId, event.tabId); + if (event.type === "opened") { + if (event.snapshot.runtime !== "server") return; + if (event.snapshot.automationOwner === undefined || pendingTabs.has(key) || tabs.has(key)) { + // Started now, so a later close finds it pending and it never registers. + const opening = ensureTab(event.snapshot); + afterTabEvents(key, () => opening); + } return; } - const key = tabKey(event.threadId, event.tabId); - const tab = tabs.get(key); - if (event.type === "closed" && !tab && pendingTabs.has(key)) closedPendingTabs.add(key); - if (!tab) return; if (event.type === "closed") { + const tab = tabs.get(key); + if (!tab) { + if (pendingTabs.has(key)) closedPendingTabs.add(key); + return; + } closedSessions.add(key); dropTab(tab, false); closedSessions.delete(key); return; } - if (event.type === "resized") { - await tab.control - .system(async () => { - if ( - !viewportSettingsEqual(event.snapshot.viewport ?? FILL_PREVIEW_VIEWPORT, tab.setting) - ) { - await applySetting(tab, event.snapshot.viewport ?? FILL_PREVIEW_VIEWPORT); - } - await applyRendering(tab, event.snapshot); - const request = event.request; - if (request?.clear === "cookies") await tab.cdp.send("Network.clearBrowserCookies"); - if (request?.clear === "cache") await tab.cdp.send("Network.clearBrowserCache"); - if (request?.hardReload) await tab.cdp.send("Page.reload", { ignoreCache: true }); - }) - .catch((cause: unknown) => - runFork(Effect.logWarning("server preview could not apply a tab setting", { cause })), - ); - } + if (event.type === "resized") afterTabEvents(key, () => applyTabSetting(key, event)); }); + const applyTabSetting = async ( + key: string, + event: Extract, + ) => { + const tab = tabs.get(key); + if (!tab) return; + await tab.control + .system(async () => { + if (!viewportSettingsEqual(event.snapshot.viewport ?? FILL_PREVIEW_VIEWPORT, tab.setting)) { + await applySetting(tab, event.snapshot.viewport ?? FILL_PREVIEW_VIEWPORT); + } + await applyRendering(tab, event.snapshot); + const request = event.request; + if (request?.clear === "cookies") await tab.cdp.send("Network.clearBrowserCookies"); + if (request?.clear === "cache") await tab.cdp.send("Network.clearBrowserCache"); + if (request?.hardReload) await tab.cdp.send("Page.reload", { ignoreCache: true }); + }) + .catch((cause: unknown) => { + // Closing the tab ends the setting with it. + if (!tab.closing) + runFork(Effect.logWarning("server preview could not apply a tab setting", { cause })); + }); + }; + const releaseViewerInput = async (viewer: ViewerState, session: CDPSession) => { for (const { key, code } of viewer.pressedKeys.values()) { await session.send("Input.dispatchKeyEvent", { type: "keyUp", key, code }).catch(constVoid); From 3894cc85b158c76ea25ec1c2889827c6953447db Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Thu, 5 Nov 2026 11:36:52 +0000 Subject: [PATCH 20/24] fix(server): viewers connect while a dialog holds the page's first frame A viewer attach waited for its screencast to start and for an initial still before returning, and the stream upgraded the socket only after that. A native dialog holding a popup's capture commands therefore left a fresh viewer stuck on Connecting, with no control state and no way to answer the dialog. The first screencast and still now start in the viewer's scope without blocking the attach, so the socket opens and control and viewport arrive first. A viewer that disconnects while its capture is held sends no further capture commands after its session detaches. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 55 ++++++++++++++++--- apps/server/src/preview/ServerBrowser.ts | 30 ++++++---- 2 files changed, 65 insertions(+), 20 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index f591247941cca..bb356361d9b50 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -1073,10 +1073,9 @@ it.live("streams to a read-only viewer without allowing takeover, input, or view yield* viewer.input(message); expect(page.setViewportSize).not.toHaveBeenCalled(); expect(session.send.mock.calls.some(([method]) => method.startsWith("Input."))).toBe(false); - const outputs = yield* Queue.takeAll(viewer.output); - expect(outputs).toContainEqual( - expect.objectContaining({ _tag: "frame", data: Buffer.from("frame") }), - ); + const outputs = [yield* Queue.take(viewer.output)]; + while (outputs.at(-1)!._tag !== "frame") outputs.push(yield* Queue.take(viewer.output)); + expect(outputs.at(-1)).toMatchObject({ _tag: "frame", data: Buffer.from("frame") }); expect(outputs).toContainEqual(expect.objectContaining({ _tag: "viewport" })); expect(outputs).toContainEqual( expect.objectContaining({ _tag: "control", canOperate: false, controller: "agent" }), @@ -2473,7 +2472,9 @@ it.live.each([ const answered = Promise.withResolvers(); const held = Promise.withResolvers(); const registered = Promise.withResolvers(); + const captureHeld = Promise.withResolvers(); popup.page.waitForLoadState = vi.fn(async () => registered.resolve()); + const holding = dialogState.startsWith("holds"); const holds = (method: string, input: unknown) => dialogState === "holdsPdfProbe" ? method === "Runtime.evaluate" && @@ -2489,6 +2490,15 @@ it.live.each([ held.resolve(); await answered.promise; } + // The modal holds every session's frames, so a viewer's first frame waits too. + if ( + !setUp && + holding && + (method === "Page.startScreencast" || method === "Page.captureScreenshot") + ) { + captureHeld.resolve(); + await answered.promise; + } if (!changedDialog) { if (method === "Runtime.evaluate" && dialogState === "closedBeforeTab") { changedDialog = true; @@ -2551,7 +2561,6 @@ it.live.each([ }); } if (host === "server") yield* Effect.promise(() => resized.promise); - const holding = dialogState.startsWith("holds"); // A held setup waits for its dialog's answer; the others finish on their own. if (host === "desktop") yield* Effect.promise(() => (holding ? held : registered).promise); const status = yield* broker.invoke({ @@ -2584,30 +2593,58 @@ it.live.each([ input: { accept }, }); const handoffs: Array> = []; + const personAnswers = dialogState === "holdsPdfProbe"; + let afterAnswer: Effect.Effect = Effect.void; if (holding) { // The held setup has not finished, yet only the tab's owner can answer its dialog. expect(popup.page.waitForLoadState).not.toHaveBeenCalled(); expect(yield* answerAs(asSession("intruder")).pipe(Effect.flip)).toMatchObject({ reason: "agentMismatch", }); + // A viewer that leaves while its first frame is held never captures again. + const leaving = yield* Scope.make(); + yield* browser + .attachViewer(viewerInput(adopted.tabId, true)) + .pipe(Effect.provideService(Scope.Scope, leaving)); + const leavingSession = popup.sessions.at(-1)!; + yield* Effect.promise(() => captureHeld.promise); + yield* Scope.close(leaving, Exit.void); + expect(leavingSession.detach).toHaveBeenCalledTimes(1); + // A fresh viewer gets control and can answer before its first frame. const person = yield* browser.attachViewer(viewerInput(adopted.tabId, true)); + let framed = false; const controlled = (byPerson: boolean) => Effect.gen(function* () { let control = yield* Queue.take(person.output); - while (control._tag !== "control" || (control.controller === "you") !== byPerson) + while ( + control._tag !== "control" || + (control.controller === "you") !== byPerson + ) { + if (control._tag === "frame") framed = true; control = yield* Queue.take(person.output); + } }); handoffs.push(yield* person.input({ type: "takeControl" }).pipe(Effect.forkScoped)); yield* controlled(true); + expect(framed).toBe(false); expect(yield* answerAs(scope).pipe(Effect.flip)).toMatchObject({ reason: "humanControl", }); + if (personAnswers) yield* person.input({ type: "dialog", accept }); handoffs.push( yield* person.input({ type: "releaseControl" }).pipe(Effect.forkScoped), ); yield* controlled(false); - expect(pendingDialog.accept).not.toHaveBeenCalled(); + expect(pendingDialog.accept).toHaveBeenCalledTimes(personAnswers ? 1 : 0); expect(pendingDialog.dismiss).not.toHaveBeenCalled(); + afterAnswer = Effect.gen(function* () { + while (!framed) framed = (yield* Queue.take(person.output))._tag === "frame"; + const detachedAt = leavingSession.detach.mock.invocationCallOrder[0]!; + const { calls, invocationCallOrder } = leavingSession.send.mock; + expect( + calls.filter((_, index) => invocationCallOrder[index]! > detachedAt), + ).toEqual([]); + }); // Its opener keeps working while the popup's setup is held. expect( yield* broker.invoke({ @@ -2618,12 +2655,12 @@ it.live.each([ }), ).toBe("evaluated"); } - const answer = yield* answerAs(scope); - expect(answer.dialog).toBeNull(); + if (!personAnswers) expect((yield* answerAs(scope)).dialog).toBeNull(); expect(pendingDialog.accept).toHaveBeenCalledTimes(accept ? 1 : 0); expect(pendingDialog.dismiss).toHaveBeenCalledTimes(accept ? 0 : 1); if (holding) yield* Effect.promise(() => registered.promise); for (const handoff of handoffs) yield* Fiber.join(handoff); + yield* afterAnswer; } } const { sessions } = yield* manager.list({ threadId: scope.thread.threadId }); diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index a50aa9971fb57..4e982092f67f5 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -2622,19 +2622,21 @@ const make = Effect.gen(function* () { capacity: VIEWER_OUTPUT_LIMIT, strategy: "dropping", }); + let detached = false; const session = yield* Effect.acquireRelease( Effect.tryPromise({ try: () => tab.page.context().newCDPSession(tab.page), catch: (cause) => new ServerBrowserLaunchError({ cause }), }), (session) => - Effect.promise(() => - session + Effect.promise(() => { + detached = true; + return session .send("Page.stopScreencast") .catch(constVoid) .then(() => session.detach()) - .catch(constVoid), - ), + .catch(constVoid); + }), ); const quality = Math.min(100, Math.max(1, Math.round(input.quality))); // Chromium may drop the final frame during a burst; send a still when it settles. @@ -2647,8 +2649,9 @@ const make = Effect.gen(function* () { const startScreencast = (scale: number) => { screencastScale = scale; screencastParams = screencastParams.then(async () => { - if (tab.capturing > 0) return; + if (detached || tab.capturing > 0) return; await session.send("Page.stopScreencast").catch(constVoid); + if (detached) return; await session .send("Page.startScreencast", { format: "jpeg", @@ -2717,8 +2720,8 @@ const make = Effect.gen(function* () { broadcastControl(tab); pushFileChooser(tab); const pushStill = async () => { - const data = await withCaptureLock(tab, () => - ServerBrowserPage.captureViewport(session, { format: "jpeg", quality }), + const data = await withCaptureLock(tab, async () => + detached ? null : ServerBrowserPage.captureViewport(session, { format: "jpeg", quality }), ).catch(() => null); if (data) viewer.push({ _tag: "frame", data: Buffer.from(data, "base64"), ack: Effect.void }); }; @@ -2774,10 +2777,15 @@ const make = Effect.gen(function* () { }); }); void broadcastViewport(tab).catch(constVoid); - yield* Effect.promise(() => startScreencast(1)); - // An idle page does not repaint for a new screencast, so the viewer - // starts from a still unless a live frame beat it. - if (!screencastStarted) yield* Effect.promise(pushStill); + // A page dialog can hold capture until it is answered, so the viewer + // returns first and can answer it. An idle page does not repaint for a + // new screencast, so the viewer starts from a still unless a live frame beat it. + yield* Effect.forkScoped( + Effect.promise(async () => { + await startScreencast(1); + if (!screencastStarted) await pushStill(); + }), + ); return { output, input: (raw: unknown) => From 9028a577fc1271c2081f83b36c2bebdf90f7cc67 Mon Sep 17 00:00:00 2001 From: shivam <91240327+shivamhwp@users.noreply.github.com> Date: Tue, 10 Nov 2026 08:42:20 +0000 Subject: [PATCH 21/24] fix(desktop): closed popups no longer block download setup --- apps/desktop/src/preview/CdpRelay.test.ts | 62 ++++++++++++++++++----- apps/desktop/src/preview/CdpRelay.ts | 6 ++- 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/apps/desktop/src/preview/CdpRelay.test.ts b/apps/desktop/src/preview/CdpRelay.test.ts index 6a7f97ea37992..09505ffc5babe 100644 --- a/apps/desktop/src/preview/CdpRelay.test.ts +++ b/apps/desktop/src/preview/CdpRelay.test.ts @@ -644,23 +644,35 @@ describe("CDP relay downloads", () => { expect(written).toEqual([]); }); - it.each([false, true])( - "keeps the previous download configuration (events enabled: %s) when configuration fails", - async (enabled) => { - const send = vi - .fn(async () => ({})) - .mockResolvedValueOnce({}) - .mockRejectedValueOnce(new Error("Page command rejected")); + it.each( + [false, true].flatMap((enabled) => + ["page", "popup", "removed popup"].map((failure) => ({ enabled, failure })), + ), + )( + "keeps download configuration consistent when $failure rejects (events enabled: $enabled)", + async ({ enabled, failure }) => { + const send = vi.fn(async () => ({})); + const popupSend = vi.fn(async () => ({})); const written: Array> = []; let directory: string | null = null; + let popupDirectory: string | null = null; const relay = makeRelay( makeTarget(send, (value) => { directory = value; }), (raw) => written.push(JSON.parse(raw)), ); + const popup = relay.addPopup({ + ...makeTarget(popupSend, (value) => { + popupDirectory = value; + }), + targetId: async () => "POPUP", + }); relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach", params: {} })); for (const [id, eventsEnabled] of [enabled, !enabled].entries()) { + const pending = Promise.withResolvers(); + if (id === 1) + (failure === "page" ? send : popupSend).mockImplementationOnce(() => pending.promise); relay.receive( JSON.stringify({ id: id + 2, @@ -672,18 +684,42 @@ describe("CDP relay downloads", () => { }, }), ); + if (id === 1) { + if (failure === "removed popup") popup.remove(); + pending.reject(new Error("Page command rejected")); + } await settle(); } - expect(written).toContainEqual({ - id: 3, - error: { code: -32000, message: "Page command rejected" }, - }); - expect(directory).toBe("/srv/artifacts"); + const removed = failure === "removed popup"; + expect(written).toContainEqual( + removed + ? { id: 3, result: {} } + : { id: 3, error: { code: -32000, message: "Page command rejected" } }, + ); + const expectedDirectory = removed + ? enabled + ? null + : "/srv/new-artifacts" + : "/srv/artifacts"; + expect(directory).toBe(expectedDirectory); + expect(popupDirectory).toBe("/srv/artifacts"); + if (removed) { + const lateSend = vi.fn(async () => ({})); + const lateDirectory = vi.fn(); + relay.addPopup(makeTarget(lateSend, lateDirectory)); + await settle(); + expect(lateDirectory).toHaveBeenCalledExactlyOnceWith(expectedDirectory); + expect(lateSend).toHaveBeenCalledExactlyOnceWith( + "Page.setDownloadBehavior", + { behavior: enabled ? "deny" : "default" }, + undefined, + ); + } written.length = 0; relay.event("Page.downloadWillBegin", { guid: "g1" }, ""); relay.event("Page.downloadProgress", { guid: "g1", state: "completed" }, ""); expect(written).toEqual( - enabled + (removed ? !enabled : 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 cd33a9ec1aaa6..50c076cb1c98e 100644 --- a/apps/desktop/src/preview/CdpRelay.ts +++ b/apps/desktop/src/preview/CdpRelay.ts @@ -215,7 +215,11 @@ export function createCdpRelayConnection( // Browser-scoped behavior targets Electron's app session; preview paths belong to will-download. const [result] = await Promise.all( [...targets].map((relayed) => - relayed.target.send("Page.setDownloadBehavior", { behavior }, undefined), + relayed.target + .send("Page.setDownloadBehavior", { behavior }, undefined) + .catch((cause: unknown) => { + if (relayed === page || !relayed.removed) throw cause; + }), ), ); downloads = { behavior, directory: allowed ? directory : null }; From dddbcbcb7fd0d28549ba4cd9c5607f1bace3a9d6 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 11 Nov 2026 03:29:07 +0000 Subject: [PATCH 22/24] fix(server): a native popup opening as its opener closes stays open AI-assisted; verified with the tests and tools listed above. --- apps/server/src/preview/ServerBrowser.test.ts | 6 +++++ apps/server/src/preview/ServerBrowser.ts | 27 +++++++++---------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 1b91aa06d0677..7746d0e8c06f9 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -897,6 +897,12 @@ it.effect("an agent's desktop tab's popup windows become its tabs, drawn by the const human = yield* manager.open({ threadId: scope.thread.threadId, runtime: "server" }); yield* broker.invoke({ scope, tabId: human.tabId, operation: "status", input: {} }); expect(desktopEndpoints.at(-1)).toEqual({ tabId: human.tabId, popups: false }); + + // A window that opens as its opener's tab closes stays the person's. + yield* broker.invoke({ scope, tabId: opener.tabId!, operation: "close", input: {} }); + const lateWindow = makeContext(undefined, "https://example.com/late"); + openerPage.emit("popup", lateWindow.page); + expect(lateWindow.page.close).not.toHaveBeenCalled(); }), ).pipe(Effect.provide(layer)), ); diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index aa89c932470bf..9374c213e827c 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -1397,17 +1397,19 @@ const make = Effect.gen(function* () { const adoptPopup = async (opener: ServerTab, popup: Page) => { if (adoptedPopups.has(popup) || popup.isClosed()) return; adoptedPopups.add(popup); + const native = opener.desktop !== null; + // A native window is the person's; only one the server opened closes with a refused adoption. + const refuse = async () => { + if (!native) await popup.close().catch(constVoid); + }; const pendingDialog: { current: Dialog | null } = { current: null }; - if (opener.desktop !== null) { + if (native) { // Keep native dialogs pending even when this popup never becomes an automation tab. popup.on("dialog", (dialog) => { pendingDialog.current = dialog; }); } - if (opener.closing) { - await popup.close().catch(constVoid); - return; - } + if (opener.closing) return refuse(); const url = popup.url(); const controller = [...opener.viewers].find( @@ -1428,18 +1430,13 @@ const make = Effect.gen(function* () { adoptedPages.set(tabKey(snapshot.threadId, snapshot.tabId), { page: popup, openerTabId: opener.tabId, - native: opener.desktop !== null, - ...(opener.desktop === null ? {} : { pendingDialog }), + native, + ...(native ? { pendingDialog } : {}), }), }), - ).then( - (tab) => { - if (opener.control.agentId === null) controller?.push({ _tag: "popup", tabId: tab.tabId }); - }, - async () => { - if (opener.desktop === null) await popup.close().catch(constVoid); - }, - ); + ).then((tab) => { + if (opener.control.agentId === null) controller?.push({ _tag: "popup", tabId: tab.tabId }); + }, refuse); }; const ensureTab = (snapshot: PreviewSessionSnapshot): Promise => { From ecec713e9809e5722cd5005ddcca0eeb793fcec7 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 11 Nov 2026 05:51:55 +0000 Subject: [PATCH 23/24] fix(server): a native popup's dialog is answered in its own window Electron leaves a native window's dialog box open when CDP answers it, so the agent can no longer resolve a native popup's dialog; the person answers it in the window, which clears it for the agent. AI-assisted; verified with the tests and tools listed above. --- apps/server/src/preview/ServerBrowser.test.ts | 40 ++++++++++++++----- apps/server/src/preview/ServerBrowser.ts | 6 +++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 7746d0e8c06f9..43a4f35c8f751 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -867,12 +867,19 @@ it.effect("an agent's desktop tab's popup windows become its tabs, drawn by the return realClose(input); }); yield* Effect.addFinalizer(() => Effect.sync(() => closes.mockRestore())); - yield* broker.invoke({ - scope, - tabId: heldEnableTab.tabId, - operation: "dialog", - input: { accept: true }, - }); + // A native window's dialog is the person's to answer there. + expect( + yield* broker + .invoke({ + scope, + tabId: heldEnableTab.tabId, + operation: "dialog", + input: { accept: true }, + }) + .pipe(Effect.flip), + ).toMatchObject({ reason: "humanControl" }); + heldEnable.popup.sessions[0]!.emit("Page.javascriptDialogClosed"); + answered.resolve(); yield* Effect.promise(() => refused.promise); const afterSetUp = yield* broker.invoke({ scope, @@ -2787,9 +2794,24 @@ it.live.each([ }), ).toBe("evaluated"); } - if (!personAnswers) expect((yield* answerAs(scope)).dialog).toBeNull(); - expect(pendingDialog.accept).toHaveBeenCalledTimes(accept ? 1 : 0); - expect(pendingDialog.dismiss).toHaveBeenCalledTimes(accept ? 0 : 1); + if (!personAnswers) { + // Electron would leave the native box open, so the agent cannot answer it. + expect(yield* answerAs(scope).pipe(Effect.flip)).toMatchObject({ + reason: "humanControl", + }); + // The person answers in the native window. + popup.sessions[0]!.emit("Page.javascriptDialogClosed"); + answered.resolve(); + const answeredStatus = yield* broker.invoke({ + scope, + tabId: adopted.tabId, + operation: "status", + input: {}, + }); + expect(answeredStatus.dialog).toBeNull(); + } + expect(pendingDialog.accept).toHaveBeenCalledTimes(personAnswers && accept ? 1 : 0); + expect(pendingDialog.dismiss).not.toHaveBeenCalled(); if (holding) yield* Effect.promise(() => registered.promise); for (const handoff of handoffs) yield* Fiber.join(handoff); yield* afterAnswer; diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 9374c213e827c..ada230eefce18 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -2236,6 +2236,12 @@ const make = Effect.gen(function* () { message: "A human controls this tab.", reason: "humanControl", }); + // Electron keeps a native window's dialog box open when CDP answers it, so the person must. + if (tab.dialog && isNativePopup(tab)) + throw new BrowserControlInterrupted({ + message: "This dialog waits for the person in its desktop window.", + reason: "humanControl", + }); await resolveDialog(tab, input as PreviewAutomationDialogInput); return statusWithTitle(tab, request.agentSessionId); } From 8125d8ca48141d490287961dd745601a74ba97fa Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 11 Nov 2026 08:54:36 +0000 Subject: [PATCH 24/24] fix(server): a dialog the person answers natively clears from its popup tab Track Playwright's dialogclosed event on the tab and on the adoption holder, so a native answer given before the tab's own CDP session listens no longer leaves the popup reporting a pending dialog. AI-assisted; verified with the tests and tools listed above. --- apps/server/src/preview/ServerBrowser.test.ts | 4 ++-- apps/server/src/preview/ServerBrowser.ts | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 43a4f35c8f751..118b76114ad10 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -2799,8 +2799,8 @@ it.live.each([ expect(yield* answerAs(scope).pipe(Effect.flip)).toMatchObject({ reason: "humanControl", }); - // The person answers in the native window. - popup.sessions[0]!.emit("Page.javascriptDialogClosed"); + // The person answers in the native window, which Playwright's own session reports. + popup.page.emit("dialogclosed", pendingDialog); answered.resolve(); const answeredStatus = yield* broker.invoke({ scope, diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index ada230eefce18..d7cf2b8cecff0 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -930,6 +930,12 @@ const make = Effect.gen(function* () { tab.dialog = dialog; broadcastControl(tab); }); + // A person can answer a native window's dialog before this tab's own CDP session listens. + page.on("dialogclosed", (dialog) => { + if (tab.dialog !== dialog) return; + tab.dialog = null; + broadcastControl(tab); + }); if (desktop) { cdp.on("Page.javascriptDialogClosed", () => { tab.dialog = null; @@ -1408,6 +1414,9 @@ const make = Effect.gen(function* () { popup.on("dialog", (dialog) => { pendingDialog.current = dialog; }); + popup.on("dialogclosed", (dialog) => { + if (pendingDialog.current === dialog) pendingDialog.current = null; + }); } if (opener.closing) return refuse(); const url = popup.url();