mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-18 22:02:49 +08:00
* test(mcp): reap the daemon-mode writer so the temp project can be removed In daemon mode the writer is a third process spawned by the proxies and never recorded in children -- the test asserts its pid is neither child's. Killing only the proxies left it holding the index, so fs.rmSync failed and its catch swallowed the error: one temp directory and one live serve --mcp process leaked per run while both tests passed. writer.pid already names the daemon and the test already parses it, so afterEach now reads it before killing the proxies and signals the writer too. The removal is no longer silenced, because the swallowed error is what made this invisible. Measured on Windows 11 with TEMP redirected to a private directory: before, 1 leaked directory and 1 surviving process per run; after, 0 and 0, both tests still passing. Fixes #1782. * fix(mcp): reap writer-lock test daemon and verify cleanup (#1782) The writer-lock test stopped its proxies but left their daemon running, hiding directory removal failures. Preserve the contributor fix and add verified daemon shutdown, bounded exit polling, and cleanup assertions. Ensure the daemon test overrides inherited direct-mode configuration. Co-authored-by: Aaron Queen <bompus@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Aaron Queen <bompus@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -10,6 +10,7 @@ import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { CodeGraph } from '../src';
|
||||
import { getWriterPidPath } from '../src/mcp/writer-lock';
|
||||
import { isProcessAlive, stopDaemonAt } from '../src/mcp/daemon-registry';
|
||||
|
||||
const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js');
|
||||
|
||||
@@ -34,6 +35,27 @@ function spawnMcp(
|
||||
return { child, getStderr: () => stderr };
|
||||
}
|
||||
|
||||
function readWriterPid(root: string): number | undefined {
|
||||
try {
|
||||
const { pid } = JSON.parse(fs.readFileSync(getWriterPidPath(root), 'utf8')) as { pid?: number };
|
||||
return typeof pid === 'number' && Number.isSafeInteger(pid) && pid > 0 && pid !== process.pid
|
||||
? pid : undefined;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
async function stopChild(child: ChildProcessWithoutNullStreams): Promise<void> {
|
||||
const exited = () => child.exitCode !== null || child.signalCode !== null;
|
||||
for (const signal of ['SIGTERM', 'SIGKILL'] as const) {
|
||||
if (exited()) return;
|
||||
child.kill(signal);
|
||||
const deadline = Date.now() + 3000;
|
||||
while (!exited() && Date.now() < deadline) await sleep(25);
|
||||
}
|
||||
expect(exited(), `Child ${child.pid} did not exit`).toBe(true);
|
||||
}
|
||||
|
||||
describe('issue #1740 — direct-mode writer lock', () => {
|
||||
let tempDir: string;
|
||||
let realRoot: string;
|
||||
@@ -49,14 +71,32 @@ describe('issue #1740 — direct-mode writer lock', () => {
|
||||
cg.close();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
for (const c of children) {
|
||||
try { c.kill('SIGTERM'); } catch { /* ignore */ }
|
||||
async function cleanup(): Promise<void> {
|
||||
// Capture the fixture writer before stopping its proxies (#1782). The
|
||||
// daemon stop helper verifies its socket identity before signaling it.
|
||||
const childPids = new Set(children.map((c) => c.pid));
|
||||
const writerPid = readWriterPid(realRoot);
|
||||
const results = await Promise.allSettled([
|
||||
...children.map(stopChild),
|
||||
...(writerPid !== undefined && !childPids.has(writerPid)
|
||||
? [stopDaemonAt(realRoot).then((result) => {
|
||||
expect(result.pid).toBe(writerPid);
|
||||
expect(result.outcome).not.toBe('unverified');
|
||||
})]
|
||||
: []),
|
||||
]);
|
||||
for (const result of results) {
|
||||
if (result.status === 'rejected') throw result.reason;
|
||||
}
|
||||
if (writerPid !== undefined) {
|
||||
expect(isProcessAlive(writerPid), `Writer ${writerPid} did not exit`).toBe(false);
|
||||
}
|
||||
children.length = 0;
|
||||
await sleep(300);
|
||||
try { fs.rmSync(tempDir, { recursive: true, force: true }); } catch { /* ignore */ }
|
||||
});
|
||||
fs.rmSync(tempDir, { recursive: true, force: true, maxRetries: 10 });
|
||||
expect(fs.existsSync(tempDir)).toBe(false);
|
||||
}
|
||||
|
||||
afterEach(cleanup, 15000);
|
||||
|
||||
it('second CODEGRAPH_NO_DAEMON serve --mcp exits with writer-lock error', async () => {
|
||||
const env = {
|
||||
@@ -100,6 +140,7 @@ describe('issue #1740 — direct-mode writer lock', () => {
|
||||
|
||||
it('default daemon mode still allows two proxies to share one writer', async () => {
|
||||
const env = {
|
||||
CODEGRAPH_NO_DAEMON: '0',
|
||||
CODEGRAPH_MCP_LOG_ATTACH: '1',
|
||||
CODEGRAPH_NO_WATCHDOG: '1',
|
||||
CODEGRAPH_STARTUP_HANDSHAKE_TIMEOUT_MS: '0',
|
||||
@@ -124,5 +165,13 @@ describe('issue #1740 — direct-mode writer lock', () => {
|
||||
expect(lock.mode).toBe('daemon');
|
||||
expect(lock.pid).not.toBe(a.child.pid);
|
||||
expect(lock.pid).not.toBe(b.child.pid);
|
||||
|
||||
// This must catch a live writer even on POSIX, where unlinking its open
|
||||
// database would otherwise hide the leak that blocks removal on Windows.
|
||||
await cleanup();
|
||||
expect(isProcessAlive(lock.pid)).toBe(false);
|
||||
expect(isProcessAlive(a.child.pid!)).toBe(false);
|
||||
expect(isProcessAlive(b.child.pid!)).toBe(false);
|
||||
expect(fs.existsSync(tempDir)).toBe(false);
|
||||
}, 25000);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user