diff --git a/CHANGELOG.md b/CHANGELOG.md index 690677cdf..27e501978 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - VB.NET designer code such as `New System.Drawing.Point(…)` or `Me.Size = New System.Drawing.Size(…)` is no longer linked to a project type or member that only shares the name, and a class nested inside another is matched by its bare name only from inside that class or one that inherits it. - Upgrading CodeGraph while an agent session is open no longer leaves the old version's background server in charge of your project: the first session started from the new install stops it and starts a current one in its place, even while sessions opened before the upgrade are still running. That old server could no longer load the language parsers the upgrade removed, so it saved every file it re-indexed with no symbols, while new sessions could only read the index beside it without keeping it up to date. A background server from a newer install is never stopped, and sessions opened before the upgrade keep the old version until you restart them. Thanks @lipchey for the report. (#2335) - A file is no longer saved with no symbols when its language parser can't be loaded, which is what happened to every file a background server re-indexed after an upgrade removed its install: the file keeps what it had and is indexed again once the parser loads, and files an earlier version emptied this way are re-indexed by the next sync. A background server also exits on its own once its install is upgraded or removed, so the next session starts one from the current install. Thanks @lipchey for the report. (#2335) +- The shared background server is no longer killed with "Main thread unresponsive" while it brings another project's index up to date, such as one a session names with `projectPath`: only its own project's index counted as progress, so a long update of any other looked like a hang, and the server was stopped and restarted again and again. Every index the server keeps up to date now counts, and so does the one a session updates on its own when the background server is unavailable. Thanks @mradwankhalil for the report and the first fix. (#2404) - `codegraph upgrade` no longer wipes your agent instructions file, such as a global `AGENTS.md`, `CLAUDE.md` or `GEMINI.md`, when it refreshes CodeGraph's section in it: a stray `` line above that section, like one left behind when part of an older section was deleted by hand, made the refresh replace everything in between, often the whole file. CodeGraph now rewrites only its own section, in `codegraph install` and `codegraph uninstall` too, and keeps everything around it exactly as it was, Windows line endings included. Thanks @pzaj2 for the report. (#2506) - `codegraph install`, `upgrade` and `uninstall` now write through an instructions or config file that is a symbolic link, such as a `~/.codex/AGENTS.md` or `~/.claude.json` linked from your dotfiles, instead of replacing the link with a copy: the shared file gets CodeGraph's settings, uninstall takes them out again, and the link stays in place. A link CodeGraph can't write through, like one to a read-only file or into a folder that doesn't exist, is now reported by name and left alone, and an upgrade still refreshes your other agents. Thanks @jguerra6 for the report and @danusha2345 for the first fix. (#2721) - `codegraph install` and `upgrade` no longer break Codex's `config.toml` when CodeGraph's `[mcp_servers.codegraph]` entry is written differently from how CodeGraph writes it, such as indented, with spaces or quotes inside the brackets, or with Windows line endings: the entry is now updated in place, where before a second copy was added and Codex could load none of the file. A file already broken that way is repaired on the next install or upgrade, an example of the entry inside a multiline setting such as `developer_instructions` is no longer mistaken for it, uninstall removes the entry however it is written, and an entry you wrote with dotted keys or as an inline table is left as you wrote it. When your `mcp_servers` settings are one inline table, CodeGraph now says it can't add itself instead of breaking the file. Thanks @dencoseca for the report, and @danusha2345 and @rudycelekli for the first fixes. (#2562) diff --git a/__tests__/liveness-watchdog-projects.test.ts b/__tests__/liveness-watchdog-projects.test.ts new file mode 100644 index 000000000..b97fde92a --- /dev/null +++ b/__tests__/liveness-watchdog-projects.test.ts @@ -0,0 +1,346 @@ +/** + * The liveness watchdog counts disk progress on every index the process + * writes, not only the one it was armed with (#2404). + * + * The shared daemon arms its watchdog with its own project's index files, yet + * it also syncs every other project a session names by `projectPath`, on the + * same main thread. A long sync of one of those read as a wedge (heartbeat + * silence, no change on the watched files), so the healthy daemon was + * SIGKILLed mid-sync, and again after every reattach. A proxy serving + * in-process, armed with no files at all, had the same blind spot. + * + * Each case spawns a real process with the real watchdog child and a short + * timeout (the technique of liveness-watchdog.test.ts), then blocks that + * process's main thread while a file or an index advances. + */ +import { describe, it, expect, beforeAll, afterEach } from 'vitest'; +import { spawn } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { getCodeGraphDir } from '../src/directory'; +import { getWriterPidPath } from '../src/mcp/writer-lock'; + +const WATCHDOG = path.resolve(__dirname, '../dist/mcp/liveness-watchdog.js'); +const LIFECYCLE = path.resolve(__dirname, '../dist/mcp/project-lifecycle.js'); + +beforeAll(() => { + for (const file of [WATCHDOG, LIFECYCLE]) { + if (!fs.existsSync(file)) throw new Error(`Build the project first: ${file} is missing (run npm run build).`); + } +}); + +const dirs: string[] = []; +afterEach(async () => { + // A killed process may hold its SQLite files for a moment on Windows. + for (const dir of dirs.splice(0)) { + const deadline = Date.now() + 5000; + for (;;) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + break; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code ?? ''; + if (!['EBUSY', 'ENOTEMPTY', 'EPERM'].includes(code) || Date.now() > deadline) throw error; + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } + } +}); + +function tempDir(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-wdp-')); + dirs.push(dir); + return dir; +} + +interface Outcome { + code: number | null; + signal: NodeJS.Signals | 'TIMEOUT' | null; + stdout: string; + stderr: string; +} + +/** + * Run `body` in a fresh Node process whose watchdog times out after + * `timeoutMs`. Resolves once the process and its watchdog child are gone (the + * child shares the process's stderr, where it writes its kill notice). + */ +function runChild(body: string, timeoutMs: number, hardTimeoutMs = 15_000): Promise { + const child = spawn(process.execPath, ['-e', body], { + env: { ...process.env, CODEGRAPH_NO_WATCHDOG: '0', CODEGRAPH_WATCHDOG_TIMEOUT_MS: String(timeoutMs) }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (d) => { stdout += d; }); + child.stderr.on('data', (d) => { stderr += d; }); + return new Promise((resolve) => { + const timer = setTimeout(() => { + child.kill('SIGKILL'); + resolve({ code: null, signal: 'TIMEOUT', stdout, stderr }); + }, hardTimeoutMs); + child.on('close', (code, signal) => { + clearTimeout(timer); + resolve({ code, signal, stdout, stderr }); + }); + }); +} + +/** The process blocked its main thread and ran to its own exit(5). */ +function expectSurvived(r: Outcome): void { + expect(r.stdout, r.stderr).toContain('BLOCKING'); + expect(r.stderr).not.toContain('Main thread unresponsive'); + expect(r.signal).toBeNull(); + expect(r.code, r.stderr).toBe(5); +} + +/** The process blocked its main thread and the watchdog killed it there. */ +function expectKilled(r: Outcome): void { + expect(r.stdout, r.stderr).toContain('BLOCKING'); + expect(r.stderr).toContain('Main thread unresponsive'); + expect(r.code).not.toBe(5); +} + +/** Grow `file` every 100ms for `forMs`. */ +function growFile(file: string, forMs: number): Promise { + return new Promise((resolve) => { + const iv = setInterval(() => { fs.appendFileSync(file, 'x'.repeat(64)); }, 100); + setTimeout(() => { clearInterval(iv); resolve(); }, forMs); + }); +} + +/** Spawned-process source: block the main thread for `ms`, then exit(5). */ +const block = (ms: number): string => ` + require('fs').writeSync(1, 'BLOCKING\\n'); + const end = Date.now() + ${ms}; + while (Date.now() < end) {} + process.exit(5); +`; + +describe('files added to the watchdog at runtime (#2404)', () => { + it('counts a file added after arming as progress, like an armed one', async () => { + const file = path.join(tempDir(), 'other.db-wal'); + fs.writeFileSync(file, 'seed'); + const [r] = await Promise.all([ + runChild(` + const { installMainThreadWatchdog, addWatchdogProgressPaths } = require(${JSON.stringify(WATCHDOG)}); + installMainThreadWatchdog(); + addWatchdogProgressPaths([${JSON.stringify(file)}]); + setTimeout(() => { ${block(2500)} }, 200); + `, 500), + growFile(file, 3500), + ]); + expectSurvived(r); + }, 20_000); + + it('counts every file one call adds, right before a block', async () => { + // Two files in one call, then the main thread blocks at once and only the + // second advances, as a sync advances the -wal it registered after the db. + const dir = tempDir(); + const db = path.join(dir, 'other.db'); + const wal = path.join(dir, 'other.db-wal'); + fs.writeFileSync(db, 'seed'); + fs.writeFileSync(wal, 'seed'); + const [r] = await Promise.all([ + runChild(` + const { installMainThreadWatchdog, addWatchdogProgressPaths } = require(${JSON.stringify(WATCHDOG)}); + installMainThreadWatchdog(); + setTimeout(() => { + addWatchdogProgressPaths([${JSON.stringify(db)}, ${JSON.stringify(wal)}]); + ${block(2500)} + }, 200); + `, 500), + growFile(wal, 3500), + ]); + expectSurvived(r); + }, 20_000); + + it('counts a file added before the watchdog was armed', async () => { + const file = path.join(tempDir(), 'other.db-wal'); + fs.writeFileSync(file, 'seed'); + const [r] = await Promise.all([ + runChild(` + const { installMainThreadWatchdog, addWatchdogProgressPaths } = require(${JSON.stringify(WATCHDOG)}); + addWatchdogProgressPaths([${JSON.stringify(file)}]); + installMainThreadWatchdog(); + setTimeout(() => { ${block(2500)} }, 200); + `, 500), + growFile(file, 3500), + ]); + expectSurvived(r); + }, 20_000); + + it('keeps counting a file while another registration of it holds', async () => { + const file = path.join(tempDir(), 'other.db-wal'); + fs.writeFileSync(file, 'seed'); + const [r] = await Promise.all([ + runChild(` + const { installMainThreadWatchdog, addWatchdogProgressPaths } = require(${JSON.stringify(WATCHDOG)}); + installMainThreadWatchdog(); + const first = addWatchdogProgressPaths([${JSON.stringify(file)}]); + addWatchdogProgressPaths([${JSON.stringify(file)}]); + first(); + first(); // idempotent: must not drop the second registration + setTimeout(() => { ${block(2500)} }, 200); + `, 500), + growFile(file, 3500), + ]); + expectSurvived(r); + }, 20_000); + + it('stops counting a file once dropped', async () => { + const file = path.join(tempDir(), 'other.db-wal'); + fs.writeFileSync(file, 'seed'); + const [r] = await Promise.all([ + runChild(` + const { installMainThreadWatchdog, addWatchdogProgressPaths } = require(${JSON.stringify(WATCHDOG)}); + installMainThreadWatchdog(); + const drop = addWatchdogProgressPaths([${JSON.stringify(file)}]); + setTimeout(() => { drop(); setTimeout(() => { ${block(2500)} }, 100); }, 100); + `, 500), + growFile(file, 3500), + ]); + expectKilled(r); + }, 20_000); + + it('leaves no watchdog process behind as files come and go', async () => { + const file = path.join(tempDir(), 'other.db-wal'); + fs.writeFileSync(file, 'seed'); + const child = spawn(process.execPath, ['-e', ` + const { installMainThreadWatchdog, addWatchdogProgressPaths } = require(${JSON.stringify(WATCHDOG)}); + const handle = installMainThreadWatchdog(); + const drop = addWatchdogProgressPaths([${JSON.stringify(file)}]); + setTimeout(() => { + drop(); + setTimeout(() => { + addWatchdogProgressPaths([${JSON.stringify(file)}]); + setTimeout(() => { handle.stop(); process.exit(0); }, 300); + }, 200); + }, 200); + `], { + env: { ...process.env, CODEGRAPH_NO_WATCHDOG: '0', CODEGRAPH_WATCHDOG_TIMEOUT_MS: '5000', CODEGRAPH_MCP_DEBUG: '1' }, + stdio: ['ignore', 'ignore', 'pipe'], + }); + let stderr = ''; + child.stderr.on('data', (d) => { stderr += d; }); + // 'exit', not 'close': a watchdog child left alive would hold the stderr pipe. + const code = await new Promise((resolve) => child.on('exit', (c) => resolve(c))); + expect(code, stderr).toBe(0); + const armed = (): number[] => [...stderr.matchAll(/armed \(child pid (\d+)\)/g)].map((m) => Number(m[1])); + for (const until = Date.now() + 2000; armed().length < 4 && Date.now() < until;) { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + const pids = armed(); + // The first child, then one per change (added, dropped, added again). + expect(pids, stderr).toHaveLength(4); + const alive = (pid: number): boolean => { try { process.kill(pid, 0); return true; } catch { return false; } }; + const deadline = Date.now() + 5000; + while (pids.some(alive) && Date.now() < deadline) await new Promise((resolve) => setTimeout(resolve, 50)); + const survivors = pids.filter(alive); + for (const pid of survivors) { try { process.kill(pid, 'SIGKILL'); } catch { /* gone */ } } + expect(survivors).toEqual([]); + }, 20_000); + + it('starts no new watchdog for files dropped on the way out', async () => { + // A server letting go of its projects as it exits drops their files all at + // once; that must not spawn a watchdog per project just to exit with it. + const files = [0, 1, 2].map((i) => path.join(tempDir(), `other${i}.db-wal`)); + const child = spawn(process.execPath, ['-e', ` + const { installMainThreadWatchdog, addWatchdogProgressPaths } = require(${JSON.stringify(WATCHDOG)}); + installMainThreadWatchdog(); + const drops = ${JSON.stringify(files)}.map((file) => addWatchdogProgressPaths([file])); + setTimeout(() => { for (const drop of drops) drop(); process.exit(0); }, 200); + `], { + env: { ...process.env, CODEGRAPH_NO_WATCHDOG: '0', CODEGRAPH_WATCHDOG_TIMEOUT_MS: '5000', CODEGRAPH_MCP_DEBUG: '1' }, + stdio: ['ignore', 'ignore', 'pipe'], + }); + let stderr = ''; + child.stderr.on('data', (d) => { stderr += d; }); + const code = await new Promise((resolve) => child.on('close', (c) => resolve(c))); + expect(code, stderr).toBe(0); + // The first child, then one per file added; none for the drops. + expect([...stderr.matchAll(/armed \(child pid (\d+)\)/g)], stderr).toHaveLength(4); + }, 20_000); + + it('does not mistake a file joining the watch list for the disk advancing', async () => { + // The file never changes. The first silent timeout must compare it with + // itself, not with the list from before it joined: kill at ~1.5s, well + // before the process's own exit at ~2.6s. + const file = path.join(tempDir(), 'other.db-wal'); + fs.writeFileSync(file, 'seed'); + const r = await runChild(` + const { installMainThreadWatchdog, addWatchdogProgressPaths } = require(${JSON.stringify(WATCHDOG)}); + installMainThreadWatchdog(); + addWatchdogProgressPaths([${JSON.stringify(file)}]); + setTimeout(() => { ${block(2400)} }, 200); + `, 1500); + expectKilled(r); + }, 20_000); +}); + +describe('another project this process writes (#2404)', () => { + /** A project with an empty index, as `codegraph init` leaves one. */ + function makeProject(): { root: string; db: string } { + const root = tempDir(); + CodeGraph.initSync(root).close(); + return { root, db: path.join(getCodeGraphDir(root), 'codegraph.db') }; + } + + /** + * Arm the watchdog for project A, as the daemon keyed on A arms it, attach + * project B through the project lifecycle the daemon opens every + * `projectPath` with, then store into B's index synchronously on the main + * thread for 3s, as a long sync of B does. The graph object stands in for + * CodeGraph: the lifecycle only watches, syncs and closes it. + */ + function attachAndStore(a: { db: string }, b: { root: string; db: string }, opts: { release?: boolean } = {}): Promise { + return runChild(` + const { installMainThreadWatchdog } = require(${JSON.stringify(WATCHDOG)}); + const { acquireProject } = require(${JSON.stringify(LIFECYCLE)}); + const { DatabaseSync } = require('node:sqlite'); + const graph = { watch: () => true, sync: async () => ({}), close() {}, isIndexing: () => false }; + installMainThreadWatchdog({ progressPaths: [${JSON.stringify(a.db)}, ${JSON.stringify(`${a.db}-wal`)}] }); + const lease = acquireProject(${JSON.stringify(b.root)}, () => graph, {}); + lease.ready().then(async () => { + if (${opts.release ? 'true' : 'false'}) { + await lease.release(); + await new Promise((resolve) => setTimeout(resolve, 50)); // dropped files re-arm on the next turn + } + require('fs').writeSync(1, 'BLOCKING\\n'); + const db = new DatabaseSync(${JSON.stringify(b.db)}); + db.exec('CREATE TABLE IF NOT EXISTS store_2404 (x BLOB)'); + const end = Date.now() + 3000; + while (Date.now() < end) db.exec('INSERT INTO store_2404 VALUES (randomblob(2048))'); + db.close(); + process.exit(5); + }); + `, 1000); + } + + it('lets a long store into another project it now writes run to completion', async () => { + const a = makeProject(); + const b = makeProject(); + const r = await attachAndStore(a, b); + expectSurvived(r); + }, 20_000); + + it('does not count a project whose writer is another process', async () => { + const a = makeProject(); + const b = makeProject(); + // B's live writer is someone else (this test process, as a direct server + // that has caught up): the spawned process only reads B, so B's files + // advancing says nothing about its own main thread. + fs.writeFileSync(getWriterPidPath(b.root), JSON.stringify({ pid: process.pid, mode: 'direct', startedAt: Date.now(), ready: true }) + '\n'); + const r = await attachAndStore(a, b); + expectKilled(r); + }, 20_000); + + it('stops counting a project once it is released', async () => { + const a = makeProject(); + const b = makeProject(); + const r = await attachAndStore(a, b, { release: true }); + expectKilled(r); + }, 20_000); +}); diff --git a/src/mcp/liveness-watchdog.ts b/src/mcp/liveness-watchdog.ts index e0f2e2e2d..4e459de62 100644 --- a/src/mcp/liveness-watchdog.ts +++ b/src/mcp/liveness-watchdog.ts @@ -30,10 +30,11 @@ * * **Won't fire on real work.** Heavy parsing runs in the parse worker * (off-thread) and the daemon's indexing shells out to a child process, so the - * daemon's main thread only ever does fast, bounded work. The default timeout - * is ~300× the 5h #850 wedge shorter, yet far longer than any legitimate - * main-thread block. Opt out with `CODEGRAPH_NO_WATCHDOG=1`; tune with - * `CODEGRAPH_WATCHDOG_TIMEOUT_MS`. + * daemon's main thread mostly does fast, bounded work; the exception, a sync + * storing into SQLite, is what the disk-progress deferral below is for. The + * default timeout is ~300× the 5h #850 wedge shorter, yet far longer than any + * legitimate main-thread block. Opt out with `CODEGRAPH_NO_WATCHDOG=1`; tune + * with `CODEGRAPH_WATCHDOG_TIMEOUT_MS`. * * **Disk-progress deferral (`progressPaths`).** The CLI `index`/`init` path is * different: it runs the SQLite store on this thread, and one long synchronous @@ -49,6 +50,18 @@ * coinciding with unrelated file activity — or I/O hung beyond all reason — * still dies. A true wedge with no disk progress dies at the base timeout, * exactly as before. + * + * **Every index the process writes counts (#2404).** An MCP server syncs on + * its main thread too, and not only the project it was armed for: the shared + * daemon writes every project a session names by `projectPath`, and a proxy's + * in-process fallback writes its own. {@link addWatchdogProgressPaths} adds a + * project's index files the moment this process becomes its writer and drops + * them when it lets go; each change re-arms the watchdog with a new child + * whose argv carries the whole list. A new process gets its list the instant + * it is spawned, whatever the parent does next: a message on the heartbeat + * pipe could not promise that, since a pipe write queued behind one still in + * flight waits for an event-loop turn, and a sync can block the loop right + * after the project is added. */ import * as fs from 'fs'; import * as os from 'os'; @@ -174,27 +187,71 @@ export interface WatchdogOptions { /** * Files whose size/mtime advancing counts as forward progress (the SQLite * DB + `-wal` for an in-process indexer). With paths supplied, a silent - * timeout only kills when the files did NOT advance — see the header. Omit - * for pure heartbeat behavior (the daemon, whose main thread never runs - * long synchronous work). + * timeout only kills when the files did NOT advance — see the header. The + * files of other indexes the process comes to write join at runtime through + * {@link addWatchdogProgressPaths}; with neither, a silent timeout kills. */ progressPaths?: string[]; } /** - * Install the main-thread liveness watchdog for a long-lived process. Returns a - * handle to stop it, or `null` when disabled or when the child can't be spawned - * (degraded, never throws — a missing watchdog must never keep a process from - * starting). + * Files added with {@link addWatchdogProgressPaths} and not yet dropped, with + * how many registrations hold each. */ -export function installMainThreadWatchdog(options: WatchdogOptions = {}): WatchdogHandle | null { - if (isEnvTruthy(process.env.CODEGRAPH_NO_WATCHDOG)) return null; +const addedProgressPaths = new Map(); - const timeoutMs = parseWatchdogTimeoutMs(process.env.CODEGRAPH_WATCHDOG_TIMEOUT_MS); - const checkMs = deriveCheckIntervalMs(timeoutMs); - const capMs = timeoutMs * PROGRESS_CAP_MULTIPLIER; - const progressPaths = options.progressPaths ?? []; +/** + * Tells each armed watchdog the list of files changed: `added` when a file + * joined it, false when files only left it. + */ +const armedWatchdogs = new Set<(added: boolean) => void>(); +/** + * Count `paths` advancing as forward progress for this process's liveness + * watchdog, armed now or later, until the returned function is called + * (#2404). For the index files of a project this process has just become the + * writer of: its syncs store on the main thread, and without this a long one + * reads as a wedge whenever the project is not the one the watchdog was armed + * for. Registrations of one file nest, and the returned function is + * idempotent. + */ +export function addWatchdogProgressPaths(paths: readonly string[]): () => void { + let added = false; + for (const file of paths) { + const held = addedProgressPaths.get(file) ?? 0; + addedProgressPaths.set(file, held + 1); + if (held === 0) added = true; + } + if (added) for (const changed of armedWatchdogs) changed(true); + let dropped = false; + return () => { + if (dropped) return; + dropped = true; + let removed = false; + for (const file of paths) { + const held = addedProgressPaths.get(file) ?? 0; + if (held > 1) { + addedProgressPaths.set(file, held - 1); + } else if (held === 1) { + addedProgressPaths.delete(file); + removed = true; + } + } + if (removed) for (const changed of armedWatchdogs) changed(false); + }; +} + +/** One watchdog child process, watching a fixed list of files. */ +interface WatchdogChild { + paths: readonly string[]; + /** Write one heartbeat byte. */ + beat(): void; + /** End the child: EOF, then a kill for good measure. */ + stop(): void; +} + +/** Spawn a watchdog child for `paths`, or null when it can't be spawned. */ +function spawnWatchdogChild(timeoutMs: number, checkMs: number, capMs: number, paths: readonly string[]): WatchdogChild | null { let child: ChildProcess; try { // No execArgv inheritance (unlike Worker), so the child carries none of our @@ -202,7 +259,7 @@ export function installMainThreadWatchdog(options: WatchdogOptions = {}): Watchd // fd 2 so the kill notice lands wherever the parent logs (daemon.log). child = spawn( process.execPath, - ['-e', CHILD_SOURCE, String(process.pid), String(timeoutMs), String(capMs), ...progressPaths], + ['-e', CHILD_SOURCE, String(process.pid), String(timeoutMs), String(capMs), ...paths], { stdio: ['pipe', 'ignore', 'inherit'], windowsHide: true, @@ -228,27 +285,88 @@ export function installMainThreadWatchdog(options: WatchdogOptions = {}): Watchd stdin.on('error', () => { /* child gone; heartbeat writes are best-effort */ }); child.on('error', (err) => debug(`child error: ${err.message}`)); - // Heartbeat: a byte per tick. When the main thread wedges, these stop and the - // child's timeout fires. unref'd so it never keeps the process alive itself. - const heartbeat = setInterval(() => { - try { stdin.write('\n'); } catch { /* child gone */ } - }, checkMs); - heartbeat.unref(); - // Neither the child nor its pipe should keep the parent alive past its work. child.unref(); try { (stdin as unknown as { unref?: () => void }).unref?.(); } catch { /* ignore */ } - debug(`armed (child pid ${child.pid ?? '?'}): timeoutMs=${timeoutMs} checkMs=${checkMs} progressPaths=${progressPaths.length}`); + debug(`armed (child pid ${child.pid ?? '?'}): timeoutMs=${timeoutMs} checkMs=${checkMs} progressPaths=${paths.length}`); + + return { + paths, + beat(): void { + try { stdin.write('\n'); } catch { /* child gone */ } + }, + stop(): void { + try { stdin.end(); } catch { /* ignore */ } // EOF -> child exits cleanly + try { child.kill(); } catch { /* ignore */ } // belt-and-suspenders + }, + }; +} + +/** + * Install the main-thread liveness watchdog for a long-lived process. Returns a + * handle to stop it, or `null` when disabled or when the child can't be spawned + * (degraded, never throws — a missing watchdog must never keep a process from + * starting). + */ +export function installMainThreadWatchdog(options: WatchdogOptions = {}): WatchdogHandle | null { + if (isEnvTruthy(process.env.CODEGRAPH_NO_WATCHDOG)) return null; + + const timeoutMs = parseWatchdogTimeoutMs(process.env.CODEGRAPH_WATCHDOG_TIMEOUT_MS); + const checkMs = deriveCheckIntervalMs(timeoutMs); + const capMs = timeoutMs * PROGRESS_CAP_MULTIPLIER; + const armedPaths = options.progressPaths ?? []; + const watchedPaths = (): string[] => [...new Set([...armedPaths, ...addedProgressPaths.keys()])]; + + let current = spawnWatchdogChild(timeoutMs, checkMs, capMs, watchedPaths()); + if (!current) return null; + + // Heartbeat: a byte per tick. When the main thread wedges, these stop and the + // child's timeout fires. unref'd so it never keeps the process alive itself. + const heartbeat = setInterval(() => current?.beat(), checkMs); + heartbeat.unref(); + + // The list of files changed (#2404): a child for the new list takes over, + // then the old one goes. Always from a turning event loop, so the old child, + // its last heartbeat recent, has no reason to kill meanwhile; a spawn that + // fails leaves it watching the old list. An added file takes effect at once, + // since a sync may block the loop right after. Dropped files wait for the + // next turn, so a process letting go of several projects, as one does on its + // way out, re-arms once or not at all. + let pendingRearm: NodeJS.Timeout | null = null; + const rearm = (): void => { + if (pendingRearm) clearTimeout(pendingRearm); + pendingRearm = null; + if (!current) return; + const paths = watchedPaths(); + if (paths.length === current.paths.length && paths.every((p) => current!.paths.includes(p))) return; + const next = spawnWatchdogChild(timeoutMs, checkMs, capMs, paths); + if (!next) return; + const previous = current; + current = next; + previous.stop(); + }; + const changed = (added: boolean): void => { + if (added) { + rearm(); + } else if (!pendingRearm) { + pendingRearm = setTimeout(rearm, 0); + pendingRearm.unref(); + } + }; + armedWatchdogs.add(changed); let stopped = false; return { stop(): void { if (stopped) return; stopped = true; + armedWatchdogs.delete(changed); + if (pendingRearm) clearTimeout(pendingRearm); + pendingRearm = null; clearInterval(heartbeat); - try { stdin.end(); } catch { /* ignore */ } // EOF -> child exits cleanly - try { child.kill(); } catch { /* ignore */ } // belt-and-suspenders + current?.stop(); + current = null; }, }; } diff --git a/src/mcp/project-lifecycle.ts b/src/mcp/project-lifecycle.ts index 1a634815e..88ffc50ae 100644 --- a/src/mcp/project-lifecycle.ts +++ b/src/mcp/project-lifecycle.ts @@ -1,10 +1,12 @@ /** Shared synchronization ownership for projects accessed by MCP engines (#1835). */ import { realpathSync } from 'fs'; +import * as path from 'path'; import type { Socket } from 'net'; import type CodeGraph from '../index'; -import { canonicalProjectRoot, isInitialized } from '../directory'; +import { canonicalProjectRoot, getCodeGraphDir, isInitialized } from '../directory'; import { LockUnavailableError, watchDisabledReason } from '../sync'; import { getDaemonSocketCandidates } from './daemon-paths'; +import { addWatchdogProgressPaths } from './liveness-watchdog'; import { connectWithHello } from './proxy'; import { markWriterReady, readWriterLock, releaseWriterLock, tryAcquireWriterLock } from './writer-lock'; @@ -20,6 +22,8 @@ interface Project { socket: Socket | null; timer: NodeJS.Timeout; options: Parameters[0]; + /** Drops the index files from the liveness watchdog's progress; set while this process writes them (#2404). */ + dropProgress: (() => void) | null; } export interface ProjectLease { @@ -42,7 +46,7 @@ export function acquireProject( if (!project) { const cg = open(); project = { key, cg, refs: 0, owner: false, caughtUp: false, retirement: null, gate: null, socket: null, options, - timer: setInterval(() => { void ready(root, project!); }, 1000) }; + dropProgress: null, timer: setInterval(() => { void ready(root, project!); }, 1000) }; project.timer.unref(); projects.set(key, project); } @@ -86,6 +90,9 @@ function retire(root: string, project: Project): Promise { projects.delete(project.key); project.cg.close(); if (project.owner) releaseWriterLock(root); + // After the close, which may still checkpoint the WAL on this thread. + project.dropProgress?.(); + project.dropProgress = null; resolve(); }; finish(); @@ -144,6 +151,15 @@ async function activate(root: string, project: Project): Promise { return; } project.owner = true; + // This process writes the index from here on, in long synchronous stretches + // on the main thread: the catch-up below and every watcher sync. The + // liveness watchdog must count its files advancing as progress, as it does + // for the project it was armed with; otherwise a long sync of any other + // project reads as a wedge and the server is killed mid-sync (#2404). + if (!project.dropProgress) { + const db = path.join(getCodeGraphDir(root), 'codegraph.db'); + project.dropProgress = addWatchdogProgressPaths([db, `${db}-wal`]); + } const disabled = watchDisabledReason(root); if (disabled) { process.stderr.write(`[CodeGraph MCP] File watcher disabled for ${root} — ${disabled}.\n`);