mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-25 03:49:59 +08:00
codegraph index told you to run `codegraph daemon stop`, which the CLI
rejects ("too many arguments"), whenever it could not verify the process
named in .codegraph/daemon.pid. In that state `codegraph daemon` lists
nothing, because it lists only daemons that answer their socket. And
`codegraph unlock` kept an old-format lock whose process was alive (the
rule from issue #1834), so nothing the CLI offered got past it. The
writer-lock message gave the same wrong advice.
Root cause: a live pid in the lock was the only evidence, and a pid
proves nothing once the OS gives it to another program. Every CodeGraph
daemon since the first one (v0.9.5) runs
`<runtime> ... codegraph.js serve --mcp --path <root>`, so the holder's
command line settles the question.
Fix:
- New src/mcp/process-identity.ts reads a process's command line and
executable name from the OS: Win32_Process through PowerShell (tasklist
as fallback) on Windows, /proc on Linux, ps elsewhere. Each lookup is
capped at 5 s and never throws. A holder is a CodeGraph MCP server,
provably something else, or unknown, and unknown is never evidence.
- stopDaemonAt treats an unverified live holder that is provably not a
CodeGraph server like a dead one. Its lock is cleared, under the same
writer-slot claim, and the process is never signalled.
clearStaleDaemonArtifacts (codegraph unlock) does the same for an
old-format lock. A holder that may be a daemon keeps its lock, as
before (issues #1553 and #1834).
- daemonStopFailureMessage builds index's error from what remains: the
pid, what it runs, and a command that clears it (taskkill /PID N /F or
kill N), or which lock file to delete when only that will do.
- writerLockHeldMessage names `codegraph daemon` instead of the missing
`codegraph daemon stop`.
Validation (Windows 11):
- New daemon-lock-holder.test.ts (11 tests) and a daemon-registry test.
With main's daemon-registry.ts, writer-lock.ts and codegraph.ts put
back, 11 tests fail, including all 8 CLI cases and the writer-lock
wording; with the fix all pass. Four existing tests used generic
processes to stand in for a daemon that doesn't answer. Those stand-ins
now carry a CodeGraph server's command line, and pass on main and here.
- Real CLI with a private HOME, 4 cases: an old-format lock naming a live
PING.EXE; an old-format lock naming a live Node process that isn't
CodeGraph; a current-format lock naming that kind of Node process with
a pipe nobody listens on; an old-format lock naming a Node process whose
command line reads `codegraph.js serve --mcp`. On main all 4 index runs
exit 1 with the old advice, `daemon stop` is rejected, `daemon` lists
nothing, and unlock clears only the third. With the fix the first 3
rebuild (exit 0), each holder stays alive, and unlock clears the first
too. The 4th exits 1, naming the pid and `taskkill /PID <pid> /F`;
after that command, index exits 0.
Contributor PR #2373 (@danusha2345) changes the two strings only; I read
it for guidance.
Fixes #2770
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
143 lines
5.2 KiB
TypeScript
143 lines
5.2 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/);
|
|
// `codegraph daemon` is the picker that stops the shared daemon; the CLI
|
|
// rejects `codegraph daemon stop` (#2770).
|
|
expect(msg).toContain('pick it in `codegraph daemon`');
|
|
expect(msg).not.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);
|
|
});
|
|
});
|