From c199da4d367356f5d983e7a0713f2539593b8211 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 31 Oct 2026 13:10:46 +0000 Subject: [PATCH 1/6] test(server): reproduce #28141, navigate cannot leave a page whose script never yields An agent's preview_navigate on a tab whose page script never yields does not land on the new page: with readiness "load" it times out, and with readiness "none" the navigation never commits. The fake renderer models a busy main thread that Chromium stops on Runtime.terminateExecution. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index f4671d89b5634..15861fe4e4baa 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -87,9 +87,20 @@ const PNG_HEADER = (() => { return header.toString("base64"); })(); +function makeRenderer() { + return { + /** A page script that never yields; Chromium stops it on `Runtime.terminateExecution`. */ + busyScript: null as PromiseWithResolvers | null, + gotoStarted: Promise.withResolvers(), + /** Rejecting these stands in for Playwright's goto timeout while the script runs. */ + stalledGotos: [] as Array<(cause: Error) => void>, + }; +} + function makeSession( frame: ReturnType | undefined, viewport: { width: number; height: number }, + renderer?: ReturnType, ) { const events = new NodeEvents.EventEmitter(); return { @@ -99,6 +110,17 @@ function makeSession( Promise.all(events.listeners(name).map((listener) => listener(...args))), detach: vi.fn(async () => {}), send: vi.fn(async (method: string, input?: unknown): Promise> => { + const busyScript = renderer?.busyScript; + if (method === "Runtime.terminateExecution" && busyScript) { + renderer.busyScript = null; + busyScript.resolve(); + return {}; + } + if (method === "Runtime.evaluate" && busyScript) { + return busyScript.promise.then(() => { + throw new Error("Execution was terminated"); + }); + } if (method === "Page.getNavigationHistory") return { currentIndex: 0, entries: [{}] }; if (method === "Page.getLayoutMetrics") return { cssVisualViewport: { pageX: 0, pageY: 0 } }; if (method === "Page.captureScreenshot") { @@ -156,6 +178,7 @@ function makeContext(onClose?: (context: BrowserContext) => void) { const nativeFileChooser = vi.fn(); const setChooserFiles = vi.fn(async (_files: unknown) => {}); const sessions: ReturnType[] = []; + const renderer = makeRenderer(); let url = "about:blank"; let viewport: ReturnType = { width: 1280, height: 800 }; const cssViewport = { width: 1920, height: 1080 }; @@ -193,6 +216,14 @@ function makeContext(onClose?: (context: BrowserContext) => void) { viewport = size; }), goto: vi.fn(async (next: string) => { + const busyScript = renderer.busyScript; + renderer.gotoStarted.resolve(); + if (busyScript) { + await new Promise((resolve, reject) => { + renderer.stalledGotos.push(reject); + void busyScript.promise.then(resolve); + }); + } url = next; events.emit("load"); const pending = initialNavigation; @@ -226,6 +257,7 @@ function makeContext(onClose?: (context: BrowserContext) => void) { const context = { page, sessions, + renderer, cssViewport, newPage: async () => page as unknown as Page, grantPermissions: vi.fn(async () => {}), @@ -233,6 +265,7 @@ function makeContext(onClose?: (context: BrowserContext) => void) { const session = makeSession( target === page ? undefined : (target as ReturnType), cssViewport, + renderer, ); sessions.push(session); return session; @@ -642,6 +675,61 @@ it.live.each([ ).pipe(Effect.provide(layer)), ); +it.live.each(["load", "none"] as const)( + "an agent navigates away from a page whose script never yields (readiness $0)", + (readiness) => + Effect.scoped( + Effect.gen(function* () { + const { broker, tabId } = yield* ready; + yield* broker.invoke({ + scope, + tabId, + operation: "navigate", + input: { url: "http://localhost:5173/busy" }, + }); + const { renderer } = contexts[0]!; + renderer.busyScript = Promise.withResolvers(); + renderer.gotoStarted = Promise.withResolvers(); + + const navigation = yield* broker + .invoke({ + scope, + tabId, + operation: "navigate", + input: { url: "http://localhost:5173/next", readiness, timeoutMs: 5_000 }, + }) + .pipe(Effect.exit, Effect.forkScoped); + yield* Effect.promise(() => renderer.gotoStarted.promise); + for (const timeOut of renderer.stalledGotos.splice(0)) { + timeOut( + Object.assign(new Error("page.goto: Timeout 5000ms exceeded."), { + name: "TimeoutError", + }), + ); + } + const navigated = yield* Fiber.join(navigation); + const waited = yield* broker + .invoke({ + scope, + tabId, + operation: "waitFor", + input: { urlIncludes: "/next", timeoutMs: 100 }, + }) + .pipe(Effect.exit); + const status = yield* broker.invoke({ + scope, + tabId, + operation: "status", + input: {}, + }); + + expect(status.url).toBe("http://localhost:5173/next"); + expect(navigated).toMatchObject({ _tag: "Success" }); + expect(waited).toMatchObject({ _tag: "Success" }); + }), + ).pipe(Effect.provide(layer)), +); + it.live("enforces provider ownership and explicit targets when a session has multiple tabs", () => Effect.scoped( Effect.gen(function* () { From 38b5a44a33297fe64de86c3e4fe4d78524378384 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 31 Oct 2026 13:16:55 +0000 Subject: [PATCH 2/6] fix(server): preview navigation leaves a page whose script never yields A same-site navigation shares the renderer, so a page script that never yields kept the old document: navigate timed out with readiness "load" and never committed with "none". Before an agent navigate or a viewer address bar, back/forward, reload or hard reload, probe the page with a trivial Runtime.evaluate and send Runtime.terminateExecution only if it does not answer within 1s. Terminating unconditionally would hit the next script on an idle renderer, possibly the new page's first one. The probe shares evaluate's race-and-terminate, and spends the navigate's own timeout. Fixes pingdotgg/t3code#28141 Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 6 +- apps/server/src/preview/ServerBrowser.ts | 31 ++++++++--- apps/server/src/preview/ServerBrowserPage.ts | 55 +++++++++++++------ 3 files changed, 68 insertions(+), 24 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 15861fe4e4baa..9e8ffba4690b6 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -687,7 +687,11 @@ it.live.each(["load", "none"] as const)( operation: "navigate", input: { url: "http://localhost:5173/busy" }, }); - const { renderer } = contexts[0]!; + const { renderer, sessions } = contexts[0]!; + // On an idle renderer V8 would terminate the next script, which may be the new page's. + expect( + sessions.flatMap((session) => session.send.mock.calls.map(([method]) => method)), + ).not.toContain("Runtime.terminateExecution"); renderer.busyScript = Promise.withResolvers(); renderer.gotoStarted = Promise.withResolvers(); diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 84299d3e9246a..1358122adc2cb 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -1619,8 +1619,11 @@ const make = Effect.gen(function* () { readiness: "load" | "domContentLoaded" | "none", timeout: number, ) => { + const probeStartedAt = performance.now(); + await ServerBrowserPage.stopUnresponsiveScript(tab.cdp); const navigation = tab.page.goto(url, { - timeout, + // The probe spends the caller's budget; the broker evicts a host that answers late. + timeout: Math.max(1, timeout - (performance.now() - probeStartedAt)), waitUntil: readiness === "domContentLoaded" ? "domcontentloaded" @@ -2269,7 +2272,10 @@ const make = Effect.gen(function* () { 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 }); + if (request?.hardReload) { + await ServerBrowserPage.stopUnresponsiveScript(tab.cdp); + await tab.cdp.send("Page.reload", { ignoreCache: true }); + } }) .catch((cause: unknown) => runFork(Effect.logWarning("server preview could not apply a tab setting", { cause })), @@ -2403,22 +2409,33 @@ const make = Effect.gen(function* () { case "navigate": if (typeof message.url === "string") { const url = normalizePreviewUrl(message.url); - tab.control.trackUntilHandoff(tab.page.goto(url, VIEWER_NAVIGATION_OPTIONS)); + tab.control.trackUntilHandoff( + ServerBrowserPage.stopUnresponsiveScript(tab.cdp).then(() => + tab.page.goto(url, VIEWER_NAVIGATION_OPTIONS), + ), + ); } return; case "history": tab.control.trackUntilHandoff( - num(message.delta) < 0 - ? tab.page.goBack(VIEWER_NAVIGATION_OPTIONS) - : tab.page.goForward(VIEWER_NAVIGATION_OPTIONS), + ServerBrowserPage.stopUnresponsiveScript(tab.cdp).then(() => + num(message.delta) < 0 + ? tab.page.goBack(VIEWER_NAVIGATION_OPTIONS) + : tab.page.goForward(VIEWER_NAVIGATION_OPTIONS), + ), ); return; case "reload": if (message.ignoreCache === true) { + await ServerBrowserPage.stopUnresponsiveScript(tab.cdp); await session.send("Page.reload", { ignoreCache: true }); return; } - tab.control.trackUntilHandoff(tab.page.reload(VIEWER_NAVIGATION_OPTIONS)); + tab.control.trackUntilHandoff( + ServerBrowserPage.stopUnresponsiveScript(tab.cdp).then(() => + tab.page.reload(VIEWER_NAVIGATION_OPTIONS), + ), + ); return; case "probe": { const x = num(message.x); diff --git a/apps/server/src/preview/ServerBrowserPage.ts b/apps/server/src/preview/ServerBrowserPage.ts index 88308ed691eed..b44c4f04eb270 100644 --- a/apps/server/src/preview/ServerBrowserPage.ts +++ b/apps/server/src/preview/ServerBrowserPage.ts @@ -206,6 +206,8 @@ const withoutTreeDataPayloads = (tree: string) => // The broker waits 15s for the host; a snapshot, retry included, answers well before. const SNAPSHOT_TIMEOUT_MS = 10_000; const FRAME_ANSWER_MS = 1_000; +// Far above any frame or ordinary long task, so only a page that is already hung pays it. +const SCRIPT_PROBE_MS = 1_000; // Include password and CSS-masked text in open shadow roots, normalized like accessible text. const DOTTED_SCRIPT = `(() => { @@ -805,32 +807,46 @@ export const scroll = async (page: Page, input: PreviewAutomationScrollInput) => await locator.evaluate((element, [x, y]) => element.scrollBy(x, y), delta); }; -/** Terminate expired page scripts so a hung expression cannot block the tab's control queue. */ -export const evaluate = async ( +/** Resolves `null` and terminates the page's script when it has not answered within `timeoutMs`. */ +const evaluateOrTerminate = async ( cdp: CDPSession, - input: PreviewAutomationEvaluateInput, + params: { expression: string; awaitPromise?: boolean; returnByValue?: boolean }, timeoutMs: number, ) => { let timer: ReturnType | undefined; - const expired = new Promise((_, reject) => { + const expired = new Promise((resolve) => { timer = setTimeout(() => { // Stops a busy script; an awaited promise is abandoned and settles unread. void cdp.send("Runtime.terminateExecution").catch(constVoid); - reject( - new ServerBrowserOperationError( - "PreviewAutomationTimeoutError", - `Evaluation did not finish within ${timeoutMs}ms and was stopped.`, - ), - ); + resolve(null); }, timeoutMs); }); - const evaluation = cdp.send("Runtime.evaluate", { - expression: input.expression, - awaitPromise: input.awaitPromise ?? true, - returnByValue: input.returnByValue ?? true, - }); + const evaluation = cdp.send("Runtime.evaluate", params); void evaluation.catch(constVoid); - const result = await Promise.race([evaluation, expired]).finally(() => clearTimeout(timer)); + return Promise.race([evaluation, expired]).finally(() => clearTimeout(timer)); +}; + +/** Terminate expired page scripts so a hung expression cannot block the tab's control queue. */ +export const evaluate = async ( + cdp: CDPSession, + input: PreviewAutomationEvaluateInput, + timeoutMs: number, +) => { + const result = await evaluateOrTerminate( + cdp, + { + expression: input.expression, + awaitPromise: input.awaitPromise ?? true, + returnByValue: input.returnByValue ?? true, + }, + timeoutMs, + ); + if (result === null) { + throw new ServerBrowserOperationError( + "PreviewAutomationTimeoutError", + `Evaluation did not finish within ${timeoutMs}ms and was stopped.`, + ); + } if (result.exceptionDetails) { throw new ServerBrowserOperationError( "PreviewAutomationExecutionError", @@ -850,6 +866,13 @@ export const evaluate = async ( return value; }; +/** + * A same-site navigation cannot commit while the page's script never yields. Terminate only after a + * probe goes unanswered: on an idle renderer V8 terminates the next script, possibly the new page's. + */ +export const stopUnresponsiveScript = (cdp: CDPSession) => + evaluateOrTerminate(cdp, { expression: "0" }, SCRIPT_PROBE_MS).then(constVoid, constVoid); + export const waitFor = async (page: Page, input: PreviewAutomationWaitForInput) => { const timeoutMs = input.timeoutMs ?? DEFAULT_TIMEOUT_MS; const deadline = Date.now() + timeoutMs; From 01257475e32dbab75717665a6606c433657ae3ac Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 31 Oct 2026 13:26:39 +0000 Subject: [PATCH 3/6] fix(server): bound the navigation probe by its timeout and cover viewer navigation The busy-script probe now waits at most the navigation's own timeout, so a short timeoutMs is not delayed by the probe's 1s bound. Viewer navigate, back, forward and reload are covered by a test on a page whose script never yields. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 24 +++++++++++++++++++ apps/server/src/preview/ServerBrowser.ts | 2 +- apps/server/src/preview/ServerBrowserPage.ts | 7 ++++-- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 9e8ffba4690b6..534af45cb6e39 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -644,6 +644,30 @@ it.live.each([ ).pipe(Effect.provide(layer)), ); +it.live.each([ + { method: "goto" as const, message: { type: "navigate", url: "http://localhost:5173/next" } }, + { method: "goBack" as const, message: { type: "history", delta: -1 } }, + { method: "goForward" as const, message: { type: "history", delta: 1 } }, + { method: "reload" as const, message: { type: "reload" } }, +])("a viewer $method leaves a page whose script never yields", ({ method, message }) => + Effect.scoped( + Effect.gen(function* () { + const { browser, tabId } = yield* ready; + const viewer = yield* browser.attachViewer(viewerInput(tabId, true)); + yield* viewer.input({ type: "takeControl" }); + const { page, renderer } = contexts[0]!; + renderer.busyScript = Promise.withResolvers(); + // A same-site navigation cannot commit until the page's script stops. + const scriptStoppedFirst = Promise.withResolvers(); + page[method].mockImplementationOnce(async () => { + scriptStoppedFirst.resolve(renderer.busyScript === null); + }); + yield* viewer.input(message); + expect(yield* Effect.promise(() => scriptStoppedFirst.promise)).toBe(true); + }), + ).pipe(Effect.provide(layer)), +); + // The fake encoder decodes every frame at 1280x800, so 30 fps hits the bitrate floor. it.live.each([ { reports: [], frameRate: 30, bitsPerSecond: 2.5e6 }, diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 1358122adc2cb..b3a75dd7541bf 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -1620,7 +1620,7 @@ const make = Effect.gen(function* () { timeout: number, ) => { const probeStartedAt = performance.now(); - await ServerBrowserPage.stopUnresponsiveScript(tab.cdp); + await ServerBrowserPage.stopUnresponsiveScript(tab.cdp, timeout); const navigation = tab.page.goto(url, { // The probe spends the caller's budget; the broker evicts a host that answers late. timeout: Math.max(1, timeout - (performance.now() - probeStartedAt)), diff --git a/apps/server/src/preview/ServerBrowserPage.ts b/apps/server/src/preview/ServerBrowserPage.ts index b44c4f04eb270..a432b4a4965bd 100644 --- a/apps/server/src/preview/ServerBrowserPage.ts +++ b/apps/server/src/preview/ServerBrowserPage.ts @@ -870,8 +870,11 @@ export const evaluate = async ( * A same-site navigation cannot commit while the page's script never yields. Terminate only after a * probe goes unanswered: on an idle renderer V8 terminates the next script, possibly the new page's. */ -export const stopUnresponsiveScript = (cdp: CDPSession) => - evaluateOrTerminate(cdp, { expression: "0" }, SCRIPT_PROBE_MS).then(constVoid, constVoid); +export const stopUnresponsiveScript = (cdp: CDPSession, budgetMs = SCRIPT_PROBE_MS) => + evaluateOrTerminate(cdp, { expression: "0" }, Math.min(SCRIPT_PROBE_MS, budgetMs)).then( + constVoid, + constVoid, + ); export const waitFor = async (page: Page, input: PreviewAutomationWaitForInput) => { const timeoutMs = input.timeoutMs ?? DEFAULT_TIMEOUT_MS; From 46467f2104401b87beb0819d139736fe67422513 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 31 Oct 2026 13:36:32 +0000 Subject: [PATCH 4/6] test(server): a busy-script probe spends the navigation's own budget Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 534af45cb6e39..2190c26854aa2 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -724,13 +724,13 @@ it.live.each(["load", "none"] as const)( scope, tabId, operation: "navigate", - input: { url: "http://localhost:5173/next", readiness, timeoutMs: 5_000 }, + input: { url: "http://localhost:5173/next", readiness, timeoutMs: 200 }, }) .pipe(Effect.exit, Effect.forkScoped); yield* Effect.promise(() => renderer.gotoStarted.promise); for (const timeOut of renderer.stalledGotos.splice(0)) { timeOut( - Object.assign(new Error("page.goto: Timeout 5000ms exceeded."), { + Object.assign(new Error("page.goto: Timeout 200ms exceeded."), { name: "TimeoutError", }), ); @@ -753,6 +753,12 @@ it.live.each(["load", "none"] as const)( expect(status.url).toBe("http://localhost:5173/next"); expect(navigated).toMatchObject({ _tag: "Success" }); + // Waiting on the busy script spends the navigation's own budget. + const [, gotoOptions] = contexts[0]!.page.goto.mock.calls.at(-1)! as [ + string, + { timeout: number }, + ]; + expect(gotoOptions.timeout).toBeLessThan(200); expect(waited).toMatchObject({ _tag: "Success" }); }), ).pipe(Effect.provide(layer)), From f054e852064cc5ca9a1248ee244f4f0970ff35fd Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 31 Oct 2026 14:00:12 +0000 Subject: [PATCH 5/6] test(server): type the fake goto's options Co-Authored-By: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.test.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index 2190c26854aa2..62181303efa50 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -215,7 +215,7 @@ function makeContext(onClose?: (context: BrowserContext) => void) { setViewportSize: vi.fn(async (size: typeof viewport) => { viewport = size; }), - goto: vi.fn(async (next: string) => { + goto: vi.fn(async (next: string, _options?: { timeout?: number }) => { const busyScript = renderer.busyScript; renderer.gotoStarted.resolve(); if (busyScript) { @@ -754,11 +754,8 @@ it.live.each(["load", "none"] as const)( expect(status.url).toBe("http://localhost:5173/next"); expect(navigated).toMatchObject({ _tag: "Success" }); // Waiting on the busy script spends the navigation's own budget. - const [, gotoOptions] = contexts[0]!.page.goto.mock.calls.at(-1)! as [ - string, - { timeout: number }, - ]; - expect(gotoOptions.timeout).toBeLessThan(200); + const [, gotoOptions] = contexts[0]!.page.goto.mock.calls.at(-1)!; + expect(gotoOptions?.timeout).toBeLessThan(200); expect(waited).toMatchObject({ _tag: "Success" }); }), ).pipe(Effect.provide(layer)), From 56f4f77295b205f5228c13050a61b32ffa3603e2 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 10 Nov 2026 23:47:54 +0000 Subject: [PATCH 6/6] style(server): format the navigation script probe AI-assisted; verified with the tests and tools listed above. --- apps/server/src/preview/ServerBrowserPage.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/server/src/preview/ServerBrowserPage.ts b/apps/server/src/preview/ServerBrowserPage.ts index a9bb3aa1841a3..9e7f1713162cf 100644 --- a/apps/server/src/preview/ServerBrowserPage.ts +++ b/apps/server/src/preview/ServerBrowserPage.ts @@ -954,10 +954,13 @@ export const evaluate = async ( export const stopUnresponsiveScript = async (cdp: CDPSession, budgetMs = SCRIPT_PROBE_MS) => { let timer: ReturnType | undefined; const expired = new Promise((resolve) => { - timer = setTimeout(() => { - void cdp.send("Runtime.terminateExecution").catch(constVoid); - resolve(); - }, Math.min(SCRIPT_PROBE_MS, budgetMs)); + timer = setTimeout( + () => { + void cdp.send("Runtime.terminateExecution").catch(constVoid); + resolve(); + }, + Math.min(SCRIPT_PROBE_MS, budgetMs), + ); }); const probe = cdp.send("Runtime.evaluate", { expression: "0" }).then(constVoid, constVoid); await Promise.race([probe, expired]).finally(() => clearTimeout(timer));