From 62748e86122b747226c76c2e112c5c4d2b4f7095 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:13:18 -0700 Subject: [PATCH 1/5] Add opt-in per-integration MCP search tools (#1741) --- .changeset/mcp-namespace-search-tools.md | 8 + apps/cli/src/main.ts | 22 +- apps/cloud/src/mcp/agent-handler.ts | 2 + apps/cloud/src/mcp/session-durable-object.ts | 4 + apps/host-cloudflare/src/mcp/agent-handler.ts | 2 + .../src/mcp/session-durable-object.ts | 4 + apps/local/src/mcp.ts | 2 + e2e/scenarios/namespace-search-tools.test.ts | 141 ++++++++++++ e2e/src/surfaces/mcp.ts | 12 +- .../core/execution/src/description.test.ts | 51 ++++- packages/core/execution/src/description.ts | 22 ++ packages/core/execution/src/index.ts | 6 +- .../src/mcp/agent-session-durable-object.ts | 8 + .../hosts/cloudflare/src/mcp/do-headers.ts | 8 +- packages/hosts/mcp/src/browser-approval.ts | 13 ++ .../hosts/mcp/src/in-memory-session-store.ts | 10 +- .../mcp/src/namespace-search-tools.test.ts | 200 ++++++++++++++++++ packages/hosts/mcp/src/tool-server.ts | 86 ++++++++ packages/react/src/api/analytics.tsx | 1 + .../src/components/mcp-install-card.test.ts | 40 ++++ .../react/src/components/mcp-install-card.tsx | 29 +++ 21 files changed, 660 insertions(+), 11 deletions(-) create mode 100644 .changeset/mcp-namespace-search-tools.md create mode 100644 e2e/scenarios/namespace-search-tools.test.ts create mode 100644 packages/hosts/mcp/src/namespace-search-tools.test.ts diff --git a/.changeset/mcp-namespace-search-tools.md b/.changeset/mcp-namespace-search-tools.md new file mode 100644 index 0000000000..28818f61ba --- /dev/null +++ b/.changeset/mcp-namespace-search-tools.md @@ -0,0 +1,8 @@ +--- +"@executor-js/execution": patch +"executor": patch +--- + +**Opt-in per-integration search tools on the MCP surface** + +Connecting with `?search_tools=true` (stdio: `executor mcp --search-tools`) adds one minimally-described `search_` MCP tool per connected integration, so the integration namespaces reach the model as tool names it can see without calling anything. Each call routes through the same flow as `tools.search({ namespace })` inside `execute`, and the tool list comes from the same inventory the `execute` description shows. Off by default; a clean endpoint URL is unchanged. diff --git a/apps/cli/src/main.ts b/apps/cli/src/main.ts index 6fa8a26fa8..fc2a9bf391 100644 --- a/apps/cli/src/main.ts +++ b/apps/cli/src/main.ts @@ -1362,6 +1362,7 @@ const mcpUrlForActiveLocalServer = (input: { readonly connection: ExecutorServerConnection; readonly elicitationMode: "browser" | "model"; readonly artifacts: boolean; + readonly searchTools: boolean; }): URL => { const url = new URL("/mcp", input.connection.origin); if (input.elicitationMode === "browser") { @@ -1372,6 +1373,11 @@ const mcpUrlForActiveLocalServer = (input: { if (!input.artifacts) { url.searchParams.set("artifacts", "false"); } + // Per-integration search tools are off by default; only the opt-in is + // spelled out. + if (input.searchTools) { + url.searchParams.set("search_tools", "true"); + } return url; }; @@ -1387,6 +1393,7 @@ const runMcpHttpBridge = async (input: { readonly manifest: ExecutorLocalServerManifest; readonly elicitationMode: "browser" | "model"; readonly artifacts: boolean; + readonly searchTools: boolean; }): Promise => { const stdio = new StdioServerTransport(); const authorization = getExecutorServerAuthorizationHeader(input.manifest.connection); @@ -1395,6 +1402,7 @@ const runMcpHttpBridge = async (input: { connection: input.manifest.connection, elicitationMode: input.elicitationMode, artifacts: input.artifacts, + searchTools: input.searchTools, }), authorization ? { requestInit: { headers: { Authorization: authorization } } } : undefined, ); @@ -1473,6 +1481,7 @@ const runMcpHttpBridge = async (input: { const runStdioMcpSession = (input: { readonly elicitationMode: "browser" | "model"; readonly artifacts: boolean; + readonly searchTools: boolean; }) => Effect.gen(function* () { // `executor mcp` never owns the local database. If a local server is already @@ -1489,6 +1498,7 @@ const runStdioMcpSession = (input: { manifest: active, elicitationMode: input.elicitationMode, artifacts: input.artifacts, + searchTools: input.searchTools, }), ); return; @@ -1515,6 +1525,7 @@ const runStdioMcpSession = (input: { manifest: elected, elicitationMode: input.elicitationMode, artifacts: input.artifacts, + searchTools: input.searchTools, }), ); }); @@ -2880,11 +2891,18 @@ const mcpCommand = Command.make( "Withhold the artifact surface from this connection: the artifact tools, the app shell resource, and the artifact skills. Served by default.", ), ), + searchTools: Options.boolean("search-tools") + .pipe(Options.withDefault(false)) + .pipe( + Options.withDescription( + "Serve one search_ tool per connected integration. Off by default; each routes through the same flow as tools.search inside execute.", + ), + ), }, - ({ scope, elicitationMode, noArtifacts }) => + ({ scope, elicitationMode, noArtifacts, searchTools }) => Effect.gen(function* () { applyScope(scope); - yield* runStdioMcpSession({ elicitationMode, artifacts: !noArtifacts }); + yield* runStdioMcpSession({ elicitationMode, artifacts: !noArtifacts, searchTools }); }), ).pipe(Command.withDescription("Start an MCP server over stdio")); diff --git a/apps/cloud/src/mcp/agent-handler.ts b/apps/cloud/src/mcp/agent-handler.ts index 7738e79c6a..9d75300bfe 100644 --- a/apps/cloud/src/mcp/agent-handler.ts +++ b/apps/cloud/src/mcp/agent-handler.ts @@ -13,6 +13,7 @@ import { currentPropagationHeaders, readArtifactsEnabled, readElicitationMode, + readSearchToolsEnabled, withVerifiedIdentityHeaders, } from "@executor-js/cloudflare/mcp/do-headers"; import type { McpSessionProps } from "@executor-js/cloudflare/mcp/agent-durable-object"; @@ -133,6 +134,7 @@ const propsForPrincipal = ( userId: principal.accountId, elicitationMode: readElicitationMode(request), artifactsEnabled: readArtifactsEnabled(request), + searchToolsEnabled: readSearchToolsEnabled(request), resource, webOrigin: new URL(request.url).origin, }, diff --git a/apps/cloud/src/mcp/session-durable-object.ts b/apps/cloud/src/mcp/session-durable-object.ts index 87161366de..c272e5026c 100644 --- a/apps/cloud/src/mcp/session-durable-object.ts +++ b/apps/cloud/src/mcp/session-durable-object.ts @@ -223,6 +223,7 @@ export class McpSessionDOSqlite extends McpAgentSessionDOBase` tool per connected integration, +// whose whole point is to carry the integration namespaces into the model's +// context as tool names. A call routes through the same flow as +// `tools.search({ namespace })` inside `execute`, so its results match what +// code-side enumeration returns. The proof is comparative: two sessions, same +// identity, same server, differing only in that query. +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { AuthTemplateSlug, ConnectionName, IntegrationSlug } from "@executor-js/sdk/shared"; + +import { scenario } from "../src/scenario"; +import { Api, Mcp, Target } from "../src/services"; + +const api = composePluginApi([openApiHttpPlugin()] as const); + +const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`; + +const spec = (baseUrl: string): string => + JSON.stringify({ + openapi: "3.0.3", + info: { title: "Searchable API", version: "1.0.0" }, + servers: [{ url: baseUrl }], + paths: { + "/alpha": { + get: { + operationId: "alphaOp", + summary: "First operation", + responses: { "200": { description: "ok" } }, + }, + }, + "/bravo": { + post: { + operationId: "bravoOp", + summary: "Second operation", + responses: { "200": { description: "ok" } }, + }, + }, + }, + }); + +scenario( + "Discovery · a session connected with search_tools=true serves one search tool per integration", + { timeout: 120_000 }, + Effect.gen(function* () { + const target = yield* Target; + const mcp = yield* Mcp; + const { client: makeClient } = yield* Api; + + const identity = yield* target.newIdentity(); + const client = yield* makeClient(api, identity); + const slug = unique("nssearch"); + + yield* Effect.ensuring( + Effect.gen(function* () { + // The connection must exist before a session opens: the tool list is + // built from the integration inventory at session creation. + yield* client.openapi.addSpec({ + payload: { + spec: { kind: "blob", value: spec("http://127.0.0.1:59999") }, + slug, + baseUrl: "http://127.0.0.1:59999", // never contacted: discovery only + authenticationTemplate: [ + { + slug: "apiKey", + type: "apiKey", + headers: { "x-api-key": [{ type: "variable", name: "token" }] }, + }, + ], + }, + }); + yield* client.connections.create({ + payload: { + owner: "org", + name: ConnectionName.make("main"), + integration: IntegrationSlug.make(slug), + template: AuthTemplateSlug.make("apiKey"), + value: "tok_nssearch", + }, + }); + + const searchTool = `search_${slug}`; + + // The default: a plain endpoint serves no per-integration search tools. + const defaultSession = mcp.session(identity); + const defaultTools = yield* defaultSession.listTools(); + expect( + defaultTools.filter((name) => name.startsWith("search_")), + "a plain session serves no search_ tools", + ).toEqual([]); + + // The opt-in: same identity, `?search_tools=true`. + const optedIn = mcp.session(identity, { searchTools: true }); + const optedInTools = yield* optedIn.describeTools(); + const names = optedInTools.map((tool) => tool.name); + expect(names, "the opted-in session serves the integration's search tool").toContain( + searchTool, + ); + // The core surface is untouched. + expect(names, "execute still works on an opted-in session").toContain("execute"); + expect(names, "skills still works on an opted-in session").toContain("skills"); + // The description is minimal and points back at the execute flow. + const described = optedInTools.find((tool) => tool.name === searchTool); + expect(described?.description, "the tool description names its namespace").toContain(slug); + expect(described?.description, "the tool description points at execute").toContain( + "execute", + ); + + // A keyword call returns the matching tool, exactly as + // `tools.search({ query, namespace })` inside execute would. + const searched = yield* optedIn.call(searchTool, { query: "alpha" }); + expect(searched.ok, `the search came back: ${searched.text}`).toBe(true); + expect(searched.text, "the keyword match is returned").toContain("alphaOp"); + expect(searched.text, "the non-match is not").not.toContain("bravoOp"); + + // An empty call enumerates the whole namespace. + const enumerated = yield* optedIn.call(searchTool, {}); + expect(enumerated.ok, `the enumeration came back: ${enumerated.text}`).toBe(true); + expect(enumerated.text, "enumeration lists every operation").toContain("alphaOp"); + expect(enumerated.text, "enumeration lists every operation").toContain("bravoOp"); + }), + Effect.gen(function* () { + yield* client.connections + .remove({ + params: { + owner: "org", + integration: IntegrationSlug.make(slug), + name: ConnectionName.make("main"), + }, + }) + .pipe(Effect.ignore); + yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore); + }), + ); + }), +); diff --git a/e2e/src/surfaces/mcp.ts b/e2e/src/surfaces/mcp.ts index 1069aad7b8..e1fa12c5f1 100644 --- a/e2e/src/surfaces/mcp.ts +++ b/e2e/src/surfaces/mcp.ts @@ -168,6 +168,10 @@ export interface McpSurface { * (`?artifacts=false`). Omitted means the product default: the full * artifact surface. */ readonly artifacts?: boolean; + /** Set `true` to opt this session into the per-integration + * `search_` tools (`?search_tools=true`). Omitted means + * the product default: none. */ + readonly searchTools?: boolean; readonly url?: string; }, ) => McpSession; @@ -303,12 +307,14 @@ export const makeMcpSurface = (target: Target, runDir?: string): McpSurface => ( const serverName = `${target.name}-${randomUUID().slice(0, 8)}`; // Per-connection settings ride the MCP endpoint query, the ecosystem // convention: `elicitation_mode` so a paused execution yields an approvalUrl - // instead of letting the model resume inline, and `artifacts=false` to opt - // the session out of the artifact surface. Both are non-defaults, so a - // plain session's URL carries no query at all. + // instead of letting the model resume inline, `artifacts=false` to opt the + // session out of the artifact surface, and `search_tools=true` to opt into + // the per-integration search tools. All are non-defaults, so a plain + // session's URL carries no query at all. const sessionQuery = [ ...(options?.elicitationMode ? [`elicitation_mode=${options.elicitationMode}`] : []), ...(options?.artifacts === false ? ["artifacts=false"] : []), + ...(options?.searchTools === true ? ["search_tools=true"] : []), ].join("&"); const sessionUrl = sessionQuery ? `${mcpUrl}?${sessionQuery}` : mcpUrl; diff --git a/packages/core/execution/src/description.test.ts b/packages/core/execution/src/description.test.ts index c95573fc10..365f115585 100644 --- a/packages/core/execution/src/description.test.ts +++ b/packages/core/execution/src/description.test.ts @@ -13,7 +13,7 @@ import { } from "@executor-js/sdk"; import { makeTestConfig } from "@executor-js/sdk/testing"; -import { buildExecuteDescription } from "./description"; +import { buildExecuteDescription, parseIntegrationInventory } from "./description"; const memoryProvider = (): CredentialProvider => { const store = new Map(); @@ -190,3 +190,52 @@ describe("buildExecuteDescription", () => { }), ); }); + +describe("parseIntegrationInventory", () => { + it.effect("round-trips the slugs a built description lists", () => + Effect.gen(function* () { + const executor = yield* createExecutor( + makeTestConfig({ plugins: [slackPlugin, githubPlugin] as const }), + ); + yield* executor["slack-plugin"].seed(); + yield* executor["github-plugin"].seed(); + yield* executor.connections.create({ + owner: "org", + name: ConnectionName.make("main"), + integration: SLACK, + template: TEMPLATE, + value: "slack-token", + }); + yield* executor.connections.create({ + owner: "user", + name: ConnectionName.make("personal"), + integration: GITHUB, + template: TEMPLATE, + value: "user-token", + }); + + const description = yield* buildExecuteDescription(executor); + + expect(parseIntegrationInventory(description)).toEqual(["github", "slack"]); + }), + ); + + it("returns nothing for a description without an inventory block", () => { + expect(parseIntegrationInventory("Execute TypeScript in a sandboxed runtime.")).toEqual([]); + }); + + it("reads item lines only, not the overflow marker or prose", () => { + const description = [ + "Execute TypeScript in a sandboxed runtime.", + "", + "## Available integrations", + "", + "Integrations you have connected. Their tools live under `tools..…`.", + "- `github`", + "- `google_gmail`", + "- ... 3 more", + ].join("\n"); + + expect(parseIntegrationInventory(description)).toEqual(["github", "google_gmail"]); + }); +}); diff --git a/packages/core/execution/src/description.ts b/packages/core/execution/src/description.ts index f9cda53d6e..dec2876fd1 100644 --- a/packages/core/execution/src/description.ts +++ b/packages/core/execution/src/description.ts @@ -74,6 +74,28 @@ const connectionPath = (connection: Connection): string => { // connected. const INVENTORY_LIMIT = 50; +/** One inventory line per integration: `` - `slug` ``. Owned here beside the + * formatter below so {@link parseIntegrationInventory} cannot drift from it. */ +const INVENTORY_ITEM_PATTERN = /^- `([^`]+)`$/; + +/** + * Recover the integration slugs from a built execute description — the exact + * list `formatIntegrationInventory` rendered, overflow marker excluded. Lets a + * host derive per-integration surfaces (the opt-in `search_` MCP + * tools) from the description it already holds, without a second + * `connections.list()` that could disagree with what the model reads. + */ +export const parseIntegrationInventory = (description: string): readonly string[] => { + const index = description.indexOf(INTEGRATION_INVENTORY_HEADER); + if (index === -1) return []; + const slugs: string[] = []; + for (const line of description.slice(index).split("\n")) { + const match = INVENTORY_ITEM_PATTERN.exec(line); + if (match?.[1]) slugs.push(match[1]); + } + return slugs; +}; + const formatIntegrationInventory = (connections: readonly Connection[]): string => { const slugs = [...new Set(connections.map((connection) => String(connection.integration)))].sort( (a, b) => a.localeCompare(b), diff --git a/packages/core/execution/src/index.ts b/packages/core/execution/src/index.ts index 3e68509cdf..23175a01b3 100644 --- a/packages/core/execution/src/index.ts +++ b/packages/core/execution/src/index.ts @@ -11,7 +11,11 @@ export { type ResumeResponse, } from "./engine"; -export { buildExecuteDescription, INTEGRATION_INVENTORY_HEADER } from "./description"; +export { + buildExecuteDescription, + parseIntegrationInventory, + INTEGRATION_INVENTORY_HEADER, +} from "./description"; export { EXECUTE_SKILL, CREATE_ARTIFACT_SKILL, diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts index 4ccc51965a..9ff71cfedd 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts @@ -43,6 +43,10 @@ export interface McpSessionInit { /** Whether this session serves artifacts, read off `?artifacts=` at connect * time. Absent means the default (enabled). */ readonly artifactsEnabled?: boolean; + /** Whether this session serves the per-integration `search_` + * tools, read off `?search_tools=` at connect time. Absent means the + * default (disabled). */ + readonly searchToolsEnabled?: boolean; /** The MCP resource the session was minted against (`/mcp` default vs a * `/mcp/toolkits/` toolkit), so the tool catalog is scoped to it. */ readonly resource: McpResource; @@ -105,6 +109,10 @@ export interface SessionMeta { * Absent — including for sessions persisted before the flag existed — means * the default (enabled). */ readonly artifactsEnabled?: boolean; + /** Whether the session serves the per-integration search tools (carried from + * {@link McpSessionInit}). Absent — including for sessions persisted before + * the flag existed — means the default (disabled). */ + readonly searchToolsEnabled?: boolean; /** The MCP resource the session serves (carried from {@link McpSessionInit}); * `buildMcpServer` scopes the tool catalog to it. */ readonly resource: McpResource; diff --git a/packages/hosts/cloudflare/src/mcp/do-headers.ts b/packages/hosts/cloudflare/src/mcp/do-headers.ts index ca101ba608..02c4b196d0 100644 --- a/packages/hosts/cloudflare/src/mcp/do-headers.ts +++ b/packages/hosts/cloudflare/src/mcp/do-headers.ts @@ -125,11 +125,13 @@ export const withMcpResponseHeaders = (response: Response): Response => { }; // The endpoint query contract — `?elicitation_mode=` (plus the legacy -// `?allow_model_resume` alias) and the `?artifacts=` opt-in — is shared with -// every host that serves these connections. Re-exported here so the worker -// dispatcher's existing import site (`./do-headers`) is unchanged. +// `?allow_model_resume` alias), the `?artifacts=` opt-out, and the +// `?search_tools=` opt-in — is shared with every host that serves these +// connections. Re-exported here so the worker dispatcher's existing import +// site (`./do-headers`) is unchanged. export { readArtifactsEnabled, readElicitationMode, + readSearchToolsEnabled, type McpElicitationMode, } from "@executor-js/host-mcp/browser-approval"; diff --git a/packages/hosts/mcp/src/browser-approval.ts b/packages/hosts/mcp/src/browser-approval.ts index 20e6693dea..a524b89fbb 100644 --- a/packages/hosts/mcp/src/browser-approval.ts +++ b/packages/hosts/mcp/src/browser-approval.ts @@ -66,6 +66,19 @@ export const readArtifactsEnabled = (request: Request): boolean => { return TRUE_QUERY_VALUES.has(value.toLowerCase()); }; +/** + * Read the per-integration search tools opt-IN off an MCP request's + * `?search_tools=` query. OFF by default: a clean endpoint URL serves only the + * core surface, and only an accepted truthy spelling (`?search_tools=true`) + * adds one `search_` tool per connected integration. Any other + * explicit value reads as the default (disabled). + */ +export const readSearchToolsEnabled = (request: Request): boolean => { + const value = new URL(request.url).searchParams.get("search_tools"); + if (value === null) return false; + return TRUE_QUERY_VALUES.has(value.toLowerCase()); +}; + /** * Build the console approval URL for a paused execution: * `//resume/?mcp_session_id=` diff --git a/packages/hosts/mcp/src/in-memory-session-store.ts b/packages/hosts/mcp/src/in-memory-session-store.ts index 2cd870fc1b..869691648f 100644 --- a/packages/hosts/mcp/src/in-memory-session-store.ts +++ b/packages/hosts/mcp/src/in-memory-session-store.ts @@ -10,6 +10,7 @@ import { formatResumeAcknowledgement, readArtifactsEnabled, readElicitationMode, + readSearchToolsEnabled, } from "./browser-approval"; import { makeInProcessBrowserApprovalStore, @@ -75,6 +76,9 @@ export interface McpBuildServerOptions { * with `?artifacts=false`; opted out, the built server registers none of * the artifact tools, resource, or skills. */ readonly artifactsEnabled?: boolean; + /** Whether this session serves the per-integration `search_` + * tools. False unless the client connected with `?search_tools=true`. */ + readonly searchToolsEnabled?: boolean; } /** Build the per-session `McpServer` + engine for a principal (the host's engine + tools). */ @@ -224,10 +228,14 @@ export const makeInMemoryMcpSessionStore = ( sessionId: () => string | null, ): McpBuildServerOptions => { const artifactsEnabled = readArtifactsEnabled(request); + const searchToolsEnabled = readSearchToolsEnabled(request); const mode = readElicitationMode(request); - if (mode !== "browser") return { artifactsEnabled, elicitationMode: { mode } }; + if (mode !== "browser") { + return { artifactsEnabled, searchToolsEnabled, elicitationMode: { mode } }; + } return { artifactsEnabled, + searchToolsEnabled, elicitationMode: { mode: "browser", // Prefer the pinned public origin; fall back to the request URL (correct diff --git a/packages/hosts/mcp/src/namespace-search-tools.test.ts b/packages/hosts/mcp/src/namespace-search-tools.test.ts new file mode 100644 index 0000000000..cbbd4a2251 --- /dev/null +++ b/packages/hosts/mcp/src/namespace-search-tools.test.ts @@ -0,0 +1,200 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import type * as Cause from "effect/Cause"; + +import type { ExecutionEngine } from "@executor-js/execution"; + +import { readSearchToolsEnabled } from "./browser-approval"; +import { createExecutorMcpServer, type ExecutorMcpServerConfig } from "./tool-server"; + +// --------------------------------------------------------------------------- +// Harness +// --------------------------------------------------------------------------- + +/** A stub engine that records every executed code string, so a test can prove + * a `search_` call became the expected `tools.search` code. */ +const makeRecordingEngine = (): { + engine: ExecutionEngine; + executed: string[]; +} => { + const executed: string[] = []; + return { + executed, + engine: { + execute: () => Effect.succeed({ result: "default" }), + executeWithPause: (code) => + Effect.sync(() => { + executed.push(code); + return { status: "completed" as const, result: { result: "default" } }; + }), + resume: () => Effect.succeed(null), + isExecutionSettled: undefined, + getPausedExecution: () => Effect.succeed(null), + pausedExecutionCount: () => Effect.succeed(0), + hasPausedExecutions: () => Effect.succeed(false), + getDescription: Effect.succeed("test executor"), + }, + }; +}; + +// The inventory block exactly as `buildExecuteDescription` renders it, +// including an overflow marker and a slug that cannot form a legal MCP tool +// name (which registration must skip, not fail on). +const DESCRIPTION_WITH_INVENTORY = [ + "Execute TypeScript in a sandboxed runtime.", + "", + "## Available integrations", + "", + "Integrations you have connected. Their tools live under `tools..…`.", + "- `github`", + "- `google_gmail`", + "- `not a tool name!`", + "- ... 3 more", +].join("\n"); + +const DESCRIPTION_WITHOUT_INVENTORY = "Execute TypeScript in a sandboxed runtime."; + +const withClient = async ( + config: ExecutorMcpServerConfig, + fn: (client: Client) => Promise, +) => { + const mcpServer = await Effect.runPromise(createExecutorMcpServer(config)); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }, { capabilities: {} }); + await mcpServer.connect(serverTransport); + await client.connect(clientTransport); + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: test helper must close MCP transports after async client assertions + try { + await fn(client); + } finally { + await clientTransport.close(); + await serverTransport.close(); + } +}; + +const toolNames = async (client: Client): Promise => + (await client.listTools()).tools.map((tool) => tool.name); + +// --------------------------------------------------------------------------- +// Registration +// --------------------------------------------------------------------------- + +describe("MCP host — per-integration search tools", () => { + it("registers none by default: the option is opt-in", async () => { + const { engine } = makeRecordingEngine(); + await withClient({ engine, description: DESCRIPTION_WITH_INVENTORY }, async (client) => { + const names = await toolNames(client); + expect(names).toContain("execute"); + expect(names.filter((name) => name.startsWith("search_"))).toEqual([]); + }); + }); + + it("registers none when the flag is explicitly false", async () => { + const { engine } = makeRecordingEngine(); + await withClient( + { engine, description: DESCRIPTION_WITH_INVENTORY, searchToolsEnabled: false }, + async (client) => { + expect((await toolNames(client)).filter((name) => name.startsWith("search_"))).toEqual([]); + }, + ); + }); + + it("registers one search_ tool per inventory entry when opted in", async () => { + const { engine } = makeRecordingEngine(); + await withClient( + { engine, description: DESCRIPTION_WITH_INVENTORY, searchToolsEnabled: true }, + async (client) => { + const tools = (await client.listTools()).tools; + const names = tools.map((tool) => tool.name); + expect(names).toContain("search_github"); + expect(names).toContain("search_google_gmail"); + // The core surface is untouched. + expect(names).toContain("execute"); + expect(names).toContain("skills"); + // The overflow marker is not an integration, and a slug that cannot + // form a legal MCP tool name is skipped rather than failing the + // session. + expect(names.filter((name) => name.startsWith("search_"))).toHaveLength(2); + + // Minimal descriptions: one line that names the namespace and points + // back at the execute flow. + const gmail = tools.find((tool) => tool.name === "search_google_gmail"); + expect(gmail?.description).toContain("google_gmail"); + expect(gmail?.description).toContain("execute"); + expect(gmail?.description?.includes("\n")).toBe(false); + }, + ); + }); + + it("registers none when the description carries no inventory", async () => { + const { engine } = makeRecordingEngine(); + await withClient( + { engine, description: DESCRIPTION_WITHOUT_INVENTORY, searchToolsEnabled: true }, + async (client) => { + expect((await toolNames(client)).filter((name) => name.startsWith("search_"))).toEqual([]); + }, + ); + }); + + // --------------------------------------------------------------------------- + // Dispatch: the call is the same flow as `tools.search` inside execute + // --------------------------------------------------------------------------- + + it("routes a call through the engine as tools.search with the namespace pinned", async () => { + const { engine, executed } = makeRecordingEngine(); + await withClient( + { engine, description: DESCRIPTION_WITH_INVENTORY, searchToolsEnabled: true }, + async (client) => { + const result = await client.callTool({ + name: "search_github", + arguments: { query: "issues", limit: 5 }, + }); + expect(executed).toEqual([ + 'return tools.search({"query":"issues","namespace":"github","limit":5})', + ]); + expect(result.isError ?? false).toBe(false); + }, + ); + }); + + it("enumerates the namespace when the query is omitted", async () => { + const { engine, executed } = makeRecordingEngine(); + await withClient( + { engine, description: DESCRIPTION_WITH_INVENTORY, searchToolsEnabled: true }, + async (client) => { + await client.callTool({ name: "search_google_gmail", arguments: {} }); + expect(executed).toEqual(['return tools.search({"query":"","namespace":"google_gmail"})']); + }, + ); + }); +}); + +// --------------------------------------------------------------------------- +// Endpoint query contract +// --------------------------------------------------------------------------- + +describe("search tools opt-in query", () => { + const requestFor = (url: string): Request => new Request(url); + + it("defaults to disabled on a clean endpoint", () => { + expect(readSearchToolsEnabled(requestFor("https://executor.example/mcp"))).toBe(false); + }); + + it("accepts the truthy spellings", () => { + for (const value of ["1", "true", "yes", "on", "TRUE", "On"]) { + expect( + readSearchToolsEnabled(requestFor(`https://executor.example/mcp?search_tools=${value}`)), + ).toBe(true); + } + }); + + it("reads any other explicit value as the default (disabled)", () => { + for (const value of ["false", "0", "no", "off", "maybe", ""]) { + expect( + readSearchToolsEnabled(requestFor(`https://executor.example/mcp?search_tools=${value}`)), + ).toBe(false); + } + }); +}); diff --git a/packages/hosts/mcp/src/tool-server.ts b/packages/hosts/mcp/src/tool-server.ts index 26014e5637..0589317217 100644 --- a/packages/hosts/mcp/src/tool-server.ts +++ b/packages/hosts/mcp/src/tool-server.ts @@ -39,6 +39,7 @@ import { formatPausedExecution, formatTtlDuration, findSkill, + parseIntegrationInventory, renderSkillsIndex, skillCatalogFor, EXECUTE_SKILL, @@ -165,6 +166,16 @@ type SharedMcpServerConfig = { * serves. `execute`, `skills` and `resume` are untouched. */ readonly artifactsEnabled?: boolean; + /** + * Per-connection opt-IN for the per-integration search tools. Defaults to + * false. A client that connects with `?search_tools=true` gets one + * `search_` tool per connected integration (the same inventory + * the `execute` description lists). The tools exist to carry the namespaces + * into the model's context as tool names; each call routes through the same + * execution flow as `tools.search({ namespace })` inside `execute`, so the + * results match what code-side search returns. + */ + readonly searchToolsEnabled?: boolean; /** * Renders an artifact once, server-side, before it is saved — so a component * that throws on its first render is refused at create time with the real @@ -1094,6 +1105,9 @@ export const createExecutorMcpServer = ( // the skills catalog below. const artifactsEnabled = config.artifactsEnabled ?? true; const skillCatalog: readonly Skill[] = skillCatalogFor({ artifacts: artifactsEnabled }); + // Per-integration search tools are off unless this connection opted in + // (`?search_tools=true`). + const searchToolsEnabled = config.searchToolsEnabled ?? false; // Captured at construction time. SDK callbacks fire later (often // deferred past the outer Effect's await), so we use the runtime to @@ -1251,6 +1265,23 @@ export const createExecutorMcpServer = ( Effect.annotateSpans(joinKeyAttributes(extra)), ); + // `search_` is `execute` running `tools.search` with the + // namespace pinned. The code is built HERE, from the slug the tool was + // registered under and JSON-encoded arguments — never concatenated from + // raw model input — and then takes the exact `executeCode` path, so the + // results, formatting, and telemetry match a hand-written + // `tools.search({ namespace })` call. + const searchNamespaceCode = ( + integration: string, + args: { readonly query?: string; readonly limit?: number; readonly offset?: number }, + ): string => + `return tools.search(${JSON.stringify({ + query: args.query ?? "", + namespace: integration, + ...(args.limit === undefined ? {} : { limit: args.limit }), + ...(args.offset === undefined ? {} : { offset: args.offset }), + })})`; + /** What the caller could bind an unresolved role to. Best effort: the * connections port is optional, and a failure to enumerate must not * replace the real error with a different one. */ @@ -1587,6 +1618,61 @@ export const createExecutorMcpServer = ( }), ); + // --- per-integration search tools (opt-in, `?search_tools=true`) --- + // + // One minimally-described tool per connected integration, named + // `search_`. Their job is to put the integration namespaces + // into the model's context as tool names it can see without calling + // anything; a call routes through the same flow as + // `tools.search({ namespace })` inside `execute` (see searchNamespaceCode). + // The inventory comes from the same built description the model reads, so + // the two surfaces cannot list different integrations. + if (searchToolsEnabled) { + // The MCP tool-name grammar ([A-Za-z0-9_-]). Integration slugs already + // conform (they are `tools.` property names in sandbox code); one + // that somehow doesn't is skipped rather than failing the whole session. + const TOOL_NAME_SAFE_SLUG = /^[A-Za-z0-9_-]+$/; + const namespaces = parseIntegrationInventory(description).filter((slug) => + TOOL_NAME_SAFE_SLUG.test(slug), + ); + yield* Effect.sync(() => { + for (const integration of namespaces) { + server.registerTool( + `search_${integration}`, + { + description: `Find \`${integration}\` tools. Same results as \`tools.search({ query, namespace: "${integration}" })\` inside execute; run what you find with execute.`, + inputSchema: { + query: z + .string() + .optional() + .describe("Keywords to match. Omit to list the whole namespace."), + limit: z.number().optional().describe("Max results per page."), + offset: z.number().optional().describe("Pagination offset."), + }, + }, + ({ query, limit, offset }, extra) => + runToolEffect( + executeCode(searchNamespaceCode(integration, { query, limit, offset }), extra).pipe( + Effect.withSpan("mcp.host.tool.namespace_search", { + attributes: { + "mcp.tool.name": `search_${integration}`, + "executor.integration": integration, + }, + }), + ), + ), + ); + } + }).pipe( + Effect.withSpan("mcp.host.register_tool", { + attributes: { + "mcp.tool.name": "search_", + "mcp.namespace_search.count": namespaces.length, + }, + }), + ); + } + // --- artifacts / MCP Apps --- // // These register unconditionally once a shell loader is configured. Whether diff --git a/packages/react/src/api/analytics.tsx b/packages/react/src/api/analytics.tsx index 2b68f57ad8..48ded24923 100644 --- a/packages/react/src/api/analytics.tsx +++ b/packages/react/src/api/analytics.tsx @@ -157,6 +157,7 @@ export interface AnalyticsEvents { mcp_install_transport_switched: { transport: "http" | "stdio" }; mcp_install_elicitation_mode_changed: { elicitation_mode: string }; mcp_install_artifacts_toggled: { artifacts: boolean }; + mcp_install_search_tools_toggled: { search_tools: boolean }; // ── Command palette ────────────────────────────────────────────────────── command_palette_navigated: { diff --git a/packages/react/src/components/mcp-install-card.test.ts b/packages/react/src/components/mcp-install-card.test.ts index 44f4c642cc..38ed2a7ba2 100644 --- a/packages/react/src/components/mcp-install-card.test.ts +++ b/packages/react/src/components/mcp-install-card.test.ts @@ -111,6 +111,46 @@ describe("MCP install command rendering", () => { ); }); + // Per-integration search tools are off by default, so only the opt-in is + // ever spelled out: a card left alone must still produce the bare endpoint. + it("emits the search tools opt-in only when enabled", () => { + expect( + buildMcpHttpEndpoint({ + origin: "https://executor.example", + desktop: null, + searchTools: false, + }), + ).toBe("https://executor.example/mcp"); + + expect( + buildMcpHttpEndpoint({ + origin: "https://executor.example", + desktop: null, + searchTools: true, + }), + ).toBe("https://executor.example/mcp?search_tools=true"); + + // Both non-defaults together, in the order the card renders them. + expect( + buildMcpHttpEndpoint({ + origin: "https://executor.example", + desktop: null, + artifacts: false, + searchTools: true, + }), + ).toBe("https://executor.example/mcp?artifacts=false&search_tools=true"); + }); + + it("passes the search tools opt-in to the stdio CLI as a flag", () => { + expect( + buildMcpInstallCommand({ mode: "stdio", isDev: false, origin: null, searchTools: false }), + ).toBe("npx add-mcp 'executor mcp' --name executor"); + + expect( + buildMcpInstallCommand({ mode: "stdio", isDev: false, origin: null, searchTools: true }), + ).toBe("npx add-mcp 'executor mcp --search-tools' --name executor"); + }); + it("passes the artifacts opt-out to the stdio CLI as a flag", () => { expect( buildMcpInstallCommand({ mode: "stdio", isDev: false, origin: null, artifacts: true }), diff --git a/packages/react/src/components/mcp-install-card.tsx b/packages/react/src/components/mcp-install-card.tsx index da371aa774..7701913ecf 100644 --- a/packages/react/src/components/mcp-install-card.tsx +++ b/packages/react/src/components/mcp-install-card.tsx @@ -52,6 +52,9 @@ export const buildMcpHttpEndpoint = (input: { /** Artifacts are on by default, so only the opt-out is spelled out on the * URL (`&artifacts=false`) and a default endpoint stays clean. */ readonly artifacts?: boolean; + /** Per-integration search tools are off by default, so only the opt-in is + * spelled out on the URL (`&search_tools=true`). */ + readonly searchTools?: boolean; // Cloud only: pins the URL to `//mcp` (the server also accepts the // legacy `//mcp` form). Desktop/local pass nothing and get the bare // `/mcp` path. @@ -73,6 +76,7 @@ export const buildMcpHttpEndpoint = (input: { params.push(["elicitation_mode", input.elicitationMode]); } if (input.artifacts === false) params.push(["artifacts", "false"]); + if (input.searchTools === true) params.push(["search_tools", "true"]); if (params.length === 0) return endpoint; const query = params.map(([key, value]) => `${key}=${value}`).join("&"); @@ -94,6 +98,7 @@ export const buildMcpInstallCommand = (input: { readonly authorizationHeader?: string | null; readonly elicitationMode?: McpElicitationMode; readonly artifacts?: boolean; + readonly searchTools?: boolean; readonly devCliCwd?: string; readonly organizationSlug?: string | null; }): string => { @@ -103,6 +108,7 @@ export const buildMcpInstallCommand = (input: { desktop: input.desktop ? { port: input.desktop.port } : null, elicitationMode: input.elicitationMode, artifacts: input.artifacts, + searchTools: input.searchTools, organizationSlug: input.organizationSlug, }); const headerFlags: string[] = []; @@ -130,6 +136,9 @@ export const buildMcpInstallCommand = (input: { if (input.artifacts === false) { innerArgs.push("--no-artifacts"); } + if (input.searchTools === true) { + innerArgs.push("--search-tools"); + } return `npx add-mcp ${shellQuoteWord(innerArgs.map(shellQuoteWord).join(" "))} --name executor`; }; @@ -138,6 +147,7 @@ export function McpInstallCard(props: { className?: string }) { const [advancedOpen, setAdvancedOpen] = useState(false); const [httpElicitationMode, setHttpElicitationMode] = useState("model"); const [artifacts, setArtifacts] = useState(true); + const [searchTools, setSearchTools] = useState(false); const organizationSlug = useOrganizationSlug(); const serverConnection = useExecutorServerConnection(); // Desktop hosts ship Electron without putting an `executor` binary on @@ -181,6 +191,7 @@ export function McpInstallCard(props: { className?: string }) { authorizationHeader, elicitationMode, artifacts, + searchTools, devCliCwd, organizationSlug, }); @@ -220,6 +231,24 @@ export function McpInstallCard(props: { className?: string }) { aria-label="Artifacts" /> +
+
+
Integration search tools
+
+ {searchTools + ? "One search tool per connected integration, so agents see your integrations as tool names." + : "Disabled: agents discover tools through search inside execute."} +
+
+ { + setSearchTools(next); + trackEvent("mcp_install_search_tools_toggled", { search_tools: next }); + }} + aria-label="Integration search tools" + /> +
Resume approvals
From 05c3b22e3fd108cc14a880df4714e0d2384a9d53 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:14:14 -0700 Subject: [PATCH 2/5] Drop eager MCP schema preload in workerd isolates (#1742) --- bun.lock | 5 ++- package.json | 3 +- ...@modelcontextprotocol%2Fclient@2.0.0.patch | 39 +++++++++++++++++++ scripts/check-patched-deps.ts | 15 +++++++ 4 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 patches/@modelcontextprotocol%2Fclient@2.0.0.patch diff --git a/bun.lock b/bun.lock index 0ca43d8135..a5060676ac 100644 --- a/bun.lock +++ b/bun.lock @@ -1243,10 +1243,11 @@ }, "patchedDependencies": { "@electric-sql/pglite-socket@0.1.4": "patches/@electric-sql%2Fpglite-socket@0.1.4.patch", - "libsql@0.5.29": "patches/libsql@0.5.29.patch", - "@1password/sdk-core@0.4.1-beta.1": "patches/@1password%2Fsdk-core@0.4.1-beta.1.patch", "agents@0.17.3": "patches/agents@0.17.3.patch", "postgres@3.4.9": "patches/postgres@3.4.9.patch", + "@modelcontextprotocol/client@2.0.0": "patches/@modelcontextprotocol%2Fclient@2.0.0.patch", + "libsql@0.5.29": "patches/libsql@0.5.29.patch", + "@1password/sdk-core@0.4.1-beta.1": "patches/@1password%2Fsdk-core@0.4.1-beta.1.patch", }, "catalog": { "@effect/atom-react": "4.0.0-beta.59", diff --git a/package.json b/package.json index 6f8ab54ab4..007a4bb012 100644 --- a/package.json +++ b/package.json @@ -137,6 +137,7 @@ "@cloudflare/vite-plugin@1.31.2": "patches/@cloudflare%2Fvite-plugin@1.31.2.patch", "agents@0.17.3": "patches/agents@0.17.3.patch", "libsql@0.5.29": "patches/libsql@0.5.29.patch", - "@electric-sql/pglite-socket@0.1.4": "patches/@electric-sql%2Fpglite-socket@0.1.4.patch" + "@electric-sql/pglite-socket@0.1.4": "patches/@electric-sql%2Fpglite-socket@0.1.4.patch", + "@modelcontextprotocol/client@2.0.0": "patches/@modelcontextprotocol%2Fclient@2.0.0.patch" } } diff --git a/patches/@modelcontextprotocol%2Fclient@2.0.0.patch b/patches/@modelcontextprotocol%2Fclient@2.0.0.patch new file mode 100644 index 0000000000..cdd2630e8e --- /dev/null +++ b/patches/@modelcontextprotocol%2Fclient@2.0.0.patch @@ -0,0 +1,39 @@ +diff --git a/dist/shimsWorkerd.cjs b/dist/shimsWorkerd.cjs +index 29ef3a5b9872dbc934ec094d7419f6c965808e86..4b2dc3e755e5566edbbfd56087587e5c080ade02 100644 +--- a/dist/shimsWorkerd.cjs ++++ b/dist/shimsWorkerd.cjs +@@ -7,7 +7,11 @@ const require_cfWorkerProvider = require('./cfWorkerProvider-B3dvjU3F.cjs'); + * + * This file is selected via package.json export conditions when running in workerd. + */ +-require_src.preloadSchemas(); ++/* executor-patch: no-preload-schemas — the upstream warm-up eagerly builds ++ * both protocol eras' zod schemas (~15MB heap) in EVERY isolate at module ++ * eval; under Cloudflare memory pressure this collapsed isolate reuse ++ * (~5-7 -> ~3 requests/isolate, 2026-08-25 latency incident). Schemas stay ++ * lazy+memoized; the first MCP use in an isolate pays ~15ms CPU instead. */ + /** + * Whether `fetch()` may throw `TypeError` due to CORS. CORS is a browser-only concept — + * in Cloudflare Workers, a `TypeError` from `fetch` is always a real network/configuration +diff --git a/dist/shimsWorkerd.mjs b/dist/shimsWorkerd.mjs +index 02d2730aca540adf88d52487ec3f3a9b3f8e80ee..beb9604d2f7f486f711ef4ba7f93a85caf72c2dc 100644 +--- a/dist/shimsWorkerd.mjs ++++ b/dist/shimsWorkerd.mjs +@@ -1,4 +1,3 @@ +-import { n as preloadSchemas } from "./src-D_zzAWoS.mjs"; + import { t as CfWorkerJsonSchemaValidator } from "./cfWorkerProvider-BylNJ1R5.mjs"; + + //#region src/shimsWorkerd.ts +@@ -7,7 +6,11 @@ import { t as CfWorkerJsonSchemaValidator } from "./cfWorkerProvider-BylNJ1R5.mj + * + * This file is selected via package.json export conditions when running in workerd. + */ +-preloadSchemas(); ++/* executor-patch: no-preload-schemas — the upstream warm-up eagerly builds ++ * both protocol eras' zod schemas (~15MB heap) in EVERY isolate at module ++ * eval; under Cloudflare memory pressure this collapsed isolate reuse ++ * (~5-7 -> ~3 requests/isolate, 2026-08-25 latency incident). Schemas stay ++ * lazy+memoized; the first MCP use in an isolate pays ~15ms CPU instead. */ + /** + * Whether `fetch()` may throw `TypeError` due to CORS. CORS is a browser-only concept — + * in Cloudflare Workers, a `TypeError` from `fetch` is always a real network/configuration diff --git a/scripts/check-patched-deps.ts b/scripts/check-patched-deps.ts index 7f982c0547..7f296702d6 100644 --- a/scripts/check-patched-deps.ts +++ b/scripts/check-patched-deps.ts @@ -81,6 +81,21 @@ const runtimeChecks: readonly RuntimeCheck[] = [ // not exist in the upstream 0.17.3 dist. sentinels: ["markStreamUndelivered", "replayUndeliveredResponses"], }, + { + package: "@modelcontextprotocol/client@2.0.0 (dist/shimsWorkerd.mjs)", + purpose: + "drops the workerd shim's module-eval preloadSchemas() (~15MB heap per isolate; " + + "collapsed isolate reuse in the 2026-08-25 latency incident) — schemas build lazily", + resolveEntry: () => { + // The workerd shim is only reachable through the `workerd` export + // condition, which require.resolve doesn't use — resolve the package + // main from the workspace package that depends on it and take the + // sibling shim file. + const main = resolveFrom("@modelcontextprotocol/client", "packages/plugins/mcp"); + return resolve(main, "..", "shimsWorkerd.mjs"); + }, + sentinels: ["executor-patch: no-preload-schemas"], + }, ]; for (const check of runtimeChecks) { From 91062c2b1d7b8edbc8470ca5eaa544045652afaa Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:05:31 -0700 Subject: [PATCH 3/5] Load the MCP client SDK lazily on first outbound connection (#1747) --- .changeset/lazy-mcp-client-module.md | 5 + packages/plugins/mcp/package.json | 3 +- .../scripts/gen-call-tool-result-schema.ts | 26 ++ .../src/sdk/call-tool-result-schema.gen.ts | 395 ++++++++++++++++++ .../src/sdk/call-tool-result-schema.test.ts | 16 + packages/plugins/mcp/src/sdk/client-module.ts | 39 ++ packages/plugins/mcp/src/sdk/connection.ts | 54 +-- .../plugins/mcp/src/sdk/http-status.test.ts | 9 +- packages/plugins/mcp/src/sdk/http-status.ts | 13 +- packages/plugins/mcp/src/sdk/invoke.test.ts | 8 +- packages/plugins/mcp/src/sdk/invoke.ts | 33 +- packages/plugins/mcp/src/sdk/plugin.ts | 9 +- 12 files changed, 567 insertions(+), 43 deletions(-) create mode 100644 .changeset/lazy-mcp-client-module.md create mode 100644 packages/plugins/mcp/scripts/gen-call-tool-result-schema.ts create mode 100644 packages/plugins/mcp/src/sdk/call-tool-result-schema.gen.ts create mode 100644 packages/plugins/mcp/src/sdk/call-tool-result-schema.test.ts create mode 100644 packages/plugins/mcp/src/sdk/client-module.ts diff --git a/.changeset/lazy-mcp-client-module.md b/.changeset/lazy-mcp-client-module.md new file mode 100644 index 0000000000..309303275c --- /dev/null +++ b/.changeset/lazy-mcp-client-module.md @@ -0,0 +1,5 @@ +--- +"@executor-js/plugin-mcp": patch +--- + +Load the MCP client SDK lazily on first outbound connection instead of at module evaluation. Runtimes that bundle the plugin (notably Cloudflare Workers) no longer pay the client package's module-eval memory and CPU on startup or on code paths that never dial an MCP server. diff --git a/packages/plugins/mcp/package.json b/packages/plugins/mcp/package.json index 79deee56fd..251a23b88e 100644 --- a/packages/plugins/mcp/package.json +++ b/packages/plugins/mcp/package.json @@ -58,7 +58,8 @@ "typecheck": "tsgo --noEmit", "test": "vitest run", "test:watch": "vitest", - "typecheck:slow": "bunx tsc --noEmit -p tsconfig.json" + "typecheck:slow": "bunx tsc --noEmit -p tsconfig.json", + "gen:call-tool-result-schema": "bun scripts/gen-call-tool-result-schema.ts" }, "dependencies": { "@cfworker/json-schema": "^4.1.1", diff --git a/packages/plugins/mcp/scripts/gen-call-tool-result-schema.ts b/packages/plugins/mcp/scripts/gen-call-tool-result-schema.ts new file mode 100644 index 0000000000..1842522cd9 --- /dev/null +++ b/packages/plugins/mcp/scripts/gen-call-tool-result-schema.ts @@ -0,0 +1,26 @@ +// Regenerates src/sdk/call-tool-result-schema.gen.ts from the installed +// @modelcontextprotocol/core. The schema is baked into a generated file so +// the runtime never imports the core package (importing it costs ~8.6MB of +// heap per Cloudflare isolate; see client-module.ts). Run after bumping the +// core dependency: +// bun scripts/gen-call-tool-result-schema.ts +// call-tool-result-schema.test.ts fails if the generated copy drifts from the +// installed package. +import { writeFileSync } from "node:fs"; +import { resolve } from "node:path"; + +import { CallToolResultSchema } from "@modelcontextprotocol/core"; + +const schema = CallToolResultSchema.toJSONSchema(); +const target = resolve(import.meta.dir, "../src/sdk/call-tool-result-schema.gen.ts"); + +writeFileSync( + target, + `// Generated by scripts/gen-call-tool-result-schema.ts — do not edit. +// JSON Schema of @modelcontextprotocol/core's CallToolResultSchema, baked in +// so the runtime never pays the core package's module-eval cost (see +// client-module.ts). call-tool-result-schema.test.ts guards drift. +export const callToolResultJsonSchema = ${JSON.stringify(schema, null, 2)} as const; +`, +); +console.log(`wrote ${target}`); diff --git a/packages/plugins/mcp/src/sdk/call-tool-result-schema.gen.ts b/packages/plugins/mcp/src/sdk/call-tool-result-schema.gen.ts new file mode 100644 index 0000000000..bba9107074 --- /dev/null +++ b/packages/plugins/mcp/src/sdk/call-tool-result-schema.gen.ts @@ -0,0 +1,395 @@ +// Generated by scripts/gen-call-tool-result-schema.ts — do not edit. +// JSON Schema of @modelcontextprotocol/core's CallToolResultSchema, baked in +// so the runtime never pays the core package's module-eval cost (see +// client-module.ts). call-tool-result-schema.test.ts guards drift. +export const callToolResultJsonSchema = { + $schema: "https://json-schema.org/draft/2020-12/schema", + type: "object", + properties: { + _meta: { + type: "object", + properties: { + "io.modelcontextprotocol/serverInfo": { + type: "object", + properties: { + name: { + type: "string", + }, + title: { + type: "string", + }, + icons: { + type: "array", + items: { + type: "object", + properties: { + src: { + type: "string", + }, + mimeType: { + type: "string", + }, + sizes: { + type: "array", + items: { + type: "string", + }, + }, + theme: { + type: "string", + enum: ["light", "dark"], + }, + }, + required: ["src"], + additionalProperties: false, + }, + }, + version: { + type: "string", + }, + websiteUrl: { + type: "string", + }, + description: { + type: "string", + }, + }, + required: ["name", "version"], + additionalProperties: false, + }, + }, + additionalProperties: {}, + }, + content: { + default: [], + type: "array", + items: { + anyOf: [ + { + type: "object", + properties: { + type: { + type: "string", + const: "text", + }, + text: { + type: "string", + }, + annotations: { + type: "object", + properties: { + audience: { + type: "array", + items: { + type: "string", + enum: ["user", "assistant"], + }, + }, + priority: { + type: "number", + minimum: 0, + maximum: 1, + }, + lastModified: { + type: "string", + format: "date-time", + pattern: + "^(?:(?:\\d\\d[2468][048]|\\d\\d[13579][26]|\\d\\d0[48]|[02468][048]00|[13579][26]00)-02-29|\\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\\d|30)|(?:02)-(?:0[1-9]|1\\d|2[0-8])))T(?:(?:[01]\\d|2[0-3]):[0-5]\\d(?::[0-5]\\d(?:\\.\\d+)?)?(?:Z|([+-](?:[01]\\d|2[0-3]):[0-5]\\d)))$", + }, + }, + additionalProperties: false, + }, + _meta: { + type: "object", + propertyNames: { + type: "string", + }, + additionalProperties: {}, + }, + }, + required: ["type", "text"], + additionalProperties: false, + }, + { + type: "object", + properties: { + type: { + type: "string", + const: "image", + }, + data: { + type: "string", + }, + mimeType: { + type: "string", + }, + annotations: { + type: "object", + properties: { + audience: { + type: "array", + items: { + type: "string", + enum: ["user", "assistant"], + }, + }, + priority: { + type: "number", + minimum: 0, + maximum: 1, + }, + lastModified: { + type: "string", + format: "date-time", + pattern: + "^(?:(?:\\d\\d[2468][048]|\\d\\d[13579][26]|\\d\\d0[48]|[02468][048]00|[13579][26]00)-02-29|\\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\\d|30)|(?:02)-(?:0[1-9]|1\\d|2[0-8])))T(?:(?:[01]\\d|2[0-3]):[0-5]\\d(?::[0-5]\\d(?:\\.\\d+)?)?(?:Z|([+-](?:[01]\\d|2[0-3]):[0-5]\\d)))$", + }, + }, + additionalProperties: false, + }, + _meta: { + type: "object", + propertyNames: { + type: "string", + }, + additionalProperties: {}, + }, + }, + required: ["type", "data", "mimeType"], + additionalProperties: false, + }, + { + type: "object", + properties: { + type: { + type: "string", + const: "audio", + }, + data: { + type: "string", + }, + mimeType: { + type: "string", + }, + annotations: { + type: "object", + properties: { + audience: { + type: "array", + items: { + type: "string", + enum: ["user", "assistant"], + }, + }, + priority: { + type: "number", + minimum: 0, + maximum: 1, + }, + lastModified: { + type: "string", + format: "date-time", + pattern: + "^(?:(?:\\d\\d[2468][048]|\\d\\d[13579][26]|\\d\\d0[48]|[02468][048]00|[13579][26]00)-02-29|\\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\\d|30)|(?:02)-(?:0[1-9]|1\\d|2[0-8])))T(?:(?:[01]\\d|2[0-3]):[0-5]\\d(?::[0-5]\\d(?:\\.\\d+)?)?(?:Z|([+-](?:[01]\\d|2[0-3]):[0-5]\\d)))$", + }, + }, + additionalProperties: false, + }, + _meta: { + type: "object", + propertyNames: { + type: "string", + }, + additionalProperties: {}, + }, + }, + required: ["type", "data", "mimeType"], + additionalProperties: false, + }, + { + type: "object", + properties: { + name: { + type: "string", + }, + title: { + type: "string", + }, + icons: { + type: "array", + items: { + type: "object", + properties: { + src: { + type: "string", + }, + mimeType: { + type: "string", + }, + sizes: { + type: "array", + items: { + type: "string", + }, + }, + theme: { + type: "string", + enum: ["light", "dark"], + }, + }, + required: ["src"], + additionalProperties: false, + }, + }, + uri: { + type: "string", + }, + description: { + type: "string", + }, + mimeType: { + type: "string", + }, + size: { + type: "number", + }, + annotations: { + type: "object", + properties: { + audience: { + type: "array", + items: { + type: "string", + enum: ["user", "assistant"], + }, + }, + priority: { + type: "number", + minimum: 0, + maximum: 1, + }, + lastModified: { + type: "string", + format: "date-time", + pattern: + "^(?:(?:\\d\\d[2468][048]|\\d\\d[13579][26]|\\d\\d0[48]|[02468][048]00|[13579][26]00)-02-29|\\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\\d|30)|(?:02)-(?:0[1-9]|1\\d|2[0-8])))T(?:(?:[01]\\d|2[0-3]):[0-5]\\d(?::[0-5]\\d(?:\\.\\d+)?)?(?:Z|([+-](?:[01]\\d|2[0-3]):[0-5]\\d)))$", + }, + }, + additionalProperties: false, + }, + _meta: { + type: "object", + properties: {}, + additionalProperties: {}, + }, + type: { + type: "string", + const: "resource_link", + }, + }, + required: ["name", "uri", "type"], + additionalProperties: false, + }, + { + type: "object", + properties: { + type: { + type: "string", + const: "resource", + }, + resource: { + anyOf: [ + { + type: "object", + properties: { + uri: { + type: "string", + }, + mimeType: { + type: "string", + }, + _meta: { + type: "object", + propertyNames: { + type: "string", + }, + additionalProperties: {}, + }, + text: { + type: "string", + }, + }, + required: ["uri", "text"], + additionalProperties: false, + }, + { + type: "object", + properties: { + uri: { + type: "string", + }, + mimeType: { + type: "string", + }, + _meta: { + type: "object", + propertyNames: { + type: "string", + }, + additionalProperties: {}, + }, + blob: { + type: "string", + }, + }, + required: ["uri", "blob"], + additionalProperties: false, + }, + ], + }, + annotations: { + type: "object", + properties: { + audience: { + type: "array", + items: { + type: "string", + enum: ["user", "assistant"], + }, + }, + priority: { + type: "number", + minimum: 0, + maximum: 1, + }, + lastModified: { + type: "string", + format: "date-time", + pattern: + "^(?:(?:\\d\\d[2468][048]|\\d\\d[13579][26]|\\d\\d0[48]|[02468][048]00|[13579][26]00)-02-29|\\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\\d|30)|(?:02)-(?:0[1-9]|1\\d|2[0-8])))T(?:(?:[01]\\d|2[0-3]):[0-5]\\d(?::[0-5]\\d(?:\\.\\d+)?)?(?:Z|([+-](?:[01]\\d|2[0-3]):[0-5]\\d)))$", + }, + }, + additionalProperties: false, + }, + _meta: { + type: "object", + propertyNames: { + type: "string", + }, + additionalProperties: {}, + }, + }, + required: ["type", "resource"], + additionalProperties: false, + }, + ], + }, + }, + structuredContent: {}, + isError: { + type: "boolean", + }, + }, + required: ["content"], + additionalProperties: {}, +} as const; diff --git a/packages/plugins/mcp/src/sdk/call-tool-result-schema.test.ts b/packages/plugins/mcp/src/sdk/call-tool-result-schema.test.ts new file mode 100644 index 0000000000..5016860939 --- /dev/null +++ b/packages/plugins/mcp/src/sdk/call-tool-result-schema.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from "@effect/vitest"; +import { CallToolResultSchema } from "@modelcontextprotocol/core"; + +import { callToolResultJsonSchema } from "./call-tool-result-schema.gen"; + +// The generated copy exists so the RUNTIME never imports +// @modelcontextprotocol/core (module eval costs ~8.6MB of heap per Cloudflare +// isolate; see client-module.ts). Tests may import it freely — this one pins +// the baked schema to the installed package so a core version bump that +// changes CallToolResultSchema fails here until the file is regenerated: +// bun scripts/gen-call-tool-result-schema.ts +describe("call-tool-result-schema.gen", () => { + it("matches the installed @modelcontextprotocol/core schema", () => { + expect(callToolResultJsonSchema).toStrictEqual(CallToolResultSchema.toJSONSchema()); + }); +}); diff --git a/packages/plugins/mcp/src/sdk/client-module.ts b/packages/plugins/mcp/src/sdk/client-module.ts new file mode 100644 index 0000000000..84db301164 --- /dev/null +++ b/packages/plugins/mcp/src/sdk/client-module.ts @@ -0,0 +1,39 @@ +// --------------------------------------------------------------------------- +// Lazy loader for `@modelcontextprotocol/client`. Evaluating the client +// barrel costs ~11MB of heap and tens of ms of module eval (transports, jose, +// eventsource, its bundled protocol schemas) — per ISOLATE, at startup, when +// imported statically into a Cloudflare Worker bundle. Only code paths that +// actually dial an outbound MCP server need any of it, so the module graph is +// loaded on first connect and memoized; isolates that never touch outbound +// MCP never pay (2026-08-25 latency incident: per-isolate baseline memory is +// what collapses isolate reuse). +// +// `mcpClientSdkIfLoaded` exists for synchronous error classification +// (`SdkHttpError.isInstance` etc. in http-status.ts / invoke.ts): every error +// of those classes is CONSTRUCTED by this module, so if it was never loaded +// the cause under inspection cannot be one of them and `undefined` is a +// correct, not a lossy, answer. +// --------------------------------------------------------------------------- + +export type McpClientModule = typeof import("@modelcontextprotocol/client"); +export type McpClientValidatorsModule = + typeof import("@modelcontextprotocol/client/validators/cf-worker"); + +export type McpClientSdk = { + readonly client: McpClientModule; + readonly validators: McpClientValidatorsModule; +}; + +let loaded: McpClientSdk | undefined; +let loading: Promise | undefined; + +export const loadMcpClientSdk = (): Promise => + (loading ??= Promise.all([ + import("@modelcontextprotocol/client"), + import("@modelcontextprotocol/client/validators/cf-worker"), + ]).then(([client, validators]) => { + loaded = { client, validators }; + return loaded; + })); + +export const mcpClientSdkIfLoaded = (): McpClientSdk | undefined => loaded; diff --git a/packages/plugins/mcp/src/sdk/connection.ts b/packages/plugins/mcp/src/sdk/connection.ts index 3035716858..af8c7fc067 100644 --- a/packages/plugins/mcp/src/sdk/connection.ts +++ b/packages/plugins/mcp/src/sdk/connection.ts @@ -1,21 +1,17 @@ -import { - Client, - SSEClientTransport, - StreamableHTTPClientTransport, - type FetchLike, - type OAuthClientProvider, -} from "@modelcontextprotocol/client"; -import { CfWorkerJsonSchemaValidator } from "@modelcontextprotocol/client/validators/cf-worker"; +import type { Client, FetchLike, OAuthClientProvider } from "@modelcontextprotocol/client"; import { Effect, Layer, Predicate, Stream } from "effect"; import { HttpClient, HttpClientRequest } from "effect/unstable/http"; -// NOTE: `StdioClientTransport` is NOT imported eagerly. The upstream module -// (`@modelcontextprotocol/client/stdio`) still imports Node process/stream and -// `cross-spawn` eagerly at evaluation time, which crashes workerd (including -// vitest-pool-workers) with SIGSEGV on module instantiation. Cloud callers set -// `dangerouslyAllowStdioMCP: false` and never reach the stdio branch below; -// prod bundles that DO use stdio load it via a dynamic import inside the -// stdio branch of `createMcpConnector`. +// NOTE: nothing from `@modelcontextprotocol/client` is imported eagerly — +// value access goes through `loadMcpClientSdk` (see client-module.ts for the +// isolate-startup cost rationale). `StdioClientTransport` additionally stays +// out of even the lazy barrel: the upstream `@modelcontextprotocol/client/stdio` +// entry imports Node process/stream and `cross-spawn` at evaluation time, +// which crashes workerd (including vitest-pool-workers) with SIGSEGV on +// module instantiation. Cloud callers set `dangerouslyAllowStdioMCP: false` +// and never reach the stdio branch below; prod bundles that DO use stdio load +// it via the dynamic import inside the stdio branch of `createMcpConnector`. +import { loadMcpClientSdk, type McpClientSdk } from "./client-module"; import type { McpRemoteIntegrationConfig, McpStdioIntegrationConfig } from "./types"; import { @@ -203,12 +199,12 @@ const fetchFromHttpClientLayer = ( // MCP plugin runs inside a Cloudflare Worker (executor.sh). The // cfworker validator does not use code generation and works in every // runtime we ship to. -const createClient = (versionNegotiation?: { readonly mode: "auto" }): Client => - new Client( +const createClient = (sdk: McpClientSdk, versionNegotiation?: { readonly mode: "auto" }): Client => + new sdk.client.Client( { name: "executor-mcp", version: "0.1.0" }, { capabilities: { elicitation: { form: {}, url: {} } }, - jsonSchemaValidator: new CfWorkerJsonSchemaValidator(), + jsonSchemaValidator: new sdk.validators.CfWorkerJsonSchemaValidator(), ...(versionNegotiation === undefined ? {} : { versionNegotiation }), }, ); @@ -249,12 +245,20 @@ const connectionFailure = ( const connectClient = (input: { transport: string; - createTransport: () => Parameters[0]; + createTransport: (sdk: McpClientSdk) => Parameters[0]; versionNegotiation?: { readonly mode: "auto" }; }): Effect.Effect => Effect.gen(function* () { - const client = createClient(input.versionNegotiation); - const transportInstance = input.createTransport(); + const sdk = yield* Effect.tryPromise({ + try: () => loadMcpClientSdk(), + catch: () => + new McpConnectionError({ + transport: input.transport, + message: "Failed to load MCP client module", + }), + }); + const client = createClient(sdk, input.versionNegotiation); + const transportInstance = input.createTransport(sdk); yield* Effect.tryPromise({ // Interruption (an HTTP 499 cancelling a health check, the discovery @@ -345,8 +349,8 @@ export const createMcpConnector = (input: ConnectorInput): McpConnector => { const connectStreamableHttp = connectClient({ transport: "streamable-http", versionNegotiation: { mode: "auto" }, - createTransport: () => - new StreamableHTTPClientTransport(endpoint, { + createTransport: (sdk) => + new sdk.client.StreamableHTTPClientTransport(endpoint, { requestInit, authProvider: input.authProvider, fetch, @@ -355,8 +359,8 @@ export const createMcpConnector = (input: ConnectorInput): McpConnector => { const connectSse = connectClient({ transport: "sse", - createTransport: () => - new SSEClientTransport(endpoint, { + createTransport: (sdk) => + new sdk.client.SSEClientTransport(endpoint, { requestInit, authProvider: input.authProvider, fetch, diff --git a/packages/plugins/mcp/src/sdk/http-status.test.ts b/packages/plugins/mcp/src/sdk/http-status.test.ts index 0344a71986..9bc9c65dac 100644 --- a/packages/plugins/mcp/src/sdk/http-status.test.ts +++ b/packages/plugins/mcp/src/sdk/http-status.test.ts @@ -1,6 +1,13 @@ -import { describe, expect, it } from "@effect/vitest"; +import { beforeAll, describe, expect, it } from "@effect/vitest"; import { InsufficientScopeError, SdkErrorCode, SdkHttpError } from "@modelcontextprotocol/client"; +import { loadMcpClientSdk } from "./client-module"; + +// Classification consults the lazily-loaded client module (client-module.ts); +// in prod every SDK error is preceded by a connect, which loads it. Mirror +// that precondition here — these tests construct SDK errors directly. +beforeAll(() => loadMcpClientSdk()); + // oxlint-disable executor/no-error-constructor -- boundary: these tests reproduce the MCP SDK's own transport rejections, which are built-in Errors import { insufficientScopeFromCause } from "./http-status"; diff --git a/packages/plugins/mcp/src/sdk/http-status.ts b/packages/plugins/mcp/src/sdk/http-status.ts index 541c631f34..97db2d930b 100644 --- a/packages/plugins/mcp/src/sdk/http-status.ts +++ b/packages/plugins/mcp/src/sdk/http-status.ts @@ -10,7 +10,10 @@ import { Option, Schema } from "effect"; import { insufficientScopeFromEmbeddedJson } from "@executor-js/sdk/core"; -import { InsufficientScopeError, SdkHttpError, SseError } from "@modelcontextprotocol/client"; +// The SDK error classes are reached through the lazy loader: any error of +// those classes was constructed by the loaded client module, so "not loaded" +// soundly classifies the cause as not-an-SDK-error (see client-module.ts). +import { mcpClientSdkIfLoaded } from "./client-module"; const SsePostErrorCause = Schema.Struct({ message: Schema.String }); const decodeSsePostErrorCause = Schema.decodeUnknownOption(SsePostErrorCause); @@ -28,8 +31,10 @@ const statusFromSsePostError = (cause: unknown): number | undefined => }); const statusFromTypedTransportError = (cause: unknown): number | undefined => { - if (SdkHttpError.isInstance(cause)) return cause.status; - if (SseError.isInstance(cause)) { + const sdk = mcpClientSdkIfLoaded(); + if (sdk === undefined) return undefined; + if (sdk.client.SdkHttpError.isInstance(cause)) return cause.status; + if (sdk.client.SseError.isInstance(cause)) { const code = cause.code; return code !== undefined && code >= 100 && code <= 599 ? code : undefined; } @@ -54,7 +59,7 @@ const SDK_STEP_UP_EXHAUSTED_RE = /^Server returned 403 insufficient_scope after step-up re-authorization \(retry limit \d+ reached\)$/; export const insufficientScopeFromCause = (cause: unknown): boolean => - InsufficientScopeError.isInstance(cause) || + (mcpClientSdkIfLoaded()?.client.InsufficientScopeError.isInstance(cause) ?? false) || Option.match(decodeSsePostErrorCause(cause), { onNone: () => false, onSome: ({ message }) => diff --git a/packages/plugins/mcp/src/sdk/invoke.test.ts b/packages/plugins/mcp/src/sdk/invoke.test.ts index ce133bfd5f..ee9100fb6b 100644 --- a/packages/plugins/mcp/src/sdk/invoke.test.ts +++ b/packages/plugins/mcp/src/sdk/invoke.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "@effect/vitest"; +import { beforeAll, describe, expect, it } from "@effect/vitest"; import { Effect, Predicate } from "effect"; import { HttpServerResponse } from "effect/unstable/http"; @@ -11,7 +11,13 @@ import { import { ElicitationResponse } from "@executor-js/sdk"; import { serveTestHttpApp } from "@executor-js/sdk/testing"; +import { loadMcpClientSdk } from "./client-module"; import { createMcpConnector, type McpConnection, type McpConnector } from "./connection"; + +// Classification consults the lazily-loaded client module (client-module.ts); +// in prod every SDK error is preceded by a connect, which loads it. Mirror +// that precondition here — these tests construct SDK errors directly. +beforeAll(() => loadMcpClientSdk()); import { McpInvocationError, McpOAuthReauthorizationRequired } from "./errors"; import { invokeMcpTool } from "./invoke"; diff --git a/packages/plugins/mcp/src/sdk/invoke.ts b/packages/plugins/mcp/src/sdk/invoke.ts index 46a08bc663..ad2a12e830 100644 --- a/packages/plugins/mcp/src/sdk/invoke.ts +++ b/packages/plugins/mcp/src/sdk/invoke.ts @@ -16,7 +16,11 @@ import { Cause, Effect, Exit, Option, Predicate, Schema } from "effect"; -import { ProtocolError, ProtocolErrorCode } from "@modelcontextprotocol/client"; +import type { ProtocolError } from "@modelcontextprotocol/client"; + +// SDK error classes come through the lazy loader; by the time a tool call can +// fail, the connect path has always loaded the module (see client-module.ts). +import { mcpClientSdkIfLoaded } from "./client-module"; import { ElicitationId, @@ -59,13 +63,25 @@ export const isUnknownToolMessage = (message: string, toolName: string): boolean ).test(message); }; -const isUnknownToolCause = (cause: unknown, toolName: string): boolean => +const asProtocolError = (cause: unknown): ProtocolError | undefined => { + const sdk = mcpClientSdkIfLoaded(); + if (sdk === undefined) return undefined; // oxlint-disable-next-line executor/no-instanceof-tagged-error -- boundary: MCP SDK surfaces JSON-RPC protocol errors as this Error subclass - cause instanceof ProtocolError && - (cause.code === ProtocolErrorCode.InvalidParams || - cause.code === ProtocolErrorCode.MethodNotFound) && - // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: instanceof narrows to the SDK's ProtocolError, whose message carries the only unknown-tool discriminator the protocol provides - isUnknownToolMessage(cause.message, toolName); + return cause instanceof sdk.client.ProtocolError ? cause : undefined; +}; + +const isUnknownToolCause = (cause: unknown, toolName: string): boolean => { + const sdk = mcpClientSdkIfLoaded(); + const protocolError = asProtocolError(cause); + return ( + sdk !== undefined && + protocolError !== undefined && + (protocolError.code === sdk.client.ProtocolErrorCode.InvalidParams || + protocolError.code === sdk.client.ProtocolErrorCode.MethodNotFound) && + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: the narrowing above reaches the SDK's ProtocolError, whose message carries the only unknown-tool discriminator the protocol provides + isUnknownToolMessage(protocolError.message, toolName) + ); +}; // --------------------------------------------------------------------------- // Elicitation bridge — decode incoming MCP ElicitRequest, route through @@ -198,8 +214,7 @@ const useConnection = ( }); } const status = httpStatusFromCause(cause); - // oxlint-disable-next-line executor/no-instanceof-tagged-error -- boundary: MCP SDK protocol failures are its ProtocolError subclass; transport failures use other error shapes - const protocolFailure = cause instanceof ProtocolError; + const protocolFailure = asProtocolError(cause) !== undefined; return new McpInvocationError({ toolName, message: `MCP tool call failed for ${toolName}`, diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index 375e4e2493..5002994236 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -2,7 +2,8 @@ import { Effect, Layer, Option, Result, Schema } from "effect"; import type { HttpClient } from "effect/unstable/http"; import type { OAuthClientProvider } from "@modelcontextprotocol/client"; -import { CallToolResultSchema } from "@modelcontextprotocol/core"; + +import { callToolResultJsonSchema } from "./call-tool-result-schema.gen"; import { authToolFailure, @@ -399,7 +400,11 @@ type JsonSchemaObject = Record & { readonly properties?: Record; }; -const McpCallToolResultJsonSchema: JsonSchemaObject = CallToolResultSchema.toJSONSchema(); +// Baked at generation time rather than derived from @modelcontextprotocol/core +// at module scope — importing core costs ~8.6MB of heap per Cloudflare isolate +// (see client-module.ts), and this schema is the only thing the plugin needs +// from it outside a live connection. +const McpCallToolResultJsonSchema: JsonSchemaObject = callToolResultJsonSchema; const mcpCallToolResultOutputSchema = (structuredContentSchema?: unknown): JsonSchemaObject => { const defaultStructuredContentSchema = From 1bbd47ec8ab9e8debf48cc691f6b084c2c3e0055 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:52:29 -0700 Subject: [PATCH 4/5] Fix dev-db socket server slot leak and add a wedge watchdog (#1744) --- apps/cloud/scripts/dev-db.ts | 147 ++++++++++++++++-- .../db/dev-db-socket-concurrency.node.test.ts | 51 ++++++ .../@electric-sql%2Fpglite-socket@0.1.4.patch | 8 +- 3 files changed, 188 insertions(+), 18 deletions(-) diff --git a/apps/cloud/scripts/dev-db.ts b/apps/cloud/scripts/dev-db.ts index 2bc447a138..ec671a4d94 100644 --- a/apps/cloud/scripts/dev-db.ts +++ b/apps/cloud/scripts/dev-db.ts @@ -15,6 +15,7 @@ import { PGlite } from "@electric-sql/pglite"; import { PGLiteSocketServer } from "@electric-sql/pglite-socket"; import { drizzle } from "drizzle-orm/pglite"; import { migrate } from "drizzle-orm/pglite/migrator"; +import postgres from "postgres"; const __dirname = dirname(fileURLToPath(import.meta.url)); // Port + data dir default to the dev values but are env-overridable so a second @@ -118,27 +119,32 @@ await migrate(drizzle(db), { migrationsFolder: MIGRATIONS_FOLDER }); // transaction or pipeline affinity they can no longer release. // src/db/dev-db-socket-concurrency.node.test.ts is the regression test for // all of the above. -const server = new PGLiteSocketServer({ - db, - port: PORT, - host: "127.0.0.1", - maxConnections: Number(process.env.DEV_DB_MAX_CONNECTIONS ?? 1000), - // Backstop for pipeline affinity: a client that stalls mid-pipeline (Parse - // sent, no Sync) with its socket still OPEN would hold the queue's handler - // affinity forever and starve every other connection, since affinity only - // releases on detach and detach needs close/error/idle-timeout. In ms; the - // timer resets on every data event. The patch scopes the reap to connections - // actually HOLDING affinity (open pipeline or transaction): an idle-at-rest - // connection is the normal state of a healthy postgres.js pool held by a - // long-lived scope (SSE), and reaping those raced live queries into - // sporadic `write CONNECTION_ENDED` 500s. - idleTimeout: Number(process.env.DEV_DB_IDLE_TIMEOUT_MS ?? 30_000), -}); +const makeServer = () => + new PGLiteSocketServer({ + db, + port: PORT, + host: "127.0.0.1", + maxConnections: Number(process.env.DEV_DB_MAX_CONNECTIONS ?? 1000), + // Backstop for pipeline affinity: a client that stalls mid-pipeline (Parse + // sent, no Sync) with its socket still OPEN would hold the queue's handler + // affinity forever and starve every other connection, since affinity only + // releases on detach and detach needs close/error/idle-timeout. In ms; the + // timer resets on every data event. The patch scopes the reap to connections + // actually HOLDING affinity (open pipeline or transaction): an idle-at-rest + // connection is the normal state of a healthy postgres.js pool held by a + // long-lived scope (SSE), and reaping those raced live queries into + // sporadic `write CONNECTION_ENDED` 500s. + idleTimeout: Number(process.env.DEV_DB_IDLE_TIMEOUT_MS ?? 30_000), + }); +let server = makeServer(); await server.start(); console.log(`[dev-db] Listening on postgresql://postgres:postgres@127.0.0.1:${PORT}/postgres`); +let stopping = false; + const shutdown = async () => { + stopping = true; console.log("\n[dev-db] Shutting down"); await server.stop(); await db.close(); @@ -147,3 +153,112 @@ const shutdown = async () => { process.on("SIGINT", shutdown); process.on("SIGTERM", shutdown); + +// --------------------------------------------------------------------------- +// Wedge watchdog +// --------------------------------------------------------------------------- +// +// Twice the socket server has shipped a state machine that could stop +// answering NEW connections while the process, the port, and PGlite all stayed +// up (the CI e2e "cloud signIn: callback set no session (500)" cascades: every +// in-flight query dies once, then every fresh connection's startup packet +// times out — CONNECT_TIMEOUT — for the rest of the shard). The known paths +// are patched with regression tests, but each recurrence so far has found a +// new path, and a wedged front-end turns ONE infra hiccup into a failure of +// every remaining test in the shard. +// +// So: probe the server the way the app does — a fresh TCP connection, real +// startup handshake, `select 1` — and when several consecutive probes fail, +// dump the server's internals to the boot log and swap in a fresh socket +// server on the same PGlite instance (all state is in PGlite; the front-end is +// stateless, so this drops only already-doomed connections). If a restart +// doesn't restore service, exit non-zero: the boot supervisor logs the exit +// loudly and the run fails fast with an attributable cause instead of minutes +// of anonymous CONNECT_TIMEOUTs. The wedge itself stays visible in the +// server-logs artifact via the [dev-db][watchdog] lines. +const WATCHDOG_INTERVAL_MS = Number(process.env.DEV_DB_WATCHDOG_INTERVAL_MS ?? 5_000); +// 3 consecutive failures ≈ 15s+ of hard unavailability. PGlite serves queries +// in milliseconds; even a deep queue clears in well under one probe interval, +// so consecutive startup failures this sustained only happen wedged. +const WATCHDOG_FAILURES_TO_RESTART = 3; +const WATCHDOG_MAX_RESTARTS = 3; + +const probe = async (): Promise => { + const sql = postgres(`postgres://postgres:postgres@127.0.0.1:${PORT}/postgres`, { + max: 1, + idle_timeout: 0, + connect_timeout: 5, + fetch_types: false, + prepare: false, + onnotice: () => undefined, + }); + try { + // connect_timeout only bounds the handshake; race the query too so a + // post-startup wedge cannot hang the watchdog itself. + await Promise.race([ + sql.unsafe("select 1"), + sleep(10_000).then(() => { + throw new Error("probe query timed out after 10s"); + }), + ]); + } finally { + await sql.end({ timeout: 5 }).catch(() => {}); + } +}; + +const watchdog = async () => { + let consecutiveFailures = 0; + let restarts = 0; + for (;;) { + await sleep(WATCHDOG_INTERVAL_MS); + if (stopping) return; + try { + await probe(); + consecutiveFailures = 0; + } catch (cause) { + consecutiveFailures += 1; + console.error( + `[dev-db][watchdog] probe failed (${consecutiveFailures}/${WATCHDOG_FAILURES_TO_RESTART}): ${String(cause)}`, + ); + if (consecutiveFailures < WATCHDOG_FAILURES_TO_RESTART) continue; + console.error( + `[dev-db][watchdog] socket server wedged; stats: ${JSON.stringify(server.getStats())}`, + ); + if (restarts >= WATCHDOG_MAX_RESTARTS) { + console.error( + `[dev-db][watchdog] still wedged after ${restarts} restarts — giving up so the boot supervisor reports it`, + ); + process.exit(1); + } + restarts += 1; + consecutiveFailures = 0; + console.error( + `[dev-db][watchdog] restarting socket server (${restarts}/${WATCHDOG_MAX_RESTARTS})`, + ); + // stop() itself goes through the query queue (detach rolls back open + // transactions), so a wedge deep enough can hang the restart too — + // bound it and treat that as fatal rather than hanging the watchdog. + const restart = async () => { + await server.stop(); + server = makeServer(); + await server.start(); + }; + try { + await Promise.race([ + restart(), + sleep(15_000).then(() => { + throw new Error("restart timed out after 15s"); + }), + ]); + console.error(`[dev-db][watchdog] socket server restarted`); + } catch (restartCause) { + console.error( + `[dev-db][watchdog] restart failed (${String(restartCause)}) — exiting so the boot supervisor reports it`, + ); + process.exit(1); + } + } + } +}; + +void watchdog(); diff --git a/apps/cloud/src/db/dev-db-socket-concurrency.node.test.ts b/apps/cloud/src/db/dev-db-socket-concurrency.node.test.ts index b9de235fe9..980e5edb32 100644 --- a/apps/cloud/src/db/dev-db-socket-concurrency.node.test.ts +++ b/apps/cloud/src/db/dev-db-socket-concurrency.node.test.ts @@ -259,6 +259,57 @@ describe("dev-db PGlite socket under concurrent connections", () => { }, ); + // Regression for the reap SLOT LEAK: detach(true) removes the socket's + // listeners before destroying it, so a server-initiated teardown (the idle + // backstop) never fired the server's 'close' bookkeeping — the reaped + // handler stayed in the server's handlers set forever, burning one + // maxConnections slot per reap. Enough reaps over a long run and the server + // answers every NEW connection with "Too many connections" while the + // process, the port, and PGlite are all healthy — postgres.js surfaces that + // as the same CONNECT_TIMEOUT cascade as the queue wedges. The server now + // drops the handler when it dispatches its terminal error. + it("reaped handlers release their connection slots", { timeout: 30_000 }, async () => { + const port = 45993; + const db = await PGlite.create(); + const server = new PGLiteSocketServer({ + db, + port, + host: "127.0.0.1", + maxConnections: 2, + idleTimeout: 250, + }); + await server.start(); + + // oxlint-disable-next-line executor/no-try-catch-or-throw -- test boundary: sockets must be closed on every path + try { + // Burn through more reaps than there are slots: each staller opens a + // pipeline and goes silent, so the idle backstop reaps it (the server + // destroys the socket — its 'close' marks that reap complete). + for (let i = 0; i < 3; i++) { + const staller = await openWireClient(port); + staller.write(parseFrame(`select ${i + 1}`)); + await new Promise((res) => staller.once("close", res)); + } + + expect( + server.getStats().activeConnections, + "reaped handlers stay counted against maxConnections", + ).toBe(0); + + const sql = makeClient(port); + // oxlint-disable-next-line executor/no-try-catch-or-throw -- test boundary: sockets must be closed on every path + try { + expect((await sql.unsafe(`select 6 as six`))[0]).toEqual({ six: 6 }); + } finally { + // oxlint-disable-next-line executor/no-promise-catch -- test boundary: a failed teardown must not mask the assertion + await sql.end({ timeout: 5 }).catch(() => {}); + } + } finally { + await server.stop(); + await db.close(); + } + }); + // Regression for the second wedge mode behind the same CI cascade: a client // whose socket dies WHILE its pipeline-opening entry is executing. detach() // clears pipeline affinity before the entry finishes, so the queue then diff --git a/patches/@electric-sql%2Fpglite-socket@0.1.4.patch b/patches/@electric-sql%2Fpglite-socket@0.1.4.patch index 1098228ef0..6e29eb0beb 100644 --- a/patches/@electric-sql%2Fpglite-socket@0.1.4.patch +++ b/patches/@electric-sql%2Fpglite-socket@0.1.4.patch @@ -1,3 +1,6 @@ +diff --git a/node_modules/@electric-sql/pglite-socket/.bun-tag-9dda1ac39b3d8eab b/.bun-tag-9dda1ac39b3d8eab +new file mode 100644 +index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/node_modules/@electric-sql/pglite-socket/.bun-tag-a8fabe72c1056a8f b/.bun-tag-a8fabe72c1056a8f new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 @@ -8,12 +11,13 @@ diff --git a/node_modules/@electric-sql/pglite-socket/.bun-tag-fbab1bb0bfbef953 new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/dist/chunk-NSUMFCRM.js b/dist/chunk-NSUMFCRM.js -index 37d45ebc5150c43c919fbdb1c7fffb51b18fda8c..1cdfaaa69b568a0bed7618fe527ab2c10b1b9403 100644 +index 37d45ebc5150c43c919fbdb1c7fffb51b18fda8c..ab5ff4de0ca78cc8a4b7905fe30f93b655473c14 100644 --- a/dist/chunk-NSUMFCRM.js +++ b/dist/chunk-NSUMFCRM.js @@ -1,3 +1,3 @@ -import{createServer as m}from"net";var b=6e4,c=class{constructor(s,e=!1){this.queue=[];this.processing=!1;this.lastHandlerId=null;this.db=s,this.debug=e}log(s,...e){this.debug&&console.log(`[QueryQueueManager] ${s}`,...e)}async enqueue(s,e,i){return new Promise((t,r)=>{let o={handlerId:s,message:e,resolve:t,reject:r,timestamp:Date.now(),onData:i};this.queue.push(o),this.log(`enqueued query from handler #${s}, queue size: ${this.queue.length}`),this.processing||this.processQueue()})}async processQueue(){if(!(this.processing||this.queue.length===0)){for(this.processing=!0;this.queue.length>0;){let s;if(this.db.isInTransaction()&&this.lastHandlerId){let t=this.queue.findIndex(r=>r.handlerId===this.lastHandlerId);t===-1?(this.log("transaction started, but no query from the same handler id found in queue",this.lastHandlerId),s=null):s=this.queue.splice(t,1)[0]}else s=this.queue.shift();if(!s)break;let e=Date.now()-s.timestamp;this.log(`processing query from handler #${s.handlerId} (waited ${e}ms)`);let i=0;try{await this.db.runExclusive(async()=>await this.db.execProtocolRawStream(s.message,{onRawData:t=>{i+=t.length,s.onData(t)}}))}catch(t){this.log(`query from handler #${s.handlerId} failed:`,t),s.reject(t);return}this.log(`query from handler #${s.handlerId} completed, ${i} bytes`),this.lastHandlerId=s.handlerId,s.resolve(i)}this.processing=!1,this.log("queue processing complete, queue length is",this.queue.length)}}getQueueLength(){return this.queue.length}clearQueueForHandler(s){let e=this.queue.length;this.queue=this.queue.filter(t=>t.handlerId===s?(t.reject(new Error("Handler disconnected")),!1):!0);let i=e-this.queue.length;i>0&&this.log(`cleared ${i} queries for handler #${s}`)}async clearTransactionIfNeeded(s){this.db.isInTransaction()&&this.lastHandlerId===s&&(await this.db.exec("ROLLBACK"),this.lastHandlerId=null,await this.processQueue())}},l=class l extends EventTarget{constructor(e){super();this.socket=null;this.active=!1;this.messageBuffer=Buffer.alloc(0);this.lastActivityTime=Date.now();this.queryQueue=e.queryQueue,this.closeOnDetach=e.closeOnDetach??!1,this.inspect=e.inspect??!1,this.debug=e.debug??!1,this.idleTimeout=e.idleTimeout??0,this.id=l.nextHandlerId++,this.log("constructor: created new handler")}get handlerId(){return this.id}log(e,...i){this.debug&&console.log(`[PGLiteSocketHandler#${this.id}] ${e}`,...i)}async attach(e){if(this.log(`attach: attaching socket from ${e.remoteAddress}:${e.remotePort}`),this.socket)throw new Error("Socket already attached");return this.socket=e,this.active=!0,this.lastActivityTime=Date.now(),e.setNoDelay(!0),this.idleTimeout>0&&this.resetIdleTimer(),this.log("attach: setting up socket event handlers"),e.on("data",i=>{this.lastActivityTime=Date.now(),this.resetIdleTimer(),setImmediate(async()=>{try{await this.handleData(i)}catch(t){this.log("socket on data error: ",t),this.handleError(t)}})}),e.on("error",i=>{setImmediate(()=>this.handleError(i))}),e.on("close",()=>{setImmediate(()=>this.handleClose())}),this.log("attach: socket handler ready"),this}resetIdleTimer(){this.idleTimeout<=0||(this.idleTimer&&clearTimeout(this.idleTimer),this.idleTimer=setTimeout(()=>{let e=Date.now()-this.lastActivityTime;this.log(`idle timeout after ${e}ms`),this.handleError(new Error("Idle timeout"))},this.idleTimeout))}async detach(e){if(this.log(`detach: detaching socket, close=${e??this.closeOnDetach}`),this.idleTimer&&(clearTimeout(this.idleTimer),this.idleTimer=void 0),this.queryQueue.clearQueueForHandler(this.id),await this.queryQueue.clearTransactionIfNeeded(this.id),!this.socket)return this.log("detach: no socket attached, nothing to do"),this;if(this.socket.removeAllListeners("data"),this.socket.removeAllListeners("error"),this.socket.removeAllListeners("close"),(e??this.closeOnDetach)&&this.socket.writable){this.log("detach: closing socket");try{this.socket.end(),this.socket.destroy()}catch(i){this.log("detach: error closing socket:",i)}}return this.socket=null,this.active=!1,this.messageBuffer=Buffer.alloc(0),this.log("detach: handler cleaned up"),this}get isAttached(){return this.socket!==null}async handleData(e){if(!this.socket||!this.active)return this.log("handleData: no active socket, ignoring data"),0;this.log(`handleData: received ${e.length} bytes`),this.messageBuffer=Buffer.concat([this.messageBuffer,e]),this.inspectData("incoming",e);try{let i=0;for(;this.messageBuffer.length>0;){let t=0,r=!1;if(this.messageBuffer.length>=4){let n=this.messageBuffer.readInt32BE(0);if(this.messageBuffer.length>=8){let a=this.messageBuffer.readInt32BE(4);(a===196608||a===196608)&&(t=n,r=this.messageBuffer.length>=t)}!r&&this.messageBuffer.length>=5&&(t=1+this.messageBuffer.readInt32BE(1),r=this.messageBuffer.length>=t)}if(!r||t===0){this.log(`handleData: incomplete message, buffering ${this.messageBuffer.length} bytes`);break}let o=this.messageBuffer.slice(0,t);if(this.messageBuffer=this.messageBuffer.slice(t),this.log(`handleData: processing message of ${o.length} bytes`),!this.active||!this.socket){this.log("handleData: socket no longer active, stopping processing");break}let h;if(await this.queryQueue.enqueue(this.id,new Uint8Array(o),n=>{this.log(`handleData: received ${n.length} bytes from PGlite`),this.inspectData("outgoing",n),n.length>0&&this.socket&&this.socket.writable&&this.active&&(this.log("handleData: writing response to socket"),this.socket?.writable?this.socket.write(Buffer.from(n),a=>{a?(this.log("handleData: error writing to socket:",a),h=a):this.log(`handleData: socket sent: ${n.length} bytes`)}):this.log("handleData: socket no longer writable")),i+=n.length}),h)throw h}return this.dispatchEvent(new CustomEvent("data",{detail:{incoming:e.length,outgoing:i}})),i}catch(i){throw this.log("handleData: error processing data:",i),i}}handleError(e){if(!this.active){this.log("handleError: handler not active, ignoring error");return}e.message?.includes("ECONNRESET")?this.log("handleError: client disconnected (ECONNRESET) - normal behavior"):e.message?.includes("Idle timeout")?this.log("handleError: connection idle timeout"):this.log("handleError:",e),this.active=!1,this.dispatchEvent(new CustomEvent("error",{detail:e})),this.detach(!0)}handleClose(){this.log("handleClose: socket closed"),this.active=!1,this.dispatchEvent(new CustomEvent("close")),this.detach(!1)}inspectData(e,i){if(this.inspect){console.log("-".repeat(75)),console.log(e==="incoming"?"-> incoming":"<- outgoing",i.length,"bytes");for(let t=0;t=32&&a<=126?String.fromCharCode(a):"."}console.log(`${t.toString(16).padStart(8,"0")} ${o} ${h}`)}}}};l.nextHandlerId=1;var d=l,u=class extends EventTarget{constructor(e){super();this.server=null;this.active=!1;this.handlers=new Set;this.db=e.db,e.path?this.path=e.path:(typeof e.port=="number"?this.port=e.port??e.port:this.port=5432,this.host=e.host||"127.0.0.1"),this.inspect=e.inspect??!1,this.debug=e.debug??!1,this.idleTimeout=e.idleTimeout??0,this.maxConnections=e.maxConnections??1,this.queryQueue=new c(this.db,this.debug),this.log(`constructor: created server on ${this.getServerConn()}`),this.log(`constructor: max connections: ${this.maxConnections}`),this.idleTimeout>0&&this.log(`constructor: idle timeout: ${this.idleTimeout}ms`)}log(e,...i){this.debug&&console.log(`[PGLiteSocketServer] ${e}`,...i)}async start(){if(this.log(`start: starting server on ${this.getServerConn()}`),this.server)throw new Error("Socket server already started");return await this.db.waitReady,this.active=!0,this.server=m(e=>{setImmediate(()=>this.handleConnection(e))}),this.server.maxConnections=this.maxConnections,new Promise((e,i)=>{if(!this.server)return i(new Error("Server not initialized"));if(this.server.on("error",t=>{this.log("start: server error:",t),this.dispatchEvent(new CustomEvent("error",{detail:t})),this.active||i(t)}),this.path)this.server.listen(this.path,()=>{this.log(`start: server listening on ${this.getServerConn()}`),this.dispatchEvent(new CustomEvent("listening",{detail:{path:this.path}})),e()});else{let t=this.server;t.listen(this.port,this.host,()=>{let r=t.address();if(r===null||typeof r!="object")throw Error("Expected address info");this.port=r.port,this.log(`start: server listening on ${this.getServerConn()}`),this.dispatchEvent(new CustomEvent("listening",{detail:{port:this.port,host:this.host}})),e()})}})}getServerConn(){return this.path?this.path:`${this.host}:${this.port}`}async stop(){this.log("stop: stopping server"),this.active=!1,this.log(`stop: detaching ${this.handlers.size} handlers`);for(let e of this.handlers)e.detach(!0);return this.handlers.clear(),this.server?new Promise(e=>{if(!this.server)return e();this.server.close(()=>{this.log("stop: server closed"),this.server=null,this.dispatchEvent(new CustomEvent("close")),e()})}):(this.log("stop: server not running, nothing to do"),Promise.resolve())}async handleConnection(e){let i={clientAddress:e.remoteAddress||"unknown",clientPort:e.remotePort||0};if(this.log(`handleConnection: new connection from ${i.clientAddress}:${i.clientPort}`),this.log(`handleConnection: active connections: ${this.handlers.size}, queued queries: ${this.queryQueue.getQueueLength()}`),!this.active){this.log("handleConnection: server not active, closing connection");try{e.end()}catch(r){this.log("handleConnection: error closing socket:",r)}return}if(this.handlers.size>=this.maxConnections){this.log("handleConnection: max connections reached, rejecting"),e.write(Buffer.from(`Too many connections +-`)),e.end();return}let t=new d({queryQueue:this.queryQueue,closeOnDetach:!0,inspect:this.inspect,debug:this.debug,idleTimeout:this.idleTimeout});this.handlers.add(t),t.addEventListener("error",r=>{let o=r.detail;o?.message?.includes("ECONNRESET")?this.log(`handler #${t.handlerId}: client disconnected (ECONNRESET)`):o?.message?.includes("Idle timeout")?this.log(`handler #${t.handlerId}: idle timeout`):this.log(`handler #${t.handlerId}: error:`,o)}),t.addEventListener("close",()=>{this.log(`handler #${t.handlerId}: closed`),this.handlers.delete(t),this.log(`handleConnection: active connections: ${this.handlers.size}`)});try{await t.attach(e),this.dispatchEvent(new CustomEvent("connection",{detail:i}))}catch(r){this.log("handleConnection: error attaching socket:",r),this.handlers.delete(t),this.dispatchEvent(new CustomEvent("error",{detail:r}));try{e.end()}catch(o){this.log("handleConnection: error closing socket:",o)}}}getStats(){return{activeConnections:this.handlers.size,queuedQueries:this.queryQueue.getQueueLength(),maxConnections:this.maxConnections}}};export{b as a,d as b,u as c}; +import{createServer as m}from"net";var b=6e4,c=class{constructor(s,e=!1){this.queue=[];this.processing=!1;this.lastHandlerId=null;this.pipelineHandlerId=null;this.dead=new Set();this.db=s,this.debug=e}log(s,...e){this.debug&&console.log(`[QueryQueueManager] ${s}`,...e)}async enqueue(s,e,i,S=!0){return new Promise((t,r)=>{let o={handlerId:s,message:e,resolve:t,reject:r,timestamp:Date.now(),onData:i,closes:S};this.queue.push(o),this.log(`enqueued query from handler #${s}, queue size: ${this.queue.length}`),this.processing||this.processQueue()})}async processQueue(){if(!(this.processing||this.queue.length===0)){for(this.processing=!0;this.queue.length>0;){let s;let __affine=this.db.isInTransaction()&&this.lastHandlerId?this.lastHandlerId:this.pipelineHandlerId;if(__affine&&this.dead.has(__affine)){this.log("affinity held by detached handler, recovering",__affine);if(this.db.isInTransaction()&&this.lastHandlerId===__affine){await this.db.exec("ROLLBACK").catch(()=>{});this.lastHandlerId=null}if(this.pipelineHandlerId===__affine){this.pipelineHandlerId=null;await this.db.runExclusive(async()=>await this.db.execProtocolRawStream(new Uint8Array([83,0,0,0,4]),{onRawData:()=>{}})).catch(()=>{})}continue}if(__affine){let t=this.queue.findIndex(r=>r.handlerId===__affine);t===-1?(this.log("affinity held, waiting for handler",__affine),s=null):s=this.queue.splice(t,1)[0]}else s=this.queue.shift();if(!s)break;let e=Date.now()-s.timestamp;this.log(`processing query from handler #${s.handlerId} (waited ${e}ms)`);let i=0;try{await this.db.runExclusive(async()=>await this.db.execProtocolRawStream(s.message,{onRawData:t=>{i+=t.length,s.onData(t)}}))}catch(t){this.log(`query from handler #${s.handlerId} failed:`,t),s.reject(t),this.pipelineHandlerId=null;continue}this.log(`query from handler #${s.handlerId} completed, ${i} bytes`),this.lastHandlerId=s.handlerId,this.pipelineHandlerId=s.closes?null:s.handlerId,s.resolve(i)}this.processing=!1,this.log("queue processing complete, queue length is",this.queue.length)}}getQueueLength(){return this.queue.length}holdsAffinity(s){return this.pipelineHandlerId===s||this.db.isInTransaction()&&this.lastHandlerId===s}clearQueueForHandler(s){this.dead.add(s);let e=this.queue.length;this.queue=this.queue.filter(t=>t.handlerId===s?(t.reject(new Error("Handler disconnected")),!1):!0);let i=e-this.queue.length;i>0&&this.log(`cleared ${i} queries for handler #${s}`)}async clearPipelineIfNeeded(s){this.pipelineHandlerId===s&&(this.pipelineHandlerId=null,await this.db.runExclusive(async()=>await this.db.execProtocolRawStream(new Uint8Array([83,0,0,0,4]),{onRawData:()=>{}})).catch(()=>{}),this.processQueue())}async clearTransactionIfNeeded(s){this.db.isInTransaction()&&this.lastHandlerId===s&&(await this.db.exec("ROLLBACK"),this.lastHandlerId=null,await this.processQueue())}},l=class l extends EventTarget{constructor(e){super();this.socket=null;this.active=!1;this.messageBuffer=Buffer.alloc(0);this.lastActivityTime=Date.now();this.queryQueue=e.queryQueue,this.closeOnDetach=e.closeOnDetach??!1,this.inspect=e.inspect??!1,this.debug=e.debug??!1,this.idleTimeout=e.idleTimeout??0,this.id=l.nextHandlerId++,this.log("constructor: created new handler")}get handlerId(){return this.id}log(e,...i){this.debug&&console.log(`[PGLiteSocketHandler#${this.id}] ${e}`,...i)}async attach(e){if(this.log(`attach: attaching socket from ${e.remoteAddress}:${e.remotePort}`),this.socket)throw new Error("Socket already attached");return this.socket=e,this.active=!0,this.lastActivityTime=Date.now(),e.setNoDelay(!0),this.idleTimeout>0&&this.resetIdleTimer(),this.log("attach: setting up socket event handlers"),e.on("data",i=>{this.lastActivityTime=Date.now(),this.resetIdleTimer(),setImmediate(async()=>{try{await this.handleData(i)}catch(t){this.log("socket on data error: ",t),this.handleError(t)}})}),e.on("error",i=>{setImmediate(()=>this.handleError(i))}),e.on("close",()=>{setImmediate(()=>this.handleClose())}),this.log("attach: socket handler ready"),this}resetIdleTimer(){this.idleTimeout<=0||(this.idleTimer&&clearTimeout(this.idleTimer),this.idleTimer=setTimeout(()=>{if(!this.queryQueue.holdsAffinity(this.id)){this.resetIdleTimer();return}let e=Date.now()-this.lastActivityTime;this.log(`idle timeout after ${e}ms`),this.handleError(new Error("Idle timeout"))},this.idleTimeout))}async detach(e){if(this.log(`detach: detaching socket, close=${e??this.closeOnDetach}`),this.idleTimer&&(clearTimeout(this.idleTimer),this.idleTimer=void 0),this.queryQueue.clearQueueForHandler(this.id),await this.queryQueue.clearTransactionIfNeeded(this.id),await this.queryQueue.clearPipelineIfNeeded(this.id),!this.socket)return this.log("detach: no socket attached, nothing to do"),this;if(this.socket.removeAllListeners("data"),this.socket.removeAllListeners("error"),this.socket.removeAllListeners("close"),(e??this.closeOnDetach)&&this.socket.writable){this.log("detach: closing socket");try{this.socket.end(),this.socket.destroy()}catch(i){this.log("detach: error closing socket:",i)}}return this.socket=null,this.active=!1,this.messageBuffer=Buffer.alloc(0),this.log("detach: handler cleaned up"),this}get isAttached(){return this.socket!==null}async handleData(e){if(!this.socket||!this.active)return this.log("handleData: no active socket, ignoring data"),0;this.log(`handleData: received ${e.length} bytes`),this.messageBuffer=Buffer.concat([this.messageBuffer,e]),this.inspectData("incoming",e);try{let i=0;const __frames=[];for(;this.messageBuffer.length>0;){let t=0,r=!1;if(this.messageBuffer.length>=4){let n=this.messageBuffer.readInt32BE(0);if(this.messageBuffer.length>=8){let a=this.messageBuffer.readInt32BE(4);(a===196608||a===196608)&&(t=n,r=this.messageBuffer.length>=t)}!r&&this.messageBuffer.length>=5&&(t=1+this.messageBuffer.readInt32BE(1),r=this.messageBuffer.length>=t)}if(!r||t===0){this.log(`handleData: incomplete message, buffering ${this.messageBuffer.length} bytes`);break}let o=this.messageBuffer.slice(0,t);if(this.messageBuffer=this.messageBuffer.slice(t),this.log(`handleData: processing message of ${o.length} bytes`),!this.active||!this.socket){this.log("handleData: socket no longer active, stopping processing");break}__frames.push(o)}if(__frames.length===0)return this.dispatchEvent(new CustomEvent("data",{detail:{incoming:e.length,outgoing:i}})),i;{let o=__frames.length===1?__frames[0]:Buffer.concat(__frames);const __lastF=__frames[__frames.length-1];const __lt=__lastF[0]>=65?__lastF[0]:null;const __closes=__lt===null||__lt===83||__lt===81||__lt===88;let h;if(await this.queryQueue.enqueue(this.id,new Uint8Array(o),n=>{this.log(`handleData: received ${n.length} bytes from PGlite`),this.inspectData("outgoing",n),n.length>0&&this.socket&&this.socket.writable&&this.active&&(this.log("handleData: writing response to socket"),this.socket?.writable?this.socket.write(Buffer.from(n),a=>{a?(this.log("handleData: error writing to socket:",a),h=a):this.log(`handleData: socket sent: ${n.length} bytes`)}):this.log("handleData: socket no longer writable")),i+=n.length},__closes),h)throw h}return this.dispatchEvent(new CustomEvent("data",{detail:{incoming:e.length,outgoing:i}})),i}catch(i){throw this.log("handleData: error processing data:",i),i}}handleError(e){if(!this.active){this.log("handleError: handler not active, ignoring error");return}e.message?.includes("ECONNRESET")?this.log("handleError: client disconnected (ECONNRESET) - normal behavior"):e.message?.includes("Idle timeout")?this.log("handleError: connection idle timeout"):this.log("handleError:",e),this.active=!1,this.dispatchEvent(new CustomEvent("error",{detail:e})),this.detach(!0).catch(()=>{})}handleClose(){this.log("handleClose: socket closed"),this.active=!1,this.dispatchEvent(new CustomEvent("close")),this.detach(!1).catch(()=>{})}inspectData(e,i){if(this.inspect){console.log("-".repeat(75)),console.log(e==="incoming"?"-> incoming":"<- outgoing",i.length,"bytes");for(let t=0;t=32&&a<=126?String.fromCharCode(a):"."}console.log(`${t.toString(16).padStart(8,"0")} ${o} ${h}`)}}}};l.nextHandlerId=1;var d=l,u=class extends EventTarget{constructor(e){super();this.server=null;this.active=!1;this.handlers=new Set;this.db=e.db,e.path?this.path=e.path:(typeof e.port=="number"?this.port=e.port??e.port:this.port=5432,this.host=e.host||"127.0.0.1"),this.inspect=e.inspect??!1,this.debug=e.debug??!1,this.idleTimeout=e.idleTimeout??0,this.maxConnections=e.maxConnections??1,this.queryQueue=new c(this.db,this.debug),this.log(`constructor: created server on ${this.getServerConn()}`),this.log(`constructor: max connections: ${this.maxConnections}`),this.idleTimeout>0&&this.log(`constructor: idle timeout: ${this.idleTimeout}ms`)}log(e,...i){this.debug&&console.log(`[PGLiteSocketServer] ${e}`,...i)}async start(){if(this.log(`start: starting server on ${this.getServerConn()}`),this.server)throw new Error("Socket server already started");return await this.db.waitReady,this.active=!0,this.server=m(e=>{setImmediate(()=>this.handleConnection(e))}),this.server.maxConnections=this.maxConnections,new Promise((e,i)=>{if(!this.server)return i(new Error("Server not initialized"));if(this.server.on("error",t=>{this.log("start: server error:",t),this.dispatchEvent(new CustomEvent("error",{detail:t})),this.active||i(t)}),this.path)this.server.listen(this.path,()=>{this.log(`start: server listening on ${this.getServerConn()}`),this.dispatchEvent(new CustomEvent("listening",{detail:{path:this.path}})),e()});else{let t=this.server;t.listen(this.port,this.host,()=>{let r=t.address();if(r===null||typeof r!="object")throw Error("Expected address info");this.port=r.port,this.log(`start: server listening on ${this.getServerConn()}`),this.dispatchEvent(new CustomEvent("listening",{detail:{port:this.port,host:this.host}})),e()})}})}getServerConn(){return this.path?this.path:`${this.host}:${this.port}`}async stop(){this.log("stop: stopping server"),this.active=!1,this.log(`stop: detaching ${this.handlers.size} handlers`);for(let e of this.handlers)e.detach(!0).catch(()=>{});return this.handlers.clear(),this.server?new Promise(e=>{if(!this.server)return e();this.server.close(()=>{this.log("stop: server closed"),this.server=null,this.dispatchEvent(new CustomEvent("close")),e()})}):(this.log("stop: server not running, nothing to do"),Promise.resolve())}async handleConnection(e){let i={clientAddress:e.remoteAddress||"unknown",clientPort:e.remotePort||0};if(this.log(`handleConnection: new connection from ${i.clientAddress}:${i.clientPort}`),this.log(`handleConnection: active connections: ${this.handlers.size}, queued queries: ${this.queryQueue.getQueueLength()}`),!this.active){this.log("handleConnection: server not active, closing connection");try{e.end()}catch(r){this.log("handleConnection: error closing socket:",r)}return}if(this.handlers.size>=this.maxConnections){this.log("handleConnection: max connections reached, rejecting"),e.write(Buffer.from(`Too many connections - `)),e.end();return}let t=new d({queryQueue:this.queryQueue,closeOnDetach:!0,inspect:this.inspect,debug:this.debug,idleTimeout:this.idleTimeout});this.handlers.add(t),t.addEventListener("error",r=>{let o=r.detail;o?.message?.includes("ECONNRESET")?this.log(`handler #${t.handlerId}: client disconnected (ECONNRESET)`):o?.message?.includes("Idle timeout")?this.log(`handler #${t.handlerId}: idle timeout`):this.log(`handler #${t.handlerId}: error:`,o)}),t.addEventListener("close",()=>{this.log(`handler #${t.handlerId}: closed`),this.handlers.delete(t),this.log(`handleConnection: active connections: ${this.handlers.size}`)});try{await t.attach(e),this.dispatchEvent(new CustomEvent("connection",{detail:i}))}catch(r){this.log("handleConnection: error attaching socket:",r),this.handlers.delete(t),this.dispatchEvent(new CustomEvent("error",{detail:r}));try{e.end()}catch(o){this.log("handleConnection: error closing socket:",o)}}}getStats(){return{activeConnections:this.handlers.size,queuedQueries:this.queryQueue.getQueueLength(),maxConnections:this.maxConnections}}};export{b as a,d as b,u as c}; ++`)),e.end();return}let t=new d({queryQueue:this.queryQueue,closeOnDetach:!0,inspect:this.inspect,debug:this.debug,idleTimeout:this.idleTimeout});this.handlers.add(t),t.addEventListener("error",r=>{let o=r.detail;o?.message?.includes("ECONNRESET")?this.log(`handler #${t.handlerId}: client disconnected (ECONNRESET)`):o?.message?.includes("Idle timeout")?this.log(`handler #${t.handlerId}: idle timeout`):this.log(`handler #${t.handlerId}: error:`,o);this.handlers.delete(t)}),t.addEventListener("close",()=>{this.log(`handler #${t.handlerId}: closed`),this.handlers.delete(t),this.log(`handleConnection: active connections: ${this.handlers.size}`)});try{await t.attach(e),this.dispatchEvent(new CustomEvent("connection",{detail:i}))}catch(r){this.log("handleConnection: error attaching socket:",r),this.handlers.delete(t),this.dispatchEvent(new CustomEvent("error",{detail:r}));try{e.end()}catch(o){this.log("handleConnection: error closing socket:",o)}}}getStats(){return{activeConnections:this.handlers.size,queuedQueries:this.queryQueue.getQueueLength(),maxConnections:this.maxConnections}}};export{b as a,d as b,u as c}; //# sourceMappingURL=chunk-NSUMFCRM.js.map \ No newline at end of file From eb34d049d76fb8aab8b0f765bac7aadb12f56950 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Tue, 25 Aug 2026 19:37:58 -0700 Subject: [PATCH 5/5] Serve the cloud console as an SPA with a prerendered shell (#1748) --- .../src/auth/{ssr-gate.ts => doc-gate.ts} | 39 ++++----- apps/cloud/src/auth/last-org-cookie.ts | 2 +- apps/cloud/src/routes/__root.tsx | 51 +++--------- apps/cloud/src/routes/bare/login.tsx | 2 +- apps/cloud/src/start.ts | 2 +- apps/cloud/vite.config.ts | 10 +++ e2e/cloud/connect-card-origin.test.ts | 55 +++++++++++++ e2e/cloud/connect-card-ssr-origin.test.ts | 79 ------------------- e2e/cloud/unauthenticated-skeleton.test.ts | 13 ++- e2e/setup/cloud.boot.ts | 12 +++ 10 files changed, 118 insertions(+), 147 deletions(-) rename apps/cloud/src/auth/{ssr-gate.ts => doc-gate.ts} (89%) create mode 100644 e2e/cloud/connect-card-origin.test.ts delete mode 100644 e2e/cloud/connect-card-ssr-origin.test.ts diff --git a/apps/cloud/src/auth/ssr-gate.ts b/apps/cloud/src/auth/doc-gate.ts similarity index 89% rename from apps/cloud/src/auth/ssr-gate.ts rename to apps/cloud/src/auth/doc-gate.ts index a85f9ffe1e..16d6145bc9 100644 --- a/apps/cloud/src/auth/ssr-gate.ts +++ b/apps/cloud/src/auth/doc-gate.ts @@ -1,19 +1,23 @@ // --------------------------------------------------------------------------- -// SSR auth gate — the server-side session check for DOCUMENT requests. +// Document auth gate — the server-side session check for DOCUMENT requests. // // The sealed `wos-session` cookie is verified right here in the worker // (unseal + JWT check against cached JWKS — no per-request WorkOS round trip -// except token refresh), so by the time the SPA is served the server KNOWS -// who it's serving: +// except token refresh), so by the time the SPA shell is served the server +// KNOWS who it's serving: // // - signed out → 302 /login (carrying ?returnTo=) before any app HTML exists // - org-less → 302 /create-org (onboarding owns those sessions) -// - signed in → the document is served WITH the verified identity: the -// auth-hint travels to the SSR render via request-middleware context (the -// root loader picks it up), and is minted as a cookie when the browser -// doesn't hold a current one — so the very first paint is the real app -// shell, never a skeleton. The hint is display-only; /account/me remains -// the authority and the client keeps it fresh from then on. +// - signed in → the prerendered shell is served, and the auth-hint cookie is +// minted when the browser doesn't hold a current one — the client seeds its +// auth state from that cookie right after mount (there is no per-request +// render to dehydrate it into). The hint is display-only; /account/me +// remains the authority and the client keeps it fresh from then on. +// +// Beyond redirects, this gate is load-bearing for SESSION LIFETIME: WorkOS +// refresh tokens are single-use, and a verify that rotates the sealed session +// must deliver the new cookie to the browser or the next expiry logs the user +// out. Document navigations are where that rotation lands. // // Scope: GET/HEAD requests that are document navigations (sec-fetch-dest / // accept), excluding app-owned paths (/api, /mcp — they answer for themselves @@ -39,7 +43,6 @@ import { parseCookie } from "./cookies"; import { LAST_ORG_COOKIE } from "./last-org-cookie"; import { sealedSessionDisplayName } from "./middleware"; import { authorizeOrganizationSelector } from "./organization"; -import { browserOriginFromRequest } from "./request-origin"; import { loginPath, safeReturnTo } from "./return-to"; import { ONBOARDING_PATHS, PUBLIC_PATHS } from "./route-paths"; import { WorkOSClient } from "./workos"; @@ -279,19 +282,11 @@ export const authGateMiddleware = createMiddleware({ type: "request" }).server( } } - // Serve the document WITH the verified identity: the hint rides to the - // SSR render through middleware context (the root loader reads it), so - // the server paints the real authenticated shell — no loading state, no - // skeleton. The request origin rides along too: it's what the connect - // card's MCP URL is built from, and the server knows it (the SPA only - // learns `window.location.origin` after mount), so passing it here lets - // SSR render the real `https://…//mcp` instead of the client-side - // `http://127.0.0.1:4000` default — which would otherwise flash until - // hydration corrected it. Set-cookie writes ride on the rendered response. + // Serve the shell, minting the auth-hint cookie when the browser lacks a + // current one so the client's post-mount cookie read seeds the verified + // identity. Set-cookie writes ride on the shell response. const { hint, mint } = await resolveAuthHint(session, cookieHeader); - const result = await next({ - context: { authHint: hint, origin: browserOriginFromRequest(request) }, - }); + const result = await next(); if (!mint && !session.refreshedSession) return result; const response = new Response(result.response.body, result.response); diff --git a/apps/cloud/src/auth/last-org-cookie.ts b/apps/cloud/src/auth/last-org-cookie.ts index dde7c92848..ef6242604f 100644 --- a/apps/cloud/src/auth/last-org-cookie.ts +++ b/apps/cloud/src/auth/last-org-cookie.ts @@ -8,7 +8,7 @@ // cookie fills the gap: the client records the slug of the org it's verifiably // viewing, and the two bare-entry deciders honor it — // -// - the SSR auth gate redirects bare document paths onto it (ssr-gate.ts) +// - the document auth gate redirects bare document paths onto it (doc-gate.ts) // - the login callback prefers it when picking the org for a fresh session // with a bare returnTo (handlers.ts) // diff --git a/apps/cloud/src/routes/__root.tsx b/apps/cloud/src/routes/__root.tsx index 474eb8045c..517d53b0b1 100644 --- a/apps/cloud/src/routes/__root.tsx +++ b/apps/cloud/src/routes/__root.tsx @@ -23,7 +23,6 @@ import { Toaster } from "@executor-js/react/components/sonner"; import { ExecutorPluginsProvider } from "@executor-js/sdk/client"; import { ArtifactRendererProvider } from "@executor-js/react/api/artifact-renderer"; import { plugins as clientPlugins } from "virtual:executor/plugins-client"; -import type { AuthHint } from "@executor-js/react/multiplayer/auth-hint"; import { AuthProvider, useAuth } from "../web/auth"; import { loginPath } from "../auth/return-to"; import { ONBOARDING_PATHS, PUBLIC_PATHS } from "../auth/route-paths"; @@ -111,27 +110,6 @@ function NotFoundPage() { export const Route = createRootRoute({ notFoundComponent: NotFoundPage, - // What the SSR gate attached to this document request (ssr-gate.ts → - // middleware context → serverContext). Loader data is dehydrated, so the - // client's first render sees the SAME values the server rendered with — the - // two can't disagree: - // - authHint: the verified identity, seeding AuthProvider's initial state. - // - origin: the request origin, seeding the server connection so the - // connect-card MCP URL SSRs as the real origin instead of the - // 127.0.0.1 client-side default (which would flash to the real - // value at hydration). - // Client-side re-runs have no serverContext and return null; both consumers - // fall back gracefully (the hint is already held, the origin to the - // window-derived global). - loader: (opts) => { - const serverContext = ( - opts as { serverContext?: { authHint?: AuthHint | null; origin?: string } } - ).serverContext; - return { - authHint: serverContext?.authHint ?? null, - origin: serverContext?.origin ?? null, - }; - }, head: () => ({ meta: [ { charSet: "utf-8" }, @@ -171,12 +149,15 @@ function RootDocument({ children }: { children: React.ReactNode }) { } function RootComponent() { - const { authHint, origin } = Route.useLoaderData(); + // SPA mode: no per-request server render, so nothing is dehydrated. Auth + // seeds from the client-readable hint cookie one frame after mount + // (AuthProvider's own fallback), and origin-derived UI reads the + // window-derived global. return ( - - + + @@ -213,7 +194,7 @@ function ShellErrorFallback() { ); } -function AuthGate({ ssrOrigin }: { ssrOrigin: string | null }) { +function AuthGate() { const auth = useAuth(); const location = useLocation(); const navigate = useNavigate(); @@ -265,11 +246,11 @@ function AuthGate({ ssrOrigin }: { ssrOrigin: string | null }) { } // Every state that isn't "authenticated with an org, on a page that wants - // the shell" is a moment between redirects or an edge the gates make - // near-impossible (a verified user whose hint hasn't seeded yet). Neutral + // the shell" is a moment between redirects, or the one frame between mount + // and the hint cookie seeding (SPA mode reads it in an effect). Neutral // blank — the one placeholder that's correct whatever happens next. The // app-shell skeleton this file used to render here is exactly the - // wrong-UI flash the SSR gate + hint exist to prevent. + // wrong-UI flash the document gate + hint exist to prevent. if (auth.status === "loading" || auth.status === "unauthenticated") { return ; } @@ -286,12 +267,6 @@ function AuthGate({ ssrOrigin }: { ssrOrigin: string | null }) { return urlOrgSlug ? : ; } - // Seed the server connection from the SSR origin so origin-derived UI (the - // connect card's MCP URL) renders the real host on the first paint instead - // of the 127.0.0.1 default the client-side global falls back to during SSR. - // Null on client loader re-runs → undefined → the window-derived global, - // which is the same origin, so the key never changes and nothing remounts. - const connection = ssrOrigin ? ({ kind: "http", origin: ssrOrigin } as const) : undefined; const activeSlug = auth.organization.slug; // The org context's slug feeds the connect card's `//mcp` install URL. // Prefer the URL's slug over the session's: on first paint `auth.organization` @@ -315,11 +290,7 @@ function AuthGate({ ssrOrigin }: { ssrOrigin: string | null }) { canonicalization remounts the registry so anything fetched header-less on first paint (rejected server-side) is refetched with the org header. */} - + }> , // and bounces already-signed-in visitors straight back to it. export const Route = createFileRoute("/login")({ diff --git a/apps/cloud/src/start.ts b/apps/cloud/src/start.ts index ebdf8256c6..aaadc1a521 100644 --- a/apps/cloud/src/start.ts +++ b/apps/cloud/src/start.ts @@ -2,7 +2,7 @@ import { createMiddleware, createStart } from "@tanstack/react-start"; import { decodeOAuthCallbackState } from "@executor-js/sdk/shared"; import { isAppOwnedPath } from "./app-paths"; -import { authGateMiddleware } from "./auth/ssr-gate"; +import { authGateMiddleware } from "./auth/doc-gate"; import { parseCookie } from "./auth/cookies"; import { ORG_SELECTOR_HEADER } from "./auth/organization"; import { loginPath } from "./auth/return-to"; diff --git a/apps/cloud/vite.config.ts b/apps/cloud/vite.config.ts index ec565da2f2..308d5d60d8 100644 --- a/apps/cloud/vite.config.ts +++ b/apps/cloud/vite.config.ts @@ -142,6 +142,16 @@ export default defineConfig(({ command, mode }) => { router: { virtualRouteConfig: routes, }, + // SPA mode: the console is 100% authenticated UI (marketing is its own + // Astro app, docs are a proxy), so nothing needs per-request React + // SSR. The shell is prerendered once at build; document requests still + // run the request-middleware chain (doc-gate auth redirects + session + // cookie rotation) but serve that static shell, which drops the whole + // React app from the worker bundle and takes per-request render cost + // to zero. + spa: { + enabled: true, + }, }), react(), ], diff --git a/e2e/cloud/connect-card-origin.test.ts b/e2e/cloud/connect-card-origin.test.ts new file mode 100644 index 0000000000..da1e80f57b --- /dev/null +++ b/e2e/cloud/connect-card-origin.test.ts @@ -0,0 +1,55 @@ +// Cloud-specific: the integrations landing page's connect card prints an +// `npx add-mcp ` install command — the first thing a new user is told to +// copy. That URL's origin must be the host actually serving the console, never +// the desktop/CLI fallback `http://127.0.0.1:4000` the server-connection +// module defaults to when no origin is known. +// +// Under SPA serving there is no per-request document render: the shell is a +// static asset and the card renders client-side, deriving its origin from +// `window.location.origin`. This scenario pins the rendered card (the DOM the +// user actually copies from) to the serving host, against the real WorkOS +// emulator session. +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; + +import { scenario } from "../src/scenario"; +import { Browser, Target } from "../src/services"; + +scenario( + "Connect card · the rendered install command uses the real host, never the 127.0.0.1 default", + {}, + Effect.gen(function* () { + const browser = yield* Browser; + const target = yield* Target; + const expectedOrigin = new URL(target.baseUrl).origin; + const identity = yield* target.newIdentity(); + + yield* browser.session(identity, async ({ page, step }) => { + let commandText = ""; + await step("Open the console and read the connect card's command", async () => { + await page.goto("/", { waitUntil: "commit" }); + const command = page.getByText(/add-mcp\s+https?:\/\//).first(); + await command.waitFor({ timeout: 30_000 }); + commandText = (await command.textContent()) ?? ""; + }); + + const endpoint = /add-mcp\s+(https?:\/\/\S+\/mcp)/.exec(commandText)?.[1] ?? null; + expect(endpoint, "the connect card renders an install command").not.toBeNull(); + + expect(new URL(endpoint!).origin, "the install URL uses the real serving origin").toBe( + expectedOrigin, + ); + expect(endpoint!, "…and not the desktop/CLI default").not.toContain("127.0.0.1:4000"); + // It's still the org-scoped path the user actually needs. Since #974 + // ("Org-slug console URLs across cloud, self-host, and cloudflare + // hosts"), the install card prints the org's URL SLUG (e.g. + // /org-user-xxx/mcp), not the legacy WorkOS org_ form — + // mount.ts's classifyMcpPath still accepts either shape, but the slug + // form is what ships, so accept both rather than pinning on the + // retired id-only shape. + expect(endpoint!, "the install URL stays org-scoped").toMatch( + /\/(?:org_[^/]+|[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)\/mcp$/, + ); + }); + }), +); diff --git a/e2e/cloud/connect-card-ssr-origin.test.ts b/e2e/cloud/connect-card-ssr-origin.test.ts deleted file mode 100644 index c8ffd4725f..0000000000 --- a/e2e/cloud/connect-card-ssr-origin.test.ts +++ /dev/null @@ -1,79 +0,0 @@ -// Cloud-specific: now that the gate renders the REAL shell during SSR (no -// skeleton), the integrations landing page's connect card is part of the very -// first painted HTML — including its `npx add-mcp ` install command. That -// URL is built from the server connection's origin, and the SPA only learns -// `window.location.origin` after it mounts; the SSR default is the desktop/CLI -// fallback `http://127.0.0.1:4000`. If SSR rendered with that default, the -// command would paint `127.0.0.1:4000` and then flip to the real host at -// hydration — a visible flash of the wrong URL on the first thing the user is -// told to copy. -// -// The gate now threads the request origin to the render, so the command is -// SSR'd against the REAL host from the first byte. These scenarios pin that on -// the raw document (pre-JS), where a flash would be observable, against the -// real WorkOS emulator session. -import { expect } from "@effect/vitest"; -import { Effect } from "effect"; - -import { scenario } from "../src/scenario"; -import { Api, Target } from "../src/services"; - -/** A document navigation request — what the SSR gate keys on. */ -const documentRequest = (url: URL, cookie: string) => - Effect.promise(() => - fetch(url, { redirect: "manual", headers: { accept: "text/html", cookie } }), - ); - -/** - * The install command is rendered inside a
 (shiki's grammar isn't loaded
- * during SSR, so the code block falls back to plain text). Stripping tags
- * reassembles the literal command even if hydration would later tokenize it
- * into colored spans.
- */
-const installEndpointFromHtml = (html: string): string | null => {
-  const text = html.replace(/<[^>]+>/g, " ");
-  return /add-mcp\s+(https?:\/\/\S+\/mcp)/.exec(text)?.[1] ?? null;
-};
-
-scenario(
-  "Connect card · the install command SSRs against the real host, never the 127.0.0.1 default",
-  {},
-  Effect.gen(function* () {
-    // Gate: the REST API plane is mounted on this target.
-    yield* Api;
-    const target = yield* Target;
-    const expectedOrigin = new URL(target.baseUrl).origin;
-
-    const identity = yield* target.newIdentity();
-    const response = yield* documentRequest(
-      new URL("/", target.baseUrl),
-      identity.headers!.cookie!,
-    );
-    expect(response.status, "the authenticated landing page is served").toBe(200);
-
-    const html = yield* Effect.promise(() => response.text());
-    // The card is part of the first paint (the whole point of deleting the
-    // skeleton) — if it weren't SSR'd, there'd be no flash to fix and this
-    // guard would be vacuous, so assert it's actually there.
-    const endpoint = installEndpointFromHtml(html);
-    expect(endpoint, "the connect card's install command is in the SSR'd HTML").not.toBeNull();
-
-    // The fix: the command's origin is the host that served the document, not
-    // the client-side fallback the SPA would otherwise paint before mounting.
-    expect(new URL(endpoint!).origin, "the install URL uses the real serving origin").toBe(
-      expectedOrigin,
-    );
-    expect(endpoint!, "…and not the desktop/CLI default that used to flash").not.toContain(
-      "127.0.0.1:4000",
-    );
-    // It's still the org-scoped path the user actually needs. Since #974
-    // ("Org-slug console URLs across cloud, self-host, and cloudflare hosts"),
-    // the install card prints the org's URL SLUG (e.g. /org-user-xxx/mcp), not
-    // the legacy WorkOS org_ form — mount.ts's classifyMcpPath still
-    // accepts either shape, but the slug form is what ships, so accept both
-    // rather than pinning on the retired id-only shape.
-    expect(endpoint!, "the install URL stays org-scoped").toMatch(
-      /\/(?:org_[^/]+|[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)\/mcp$/,
-    );
-  }),
-);
diff --git a/e2e/cloud/unauthenticated-skeleton.test.ts b/e2e/cloud/unauthenticated-skeleton.test.ts
index 5898bb7e70..277729797e 100644
--- a/e2e/cloud/unauthenticated-skeleton.test.ts
+++ b/e2e/cloud/unauthenticated-skeleton.test.ts
@@ -4,8 +4,8 @@
 // AuthGate used to SSR the AUTHENTICATED app-shell skeleton (sidebar + card
 // grid) for every visitor and only swap to a login page after a client-side
 // `/account/me` 401 — signed-out users were shown an app they'd never reach.
-// Now the SSR auth gate (apps/cloud/src/auth/ssr-gate.ts) verifies the sealed
-// session cookie in the worker and 302s signed-out document requests to
+// Now the document auth gate (apps/cloud/src/auth/doc-gate.ts) verifies the
+// sealed session cookie in the worker and 302s signed-out document requests to
 // /login (carrying ?returnTo=), so the app shell never exists for them.
 import { expect } from "@effect/vitest";
 import { Effect } from "effect";
@@ -136,7 +136,14 @@ scenario(
         await page.goto("/", { waitUntil: "commit" });
         await page.locator("aside").first().waitFor({ state: "visible" });
       });
-      expect(new URL(page.url()).pathname, "no login detour for a valid session").toBe("/");
+      // The contract is "no login detour": under SPA serving the shell becomes
+      // visible only after auth resolves, by which point OrgSlugGate may have
+      // already canonicalized the bare root onto the org's slug (that landing
+      // is pinned by auth-routing-flow). Either resting place is signed-in
+      // routing working; /login is the only wrong answer.
+      expect(new URL(page.url()).pathname, "no login detour for a valid session").not.toBe(
+        "/login",
+      );
     });
 
     // A signed-in visitor landing on /login is bounced back into the app.
diff --git a/e2e/setup/cloud.boot.ts b/e2e/setup/cloud.boot.ts
index 799c1cfa47..851ab85cc4 100644
--- a/e2e/setup/cloud.boot.ts
+++ b/e2e/setup/cloud.boot.ts
@@ -56,6 +56,18 @@ export const bootCloud = async (options: CloudBootOptions): Promise
   const dbPath = resolve(cloudDir, ".e2e-stub-db");
   if (options.fresh ?? true) rmSync(dbPath, { recursive: true, force: true });
 
+  // Fresh worker state per boot, for the same reason as the DB: miniflare
+  // persists the Workers Cache API under .wrangler/state across dev-server
+  // runs, and the JWKS L2 cache (auth/jwks-cache.ts) lives there keyed by
+  // URL. The WorkOS emulator binds this checkout's stable port block, so run
+  // N+1's dev server serves run N's cached JWKS (stale-while-revalidate)
+  // against a fresh emulator's keys — every session verify then fails
+  // signature, falls into single-use refresh, and concurrent requests race
+  // each other's rotation into 401s. One run poisons the next.
+  if (options.fresh ?? true) {
+    rmSync(resolve(cloudDir, ".wrangler", "state"), { recursive: true, force: true });
+  }
+
   // MCP access tokens minted by the emulator's OAuth server must carry the
   // app's client id as audience (what the resource server verifies).
   process.env.EMULATE_WORKOS_AUDIENCE = options.workosClientId;