From cac4021da2a3fd0e39468cf79321e26dad5791fa Mon Sep 17 00:00:00 2001 From: colbymchenry Date: Mon, 13 Jul 2026 18:26:19 +0200 Subject: [PATCH] feat(daemon): auto-respawn via per-project watchdog (issue #116) When the shared MCP daemon is killed (Stop-Process, kill -9, crash) while the proxy host is still running, the incremental file-watcher was lost until the user manually ran 'codegraph-vba sync'. This change adds a per-project daemon liveness watchdog inside the proxy that polls the daemon's .codegraph-vba/daemon.pid lockfile every 30s and respawns the daemon (via the same detached-spawn recipe the launcher uses) when the pid points at a dead process. Architecture: - src/mcp/daemon-watchdog.ts (new): DaemonWatchdog class. Constructor takes an injectable spawnFn so tests can stub the detached spawn without ESM-fragile vi.mock('child_process') patches. hasLiveDaemon(root) reads the lockfile via decodeLockInfo (the lockfile is the authoritative pointer; listDaemons filters dead entries from its return value, so it cannot be used to detect a dead daemon -- that was the trap in v1). - src/mcp/index.ts: MCPServer (proxy mode) instantiates a watchdog per project root, starts it, and tears it down on stop(). Watchdog intervals are unref-ed so they never keep the proxy alive. Why a watchdog (vs. spawn-on-demand at MCP request time): the issue's repro is Stop-Process -Id while the MCP keeps serving -- file edits are missed until the user manually runs 'codegraph-vba sync'. The watchdog closes that gap in <30s. Tests (15/15 green across the daemon tests): - __tests__/daemon-watchdog.test.ts (9 tests): covers hasLiveDaemon for the 3 lockfile states, checkAndRespawn (live=no-op, dead=respawn, no-.codegraph=no-op), tick -> respawn integration (single + double), start/stop behavior. - __tests__/daemon-registry.test.ts: unchanged, still 6/6. Closes #116. --- CHANGELOG.md | 4 + __tests__/daemon-watchdog.test.ts | 216 ++++++++++++++++++++++++++++ src/mcp/daemon-watchdog.ts | 232 ++++++++++++++++++++++++++++++ src/mcp/index.ts | 15 ++ 4 files changed, 467 insertions(+) create mode 100644 __tests__/daemon-watchdog.test.ts create mode 100644 src/mcp/daemon-watchdog.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 102444df..51dfc119 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - The post-extraction VBA stub resolver's actual contract is now documented for consumers. `metadata.repointDecision` carries one of `reponted-to-real`, `declined-runtime`, `declined-ambiguous`, or `declined-not-found` — consumers detecting "missing callees" must filter on `repointDecision='declined-not-found'`, NOT on the raw `stub=true` count (which is dominated by runtime-object noise from `DAO.*`, `fso.*`, etc.). The original round-5 prompt's `stub_true_count < 500` acceptance criterion was replaced by the `declined-not-found` filter. Reference: `docs/vba-stub-repoint-decision.md`. (#115) +### Fixes + +- The shared MCP daemon auto-respawns when killed while the proxy is still up. After `Stop-Process -Id ` (or any other death cause) the proxy's per-project daemon watchdog detects the dead daemon via its `daemon.pid` lockfile and respawns it within 30s — file edits are picked up automatically instead of needing a manual `codegraph-vba sync`. The watchdog polls every 30s by default and unrefs its interval so it never keeps the event loop alive on its own. (#116) + ## [1.7.0] - 2026-07-13 ### Fixes diff --git a/__tests__/daemon-watchdog.test.ts b/__tests__/daemon-watchdog.test.ts new file mode 100644 index 00000000..fffa5803 --- /dev/null +++ b/__tests__/daemon-watchdog.test.ts @@ -0,0 +1,216 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { spawn } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { + getRegistryDir, + isProcessAlive, + registerDaemon, + deregisterDaemon, + type DaemonRecord, +} from '../src/mcp/daemon-registry'; +import { DaemonWatchdog } from '../src/mcp/daemon-watchdog'; +import { decodeLockInfo, getDaemonPidPath } from '../src/mcp/daemon-paths'; + +/** A pid that's guaranteed dead: spawn a trivial process, let it exit, reap it. */ +async function deadPid(): Promise { + const child = spawn(process.execPath, ['-e', 'process.exit(0)']); + const pid = child.pid!; + await new Promise((r) => child.on('exit', () => r())); + await new Promise((r) => setTimeout(r, 50)); // let the OS reap it + return pid; +} + +function rec(root: string, pid: number, startedAt = Date.now()): DaemonRecord { + return { root, pid, version: '1.0.0', socketPath: `${root}/.codegraph/daemon.sock`, startedAt }; +} + +function fakeProject(): string { + // Make a fake .codegraph-vba/ dir (note the suffix — codegraph-vba's + // getCodeGraphDir() uses `.codegraph-vba`, NOT `.codegraph`) so + // findNearestCodeGraphRoot() and getDaemonPidPath() both resolve correctly. + // isInitialized() requires BOTH the dir AND a codegraph.db file, so create + // an empty placeholder db so the project looks "initialized" to the daemon + // logic. + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-watchdog-')); + fs.mkdirSync(path.join(root, '.codegraph-vba'), { recursive: true }); + fs.writeFileSync(path.join(root, '.codegraph-vba', 'codegraph.db'), ''); + return root; +} + +describe('daemon-watchdog', () => { + let tmpHome: string; + let prevHome: string | undefined; + let prevUserProfile: string | undefined; + + beforeEach(() => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-watchdog-home-')); + prevHome = process.env.HOME; + prevUserProfile = process.env.USERPROFILE; + process.env.HOME = tmpHome; + process.env.USERPROFILE = tmpHome; + expect(getRegistryDir().startsWith(tmpHome)).toBe(true); + }); + + afterEach(() => { + if (prevHome === undefined) delete process.env.HOME; else process.env.HOME = prevHome; + if (prevUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = prevUserProfile; + try { fs.rmSync(tmpHome, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + describe('checkAndRespawn', () => { + it('returns false when a live daemon already exists for the root', async () => { + const root = fakeProject(); + // Write a live daemon's pidfile — the canonical pointer. Our own + // process is alive, so isProcessAlive(process.pid) returns true and + // checkAndRespawn() should skip the respawn. + fs.writeFileSync( + getDaemonPidPath(root), + JSON.stringify({ pid: process.pid, version: '1.0.0', socketPath: '', startedAt: Date.now() }, null, 2) + ); + + const wd = new DaemonWatchdog(); + wd.watch(root); + const spawned = await wd.checkAndRespawn(root); + expect(spawned).toBe(false); + fs.unlinkSync(getDaemonPidPath(root)); + }); + + it('returns false (no-op) when the root has no .codegraph/ to bind a daemon under', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-no-cg-')); // no .codegraph/ + const wd = new DaemonWatchdog(); + wd.watch(root); + const spawned = await wd.checkAndRespawn(root); + expect(spawned).toBe(false); + }); + + it('returns true after tick() detects a dead daemon', async () => { + const root = fakeProject(); + const dead = await deadPid(); + // Place a dead daemon's pidfile so hasLiveDaemon() returns false. + fs.writeFileSync( + getDaemonPidPath(root), + JSON.stringify({ pid: dead, version: '1.0.0', socketPath: '', startedAt: 0 }) + ); + + let spawnCalls = 0; + const wd = new DaemonWatchdog({ + scriptPath: 'codegraph-vba', + spawnFn: () => { spawnCalls++; return true; }, + }); + wd.watch(root); + await wd.tick(); + expect(spawnCalls).toBe(1); + fs.unlinkSync(getDaemonPidPath(root)); + }); + }); + + describe('watchdog tick → respawn integration', () => { + it('a dead daemon record is followed by another spawn on the next tick', async () => { + const root = fakeProject(); + // Dead pidfile at the canonical path. + fs.writeFileSync( + getDaemonPidPath(root), + JSON.stringify({ pid: await deadPid(), version: '1.0.0', socketPath: '', startedAt: 0 }) + ); + + let spawnCalls = 0; + const wd = new DaemonWatchdog({ + scriptPath: 'codegraph-vba', + spawnFn: () => { spawnCalls++; return true; }, + }); + wd.watch(root); + await wd.tick(); + await wd.tick(); // second tick: still dead (we don't simulate a real spawn replacing the pidfile), spawns again + expect(spawnCalls).toBe(2); + fs.unlinkSync(getDaemonPidPath(root)); + }); + + it('start() begins the polling loop; stop() ends it', async () => { + const root = fakeProject(); + fs.writeFileSync( + getDaemonPidPath(root), + JSON.stringify({ pid: await deadPid(), version: '1.0.0', socketPath: '', startedAt: 0 }) + ); + + let spawnCalls = 0; + const wd = new DaemonWatchdog({ + scriptPath: 'codegraph-vba', + intervalMs: 5_000, // long enough that ONLY the explicit ticks fire + spawnFn: () => { spawnCalls++; return true; }, + }); + wd.watch(root); + wd.start(); + try { + // `start()` fires one immediate tick (fire-and-forget). Give the + // microtask queue a chance to drain before we sample. + await new Promise((r) => setImmediate(r)); + expect(spawnCalls).toBe(1); + } finally { + wd.stop(); + } + fs.unlinkSync(getDaemonPidPath(root)); + }); + + it('stop() prevents further polls after the interval ticks', async () => { + const root = fakeProject(); + fs.writeFileSync( + getDaemonPidPath(root), + JSON.stringify({ pid: await deadPid(), version: '1.0.0', socketPath: '', startedAt: 0 }) + ); + + let spawnCalls = 0; + const wd = new DaemonWatchdog({ + scriptPath: 'codegraph-vba', + intervalMs: 20, // poll fast so we can sample after stop() + spawnFn: () => { spawnCalls++; return true; }, + }); + wd.watch(root); + wd.start(); + // Let the immediate tick + a couple of interval ticks happen. + await new Promise((r) => setTimeout(r, 80)); + const beforeStop = spawnCalls; + expect(beforeStop).toBeGreaterThanOrEqual(1); + wd.stop(); + // After stop(), the interval is cleared — spawnCalls must not grow. + await new Promise((r) => setTimeout(r, 80)); + expect(spawnCalls).toBe(beforeStop); + fs.unlinkSync(getDaemonPidPath(root)); + }); + }); + + describe('hasLiveDaemon', () => { + it('returns false when no pid file exists', () => { + const root = fakeProject(); + const wd = new DaemonWatchdog(); + expect(wd.hasLiveDaemon(root)).toBe(false); + }); + + it('returns true when the pid file points at our own process', () => { + const root = fakeProject(); + const pidPath = getDaemonPidPath(root); + const body = JSON.stringify( + { pid: process.pid, version: '1.0.0', socketPath: '', startedAt: Date.now() }, + null, + 2 + ); + fs.writeFileSync(pidPath, body); + const wd = new DaemonWatchdog(); + expect(wd.hasLiveDaemon(root)).toBe(true); + fs.unlinkSync(pidPath); + }); + + it('returns false when the pid file points at a dead process', async () => { + const root = fakeProject(); + const pidPath = getDaemonPidPath(root); + fs.writeFileSync( + pidPath, + JSON.stringify({ pid: await deadPid(), version: '1.0.0', socketPath: '', startedAt: 0 }) + ); + const wd = new DaemonWatchdog(); + expect(wd.hasLiveDaemon(root)).toBe(false); + fs.unlinkSync(pidPath); + }); + }); +}); \ No newline at end of file diff --git a/src/mcp/daemon-watchdog.ts b/src/mcp/daemon-watchdog.ts new file mode 100644 index 00000000..f99b49fb --- /dev/null +++ b/src/mcp/daemon-watchdog.ts @@ -0,0 +1,232 @@ +/** + * Daemon liveness watchdog — issue #116. + * + * The MCP server (or any long-lived host) registers project roots it serves. + * Every `intervalMs`, the watchdog checks each root's daemon (via the + * {@link listDaemons} discovery index + liveness probe). If a daemon that + * SHOULD exist is dead (kill -9, Stop-Process, crash, …), the watchdog + * respawns it with the same spawnDetachedDaemon recipe the launcher uses + * (`process.execPath` + `process.execArgv` + `scriptPath serve --mcp --path `). + * + * Why a watchdog (vs. spawn-on-demand at MCP request time): the issue's repro + * is `Stop-Process -Id ` while the MCP keeps serving — file edits are + * missed until the user manually runs `codegraph-vba sync`. The watchdog + * closes that gap in . + * + * Spawned detaches (own session/process group) so closing the MCP process + * does not take the watchdog down with it. + */ + +import { spawn } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import { findNearestCodeGraphRoot, getCodeGraphDir } from '../directory'; +import { decodeLockInfo, getDaemonPidPath, getDaemonSocketCandidates } from './daemon-paths'; +import { isProcessAlive } from './daemon-registry'; + +const DAEMON_INTERNAL_ENV = 'CODEGRAPH_DAEMON_INTERNAL'; +const HOST_PPID_ENV = 'CODEGRAPH_HOST_PPID'; + +const DEFAULT_INTERVAL_MS = 30_000; + +/** + * Default spawn implementation: spawn the daemon detached and unref so it + * survives the launcher's exit. Extracted so tests can inject a stub. + */ +function defaultSpawnFn( + nodePath: string, + args: string[], + opts: { detached: boolean; env: NodeJS.ProcessEnv; windowsHide?: boolean; stdio?: 'ignore' | [ 'ignore', number, number ] } +): boolean { + const child = spawn(nodePath, args, { + detached: opts.detached, + stdio: opts.stdio, + windowsHide: opts.windowsHide, + env: opts.env, + }); + child.unref(); + return true; +} + +export interface DaemonWatchdogOptions { + /** Poll interval (ms). Default 30s. */ + intervalMs?: number; + /** Override the path to the codegraph binary used to spawn the daemon. */ + scriptPath?: string; + /** Override node executable used to spawn the daemon. */ + nodePath?: string; + /** + * Override the spawn implementation. Used by tests; production callers + * leave this unset to use the real `child_process.spawn`. + */ + spawnFn?: (nodePath: string, args: string[], opts: { detached: boolean; env: NodeJS.ProcessEnv; windowsHide?: boolean; stdio?: 'ignore' | [ 'ignore', number, number ] }) => boolean; +} + +/** + * How long the watchdog waits for a freshly-spawned daemon to bind its socket + * before declaring the spawn a failure (next poll retries). Same ~6s budget + * the launcher uses for cold start. + */ +const SPAWN_BIND_MAX_RETRIES = 240; +const SPAWN_BIND_RETRY_DELAY_MS = 25; + +/** + * Per-project daemon liveness watchdog. Registers roots; polls every + * `intervalMs`; respawns a daemon if the live one dies. + */ +export class DaemonWatchdog { + private readonly roots = new Set(); + private interval: NodeJS.Timeout | null = null; + private readonly intervalMs: number; + private readonly scriptPath: string; + private readonly nodePath: string; + private readonly spawnFn: (nodePath: string, args: string[], opts: { detached: boolean; env: NodeJS.ProcessEnv; windowsHide?: boolean; stdio?: 'ignore' | [ 'ignore', number, number ] }) => boolean; + + constructor(opts: DaemonWatchdogOptions = {}) { + this.intervalMs = opts.intervalMs ?? DEFAULT_INTERVAL_MS; + // Resolve the spawn target at construction time. `process.argv[1]` is the + // script that the launcher used; reusing it keeps the detached daemon + // launch identical to the launcher's spawn. + this.scriptPath = opts.scriptPath ?? process.argv[1] ?? ''; + this.nodePath = opts.nodePath ?? process.execPath; + this.spawnFn = opts.spawnFn ?? defaultSpawnFn; + } + + /** Watch a project root: if its daemon dies, respawn it. Idempotent. */ + watch(root: string): void { + this.roots.add(path.resolve(root)); + } + + /** Stop watching a root. Idempotent. */ + unwatch(root: string): void { + this.roots.delete(path.resolve(root)); + } + + /** Number of roots currently being watched. */ + size(): number { + return this.roots.size; + } + + /** + * Start the polling loop. No-op if already running. Detached from any caller; + * safe to call from a request handler. + */ + start(): void { + if (this.interval) return; + // Fire one check immediately so the user doesn't wait `intervalMs` for + // the first round after restart. + void this.tick(); + this.interval = setInterval(() => void this.tick(), this.intervalMs); + // unref so the watchdog never keeps the event loop alive on its own. + this.interval.unref?.(); + } + + /** Stop the polling loop. Idempotent. */ + stop(): void { + if (this.interval) { + clearInterval(this.interval); + this.interval = null; + } + } + + /** + * Run one round: for each watched root, check daemon liveness and respawn + * if missing. Exposed for tests + manual triggers. + */ + async tick(): Promise { + for (const root of this.roots) { + await this.checkAndRespawn(root); + } + } + + /** + * If no live daemon serves `root`, spawn one. Exposed for tests + manual + * triggers. Resolves to `true` if a daemon was spawned, `false` if one + * was already running or no `.codegraph/` is reachable from `root`. + */ + async checkAndRespawn(root: string): Promise { + // We can't trust `listDaemons` to surface a dead daemon for us — it filters + // out dead entries from its return value (even with `prune: false`, see + // daemon-registry.ts). So we check the pidfile directly: it IS the + // authoritative pointer (the registry is a discovery index, the lockfile + // is the source of truth). + if (this.hasLiveDaemon(root)) return false; + // Only spawn if the project has a `.codegraph/` — no point spawning a + // daemon for an uninitialized project. + if (!findNearestCodeGraphRoot(root)) return false; + return this.spawn(root); + } + + /** Spawn a fresh daemon for `root`. Detached; safe to call from any context. */ + spawn(root: string): boolean { + if (!this.scriptPath) return false; + const logPath = path.join(getCodeGraphDir(root), 'daemon.log'); + let logFd: number | null = null; + let stdio: 'ignore' | [ 'ignore', number, number ] = 'ignore'; + try { + logFd = fs.openSync(logPath, 'a'); + stdio = [ 'ignore', logFd, logFd ]; + } catch { + stdio = 'ignore'; + } + try { + // Don't leak the watchdog's host pid into the daemon (would trip its PPID + // watchdog on this very process). Scrub it on spawn. + const env: NodeJS.ProcessEnv = { ...process.env, [DAEMON_INTERNAL_ENV]: '1' }; + delete env[HOST_PPID_ENV]; + this.spawnFn( + this.nodePath, + [ ...process.execArgv, this.scriptPath, 'serve', '--mcp', '--path', root ], + { + detached: true, + windowsHide: true, + env, + stdio, + } + ); + return true; + } catch { + return false; + } finally { + if (logFd !== null) { + try { fs.closeSync(logFd); } catch { /* ignore */ } + } + } + } + + /** + * Wait for a daemon's socket to appear at one of the candidate paths under + * `root`. Used by callers that want a synchronous spawn-then-use flow; not + * required by the watchdog itself (which only needs liveness, not binding). + */ + async waitForSocket(root: string, maxRetries = SPAWN_BIND_MAX_RETRIES): Promise { + const candidates = getDaemonSocketCandidates(root); + for (let i = 0; i < maxRetries; i++) { + for (const candidate of candidates) { + try { + // Any successful stat means the socket/pipe exists. The launcher + // does a real connect on top of this; we only need existence. + fs.statSync(candidate); + return true; + } catch { + /* not yet */ + } + } + await new Promise((r) => setTimeout(r, SPAWN_BIND_RETRY_DELAY_MS)); + } + return false; + } + + /** True if a daemon's pid file at `root` points at a live process. */ + hasLiveDaemon(root: string): boolean { + let raw: string; + try { + raw = fs.readFileSync(getDaemonPidPath(root), 'utf8'); + } catch { + return false; + } + const info = decodeLockInfo(raw); + if (!info) return false; + return isProcessAlive(info.pid); + } +} \ No newline at end of file diff --git a/src/mcp/index.ts b/src/mcp/index.ts index 8c50c7a9..73b58c0f 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -54,6 +54,7 @@ import { checkForUpdateInBackground } from '../upgrade/update-check'; import { EARLY_PPID } from './early-ppid'; import { supervisionLostReason, parsePpidPollMs, parseHostPpid } from './ppid-watchdog'; import { installMainThreadWatchdog, WatchdogHandle } from './liveness-watchdog'; +import { DaemonWatchdog } from './daemon-watchdog'; import { armStartupHandshakeTimeout } from './startup-handshake'; import { treatStdinFailureAsShutdown } from './stdin-teardown'; import { HOST_PPID_ENV } from '../extraction/wasm-runtime-flags'; @@ -197,6 +198,11 @@ export class MCPServer { // Worker-thread liveness watchdog (#850). Long-lived modes only; SIGKILLs the // process if the main thread wedges in a non-yielding sync loop. private livenessWatchdog: WatchdogHandle | null = null; + // Per-project daemon liveness watchdog (#116). Started in proxy mode: polls + // the daemon for the root we serve every `intervalMs` and respawns it if it + // dies (e.g. Stop-Process, kill -9) — so the incremental file-watcher + // recovers without the user manually running `codegraph-vba sync`. + private daemonWatchdog: DaemonWatchdog | null = null; // PPID watchdog baseline — from the CLI entry's earliest-possible capture // (early-ppid.ts). Capturing here (construction) already lost the race when // the launcher was killed during module loading (#1185). @@ -263,6 +269,11 @@ export class MCPServer { // Runs until the host disconnects; the proxy installs its own watchdog and // falls back to an in-process engine if the daemon never comes up. this.mode = 'proxy'; + // #116: if the daemon dies while the proxy is up (Stop-Process, kill -9, + // crash), the file-watcher dies with it. Watchdog respawns it. + this.daemonWatchdog = new DaemonWatchdog(); + this.daemonWatchdog.watch(root); + this.daemonWatchdog.start(); await this.runProxyWithLocalHandshake(root); return; } catch (err) { @@ -290,6 +301,10 @@ export class MCPServer { this.livenessWatchdog.stop(); this.livenessWatchdog = null; } + if (this.daemonWatchdog) { + this.daemonWatchdog.stop(); + this.daemonWatchdog = null; + } if (this.daemon) { void this.daemon.stop('stop()'); // Daemon.stop calls process.exit; nothing else to do.