From d79d4b0666f9b26cf14fd89f37c08def643511a2 Mon Sep 17 00:00:00 2001 From: Aaron Bockelie Date: Wed, 24 Jun 2026 10:00:41 -0500 Subject: [PATCH] fix(bridge): start under Claude Desktop's loader, not just `node server.js` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0.11.34 shipped a broken bridge: #243 gated startup on `require.main === module`, but Claude Desktop runs the .mcpb with its built-in Node ("Using built-in Node.js for MCP server") via a loader that does NOT make server.js the main module. So main() never ran, stdin was never read, the client's `initialize` went unanswered, and Desktop timed out after 60s ("Request timed out", -32001) — the extension "couldn't connect at all". (Local `node mcpb/server.js` worked, which is why it passed review: that path makes require.main === module true.) Gate on the absence of Jest's worker env instead. main() then runs in every real launch — system `node server.js` and Desktop's built-in Node alike — while the require()-based test seam stays inert under Jest. Reproduced via `node -e "require('./mcpb/server.js')"` (hangs before, responds after). Add tests/bridge-bootstrap.test.ts: spawns the bridge against a stub HTTP server and asserts it answers `initialize` in BOTH launch shapes (`node server.js` AND non-main require). The existing unit tests `require()` the module — which by design skips main() — so they could never catch a boot failure; this is that missing coverage. Verified non-vacuous: reverting the guard fails the non-main case. Refs #243. The GitHub 0.11.34 release .mcpb has this bug and must not be promoted; ship the fix in 0.11.35. Claude-Session: https://claude.ai/code/session_011yowKCMFCBp61DRBu5xQm4 --- mcpb/server.js | 19 ++++-- tests/bridge-bootstrap.test.ts | 104 +++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 tests/bridge-bootstrap.test.ts diff --git a/mcpb/server.js b/mcpb/server.js index 18baf5f..05d6a8e 100644 --- a/mcpb/server.js +++ b/mcpb/server.js @@ -229,9 +229,9 @@ async function dispatch(message, isReplay = false) { } } -// Stdio bootstrap. Guarded so the module can be `require`d in tests (which -// drive `dispatch` directly) without validating env, claiming stdin, or -// exiting the process. When launched as the bridge, `require.main === module`. +// Stdio bootstrap. Invoked unless the Jest test suite is importing this module +// to drive `dispatch` directly (see the JEST_WORKER_ID guard at the bottom), so +// tests don't validate env, claim stdin, or exit the process. function main() { if (!MCP_URL) { process.stderr.write('[obsidian-mcp-bridge] MCP_URL not set\n'); @@ -283,7 +283,18 @@ function main() { }); } -if (require.main === module) { +// Start the bridge unless we're being imported by the Jest test suite. +// +// We intentionally do NOT gate on `require.main === module`. Claude Desktop +// runs the bundle with its built-in Node ("Using built-in Node.js for MCP +// server" in its logs) via a loader that does NOT make server.js the main +// module, so that check is false in production: main() never runs, stdin is +// never read, the client's `initialize` goes unanswered, and Desktop times out +// after 60s ("Request timed out", -32001). That regression shipped in 0.11.34. +// Gating on the absence of Jest's worker env runs main() in every real launch +// — system `node server.js` and Desktop's built-in Node alike — while keeping +// the require()-based test seam below inert. +if (!process.env.JEST_WORKER_ID) { main(); } diff --git a/tests/bridge-bootstrap.test.ts b/tests/bridge-bootstrap.test.ts new file mode 100644 index 0000000..72fd30e --- /dev/null +++ b/tests/bridge-bootstrap.test.ts @@ -0,0 +1,104 @@ +/** + * Regression harness for the bridge bootstrap (0.11.34 boot bug). + * + * The .mcpb bridge (mcpb/server.js) is bundled as a single file and launched as + * a process. The unit tests (bridge-self-heal) `require()` it and drive + * `dispatch` directly — which by design skips `main()` — so they cannot catch a + * failure to *start*. That gap shipped: #243 gated startup on + * `require.main === module`, but Claude Desktop loads the bundle with its + * built-in Node via a loader that does NOT make server.js the main module, so + * main() never ran, stdin was never read, and `initialize` hung until Desktop's + * 60s timeout. + * + * This test actually spawns the bridge against a stub HTTP server and asserts it + * answers `initialize` — in BOTH launch shapes: + * 1. `node server.js` (require.main === module) + * 2. `node -e "require('server.js')"` (require.main !== module — Desktop-like) + * Mode 2 is the one that regressed. + */ +import { spawn } from 'node:child_process'; +import http from 'node:http'; +import path from 'node:path'; +import type { AddressInfo } from 'node:net'; + +const SERVER = path.resolve(__dirname, '../mcpb/server.js'); +const INIT = JSON.stringify({ + jsonrpc: '2.0', id: 0, method: 'initialize', + params: { protocolVersion: '2025-06-18', capabilities: {}, clientInfo: { name: 't', version: '0' } }, +}); + +// Minimal MCP HTTP stub: answers initialize with a session id + result, drains +// the GET notify stream, and 200s everything else. Enough for the bridge to +// complete a handshake and emit the initialize result to stdout. +function startStub(): Promise<{ url: string; close: () => Promise }> { + return new Promise((resolve) => { + const server = http.createServer((req, res) => { + if (req.method === 'GET' || req.method === 'DELETE') { res.writeHead(200).end(); return; } + let body = ''; + req.on('data', (c) => { body += c; }); + req.on('end', () => { + const msg = JSON.parse(body || '{}'); + if (msg.method === 'initialize') { + res.writeHead(200, { 'Content-Type': 'application/json', 'mcp-session-id': 'stub-session' }); + res.end(JSON.stringify({ jsonrpc: '2.0', id: msg.id, result: { serverInfo: { name: 'stub' } } })); + } else { + res.writeHead(202).end(); + } + }); + }); + server.listen(0, '127.0.0.1', () => { + const port = (server.address() as AddressInfo).port; + resolve({ + url: `http://127.0.0.1:${port}/mcp`, + close: () => new Promise((r) => server.close(() => r())), + }); + }); + }); +} + +// Spawn the bridge with the given node args, feed it `initialize`, and resolve +// with the first JSON object it writes to stdout (the initialize result). +function bootAndInitialize(nodeArgs: string[], mcpUrl: string): Promise> { + return new Promise((resolve, reject) => { + // Strip JEST_WORKER_ID: the child IS a real bridge launch, and the bootstrap + // guard deliberately skips main() when that var is present (test seam). + const env: NodeJS.ProcessEnv = { ...process.env, MCP_URL: mcpUrl, MCP_API_KEY: 'k' }; + delete env.JEST_WORKER_ID; + + const child = spawn(process.execPath, nodeArgs, { env, stdio: ['pipe', 'pipe', 'pipe'] }); + let out = ''; + const timer = setTimeout(() => { child.kill(); reject(new Error(`no initialize response (bridge hung). stderr: ${err}`)); }, 8000); + let err = ''; + child.stderr.on('data', (c) => { err += c; }); + child.stdout.on('data', (c) => { + out += c; + const line = out.split('\n').find((l) => l.trim()); + if (line) { + clearTimeout(timer); + child.kill(); + try { resolve(JSON.parse(line)); } catch (e) { reject(e as Error); } + } + }); + child.on('error', (e) => { clearTimeout(timer); reject(e); }); + child.stdin.write(INIT + '\n'); + }); +} + +describe('mcpb bridge bootstrap — starts under both launch shapes (0.11.34 boot bug)', () => { + let stub: { url: string; close: () => Promise }; + beforeAll(async () => { stub = await startStub(); }); + afterAll(async () => { await stub.close(); }); + + test('launched as `node server.js` (require.main === module) answers initialize', async () => { + const res = await bootAndInitialize([SERVER], stub.url); + expect(res.id).toBe(0); + expect((res as { result?: unknown }).result).toBeDefined(); + }, 12000); + + test('launched as a non-main require (Desktop-style loader) answers initialize', async () => { + const requireExpr = `require(${JSON.stringify(SERVER)})`; + const res = await bootAndInitialize(['-e', requireExpr], stub.url); + expect(res.id).toBe(0); + expect((res as { result?: unknown }).result).toBeDefined(); + }, 12000); +});