From 48b0f3d0029e29e08e3aac6b4d0ddff652c6ab40 Mon Sep 17 00:00:00 2001 From: Aaron Queen Date: Fri, 2 Oct 2026 15:38:40 -0600 Subject: [PATCH] test(mcp): drive the pending-connection stop test by idle timeout "stop is not held open by an accepted connection that never sent its client-hello" stopped the daemon with SIGTERM. On Windows that ends the process without running its stop, so writer.pid was never removed and the test failed there on every run, testing nothing. The test now lets the daemon stop on its idle timeout, which runs the same stop on every platform. It sends a ping through the proxy, checks the daemon is still running, connects the pending socket and waits for the daemon's hello, and only then lets the proxy leave. A new CODEGRAPH_DAEMON_CLIENT_HELLO_TIMEOUT_MS override (default 3 s, like the daemon's other timing overrides) lets the test stretch the client-hello wait so the socket can't become a client before the stop runs. The test assumes the proxy registers within the daemon's 2 s startup idle window. If it doesn't, the daemon stops by design and the test fails before its assertion, at the alive check or the pending connection. Removing the stop's socket teardown still fails the test. --- __tests__/mcp-daemon.test.ts | 26 +++++++++++++++++++++----- src/mcp/daemon.ts | 17 ++++++++++++++--- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/__tests__/mcp-daemon.test.ts b/__tests__/mcp-daemon.test.ts index 6925556adf..6e4bd0eeab 100644 --- a/__tests__/mcp-daemon.test.ts +++ b/__tests__/mcp-daemon.test.ts @@ -859,21 +859,37 @@ describe('Shared MCP daemon (issue #411)', () => { // A launcher connecting just as the daemon stops is accepted but is not yet a // client. The stop used to wait on that socket with the writer lock held and // the socket gone, so every other new client failed until it closed. - const env = { CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '30000', CODEGRAPH_PPID_POLL_MS: '5000' }; + // The idle timeout drives the stop: SIGTERM on Windows ends the process + // without running it. The pending socket connects while the proxy is a + // registered client, so the idle timer can't fire first, and the long + // client-hello wait keeps it from becoming a client before the stop runs. + const env = { + CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '2000', + CODEGRAPH_DAEMON_CLIENT_HELLO_TIMEOUT_MS: '60000', + 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), 20000, 25, 'initialize response'); - // Attached means the daemon is listening with its SIGTERM handler installed. await waitFor(() => server.stderr.some((l) => l.includes('Attached to shared daemon')), 8000, 25, 'daemon attach log'); await waitFor(() => (readLockPid(realRoot) ?? 0) > 0, 8000, 25, 'daemon pidfile'); const daemonPid = readLockPid(realRoot)!; const { socketPath } = JSON.parse(fs.readFileSync(path.join(realRoot, '.codegraph', 'daemon.pid'), 'utf8')); + // The daemon arms its idle timer when it starts listening, so the proxy must + // register within those 2 s. If it doesn't, the daemon stops by design and + // the test fails before its assertion: at the alive check, or at the pending + // connection once the daemon has closed its listener. + sendMessage(server.child, { jsonrpc: '2.0', id: 2, method: 'ping' }); + await waitFor(() => findResponse(server.stdout, 2), 8000, 25, 'ping response'); + expect(isAlive(daemonPid)).toBe(true); const pending = net.connect(socketPath); - await new Promise((resolve, reject) => pending.once('connect', resolve).once('error', reject)); + // The daemon's hello line proves it accepted the connection. + await new Promise((resolve, reject) => pending.once('data', resolve).once('error', reject)); + // The only client leaves; the daemon idles out with the socket still pending. + server.child.stdin.end(); try { - process.kill(daemonPid, 'SIGTERM'); - expect(await waitProcessExit(daemonPid, 5000)).toBe(true); + expect(await waitProcessExit(daemonPid, 8000)).toBe(true); expect(fs.existsSync(path.join(realRoot, '.codegraph', 'writer.pid'))).toBe(false); } finally { pending.destroy(); diff --git a/src/mcp/daemon.ts b/src/mcp/daemon.ts index 7c7dc37780..6f23386114 100644 --- a/src/mcp/daemon.ts +++ b/src/mcp/daemon.ts @@ -118,8 +118,11 @@ export function finalizeDaemonExit( /** How often the daemon sweeps connected clients for a dead peer process (#692). */ const DEFAULT_CLIENT_SWEEP_MS = 30_000; -/** How long the daemon waits for the optional client-hello before proceeding without it. */ -const CLIENT_HELLO_TIMEOUT_MS = 3_000; +/** + * How long the daemon waits for the optional client-hello before proceeding + * without it. `CODEGRAPH_DAEMON_CLIENT_HELLO_TIMEOUT_MS` overrides it. + */ +const DEFAULT_CLIENT_HELLO_TIMEOUT_MS = 3_000; /** Bytes/parse-window for an oversized hello line — bounded against a malicious peer. */ const MAX_HELLO_LINE_BYTES = 4096; @@ -862,6 +865,14 @@ function resolveClientSweepMs(): number { return Math.floor(parsed); // 0 disables the sweep } +function resolveClientHelloTimeoutMs(): number { + const raw = process.env.CODEGRAPH_DAEMON_CLIENT_HELLO_TIMEOUT_MS; + if (raw === undefined || raw === '') return DEFAULT_CLIENT_HELLO_TIMEOUT_MS; + const parsed = Number(raw); + if (!Number.isFinite(parsed) || parsed <= 0) return DEFAULT_CLIENT_HELLO_TIMEOUT_MS; + return Math.floor(parsed); +} + /** * Parse one client-hello line. Returns the peer pids if `line` is a well-formed * client-hello (carries the `codegraph_client` marker), or null otherwise — in @@ -967,7 +978,7 @@ function readClientHello( const timer = setTimeout(() => { const partial = chunks.length === 0 ? undefined : (chunks.length === 1 ? chunks[0] : Buffer.concat(chunks, total)); finish({ pid: null, hostPid: null }, partial); - }, CLIENT_HELLO_TIMEOUT_MS); + }, resolveClientHelloTimeoutMs()); timer.unref?.(); socket.on('data', onData); socket.on('error', onEnd);