From 05bae6dc1c65e43473e847d1cb94260485da4a84 Mon Sep 17 00:00:00 2001 From: macodev00 <273427913+macodev00@users.noreply.github.com> Date: Thu, 1 Oct 2026 06:47:31 +0000 Subject: [PATCH] fix(preview): keep the desktop host when page metadata times out The optional 500ms status read disconnected the only preview host, so the next Codex preview tool reported that no browser was available. --- apps/server/src/mcp/McpHttpServer.test.ts | 66 +++++++++++++++++++ .../src/mcp/PreviewAutomationBroker.test.ts | 47 +++++++++++++ .../server/src/mcp/PreviewAutomationBroker.ts | 16 ++++- .../src/mcp/toolkits/preview/handlers.ts | 4 ++ 4 files changed, 130 insertions(+), 3 deletions(-) diff --git a/apps/server/src/mcp/McpHttpServer.test.ts b/apps/server/src/mcp/McpHttpServer.test.ts index 5619f19b2549..dc386f85d8bc 100644 --- a/apps/server/src/mcp/McpHttpServer.test.ts +++ b/apps/server/src/mcp/McpHttpServer.test.ts @@ -4,8 +4,10 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { EnvironmentId, PreviewTabId, ProviderInstanceId, ThreadId } from "@t3tools/contracts"; import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; +import * as TestClock from "effect/testing/TestClock"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; @@ -213,6 +215,70 @@ it.effect.each([ ).pipe(Effect.provide(TestLayer)), ); +it.effect("keeps the preview host when optional page metadata times out", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* McpServer.McpServer; + const broker = yield* PreviewAutomationBroker.PreviewAutomationBroker; + const connected = yield* Deferred.make(); + const metadataSeen = yield* Deferred.make(); + let ignoredMetadata = false; + const page = { + available: true, + visible: true, + tabId, + url: "http://example.test/", + title: "Example", + loading: false, + }; + const events = yield* broker.connect({ + clientId: "mcp-metadata-timeout-client", + environmentId, + }); + yield* Stream.runForEach(events, (event) => { + if (event.type === "connected") return Deferred.succeed(connected, undefined); + // The post-click icon lookup is the only status call with this budget. + if (event.request.operation === "status" && event.request.timeoutMs === 500) { + if (!ignoredMetadata) { + ignoredMetadata = true; + return Deferred.succeed(metadataSeen, undefined); + } + } + return broker.respond({ + clientId: "mcp-metadata-timeout-client", + connectionId: event.connectionId, + requestId: event.request.requestId, + ok: true, + result: event.request.operation === "click" ? {} : page, + }); + }).pipe(Effect.forkScoped); + yield* Deferred.await(connected); + + const click = yield* server + .callTool({ name: "preview_click", arguments: { x: 1, y: 1 } }) + .pipe( + Effect.provideService(McpInvocationContext.McpInvocationContext, invocation), + Effect.provideService(McpSchema.McpServerClient, client), + Effect.forkScoped, + ); + yield* Deferred.await(metadataSeen); + yield* TestClock.adjust(500); + const clicked = yield* Fiber.join(click); + expect(clicked.isError).toBe(false); + expect(clicked.structuredContent).toEqual({}); + + const status = yield* server + .callTool({ name: "preview_status", arguments: {} }) + .pipe( + Effect.provideService(McpInvocationContext.McpInvocationContext, invocation), + Effect.provideService(McpSchema.McpServerClient, client), + ); + expect(status.isError).toBe(false); + expect(status.structuredContent).toMatchObject({ available: true, tabId }); + }), + ).pipe(Effect.provide(TestLayer)), +); + it.effect("tells the agent how to fall back when no desktop app can run the snapshot", () => Effect.gen(function* () { const snapshot = yield* callSnapshot({}); diff --git a/apps/server/src/mcp/PreviewAutomationBroker.test.ts b/apps/server/src/mcp/PreviewAutomationBroker.test.ts index 7a8ed739d734..a7eafa49d0ff 100644 --- a/apps/server/src/mcp/PreviewAutomationBroker.test.ts +++ b/apps/server/src/mcp/PreviewAutomationBroker.test.ts @@ -1348,6 +1348,53 @@ it.effect("evicts an unanswered host and lets later calls use a healthy runtime" ), ); +it.effect("keeps the host when an optional status read times out", () => + Effect.scoped( + Effect.gen(function* () { + const broker = yield* makeBroker; + const connected = yield* Deferred.make(); + const metadataReceived = yield* Deferred.make(); + const operations: string[] = []; + yield* Stream.runForEach(yield* broker.connect(makeHost()), (event) => { + if (event.type === "connected") return Deferred.succeed(connected, undefined); + operations.push(event.request.operation); + if (event.request.timeoutMs === 500) { + return Deferred.succeed(metadataReceived, undefined); + } + return broker.respond({ + clientId: "client-1", + connectionId: event.connectionId, + requestId: event.request.requestId, + ok: true, + result: { available: true }, + }); + }).pipe(Effect.forkScoped); + yield* Deferred.await(connected); + + const metadata = yield* broker + .invoke({ + scope, + operation: "status", + input: {}, + timeoutMs: 500, + updateCurrentTab: false, + disconnectOnTimeout: false, + }) + .pipe(Effect.flip, Effect.forkScoped); + yield* Deferred.await(metadataReceived); + yield* TestClock.adjust(500); + expect(yield* Fiber.join(metadata)).toMatchObject({ + _tag: "PreviewAutomationTimeoutError", + }); + + expect(yield* broker.invoke({ scope, operation: "status", input: {} })).toEqual({ + available: true, + }); + expect(operations).toEqual(["status", "status"]); + }), + ), +); + it.effect("discards buffered actions before completing an evicted host stream", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/mcp/PreviewAutomationBroker.ts b/apps/server/src/mcp/PreviewAutomationBroker.ts index 65e3064f49a5..fe06a96d6f84 100644 --- a/apps/server/src/mcp/PreviewAutomationBroker.ts +++ b/apps/server/src/mcp/PreviewAutomationBroker.ts @@ -47,6 +47,12 @@ export interface PreviewAutomationInvokeInput { readonly timeoutMs?: number; /** Background metadata reads must not change the agent's current tab. */ readonly updateCurrentTab?: boolean; + /** + * Unanswered primary operations still drop the host. Optional reads, such as + * the page-icon status lookup, pass false so a short deadline cannot evict + * the only desktop browser. + */ + readonly disconnectOnTimeout?: boolean; /** Capture the routed tab before another request changes the current assignment. */ readonly onTargetTab?: (tabId: PreviewTabId | undefined) => void; } @@ -614,9 +620,13 @@ export const make = Effect.gen(function* PreviewAutomationBrokerMake() { return yield* Option.match(result, { onNone: () => Effect.gen(function* () { - // An unanswered request invalidates this connection. Do not replay - // actions: the client may have applied them before becoming unreachable. - yield* disconnect(connection.clientId, connection.queue, true); + // An unanswered primary request invalidates this connection. Do not + // replay actions: the client may have applied them before becoming + // unreachable. Optional reads leave the host registered; pending + // cleanup still drops a reply that arrives after this deadline. + if (input.disconnectOnTimeout !== false) { + yield* disconnect(connection.clientId, connection.queue, true); + } return yield* new PreviewAutomationTimeoutError(requestContext); }), onSome: (value) => Effect.succeed(value as A), diff --git a/apps/server/src/mcp/toolkits/preview/handlers.ts b/apps/server/src/mcp/toolkits/preview/handlers.ts index b70e68862683..ac5dc1a22c2c 100644 --- a/apps/server/src/mcp/toolkits/preview/handlers.ts +++ b/apps/server/src/mcp/toolkits/preview/handlers.ts @@ -85,6 +85,10 @@ const invoke = Effect.fn("PreviewToolkit.invoke")(function* ( input: {}, timeoutMs: 500, updateCurrentTab: false, + // The icon lookup is best-effort. Its 500ms deadline must not disconnect + // the only desktop host, or the next preview tool reports that no browser + // is available. + disconnectOnTimeout: false, ...(statusTabId === undefined ? {} : { tabId: statusTabId }), }) .pipe(Effect.orElseSucceed(() => null));