fix(mcp): prevent "Transport closed" from a stray daemon-socket error (#974) (#983)

The client-facing MCP proxy could exit with "Transport closed" when its
connection to the shared daemon hit a socket 'error' with no listener
attached — common on WSL2 /mnt (DrvFs), where AF_UNIX is flaky. The global
fatal handler turned that uncaughtException into process.exit(1), which the
MCP client saw as a bare transport close even though the index was healthy.

proxy.ts now keeps an 'error' listener on the daemon socket for its whole
life (and skips a socket destroyed in the connect window), so a stray error
degrades to the existing in-process fallback instead of crashing. daemon.ts
releases the lockfile it acquired when it fails to bind, so the next launch
doesn't spin on a stale lock (the duplicate serve --mcp pileup).

No default behavior change for anyone; WSL /mnt users who still hit trouble
can set CODEGRAPH_NO_DAEMON=1 to skip the shared daemon entirely. Validated
on macOS (unit + live serve probe) and Linux (Docker, --init): 64/64 across
the daemon/socket/lifecycle suites, incl. real AF_UNIX.

Closes #974

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-06-24 14:48:16 -05:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 1e48861cfb
commit 7c6417ef8f
6 changed files with 164 additions and 1 deletions
+55
View File
@@ -0,0 +1,55 @@
/**
* Daemon bind-failure cleanup — issue #974.
*
* A detached daemon acquires the `.codegraph/daemon.pid` lock (via
* `tryAcquireDaemonLock`) BEFORE it binds its socket. If the bind then fails —
* e.g. AF_UNIX is unsupported/unreliable on the filesystem (the WSL2 DrvFs
* hazard behind #974) — `Daemon.start()` must release that lockfile before it
* propagates the error and exits. Otherwise the next launcher reads a stale lock
* pointing at the now-dead pid and the process pileup the issue reported recurs.
*
* We force a deterministic bind failure by planting a *directory* at the socket
* path: `unlinkSync` (the daemon's stale-socket clear) can't remove a directory,
* so it survives and `listen()` fails with EADDRINUSE.
*/
import { afterEach, describe, expect, it } from 'vitest';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { Daemon, tryAcquireDaemonLock } from '../src/mcp/daemon';
import { getDaemonPidPath, getDaemonSocketPath } from '../src/mcp/daemon-paths';
const tmpRoots: string[] = [];
afterEach(() => {
while (tmpRoots.length) {
const root = tmpRoots.pop()!;
try { fs.rmSync(root, { recursive: true, force: true }); } catch { /* best-effort */ }
}
});
describe('Daemon.start() bind failure (#974)', () => {
it.runIf(process.platform !== 'win32')('releases the lockfile it acquired when the socket cannot bind', async () => {
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-bind-'));
tmpRoots.push(root);
// Acquire the lock exactly as the detached-daemon startup does.
const lock = tryAcquireDaemonLock(root);
expect(lock.kind).toBe('acquired');
const pidPath = getDaemonPidPath(root);
expect(fs.existsSync(pidPath)).toBe(true);
// Make the socket path un-bindable: a directory can't be unlink'd by the
// daemon's stale-socket clear, and listen() on it fails with EADDRINUSE.
const sockPath = getDaemonSocketPath(root);
fs.mkdirSync(sockPath, { recursive: true });
// The tmpdir-fallback socket path can live outside `root`; clean it too.
tmpRoots.push(sockPath);
const daemon = new Daemon(root);
await expect(daemon.start()).rejects.toThrow();
// The lockfile must be gone so the next launcher doesn't spin on a stale lock.
expect(fs.existsSync(pidPath)).toBe(false);
});
});
+78
View File
@@ -0,0 +1,78 @@
/**
* Proxy connect resilience — issue #974.
*
* `connectWithHello` returns a live socket to the caller, which then attaches
* its own onDaemonLost handler. Before #974, `readHelloLine` attached an
* 'error' listener and REMOVED it on success, leaving a window where the socket
* had no 'error' listener — and a socket 'error' with no listener is re-thrown
* by Node as an uncaughtException, which the global fatal handler turns into
* process.exit(1). To an MCP client that is a bare "Transport closed". The fix
* keeps a guard 'error' listener attached for the socket's whole life.
*
* AF_UNIX over WSL2/DrvFs makes that window common; here we just prove the
* invariant on a normal socket: the returned socket always has an 'error'
* listener, and emitting an error on it never throws.
*/
import { afterEach, describe, expect, it } from 'vitest';
import * as net from 'net';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { connectWithHello } from '../src/mcp/proxy';
import { CodeGraphPackageVersion } from '../src/mcp/version';
const cleanups: Array<() => void> = [];
afterEach(() => {
while (cleanups.length) {
try { cleanups.pop()!(); } catch { /* best-effort */ }
}
});
/** Stand up a fake daemon that emits a valid hello line on connect. */
async function fakeDaemon(version: string): Promise<{ sockPath: string; server: net.Server }> {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-proxy-'));
const sockPath = path.join(dir, 'd.sock');
const server = net.createServer((socket) => {
const hello = { codegraph: version, pid: process.pid, socketPath: sockPath, protocol: 1 };
socket.write(JSON.stringify(hello) + '\n');
});
await new Promise<void>((resolve) => server.listen(sockPath, resolve));
cleanups.push(() => server.close());
cleanups.push(() => { try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } });
return { sockPath, server };
}
describe('connectWithHello — socket is never left without an error listener (#974)', () => {
it.runIf(process.platform !== 'win32')('returns a socket that has an error listener and never throws on error', async () => {
const { sockPath } = await fakeDaemon(CodeGraphPackageVersion);
const result = await connectWithHello(sockPath);
expect(result).not.toBeNull();
expect(result).not.toBe('version-mismatch');
const socket = result as net.Socket;
cleanups.push(() => socket.destroy());
// The invariant: a guard 'error' listener is attached for the socket's whole
// life, so a stray socket error can't escalate to an uncaughtException.
expect(socket.listenerCount('error')).toBeGreaterThanOrEqual(1);
// Emitting an error must NOT throw. Without the guard this is exactly the
// path that crashed the proxy with "Transport closed".
expect(() => socket.emit('error', new Error('simulated ECONNRESET'))).not.toThrow();
});
it.runIf(process.platform !== 'win32')('still reports version-mismatch (and that path does not throw)', async () => {
const { sockPath } = await fakeDaemon('0.0.0-not-our-version');
const result = await connectWithHello(sockPath);
expect(result).toBe('version-mismatch');
});
it.runIf(process.platform !== 'win32')('returns null when no daemon is listening', async () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-proxy-none-'));
cleanups.push(() => { try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } });
const result = await connectWithHello(path.join(dir, 'missing.sock'));
expect(result).toBeNull();
});
});