When an MCP host (opencode and others) SIGTERM's the shared daemon as a new session starts, the existing session's proxy used to exit on the dropped socket — silently losing CodeGraph for that session, and hanging any request in flight at the drop. The SIGTERM originates in the host's process-tree teardown, not in CodeGraph (nothing here signals another process), so the fix is proxy resilience, not chasing the signal. The local-handshake proxy now treats a daemon disconnect as recoverable rather than terminal: it falls back to its in-process engine for the rest of the session (the same path used when no daemon is reachable at startup, and what CODEGRAPH_NO_DAEMON does) and re-serves any requests that were in flight to the dead daemon, so the host never hangs. The proxy still exits when the HOST goes away (stdin close / PPID watchdog) — only daemon loss is now non-fatal. Also replaces the over-the-wire liveness-sweep test added in #712 — which was flaky under heavy parallel load (a raced raw-socket connect) — with a deterministic Daemon.reapDeadClients unit test. The client-hello round-trip is still exercised by every daemon test (the real proxy now sends it). Validated with a reproduction (proxy stays alive, in-flight request answered, post-drop request recovers) and a regression test in mcp-daemon.test.ts. Confirmed on macOS (full suite green) and a Windows 11 VM. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
80358a84d9
commit
471084dd6e
@@ -7,7 +7,7 @@
|
||||
* the full handshake + sweep is exercised end-to-end in `mcp-daemon.test.ts`.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { parseClientHelloLine, peerIsDead } from '../src/mcp/daemon';
|
||||
import { Daemon, parseClientHelloLine, peerIsDead } from '../src/mcp/daemon';
|
||||
|
||||
describe('parseClientHelloLine', () => {
|
||||
it('parses a well-formed client-hello', () => {
|
||||
@@ -67,3 +67,44 @@ describe('peerIsDead', () => {
|
||||
expect(peerIsDead({ pid: 100, hostPid: 42 }, aliveAll)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Daemon.reapDeadClients', () => {
|
||||
// Construct with idleTimeoutMs:0 so dropping the last client doesn't arm a real
|
||||
// idle timer. The constructor opens no sockets/DB, so this stays a fast unit test.
|
||||
const makeDaemon = () => new Daemon('/tmp/codegraph-reap-unit-test', { idleTimeoutMs: 0 }) as any;
|
||||
const fakeSession = () => ({ stopped: false, stop() { this.stopped = true; } });
|
||||
|
||||
it('drops clients with a dead peer and leaves live ones attached', () => {
|
||||
const d = makeDaemon();
|
||||
const dead = fakeSession();
|
||||
const live = fakeSession();
|
||||
d.clients.add(dead); d.clientPeers.set(dead, { pid: 111, hostPid: null });
|
||||
d.clients.add(live); d.clientPeers.set(live, { pid: 222, hostPid: null });
|
||||
|
||||
const reaped = d.reapDeadClients((pid: number) => pid !== 111); // 111 dead, 222 alive
|
||||
|
||||
expect(reaped).toBe(1);
|
||||
expect(dead.stopped).toBe(true);
|
||||
expect(d.clients.has(dead)).toBe(false);
|
||||
expect(d.clientPeers.has(dead)).toBe(false); // peer record cleaned up too
|
||||
expect(d.clients.has(live)).toBe(true);
|
||||
});
|
||||
|
||||
it('never reaps a client with an unknown pid (no client-hello)', () => {
|
||||
const d = makeDaemon();
|
||||
const s = fakeSession();
|
||||
d.clients.add(s); d.clientPeers.set(s, { pid: null, hostPid: null });
|
||||
|
||||
expect(d.reapDeadClients(() => false)).toBe(0); // everything "dead", but pid unknown
|
||||
expect(d.clients.has(s)).toBe(true);
|
||||
});
|
||||
|
||||
it('reaps a client whose host pid is gone even if its proxy pid is alive', () => {
|
||||
const d = makeDaemon();
|
||||
const s = fakeSession();
|
||||
d.clients.add(s); d.clientPeers.set(s, { pid: 100, hostPid: 42 });
|
||||
|
||||
expect(d.reapDeadClients((pid: number) => pid !== 42)).toBe(1); // proxy 100 alive, host 42 dead
|
||||
expect(d.clients.has(s)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -143,16 +143,6 @@ function readLockPid(root: string): number | null {
|
||||
} catch { return null; }
|
||||
}
|
||||
|
||||
/** The socket path the daemon actually bound, as it recorded in its lockfile —
|
||||
* robust on Windows where a recomputed pipe path can differ from the daemon's. */
|
||||
function readLockSocketPath(root: string): string | null {
|
||||
try {
|
||||
const raw = fs.readFileSync(path.join(root, '.codegraph', 'daemon.pid'), 'utf8');
|
||||
const info = JSON.parse(raw);
|
||||
return typeof info.socketPath === 'string' ? info.socketPath : null;
|
||||
} catch { return null; }
|
||||
}
|
||||
|
||||
function readDaemonLog(root: string): string {
|
||||
try { return fs.readFileSync(path.join(root, '.codegraph', 'daemon.log'), 'utf8'); }
|
||||
catch { return ''; }
|
||||
@@ -369,47 +359,11 @@ describe('Shared MCP daemon (issue #411)', () => {
|
||||
}
|
||||
}, 30000);
|
||||
|
||||
it('reaps a client whose process died without the socket closing (liveness sweep, #692)', async () => {
|
||||
const net = await import('net');
|
||||
// Bring a daemon up via a real proxy (a live client), sweep fast.
|
||||
const env = { CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '30000', CODEGRAPH_DAEMON_CLIENT_SWEEP_MS: '300' };
|
||||
const server = spawnServer(tempDir, env);
|
||||
servers.push(server);
|
||||
sendInitialize(server.child, `file://${tempDir}`, 1);
|
||||
await waitFor(() => findResponse(server.stdout, 1), 10000);
|
||||
await waitFor(() => (readLockPid(realRoot) ?? 0) > 0, 8000);
|
||||
|
||||
// Connect a RAW client that announces a dead pid and then never closes its
|
||||
// socket — the exact phantom-client shape the sweep exists to catch. Use the
|
||||
// socket path the daemon recorded in its lockfile (robust on Windows, where
|
||||
// a recomputed named-pipe path can differ from the one the daemon bound).
|
||||
const sockPath = await waitFor(() => readLockSocketPath(realRoot), 8000);
|
||||
const raw = net.createConnection(sockPath);
|
||||
raw.on('error', () => { /* ignore — we destroy it ourselves */ });
|
||||
try {
|
||||
// Consume the daemon hello (one line), then send our client-hello.
|
||||
// Generous timeouts: the unref'd sweep interval can stretch under a busy
|
||||
// event loop (engine init / a loaded CI box), so don't race it tight.
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
let buf = '';
|
||||
const to = setTimeout(() => reject(new Error('no daemon hello within 15s')), 15000);
|
||||
raw.on('data', (c: Buffer) => {
|
||||
buf += c.toString('utf8');
|
||||
if (buf.includes('\n')) { clearTimeout(to); resolve(); }
|
||||
});
|
||||
});
|
||||
raw.write(JSON.stringify({ codegraph_client: 1, pid: 999_999, hostPid: null }) + '\n');
|
||||
|
||||
// The sweep should detect pid 999999 is dead and reap that client.
|
||||
await waitFor(
|
||||
() => readDaemonLog(realRoot).includes('Reaping client with dead peer (pid 999999'),
|
||||
15000,
|
||||
);
|
||||
} finally {
|
||||
raw.destroy();
|
||||
}
|
||||
}, 60000);
|
||||
|
||||
// The over-the-wire client-hello → record → sweep path is covered by the
|
||||
// deterministic `Daemon.reapDeadClients` unit test in daemon-client-liveness
|
||||
// (a raw-socket variant here was flaky under heavy parallel load), plus the
|
||||
// client-hello round-trip exercised by every test above (the real proxy now
|
||||
// sends it). What stays here is the lifecycle behavior that needs real procs.
|
||||
it('exits on the inactivity backstop even while a client stays connected (#692)', async () => {
|
||||
// Backstop short, idle timeout long: with a client connected the idle timer
|
||||
// never arms, so only the inactivity backstop can take the daemon down.
|
||||
@@ -445,4 +399,34 @@ describe('Shared MCP daemon (issue #411)', () => {
|
||||
expect(await waitProcessExit(daemonPid, 10000)).toBe(true);
|
||||
expect(fs.existsSync(path.join(realRoot, '.codegraph', 'daemon.pid'))).toBe(false);
|
||||
}, 30000);
|
||||
|
||||
it('proxy survives the daemon dying mid-session and keeps serving (#662)', async () => {
|
||||
// The #662 scenario: an MCP host SIGTERM's the shared daemon while a session
|
||||
// is live. The proxy must NOT exit (losing CodeGraph for that session) — it
|
||||
// falls back to an in-process engine and keeps answering.
|
||||
const env = { CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '30000', CODEGRAPH_PPID_POLL_MS: '5000' };
|
||||
const server = spawnServer(tempDir, env);
|
||||
servers.push(server);
|
||||
sendInitialize(server.child, `file://${tempDir}`, 1);
|
||||
await waitFor(() => findResponse(server.stdout, 1), 10000);
|
||||
await waitFor(() => server.stderr.some((l) => l.includes('Attached to shared daemon')), 8000);
|
||||
await waitFor(() => (readLockPid(realRoot) ?? 0) > 0, 8000);
|
||||
const daemonPid = readLockPid(realRoot)!;
|
||||
|
||||
// A warm call goes through the daemon.
|
||||
sendMessage(server.child, { jsonrpc: '2.0', id: 2, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } });
|
||||
await waitFor(() => findResponse(server.stdout, 2), 10000);
|
||||
|
||||
// Kill the daemon out from under the live proxy.
|
||||
process.kill(daemonPid, 'SIGTERM');
|
||||
expect(await waitProcessExit(daemonPid, 8000)).toBe(true);
|
||||
|
||||
// The proxy must still be alive and still answer — served in-process now.
|
||||
expect(isAlive(server.child.pid!)).toBe(true);
|
||||
await waitFor(() => server.stderr.some((l) => l.includes('serving this session in-process')), 8000);
|
||||
sendMessage(server.child, { jsonrpc: '2.0', id: 3, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } });
|
||||
const resp = await waitFor(() => findResponse(server.stdout, 3), 15000);
|
||||
expect(resp.result !== undefined || resp.error !== undefined).toBe(true);
|
||||
expect(isAlive(server.child.pid!)).toBe(true);
|
||||
}, 45000);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user