Merge pull request #247 from aaronsb/fix/bridge-bootstrap-desktop-loader

fix(bridge): start under Claude Desktop's loader, not just `node server.js`

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,24 @@ function main() {
   });
 }
 
-if (require.main === module) {
+// Autostart contract: the bridge starts in every real launch unless a caller
+// explicitly opts out by setting MCP_BRIDGE_NO_AUTOSTART (the in-process tests
+// that `require()` this module do). Default-on is the whole point.
+//
+// We intentionally do NOT gate on `require.main === module`. Although the .mcpb
+// manifest launches `node ${__dirname}/server.js` (where that check would be
+// true), Claude Desktop actually runs the bundle through 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 the 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.
+//
+// We also avoid gating on Jest's own env (JEST_WORKER_ID): it isn't reliably
+// set under `--runInBand`, so a future in-band run would let the test's
+// require() fire main() and process.exit(1) — the same boot-trap class. A
+// bridge-owned opt-out keeps the boot contract declared here, not inferred.
+if (!process.env.MCP_BRIDGE_NO_AUTOSTART) {
   main();
 }
 

tests/bridge-bootstrap.test.ts

@@ -0,0 +1,107 @@
+/**
+ * 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<void> }> {
+  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<Record<string, unknown>> {
+  return new Promise((resolve, reject) => {
+    // The child IS a real bridge launch, so it must autostart. Strip the
+    // opt-out in case a sibling test set it in this worker's process.env.
+    const env: NodeJS.ProcessEnv = { ...process.env, MCP_URL: mcpUrl, MCP_API_KEY: 'k' };
+    delete env.MCP_BRIDGE_NO_AUTOSTART;
+
+    const child = spawn(process.execPath, nodeArgs, { env, stdio: ['pipe', 'pipe', 'pipe'] });
+    let out = '';
+    let err = '';
+    const finish = (fn: () => void) => { clearTimeout(timer); child.kill(); fn(); };
+    const timer = setTimeout(() => finish(() => reject(new Error(`no initialize response (bridge hung). stderr: ${err}`))), 8000);
+    child.stderr.on('data', (c) => { err += c; });
+    child.stdout.on('data', (c) => {
+      out += c;
+      // Only parse a complete, newline-terminated line — a chunk split mid-line
+      // would otherwise throw on partial JSON and flake the test.
+      const nl = out.indexOf('\n');
+      if (nl === -1) return;
+      const line = out.slice(0, nl);
+      finish(() => {
+        try { resolve(JSON.parse(line)); } catch (e) { reject(e as Error); }
+      });
+    });
+    child.on('error', (e) => finish(() => 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<void> };
+  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);
+});

tests/bridge-self-heal.test.ts

@@ -14,6 +14,9 @@
  */
 
 // Must be set before the bridge module is required (it reads env at load).
+// MCP_BRIDGE_NO_AUTOSTART keeps the bootstrap inert so requiring the module
+// only exposes dispatch/state — it does not claim stdin or exit the process.
+process.env.MCP_BRIDGE_NO_AUTOSTART = '1';
 process.env.MCP_URL = 'http://localhost:3001/mcp';
 process.env.MCP_API_KEY = 'test-key';