From 34e48ff39b4765d3551c4f893850091b88ccd85a Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sat, 26 Sep 2026 10:44:28 +0300 Subject: [PATCH 1/2] fix(mcp): close pending sockets and allow read-only fallback (#1963) --- CHANGELOG.md | 1 + __tests__/mcp-daemon.test.ts | 34 +++++++++++++++++++++++++++------- src/mcp/daemon.ts | 9 +++++++++ src/mcp/index.ts | 9 ++++----- 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75535a0bbd..11a8696b83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### MCP / indexing +- A shared MCP daemon no longer hangs on shutdown behind a connection that never finished its handshake, and a fallback server whose project is being written by another process now answers read-only instead of refusing, so sessions keep CodeGraph. (#1963) - Daemon startup and cleanup now preserve live legacy PID-only locks while still reclaiming dead or identity-disproved records, preventing two writers from serving the same project. - Incremental sync now keeps edge rebinding crash-safe: replacing a resolved edge with its recovery reference commits atomically, so an interruption cannot permanently remove the relationship. - Status now detects committed but unindexed changes and restored edits without scanning every source file; thanks @inth3shadows. (#1829) diff --git a/__tests__/mcp-daemon.test.ts b/__tests__/mcp-daemon.test.ts index a39feb1b2e..66d9c1616f 100644 --- a/__tests__/mcp-daemon.test.ts +++ b/__tests__/mcp-daemon.test.ts @@ -35,6 +35,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { ChildProcessWithoutNullStreams, spawn } from 'child_process'; import * as fs from 'fs'; +import * as net from 'net'; import * as os from 'os'; import * as path from 'path'; import { CodeGraph } from '../src'; @@ -199,6 +200,25 @@ describe('Shared MCP daemon (issue #411)', () => { fs.rmSync(tempDir, { recursive: true, force: true }); }); + it.runIf(process.platform !== 'win32')('stops despite a socket still waiting for its client hello (#1963)', async () => { + const server = spawnServer(tempDir); + servers.push(server); + sendInitialize(server.child, `file://${tempDir}`, 1); + await waitFor(() => findResponse(server.stdout, 1), 10000); + const pid = await waitFor(() => readLockPid(realRoot), 10000); + const raw = net.connect(getDaemonSocketPath(realRoot)); + try { + await new Promise((resolve, reject) => { + raw.once('data', () => resolve()); + raw.once('error', reject); + }); + process.kill(pid, 'SIGTERM'); + expect(await waitProcessExit(pid, 1500)).toBe(true); + } finally { + raw.destroy(); + } + }, 20000); + it('two invocations share ONE detached daemon; both attach as proxies', async () => { const env = { CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '15000' }; @@ -394,9 +414,9 @@ describe('Shared MCP daemon (issue #411)', () => { params: { name: 'codegraph_status', arguments: {} }, }); const toolResponse = await waitFor(() => findResponse(second.stdout, 3), 5000); - expect(toolResponse).toMatchObject({ - error: { message: expect.stringContaining('writer lock held') }, - }); + expect(toolResponse.error).toBeUndefined(); + expect(toolResponse.result?.isError).not.toBe(true); + expect(JSON.stringify(toolResponse.result)).toContain('CodeGraph Status'); }, 50000); it('does not replace a live legacy lock with a second daemon', async () => { @@ -461,7 +481,7 @@ describe('Shared MCP daemon (issue #411)', () => { }); }, 30000); - it('proxy falls back to direct mode on a daemon version mismatch', async () => { + it('proxy falls back to read-only mode on a daemon version mismatch', async () => { const net = await import('net'); const sockPath = getDaemonSocketPath(realRoot); // Plant a live-pid lockfile so the launcher treats the lock as held, and a @@ -501,9 +521,9 @@ describe('Shared MCP daemon (issue #411)', () => { params: { name: 'codegraph_status', arguments: {} }, }); const toolResponse = await waitFor(() => findResponse(server.stdout, 2), 5000); - expect(toolResponse).toMatchObject({ - error: { message: expect.stringContaining('live daemon') }, - }); + expect(toolResponse.error).toBeUndefined(); + expect(toolResponse.result?.isError).not.toBe(true); + expect(JSON.stringify(toolResponse.result)).toContain('CodeGraph Status'); expect(fs.existsSync(path.join(realRoot, '.codegraph', 'writer.pid'))).toBe(false); } finally { await new Promise((resolve) => miniServer.close(() => resolve())); diff --git a/src/mcp/daemon.ts b/src/mcp/daemon.ts index d80dad5bea..4dcf5c7b72 100644 --- a/src/mcp/daemon.ts +++ b/src/mcp/daemon.ts @@ -170,6 +170,9 @@ export interface DaemonStartResult { */ export class Daemon { private server: net.Server | null = null; + // Includes sockets waiting for the optional client hello, before a session + // exists. server.close() waits for these too, so stop must destroy them. + private acceptedSockets = new Set(); private clients = new Set(); /** Per-client peer pids from the optional client-hello, for the liveness sweep. */ private clientPeers = new Map(); @@ -362,6 +365,8 @@ export class Daemon { try { session.stop(); } catch { /* best-effort */ } } this.clients.clear(); + for (const socket of this.acceptedSockets) socket.destroy(); + this.acceptedSockets.clear(); if (this.server) { await new Promise((resolve) => this.server!.close(() => resolve())); this.server = null; @@ -379,6 +384,9 @@ export class Daemon { } private handleConnection(socket: net.Socket): void { + if (this.stopping) { socket.destroy(); return; } + this.acceptedSockets.add(socket); + socket.once('close', () => this.acceptedSockets.delete(socket)); // Hello first so the proxy can verify versions before piping any // application bytes. The proxy reads exactly one line, then forwards. const hello: DaemonHello = { @@ -394,6 +402,7 @@ export class Daemon { // timeout, a non-hello first line, an early close — yields null pids and we // fall back to the socket-close lifecycle exactly as before (#692). void readClientHello(socket).then((peers) => { + if (this.stopping || socket.destroyed) { socket.destroy(); return; } const transport = new SocketTransport(socket); const session = new MCPSession(transport, this.engine, { explicitProjectPath: this.projectRoot, diff --git a/src/mcp/index.ts b/src/mcp/index.ts index c12ab81052..7938a806df 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -50,7 +50,6 @@ import { import { clearStaleDaemonArtifacts } from './daemon-registry'; import { connectWithHello, runLocalHandshakeProxy } from './proxy'; import { - getWriterPidPath, readWriterLock, releaseWriterLock, tryAcquireWriterLock, @@ -114,12 +113,12 @@ function makeFallbackEngine(root: string): MCPEngine { } const writer = readWriterLock(root); if (writer && writer.pid > 0 && isProcessAlive(writer.pid)) { - throw new Error(writerLockHeldMessage(writer, getWriterPidPath(root))); + // Another process owns updates. A fallback may still serve read-only WAL + // queries without claiming a second writer or starting a watcher (#1963). + return new MCPEngine({ watch: false }); } if (existing && isProcessAlive(existing.pid)) { - throw new Error( - `Cannot start an in-process fallback while live daemon pid ${existing.pid} holds the project lock.` - ); + return new MCPEngine({ watch: false }); } return new MCPEngine({ writerLockRoot: root }); } From 364d3795c02b6998fa40daeff32e5bc2fc89fe6f Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sun, 27 Sep 2026 09:49:45 +0300 Subject: [PATCH 2/2] fix(mcp): log when a fallback serves reads without auto-sync (#1963) A read-only fallback starts no watcher and holds no writer lock, so a session that quietly stopped syncing looked the same as a healthy one in the logs. Name the holder on stderr when the fallback goes read-only. Suggested by @bompus in #1979. Co-Authored-By: Claude Opus 5.5 --- __tests__/mcp-daemon.test.ts | 2 ++ src/mcp/index.ts | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/__tests__/mcp-daemon.test.ts b/__tests__/mcp-daemon.test.ts index 66d9c1616f..6519a2496b 100644 --- a/__tests__/mcp-daemon.test.ts +++ b/__tests__/mcp-daemon.test.ts @@ -417,6 +417,7 @@ describe('Shared MCP daemon (issue #411)', () => { expect(toolResponse.error).toBeUndefined(); expect(toolResponse.result?.isError).not.toBe(true); expect(JSON.stringify(toolResponse.result)).toContain('CodeGraph Status'); + expect(second.stderr.some((line) => line.includes('Serving reads in-process without auto-sync'))).toBe(true); }, 50000); it('does not replace a live legacy lock with a second daemon', async () => { @@ -525,6 +526,7 @@ describe('Shared MCP daemon (issue #411)', () => { expect(toolResponse.result?.isError).not.toBe(true); expect(JSON.stringify(toolResponse.result)).toContain('CodeGraph Status'); expect(fs.existsSync(path.join(realRoot, '.codegraph', 'writer.pid'))).toBe(false); + expect(server.stderr.some((l) => l.includes('Serving reads in-process without auto-sync: live daemon PID'))).toBe(true); } finally { await new Promise((resolve) => miniServer.close(() => resolve())); } diff --git a/src/mcp/index.ts b/src/mcp/index.ts index 7938a806df..cb6384bf53 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -92,6 +92,16 @@ const TAKEOVER_RETRY_DELAY_MS = 100; * legacy daemon. Plain-PID locks cannot prove daemon identity, but they still * prove that a process owns the legacy writer slot. */ +/** + * A fallback that serves reads without a watcher or a writer lock (#1963). + * Say so on stderr: otherwise a session that quietly stopped syncing looks + * the same as a healthy one in the logs. + */ +function readOnlyFallback(holder: string): MCPEngine { + process.stderr.write(`[CodeGraph MCP] Serving reads in-process without auto-sync: ${holder}.\n`); + return new MCPEngine({ watch: false }); +} + function makeFallbackEngine(root: string): MCPEngine { let existing: ReturnType = null; try { @@ -115,10 +125,10 @@ function makeFallbackEngine(root: string): MCPEngine { if (writer && writer.pid > 0 && isProcessAlive(writer.pid)) { // Another process owns updates. A fallback may still serve read-only WAL // queries without claiming a second writer or starting a watcher (#1963). - return new MCPEngine({ watch: false }); + return readOnlyFallback(`writer lock held by PID ${writer.pid} (${writer.mode} mode)`); } if (existing && isProcessAlive(existing.pid)) { - return new MCPEngine({ watch: false }); + return readOnlyFallback(`live daemon PID ${existing.pid} holds the project lock`); } return new MCPEngine({ writerLockRoot: root }); }