Browse Source

fix(daemon): preserve live legacy locks (#1834) (#1850)

Co-authored-by: Colby Mchenry <me@colbymchenry.com>
Christopher Beaulieu 4 days ago
parent
commit
1e46123758

+ 1 - 0
CHANGELOG.md

@@ -157,6 +157,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 #### MCP / indexing
 
+- Daemon startup and cleanup now preserve live legacy PID-only locks while still reclaiming dead or identity-disproved records, preventing two writers from serving the same project.
 - Incremental sync now keeps edge rebinding crash-safe: replacing a resolved edge with its recovery reference commits atomically, so an interruption cannot permanently remove the relationship.
 
 - The prompt hook no longer injects unrelated projects when run from your home directory or a broader directory containing a stray workspace manifest. (#1454)

+ 31 - 0
__tests__/cli-unlock.test.ts

@@ -99,4 +99,35 @@ describe('codegraph unlock — daemon artifact recovery (#1553)', () => {
       await new Promise<void>((resolve) => server.close(() => resolve()));
     }
   });
+
+  it('preserves a live legacy lock whose daemon identity cannot be probed', () => {
+    const pidPath = getDaemonPidPath(tempDir);
+    fs.writeFileSync(pidPath, `${process.pid}\n`);
+
+    const output = runCodegraph(['unlock', tempDir], tempDir);
+
+    expect(output).toContain('No stale lock files found');
+    expect(fs.readFileSync(pidPath, 'utf8')).toBe(`${process.pid}\n`);
+    expect(() => process.kill(process.pid, 0)).not.toThrow();
+  });
+
+  it('removes a legacy lock whose PID is dead', () => {
+    const pidPath = getDaemonPidPath(tempDir);
+    fs.writeFileSync(pidPath, '999999\n');
+
+    const output = runCodegraph(['unlock', tempDir], tempDir);
+
+    expect(output).toContain('Removed stale lock artifacts');
+    expect(fs.existsSync(pidPath)).toBe(false);
+  });
+
+  it('removes a malformed daemon lock', () => {
+    const pidPath = getDaemonPidPath(tempDir);
+    fs.writeFileSync(pidPath, 'not-a-lock\n');
+
+    const output = runCodegraph(['unlock', tempDir], tempDir);
+
+    expect(output).toContain('Removed stale lock artifacts');
+    expect(fs.existsSync(pidPath)).toBe(false);
+  });
 });

+ 29 - 0
__tests__/daemon-manager.test.ts

@@ -98,6 +98,35 @@ describe('runDaemonPicker', () => {
     expect(h.getDone()).toBe('Done.');
   });
 
+  it('does not report an unverified daemon as stopped', async () => {
+    const h = harness([rec('/p/a', 42, 1)], ['/p/a', CANCEL]);
+    h.deps.stop = async (root): Promise<StopResult> => ({
+      root,
+      pid: 42,
+      outcome: 'unverified',
+    });
+
+    await runDaemonPicker(h.deps);
+
+    expect(h.notes).toEqual([
+      'Could not verify daemon (pid 42); left it running with its artifacts intact — /p/a',
+    ]);
+    expect(h.getDone()).toContain('Cancelled');
+  });
+
+  it.each([
+    ['not-running', 42, 'Daemon was no longer running; removed stale artifacts — /p/a'],
+    ['no-daemon', null, 'No daemon was found — /p/a'],
+  ] as const)('reports the %s race outcome accurately', async (outcome, pid, message) => {
+    const h = harness([rec('/p/a', 42, 1)], ['/p/a', CANCEL]);
+    h.deps.stop = async (root): Promise<StopResult> => ({ root, pid, outcome });
+
+    await runDaemonPicker(h.deps);
+
+    expect(h.notes).toEqual([message]);
+    expect(h.getDone()).toContain('Cancelled');
+  });
+
   it('Cancel (and Esc/Ctrl-C) stop nothing', async () => {
     const h1 = harness([rec('/p/a', 1, 1)], [CANCEL]);
     await runDaemonPicker(h1.deps);

+ 85 - 0
__tests__/daemon-registry.test.ts

@@ -11,10 +11,12 @@ import {
   deregisterDaemon,
   listDaemons,
   listVerifiedDaemons,
+  clearStaleDaemonArtifacts,
   stopDaemonAt,
   type DaemonRecord,
 } from '../src/mcp/daemon-registry';
 import { encodeLockInfo, getDaemonPidPath } from '../src/mcp/daemon-paths';
+import { releaseWriterLock, tryAcquireWriterLock } from '../src/mcp/writer-lock';
 
 /** A pid that's guaranteed dead: spawn a trivial process, let it exit, reap it. */
 async function deadPid(): Promise<number> {
@@ -155,4 +157,87 @@ describe('daemon-registry', () => {
     expect(isProcessAlive(process.pid)).toBe(true);
     expect(fs.existsSync(pidPath)).toBe(false);
   });
+
+  it('preserves a live legacy lock when stop cannot verify daemon identity', async () => {
+    const root = fs.mkdtempSync(path.join(tmpHome, 'legacy-stop-'));
+    const pidPath = getDaemonPidPath(root);
+    fs.mkdirSync(path.dirname(pidPath), { recursive: true });
+    fs.writeFileSync(pidPath, `${process.pid}\n`);
+
+    const result = await stopDaemonAt(root);
+
+    expect(result).toMatchObject({ pid: process.pid, outcome: 'unverified' });
+    expect(fs.readFileSync(pidPath, 'utf8')).toBe(`${process.pid}\n`);
+    expect(isProcessAlive(process.pid)).toBe(true);
+  });
+
+  it('preserves a replacement lock written while stale identity is probed', async () => {
+    const root = fs.mkdtempSync(path.join(tmpHome, 'probe-race-'));
+    const pidPath = getDaemonPidPath(root);
+    const socketPath = process.platform === 'win32'
+      ? `\\\\.\\pipe\\cg-race-old-${process.pid}-${Date.now()}`
+      : path.join(tmpHome, 'probe-race-old.sock');
+    const replacementSocketPath = process.platform === 'win32'
+      ? `\\\\.\\pipe\\cg-race-new-${process.pid}-${Date.now()}`
+      : path.join(tmpHome, 'probe-race-new.sock');
+    let acceptConnection!: () => void;
+    const connected = new Promise<void>((resolve) => { acceptConnection = resolve; });
+    let acceptedSocket: net.Socket | null = null;
+    const server = net.createServer((socket) => {
+      acceptedSocket = socket;
+      acceptConnection();
+    });
+    await new Promise<void>((resolve, reject) => {
+      server.once('error', reject);
+      server.listen(socketPath, resolve);
+    });
+    const original = encodeLockInfo({
+      pid: process.pid,
+      version: '1.5.0',
+      socketPath,
+      startedAt: 1,
+    });
+    const replacement = encodeLockInfo({
+      pid: process.pid,
+      version: '1.5.0',
+      socketPath: replacementSocketPath,
+      startedAt: 2,
+    });
+    fs.mkdirSync(path.dirname(pidPath), { recursive: true });
+    fs.writeFileSync(pidPath, original);
+
+    try {
+      const clearing = clearStaleDaemonArtifacts(root);
+      await connected;
+      fs.writeFileSync(pidPath, replacement);
+      acceptedSocket!.end('{"protocol":0}\n');
+
+      expect(await clearing).toBe(false);
+      expect(fs.readFileSync(pidPath, 'utf8')).toBe(replacement);
+    } finally {
+      acceptedSocket?.destroy();
+      await new Promise<void>((resolve) => server.close(() => resolve()));
+    }
+  });
+
+  it('does not clean daemon artifacts while another writer owns the project', async () => {
+    const root = fs.mkdtempSync(path.join(tmpHome, 'writer-claim-'));
+    const pidPath = getDaemonPidPath(root);
+    const lock = encodeLockInfo({
+      pid: process.pid,
+      version: '1.5.0',
+      socketPath: path.join(root, '.codegraph', 'not-listening.sock'),
+      startedAt: 1,
+    });
+    fs.mkdirSync(path.dirname(pidPath), { recursive: true });
+    fs.writeFileSync(pidPath, lock);
+    expect(tryAcquireWriterLock(root, 'daemon').kind).toBe('acquired');
+
+    try {
+      expect(await clearStaleDaemonArtifacts(root)).toBe(false);
+      expect(fs.readFileSync(pidPath, 'utf8')).toBe(lock);
+    } finally {
+      releaseWriterLock(root);
+    }
+  });
 });

+ 42 - 0
__tests__/daemon-socket-fallback.test.ts

@@ -43,6 +43,7 @@ import { decodeLockInfo } from '../src/mcp/daemon-paths';
 import {
   acquireLockViaExclusiveOpen,
   bindFirstUsableSocket,
+  clearStaleDaemonLock,
   tryAcquireDaemonLock,
 } from '../src/mcp/daemon';
 
@@ -244,3 +245,44 @@ describe('lock acquisition without hard links (#997)', () => {
     expect(decodeLockInfo(fs.readFileSync(pidPath, 'utf8'))).toEqual(winner);
   });
 });
+
+describe('legacy daemon lock decoding', () => {
+  it('decodes a plain decimal PID as a legacy lock record', () => {
+    expect(decodeLockInfo('4242\n')).toEqual({
+      pid: 4242,
+      version: 'unknown',
+      socketPath: '',
+      startedAt: 0,
+    });
+  });
+
+  it.each(['1e3', '0x3e8', '1000.0'])('rejects non-decimal PID syntax %s', (raw) => {
+    expect(decodeLockInfo(raw)).toBeNull();
+  });
+});
+
+describe('stale daemon lock snapshot validation', () => {
+  it('does not delete a same-PID replacement whose identity was never probed', () => {
+    const pidPath = path.join(os.tmpdir(), `cg-snapshot-${process.pid}-${Date.now()}.pid`);
+    tmpFiles.push(pidPath);
+    const original = JSON.stringify({
+      pid: process.pid,
+      version: '1.5.0',
+      socketPath: '/old.sock',
+      startedAt: 1,
+    });
+    const replacement = JSON.stringify({
+      pid: process.pid,
+      version: '1.5.0',
+      socketPath: '/new.sock',
+      startedAt: 2,
+    });
+    fs.writeFileSync(pidPath, replacement);
+
+    expect(clearStaleDaemonLock(pidPath, process.pid, {
+      allowLivePid: true,
+      expectedLockContents: original,
+    })).toBe(false);
+    expect(fs.readFileSync(pidPath, 'utf8')).toBe(replacement);
+  });
+});

+ 119 - 16
__tests__/mcp-daemon.test.ts

@@ -337,7 +337,7 @@ describe('Shared MCP daemon (issue #411)', () => {
     expect(isAlive(livePid!)).toBe(true);
   }, 40000);
 
-  it('takes over after SIGKILL even when the stale PID has been reused (#1553)', async () => {
+  it('preserves paired daemon/writer locks when their live PID may have been reused', async () => {
     const env = { CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '30000' };
     const first = spawnServer(tempDir, env);
     servers.push(first);
@@ -351,30 +351,116 @@ describe('Shared MCP daemon (issue #411)', () => {
 
     // Model OS PID reuse without risking another process: the stale lock now
     // names this live vitest worker, but no daemon answers the leftover socket.
-    fs.writeFileSync(
-      path.join(realRoot, '.codegraph', 'daemon.pid'),
-      JSON.stringify({
-        pid: process.pid,
-        version: CodeGraphPackageVersion,
-        socketPath: getDaemonSocketPath(realRoot),
-        startedAt: Date.now() - 60_000,
-      }),
-    );
+    const daemonPath = path.join(realRoot, '.codegraph', 'daemon.pid');
+    const writerPath = path.join(realRoot, '.codegraph', 'writer.pid');
+    const staleDaemonLock = JSON.stringify({
+      pid: process.pid,
+      version: CodeGraphPackageVersion,
+      socketPath: getDaemonSocketPath(realRoot),
+      startedAt: Date.now() - 60_000,
+    });
+    const staleWriterLock = JSON.stringify({
+      pid: process.pid,
+      mode: 'daemon',
+      startedAt: Date.now() - 60_000,
+    }) + '\n';
+    fs.writeFileSync(daemonPath, staleDaemonLock);
+    fs.writeFileSync(writerPath, staleWriterLock);
 
     const second = spawnServer(tempDir, env);
     servers.push(second);
     sendInitialize(second.child, `file://${tempDir}`, 2);
     const response = await waitFor(() => findResponse(second.stdout, 2), 12000);
     expect(response.result.serverInfo.name).toBe('codegraph');
-    await waitFor(() => countListeningLines(realRoot) >= 2, 10000);
+    await waitFor(
+      () => second.stderr.some((line) =>
+        line.includes('Attached to shared daemon') || line.includes('Shared daemon unavailable')
+      ),
+      12000,
+      25,
+      'the proxy to attach or fall back',
+    );
 
-    const replacementPid = readLockPid(realRoot)!;
-    expect(replacementPid).not.toBe(killedPid);
-    expect(replacementPid).not.toBe(process.pid);
-    expect(isAlive(replacementPid)).toBe(true);
+    expect(second.stderr.some((line) => line.includes('Attached to shared daemon'))).toBe(false);
+    expect(countListeningLines(realRoot)).toBe(1);
+    expect(fs.readFileSync(daemonPath, 'utf8')).toBe(staleDaemonLock);
+    expect(fs.readFileSync(writerPath, 'utf8')).toBe(staleWriterLock);
     expect(isAlive(process.pid)).toBe(true);
+
+    sendMessage(second.child, {
+      jsonrpc: '2.0',
+      id: 3,
+      method: 'tools/call',
+      params: { name: 'codegraph_status', arguments: {} },
+    });
+    const toolResponse = await waitFor(() => findResponse(second.stdout, 3), 5000);
+    expect(toolResponse).toMatchObject({
+      error: { message: expect.stringContaining('writer lock held') },
+    });
   }, 50000);
 
+  it('does not replace a live legacy lock with a second daemon', async () => {
+    const pidPath = path.join(realRoot, '.codegraph', 'daemon.pid');
+    fs.writeFileSync(pidPath, `${process.pid}\n`);
+
+    const server = spawnServer(tempDir, { CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '15000' });
+    servers.push(server);
+    sendInitialize(server.child, `file://${tempDir}`, 1);
+    const response = await waitFor(() => findResponse(server.stdout, 1), 12000);
+    expect(response.result.serverInfo.name).toBe('codegraph');
+
+    await waitFor(
+      () => server.stderr.some((line) =>
+        line.includes('Attached to shared daemon') || line.includes('Shared daemon unavailable')
+      ),
+      12000,
+      25,
+      'the proxy to attach or fall back',
+    );
+
+    expect(server.stderr.some((line) => line.includes('Attached to shared daemon'))).toBe(false);
+    expect(fs.readFileSync(pidPath, 'utf8')).toBe(`${process.pid}\n`);
+    expect(countListeningLines(realRoot)).toBe(0);
+    expect(isAlive(process.pid)).toBe(true);
+
+    sendMessage(server.child, {
+      jsonrpc: '2.0',
+      id: 2,
+      method: 'tools/call',
+      params: { name: 'codegraph_status', arguments: {} },
+    });
+    const toolResponse = await waitFor(() => findResponse(server.stdout, 2), 5000);
+    expect(toolResponse).toMatchObject({
+      error: { message: expect.stringContaining('live legacy daemon') },
+    });
+  }, 30000);
+
+  it('does not start a fallback writer when the daemon lock is unreadable', async () => {
+    const pidPath = path.join(realRoot, '.codegraph', 'daemon.pid');
+    fs.mkdirSync(pidPath);
+
+    const server = spawnServer(tempDir);
+    servers.push(server);
+    sendInitialize(server.child, `file://${tempDir}`, 1);
+    await waitFor(
+      () => server.stderr.some((line) => line.includes('Shared daemon unavailable')),
+      12000,
+      25,
+      'the proxy to fall back',
+    );
+
+    sendMessage(server.child, {
+      jsonrpc: '2.0',
+      id: 2,
+      method: 'tools/call',
+      params: { name: 'codegraph_status', arguments: {} },
+    });
+    const toolResponse = await waitFor(() => findResponse(server.stdout, 2), 5000);
+    expect(toolResponse).toMatchObject({
+      error: { message: expect.stringContaining('daemon lock could not be read') },
+    });
+  }, 30000);
+
   it('proxy falls back to direct mode on a daemon version mismatch', async () => {
     const net = await import('net');
     const sockPath = getDaemonSocketPath(realRoot);
@@ -385,7 +471,12 @@ describe('Shared MCP daemon (issue #411)', () => {
       JSON.stringify({ pid: process.pid, version: '0.0.0-mismatch', socketPath: sockPath, startedAt: Date.now() }),
     );
     const miniServer = net.createServer((sock) => {
-      sock.write(JSON.stringify({ codegraph: '0.0.0-mismatch', pid: 1, socketPath: sockPath, protocol: 1 }) + '\n');
+      sock.write(JSON.stringify({
+        codegraph: '0.0.0-mismatch',
+        pid: process.pid,
+        socketPath: sockPath,
+        protocol: 1,
+      }) + '\n');
     });
     await new Promise<void>((resolve) => miniServer.listen(sockPath, () => resolve()));
 
@@ -402,6 +493,18 @@ describe('Shared MCP daemon (issue #411)', () => {
         () => server.stderr.some((l) => l.includes('serving this session in-process')),
         6000,
       );
+
+      sendMessage(server.child, {
+        jsonrpc: '2.0',
+        id: 2,
+        method: 'tools/call',
+        params: { name: 'codegraph_status', arguments: {} },
+      });
+      const toolResponse = await waitFor(() => findResponse(server.stdout, 2), 5000);
+      expect(toolResponse).toMatchObject({
+        error: { message: expect.stringContaining('live daemon') },
+      });
+      expect(fs.existsSync(path.join(realRoot, '.codegraph', 'writer.pid'))).toBe(false);
     } finally {
       await new Promise<void>((resolve) => miniServer.close(() => resolve()));
     }

+ 30 - 0
__tests__/writer-lock.test.ts

@@ -4,9 +4,11 @@
  */
 
 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,
@@ -17,8 +19,11 @@ import {
 
 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 */ }
@@ -84,4 +89,29 @@ describe('writer lock (#1740)', () => {
     expect(r.kind).toBe('acquired');
     releaseWriterLock(root);
   });
+
+  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);
+  });
 });

+ 14 - 0
src/mcp/daemon-manager.ts

@@ -109,6 +109,20 @@ export async function runDaemonPicker(deps: PickerDeps): Promise<void> {
     }
 
     const result = await deps.stop(String(choice));
+    if (result.outcome === 'unverified') {
+      deps.note(
+        `Could not verify daemon (pid ${result.pid}); left it running with its artifacts intact — ${choice}`
+      );
+      continue;
+    }
+    if (result.outcome === 'not-running') {
+      deps.note(`Daemon was no longer running; removed stale artifacts — ${choice}`);
+      continue;
+    }
+    if (result.outcome === 'no-daemon') {
+      deps.note(`No daemon was found — ${choice}`);
+      continue;
+    }
     const forced = result.outcome === 'kill' ? ', forced' : '';
     deps.note(`Stopped daemon (pid ${result.pid}${forced}) — ${choice}`);
     // Loop: the next iteration re-lists; if more remain it re-prompts, otherwise

+ 13 - 3
src/mcp/daemon-paths.ts

@@ -102,13 +102,23 @@ export interface DaemonLockInfo {
   startedAt: number;
 }
 
+/** Whether a lock record contains enough identity data for a socket hello. */
+export function canProbeDaemonIdentity(info: DaemonLockInfo): boolean {
+  return (
+    Number.isInteger(info.pid) &&
+    info.pid > 0 &&
+    typeof info.socketPath === 'string' &&
+    info.socketPath.length > 0
+  );
+}
+
 /**
  * Verify that the process named by a lockfile is the CodeGraph daemon serving
  * its socket. A bare PID liveness probe is insufficient because OSes reuse PIDs
  * after an OOM/SIGKILL (#1553).
  */
 export function probeDaemonIdentity(info: DaemonLockInfo, timeoutMs = 1_000): Promise<boolean> {
-  if (!Number.isInteger(info.pid) || info.pid <= 0 || !info.socketPath) return Promise.resolve(false);
+  if (!canProbeDaemonIdentity(info)) return Promise.resolve(false);
   return new Promise<boolean>((resolve) => {
     let socket: net.Socket;
     let buffer = '';
@@ -178,12 +188,12 @@ export function decodeLockInfo(raw: string): DaemonLockInfo | null {
     ) {
       return parsed as DaemonLockInfo;
     }
-    return null;
   } catch {
     // Fall through to legacy plain-pid handling.
   }
+  if (!/^[1-9]\d*$/.test(trimmed)) return null;
   const pid = Number(trimmed);
-  if (Number.isFinite(pid) && pid > 0) {
+  if (Number.isSafeInteger(pid)) {
     return { pid, version: 'unknown', socketPath: '', startedAt: 0 };
   }
   return null;

+ 65 - 24
src/mcp/daemon-registry.ts

@@ -26,9 +26,11 @@ import {
   getDaemonPidPath,
   getDaemonSocketCandidates,
   decodeLockInfo,
+  canProbeDaemonIdentity,
   probeDaemonIdentity,
   type DaemonLockInfo,
 } from './daemon-paths';
+import { readWriterLock, releaseWriterLock, tryAcquireWriterLock } from './writer-lock';
 
 export interface DaemonRecord {
   /** Realpath'd project root the daemon serves. */
@@ -140,18 +142,45 @@ export async function listVerifiedDaemons(opts: { prune?: boolean } = {}): Promi
   return verified;
 }
 
-/** Remove a stopped daemon's leftover lockfile + socket + registry record. */
-function cleanupDaemonArtifacts(root: string): void {
-  try { fs.unlinkSync(getDaemonPidPath(root)); } catch { /* gone */ }
-  // POSIX sockets are real files; Windows named pipes vanish with the process.
-  // Sweep every candidate — a daemon that relocated past an unusable in-project
-  // FS (ExFAT/FAT; #997) left its socket at the tmpdir fallback, not candidate 0.
-  if (process.platform !== 'win32') {
-    for (const candidate of getDaemonSocketCandidates(root)) {
-      try { fs.unlinkSync(candidate); } catch { /* gone */ }
+/** Remove stale artifacts while holding the project writer slot exclusively. */
+function cleanupDaemonArtifacts(
+  root: string,
+  expectedLockContents: string | null,
+): boolean {
+  const pidPath = getDaemonPidPath(root);
+  // A daemon owns writer.pid before binding or relocating its socket. Claiming
+  // the writer slot therefore freezes every legitimate daemon artifact writer
+  // while we compare the inspected lock snapshot and clean it up.
+  if (readWriterLock(root)?.pid === process.pid) return false;
+  const claim = tryAcquireWriterLock(root, 'cleanup');
+  if (claim.kind === 'taken') return false;
+
+  try {
+    if (expectedLockContents === null) {
+      if (fs.existsSync(pidPath)) return false;
+    } else {
+      try {
+        if (fs.readFileSync(pidPath, 'utf8') !== expectedLockContents) return false;
+      } catch {
+        return false;
+      }
+    }
+    // POSIX sockets are real files; Windows named pipes vanish with the process.
+    // Sweep every candidate before releasing daemon.pid, so no successor can
+    // acquire the lock and bind a socket that this cleanup then removes.
+    if (process.platform !== 'win32') {
+      for (const candidate of getDaemonSocketCandidates(root)) {
+        try { fs.unlinkSync(candidate); } catch { /* gone */ }
+      }
+    }
+    deregisterDaemon(root);
+    try { fs.unlinkSync(pidPath); } catch (err) {
+      if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return false;
     }
+    return true;
+  } finally {
+    releaseWriterLock(root);
   }
-  deregisterDaemon(root);
 }
 
 /** Remove daemon artifacts only when no matching daemon answers the socket hello. */
@@ -162,10 +191,17 @@ export async function clearStaleDaemonArtifacts(root: string): Promise<boolean>
   );
   if (!hadArtifacts) return false;
   let info: DaemonLockInfo | null = null;
-  try { info = decodeLockInfo(fs.readFileSync(pidPath, 'utf8')); } catch { /* missing/corrupt */ }
-  if (info && isProcessAlive(info.pid) && await probeDaemonIdentity(info)) return false;
-  cleanupDaemonArtifacts(root);
-  return true;
+  let lockContents: string | null = null;
+  try {
+    lockContents = fs.readFileSync(pidPath, 'utf8');
+    info = decodeLockInfo(lockContents);
+  } catch { /* missing/corrupt */ }
+  if (info && isProcessAlive(info.pid)) {
+    // A live legacy holder has no socket path to probe. That is inconclusive,
+    // not proof of PID reuse, so preserve its lock rather than risk two writers.
+    if (!canProbeDaemonIdentity(info) || await probeDaemonIdentity(info)) return false;
+  }
+  return cleanupDaemonArtifacts(root, lockContents);
 }
 
 const sleep = (ms: number): Promise<void> => new Promise((r) => setTimeout(r, ms));
@@ -182,8 +218,8 @@ async function waitForDeath(pid: number, timeoutMs: number): Promise<boolean> {
 export interface StopResult {
   root: string;
   pid: number | null;
-  /** 'term' graceful, 'kill' force, 'not-running' stale lock, 'no-daemon' none found. */
-  outcome: 'term' | 'kill' | 'not-running' | 'no-daemon';
+  /** 'term' graceful, 'kill' force, 'not-running' stale, 'no-daemon' absent, 'unverified' preserved. */
+  outcome: 'term' | 'kill' | 'not-running' | 'no-daemon' | 'unverified';
 }
 
 /**
@@ -195,8 +231,10 @@ export interface StopResult {
 export async function stopDaemonAt(root: string): Promise<StopResult> {
   let pid: number | null = null;
   let identity: DaemonLockInfo | null = null;
+  let lockContents: string | null = null;
   try {
-    identity = decodeLockInfo(fs.readFileSync(getDaemonPidPath(root), 'utf8'));
+    lockContents = fs.readFileSync(getDaemonPidPath(root), 'utf8');
+    identity = decodeLockInfo(lockContents);
     pid = identity?.pid ?? null;
   } catch {
     /* no lockfile */
@@ -210,18 +248,21 @@ export async function stopDaemonAt(root: string): Promise<StopResult> {
   }
 
   if (pid == null) {
-    cleanupDaemonArtifacts(root);
+    cleanupDaemonArtifacts(root, lockContents);
     return { root, pid: null, outcome: 'no-daemon' };
   }
   if (!isProcessAlive(pid)) {
-    cleanupDaemonArtifacts(root);
-    return { root, pid, outcome: 'not-running' };
+    const removed = cleanupDaemonArtifacts(root, lockContents);
+    return { root, pid, outcome: removed ? 'not-running' : 'unverified' };
   }
   // Never signal a process merely because it reused a stale daemon PID. The
   // daemon's immediate hello is the process-identity proof (#1553).
-  if (!identity || !await probeDaemonIdentity(identity)) {
-    cleanupDaemonArtifacts(root);
-    return { root, pid, outcome: 'not-running' };
+  if (!identity || !canProbeDaemonIdentity(identity)) {
+    return { root, pid, outcome: 'unverified' };
+  }
+  if (!await probeDaemonIdentity(identity)) {
+    const removed = cleanupDaemonArtifacts(root, lockContents);
+    return { root, pid, outcome: removed ? 'not-running' : 'unverified' };
   }
 
   // POSIX: SIGTERM runs the daemon's graceful shutdown. Windows: TerminateProcess
@@ -233,7 +274,7 @@ export async function stopDaemonAt(root: string): Promise<StopResult> {
     await waitForDeath(pid, 2000);
     outcome = 'kill';
   }
-  cleanupDaemonArtifacts(root);
+  cleanupDaemonArtifacts(root, lockContents);
   return { root, pid, outcome };
 }
 

+ 55 - 27
src/mcp/daemon.ts

@@ -55,7 +55,11 @@ import {
   getDaemonSocketPath,
 } from './daemon-paths';
 import { CodeGraphPackageVersion } from './version';
-import { releaseWriterLock, tryAcquireWriterLock, writerLockHeldMessage } from './writer-lock';
+import {
+  releaseWriterLock,
+  tryAcquireWriterLock,
+  writerLockHeldMessage,
+} from './writer-lock';
 import { registerDaemon, deregisterDaemon } from './daemon-registry';
 
 /** Default idle linger after the last client disconnects. */
@@ -161,9 +165,8 @@ export interface DaemonStartResult {
  *
  * Race-safe: callers must first call `tryAcquireDaemonLock(projectRoot)` and
  * only construct a Daemon if they got the lock (`kind: 'acquired'`). The atomic
- * `O_EXCL` create inside the acquire helper — which now also writes the full
- * record before returning — is the only synchronization between competing
- * daemons.
+ * create/link inside the acquire helper elects one candidate. The project
+ * writer lock then fences bind/ownership refresh against stale-artifact cleanup.
  */
 export class Daemon {
   private server: net.Server | null = null;
@@ -199,10 +202,9 @@ export class Daemon {
   }
 
   /**
-   * Bind the socket, kick off engine init, and register signal handlers. The
-   * lockfile body was already written atomically by `tryAcquireDaemonLock`, so
-   * there is nothing to write here. The promise resolves once the server is
-   * listening — the daemon then sticks around until idle/shutdown.
+   * Bind the socket, refresh the ownership record, kick off engine init, and
+   * register signal handlers. The promise resolves once the server is listening
+   * — the daemon then sticks around until idle/shutdown.
    */
   async start(): Promise<DaemonStartResult> {
     // #1740: claim the project writer lock before opening/watching so a
@@ -215,10 +217,16 @@ export class Daemon {
       throw new Error(msg);
     }
 
-    // Engine init is deliberately backgrounded — see #172. The first session
-    // to land waits on `ensureInitialized` either way, and unloaded sessions
-    // (cross-project tool calls only) shouldn't pay any open cost.
-    void this.engine.ensureInitialized(this.projectRoot);
+    let initialLockContents: string;
+    try {
+      initialLockContents = fs.readFileSync(this.pidPath, 'utf8');
+      if (decodeLockInfo(initialLockContents)?.pid !== process.pid) {
+        throw new Error('daemon lock belongs to another process');
+      }
+    } catch {
+      releaseWriterLock(this.projectRoot);
+      throw new Error('Lost daemon lock ownership before startup.');
+    }
 
     // Walk the ordered socket candidates and bind the first that works. The
     // in-project path comes first; the deterministic tmpdir path is the fallback
@@ -282,19 +290,27 @@ export class Daemon {
       startedAt: Date.now(),
     };
 
-    // `tryAcquireDaemonLock` wrote the pidfile with the PREFERRED path (candidate
-    // 0) before we knew which one would bind. If we relocated, rewrite it so the
-    // per-project record is honest. Atomic temp+rename; safe because we hold the
-    // lock and we're alive — `clearStaleDaemonLock` pid-verifies, so no racing
-    // candidate clears or clobbers a live daemon's lock.
-    if (this.socketPath !== candidates[0]) {
-      try {
-        const tmpPid = `${this.pidPath}.${process.pid}.relocate`;
-        fs.writeFileSync(tmpPid, encodeLockInfo(lock), { mode: 0o600 });
-        fs.renameSync(tmpPid, this.pidPath);
-      } catch { /* best-effort; the registry record below carries the real path */ }
+    // Refresh the lock on every successful bind, not only relocation. The
+    // writer lock prevents stale-artifact cleanup from racing this ownership
+    // check, and the exact snapshot prevents overwriting a replacement record.
+    try {
+      if (fs.readFileSync(this.pidPath, 'utf8') !== initialLockContents) {
+        throw new Error('Lost daemon lock ownership after binding.');
+      }
+      const tmpPid = `${this.pidPath}.${process.pid}.bound`;
+      fs.writeFileSync(tmpPid, encodeLockInfo(lock), { mode: 0o600 });
+      fs.renameSync(tmpPid, this.pidPath);
+    } catch (err) {
+      try { bound.server.close(); } catch { /* best-effort */ }
+      this.cleanupLockfile();
+      throw err;
     }
 
+    // Engine init is deliberately backgrounded — see #172. It starts only
+    // after bind and ownership refresh, so a delayed daemon that lost election
+    // can never open a second watcher or writer.
+    void this.engine.ensureInitialized(this.projectRoot);
+
     // Drop a discovery record so `codegraph list` / `stop --all` can find us.
     // Best-effort; a missing record only means list's liveness prune covers it.
     registerDaemon({ root: this.projectRoot, ...lock });
@@ -531,7 +547,13 @@ export class Daemon {
  */
 export type AcquireResult =
   | { kind: 'acquired'; pidPath: string; info: DaemonLockInfo }
-  | { kind: 'taken'; existing: DaemonLockInfo | null; pidPath: string };
+  | {
+      kind: 'taken';
+      existing: DaemonLockInfo | null;
+      /** Exact record read after losing acquisition; null when it was unreadable. */
+      lockContents: string | null;
+      pidPath: string;
+    };
 
 /**
  * Atomically create the daemon pidfile with its full record already in place.
@@ -609,10 +631,12 @@ export function tryAcquireDaemonLock(projectRoot: string): AcquireResult {
   // record — `existing` is null only for a genuinely corrupt leftover, never a
   // mid-write race.
   let existing: DaemonLockInfo | null = null;
+  let lockContents: string | null = null;
   try {
-    existing = decodeLockInfo(fs.readFileSync(pidPath, 'utf8'));
+    lockContents = fs.readFileSync(pidPath, 'utf8');
+    existing = decodeLockInfo(lockContents);
   } catch { /* unreadable lockfile — treat as malformed */ }
-  return { kind: 'taken', existing, pidPath };
+  return { kind: 'taken', existing, lockContents, pidPath };
 }
 
 /**
@@ -655,10 +679,14 @@ export function acquireLockViaExclusiveOpen(pidPath: string, info: DaemonLockInf
 export function clearStaleDaemonLock(
   pidPath: string,
   expectedDeadPid?: number,
-  opts: { allowLivePid?: boolean } = {}
+  opts: { allowLivePid?: boolean; expectedLockContents?: string } = {}
 ): boolean {
   try {
     const raw = fs.readFileSync(pidPath, 'utf8');
+    // The identity record changed after the caller inspected it. Even the same
+    // PID may now advertise a newly-bound socket, so this snapshot was never
+    // disproved and must not be deleted.
+    if (opts.expectedLockContents !== undefined && raw !== opts.expectedLockContents) return false;
     const info = decodeLockInfo(raw);
     if (info) {
       // A different pid took over since we read it — not ours to clear.

+ 14 - 1
src/mcp/engine.ts

@@ -47,6 +47,12 @@ export interface MCPEngineOptions {
    * disables it even in daemon mode.
    */
   queryPool?: boolean;
+  /**
+   * Project root whose writer slot must be claimed synchronously before this
+   * engine can open the graph. Used by proxy fallback to fence catch-up sync,
+   * not just the later file watcher.
+   */
+  writerLockRoot?: string;
 }
 
 /**
@@ -70,7 +76,7 @@ export class MCPEngine {
   private watcherStarted = false;
   /** Set when this engine holds writer.pid (#1740). */
   private writerLockRoot: string | null = null;
-  private opts: Required<MCPEngineOptions>;
+  private opts: Required<Omit<MCPEngineOptions, 'writerLockRoot'>>;
   private closed = false;
   // Off-loop read-tool pool (daemon mode only). Created lazily once the default
   // project is open — workers each hold their own WAL read connection.
@@ -79,6 +85,13 @@ export class MCPEngine {
   constructor(opts: MCPEngineOptions = {}) {
     this.opts = { watch: opts.watch ?? true, queryPool: opts.queryPool ?? false };
     this.toolHandler = new ToolHandler(null);
+    if (opts.writerLockRoot) {
+      const writer = tryAcquireWriterLock(opts.writerLockRoot, 'fallback');
+      if (writer.kind === 'taken') {
+        throw new Error(writerLockHeldMessage(writer.existing, writer.pidPath));
+      }
+      this.writerLockRoot = opts.writerLockRoot;
+    }
   }
 
   /**

+ 77 - 8
src/mcp/index.ts

@@ -47,9 +47,22 @@ import {
   isProcessAlive,
   tryAcquireDaemonLock,
 } from './daemon';
+import { clearStaleDaemonArtifacts } from './daemon-registry';
 import { connectWithHello, runLocalHandshakeProxy } from './proxy';
-import { releaseWriterLock, tryAcquireWriterLock, writerLockHeldMessage } from './writer-lock';
-import { getDaemonSocketCandidates, probeDaemonIdentity } from './daemon-paths';
+import {
+  getWriterPidPath,
+  readWriterLock,
+  releaseWriterLock,
+  tryAcquireWriterLock,
+  writerLockHeldMessage,
+} from './writer-lock';
+import {
+  canProbeDaemonIdentity,
+  decodeLockInfo,
+  getDaemonPidPath,
+  getDaemonSocketCandidates,
+  probeDaemonIdentity,
+} from './daemon-paths';
 import { getTelemetry } from '../telemetry';
 import { checkForUpdateInBackground } from '../upgrade/update-check';
 import { EARLY_PPID } from './early-ppid';
@@ -75,6 +88,42 @@ const DAEMON_INTERNAL_ENV = 'CODEGRAPH_DAEMON_INTERNAL';
 const TAKEOVER_MAX_RETRIES = 5;
 const TAKEOVER_RETRY_DELAY_MS = 100;
 
+/**
+ * Create an in-process fallback only when it cannot conflict with a live
+ * legacy daemon. Plain-PID locks cannot prove daemon identity, but they still
+ * prove that a process owns the legacy writer slot.
+ */
+function makeFallbackEngine(root: string): MCPEngine {
+  let existing: ReturnType<typeof decodeLockInfo> = null;
+  try {
+    existing = decodeLockInfo(fs.readFileSync(getDaemonPidPath(root), 'utf8'));
+  } catch (err) {
+    const code = (err as NodeJS.ErrnoException).code;
+    if (code !== 'ENOENT') {
+      throw new Error(`The daemon lock could not be read (${code ?? 'unknown error'}); refusing an in-process fallback.`);
+    }
+  }
+  if (
+    existing &&
+    isProcessAlive(existing.pid) &&
+    !canProbeDaemonIdentity(existing)
+  ) {
+    throw new Error(
+      `Cannot start an in-process fallback while live legacy daemon pid ${existing.pid} holds the project lock.`
+    );
+  }
+  const writer = readWriterLock(root);
+  if (writer && writer.pid > 0 && isProcessAlive(writer.pid)) {
+    throw new Error(writerLockHeldMessage(writer, getWriterPidPath(root)));
+  }
+  if (existing && isProcessAlive(existing.pid)) {
+    throw new Error(
+      `Cannot start an in-process fallback while live daemon pid ${existing.pid} holds the project lock.`
+    );
+  }
+  return new MCPEngine({ writerLockRoot: root });
+}
+
 /**
  * How long a launcher waits for a freshly-spawned daemon to bind its socket
  * before giving up and running in-process. The daemon binds the socket *before*
@@ -449,23 +498,43 @@ export class MCPServer {
       // Taken. If the holder is alive, another daemon already serves (or is
       // binding) — we're redundant; exit cleanly so the launcher proxies to it.
       const existing = lock.existing;
+      let disprovedLiveIdentity = false;
       if (existing && existing.pid > 0 && isProcessAlive(existing.pid)) {
         // Give a newly-elected daemon time to bind, then require its socket hello
         // to match the lock PID/version. PID existence alone accepts an unrelated
         // process after OS PID reuse and permanently wedges startup (#1553).
         const age = Date.now() - existing.startedAt;
-        const stillStarting = existing.startedAt > 0 && age >= 0 && age < 10_000;
-        if (stillStarting || await probeDaemonIdentity(existing)) {
+        const startupGraceMs = 10_000;
+        const stillStarting = existing.startedAt > 0 && age >= 0 && age < startupGraceMs;
+        // Legacy plain-PID locks have no socket identity to test. Preserve those
+        // live holders: an inconclusive probe is not permission to create a
+        // second writer.
+        if (
+          !canProbeDaemonIdentity(existing) ||
+          stillStarting ||
+          await probeDaemonIdentity(existing)
+        ) {
           process.stderr.write(
             `[CodeGraph daemon] Another daemon (pid ${existing.pid}) already holds the lock; exiting.\n`
           );
           process.exit(0);
         }
+        disprovedLiveIdentity = true;
       }
 
-      // Holder is dead (or the record is unreadable) — clear it (pid-verified,
-      // so we never delete a live daemon's lock) and retry the acquire.
-      clearStaleDaemonLock(lock.pidPath, existing?.pid, { allowLivePid: true });
+      // The holder is dead, the record is unreadable, or a completed socket
+      // hello disproved a live PID's identity. Revalidate the exact record and
+      // retry the acquire only after cleanup succeeds safely.
+      if (disprovedLiveIdentity) {
+        // Re-probe and claim writer.pid before cleanup. A daemon that is merely
+        // delayed already owns that writer lock, and a paired live-PID record is
+        // ambiguous under the legacy lock format, so both cases fail closed.
+        await clearStaleDaemonArtifacts(root);
+      } else if (lock.lockContents !== null) {
+        clearStaleDaemonLock(lock.pidPath, existing?.pid, {
+          expectedLockContents: lock.lockContents,
+        });
+      }
       await sleep(TAKEOVER_RETRY_DELAY_MS);
     }
 
@@ -515,7 +584,7 @@ export class MCPServer {
       }
       return null; // never bound — the proxy serves this session in-process
     };
-    await runLocalHandshakeProxy({ getDaemonSocket, makeEngine: () => new MCPEngine(), root });
+    await runLocalHandshakeProxy({ getDaemonSocket, makeEngine: () => makeFallbackEngine(root), root });
   }
 
   /** Standard SIGINT/SIGTERM handlers that route to our `stop()` (direct mode). */