From 1926ec39a218d12e632f6c03f4d89dce6b18b1f5 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sun, 27 Sep 2026 15:26:49 +0000 Subject: [PATCH] fix(index): coordinate rebuilds with running MCP daemons (#1325) (#2051) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: safely stop daemon before reindex * fix(index): coordinate rebuilds with running MCP daemons (#1325) Stop verified project daemons and confirm termination before recreating SQLite. Fence daemon and proxy reopening throughout rebuilds while preserving foreign and newer locks. Add Windows lifecycle coverage and cross-platform shutdown regressions. Co-authored-by: Harry B ✦ Co-Authored-By: Claude Opus 5.5 * fix(mcp): fence project opens during index rebuilds after rebase Carry the #1325 rebuild fence through the upstream explicit-project lifecycle, read-only engine opens, synchronous initialization retries, and direct writer acquisition. Preserve shared project leases, read-only fallback behavior, freshness handling, and direct-mode query pools. Cover watched, unwatched, and read-only engines blocking during a foreign rebuild and recovering after the fence is released. * fix(mcp): answer a running index rebuild as guidance, not a tool error (#1325) A projectPath call during `codegraph index` hit assertNoRebuild's plain Error and returned isError; it is an expected, temporary condition, so it now uses a RebuildInProgressError that the tool handler answers success-shaped. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Harry B ✦ Co-authored-by: Claude Opus 5.5 --- __tests__/daemon-manager.test.ts | 8 +- __tests__/daemon-registry.test.ts | 113 ++++++++++++++++- __tests__/index-command.test.ts | 45 +++++++ __tests__/index-daemon-rebuild.test.ts | 121 ++++++++++++++++++ __tests__/mcp-projectpath-lifecycle.test.ts | 44 +++++++ src/bin/codegraph.ts | 134 ++++++++++++-------- src/mcp/daemon-manager.ts | 4 + src/mcp/daemon-registry.ts | 25 +++- src/mcp/daemon.ts | 2 + src/mcp/engine.ts | 8 +- src/mcp/index.ts | 3 + src/mcp/tools.ts | 7 +- src/mcp/writer-lock.ts | 35 ++++- 13 files changed, 472 insertions(+), 77 deletions(-) create mode 100644 __tests__/index-daemon-rebuild.test.ts diff --git a/__tests__/daemon-manager.test.ts b/__tests__/daemon-manager.test.ts index af000d66..de24153d 100644 --- a/__tests__/daemon-manager.test.ts +++ b/__tests__/daemon-manager.test.ts @@ -98,18 +98,20 @@ describe('runDaemonPicker', () => { expect(h.getDone()).toBe('Done.'); }); - it('does not report an unverified daemon as stopped', async () => { + it.each(['unverified', 'still-running'] as const)('does not report %s as stopped', async (outcome) => { const h = harness([rec('/p/a', 42, 1)], ['/p/a', CANCEL]); h.deps.stop = async (root): Promise => ({ root, pid: 42, - outcome: 'unverified', + outcome, }); await runDaemonPicker(h.deps); expect(h.notes).toEqual([ - 'Could not verify daemon (pid 42); left it running with its artifacts intact — /p/a', + outcome === 'unverified' + ? 'Could not verify daemon (pid 42); left it running with its artifacts intact — /p/a' + : 'Could not stop daemon (pid 42); left its artifacts intact — /p/a', ]); expect(h.getDone()).toContain('Cancelled'); }); diff --git a/__tests__/daemon-registry.test.ts b/__tests__/daemon-registry.test.ts index 881d215f..0ad4707d 100644 --- a/__tests__/daemon-registry.test.ts +++ b/__tests__/daemon-registry.test.ts @@ -1,5 +1,5 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { spawn } from 'child_process'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { execFileSync, spawn } from 'child_process'; import * as fs from 'fs'; import * as net from 'net'; import * as os from 'os'; @@ -15,7 +15,7 @@ import { stopDaemonAt, type DaemonRecord, } from '../src/mcp/daemon-registry'; -import { encodeLockInfo, getDaemonPidPath } from '../src/mcp/daemon-paths'; +import { encodeLockInfo, getDaemonPidPath, getDaemonSocketPath } 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. */ @@ -31,6 +31,24 @@ function rec(root: string, pid: number, startedAt = Date.now()): DaemonRecord { return { root, pid, version: '1.0.0', socketPath: `${root}/.codegraph/daemon.sock`, startedAt }; } +function waitForExit(child: ReturnType): Promise { + return child.exitCode !== null + ? Promise.resolve() + : new Promise((resolve) => child.once('exit', () => resolve())); +} + +function startDetachedProcess(): number { + const source = [ + "const { spawn } = require('child_process');", + "const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { detached: true, stdio: 'ignore' });", + 'child.unref();', + 'console.log(child.pid);', + ].join(' '); + const pid = Number(execFileSync(process.execPath, ['-e', source], { encoding: 'utf8' }).trim()); + if (!Number.isInteger(pid) || pid <= 0) throw new Error('could not start daemon fixture'); + return pid; +} + describe('daemon-registry', () => { let tmpHome: string; let prevHome: string | undefined; @@ -63,6 +81,95 @@ describe('daemon-registry', () => { }); }); + it('does not signal a live PID unless its socket identifies a CodeGraph daemon', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-foreign-daemon-')); + fs.mkdirSync(path.join(root, '.codegraph')); + const pid = startDetachedProcess(); + fs.writeFileSync( + getDaemonPidPath(root), + encodeLockInfo({ pid, version: 'test', socketPath: path.join(root, '.codegraph', 'missing.sock'), startedAt: Date.now() }), + ); + + try { + const result = await stopDaemonAt(root, { preserveUnverified: true }); + expect(result.outcome).toBe('unverified'); + expect(isProcessAlive(pid)).toBe(true); + expect(fs.existsSync(getDaemonPidPath(root))).toBe(true); + } finally { + if (isProcessAlive(pid)) process.kill(pid, 'SIGKILL'); + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it.each([false, true])('confirms daemon termination (refused: %s)', async (refused) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-daemon-identity-')); + fs.mkdirSync(path.join(root, '.codegraph')); + const socketPath = getDaemonSocketPath(root); + const pid = startDetachedProcess(); + const server = net.createServer((socket) => { + socket.end(`${JSON.stringify({ protocol: 1, codegraph: 'test', pid, socketPath })}\n`); + }); + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(socketPath, resolve); + }); + fs.writeFileSync( + getDaemonPidPath(root), + encodeLockInfo({ pid, version: 'test', socketPath, startedAt: Date.now() }), + ); + + const originalKill = process.kill.bind(process); + const kill = vi.spyOn(process, 'kill').mockImplementation((target, signal) => { + if (refused && target === pid && signal !== 0) { + throw Object.assign(new Error('Access denied'), { code: 'EPERM' }); + } + return originalKill(target, signal); + }); + try { + const result = await stopDaemonAt(root); + if (refused) { + expect(result.outcome).toBe('still-running'); + expect(isProcessAlive(pid)).toBe(true); + expect(fs.existsSync(getDaemonPidPath(root))).toBe(true); + return; + } + expect(result.outcome).toMatch(/term|kill/); + expect(isProcessAlive(pid)).toBe(false); + } finally { + kill.mockRestore(); + if (isProcessAlive(pid)) process.kill(pid, 'SIGKILL'); + await new Promise(resolve => server.close(() => resolve())); + fs.rmSync(root, { recursive: true, force: true }); + } + }, 15000); + + it('preserves a newer lock installed during a successful stop identity probe', async () => { + const root = fs.mkdtempSync(path.join(tmpHome, 'stop-race-')); + fs.mkdirSync(path.join(root, '.codegraph')); + const pid = startDetachedProcess(); + const socketPath = getDaemonSocketPath(root); + const pidPath = getDaemonPidPath(root); + const original = { pid, version: 'test', socketPath, startedAt: 1 }; + const replacement = encodeLockInfo({ ...original, startedAt: 2 }); + fs.writeFileSync(pidPath, encodeLockInfo(original)); + const server = net.createServer(socket => { + fs.writeFileSync(pidPath, replacement); + socket.end(JSON.stringify({ protocol: 1, codegraph: 'test', pid }) + '\n'); + }); + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(socketPath, resolve); + }); + try { + expect((await stopDaemonAt(root, { preserveUnverified: true })).outcome).toBe('unverified'); + expect(isProcessAlive(pid)).toBe(true); + expect(fs.readFileSync(pidPath, 'utf8')).toBe(replacement); + } finally { + if (isProcessAlive(pid)) process.kill(pid, 'SIGKILL'); + await new Promise(resolve => server.close(() => resolve())); + } + }); + it('listDaemons returns [] when nothing is registered (no dir yet)', () => { expect(listDaemons()).toEqual([]); }); diff --git a/__tests__/index-command.test.ts b/__tests__/index-command.test.ts index 47b2aec0..b4b405f8 100644 --- a/__tests__/index-command.test.ts +++ b/__tests__/index-command.test.ts @@ -20,6 +20,7 @@ import * as path from 'path'; import * as os from 'os'; import { CodeGraph } from '../src'; import { DatabaseConnection } from '../src/db'; +import { encodeLockInfo, getDaemonPidPath } from '../src/mcp/daemon-paths'; const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); @@ -49,6 +50,27 @@ function graphCounts(dir: string): { nodes: number; edges: number } { } } +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +function startDetachedProcess(): number { + const source = [ + "const { spawn } = require('child_process');", + "const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { detached: true, stdio: 'ignore' });", + 'child.unref();', + 'console.log(child.pid);', + ].join(' '); + const pid = Number(execFileSync(process.execPath, ['-e', source], { encoding: 'utf8' }).trim()); + if (!Number.isInteger(pid) || pid <= 0) throw new Error('could not start daemon fixture'); + return pid; +} + describe('codegraph index — full re-index keeps the graph populated (#874)', () => { let tempDir: string; @@ -112,6 +134,29 @@ describe('codegraph index — full re-index keeps the graph populated (#874)', ( expect(afterIndex.nodes).toBe(afterInit.nodes); expect(afterIndex.edges).toBe(afterInit.edges); }); + + it('refuses to recreate while a live daemon cannot be verified (#1325)', () => { + runCodegraph(['init'], tempDir); + const daemonPid = startDetachedProcess(); + const lockPath = getDaemonPidPath(fs.realpathSync(tempDir)); + fs.writeFileSync( + lockPath, + encodeLockInfo({ + pid: daemonPid, + version: 'test', + socketPath: path.join(tempDir, '.codegraph', 'missing.sock'), + startedAt: Date.now(), + }), + ); + + try { + expect(() => runCodegraph(['index', '--quiet'], tempDir)).toThrow(/could not verify.*daemon/i); + expect(isAlive(daemonPid)).toBe(true); + expect(fs.existsSync(lockPath)).toBe(true); + } finally { + if (isAlive(daemonPid)) process.kill(daemonPid, 'SIGKILL'); + } + }); }); /** diff --git a/__tests__/index-daemon-rebuild.test.ts b/__tests__/index-daemon-rebuild.test.ts new file mode 100644 index 00000000..0e5e51c9 --- /dev/null +++ b/__tests__/index-daemon-rebuild.test.ts @@ -0,0 +1,121 @@ +import { afterEach, beforeEach, expect, it } from 'vitest'; +import { spawn, type ChildProcessWithoutNullStreams } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { stopDaemonAt, isProcessAlive } from '../src/mcp/daemon-registry'; +import { readWriterLock, releaseWriterLock, tryAcquireWriterLock } from '../src/mcp/writer-lock'; + +const bin = path.resolve(__dirname, '../dist/bin/codegraph.js'); +let root: string; +let children: ChildProcessWithoutNullStreams[]; +const env = { ...process.env, CODEGRAPH_TELEMETRY: '0', DO_NOT_TRACK: '1', + CODEGRAPH_NO_PROMPT_HOOK: '1', CODEGRAPH_NO_DAEMON: '', CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '60000' }; + +async function until(fn: () => T, timeout = 20000): Promise> { + const end = Date.now() + timeout; + while (Date.now() < end) { + const value = fn(); + if (value) return value as NonNullable; + await new Promise(r => setTimeout(r, 25)); + } + throw new Error('Timed out waiting for process/response'); +} +function start(args: string[]) { + const child = spawn(process.execPath, ['--liftoff-only', bin, ...args], { cwd: root, env }); + children.push(child); + let out = '', err = ''; + child.stdout.on('data', b => out += b); + child.stderr.on('data', b => err += b); + child.stdin.on('error', () => {}); + return { child, output: () => out, errors: () => err }; +} +type Server = ReturnType; +function send(server: Server, id: number, method: string, params: unknown = {}) { + server.child.stdin.write(JSON.stringify({ jsonrpc: '2.0', id, method, params }) + '\n'); +} +async function response(server: Server, id: number) { + return until(() => server.output().split('\n').map(l => { + try { return JSON.parse(l); } catch { return null; } + }).find(m => m?.id === id)); +} +async function client() { + const server = start(['serve', '--mcp', '--path', root]); + send(server, 1, 'initialize', { protocolVersion: '2024-11-05', capabilities: {}, + clientInfo: { name: 'rebuild-test', version: '1' } }); + await response(server, 1); + send(server, 2, 'tools/call', { name: 'codegraph_explore', arguments: { query: 'originalSymbol' } }); + expect((await response(server, 2)).result.isError).not.toBe(true); + return server; +} +async function rebuild() { + const cmd = start(['index', '--quiet']); + await until(() => cmd.child.exitCode !== null || cmd.child.signalCode !== null, 60000); + expect(cmd.errors()).not.toContain('Failed to index'); + expect(cmd.child.exitCode).toBe(0); +} +beforeEach(async () => { + root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'cg-rebuild-'))); + children = []; + fs.writeFileSync(path.join(root, 'app.ts'), 'export function originalSymbol() {}\n'); + const cg = await CodeGraph.init(root); + await cg.indexAll(); + cg.close(); +}); +afterEach(async () => { + releaseWriterLock(root, 'rebuild.pid'); + for (const child of children) { + child.stdin.end(); + if (child.exitCode === null && child.signalCode === null) child.kill(); + } + await until(() => children.every(c => c.exitCode !== null || c.signalCode !== null)); + await stopDaemonAt(root); + fs.rmSync(root, { recursive: true, force: true, maxRetries: 20, retryDelay: 100 }); +}); + +it.runIf(process.platform === 'win32')('rebuilds after the real MCP client closes with SQLite held by its daemon (#1325)', async () => { + const server = await client(); + const pid = JSON.parse(fs.readFileSync(path.join(root, '.codegraph/daemon.pid'), 'utf8')).pid; + server.child.stdin.end(); + await until(() => server.child.exitCode !== null); + expect(isProcessAlive(pid)).toBe(true); + await rebuild(); + expect(isProcessAlive(pid)).toBe(false); + const cg = CodeGraph.openSync(root); + try { expect(cg.searchNodes('originalSymbol').length).toBeGreaterThan(0); } + finally { cg.close(); } +}, 90000); + +it('keeps an active client from reopening SQLite during rebuild and serves the new graph afterward', async () => { + const server = await client(); + // Hold the exact CLI rebuild fence to deterministically exercise a request + // arriving between daemon termination and database recreation. + expect(tryAcquireWriterLock(root, 'rebuild', 'rebuild.pid').kind).toBe('acquired'); + expect((await stopDaemonAt(root)).outcome).toMatch(/term|kill/); + await until(() => server.errors().includes('connection lost')); + send(server, 3, 'tools/call', { name: 'codegraph_explore', arguments: { query: 'originalSymbol' } }); + expect((await response(server, 3)).error.message).toContain('rebuild is in progress'); + expect(readWriterLock(root)?.mode).not.toBe('fallback'); + releaseWriterLock(root, 'rebuild.pid'); + fs.writeFileSync(path.join(root, 'app.ts'), 'export function replacementSymbol() {}\n'); + await rebuild(); + send(server, 4, 'tools/call', { name: 'codegraph_explore', arguments: { query: 'replacementSymbol' } }); + const reply = await response(server, 4); + expect(reply.error).toBeUndefined(); + expect(reply.result.isError).not.toBe(true); + expect(JSON.stringify(reply.result)).toContain('replacementSymbol'); +}, 90000); + +it('preserves a foreign writer and the existing database', async () => { + expect(tryAcquireWriterLock(root, 'direct').kind).toBe('acquired'); + const before = fs.readFileSync(path.join(root, '.codegraph/codegraph.db')); + try { + const cmd = start(['index', '--quiet']); + await until(() => cmd.child.exitCode !== null); + expect(cmd.child.exitCode).toBe(1); + expect(cmd.errors()).toContain('writer lock held'); + expect(readWriterLock(root)?.pid).toBe(process.pid); + expect(fs.readFileSync(path.join(root, '.codegraph/codegraph.db'))).toEqual(before); + } finally { releaseWriterLock(root); } +}); diff --git a/__tests__/mcp-projectpath-lifecycle.test.ts b/__tests__/mcp-projectpath-lifecycle.test.ts index 4030aa16..e8b302be 100644 --- a/__tests__/mcp-projectpath-lifecycle.test.ts +++ b/__tests__/mcp-projectpath-lifecycle.test.ts @@ -18,6 +18,9 @@ import CodeGraph from '../src/index'; import { MCPEngine } from '../src/mcp/engine'; import { MAX_CACHED_PROJECTS, __setLoadCodeGraphForTests } from '../src/mcp/tools'; +// Default and read-only opens use the engine's lazy CommonJS loader. +const { MCPEngine: BuiltMCPEngine } = require('../dist/mcp/engine') as typeof import('../src/mcp/engine'); + const opened: CodeGraph[] = []; let onOpen: ((cg: CodeGraph) => void) | undefined; /** CodeGraph that records every instance the ToolHandler opens. */ @@ -145,6 +148,47 @@ describe('MCP explicit projectPath lifecycle (#1835)', () => { return res.content.map((c) => (c.type === 'text' ? c.text : '')).join('\n'); } + for (const options of [{ watch: true }, { watch: false }, { readOnly: true }]) { + it(`defers explicit project opens during a rebuild (${JSON.stringify(options)})`, async () => { + await engine.stop(); + engine = new BuiltMCPEngine(options); + engines.push(engine); + const holder = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'ignore' }); + children.push(holder); + if (!holder.pid) throw new Error('Failed to spawn rebuild holder'); + const fence = path.join(serviceA, '.codegraph/rebuild.pid'); + fs.writeFileSync(fence, JSON.stringify({ pid: holder.pid, mode: 'rebuild', startedAt: Date.now() })); + const response = await engine.getToolHandler().execute('codegraph_search', { + projectPath: serviceA, query: 'alphaOriginal', + }); + expect(JSON.stringify(response)).toContain('rebuild is in progress'); + // Expected and temporary: guidance, never a tool error that teaches abandonment. + expect(JSON.stringify(response)).not.toContain('"isError":true'); + expect(opened).toHaveLength(0); + expect(fs.existsSync(path.join(serviceA, '.codegraph/writer.pid'))).toBe(false); + fs.unlinkSync(fence); + expect(await search(serviceA, 'alphaOriginal')).toContain('alphaOriginal'); + }); + + it(`defers default project opens and synchronous retries during a rebuild (${JSON.stringify(options)})`, async () => { + const reader = new BuiltMCPEngine(options); + engines.push(reader); + const holder = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'ignore' }); + children.push(holder); + if (!holder.pid) throw new Error('Failed to spawn rebuild holder'); + const fence = path.join(serviceA, '.codegraph/rebuild.pid'); + fs.writeFileSync(fence, JSON.stringify({ pid: holder.pid, mode: 'rebuild', startedAt: Date.now() })); + await reader.ensureInitialized(serviceA); + expect(reader.hasDefaultCodeGraph()).toBe(false); + reader.retryInitializeSync(serviceA); + expect(reader.hasDefaultCodeGraph()).toBe(false); + expect(fs.existsSync(path.join(serviceA, '.codegraph/writer.pid'))).toBe(false); + fs.unlinkSync(fence); + reader.retryInitializeSync(serviceA); + expect(reader.hasDefaultCodeGraph()).toBe(true); + }); + } + it('catches up an edit made before the first call and watches later edits', async () => { // Edited while no server owned the index — the catch-up path. fs.writeFileSync(path.join(serviceA, 'src', 'sample.ts'), 'export function alphaRenamed() { return 1; }\n'); diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 82341a12..6664d1c5 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -864,69 +864,91 @@ program process.exit(1); } - const { default: CodeGraph, getDatabasePath } = await loadCodeGraph(); - // `index` is a FULL re-index — identical to a fresh `init`. RECREATE the - // database from scratch (discard .codegraph/codegraph.db + its WAL) rather - // than opening the old graph and DELETE-ing every row. The clear-then-index - // approach reported "0 nodes" without the clear (#874); the recreate keeps - // that fixed AND avoids the failure mode where, on a large or pre-fix - // poisoned index, the per-row FTS delete churn wedged the main thread long - // enough to trip the liveness watchdog before scanning even began (#1067). - // recreate() hands back a fresh, empty instance — no clear() needed. For - // fast incremental updates use `sync`. - const cg = await CodeGraph.recreate(projectPath); - - // Supervise the indexer: self-terminate if orphaned (parent shim killed) - // or if the main thread wedges — neither was guarded on this path (#999). - // The DB + WAL paths let the liveness watchdog tell a slow store on - // degraded storage from a true wedge (#1231). - const dbPath = getDatabasePath(projectPath); - const supervision = installCommandSupervision('index', { progressPaths: [dbPath, `${dbPath}-wal`] }); + const { tryAcquireWriterLock, releaseWriterLock, writerLockHeldMessage } = await import('../mcp/writer-lock'); + const rebuild = tryAcquireWriterLock(projectPath, 'rebuild', 'rebuild.pid'); + if (rebuild.kind === 'taken') throw new Error('Another index rebuild is already in progress.'); try { - if (options.quiet) { - // Quiet mode: no UI, just run against the freshly-recreated graph. - const result = await cg.indexAll(); - if (!result.success) process.exit(1); - cg.destroy(); - return; + // A live MCP daemon keeps SQLite handles open. Verify it by its socket + // before stopping it so a stale pidfile can never signal another process. + const { stopDaemonAt } = await import('../mcp/daemon-registry'); + const daemonStop = await stopDaemonAt(fs.realpathSync(projectPath), { preserveUnverified: true }); + if (daemonStop.outcome === 'unverified' || daemonStop.outcome === 'still-running') { + throw new Error('Could not verify that the active CodeGraph daemon has stopped. Run `codegraph daemon stop` to stop it, then retry `codegraph index`.'); } - const clack = await importESM('@clack/prompts'); - clack.intro('Indexing project'); + // Keep the writer slot through recreation AND indexing. A reconnecting + // proxy/daemon must not open the replacement database halfway through. + const writer = tryAcquireWriterLock(projectPath, 'rebuild'); + if (writer.kind === 'taken') throw new Error(writerLockHeldMessage(writer.existing, writer.pidPath)); + try { + const { default: CodeGraph, getDatabasePath } = await loadCodeGraph(); + // `index` is a FULL re-index — identical to a fresh `init`. RECREATE the + // database from scratch (discard .codegraph/codegraph.db + its WAL) rather + // than opening the old graph and DELETE-ing every row. The clear-then-index + // approach reported "0 nodes" without the clear (#874); the recreate keeps + // that fixed AND avoids the failure mode where, on a large or pre-fix + // poisoned index, the per-row FTS delete churn wedged the main thread long + // enough to trip the liveness watchdog before scanning even began (#1067). + // recreate() hands back a fresh, empty instance — no clear() needed. For + // fast incremental updates use `sync`. + const cg = await CodeGraph.recreate(projectPath); - // A closure so a re-index (after opting gitignored child repos in, #1156) - // renders identically. Supervision already wraps the whole command. - const renderIndex = async (): Promise => { - if (options.verbose) { - return await cg.indexAll({ onProgress: createVerboseProgress(), verbose: true }); + // Supervise the indexer: self-terminate if orphaned (parent shim killed) + // or if the main thread wedges — neither was guarded on this path (#999). + // The DB + WAL paths let the liveness watchdog tell a slow store on + // degraded storage from a true wedge (#1231). + const dbPath = getDatabasePath(projectPath); + const supervision = installCommandSupervision('index', { progressPaths: [dbPath, `${dbPath}-wal`] }); + try { + if (options.quiet) { + // Quiet mode: no UI, just run against the freshly-recreated graph. + const result = await cg.indexAll(); + if (!result.success) process.exit(1); + return; + } + + const clack = await importESM('@clack/prompts'); + clack.intro('Indexing project'); + + // A closure so a re-index (after opting gitignored child repos in, #1156) + // renders identically. Supervision already wraps the whole command. + const renderIndex = async (): Promise => { + if (options.verbose) { + return await cg.indexAll({ onProgress: createVerboseProgress(), verbose: true }); + } + process.stdout.write(`${colors.dim}${getGlyphs().rail}${colors.reset}\n`); + const progress = createShimmerProgress(); + const r = await cg.indexAll({ onProgress: progress.onProgress }); + await progress.stop(); + return r; + }; + + const result = await renderIndex(); + + printIndexResult(clack, result, projectPath); + await recordIndexTelemetry(cg, result); + + // Empty graph at a git super-repo → likely `.gitignore`d child repos; + // name them and offer to opt in instead of a silent 0-node result (#1156). + let finalResult = result; + if (result.nodesCreated === 0) { + finalResult = (await offerIndexIgnoredRepos(clack, projectPath, renderIndex, { interactive: true })) ?? result; + } + + if (!finalResult.success) { + process.exit(1); + } + + clack.outro('Done'); + } finally { + supervision.stop(); + cg.destroy(); } - process.stdout.write(`${colors.dim}${getGlyphs().rail}${colors.reset}\n`); - const progress = createShimmerProgress(); - const r = await cg.indexAll({ onProgress: progress.onProgress }); - await progress.stop(); - return r; - }; - - const result = await renderIndex(); - - printIndexResult(clack, result, projectPath); - await recordIndexTelemetry(cg, result); - - // Empty graph at a git super-repo → likely `.gitignore`d child repos; - // name them and offer to opt in instead of a silent 0-node result (#1156). - let finalResult = result; - if (result.nodesCreated === 0) { - finalResult = (await offerIndexIgnoredRepos(clack, projectPath, renderIndex, { interactive: true })) ?? result; + } finally { + releaseWriterLock(projectPath); } - - if (!finalResult.success) { - process.exit(1); - } - - clack.outro('Done'); - cg.destroy(); } finally { - supervision.stop(); + releaseWriterLock(projectPath, 'rebuild.pid'); } } catch (err) { error(`Failed to index: ${err instanceof Error ? err.message : String(err)}`); diff --git a/src/mcp/daemon-manager.ts b/src/mcp/daemon-manager.ts index 634e633b..dd7f8efe 100644 --- a/src/mcp/daemon-manager.ts +++ b/src/mcp/daemon-manager.ts @@ -109,6 +109,10 @@ export async function runDaemonPicker(deps: PickerDeps): Promise { } const result = await deps.stop(String(choice)); + if (result.outcome === 'still-running') { + deps.note(`Could not stop daemon (pid ${result.pid}); left its artifacts intact — ${choice}`); + continue; + } if (result.outcome === 'unverified') { deps.note( `Could not verify daemon (pid ${result.pid}); left it running with its artifacts intact — ${choice}` diff --git a/src/mcp/daemon-registry.ts b/src/mcp/daemon-registry.ts index 0d3b7cf4..b4a01c30 100644 --- a/src/mcp/daemon-registry.ts +++ b/src/mcp/daemon-registry.ts @@ -218,17 +218,18 @@ async function waitForDeath(pid: number, timeoutMs: number): Promise { export interface StopResult { root: string; pid: number | null; - /** 'term' graceful, 'kill' force, 'not-running' stale, 'no-daemon' absent, 'unverified' preserved. */ - outcome: 'term' | 'kill' | 'not-running' | 'no-daemon' | 'unverified'; + /** 'term' graceful, 'kill' force, 'still-running' refused, 'not-running' stale, 'no-daemon' absent, 'unverified' preserved. */ + outcome: 'term' | 'kill' | 'still-running' | 'not-running' | 'no-daemon' | 'unverified'; } /** * Stop the daemon serving `root`: SIGTERM, wait, then SIGKILL if it won't go, * then sweep its artifacts. `root` must be realpath'd (match how the daemon * keys its socket/lockfile). Resolves the pid from the authoritative lockfile, - * falling back to the registry. + * falling back to the registry. Rebuild callers preserve unverified live locks + * instead of interpreting a failed probe as permission to discard the database. */ -export async function stopDaemonAt(root: string): Promise { +export async function stopDaemonAt(root: string, options: { preserveUnverified?: boolean } = {}): Promise { let pid: number | null = null; let identity: DaemonLockInfo | null = null; let lockContents: string | null = null; @@ -261,17 +262,31 @@ export async function stopDaemonAt(root: string): Promise { return { root, pid, outcome: 'unverified' }; } if (!await probeDaemonIdentity(identity)) { + if (options.preserveUnverified) return { root, pid, outcome: 'unverified' }; const removed = cleanupDaemonArtifacts(root, lockContents); return { root, pid, outcome: removed ? 'not-running' : 'unverified' }; } + // Identity probing awaits I/O: never act on a superseded ownership record. + const sameLock = (): boolean => { + try { return fs.readFileSync(getDaemonPidPath(root), 'utf8') === lockContents; } + catch { return lockContents === null; } + }; + if (!sameLock()) return { root, pid, outcome: 'unverified' }; + // POSIX: SIGTERM runs the daemon's graceful shutdown. Windows: TerminateProcess // (no graceful path), so we always sweep artifacts ourselves below. try { process.kill(pid, 'SIGTERM'); } catch { /* raced to exit */ } let outcome: StopResult['outcome'] = 'term'; if (!(await waitForDeath(pid, 3000))) { + // Re-prove identity before escalating; the old PID may have been reused. + if (!sameLock() || !await probeDaemonIdentity(identity) || !sameLock()) { + return { root, pid, outcome: 'still-running' }; + } try { process.kill(pid, 'SIGKILL'); } catch { /* raced to exit */ } - await waitForDeath(pid, 2000); + if (!(await waitForDeath(pid, 2000))) { + return { root, pid, outcome: 'still-running' }; + } outcome = 'kill'; } cleanupDaemonArtifacts(root, lockContents); diff --git a/src/mcp/daemon.ts b/src/mcp/daemon.ts index 72ed8a2d..d173a573 100644 --- a/src/mcp/daemon.ts +++ b/src/mcp/daemon.ts @@ -58,6 +58,7 @@ import { CodeGraphPackageVersion } from './version'; import { releaseWriterLock, tryAcquireWriterLock, + assertNoRebuild, writerLockHeldMessage, } from './writer-lock'; import { registerDaemon, deregisterDaemon } from './daemon-registry'; @@ -212,6 +213,7 @@ export class Daemon { async start(): Promise { // #1740: claim the project writer lock before opening/watching so a // concurrent direct-mode serve --mcp cannot start a second watcher. + assertNoRebuild(this.projectRoot); const writer = tryAcquireWriterLock(this.projectRoot, 'daemon'); if (writer.kind === 'taken') { const msg = writerLockHeldMessage(writer.existing, writer.pidPath); diff --git a/src/mcp/engine.ts b/src/mcp/engine.ts index bc5e9229..cb6fa4c1 100644 --- a/src/mcp/engine.ts +++ b/src/mcp/engine.ts @@ -15,7 +15,7 @@ import * as path from 'path'; import type CodeGraph from '../index'; import { resolveServerRoot } from '../directory'; import { ToolHandler } from './tools'; -import { releaseWriterLock, tryAcquireWriterLock, writerLockHeldMessage } from './writer-lock'; +import { assertNoRebuild, releaseWriterLock, tryAcquireWriterLock, writerLockHeldMessage } from './writer-lock'; import { QueryPool, resolvePoolSize } from './query-pool'; import { acquireProject, ProjectLease } from './project-lifecycle'; @@ -96,6 +96,9 @@ export class MCPEngine { this.toolHandler = new ToolHandler(null); this.toolHandler.setProjectLifecycle({ open: (root, open) => { + // Explicit projects and read-only fallbacks also hold SQLite handles. + // Fence them before opening, just like the default daemon project. + assertNoRebuild(root); if (this.opts.readOnly) return loadCodeGraph().openSync(root, { readOnly: true }); if (!this.opts.watch) return open(); const lease = acquireProject(root, open, this.watchOptions()); @@ -106,6 +109,7 @@ export class MCPEngine { release: (cg) => this.releaseExplicitProject(cg), }); if (opts.writerLockRoot && !this.opts.readOnly) { + assertNoRebuild(opts.writerLockRoot); const writer = tryAcquireWriterLock(opts.writerLockRoot, 'fallback'); if (writer.kind === 'taken') { throw new Error(writerLockHeldMessage(writer.existing, writer.pidPath)); @@ -225,6 +229,7 @@ export class MCPEngine { try { this.cg.close(); } catch { /* ignore */ } this.cg = null; } + assertNoRebuild(resolvedRoot); this.cg = loadCodeGraph().openSync(resolvedRoot, { readOnly: this.opts.readOnly }); this.projectPath = resolvedRoot; this.toolHandler.setDefaultCodeGraph(this.cg); @@ -348,6 +353,7 @@ export class MCPEngine { this.projectPath = resolvedRoot; try { + assertNoRebuild(resolvedRoot); const opened = await loadCodeGraph().open(resolvedRoot, { readOnly: this.opts.readOnly }); if (this.closed) { opened.close(); return; } this.cg = opened; diff --git a/src/mcp/index.ts b/src/mcp/index.ts index 0a15a310..824132d6 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -51,6 +51,7 @@ import { clearStaleDaemonArtifacts } from './daemon-registry'; import { connectWithHello, runLocalHandshakeProxy } from './proxy'; import { readWriterLock, + assertNoRebuild, releaseWriterLock, tryAcquireWriterLock, writerLockHeldMessage, @@ -106,6 +107,7 @@ function readOnlyFallback(holder: string): MCPEngine { * prove that a process owns the legacy writer slot. */ function makeFallbackEngine(root: string): MCPEngine { + assertNoRebuild(root); let existing: ReturnType = null; try { existing = decodeLockInfo(fs.readFileSync(getDaemonPidPath(root), 'utf8')); @@ -431,6 +433,7 @@ export class MCPServer { // mode multiplexes clients; direct mode is single-writer-per-project. const writerRoot = resolveDaemonRoot(this.projectPath); if (writerRoot) { + assertNoRebuild(writerRoot); const writer = tryAcquireWriterLock(writerRoot, 'direct'); if (writer.kind === 'taken') { const msg = writerLockHeldMessage(writer.existing, writer.pidPath); diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index c6c9fea3..91666fcc 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -2401,8 +2401,11 @@ export class ToolHandler { // Expected condition, not a malfunction: answer as a SUCCESS so the // agent keeps trusting the toolset for projects that ARE indexed. // (An isError here teaches session-long abandonment — see NotIndexedError.) - if (err instanceof NotIndexedError) { - return this.textResult(err.message); + // A running `codegraph index` rebuild is the same kind of expected, + // temporary condition (#1325). Matched by name: tools.ts stays free of + // the writer-lock module on the MCP startup path. + if (err instanceof NotIndexedError || (err as Error | null)?.name === 'RebuildInProgressError') { + return this.textResult((err as Error).message); } // Security refusal: a clean error, no retry encouragement. if (err instanceof PathRefusalError) { diff --git a/src/mcp/writer-lock.ts b/src/mcp/writer-lock.ts index dcf72850..9acbdb9a 100644 --- a/src/mcp/writer-lock.ts +++ b/src/mcp/writer-lock.ts @@ -30,10 +30,10 @@ function isProcessAlive(pid: number): boolean { /** Absolute path to the writer pid lockfile for `projectRoot`. */ -export function getWriterPidPath(projectRoot: string): string { +export function getWriterPidPath(projectRoot: string, lockName: 'writer.pid' | 'rebuild.pid' = 'writer.pid'): string { let root = projectRoot; try { root = fs.realpathSync(projectRoot); } catch { /* keep lexical */ } - return path.join(getCodeGraphDir(root), 'writer.pid'); + return path.join(getCodeGraphDir(root), lockName); } /** Structured contents of the writer pidfile. */ @@ -76,8 +76,9 @@ export function decodeWriterLockInfo(raw: string): WriterLockInfo | null { export function tryAcquireWriterLock( projectRoot: string, mode: string, + lockName: 'writer.pid' | 'rebuild.pid' = 'writer.pid', ): WriterAcquireResult { - const pidPath = getWriterPidPath(projectRoot); + const pidPath = getWriterPidPath(projectRoot, lockName); fs.mkdirSync(path.dirname(pidPath), { recursive: true }); const info: WriterLockInfo = { @@ -165,8 +166,8 @@ export function markWriterReady(projectRoot: string): void { } /** Release if we still own the lock (pid match). */ -export function releaseWriterLock(projectRoot: string): void { - const pidPath = getWriterPidPath(projectRoot); +export function releaseWriterLock(projectRoot: string, lockName: 'writer.pid' | 'rebuild.pid' = 'writer.pid'): void { + const pidPath = getWriterPidPath(projectRoot, lockName); try { if (!fs.existsSync(pidPath)) return; const info = decodeWriterLockInfo(fs.readFileSync(pidPath, 'utf8')); @@ -177,8 +178,8 @@ export function releaseWriterLock(projectRoot: string): void { } /** Read current lock without acquiring. */ -export function readWriterLock(projectRoot: string): WriterLockInfo | null { - const pidPath = getWriterPidPath(projectRoot); +export function readWriterLock(projectRoot: string, lockName: 'writer.pid' | 'rebuild.pid' = 'writer.pid'): WriterLockInfo | null { + const pidPath = getWriterPidPath(projectRoot, lockName); try { return decodeWriterLockInfo(fs.readFileSync(pidPath, 'utf8')); } catch { @@ -204,3 +205,23 @@ export function writerLockHeldMessage( 'If this is stale, delete ' + pidPath ); } + +/** Rebuild intent is separate from writer ownership: acquire BEFORE stopping + * the daemon, so its disconnected proxies cannot reopen SQLite in the gap. */ +/** + * An index rebuild (`codegraph index`) owns the database. Expected and brief, + * so the MCP layer answers it as guidance, never as a tool error (#1325). + */ +export class RebuildInProgressError extends Error { + constructor() { + super('CodeGraph index rebuild is in progress; retry when it finishes.'); + this.name = 'RebuildInProgressError'; + } +} + +export function assertNoRebuild(root: string): void { + const lock = readWriterLock(root, 'rebuild.pid'); + if (lock && lock.pid !== process.pid && isProcessAlive(lock.pid)) { + throw new RebuildInProgressError(); + } +}