Files
codegraph/__tests__/writer-lock.test.ts
Colby Mchenry 594bad7ec4 test(writer-lock): hold the lock with a live child, not a pid assumed alive (#2045)
The live-holder case wrote `pid: 1` and leaned on its own comment, "On Linux,
PID 1 is almost always alive". Windows has no PID 1, so tryAcquireWriterLock
read the holder as dead and acquired — the correct behaviour for a dead
holder, and a failing assertion. The platform-specific part was the fixture,
not the lock.

A parked child process is alive on every platform, and not stealing from a
live holder is what the lock actually promises, so the case now tests the
promise rather than a number that happens to name a running process. The child
is reaped in afterEach.

Fixes #1752.

(cherry picked from commit bee140288d)

Co-authored-by: Aaron Queen <bompus@users.noreply.github.com>
2026-09-27 15:01:02 +00:00

140 lines
5.0 KiB
TypeScript

/**
* Project writer lock (#1740) — unit coverage for acquire / re-entrant /
* stale-dead-pid / live-holder refusal.
*/
import { afterEach, describe, expect, it } from 'vitest';
import { spawn, type ChildProcess } from 'child_process';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { MCPEngine } from '../src/mcp/engine';
import {
decodeWriterLockInfo,
getWriterPidPath,
markWriterReady,
readWriterLock,
releaseWriterLock,
tryAcquireWriterLock,
writerLockHeldMessage,
} from '../src/mcp/writer-lock';
describe('writer lock (#1740)', () => {
let dir: string;
let holder: ChildProcess | null = null;
afterEach(() => {
try { holder?.kill('SIGKILL'); } catch { /* already gone */ }
holder = null;
if (dir) {
releaseWriterLock(dir);
try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ }
}
});
function makeProject(): string {
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg1740-lock-'));
fs.mkdirSync(path.join(dir, '.codegraph'), { recursive: true });
return dir;
}
it('acquires and releases writer.pid', () => {
const root = makeProject();
const r = tryAcquireWriterLock(root, 'direct');
expect(r.kind).toBe('acquired');
expect(fs.existsSync(getWriterPidPath(root))).toBe(true);
const info = decodeWriterLockInfo(fs.readFileSync(getWriterPidPath(root), 'utf8'));
expect(info?.pid).toBe(process.pid);
expect(info?.mode).toBe('direct');
releaseWriterLock(root);
expect(fs.existsSync(getWriterPidPath(root))).toBe(false);
});
it('is re-entrant for the same pid', () => {
const root = makeProject();
expect(tryAcquireWriterLock(root, 'daemon').kind).toBe('acquired');
const again = tryAcquireWriterLock(root, 'fallback');
expect(again.kind).toBe('acquired');
releaseWriterLock(root);
});
it('reports taken when a live foreign pid holds the lock', () => {
const root = makeProject();
// A foreign process that is genuinely alive, rather than a pid assumed to
// be: PID 1 is init on Linux but does not exist on Windows, where the lock
// then reads the holder as dead and correctly acquires — the assertion was
// failing on the fixture, not on the lock. A parked child is alive
// everywhere, and it is what the lock actually promises not to steal from.
// Uses the shared holder so afterEach reaps it (same as the fallback-
// engine case below).
holder = spawn(process.execPath, ['-e', 'setTimeout(() => {}, 60000)'], {
stdio: 'ignore',
});
if (!holder.pid) throw new Error('Failed to spawn writer-lock holder');
fs.writeFileSync(
getWriterPidPath(root),
JSON.stringify({ pid: holder.pid, mode: 'direct', startedAt: Date.now() }) + '\n',
{ flag: 'wx' },
);
const r = tryAcquireWriterLock(root, 'direct');
expect(r.kind).toBe('taken');
if (r.kind === 'taken') {
expect(r.existing?.pid).toBe(holder.pid);
const msg = writerLockHeldMessage(r.existing, r.pidPath);
expect(msg).toMatch(/writer lock held/i);
expect(msg).toMatch(/CODEGRAPH_NO_DAEMON/);
expect(msg).toMatch(/daemon stop/);
}
});
it('clears a stale dead-pid lock and acquires', () => {
const root = makeProject();
// Pick a pid that is extremely unlikely to be alive.
const deadPid = 2147483646;
fs.writeFileSync(
getWriterPidPath(root),
JSON.stringify({ pid: deadPid, mode: 'direct', startedAt: Date.now() }) + '\n',
);
const r = tryAcquireWriterLock(root, 'direct');
expect(r.kind).toBe('acquired');
releaseWriterLock(root);
});
it('publishes catch-up completion for readers without changing writer identity', () => {
const root = makeProject();
const acquired = tryAcquireWriterLock(root, 'direct');
expect(acquired.kind).toBe('acquired');
const before = readWriterLock(root);
expect(before?.ready).toBe(false);
markWriterReady(root);
expect(readWriterLock(root)).toEqual({ ...before, ready: true });
tryAcquireWriterLock(root, 'fallback');
expect(readWriterLock(root)).toEqual({ ...before, ready: true });
});
it('lets a fallback engine atomically claim and release writer ownership', () => {
const root = makeProject();
const engine = new MCPEngine({ writerLockRoot: root });
expect(decodeWriterLockInfo(fs.readFileSync(getWriterPidPath(root), 'utf8'))).toMatchObject({
pid: process.pid,
mode: 'fallback',
});
engine.stop();
expect(fs.existsSync(getWriterPidPath(root))).toBe(false);
});
it('rejects a fallback engine before opening when another process owns writer.pid', () => {
const root = makeProject();
holder = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'ignore' });
if (!holder.pid) throw new Error('Failed to spawn writer-lock holder');
fs.writeFileSync(
getWriterPidPath(root),
JSON.stringify({ pid: holder.pid, mode: 'daemon', startedAt: Date.now() }) + '\n',
);
expect(() => new MCPEngine({ writerLockRoot: root })).toThrow(/writer lock held/i);
});
});