diff --git a/.changeset/warm-stdio-mcp-pooling.md b/.changeset/warm-stdio-mcp-pooling.md new file mode 100644 index 000000000..48f9d7dae --- /dev/null +++ b/.changeset/warm-stdio-mcp-pooling.md @@ -0,0 +1,9 @@ +--- +"@executor-js/plugin-mcp": patch +--- + +**Stdio MCP servers are kept alive between tool calls** + +Every tool call on a stdio MCP integration used to spawn a fresh child process, run the full MCP handshake, call the one tool, and tear the child down — roughly a second of overhead per call for an `npx`-launched server, on every call. Remote and app-server connections already reused sessions through the connection pool; plain stdio now joins them, with the same five-minute idle window, the same hashed identity key (command, args, cwd, secret env, credential values, owner and connection all separate identities), and the same drop-on-transport-failure semantics. This matches how MCP clients drive stdio servers generally: one long-lived child per session, not one per call. + +A server that genuinely depends on fresh-process semantics can opt out with `spawnPerCall: true` in its stdio config (also accepted by the add-server API). The Codex app-server bridge ignores the opt-out — its approvals are session state, so it must pool. diff --git a/packages/plugins/mcp/src/api/group.ts b/packages/plugins/mcp/src/api/group.ts index 1dc0ffa3f..69a76ee2a 100644 --- a/packages/plugins/mcp/src/api/group.ts +++ b/packages/plugins/mcp/src/api/group.ts @@ -63,6 +63,9 @@ const AddStdioServerPayload = Schema.Struct({ * 2026-07-28) for modern-only servers; default is the legacy `initialize` * handshake. */ versionNegotiation: Schema.optional(Schema.Literals(["legacy", "auto"])), + /** Opt out of process reuse — spawn a fresh child for every tool call. + * Absent means the spawned server is kept alive between calls. */ + spawnPerCall: Schema.optional(Schema.Boolean), /** Reach the server through the Codex app-server bridge: the command spawns * `codex app-server` and `server` names the MCP server inside Codex. */ appServer: Schema.optional( diff --git a/packages/plugins/mcp/src/api/handlers.ts b/packages/plugins/mcp/src/api/handlers.ts index 9c57da78e..4838af216 100644 --- a/packages/plugins/mcp/src/api/handlers.ts +++ b/packages/plugins/mcp/src/api/handlers.ts @@ -41,6 +41,7 @@ const toServerInput = ( env?: Record; cwd?: string; versionNegotiation?: "legacy" | "auto"; + spawnPerCall?: boolean; appServer?: { server: string; surface?: "sky" | "browser"; modulePath?: string }; slug?: string; }; @@ -55,6 +56,7 @@ const toServerInput = ( env: p.env, cwd: p.cwd, versionNegotiation: p.versionNegotiation, + spawnPerCall: p.spawnPerCall, appServer: p.appServer, slug: p.slug, }; diff --git a/packages/plugins/mcp/src/sdk/connection-pool-key.test.ts b/packages/plugins/mcp/src/sdk/connection-pool-key.test.ts index 8d3bcbeeb..54e11c787 100644 --- a/packages/plugins/mcp/src/sdk/connection-pool-key.test.ts +++ b/packages/plugins/mcp/src/sdk/connection-pool-key.test.ts @@ -24,13 +24,14 @@ import { describe, expect, it } from "@effect/vitest"; import { Effect } from "effect"; -import { connectionPoolKey } from "./plugin"; +import { connectionPoolKey, isPoolableConnectorInput } from "./plugin"; import type { ConnectorInput } from "./connection"; const SECRET = "sk-live-poolkey-Zq7!x-SECRET"; const OTHER_SECRET = "sk-live-poolkey-Zq7!x-ROTATED"; type RemoteInput = Extract; +type StdioInput = Extract; const remoteInput = (overrides: Partial = {}): RemoteInput => ({ transport: "remote", @@ -293,3 +294,145 @@ describe("MCP connection-pool key", () => { }), ); }); + +// --------------------------------------------------------------------------- +// Plain stdio joined the poolable inputs (spawn-per-call cost ~1s for an +// `npx`-launched server). These pin the two properties that make that safe: +// the opt-out is honored, and stdio identities separate on every field that +// changes what the child is or what secrets it carries. +// --------------------------------------------------------------------------- + +const stdioInput = (overrides: Partial = {}): StdioInput => ({ + transport: "stdio", + command: "npx", + args: ["-y", "@example/mcp-server"], + env: { API_KEY: SECRET }, + ...overrides, +}); + +describe("stdio poolability", () => { + it("pools plain stdio by default", () => { + expect(isPoolableConnectorInput(stdioInput())).toBe(true); + }); + + it("honors the spawn-per-call opt-out", () => { + expect(isPoolableConnectorInput(stdioInput({ spawnPerCall: true }))).toBe(false); + }); + + it("always pools the app-server bridge — its approvals are session state", () => { + expect( + isPoolableConnectorInput( + stdioInput({ spawnPerCall: true, appServer: { server: "messages" } }), + ), + ).toBe(true); + }); +}); + +describe("stdio pool key", () => { + it.effect("is a bare digest that retains neither the secret env nor the command", () => + Effect.gen(function* () { + const key = yield* connectionPoolKey( + stdioInput(), + "stdio_env", + { API_KEY: SECRET }, + IDENTITY, + ); + + expect(key).toMatch(/^[0-9a-f]{64}$/); + expect(key).not.toContain(SECRET); + expect(key).not.toContain("@example/mcp-server"); + }), + ); + + it.effect("the same stdio identity reuses one key", () => + Effect.gen(function* () { + const first = yield* connectionPoolKey( + stdioInput(), + "stdio_env", + { API_KEY: SECRET }, + IDENTITY, + ); + const second = yield* connectionPoolKey( + stdioInput(), + "stdio_env", + { API_KEY: SECRET }, + IDENTITY, + ); + + expect(first).toBe(second); + }), + ); + + it.effect("a different secret env value dials a fresh child", () => + Effect.gen(function* () { + const mine = yield* connectionPoolKey( + stdioInput(), + "stdio_env", + { API_KEY: SECRET }, + IDENTITY, + ); + const theirs = yield* connectionPoolKey( + stdioInput({ env: { API_KEY: OTHER_SECRET } }), + "stdio_env", + { API_KEY: OTHER_SECRET }, + IDENTITY, + ); + + expect(theirs).not.toBe(mine); + }), + ); + + it.effect("a different command, args, or cwd dials a fresh child", () => + Effect.gen(function* () { + const base = yield* connectionPoolKey(stdioInput(), "stdio_env", {}, IDENTITY); + const command = yield* connectionPoolKey( + stdioInput({ command: "bunx" }), + "stdio_env", + {}, + IDENTITY, + ); + const args = yield* connectionPoolKey( + stdioInput({ args: ["-y", "@example/mcp-server", "--verbose"] }), + "stdio_env", + {}, + IDENTITY, + ); + const cwd = yield* connectionPoolKey(stdioInput({ cwd: "/tmp" }), "stdio_env", {}, IDENTITY); + + expect(new Set([base, command, args, cwd]).size).toBe(4); + }), + ); + + it.effect("plain stdio and the app-server bridge never share a session", () => + Effect.gen(function* () { + // Same command and args — the bridge spawns `codex app-server` too, but + // its session is a Codex thread, not the server's own MCP session. + const plain = yield* connectionPoolKey(stdioInput(), "none", {}, IDENTITY); + const bridge = yield* connectionPoolKey( + stdioInput({ appServer: { server: "messages" } }), + "none", + {}, + IDENTITY, + ); + + expect(bridge).not.toBe(plain); + }), + ); + + it.effect("two owners never share one child", () => + Effect.gen(function* () { + const org = yield* connectionPoolKey(stdioInput(), "none", {}, IDENTITY); + const user = yield* connectionPoolKey( + stdioInput(), + "none", + {}, + { + owner: "user", + connection: "default", + }, + ); + + expect(user).not.toBe(org); + }), + ); +}); diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index a4e2bd60d..fa7c30589 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -242,6 +242,9 @@ const McpStdioServerInputSchema = Schema.Struct({ * handshake — the right call for spawn-per-call servers, where the auto * probe costs an extra child process per connect. */ versionNegotiation: Schema.optional(McpStdioVersionNegotiation), + /** Opt out of process reuse — spawn a fresh child for every tool call (see + * `McpStdioIntegrationConfig.spawnPerCall`). */ + spawnPerCall: Schema.optional(Schema.Boolean), /** Reach the server through the Codex app-server bridge: the command spawns * `codex app-server` and `server` names the MCP server inside Codex whose * tools this integration exposes. Set by the Codex plugin add flow. */ @@ -417,6 +420,7 @@ const toIntegrationConfig = (input: McpServerInput): McpIntegrationConfigType => args: input.args ? [...input.args] : undefined, cwd: input.cwd, versionNegotiation: input.versionNegotiation, + spawnPerCall: input.spawnPerCall, appServer: input.appServer, authenticationTemplate: vars.length > 0 @@ -645,6 +649,7 @@ const buildConnectorInput = ( env: Object.keys(env).length > 0 ? env : undefined, cwd: config.cwd, versionNegotiation: config.versionNegotiation, + spawnPerCall: config.spawnPerCall, appServer: config.appServer, } satisfies McpStdioIntegrationConfig); } @@ -714,20 +719,25 @@ const sortedRecord = ( * it through pool behaviour alone would not see it. */ /** The connector inputs the pool accepts. * - * Remote servers, and app-server bridge connections — NOT stdio generally. - * Pooling the bridge is what makes a Codex plugin's "for this conversation" - * approval mean anything: that grant lives on the Codex THREAD, and the - * bridge starts one thread per connection, so a connection per call re-asked - * on every call. Plain stdio stays unpooled on purpose: a spawn-per-call CLI - * server is entitled to assume a fresh process each time. */ + * Remote servers, app-server bridge connections, and plain stdio servers + * that have not opted out via `spawnPerCall`. Pooling the bridge is what + * makes a Codex plugin's "for this conversation" approval mean anything: + * that grant lives on the Codex THREAD, and the bridge starts one thread per + * connection, so a connection per call re-asked on every call. Plain stdio + * is pooled for latency: a spawn-per-call server pays the child spawn plus a + * full MCP handshake on EVERY tool call (~1s for an `npx`-launched server), + * which is how every other MCP client avoids it — they keep the child alive + * for the whole session. A server that genuinely depends on fresh-process + * semantics sets `spawnPerCall: true` in its stdio config. The bridge + * ignores that flag: its approvals are session state, so it must pool. */ export type PoolableConnectorInput = | Extract - | (McpStdioIntegrationConfig & { readonly appServer: { readonly server: string } }); + | McpStdioIntegrationConfig; /** Whether this connection may be retained between calls (see * `PoolableConnectorInput`). */ export const isPoolableConnectorInput = (input: ConnectorInput): input is PoolableConnectorInput => - input.transport === "remote" || input.appServer !== undefined; + input.transport === "remote" || input.appServer !== undefined || input.spawnPerCall !== true; export const connectionPoolKey = ( input: PoolableConnectorInput, @@ -757,14 +767,17 @@ export const connectionPoolKey = ( : { owner: identity.owner, connection: identity.connection, - transport: "appserver", + transport: input.appServer !== undefined ? "appserver" : "stdio", command: input.command, args: input.args ?? [], cwd: input.cwd ?? null, env: sortedRecord(input.env), - server: input.appServer.server, - surface: input.appServer.surface ?? null, - modulePath: input.appServer.modulePath ?? null, + // Plain stdio negotiates the protocol at connect, so two configs + // that handshake differently must never share a parked session. + versionNegotiation: input.versionNegotiation ?? null, + server: input.appServer?.server ?? null, + surface: input.appServer?.surface ?? null, + modulePath: input.appServer?.modulePath ?? null, template, values: sortedRecord(values), }, diff --git a/packages/plugins/mcp/src/sdk/types.ts b/packages/plugins/mcp/src/sdk/types.ts index 3b14a4ab3..6f6435eb1 100644 --- a/packages/plugins/mcp/src/sdk/types.ts +++ b/packages/plugins/mcp/src/sdk/types.ts @@ -254,6 +254,13 @@ export const McpStdioIntegrationConfig = Schema.Struct({ /** Protocol negotiation at connect. Absent means `legacy` (see * `McpStdioVersionNegotiation` for why that stays the default). */ versionNegotiation: Schema.optional(McpStdioVersionNegotiation), + /** Opt out of process reuse: spawn a fresh child for every tool call. + * Absent means pooled — the spawned server is kept alive between calls + * (five-minute idle window), which is how every mainstream MCP client + * drives stdio servers and what the protocol's session model assumes. Set + * this only for a server that genuinely depends on fresh-process + * semantics, e.g. one that re-reads state at boot and never afterwards. */ + spawnPerCall: Schema.optional(Schema.Boolean), /** Present when the spawned command is `codex app-server` rather than an * MCP server itself: the connector then bridges MCP to the Codex * app-server protocol in process, and `server` names the MCP server