fix(writer-lock): reap a live writer stuck at ready:false past the timeout (fixes #2918)

CHANGELOG.md

@@ -19,6 +19,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
+- A project's MCP server no longer stays stuck holding the writer lock with every query timing out when it never finishes its first catch-up: after ten minutes the next server reclaims the lock and logs the old process and how long it went without reporting ready, tunable with `CODEGRAPH_WRITER_READY_TIMEOUT_MS`, while a server that is serving normally keeps its lock however long it runs and the wedged process is never signalled. (#2918)
 - In TypeScript, an awaited call with type arguments like `await fetchApi<User>("/me")` or `await api.get<T>(url)` now links to the function it calls. Before, the call was dropped, so typed fetch wrappers looked like they had no callers. (#2586)
 - In JavaScript and TypeScript, a call or `new` in a parameter's default value, like `function renderWithNote(note = buildNote())`, a destructured `{ size = defaultSize() }` or `constructor(private svc = new Service())`, now counts as the function's own call. Before, it was dropped, so `buildNote` was missing that caller and impact on it came back incomplete. Re-index JavaScript and TypeScript projects after upgrading. Thanks @inth3shadows for the report and the pull request. (#2588)
 - In React, a component rendered at module scope, in a top-level constant or inside a test's `it(…)` callback, like `createRoot(el).render(<App />)` or `render(<Card />)`, now counts as rendered there, so an app's entry reaches its root component and a component's tests show among its callers. (#2587)

README.md

@@ -890,7 +890,7 @@ Framework routing is validated the same way, on a canonical app per framework: E
 
 **MCP server not connecting** — Your agent starts the server itself, so you don't launch it by hand. Make sure the project is initialized and indexed (`codegraph status`) and that the path in your MCP config is correct. If it still won't connect, re-run `codegraph install` to rewrite the config.
 
-**Two `codegraph serve --mcp` on one project fight over the index / auto-sync stops** — CodeGraph allows one live MCP *writer* per project (the shared background daemon, or a single direct-mode process). Extra clients should proxy to that daemon. If you set `CODEGRAPH_NO_DAEMON=1`, run only one `serve --mcp` for that project; a second instance exits with a clear writer-lock error (see `writer.pid` under `.codegraph/`). Prefer leaving the daemon enabled so multiple MCP hosts share one watcher.
+**Two `codegraph serve --mcp` on one project fight over the index / auto-sync stops** — CodeGraph allows one live MCP *writer* per project (the shared background daemon, or a single direct-mode process). Extra clients should proxy to that daemon. If you set `CODEGRAPH_NO_DAEMON=1`, run only one `serve --mcp` for that project; a second instance exits with a clear writer-lock error (see `writer.pid` under `.codegraph/`). Prefer leaving the daemon enabled so multiple MCP hosts share one watcher. A server that never finishes its first catch-up no longer holds the project forever: after ten minutes at `ready: false` the next server reclaims the lock and logs how long the old process went without reporting ready (tune with `CODEGRAPH_WRITER_READY_TIMEOUT_MS`). A server that is ready keeps its lock however long it runs.
 
 **MCP tool calls fail with `Transport closed` while `codegraph status`/`sync` are healthy** — almost always WSL2 with the project on a Windows drive (a `/mnt/c` or `/mnt/d` path), where the local socket CodeGraph uses to share one background server across sessions is unreliable. CodeGraph now falls back to serving the session in-process instead of dropping the connection, but if you still hit it, set `CODEGRAPH_NO_DAEMON=1` in your MCP server's environment to skip the shared server entirely (each session runs in its own process). Moving the project onto the Linux-native filesystem (e.g. under `~/` instead of `/mnt/`) restores the shared server.
 

__tests__/writer-lock.test.ts

@@ -12,11 +12,14 @@ import { MCPEngine } from '../src/mcp/engine';
 import {
   decodeWriterLockInfo,
   getWriterPidPath,
+  isWriterReadyStale,
   markWriterReady,
   readWriterLock,
   releaseWriterLock,
+  resolveWriterReadyTimeoutMs,
   tryAcquireWriterLock,
   writerLockHeldMessage,
+  WRITER_READY_TIMEOUT_MS,
 } from '../src/mcp/writer-lock';
 
 describe('writer lock (#1740)', () => {
@@ -139,4 +142,66 @@ describe('writer lock (#1740)', () => {
 
     expect(() => new MCPEngine({ writerLockRoot: root })).toThrow(/writer lock held/i);
   });
+
+  it('classifies ready:false by age, not by pid liveness alone (#2918)', () => {
+    const now = Date.now();
+    expect(isWriterReadyStale({ pid: 123, mode: 'fallback', startedAt: now - WRITER_READY_TIMEOUT_MS - 1, ready: false }, now)).toBe(true);
+    expect(isWriterReadyStale({ pid: 123, mode: 'fallback', startedAt: now, ready: false }, now)).toBe(false);
+    expect(isWriterReadyStale({ pid: 123, mode: 'fallback', startedAt: now - WRITER_READY_TIMEOUT_MS - 1, ready: true }, now)).toBe(false);
+    expect(isWriterReadyStale({ pid: 123, mode: 'fallback', startedAt: 0, ready: false }, now)).toBe(false);
+    expect(isWriterReadyStale(null, now)).toBe(false);
+  });
+
+  it('honours CODEGRAPH_WRITER_READY_TIMEOUT_MS', () => {
+    const prev = process.env.CODEGRAPH_WRITER_READY_TIMEOUT_MS;
+    process.env.CODEGRAPH_WRITER_READY_TIMEOUT_MS = '1000';
+    try {
+      expect(resolveWriterReadyTimeoutMs()).toBe(1000);
+      expect(isWriterReadyStale({ pid: 1, mode: 'fallback', startedAt: Date.now() - 2000, ready: false })).toBe(true);
+    } finally {
+      if (prev === undefined) delete process.env.CODEGRAPH_WRITER_READY_TIMEOUT_MS;
+      else process.env.CODEGRAPH_WRITER_READY_TIMEOUT_MS = prev;
+    }
+  });
+
+  it('steals a live writer lock that stayed at ready:false past the timeout (#2918)', () => {
+    const root = makeProject();
+    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: 'fallback', startedAt: Date.now() - WRITER_READY_TIMEOUT_MS - 1_000, ready: false }) + '\n',
+      { mode: 0o600 },
+    );
+    const r = tryAcquireWriterLock(root, 'daemon');
+    expect(r.kind).toBe('acquired');
+    if (r.kind === 'acquired') {
+      expect(r.info.pid).toBe(process.pid);
+      expect(r.info.ready).toBe(false);
+    }
+    expect(readWriterLock(root)?.pid).toBe(process.pid);
+    releaseWriterLock(root);
+  });
+
+  it('does not steal a live writer that is still within the ready timeout (#2918)', () => {
+    const root = makeProject();
+    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: 'fallback', startedAt: Date.now(), ready: false }) + '\n',
+    );
+    const r = tryAcquireWriterLock(root, 'daemon');
+    expect(r.kind).toBe('taken');
+    if (r.kind === 'taken') expect(r.existing?.pid).toBe(holder.pid);
+  });
+
+  it('held message names a not-ready writer, its pid, and the lock file (#2918)', () => {
+    const root = makeProject();
+    const pidPath = getWriterPidPath(root);
+    const msg = writerLockHeldMessage({ pid: 20632, mode: 'fallback', startedAt: Date.now(), ready: false }, pidPath);
+    expect(msg).toContain('20632');
+    expect(msg).toContain(pidPath);
+    expect(msg).toContain('has not reported ready');
+  });
 });

src/mcp/writer-lock.ts

@@ -46,6 +46,48 @@ export interface WriterLockInfo {
   ready?: boolean;
 }
 
+/**
+ * How long a writer may stay at `ready: false` before it is stale (#2918).
+ *
+ * Deliberately far longer than normal catch-up (and the 30 s reader wait in
+ * project-lifecycle.ts), and far shorter than the ~28 h wedged daemon in
+ * #2918. A live pid is not, by itself, proof of a live writer: one that never
+ * reports ready is indistinguishable from a wedged one once this long has
+ * passed. Overridable for tests and very large projects via
+ * `CODEGRAPH_WRITER_READY_TIMEOUT_MS`.
+ */
+export const WRITER_READY_TIMEOUT_MS = 10 * 60 * 1000;
+
+export function resolveWriterReadyTimeoutMs(): number {
+  const raw = Number(process.env.CODEGRAPH_WRITER_READY_TIMEOUT_MS);
+  return Number.isFinite(raw) && raw > 0 ? raw : WRITER_READY_TIMEOUT_MS;
+}
+
+/**
+ * True only for a record that explicitly says `ready: false` and whose
+ * `startedAt` is at least the ready timeout old. `ready: true`, a missing
+ * `startedAt`, and a future `startedAt` are never stale here, so legacy
+ * records without a `ready` field are not reaped by this check.
+ */
+export function isWriterReadyStale(info: WriterLockInfo | null, now = Date.now()): boolean {
+  if (!info || info.ready !== false || !(info.startedAt > 0)) return false;
+  return now - info.startedAt >= resolveWriterReadyTimeoutMs();
+}
+
+/**
+ * Modes that run an MCP writer and publish `ready: true` once its first
+ * catch-up finishes. Only these can go stale by staying at `ready: false`:
+ * `rebuild`, `cleanup` and `handover` also take `writer.pid` (or its sibling
+ * `rebuild.pid`) but never publish readiness, so age alone is not evidence
+ * they are wedged — a full re-index stays `ready: false` for as long as it
+ * runs. Kept narrow on purpose: an unknown mode is never stolen.
+ */
+const READY_PUBLISHING_MODES = new Set(['daemon', 'direct', 'fallback']);
+
+function isReapableNotReadyWriter(info: WriterLockInfo | null): boolean {
+  return !!info && READY_PUBLISHING_MODES.has(info.mode) && isWriterReadyStale(info);
+}
+
 export type WriterAcquireResult =
   | { kind: 'acquired'; pidPath: string; info: WriterLockInfo }
   | { kind: 'taken'; existing: WriterLockInfo | null; pidPath: string };
@@ -71,7 +113,13 @@ export function decodeWriterLockInfo(raw: string): WriterLockInfo | null {
 
 /**
  * Atomically create `writer.pid` (link-into-place, O_EXCL fallback). If held
- * by a dead PID, clear and retry once. Does not steal from a live holder.
+ * by a dead PID, clear and retry once. A live MCP writer that has stayed at
+ * `ready: false` past `WRITER_READY_TIMEOUT_MS` is also stale (#2918) and is
+ * cleared the same pid-verified way, with a stderr line naming its pid and
+ * the lock file. A holder that is ready, or still within the timeout, is
+ * never stolen — no matter how old — and the stale holder is never signalled
+ * (#1978, #2819): only the lock is cleared, so a live process that later
+ * exits still cannot delete the successor's record (see releaseWriterLock).
  */
 export function tryAcquireWriterLock(
   projectRoot: string,
@@ -134,14 +182,22 @@ export function tryAcquireWriterLock(
   }
   if (result.kind === 'taken') {
     const existing = result.existing;
-    if (!existing || existing.pid <= 0 || !isProcessAlive(existing.pid)) {
+    if (!existing || existing.pid <= 0 || !isProcessAlive(existing.pid) || isReapableNotReadyWriter(existing)) {
       // Stale — clear (pid-verified) and retry once.
       try {
         const raw = fs.readFileSync(pidPath, 'utf8');
         const cur = decodeWriterLockInfo(raw);
         if (!cur || cur.pid === existing?.pid) {
-          if (!cur || cur.pid <= 0 || !isProcessAlive(cur.pid)) {
+          if (!cur || cur.pid <= 0 || !isProcessAlive(cur.pid) || isReapableNotReadyWriter(cur)) {
             fs.unlinkSync(pidPath);
+            // Only the lock is cleared — never the holder (#1978, #2819). The
+            // wedged process is left to the user's daemon controls.
+            if (cur && cur.pid !== process.pid && isReapableNotReadyWriter(cur)) {
+              console.error(
+                `CodeGraph writer lock: cleared stale not-ready writer PID ${cur.pid} (${cur.mode}) at ${pidPath} ` +
+                `after ${Math.round((Date.now() - cur.startedAt) / 1000)}s without reporting ready; a new writer may now acquire the lock.`,
+              );
+            }
           }
         }
       } catch { /* ENOENT ok */ }
@@ -235,12 +291,15 @@ export function writerLockHeldMessage(
   const who = existing && existing.pid > 0
     ? `PID ${existing.pid} (${existing.mode || 'unknown'} mode)`
     : 'another process';
+  const notReady = existing && existing.ready === false && existing.startedAt > 0
+    ? ` It started at ${new Date(existing.startedAt).toISOString()} and has not reported ready.`
+    : '';
   return (
     'CodeGraph writer lock held by ' + who + '. ' +
     'Only one live MCP writer may serve a project (auto-sync / index). ' +
     'Stop the other server (if it is the shared daemon, pick it in `codegraph daemon` to stop it; otherwise end the other MCP session), ' +
     'or unset CODEGRAPH_NO_DAEMON so additional clients proxy to the shared daemon. ' +
-    'If this is stale, delete ' + pidPath
+    'If this is stale, delete ' + pidPath + '.' + notReady
   );
 }