fix(bridge): recover concurrent 404s; address review of #243

Code review of the self-heal path surfaced a concurrency correctness
gap and some observability refinements.

Correctness: the 404 self-heal was gated on `response.status === 404
&& sessionId`. When two tool calls 404 concurrently on a dead
session, the first call's reinitialize() synchronously nulls
sessionId before the second call's 404 is handled, so the sibling
failed the `&& sessionId` guard, skipped self-heal, and surfaced
-32603 "Bridge: HTTP 404" instead of recovering. Drop the guard: a
404 on /mcp always means a session id was presented (the server
answers 400 when none is), so any non-initialize 404 is a self-heal
trigger regardless of current bridge state. Both concurrent calls now
coalesce onto the single in-flight reinit and replay under the fresh
session. Added a concurrent-dispatch regression test proving one
reinit + both replays under the new session id.

Observability: log a non-2xx (or thrown) notifications/initialized
during reinit so a required-but-failing handshake notification is
diagnosable from stderr; soften the comment to match its best-effort
reality; note the startNotifyStream fire-and-forget intent. Align the
test's __state() type with the seam.

Claude-Session: https://claude.ai/code/session_011yowKCMFCBp61DRBu5xQm4

mcpb/server.js

@@ -122,13 +122,23 @@ async function reinitialize() {
   }
   sessionId = sid;
 
-  // Best-effort: a stateful server expects `notifications/initialized` before
-  // it will service tool calls. It's a notification (202, no body to emit).
+  // Complete the handshake. If the server gates tool calls on
+  // `notifications/initialized` this is load-bearing; if not, it's harmless.
+  // Either way it's best-effort — a lost notification just means the replay
+  // 404s again and recovery terminates cleanly via the single-attempt cap in
+  // dispatch. Log a non-2xx (or a throw) so a required-but-failing
+  // notification is diagnosable from stderr instead of silently swallowed.
   try {
     const note = await postOnce({ jsonrpc: '2.0', method: 'notifications/initialized' });
+    if (note.status !== 200 && note.status !== 202) {
+      process.stderr.write(`[obsidian-mcp-bridge] reinit: notifications/initialized → HTTP ${note.status}\n`);
+    }
     await note.body?.cancel().catch(() => {});
-  } catch { /* server may not require it; the replay will reveal the truth */ }
+  } catch (e) {
+    process.stderr.write(`[obsidian-mcp-bridge] reinit: notifications/initialized failed: ${e.message}\n`);
+  }
 
+  // Fire-and-forget; startNotifyStream self-handles its own errors internally.
   startNotifyStream();
   return true;
 }
@@ -161,13 +171,21 @@ async function dispatch(message, isReplay = false) {
     }
   }
 
-  if (response.status === 404 && sessionId) {
-    // The server evicted/lost our session (idle GC, or a server restart). Per
-    // MCP spec the 404 means "re-initialize," but some clients (e.g. Claude
-    // Desktop) don't act on it and the connection dead-ends. Self-heal once:
-    // re-establish a session and replay this exact message so recovery is
-    // invisible to the client (#238). `isReplay` caps it at a single attempt
-    // so a session that dies again instantly can't spin a reinit loop.
+  if (response.status === 404) {
+    // A 404 on /mcp means the server terminated/lost our session (idle GC or a
+    // server restart); per MCP spec the bridge should re-initialize, but some
+    // clients (e.g. Claude Desktop) don't act on the signal and the connection
+    // dead-ends. We deliberately do NOT gate on the bridge's current
+    // `sessionId`: a concurrent sibling's reinit may have already nulled it,
+    // and the 404 itself proves a session id was presented on this request
+    // (the server answers 400, handled below, when none is). So any 404 on a
+    // non-initialize call is a self-heal trigger.
+    //
+    // Self-heal once: re-establish a session and replay this exact message so
+    // recovery is invisible to the client (#238). Concurrent 404s coalesce
+    // onto a single reinit via the `initializing` gate and each replays under
+    // the fresh session. `isReplay` caps it at one attempt, so a session that
+    // dies again instantly terminates with -32000 rather than looping.
     if (!isReplay && message.method !== 'initialize' && cachedInitialize) {
       if (!initializing) {
         initializing = reinitialize().finally(() => { initializing = null; });

tests/bridge-self-heal.test.ts

@@ -21,7 +21,7 @@ process.env.MCP_API_KEY = 'test-key';
 const bridge = require('../mcpb/server.js') as {
   dispatch: (msg: unknown, isReplay?: boolean) => Promise<void>;
   __reset: () => void;
-  __state: () => { sessionId: string | null; cachedInitialize: unknown };
+  __state: () => { sessionId: string | null; cachedInitialize: unknown; initializing: unknown };
 };
 
 interface FakeResponse {
@@ -146,6 +146,36 @@ describe('mcpb bridge self-heal on session expiry (#238)', () => {
     expect(calls.filter(c => c.rpc === 'tools/call')).toHaveLength(1);
   });
 
+  test('concurrent 404s coalesce onto a single reinit; each call replays under the fresh session', async () => {
+    let sidSeq = 0;
+    installFetch((rec) => {
+      if (rec.rpc === 'initialize') {
+        sidSeq += 1;
+        return res(200, { sid: `sess-${sidSeq}`, json: { jsonrpc: '2.0', id: rec.id, result: {} } });
+      }
+      if (rec.rpc === 'notifications/initialized') return res(202);
+      if (rec.session === 'sess-1') return res(404, { sid: 'sess-1', json: { jsonrpc: '2.0', id: rec.id, error: { code: -32001, message: 'expired' } } });
+      return res(200, { json: { jsonrpc: '2.0', id: rec.id, result: { ok: true } } });
+    });
+
+    await bridge.dispatch(INIT);
+    // Two tool calls in flight together: both 404 on sess-1 and must coalesce
+    // onto one re-init rather than each kicking off its own handshake.
+    const A = { jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name: 'vault', arguments: { action: 'list' } } };
+    const B = { jsonrpc: '2.0', id: 2, method: 'tools/call', params: { name: 'vault', arguments: { action: 'read' } } };
+    await Promise.all([bridge.dispatch(A), bridge.dispatch(B)]);
+
+    // Both clients get genuine results, neither sees the expiry error.
+    expect((emitted.find(m => m.id === 1) as { result?: { ok?: boolean } } | undefined)?.result?.ok).toBe(true);
+    expect((emitted.find(m => m.id === 2) as { result?: { ok?: boolean } } | undefined)?.result?.ok).toBe(true);
+    expect(emitted.some(m => (m as { error?: { code?: number } }).error?.code === -32000)).toBe(false);
+    // Exactly one self-heal despite two concurrent 404s: 1 initial + 1 reinit.
+    expect(calls.filter(c => c.rpc === 'initialize')).toHaveLength(2);
+    // Each tool call was attempted on the dead session and replayed on the new.
+    expect(calls.filter(c => c.rpc === 'tools/call' && c.session === 'sess-1')).toHaveLength(2);
+    expect(calls.filter(c => c.rpc === 'tools/call' && c.session === 'sess-2')).toHaveLength(2);
+  });
+
   test('initialize handshake is cached so a later session loss can be replayed', async () => {
     installFetch((rec) => {
       if (rec.rpc === 'initialize') return res(200, { sid: 'sess-1', json: { jsonrpc: '2.0', id: rec.id, result: {} } });