From dfda67de7431f27e9e35beef425b786b7e06196a Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Sun, 25 Oct 2026 21:52:10 +0000 Subject: [PATCH] fix(preview): reserve capacity before opening browser tabs Co-authored-by: Alex Southwell <4596216+saphid@users.noreply.github.com> --- apps/server/src/preview/ServerBrowser.test.ts | 161 ++++++++++++++++++ apps/server/src/preview/ServerBrowser.ts | 107 ++++++++---- 2 files changed, 239 insertions(+), 29 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 0a9118e7b6d8b..d4986a7250787 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -1252,3 +1252,164 @@ it.live( }), ).pipe(Effect.provide(layer)), ); + +it.live.each([ + { budget: "agent", limit: 8 }, + { budget: "server", limit: 32 }, +])("opens and popups reserve the $budget budget before session creation", ({ budget, limit }) => + Effect.scoped( + Effect.gen(function* () { + const { broker, browser, tabId } = yield* ready; + 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; + popup.newCDPSession = async (target) => { + await launchGate.promise; + return newCDPSession(target); + }; + contexts[0]!.page.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. + 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, + ); + + 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(); + contexts[0]!.page.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" }, + }); + } + 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); + const status = yield* broker.invoke({ + scope, + tabId, + operation: "status", + input: {}, + }); + expect(status.tabs).toHaveLength(limit); + }), + ).pipe(Effect.provide(layer)), +); + +it.live("failed session and browser launches release capacity, as does closing a tab", () => + Effect.scoped( + Effect.gen(function* () { + const { broker, tabId } = yield* ready; + const manager = yield* Manager.PreviewManager; + const open = broker.invoke({ + scope, + operation: "open", + input: { reuseExistingTab: false, show: false }, + }); + for (let count = 1; count < 7; count++) yield* open; + const failSession = vi + .spyOn(manager, "open") + .mockImplementationOnce(() => Effect.die(new Error("session creation failed"))); + yield* Effect.addFinalizer(() => Effect.sync(() => failSession.mockRestore())); + expect(yield* open.pipe(Effect.result)).toMatchObject({ _tag: "Failure" }); + failSession.mockRestore(); + const events = yield* manager.subscribeEvents; + contextFailure = new Error("Chromium launch failed"); + expect(yield* open.pipe(Effect.result)).toMatchObject({ _tag: "Failure" }); + let failedSession = yield* PubSub.take(events); + while (failedSession.type !== "opened") failedSession = yield* PubSub.take(events); + // A failed launch must not leave a session a viewer can relaunch past the budget. + expect((yield* manager.list({ threadId: scope.thread.threadId })).sessions).toHaveLength(7); + contextFailure = null; + expect((yield* open).available).toBe(true); + expect(yield* open.pipe(Effect.flip)).toMatchObject({ reason: "tabLimit" }); + expect( + yield* broker + .invoke({ + scope, + tabId: PreviewTabId.make(failedSession.tabId), + operation: "open", + input: { show: false }, + }) + .pipe(Effect.flip), + ).toMatchObject({ _tag: "PreviewAutomationTabNotFoundError" }); + const browser = yield* ServerBrowser.ServerBrowser; + expect( + yield* browser.attachViewer(viewerInput(failedSession.tabId, false)).pipe(Effect.flip), + ).toMatchObject({ _tag: "ServerBrowserTabNotFoundError" }); + yield* broker.invoke({ scope, tabId, operation: "close", input: {} }); + expect((yield* open).available).toBe(true); + const status = yield* broker.invoke({ + scope, + operation: "status", + input: {}, + }); + expect(status.tabs).toHaveLength(8); + }), + ).pipe(Effect.provide(layer)), +); diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 3176d0ad96823..c33fa8bcf1862 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -459,7 +459,10 @@ const make = Effect.gen(function* () { const setupCommand = yield* resolveRootCliCommand(PreviewBrowserHost.SETUP_SUBCOMMAND); const tabs = new Map(); - const pendingTabs = new Map>(); + const pendingTabs = new Map< + string, + { readonly agentId: string | null; promise?: Promise } + >(); const closedPendingTabs = new Set(); const adoptedPages = new Map(); @@ -1172,8 +1175,7 @@ const make = Effect.gen(function* () { }; const adoptPopup = async (opener: ServerTab, popup: Page) => { - const agentId = opener.control.agentId; - if (opener.closing || (agentId !== null && atTabLimit(agentId))) { + if (opener.closing) { await popup.close().catch(constVoid); return; } @@ -1183,7 +1185,7 @@ const make = Effect.gen(function* () { (viewer) => viewer.id === opener.control.controller, ); await Effect.runPromise( - manager.open({ + openTab({ threadId: opener.threadId, ...(/^https?:/i.test(url) ? { url } : {}), runtime: "server", @@ -1198,9 +1200,8 @@ const make = Effect.gen(function* () { }), }), ).then( - (snapshot) => { - if (opener.control.agentId === null) - controller?.push({ _tag: "popup", tabId: snapshot.tabId }); + (tab) => { + if (opener.control.agentId === null) controller?.push({ _tag: "popup", tabId: tab.tabId }); }, async () => { await popup.close().catch(constVoid); @@ -1211,7 +1212,7 @@ const make = Effect.gen(function* () { const ensureTab = (snapshot: PreviewSessionSnapshot): Promise => { const key = tabKey(snapshot.threadId, snapshot.tabId); const pending = pendingTabs.get(key); - if (pending) return pending; + if (pending?.promise) return pending.promise; const existing = tabs.get(key); if (existing) return Promise.resolve(existing); const opening = createTab(snapshot) @@ -1228,7 +1229,7 @@ const make = Effect.gen(function* () { pendingTabs.delete(key); closedPendingTabs.delete(key); }); - pendingTabs.set(key, opening); + pendingTabs.set(key, { agentId: snapshot.automationOwner ?? null, promise: opening }); return opening; }; @@ -1246,10 +1247,15 @@ const make = Effect.gen(function* () { }); }); - const atTabLimit = (agentSessionId: string) => - tabs.size + pendingTabs.size >= SERVER_TAB_LIMIT || - [...tabs.values()].filter((tab) => tab.control.agentId === agentSessionId).length >= - AGENT_TAB_LIMIT; + const atTabLimit = (agentSessionId: string) => { + const opening = [...pendingTabs.entries()].filter(([key]) => !tabs.has(key)); + return ( + tabs.size + opening.length >= SERVER_TAB_LIMIT || + [...tabs.values()].filter((tab) => tab.control.agentId === agentSessionId).length + + opening.filter(([, pending]) => pending.agentId === agentSessionId).length >= + AGENT_TAB_LIMIT + ); + }; const assertTabCapacity = (agentSessionId: string) => { if (atTabLimit(agentSessionId)) @@ -1259,6 +1265,52 @@ const make = Effect.gen(function* () { ); }; + // A Playwright launch must settle before its capacity can be released. + const openTab = Effect.fn("ServerBrowser.openTab")(function* ( + input: Parameters[0], + ) { + let key: string = NodeCrypto.randomUUID(); + const reservation = { agentId: input.automationOwner ?? null }; + return yield* Effect.acquireUseRelease( + Effect.try({ + try: () => { + if (input.automationOwner !== undefined) assertTabCapacity(input.automationOwner); + pendingTabs.set(key, reservation); + }, + catch: ServerBrowserPage.toOperationError, + }), + () => + manager + .open({ + ...input, + beforePublish: (snapshot) => { + pendingTabs.delete(key); + key = tabKey(snapshot.threadId, snapshot.tabId); + pendingTabs.set(key, reservation); + input.beforePublish?.(snapshot); + }, + }) + .pipe( + Effect.flatMap((snapshot) => + Effect.tryPromise({ + try: () => ensureTab(snapshot), + catch: ServerBrowserPage.toOperationError, + }).pipe( + Effect.onError(() => + manager + .close({ threadId: ThreadId.make(snapshot.threadId), tabId: snapshot.tabId }) + .pipe(Effect.ignore), + ), + ), + ), + ), + () => + Effect.sync(() => { + pendingTabs.delete(key); + }), + ); + }, Effect.uninterruptible); + // Desktop browsing bypasses streamed viewers and server input tracking, // so these idle signals only apply to headless tabs. const closeIdleAgentTabs = () => { @@ -1768,23 +1820,18 @@ const make = Effect.gen(function* () { : undefined; const navigationTimeout = Math.min(request.timeoutMs, NAVIGATION_TIMEOUT_MS); const profileId = existing ? undefined : resolveOpenProfile(open.profileId); - if (!existing) { - closeIdleAgentTabs(); - assertTabCapacity(request.agentSessionId); - } + if (!existing) closeIdleAgentTabs(); const tab = existing ?? - (await ensureTab( - await Effect.runPromise( - manager.open({ - threadId: request.threadId, - ...(url ? { url } : {}), - ...(profileId === undefined ? {} : { profileId }), - runtime: "server", - reveal: false, - automationOwner: request.agentSessionId, - }), - ), + (await Effect.runPromise( + openTab({ + threadId: request.threadId, + ...(url ? { url } : {}), + ...(profileId === undefined ? {} : { profileId }), + runtime: "server", + reveal: false, + automationOwner: request.agentSessionId, + }), )); if (existing?.dialog) throw new BrowserControlInterrupted( @@ -2046,7 +2093,9 @@ const make = Effect.gen(function* () { const mirrorManagerEvent = (event: PreviewEvent) => Effect.promise(async () => { if (event.type === "opened" && event.snapshot.runtime === "server") { - await ensureTab(event.snapshot).catch(constVoid); + 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); return; } const key = tabKey(event.threadId, event.tabId);