From 55cccf702a5109b525f5eb5a4ccd6878ebdfe878 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 6 Nov 2026 10:03:42 +0000 Subject: [PATCH 1/8] fix(server): preview_evaluate accepts top-level await and a timeoutMs preview_evaluate rejected `await` outside a function, and always used the broker's 15s default because the tool had no timeoutMs. An expression that compiles as a classic script keeps the single Runtime.evaluate call it had before. Only one that needs top-level await runs in CDP REPL mode, where a promise completion is awaited with Runtime.awaitPromise and the object group is released on settle or timeout. PreviewAutomationEvaluateInput gains the shared OptionalTimeoutMs, and the handler forwards it to the broker as preview_wait_for does. The MCP tool list budget grows by that field. Fixes pingdotgg/t3code#31892 AI-assisted; verified with the tests and tools listed in the PR. --- .../src/mcp/toolkits/preview/handlers.test.ts | 75 ++++++++++++++----- .../src/mcp/toolkits/preview/handlers.ts | 2 +- .../src/preview/ServerBrowserPage.test.ts | 33 ++++++++ apps/server/src/preview/ServerBrowserPage.ts | 55 ++++++++++++-- .../src/provider/agentContextBudget.json | 2 +- packages/contracts/src/previewAutomation.ts | 1 + 6 files changed, 142 insertions(+), 26 deletions(-) diff --git a/apps/server/src/mcp/toolkits/preview/handlers.test.ts b/apps/server/src/mcp/toolkits/preview/handlers.test.ts index 25eebc0c16f8e..48d5429661500 100644 --- a/apps/server/src/mcp/toolkits/preview/handlers.test.ts +++ b/apps/server/src/mcp/toolkits/preview/handlers.test.ts @@ -27,6 +27,27 @@ import * as PreviewAutomationBroker from "../../PreviewAutomationBroker.ts"; import { claimPreviewRecording, layerStandard } from "./handlers.ts"; import { PreviewStandardToolkit } from "./tools.ts"; +const withPreviewThread = (effect: Effect.Effect) => + effect.pipe( + Effect.provideService(McpInvocationContext.McpInvocationContext, { + environmentId: EnvironmentId.make("environment-1"), + requestNamespace: "provider-session-1", + thread: { + threadId: ThreadId.make("thread-1"), + providerSessionId: "provider-session-1", + providerInstanceId: ProviderInstanceId.make("codex"), + }, + client: undefined, + capabilities: new Set(["preview"] as const), + issuedAt: 1, + }), + Effect.provide( + Layer.mergeAll(PreviewAutomationBroker.layer, liveThreadsLayer).pipe( + Layer.provide(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer)), + ), + ), + ); + it.effect("preview_open navigates the opened tab to a dev server in one tool call", () => Effect.scoped( Effect.gen(function* () { @@ -82,25 +103,41 @@ it.effect("preview_open navigates the opened tab to a dev server in one tool cal ]); expect(requests[1]?.tabId).toBe(tabId); expect(requests[1]?.tabIdExplicit).toBe(true); - }).pipe( - Effect.provideService(McpInvocationContext.McpInvocationContext, { - environmentId: EnvironmentId.make("environment-1"), - requestNamespace: "provider-session-1", - thread: { - threadId: ThreadId.make("thread-1"), - providerSessionId: "provider-session-1", - providerInstanceId: ProviderInstanceId.make("codex"), - }, - client: undefined, - capabilities: new Set(["preview"] as const), - issuedAt: 1, - }), - Effect.provide( - Layer.mergeAll(PreviewAutomationBroker.layer, liveThreadsLayer).pipe( - Layer.provide(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer)), - ), - ), - ), + }).pipe(withPreviewThread), + ), +); + +it.effect("preview_evaluate gives the broker the caller's timeout", () => + Effect.scoped( + Effect.gen(function* () { + const scope = yield* McpInvocationContext.McpInvocationContext; + const broker = yield* PreviewAutomationBroker.PreviewAutomationBroker; + const connected = yield* Deferred.make(); + const requests: PreviewAutomationRequest[] = []; + const events = yield* broker.connect({ + clientId: "client-1", + environmentId: scope.environmentId, + }); + yield* Stream.runForEach(events, (event) => { + if (event.type === "connected") return Deferred.succeed(connected, undefined); + requests.push(event.request); + return broker.respond({ + clientId: "client-1", + connectionId: event.connectionId, + requestId: event.request.requestId, + ok: true, + result: "/", + }); + }).pipe(Effect.forkScoped); + yield* Deferred.await(connected); + const toolkit = yield* PreviewStandardToolkit.pipe( + Effect.provide(McpToolAccess.HandlersLayer.layer(layerStandard)), + ); + yield* toolkit + .handle("preview_evaluate", { expression: "location.pathname", timeoutMs: 45_000 }) + .pipe(Stream.unwrap, Stream.runCollect); + expect(requests.find(({ operation }) => operation === "evaluate")?.timeoutMs).toBe(45_000); + }).pipe(withPreviewThread), ), ); diff --git a/apps/server/src/mcp/toolkits/preview/handlers.ts b/apps/server/src/mcp/toolkits/preview/handlers.ts index 94df4a13fec48..b543e88bb1954 100644 --- a/apps/server/src/mcp/toolkits/preview/handlers.ts +++ b/apps/server/src/mcp/toolkits/preview/handlers.ts @@ -239,7 +239,7 @@ const handlers = { ), ), preview_evaluate: McpToolAccess.actsAsCaller(({ tabId, ...input }) => - invoke("evaluate", input, undefined, tabId).pipe( + invoke("evaluate", input, input.timeoutMs, tabId).pipe( Effect.map(({ result, toolIcon }) => ({ value: result ?? null, ...(toolIcon ? { toolIcon } : {}), diff --git a/apps/server/src/preview/ServerBrowserPage.test.ts b/apps/server/src/preview/ServerBrowserPage.test.ts index 2c6d3a01bcf74..6e326aed13e35 100644 --- a/apps/server/src/preview/ServerBrowserPage.test.ts +++ b/apps/server/src/preview/ServerBrowserPage.test.ts @@ -269,6 +269,39 @@ describe("server browser element refs", () => { await ServerBrowserPage.click(page, { selector: 'button[data-example=" >> aria-ref=e1"]' }); expect(await page.locator("button").textContent()).toBe("saved"); }); + + it("evaluates top-level await and returns the last expression's value", async () => { + await page.setContent("Ready"); + const result = await ServerBrowserPage.evaluate( + cdp, + { expression: "await Promise.resolve(); document.title" }, + 5_000, + ); + expect(result).toBe("Ready"); + }); + + it("still awaits a promise the last expression returns", async () => { + await page.setContent("Ready"); + const result = await ServerBrowserPage.evaluate( + cdp, + { expression: "await 0; (async () => document.title)()" }, + 5_000, + ); + expect(result).toBe("Ready"); + }); + + it("returns the result of an expression that reloads the page", async () => { + await page.route("http://preview.test/", (route) => + route.fulfill({ contentType: "text/html", body: "Ready" }), + ); + await page.goto("http://preview.test/"); + const result = await ServerBrowserPage.evaluate( + cdp, + { expression: "location.reload(); ({ clicked: true })" }, + 5_000, + ); + expect(result).toEqual({ clicked: true }); + }); }); describe("server browser snapshot screenshot", () => { diff --git a/apps/server/src/preview/ServerBrowserPage.ts b/apps/server/src/preview/ServerBrowserPage.ts index 622482cd1b86f..ab5b3ea6498ff 100644 --- a/apps/server/src/preview/ServerBrowserPage.ts +++ b/apps/server/src/preview/ServerBrowserPage.ts @@ -22,6 +22,7 @@ import { constVoid } from "effect/Function"; import * as Schema from "effect/Schema"; import type { CDPSession, ConsoleMessage, Frame, Locator, Page } from "playwright-core"; import * as NodeCrypto from "node:crypto"; +import * as NodeVM from "node:vm"; import { BrowserControlInterrupted } from "./SessionControl.ts"; const isBrowserControlInterrupted = Schema.is(BrowserControlInterrupted); @@ -878,17 +879,59 @@ export const scroll = async (page: Page, input: PreviewAutomationScrollInput) => await locator.evaluate((element, [x, y]) => element.scrollBy(x, y), delta); }; +// Parses without running, so a classic script keeps the single evaluate call it always had. +const compilesAsScript = (expression: string) => { + try { + return new NodeVM.Script(expression) instanceof NodeVM.Script; + } catch { + return false; + } +}; + +// The devtools console's mode: top-level await, and the last expression is the result. +// By value it reports a promise result as `{}`, so the result comes back by reference first. +const evaluateRepl = async ( + cdp: CDPSession, + input: PreviewAutomationEvaluateInput, + objectGroup: string, +) => { + const evaluated = await cdp.send("Runtime.evaluate", { + expression: input.expression, + replMode: true, + objectGroup, + }); + const { objectId, subtype } = evaluated.result; + if (evaluated.exceptionDetails || objectId === undefined) return evaluated; + const returnByValue = input.returnByValue ?? true; + if (subtype === "promise" && (input.awaitPromise ?? true)) { + return cdp.send("Runtime.awaitPromise", { promiseObjectId: objectId, returnByValue }); + } + if (!returnByValue) return evaluated; + return cdp.send("Runtime.callFunctionOn", { + objectId, + functionDeclaration: 'function () { "use strict"; return this; }', + returnByValue, + objectGroup, + }); +}; + /** 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 repl = input.expression.includes("await") && !compilesAsScript(input.expression); + // Per call, so a timed-out evaluation settling late cannot release the next one's objects. + const objectGroup = `t3-preview-evaluate-${NodeCrypto.randomUUID()}`; + const release = () => + void cdp.send("Runtime.releaseObjectGroup", { objectGroup }).catch(constVoid); let timer: ReturnType | undefined; const expired = new Promise((_, reject) => { timer = setTimeout(() => { // Stops a busy script; an awaited promise is abandoned and settles unread. void cdp.send("Runtime.terminateExecution").catch(constVoid); + if (repl) release(); reject( new ServerBrowserOperationError({ tag: "PreviewAutomationTimeoutError", @@ -897,11 +940,13 @@ export const evaluate = async ( ); }, timeoutMs); }); - const evaluation = cdp.send("Runtime.evaluate", { - expression: input.expression, - awaitPromise: input.awaitPromise ?? true, - returnByValue: input.returnByValue ?? true, - }); + const evaluation = repl + ? evaluateRepl(cdp, input, objectGroup).finally(release) + : cdp.send("Runtime.evaluate", { + expression: input.expression, + awaitPromise: input.awaitPromise ?? true, + returnByValue: input.returnByValue ?? true, + }); void evaluation.catch(constVoid); const result = await Promise.race([evaluation, expired]).finally(() => clearTimeout(timer)); if (result.exceptionDetails) { diff --git a/apps/server/src/provider/agentContextBudget.json b/apps/server/src/provider/agentContextBudget.json index 0d9f26d44c40a..ce19b1e649d67 100644 --- a/apps/server/src/provider/agentContextBudget.json +++ b/apps/server/src/provider/agentContextBudget.json @@ -1,5 +1,5 @@ { "claudeSystemAppendBytes": 5366, "codexAdditionalContextBytes": 7096, - "mcpToolListJsonBytes": 48119 + "mcpToolListJsonBytes": 48218 } diff --git a/packages/contracts/src/previewAutomation.ts b/packages/contracts/src/previewAutomation.ts index bfeb5cf916672..77d7b41a20fcd 100644 --- a/packages/contracts/src/previewAutomation.ts +++ b/packages/contracts/src/previewAutomation.ts @@ -468,6 +468,7 @@ export const PreviewAutomationEvaluateInput = Schema.Struct({ description: "False returns a ref. Default true.", }), ), + timeoutMs: OptionalTimeoutMs, }); export type PreviewAutomationEvaluateInput = typeof PreviewAutomationEvaluateInput.Type; From 767edae7599575d2d21ddc2de0c7d44863329fbc Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 9 Nov 2026 19:24:11 +0000 Subject: [PATCH 2/8] fix(server): keep preview_evaluate's input unchanged The fixed 15 s limit is named in the timeout error and preview_wait_for takes a timeoutMs for waits on page state, so the new field is dropped. Co-Authored-By: Claude Opus 5.5 --- .../src/mcp/toolkits/preview/handlers.test.ts | 75 +++++-------------- .../src/mcp/toolkits/preview/handlers.ts | 2 +- packages/contracts/src/previewAutomation.ts | 1 - 3 files changed, 20 insertions(+), 58 deletions(-) diff --git a/apps/server/src/mcp/toolkits/preview/handlers.test.ts b/apps/server/src/mcp/toolkits/preview/handlers.test.ts index 48d5429661500..25eebc0c16f8e 100644 --- a/apps/server/src/mcp/toolkits/preview/handlers.test.ts +++ b/apps/server/src/mcp/toolkits/preview/handlers.test.ts @@ -27,27 +27,6 @@ import * as PreviewAutomationBroker from "../../PreviewAutomationBroker.ts"; import { claimPreviewRecording, layerStandard } from "./handlers.ts"; import { PreviewStandardToolkit } from "./tools.ts"; -const withPreviewThread = (effect: Effect.Effect) => - effect.pipe( - Effect.provideService(McpInvocationContext.McpInvocationContext, { - environmentId: EnvironmentId.make("environment-1"), - requestNamespace: "provider-session-1", - thread: { - threadId: ThreadId.make("thread-1"), - providerSessionId: "provider-session-1", - providerInstanceId: ProviderInstanceId.make("codex"), - }, - client: undefined, - capabilities: new Set(["preview"] as const), - issuedAt: 1, - }), - Effect.provide( - Layer.mergeAll(PreviewAutomationBroker.layer, liveThreadsLayer).pipe( - Layer.provide(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer)), - ), - ), - ); - it.effect("preview_open navigates the opened tab to a dev server in one tool call", () => Effect.scoped( Effect.gen(function* () { @@ -103,41 +82,25 @@ it.effect("preview_open navigates the opened tab to a dev server in one tool cal ]); expect(requests[1]?.tabId).toBe(tabId); expect(requests[1]?.tabIdExplicit).toBe(true); - }).pipe(withPreviewThread), - ), -); - -it.effect("preview_evaluate gives the broker the caller's timeout", () => - Effect.scoped( - Effect.gen(function* () { - const scope = yield* McpInvocationContext.McpInvocationContext; - const broker = yield* PreviewAutomationBroker.PreviewAutomationBroker; - const connected = yield* Deferred.make(); - const requests: PreviewAutomationRequest[] = []; - const events = yield* broker.connect({ - clientId: "client-1", - environmentId: scope.environmentId, - }); - yield* Stream.runForEach(events, (event) => { - if (event.type === "connected") return Deferred.succeed(connected, undefined); - requests.push(event.request); - return broker.respond({ - clientId: "client-1", - connectionId: event.connectionId, - requestId: event.request.requestId, - ok: true, - result: "/", - }); - }).pipe(Effect.forkScoped); - yield* Deferred.await(connected); - const toolkit = yield* PreviewStandardToolkit.pipe( - Effect.provide(McpToolAccess.HandlersLayer.layer(layerStandard)), - ); - yield* toolkit - .handle("preview_evaluate", { expression: "location.pathname", timeoutMs: 45_000 }) - .pipe(Stream.unwrap, Stream.runCollect); - expect(requests.find(({ operation }) => operation === "evaluate")?.timeoutMs).toBe(45_000); - }).pipe(withPreviewThread), + }).pipe( + Effect.provideService(McpInvocationContext.McpInvocationContext, { + environmentId: EnvironmentId.make("environment-1"), + requestNamespace: "provider-session-1", + thread: { + threadId: ThreadId.make("thread-1"), + providerSessionId: "provider-session-1", + providerInstanceId: ProviderInstanceId.make("codex"), + }, + client: undefined, + capabilities: new Set(["preview"] as const), + issuedAt: 1, + }), + Effect.provide( + Layer.mergeAll(PreviewAutomationBroker.layer, liveThreadsLayer).pipe( + Layer.provide(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer)), + ), + ), + ), ), ); diff --git a/apps/server/src/mcp/toolkits/preview/handlers.ts b/apps/server/src/mcp/toolkits/preview/handlers.ts index 3478207a632bd..c541ad1664a96 100644 --- a/apps/server/src/mcp/toolkits/preview/handlers.ts +++ b/apps/server/src/mcp/toolkits/preview/handlers.ts @@ -243,7 +243,7 @@ const handlers = { ), ), preview_evaluate: McpToolAccess.actsAsCaller(({ tabId, ...input }) => - invoke("evaluate", input, input.timeoutMs, tabId).pipe( + invoke("evaluate", input, undefined, tabId).pipe( Effect.map(({ result, toolIcon }) => ({ value: result ?? null, ...(toolIcon ? { toolIcon } : {}), diff --git a/packages/contracts/src/previewAutomation.ts b/packages/contracts/src/previewAutomation.ts index a48281a88efc8..8c038f4fe81cc 100644 --- a/packages/contracts/src/previewAutomation.ts +++ b/packages/contracts/src/previewAutomation.ts @@ -468,7 +468,6 @@ export const PreviewAutomationEvaluateInput = Schema.Struct({ description: "False returns a ref. Default true.", }), ), - timeoutMs: OptionalTimeoutMs, }); export type PreviewAutomationEvaluateInput = typeof PreviewAutomationEvaluateInput.Type; From 65c5515ab406d99e4c9979d9a17ae4ef5eb152e8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 9 Nov 2026 19:34:37 +0000 Subject: [PATCH 3/8] fix(server): run top-level await in one replMode evaluate call The second call that read the result by reference failed when the expression navigated, after the script had already run. Co-Authored-By: Claude Opus 5.5 --- .../src/preview/ServerBrowserPage.test.ts | 37 ++++++-------- apps/server/src/preview/ServerBrowserPage.ts | 49 +++---------------- 2 files changed, 23 insertions(+), 63 deletions(-) diff --git a/apps/server/src/preview/ServerBrowserPage.test.ts b/apps/server/src/preview/ServerBrowserPage.test.ts index 18376e99f4f30..3f1610d415239 100644 --- a/apps/server/src/preview/ServerBrowserPage.test.ts +++ b/apps/server/src/preview/ServerBrowserPage.test.ts @@ -280,28 +280,21 @@ describe("server browser element refs", () => { expect(result).toBe("Ready"); }); - it("still awaits a promise the last expression returns", async () => { - await page.setContent("Ready"); - const result = await ServerBrowserPage.evaluate( - cdp, - { expression: "await 0; (async () => document.title)()" }, - 5_000, - ); - expect(result).toBe("Ready"); - }); - - it("returns the result of an expression that reloads the page", async () => { - await page.route("http://preview.test/", (route) => - route.fulfill({ contentType: "text/html", body: "Ready" }), - ); - await page.goto("http://preview.test/"); - const result = await ServerBrowserPage.evaluate( - cdp, - { expression: "location.reload(); ({ clicked: true })" }, - 5_000, - ); - expect(result).toEqual({ clicked: true }); - }); + it.each(["", "await 0; "])( + "returns the result of an expression that reloads the page (prefix %j)", + async (prefix) => { + await page.route("http://preview.test/", (route) => + route.fulfill({ contentType: "text/html", body: "Ready" }), + ); + await page.goto("http://preview.test/"); + const result = await ServerBrowserPage.evaluate( + cdp, + { expression: `${prefix}location.reload(); ({ clicked: true })` }, + 5_000, + ); + expect(result).toEqual({ clicked: true }); + }, + ); }); describe("server browser snapshot screenshot", () => { diff --git a/apps/server/src/preview/ServerBrowserPage.ts b/apps/server/src/preview/ServerBrowserPage.ts index 01f73734641d9..7b0bda7efe644 100644 --- a/apps/server/src/preview/ServerBrowserPage.ts +++ b/apps/server/src/preview/ServerBrowserPage.ts @@ -886,7 +886,6 @@ export const scroll = async (page: Page, input: PreviewAutomationScrollInput) => await locator.evaluate((element, [x, y]) => element.scrollBy(x, y), delta); }; -// Parses without running, so a classic script keeps the single evaluate call it always had. const compilesAsScript = (expression: string) => { try { return new NodeVM.Script(expression) instanceof NodeVM.Script; @@ -895,50 +894,17 @@ const compilesAsScript = (expression: string) => { } }; -// The devtools console's mode: top-level await, and the last expression is the result. -// By value it reports a promise result as `{}`, so the result comes back by reference first. -const evaluateRepl = async ( - cdp: CDPSession, - input: PreviewAutomationEvaluateInput, - objectGroup: string, -) => { - const evaluated = await cdp.send("Runtime.evaluate", { - expression: input.expression, - replMode: true, - objectGroup, - }); - const { objectId, subtype } = evaluated.result; - if (evaluated.exceptionDetails || objectId === undefined) return evaluated; - const returnByValue = input.returnByValue ?? true; - if (subtype === "promise" && (input.awaitPromise ?? true)) { - return cdp.send("Runtime.awaitPromise", { promiseObjectId: objectId, returnByValue }); - } - if (!returnByValue) return evaluated; - return cdp.send("Runtime.callFunctionOn", { - objectId, - functionDeclaration: 'function () { "use strict"; return this; }', - returnByValue, - objectGroup, - }); -}; - /** 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 repl = input.expression.includes("await") && !compilesAsScript(input.expression); - // Per call, so a timed-out evaluation settling late cannot release the next one's objects. - const objectGroup = `t3-preview-evaluate-${NodeCrypto.randomUUID()}`; - const release = () => - void cdp.send("Runtime.releaseObjectGroup", { objectGroup }).catch(constVoid); let timer: ReturnType | undefined; const expired = new Promise((_, reject) => { timer = setTimeout(() => { // Stops a busy script; an awaited promise is abandoned and settles unread. void cdp.send("Runtime.terminateExecution").catch(constVoid); - if (repl) release(); reject( new ServerBrowserOperationError({ tag: "PreviewAutomationTimeoutError", @@ -947,13 +913,14 @@ export const evaluate = async ( ); }, timeoutMs); }); - const evaluation = repl - ? evaluateRepl(cdp, input, objectGroup).finally(release) - : cdp.send("Runtime.evaluate", { - expression: input.expression, - awaitPromise: input.awaitPromise ?? true, - returnByValue: input.returnByValue ?? true, - }); + const evaluation = cdp.send("Runtime.evaluate", { + expression: input.expression, + awaitPromise: input.awaitPromise ?? true, + returnByValue: input.returnByValue ?? true, + // The devtools console's mode, for top-level await. It returns a promise left as the last + // expression unawaited, so expressions that parse as a classic script keep the classic mode. + replMode: input.expression.includes("await") && !compilesAsScript(input.expression), + }); void evaluation.catch(constVoid); const result = await Promise.race([evaluation, expired]).finally(() => clearTimeout(timer)); if (result.exceptionDetails) { From c8ef2ade9b46e6ccd66ccc3ca0da44560a2d2338 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 9 Nov 2026 19:42:22 +0000 Subject: [PATCH 4/8] test(server): pin preview_evaluate's classic mode for async functions An unconditional replMode would return `{}` for an async IIFE; this test fails if the await gate is removed. Co-Authored-By: Claude Opus 5.5 --- .../src/preview/ServerBrowserPage.test.ts | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/apps/server/src/preview/ServerBrowserPage.test.ts b/apps/server/src/preview/ServerBrowserPage.test.ts index 3f1610d415239..0d9cebf70db3b 100644 --- a/apps/server/src/preview/ServerBrowserPage.test.ts +++ b/apps/server/src/preview/ServerBrowserPage.test.ts @@ -280,21 +280,27 @@ describe("server browser element refs", () => { expect(result).toBe("Ready"); }); - it.each(["", "await 0; "])( - "returns the result of an expression that reloads the page (prefix %j)", - async (prefix) => { - await page.route("http://preview.test/", (route) => - route.fulfill({ contentType: "text/html", body: "Ready" }), - ); - await page.goto("http://preview.test/"); - const result = await ServerBrowserPage.evaluate( - cdp, - { expression: `${prefix}location.reload(); ({ clicked: true })` }, - 5_000, - ); - expect(result).toEqual({ clicked: true }); - }, - ); + it("still awaits the promise an async function returns", async () => { + const result = await ServerBrowserPage.evaluate( + cdp, + { expression: "(async () => { await 0; return 1 })()" }, + 5_000, + ); + expect(result).toBe(1); + }); + + it("returns the result of a top-level await expression that reloads the page", async () => { + await page.route("http://preview.test/", (route) => + route.fulfill({ contentType: "text/html", body: "Ready" }), + ); + await page.goto("http://preview.test/"); + const result = await ServerBrowserPage.evaluate( + cdp, + { expression: "await 0; location.reload(); ({ clicked: true })" }, + 5_000, + ); + expect(result).toEqual({ clicked: true }); + }); }); describe("server browser snapshot screenshot", () => { From caa23495a0a28c2d84cf1bd3316ff1fe798e770b Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 9 Nov 2026 23:34:51 +0000 Subject: [PATCH 5/8] fix(server): preview_evaluate's description states its 15 s limit Co-Authored-By: Claude Opus 5.5 --- apps/server/src/mcp/toolkits/preview/tools.ts | 2 +- apps/server/src/provider/agentContextBudget.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/server/src/mcp/toolkits/preview/tools.ts b/apps/server/src/mcp/toolkits/preview/tools.ts index 33c3443a1c26e..26f40eb9fe58e 100644 --- a/apps/server/src/mcp/toolkits/preview/tools.ts +++ b/apps/server/src/mcp/toolkits/preview/tools.ts @@ -182,7 +182,7 @@ export const PreviewEvaluateResult = Schema.Struct({ const PreviewEvaluateTool = browserTool( Tool.make("preview_evaluate", { description: - "Run JavaScript in the page. It can change page state. Returns `{value}`, up to 64 KB. Needs your tab, or an unclaimed one. To read a tab the user drives, use `preview_snapshot`. Page tools reached here: read data only, never create, save, or display. Show results with `html_render`.", + "Run JavaScript in the page. It can change page state. Returns `{value}`, up to 64 KB. Stops after 15 s; for longer waits, use `preview_wait_for`. Needs your tab, or an unclaimed one. To read a tab the user drives, use `preview_snapshot`. Page tools reached here: read data only, never create, save, or display. Show results with `html_render`.", parameters: PreviewAutomationEvaluateInput, success: PreviewEvaluateResult, failure: PreviewToolFailure, diff --git a/apps/server/src/provider/agentContextBudget.json b/apps/server/src/provider/agentContextBudget.json index 0d46d49479a95..1a9f3c9a80ab0 100644 --- a/apps/server/src/provider/agentContextBudget.json +++ b/apps/server/src/provider/agentContextBudget.json @@ -1,5 +1,5 @@ { "claudeSystemAppendBytes": 5549, "codexAdditionalContextBytes": 7279, - "mcpToolListJsonBytes": 48233 + "mcpToolListJsonBytes": 48293 } From 42cfe75c9d5fbe0063cb4d3c9c31f6f3f0f98eb3 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 9 Nov 2026 23:45:05 +0000 Subject: [PATCH 6/8] revert(server): preview_evaluate's description stays unchanged Stating the limit grows the agent tool list, which needs evals and the agent-context-increase label; the timeout error already names the limit. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/mcp/toolkits/preview/tools.ts | 2 +- apps/server/src/provider/agentContextBudget.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/server/src/mcp/toolkits/preview/tools.ts b/apps/server/src/mcp/toolkits/preview/tools.ts index 26f40eb9fe58e..33c3443a1c26e 100644 --- a/apps/server/src/mcp/toolkits/preview/tools.ts +++ b/apps/server/src/mcp/toolkits/preview/tools.ts @@ -182,7 +182,7 @@ export const PreviewEvaluateResult = Schema.Struct({ const PreviewEvaluateTool = browserTool( Tool.make("preview_evaluate", { description: - "Run JavaScript in the page. It can change page state. Returns `{value}`, up to 64 KB. Stops after 15 s; for longer waits, use `preview_wait_for`. Needs your tab, or an unclaimed one. To read a tab the user drives, use `preview_snapshot`. Page tools reached here: read data only, never create, save, or display. Show results with `html_render`.", + "Run JavaScript in the page. It can change page state. Returns `{value}`, up to 64 KB. Needs your tab, or an unclaimed one. To read a tab the user drives, use `preview_snapshot`. Page tools reached here: read data only, never create, save, or display. Show results with `html_render`.", parameters: PreviewAutomationEvaluateInput, success: PreviewEvaluateResult, failure: PreviewToolFailure, diff --git a/apps/server/src/provider/agentContextBudget.json b/apps/server/src/provider/agentContextBudget.json index 1a9f3c9a80ab0..0d46d49479a95 100644 --- a/apps/server/src/provider/agentContextBudget.json +++ b/apps/server/src/provider/agentContextBudget.json @@ -1,5 +1,5 @@ { "claudeSystemAppendBytes": 5549, "codexAdditionalContextBytes": 7279, - "mcpToolListJsonBytes": 48293 + "mcpToolListJsonBytes": 48233 } From 91525fa36a57a9a9d2ea7ea0ae74eaaf47d87db0 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 9 Nov 2026 23:55:29 +0000 Subject: [PATCH 7/8] fix(server): preview_evaluate lets the page's parser pick replMode The server decided replMode with its own Node parser, which can be older than the page's Chromium. On Node 22 a valid browser expression the host cannot parse got replMode and returned its trailing promise unawaited. Run the classic Runtime.evaluate first, and retry with replMode only when the page rejected the expression with a parse-time SyntaxError (no stack trace, so nothing ran) and the expression mentions await. Both calls share the existing timeout. Co-Authored-By: Claude Opus 5.5 --- .../src/preview/ServerBrowserPage.test.ts | 10 ++++++++ apps/server/src/preview/ServerBrowserPage.ts | 24 +++++++++---------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/apps/server/src/preview/ServerBrowserPage.test.ts b/apps/server/src/preview/ServerBrowserPage.test.ts index 0d9cebf70db3b..4b7e8af0310b5 100644 --- a/apps/server/src/preview/ServerBrowserPage.test.ts +++ b/apps/server/src/preview/ServerBrowserPage.test.ts @@ -289,6 +289,16 @@ describe("server browser element refs", () => { expect(result).toBe(1); }); + it("does not rerun an expression whose SyntaxError is thrown at runtime", async () => { + const evaluation = ServerBrowserPage.evaluate( + cdp, + { expression: "window.runs = (window.runs ?? 0) + 1; new Function('await 0')" }, + 5_000, + ); + await expect(evaluation).rejects.toThrow("await is only valid"); + expect(await page.evaluate("window.runs")).toBe(1); + }); + it("returns the result of a top-level await expression that reloads the page", async () => { await page.route("http://preview.test/", (route) => route.fulfill({ contentType: "text/html", body: "Ready" }), diff --git a/apps/server/src/preview/ServerBrowserPage.ts b/apps/server/src/preview/ServerBrowserPage.ts index 7b0bda7efe644..9d40ab63d5953 100644 --- a/apps/server/src/preview/ServerBrowserPage.ts +++ b/apps/server/src/preview/ServerBrowserPage.ts @@ -22,7 +22,6 @@ import { constVoid } from "effect/Function"; import * as Schema from "effect/Schema"; import type { CDPSession, ConsoleMessage, Frame, Locator, Page } from "playwright-core"; import * as NodeCrypto from "node:crypto"; -import * as NodeVM from "node:vm"; import { BrowserControlInterrupted } from "./SessionControl.ts"; const isBrowserControlInterrupted = Schema.is(BrowserControlInterrupted); @@ -886,14 +885,6 @@ export const scroll = async (page: Page, input: PreviewAutomationScrollInput) => await locator.evaluate((element, [x, y]) => element.scrollBy(x, y), delta); }; -const compilesAsScript = (expression: string) => { - try { - return new NodeVM.Script(expression) instanceof NodeVM.Script; - } catch { - return false; - } -}; - /** Terminate expired page scripts so a hung expression cannot block the tab's control queue. */ export const evaluate = async ( cdp: CDPSession, @@ -913,13 +904,20 @@ export const evaluate = async ( ); }, timeoutMs); }); - const evaluation = cdp.send("Runtime.evaluate", { + const params = { expression: input.expression, awaitPromise: input.awaitPromise ?? true, returnByValue: input.returnByValue ?? true, - // The devtools console's mode, for top-level await. It returns a promise left as the last - // expression unawaited, so expressions that parse as a classic script keep the classic mode. - replMode: input.expression.includes("await") && !compilesAsScript(input.expression), + }; + // The devtools console's replMode allows top-level await but returns a trailing promise unawaited, + // so it only retries what the page's parser rejected. A parse error has no stack and ran nothing. + const evaluation = cdp.send("Runtime.evaluate", params).then((classic) => { + const failure = classic.exceptionDetails; + const unparsed = + failure?.exception?.className === "SyntaxError" && failure.stackTrace === undefined; + return unparsed && input.expression.includes("await") + ? cdp.send("Runtime.evaluate", { ...params, replMode: true }) + : classic; }); void evaluation.catch(constVoid); const result = await Promise.race([evaluation, expired]).finally(() => clearTimeout(timer)); From a04186966665a74e547cad9f4cb64c85e2009cbc Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 10 Nov 2026 00:40:11 +0000 Subject: [PATCH 8/8] fix(server): preview_evaluate retries only parse errors, never after timeout An awaited promise rejected by browser code, such as Response.json() on an HTML body, also reports a stackless SyntaxError, so the expression ran a second time in replMode. A real parse error carries a scriptId; the browser-internal rejection does not, so the retry now requires one. If the classic evaluation answered after the timeout had already terminated it, the replMode retry still went out and ran unbounded. The timer now marks the evaluation expired and the retry is skipped. --- .../src/preview/ServerBrowserPage.test.ts | 64 ++++++++++++++++++- apps/server/src/preview/ServerBrowserPage.ts | 11 +++- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/apps/server/src/preview/ServerBrowserPage.test.ts b/apps/server/src/preview/ServerBrowserPage.test.ts index 4b7e8af0310b5..78f03f1f91b72 100644 --- a/apps/server/src/preview/ServerBrowserPage.test.ts +++ b/apps/server/src/preview/ServerBrowserPage.test.ts @@ -5,7 +5,16 @@ import { type CDPSession, type Page, } from "playwright-core"; -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "vite-plus/test"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vite-plus/test"; import * as ServerBrowserPage from "./ServerBrowserPage.ts"; @@ -299,6 +308,25 @@ describe("server browser element refs", () => { expect(await page.evaluate("window.runs")).toBe(1); }); + it("does not rerun an expression whose awaited browser call rejects with a SyntaxError", async () => { + let posts = 0; + await page.route("http://preview.test/**", (route) => { + if (route.request().method() === "POST") posts++; + return route.fulfill({ contentType: "text/html", body: "Ready" }); + }); + await page.goto("http://preview.test/"); + const evaluation = ServerBrowserPage.evaluate( + cdp, + { + expression: + "(async () => { const r = await fetch('/api', { method: 'POST' }); return r.json() })()", + }, + 5_000, + ); + await expect(evaluation).rejects.toThrow("is not valid JSON"); + expect(posts).toBe(1); + }); + it("returns the result of a top-level await expression that reloads the page", async () => { await page.route("http://preview.test/", (route) => route.fulfill({ contentType: "text/html", body: "Ready" }), @@ -459,6 +487,40 @@ describe("server browser drag", () => { }); }); +describe("server browser evaluate", () => { + it("does not retry a parse error that arrives after the timeout", async () => { + vi.useFakeTimers(); + try { + const classic = Promise.withResolvers(); + const send = vi.fn((method: string) => + method === "Runtime.evaluate" ? classic.promise : Promise.resolve({}), + ); + const cdp = { send } as unknown as CDPSession; + const evaluation = ServerBrowserPage.evaluate(cdp, { expression: "await 0; 1" }, 10); + const rejected = expect(evaluation).rejects.toMatchObject({ + tag: "PreviewAutomationTimeoutError", + }); + await vi.advanceTimersByTimeAsync(10); + await rejected; + classic.resolve({ + result: { type: "object" }, + exceptionDetails: { + text: "Uncaught", + scriptId: "4", + exception: { type: "object", className: "SyntaxError" }, + }, + }); + await classic.promise; + expect(send.mock.calls.map(([method]) => method)).toEqual([ + "Runtime.evaluate", + "Runtime.terminateExecution", + ]); + } finally { + vi.useRealTimers(); + } + }); +}); + describe("server browser net errors", () => { it.each([ ["net::ERR_CONNECTION_REFUSED", "ERR_CONNECTION_REFUSED", -102], diff --git a/apps/server/src/preview/ServerBrowserPage.ts b/apps/server/src/preview/ServerBrowserPage.ts index 9d40ab63d5953..31cff821c6e42 100644 --- a/apps/server/src/preview/ServerBrowserPage.ts +++ b/apps/server/src/preview/ServerBrowserPage.ts @@ -892,8 +892,10 @@ export const evaluate = async ( timeoutMs: number, ) => { let timer: ReturnType | undefined; + let timedOut = false; const expired = new Promise((_, reject) => { timer = setTimeout(() => { + timedOut = true; // Stops a busy script; an awaited promise is abandoned and settles unread. void cdp.send("Runtime.terminateExecution").catch(constVoid); reject( @@ -910,12 +912,15 @@ export const evaluate = async ( returnByValue: input.returnByValue ?? true, }; // The devtools console's replMode allows top-level await but returns a trailing promise unawaited, - // so it only retries what the page's parser rejected. A parse error has no stack and ran nothing. + // so it only retries what the page's parser rejected. A parse error has a scriptId but no stack and + // ran nothing; a browser-internal rejection has neither. const evaluation = cdp.send("Runtime.evaluate", params).then((classic) => { const failure = classic.exceptionDetails; const unparsed = - failure?.exception?.className === "SyntaxError" && failure.stackTrace === undefined; - return unparsed && input.expression.includes("await") + failure?.exception?.className === "SyntaxError" && + failure.scriptId !== undefined && + failure.stackTrace === undefined; + return unparsed && !timedOut && input.expression.includes("await") ? cdp.send("Runtime.evaluate", { ...params, replMode: true }) : classic; });