From 0b57ddeaaaf2ffd9a7678cc96990afc93f9edaec Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 04:03:28 +0000 Subject: [PATCH] =?UTF-8?q?fix(mcp):=20the=20stdio=20transport=20answers?= =?UTF-8?q?=20again=20=E2=80=94=20resume=20the=20stdin=20it=20owns=20(#764?= =?UTF-8?q?5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `objectstack serve` with `OS_MCP_STDIO_ENABLED=true` logged `[MCP] Server started (transport: stdio)`, bound the transport to a real `osk_` identity, and then never answered a single request: `initialize`, `tools/list`, `resources/list` and `resources/read` all timed out with zero bytes on stdout. The pause came from the host, above the plugin. oclif's argument parser reads stdin for any positional argument the caller did not supply (`tryStdin` -> `createInterface({input: process.stdin})`, aborted after 10ms), and `Interface.close()` calls `stdin.pause()`. `serve` declares an optional `config` positional, so plain `objectstack serve --dev` left `process.stdin` explicitly paused before the kernel booted. `StdioServerTransport.start()` only attaches a `data` listener, and Node auto-switches a stream to flowing mode on that listener only while `readableFlowing` is still `null` — never after an explicit `pause()`. Listener attached, `bytesRead` 0, transport deaf. `MCPServerRuntime.start()` now resumes `process.stdin` immediately after `connect()` — the moment the transport takes ownership of it, and after the transport's reader is attached so no byte can flow unread. The resume lives in the runtime rather than in the CLI's argument definitions because the pause is not oclif-specific: any host that touched stdin before `start()` leaves the transport equally deaf. Pinned by a spawned-CLI e2e that writes a real `initialize` down the child's stdin and requires a real result back — the consequence, not the stream flag. Reverse-verified: with the resume removed it times out with zero bytes. The ADR-0101 fail-closed startup contract is untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Qxkv33VVJAYJptjGmjTzFG --- .../mcp-stdio-transport-resume-stdin.md | 43 +++ .../test/serve-mcp-stdio-answers.e2e.test.ts | 288 ++++++++++++++++++ packages/mcp/src/mcp-server-runtime.ts | 29 ++ 3 files changed, 360 insertions(+) create mode 100644 .changeset/mcp-stdio-transport-resume-stdin.md create mode 100644 packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts diff --git a/.changeset/mcp-stdio-transport-resume-stdin.md b/.changeset/mcp-stdio-transport-resume-stdin.md new file mode 100644 index 0000000000..c2eb689ef3 --- /dev/null +++ b/.changeset/mcp-stdio-transport-resume-stdin.md @@ -0,0 +1,43 @@ +--- +"@objectstack/mcp": patch +--- + +fix(mcp): the stdio MCP transport answers again — resume the stdin it just took ownership of (#7645) + +`objectstack serve` with `OS_MCP_STDIO_ENABLED=true` logged +`[MCP] Server started (transport: stdio)`, bound the transport to a real `osk_` +identity — and then **never answered a single request**. `initialize`, +`tools/list`, `resources/list` and `resources/read` all timed out with **zero +bytes on stdout**; malformed input drew no error either. Every stdio MCP session +against the CLI was unusable, and the failure was silent on both sides: the +server looked started, the client just waited. + +The pause came from the **host**, above the plugin. oclif's argument parser +reads stdin for any positional argument the caller did not supply (`tryStdin` → +`createInterface({input: process.stdin})`, aborted after 10 ms), and +`Interface.close()` calls `stdin.pause()`. `serve` declares an optional `config` +positional, so plain `objectstack serve --dev` left `process.stdin` explicitly +paused before the kernel ever booted. `StdioServerTransport.start()` only +attaches a `data` listener, and Node auto-switches a stream to flowing mode on +that listener **only while `readableFlowing` is still `null`** — never after an +explicit `pause()`. Listener attached, `bytesRead` stuck at 0, transport deaf. + +`MCPServerRuntime.start()` now resumes `process.stdin` immediately after +`connect()`, which is the moment the transport takes ownership of it (after, so +the transport's reader is attached before any byte can flow). The resume lives +in the runtime rather than in the CLI's argument definitions because the pause +is not oclif-specific: any host that touched stdin before `start()` — a readline +prompt, a supervisor, an embedding process — left the transport equally deaf, +and this is the one place that knows a long-lived stdio transport was just +attached. + +Measured, both directions: `objectstack serve --dev` (no config path, parser +reads stdin) went from timing out to answering `initialize`, while `objectstack +serve objectstack.config.ts --dev` (parser never touches stdin) answered before +and after. The HTTP transport at `/api/v1/mcp` is unaffected — it is served +per-request and never touches stdin. + +Not changed, and deliberately so: the ADR-0101 fail-closed startup contract. +stdio enabled without `OS_MCP_STDIO_API_KEY` still throws at plugin start, an +unknown/revoked key still refuses with no anonymous-but-serving fallback, and a +member key still binds the principal to that member. diff --git a/packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts b/packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts new file mode 100644 index 0000000000..e471eb9fcf --- /dev/null +++ b/packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts @@ -0,0 +1,288 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7645 — a started stdio MCP transport must ANSWER. + * + * The defect: `os serve` with `OS_MCP_STDIO_ENABLED=true` logged + * `[MCP] Server started (transport: stdio)`, bound the principal to a real + * `osk_` identity, and then never replied to a single JSON-RPC request — + * `initialize`, `tools/list`, `resources/list`, `resources/read` all timed out + * with ZERO bytes on stdout. Malformed input drew no error either. + * + * The cause sat in the HOST, above the plugin: oclif's argument parser reads + * stdin for any positional arg the caller did not supply (`tryStdin` → + * `createInterface({input: process.stdin})`, aborted after 10 ms), and + * `Interface.close()` calls `stdin.pause()`. `serve` declares an optional + * `config` positional, so `os serve --dev` left `process.stdin` explicitly + * paused. `StdioServerTransport.start()` only attaches a `data` listener, and + * Node auto-switches to flowing mode on that listener ONLY while + * `readableFlowing` is `null` — never after an explicit `pause()`. Listener + * attached, `bytesRead` 0, server deaf. + * + * WHY THIS FILE SPAWNS THE CLI. `packages/mcp`'s 17 existing stdio pins were + * all green while every real `os serve` stdio session was unusable, because + * they sit BELOW the gap: they exercise the runtime in a plain node process, + * where nothing ever paused stdin. Only a test that spawns the actual command + * and speaks JSON-RPC down the child's pipe can see it. + * + * WHY IT ASSERTS AN ANSWER, not a stream flag. `process.stdin.isPaused() === + * false` would pass over a transport that still replies to nothing — it pins + * the mechanism, not the consequence. The assertion below is the card's own + * repro: write a real `initialize` to the child's stdin, get a real result + * back. Reverse-verified — with the `resume()` in `MCPServerRuntime.start()` + * removed, this file times out with zero bytes on stdout. + * + * WHY IT MINTS A KEY OVER HTTP FIRST. stdio auto-start is fail-closed + * (ADR-0101): without an `OS_MCP_STDIO_API_KEY` that resolves to a real + * identity the plugin REFUSES to start, so there is no transport to test. That + * contract is correct and is not what this file is about — the first boot just + * mints a key through the product route (`POST /keys`) against a file-backed + * DB, and the second boot reuses that DB. A fabricated `sys_api_key` row would + * have to guess the at-rest hashing the mint path owns. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { randomPort } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +/** `bin/run.js` — the SHIPPED entrypoint, i.e. the one the card's repro names. */ +const CLI = resolve(HERE, '../bin/run.js'); + +const CONFIG = ` +export default { + manifest: { + id: 'com.example.stdioprobe', + namespace: 'stdioprobe', + version: '1.0.0', + type: 'app', + name: 'MCP stdio probe', + }, + objects: [{ + name: 'stdioprobe_task', + label: 'Task', + sharingModel: 'public', + fields: { title: { type: 'text', label: 'Title' } }, + }], +}; +`; + +let dir: string; +let port: string; +let apiKey: string; +/** Every child this file spawns, so a failed assertion never leaks a server. */ +const children: ChildProcessWithoutNullStreams[] = []; + +interface Booted { + child: ChildProcessWithoutNullStreams; + stdout: () => string; + stderr: () => string; +} + +/** + * Spawn `os serve` and resolve once `waitFor` matches its stdout, leaving the + * child RUNNING — the shared `runServe` helper kills on match, and this file + * has to keep talking to the process afterwards. + * + * NOTE: no positional config path is passed. That is deliberate and load-bearing: + * supplying one makes oclif skip `tryStdin` entirely, so stdin is never paused + * and the defect cannot reproduce. `os serve --dev` is also the form every user + * types. + */ +function boot(env: Record, waitFor: RegExp): Promise { + return new Promise((resolveBoot, rejectBoot) => { + const child = spawn(process.execPath, [CLI, 'serve', '-p', port, '--dev'], { + cwd: dir, + stdio: ['pipe', 'pipe', 'pipe'], + env: { + ...process.env, + NO_COLOR: '1', + OS_LOG_LEVEL: 'info', + OS_DISABLE_CONSOLE: '1', + // Explicit, not inherited: the dev-admin seed this fixture signs in as + // is hard-gated on `NODE_ENV === 'development'`, and vitest exports + // `test`, which would leave the DB user-less and the mint unauthorized. + NODE_ENV: 'development', + ...env, + }, + }) as ChildProcessWithoutNullStreams; + children.push(child); + + let out = ''; + let err = ''; + let settled = false; + + const timer = setTimeout(() => { + if (settled) return; + settled = true; + rejectBoot( + new Error(`serve never printed ${waitFor}\n--- stdout ---\n${out.slice(-4000)}\n--- stderr ---\n${err.slice(-4000)}`), + ); + }, 150_000); + + child.stdout.on('data', (d) => { + out += String(d); + if (!settled && waitFor.test(out)) { + settled = true; + clearTimeout(timer); + resolveBoot({ child, stdout: () => out, stderr: () => err }); + } + }); + child.stderr.on('data', (d) => { + err += String(d); + }); + child.on('exit', (code) => { + if (settled) return; + settled = true; + clearTimeout(timer); + rejectBoot( + new Error(`serve exited ${code} before ${waitFor}\n--- stdout ---\n${out.slice(-4000)}\n--- stderr ---\n${err.slice(-4000)}`), + ); + }); + }); +} + +async function stop(child: ChildProcessWithoutNullStreams): Promise { + if (child.exitCode !== null || child.signalCode !== null) return; + await new Promise((done) => { + const give = setTimeout(() => { + try { + child.kill('SIGKILL'); + } catch { + /* already gone */ + } + done(); + }, 10_000); + child.once('exit', () => { + clearTimeout(give); + done(); + }); + try { + child.kill('SIGTERM'); + } catch { + clearTimeout(give); + done(); + } + }); +} + +describe('#7645: the stdio MCP transport answers over a spawned CLI process', () => { + beforeAll(async () => { + dir = mkdtempSync(join(tmpdir(), 'mcp-stdio-e2e-')); + writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8'); + writeFileSync( + join(dir, 'package.json'), + JSON.stringify({ name: 'mcp-stdio-e2e-fixture', private: true, type: 'module' }, null, 2), + 'utf8', + ); + port = randomPort(); + + // Boot 1 — mint a real key through the product route, against a FILE db so + // boot 2 sees the same row (`:memory:` would not survive the restart). + const first = await boot({ OS_DATABASE_URL: join(dir, 'probe.db') }, /Server is ready/); + const base = `http://localhost:${port}/api/v1`; + const signIn = await fetch(`${base}/auth/sign-in/email`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + // `serve --dev` seeds this admin on an empty DB. + body: JSON.stringify({ email: 'admin@objectos.ai', password: 'admin123' }), + }); + expect(signIn.status).toBe(200); + const token = ((await signIn.json()) as { token?: string }).token; + expect(token).toBeTruthy(); + + const minted = await fetch(`${base}/keys`, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${token}` }, + body: JSON.stringify({ name: 'mcp-stdio-e2e' }), + }); + expect(minted.status).toBe(201); + apiKey = String(((await minted.json()) as { data: { key: string } }).data.key); + expect(apiKey.startsWith('osk_')).toBe(true); + + await stop(first.child); + }, 240_000); + + afterAll(async () => { + for (const child of children) await stop(child); + if (dir) rmSync(dir, { recursive: true, force: true }); + }, 60_000); + + it('replies to a real JSON-RPC initialize written to the child process stdin', async () => { + const booted = await boot( + { + OS_DATABASE_URL: join(dir, 'probe.db'), + OS_MCP_STDIO_ENABLED: 'true', + OS_MCP_STDIO_API_KEY: apiKey, + }, + /\[MCP\] Server started \(transport: stdio/, + ); + + // The started transport is the premise, not the assertion — the whole + // defect was a transport that reached exactly this line and then went deaf. + const reply = await new Promise((resolveReply) => { + let buf = ''; + const onData = (d: Buffer | string) => { + buf += String(d); + // A JSON-RPC frame carrying OUR request id — not merely "some bytes". + if (/"jsonrpc"\s*:\s*"2\.0"/.test(buf) && /"id"\s*:\s*1\b/.test(buf)) { + clearTimeout(giveUp); + booted.child.stdout.off('data', onData); + resolveReply(buf); + } + }; + booted.child.stdout.on('data', onData); + + const giveUp = setTimeout(() => { + booted.child.stdout.off('data', onData); + resolveReply(null); + }, 45_000); + + booted.child.stdin.write( + `${JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { + protocolVersion: '2024-11-05', + capabilities: {}, + clientInfo: { name: 'objectstack-e2e', version: '0.0.0' }, + }, + })}\n`, + ); + }); + + expect( + reply, + 'the stdio transport started but never answered `initialize` (#7645: stdin left paused by the host)', + ).not.toBeNull(); + + // An ANSWER, not just traffic: the frame has to parse as this request's + // result and carry the server's identity, so a stray log line that happens + // to contain `"jsonrpc"` cannot pass. + const frame = (reply as string) + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.startsWith('{') && line.includes('"jsonrpc"')) + .map((line) => { + try { + return JSON.parse(line) as Record; + } catch { + return undefined; + } + }) + .find((msg) => msg?.id === 1); + + expect(frame, `no parseable JSON-RPC frame for id 1 in:\n${reply}`).toBeTruthy(); + const result = frame!.result as { protocolVersion?: string; serverInfo?: { name?: string } } | undefined; + expect(frame!.error).toBeUndefined(); + expect(result?.protocolVersion).toBeTruthy(); + expect(result?.serverInfo?.name).toBeTruthy(); + + await stop(booted.child); + }, 240_000); +}); diff --git a/packages/mcp/src/mcp-server-runtime.ts b/packages/mcp/src/mcp-server-runtime.ts index cf4ae70617..2930c69954 100644 --- a/packages/mcp/src/mcp-server-runtime.ts +++ b/packages/mcp/src/mcp-server-runtime.ts @@ -1022,6 +1022,35 @@ export class MCPServerRuntime { if (this.config.transport === 'stdio') { this.transport = new StdioServerTransport(); await this.mcpServer.connect(this.transport); + // [#7645] The transport now OWNS this process's stdin — so make sure it + // is actually flowing. `StdioServerTransport.start()` only attaches a + // `data` listener, and Node auto-switches a stream to flowing mode on + // that listener ONLY while `readableFlowing` is still `null`. Once + // something has explicitly called `pause()`, the flag is `false` and a + // later `data` listener does NOT resume it: the listener is attached, + // `bytesRead` stays 0, and the server is started-but-permanently-deaf. + // + // That is not hypothetical. Under `objectstack serve`, oclif's argument + // parser reads stdin for any positional arg the user did not supply + // (`tryStdin` → `createInterface({input: process.stdin})`, aborted after + // 10 ms), and `Interface.close()` calls `stdin.pause()`. `serve` declares + // an optional `config` positional, so `os serve --dev` (no path) left + // stdin paused and EVERY `initialize` / `tools/list` / `resources/read` + // timed out with zero bytes on stdout — while the same command WITH the + // path (parser never touches stdin) answered fine. Measured both ways. + // + // The resume lives here rather than in the CLI because the pause is not + // oclif-specific: any host that touched stdin before `start()` (a + // readline prompt, a supervisor, an embedding process) leaves it paused, + // and every one of them yields the same silent deafness. This is the one + // place that knows a long-lived stdio transport was just attached. + // + // Resumed AFTER `connect()` on purpose: `connect()` attaches the + // transport's `data` listener, so no byte can flow before there is a + // reader for it. + if (typeof process !== 'undefined' && typeof process.stdin?.resume === 'function') { + process.stdin.resume(); + } this.started = true; logger?.info(`[MCP] Server started (transport: stdio, name: ${this.config.name})`); } else {