Skip to content

QueryPool enrolls a worker whose openSync failed (ready ok:false) into the idle set, serving isError on every routed call forever; crash budget counts it once so the breaker never trips — verified #1357

Description

@inth3shadows

Summary

When a query-pool worker's spawn-time openSync(root) throws, the worker reports {type:'ready', ok:false} — but QueryPool.onMessage enrolls it in the idle set anyway. The non-functional worker (its handler is null) then gets dispatched real jobs and returns {isError:true} for every one, indefinitely. Because the failure is counted against the crash budget only once (at the ready message, not per call), the circuit breaker (healthy) may never trip, so the pool never falls back to in-process dispatch. Verified with a deterministic harness (injected fake worker) and confirmed with a real worker.

Root cause

// src/mcp/query-pool.ts:219-227 (onMessage)
if (m.type === 'ready') {
  this.pendingWorkers.delete(w);
  if (m.ok === false) this.totalCrashes++; // counted ONCE
  else this.everReady = true;
  this.idle.push(w);                        // <- enrolled regardless of ok
  this.drain();
  return;
}
// src/mcp/query-worker.ts:55-64 — a failed open still reports ready
try { const cg = loadCodeGraph().openSync(root); handler = new (loadToolHandler())(cg); }
catch (err) { initError = ...; }
port.postMessage({ type: 'ready', ok: initError === null, error: initError });
// ...:78-85 — with handler === null, every call returns isError forever

The dead worker is never terminate()d or replaced (contrast onWorkerGone, which deletes the worker, respawns, and retries). It stays in idle, so drain() keeps routing jobs to it. And since totalCrashes doesn't grow per call, on a box where maxSize < CRASH_BUDGET (12) the pool's healthy getter stays true, so the documented "circuit breaker falls back to in-process" guarantee doesn't cover this failure mode. A persistent isError:true on every routed call is exactly what this repo's own guidance says teaches an agent to abandon codegraph.

Repro

Arm A — deterministic (injected fake worker via the createWorker test seam), simulating openSync failure:

after ready(ok:false):  idle=1  liveWorkers=1  totalCrashes=1  healthy=true  everReady=false
20 calls routed to the dead worker → isError responses: 20/20
after 20 calls:         totalCrashes=1  healthy=true  (worker still enrolled, never terminated)

Arm B — real worker, root with no .codegraph DB (real openSync throw):

real worker responses: r1.isError=true  r2.isError=true
text: "codegraph worker could not open the project: CodeGraph not initialized in /tmp/cg-noidx-..."
liveWorkers=1  totalCrashes=1  healthy=true

Honest scope note

In normal daemon operation the pool's root is the same project the main thread already opened successfully, so real workers open the same valid DB and this path isn't hit. The realistic trigger is a transient / worker-local open failure (FS hiccup, fd or memory pressure at the moment a worker cold-starts) — not a permanently-bad root, which the main thread would reject upstream anyway. Arm B forces the failure deterministically to exercise the real worker→pool message path; Arm A isolates the pool-logic defect independent of cause. Flagging severity as medium for that reason — the trigger is uncommon, but when it happens the slot is poisoned for the pool's lifetime and the breaker doesn't rescue it.

Suggested direction

On ok === false, don't enroll the worker: terminate() it and (if healthy) spawnOne() a replacement, mirroring onWorkerGone's recovery path — so a failed-open worker degrades like a crashed one instead of becoming a permanent isError source.

Environment

main @ 5955d04, package 1.4.1, Node 22.23.1 (Linux). Reproduces on a clean npm ci && npm run build.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions