From 713a78ce7638df84ee5f2cf3d94f6d0c5c857709 Mon Sep 17 00:00:00 2001 From: stsoul Date: Fri, 31 Jul 2026 12:17:53 +0900 Subject: [PATCH 1/3] fix(watcher): recover after prolonged lock contention --- __tests__/watcher.test.ts | 42 +++++++++++++++++++++---------------- src/mcp/engine.ts | 4 ++-- src/sync/watcher.ts | 44 ++++++++++++++++----------------------- 3 files changed, 44 insertions(+), 46 deletions(-) diff --git a/__tests__/watcher.test.ts b/__tests__/watcher.test.ts index f493a96cc..87e4932d8 100644 --- a/__tests__/watcher.test.ts +++ b/__tests__/watcher.test.ts @@ -264,13 +264,19 @@ describe('FileWatcher', () => { }); }); - describe('lock contention degradation (#876)', () => { - it('disables auto-sync after prolonged lock contention, with bounded retries', async () => { - const syncFn = vi.fn().mockRejectedValue(new LockUnavailableError()); + describe('lock contention recovery (#876)', () => { + it('keeps retrying with capped backoff and recovers after prolonged lock contention', async () => { + let attempts = 0; + const syncFn = vi.fn(async () => { + attempts += 1; + if (attempts <= 8) { + throw new LockUnavailableError(); + } + return { filesChanged: 1, durationMs: 5 }; + }); const onSyncComplete = vi.fn(); const onSyncError = vi.fn(); const onDegraded = vi.fn(); - const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); const watcher = newWatcher(syncFn, { debounceMs: 25, onSyncComplete, @@ -281,22 +287,22 @@ describe('FileWatcher', () => { await watcher.waitUntilReady(); __emitWatchEventForTests(testDir, 'src/long-lock.ts'); - // 5 backoff retries (25·1,2,4,8,16 ms), then degrade on the 6th attempt. - await waitFor(() => !watcher.isActive(), 8000, 20); + // Cross the former five-retry budget and prove the watcher remains live + // with its stale entry retained while the competing writer owns the lock. + await waitFor(() => syncFn.mock.calls.length >= 7, 8000, 20); + expect(watcher.isActive()).toBe(true); + expect(watcher.isDegraded()).toBe(false); + expect(onDegraded).not.toHaveBeenCalled(); + expect(watcher.getPendingFiles().some((p) => p.path === 'src/long-lock.ts')).toBe(true); - expect(syncFn.mock.calls.length).toBeGreaterThanOrEqual(6); // MAX_LOCK_RETRIES + 1 - expect(watcher.isDegraded()).toBe(true); - expect(onDegraded).toHaveBeenCalledTimes(1); - expect(onDegraded).toHaveBeenCalledWith(expect.stringContaining('auto-sync disabled')); - // A held lock is neither a sync error nor a completion. + // The ninth attempt succeeds after eight lock-contention failures. + await waitFor(() => onSyncComplete.mock.calls.length > 0, 8000, 20); + expect(syncFn).toHaveBeenCalledTimes(9); expect(onSyncError).not.toHaveBeenCalled(); - expect(onSyncComplete).not.toHaveBeenCalled(); - // Degrade stops the watcher, which clears pending state. - expect(watcher.getPendingFiles()).toEqual([]); - const disableWarnings = warnSpy.mock.calls.filter( - (c) => typeof c[0] === 'string' && c[0].includes('File watcher disabled') - ); - expect(disableWarnings).toHaveLength(1); + expect(onSyncComplete).toHaveBeenCalledTimes(1); + expect(watcher.getPendingFiles().some((p) => p.path === 'src/long-lock.ts')).toBe(false); + + watcher.stop(); }); it('does NOT degrade on brief contention — backoff resets after a clean sync', async () => { diff --git a/src/mcp/engine.ts b/src/mcp/engine.ts index 9ee132d64..73f107923 100644 --- a/src/mcp/engine.ts +++ b/src/mcp/engine.ts @@ -265,8 +265,8 @@ export class MCPEngine { }, onDegraded: (reason) => { // Live watching gave up permanently (watch-resource exhaustion or a - // write lock held past the retry budget). Say so loudly and ONCE — the - // graph will no longer auto-update, so a long-running MCP session must + // persistent generic sync failure). Say so loudly and ONCE — the graph + // will no longer auto-update, so a long-running MCP session must // not keep assuming it's fresh. The reason already names the remedy // (`codegraph sync` / git sync hooks). process.stderr.write(`[CodeGraph MCP] File watcher degraded — ${reason}\n`); diff --git a/src/sync/watcher.ts b/src/sync/watcher.ts index fed6ea608..1683242be 100644 --- a/src/sync/watcher.ts +++ b/src/sync/watcher.ts @@ -41,11 +41,11 @@ import { isCodeGraphDataDir } from '../directory'; import { watchDisabledReason } from './watch-policy'; /** - * Number of consecutive lock-contention retries the watcher tolerates before - * it gives up and degrades auto-sync. Brief contention (another writer for a - * few cycles) stays under this; a long-lived external writer crosses it. + * Maximum exponential-backoff step for lock contention. A competing writer is + * recoverable no matter how long it holds the lock, so retries continue at the + * capped interval until a clean sync resets the counter. */ -const MAX_LOCK_RETRIES = 5; +const MAX_LOCK_BACKOFF_STEPS = 5; /** * Number of consecutive GENERIC (non-lock) sync failures the watcher tolerates * before it degrades auto-sync. A deterministic failure — a tree-sitter @@ -193,9 +193,9 @@ export interface WatchOptions { /** * Callback fired ONCE when live watching degrades permanently and auto-sync - * is disabled — OS watch-resource exhaustion (EMFILE/ENFILE), a write lock - * held past the retry budget, or a generic sync failure that persists past - * the retry budget (#1127). The string is an actionable, human-readable + * is disabled — OS watch-resource exhaustion (EMFILE/ENFILE), or a generic + * sync failure that persists past the retry budget (#1127). The string is an + * actionable, human-readable * reason. Lets a host (MCP server, daemon, CLI) tell the user that the index * will no longer auto-update instead of silently serving stale results. */ @@ -277,8 +277,8 @@ export class FileWatcher { private inotifyLimitWarned = false; /** * One-way latch: the reason live watching was permanently disabled at runtime - * (watch-resource exhaustion, lock contention past the retry budget, or a - * persistent generic sync failure past the retry budget), or null while + * (watch-resource exhaustion or a persistent generic sync failure past the + * retry budget), or null while * healthy. Set by {@link degrade}; cleared only by a fresh start(). */ private degradedReason: string | null = null; @@ -661,8 +661,8 @@ export class FileWatcher { /** * Permanently disable live watching after a terminal runtime failure - * (watch-resource exhaustion, lock contention past the retry budget, or a - * persistent generic sync failure past the retry budget). + * (watch-resource exhaustion or a persistent generic sync failure past the + * retry budget). * Idempotent: logs one actionable warning, fires {@link WatchOptions.onDegraded} * once, and stops the watcher. A subsequent start() clears the latch. */ @@ -872,24 +872,16 @@ export class FileWatcher { this.onSyncComplete?.(result); } catch (err) { if (err instanceof LockUnavailableError) { - this.lockRetryCount += 1; + this.lockRetryCount = Math.min(this.lockRetryCount + 1, MAX_LOCK_BACKOFF_STEPS); // Lock-failure no-op (another writer holds the lock). pendingFiles - // stays intact and the `finally` block reschedules with backoff. Keep - // brief contention quiet (debug-only — a long external index would - // otherwise spam stderr every cycle), but stop retrying forever: once a - // writer holds the lock past the budget, degrade auto-sync explicitly. + // stays intact and the `finally` block reschedules with bounded + // backoff. Keep contention quiet (debug-only — a long external index + // would otherwise spam stderr every cycle) and keep retrying: a writer + // that eventually releases the lock must not permanently disable sync. logDebug('Watch sync skipped: file lock unavailable', { pendingFiles: this.pendingFiles.size, retryCount: this.lockRetryCount, }); - if (this.lockRetryCount > MAX_LOCK_RETRIES) { - this.degrade( - 'CodeGraph file lock held by another process past the retry budget; ' + - 'auto-sync disabled. Run `codegraph sync` once the other writer finishes ' + - '(or install git sync hooks) to refresh the graph.', - { pendingFiles: this.pendingFiles.size, retryCount: this.lockRetryCount } - ); - } } else { this.lockRetryCount = 0; // a non-lock failure isn't contention; reset that streak this.syncFailureRetryCount += 1; @@ -926,8 +918,8 @@ export class FileWatcher { // generic sync failure — back off exponentially (debounceMs · 2^(n-1), // capped) instead of retrying at the normal debounce cadence; a clean // sync resets both counters so normal edits keep the fast debounce. Use - // the larger streak so interleaved failures still back off. A degrade() - // above already set `stopped`, so this won't reschedule a watcher that + // the larger streak so interleaved failures still back off. A generic- + // failure degrade() above already set `stopped`, so this won't reschedule a watcher that // has given up. if (this.pendingFiles.size > 0 && !this.stopped) { const retryCount = Math.max(this.lockRetryCount, this.syncFailureRetryCount); From 77c7959ffd273192bfee7fe9f69bdf901f51c235 Mon Sep 17 00:00:00 2001 From: "deukkyu.lee" Date: Sun, 2 Aug 2026 15:59:03 +0900 Subject: [PATCH 2/3] fix: serialize CodeGraph database writers --- __tests__/security.test.ts | 24 ++++++++++++ src/bin/codegraph.ts | 60 +++++++++++++++++++++++------- src/db/writer-lease.ts | 19 ++++++++++ src/mcp/engine.ts | 32 ++++++++++++++++ src/utils.ts | 76 +++++++++++++++++++++----------------- 5 files changed, 164 insertions(+), 47 deletions(-) create mode 100644 src/db/writer-lease.ts diff --git a/__tests__/security.test.ts b/__tests__/security.test.ts index 3b3171782..2494ee2c6 100644 --- a/__tests__/security.test.ts +++ b/__tests__/security.test.ts @@ -18,6 +18,7 @@ import { ToolHandler, tools } from '../src/mcp/tools'; import { scanDirectory, isSourceFile } from '../src/extraction'; import { DatabaseConnection, getDatabasePath } from '../src/db'; import { QueryBuilder } from '../src/db/queries'; +import { acquireDatabaseWriterLease } from '../src/db/writer-lease'; function createTempDir(): string { return fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-security-test-')); @@ -66,6 +67,29 @@ describe('FileLock', () => { lock1.release(); }); + it('should never steal an old lock from a live process', () => { + fs.writeFileSync(lockPath, String(process.pid)); + const old = new Date(Date.now() - 10 * 60 * 1000); + fs.utimesSync(lockPath, old, old); + + const lock = new FileLock(lockPath); + expect(() => lock.acquire()).toThrow(/locked by another process/); + expect(fs.readFileSync(lockPath, 'utf8').trim()).toBe(String(process.pid)); + }); + + it('should keep the database writer lease exclusive across operation locks', () => { + const writerLease = acquireDatabaseWriterLease(tempDir); + const operationLock = new FileLock(path.join(tempDir, '.codegraph', 'codegraph.lock')); + operationLock.acquire(); + operationLock.release(); + + expect(() => acquireDatabaseWriterLease(tempDir)).toThrow(/locked by another process/); + writerLease.release(); + + const nextWriter = acquireDatabaseWriterLease(tempDir); + nextWriter.release(); + }); + it('should detect and remove stale locks from dead processes', () => { // Write a lock file with a PID that doesn't exist // PID 99999999 is extremely unlikely to be a real process diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index c6e259374..cee392fd4 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -53,6 +53,8 @@ import { relaunchWithWasmRuntimeFlagsIfNeeded } from '../extraction/wasm-runtime import { installCommandSupervision } from './command-supervision'; import { EXTRACTION_VERSION } from '../extraction/extraction-version'; import { getTelemetry, TELEMETRY_DOCS, recordIndexEvent } from '../telemetry'; +import { FileLock } from '../utils'; +import { acquireDatabaseWriterLease } from '../db/writer-lease'; // Decided once, before `--color`/`--no-color` are stripped from argv below // (#1281). Piped/redirected stdout, NO_COLOR, or --no-color -> plain output. @@ -601,6 +603,7 @@ program .action(async (pathArg: string | undefined, options: { index?: boolean; force?: boolean; verbose?: boolean }) => { const projectPath = path.resolve(pathArg || process.cwd()); const clack = await importESM('@clack/prompts'); + let writerLease: FileLock | null = null; clack.intro('Initializing CodeGraph'); @@ -628,6 +631,7 @@ program return; } + writerLease = acquireDatabaseWriterLease(projectPath); const { default: CodeGraph, getDatabasePath } = await loadCodeGraph(); const cg = await CodeGraph.init(projectPath, { index: false }); clack.log.success(`Initialized in ${projectPath}`); @@ -676,7 +680,9 @@ program cg.destroy(); } catch (err) { clack.log.error(`Failed: ${err instanceof Error ? err.message : String(err)}`); - process.exit(1); + process.exitCode = 1; + } finally { + writerLease?.release(); } }); @@ -689,6 +695,7 @@ program .option('-f, --force', 'Skip confirmation prompt') .action(async (pathArg: string | undefined, options: { force?: boolean }) => { const projectPath = resolveProjectPath(pathArg); + let writerLease: FileLock | null = null; try { if (!isInitialized(projectPath)) { @@ -714,6 +721,7 @@ program } } + writerLease = acquireDatabaseWriterLease(projectPath); const { default: CodeGraph } = await loadCodeGraph(); const cg = CodeGraph.openSync(projectPath); cg.uninitialize(); @@ -737,7 +745,9 @@ program } catch { /* non-fatal */ } } catch (err) { error(`Failed to uninitialize: ${err instanceof Error ? err.message : String(err)}`); - process.exit(1); + process.exitCode = 1; + } finally { + writerLease?.release(); } }); @@ -752,6 +762,7 @@ program .option('-v, --verbose', 'Show detailed worker lifecycle and memory info') .action(async (pathArg: string | undefined, options: { force?: boolean; quiet?: boolean; verbose?: boolean }) => { const projectPath = resolveProjectPath(pathArg); + let writerLease: FileLock | null = null; try { // Don't (re)index your home directory / a filesystem root (#845). --force @@ -759,15 +770,18 @@ program const unsafe = unsafeIndexRootReason(projectPath); if (unsafe && !options.force) { error(`Refusing to index ${projectPath} — it looks like ${unsafe}. Pass --force to override.`); - process.exit(1); + process.exitCode = 1; + return; } if (!isInitialized(projectPath)) { error(`CodeGraph not initialized in ${projectPath}`); info('Run "codegraph init" first'); - process.exit(1); + process.exitCode = 1; + return; } + writerLease = acquireDatabaseWriterLease(projectPath); 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 @@ -790,7 +804,7 @@ program 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); + if (!result.success) process.exitCode = 1; cg.destroy(); return; } @@ -824,7 +838,8 @@ program } if (!finalResult.success) { - process.exit(1); + process.exitCode = 1; + return; } clack.outro('Done'); @@ -834,7 +849,9 @@ program } } catch (err) { error(`Failed to index: ${err instanceof Error ? err.message : String(err)}`); - process.exit(1); + process.exitCode = 1; + } finally { + writerLease?.release(); } }); @@ -847,15 +864,18 @@ program .option('-q, --quiet', 'Suppress output (for git hooks)') .action(async (pathArg: string | undefined, options: { quiet?: boolean }) => { const projectPath = resolveProjectPath(pathArg); + let writerLease: FileLock | null = null; try { if (!isInitialized(projectPath)) { if (!options.quiet) { error(`CodeGraph not initialized in ${projectPath}`); } - process.exit(1); + process.exitCode = 1; + return; } + writerLease = acquireDatabaseWriterLease(projectPath); const { default: CodeGraph } = await loadCodeGraph(); const cg = await CodeGraph.open(projectPath); @@ -896,7 +916,9 @@ program if (!options.quiet) { error(`Failed to sync: ${err instanceof Error ? err.message : String(err)}`); } - process.exit(1); + process.exitCode = 1; + } finally { + writerLease?.release(); } }); @@ -1822,18 +1844,28 @@ program return; } - const lockPath = path.join(getCodeGraphDir(projectPath), 'codegraph.lock'); + const lockPaths = [ + path.join(getCodeGraphDir(projectPath), 'codegraph.lock'), + path.join(getCodeGraphDir(projectPath), 'database-writer.lock'), + ]; + const existingLocks = lockPaths.filter((lockPath) => fs.existsSync(lockPath)); - if (!fs.existsSync(lockPath)) { + if (existingLocks.length === 0) { info(`No lock file found ${getGlyphs().dash} nothing to do`); return; } - fs.unlinkSync(lockPath); - success('Removed lock file. You can now run indexing again.'); + for (const lockPath of existingLocks) { + // FileLock only replaces a dead owner. A live writer is never removed, + // even when the lock is old. + const staleLock = new FileLock(lockPath); + staleLock.acquire(); + staleLock.release(); + } + success(`Removed ${existingLocks.length} stale lock file(s). You can now run indexing again.`); } catch (err) { error(`Failed to remove lock: ${err instanceof Error ? err.message : String(err)}`); - process.exit(1); + process.exitCode = 1; } }); diff --git a/src/db/writer-lease.ts b/src/db/writer-lease.ts new file mode 100644 index 000000000..6651bfcc4 --- /dev/null +++ b/src/db/writer-lease.ts @@ -0,0 +1,19 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import { getCodeGraphDir } from '../directory'; +import { FileLock } from '../utils'; + +/** + * Acquire the process-lifetime writer lease for a project database. + * + * MCP/watch holds this lease until shutdown. Maintenance commands hold it from + * before opening/recreating SQLite until the command completes. The ordinary + * per-operation `codegraph.lock` remains in place as a second line of defense. + */ +export function acquireDatabaseWriterLease(projectRoot: string): FileLock { + const codegraphDir = getCodeGraphDir(path.resolve(projectRoot)); + fs.mkdirSync(codegraphDir, { recursive: true }); + const lease = new FileLock(path.join(codegraphDir, 'database-writer.lock')); + lease.acquire(); + return lease; +} diff --git a/src/mcp/engine.ts b/src/mcp/engine.ts index 73f107923..fa73be5b5 100644 --- a/src/mcp/engine.ts +++ b/src/mcp/engine.ts @@ -16,6 +16,8 @@ import { findNearestCodeGraphRoot } from '../directory'; import { watchDisabledReason } from '../sync'; import { ToolHandler } from './tools'; import { QueryPool, resolvePoolSize } from './query-pool'; +import { FileLock } from '../utils'; +import { acquireDatabaseWriterLease } from '../db/writer-lease'; // Lazy-load the heavy CodeGraph chain (sqlite + query/graph/context layers) OFF // the MCP startup path. It's only needed once a tool actually opens a project — @@ -65,6 +67,10 @@ export class MCPEngine { // Off-loop read-tool pool (daemon mode only). Created lazily once the default // project is open — workers each hold their own WAL read connection. private queryPool: QueryPool | null = null; + // Held for the engine lifetime so watcher writes and maintenance commands + // can never become independent SQLite writers for the same project. + private writerLease: FileLock | null = null; + private writerLeaseRoot: string | null = null; constructor(opts: MCPEngineOptions = {}) { this.opts = { watch: opts.watch ?? true, queryPool: opts.queryPool ?? false }; @@ -161,6 +167,7 @@ export class MCPEngine { const resolvedRoot = findNearestCodeGraphRoot(searchFrom); if (!resolvedRoot) return; try { + this.ensureWriterLease(resolvedRoot); // Close any previously failed instance to avoid leaking resources. if (this.cg) { try { this.cg.close(); } catch { /* ignore */ } @@ -173,6 +180,7 @@ export class MCPEngine { this.catchUpSync(); this.maybeStartPool(resolvedRoot); } catch { + this.releaseWriterLease(); // Still failing — caller will try again on the next tool call. } } @@ -196,6 +204,7 @@ export class MCPEngine { try { this.cg.close(); } catch { /* ignore */ } this.cg = null; } + this.releaseWriterLease(); } private async doInitialize(searchFrom: string): Promise { @@ -210,17 +219,40 @@ export class MCPEngine { this.projectPath = resolvedRoot; try { + this.ensureWriterLease(resolvedRoot); this.cg = await loadCodeGraph().open(resolvedRoot); this.toolHandler.setDefaultCodeGraph(this.cg); this.startWatching(); this.catchUpSync(); this.maybeStartPool(resolvedRoot); } catch (err) { + this.releaseWriterLease(); const msg = err instanceof Error ? err.message : String(err); process.stderr.write(`[CodeGraph MCP] Failed to open project at ${resolvedRoot}: ${msg}\n`); } } + private ensureWriterLease(projectRoot: string): void { + if (this.writerLease) { + if (this.writerLeaseRoot !== projectRoot) { + throw new Error( + `CodeGraph MCP writer lease already belongs to ${this.writerLeaseRoot}; ` + + `refusing to switch to ${projectRoot}` + ); + } + return; + } + this.writerLease = acquireDatabaseWriterLease(projectRoot); + this.writerLeaseRoot = projectRoot; + } + + private releaseWriterLease(): void { + if (!this.writerLease) return; + this.writerLease.release(); + this.writerLease = null; + this.writerLeaseRoot = null; + } + /** * Start file watching on the active CodeGraph instance. Idempotent — the * watcher is per-engine, not per-session, which is why the daemon path diff --git a/src/utils.ts b/src/utils.ts index ba5c13261..40f467008 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -224,9 +224,6 @@ export class FileLock { private lockPath: string; private held = false; - /** Locks older than this are considered stale regardless of PID status */ - private static readonly STALE_TIMEOUT_MS = 2 * 60 * 1000; // 2 minutes - constructor(lockPath: string) { this.lockPath = lockPath; } @@ -235,47 +232,59 @@ export class FileLock { * Acquire the lock. Throws if the lock is held by another live process. */ acquire(): void { - // Check for existing lock - if (fs.existsSync(this.lockPath)) { - try { - const content = fs.readFileSync(this.lockPath, 'utf-8').trim(); - const pid = parseInt(content, 10); - const stat = fs.statSync(this.lockPath); - const lockAge = Date.now() - stat.mtimeMs; + if (this.held) { + throw new Error(`CodeGraph lock is already held by this instance: ${this.lockPath}`); + } - // Treat locks older than the timeout as stale, regardless of PID - if (lockAge < FileLock.STALE_TIMEOUT_MS && !isNaN(pid) && this.isProcessAlive(pid)) { + // Try the atomic create first. This keeps the uncontended path free of a + // check-then-create race and makes the existing-file branch contention-only. + try { + fs.writeFileSync(this.lockPath, String(process.pid), { flag: 'wx' }); + this.held = true; + return; + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err; + } + + // A live writer owns the lock for as long as its operation takes. The old + // two-minute timeout let an MCP watcher steal a healthy full-index lock, + // which reintroduced concurrent SQLite writers on large repositories. + for (let attempt = 0; attempt < 3; attempt++) { + try { + const observed = fs.readFileSync(this.lockPath, 'utf-8').trim(); + const pid = parseInt(observed, 10); + if (isNaN(pid) || pid <= 0) { + throw new Error( + `CodeGraph lock record is malformed. ` + + `Run 'codegraph unlock' after verifying no CodeGraph writer is active: ${this.lockPath}` + ); + } + if (this.isProcessAlive(pid)) { throw new Error( `CodeGraph database is locked by another process (PID ${pid}). ` + `If this is stale, run 'codegraph unlock' or delete ${this.lockPath}` ); } - // Stale lock (dead process or timed out) - remove it + // Compare immediately before delete. If another writer replaced the + // record after our liveness check, retry instead of deleting its lock. + if (fs.readFileSync(this.lockPath, 'utf-8').trim() !== observed) continue; fs.unlinkSync(this.lockPath); - } catch (err) { - if (err instanceof Error && err.message.includes('locked by another')) { - throw err; - } - // Other errors reading lock file - try to remove it - try { fs.unlinkSync(this.lockPath); } catch { /* ignore */ } + } catch (err: unknown) { + const code = (err as NodeJS.ErrnoException).code; + if (code !== 'ENOENT') throw err; } - } - // Write our PID to the lock file using exclusive create flag - try { - fs.writeFileSync(this.lockPath, String(process.pid), { flag: 'wx' }); - this.held = true; - } catch (err: any) { - if (err.code === 'EEXIST') { - // Race condition: another process grabbed the lock between our check and write - throw new Error( - 'CodeGraph database is locked by another process. ' + - `If this is stale, run 'codegraph unlock' or delete ${this.lockPath}` - ); + try { + fs.writeFileSync(this.lockPath, String(process.pid), { flag: 'wx' }); + this.held = true; + return; + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err; } - throw err; } + + throw new Error(`CodeGraph database lock changed repeatedly while acquiring: ${this.lockPath}`); } /** @@ -326,7 +335,8 @@ export class FileLock { try { process.kill(pid, 0); return true; - } catch { + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'EPERM') return true; return false; } } From 373f1f38a67c3ceb16ddae43420897a9a52389c5 Mon Sep 17 00:00:00 2001 From: "deukkyu.lee" Date: Sun, 2 Aug 2026 17:57:15 +0900 Subject: [PATCH 3/3] test: await Windows MCP daemon teardown --- __tests__/mcp-daemon.test.ts | 67 +++++++++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 4 deletions(-) diff --git a/__tests__/mcp-daemon.test.ts b/__tests__/mcp-daemon.test.ts index ab7613664..844800aea 100644 --- a/__tests__/mcp-daemon.test.ts +++ b/__tests__/mcp-daemon.test.ts @@ -163,7 +163,9 @@ function countListeningLines(root: string): number { function killTree(...procs: ChildProcessWithoutNullStreams[]): void { for (const p of procs) { - if (!p.killed) { try { p.kill('SIGKILL'); } catch { /* gone */ } } + if (p.exitCode === null && p.signalCode === null) { + try { p.kill('SIGKILL'); } catch { /* gone */ } + } } } @@ -171,6 +173,46 @@ async function waitProcessExit(pid: number, timeoutMs: number): Promise return waitFor(() => !isAlive(pid), timeoutMs).then(() => true).catch(() => false); } +function waitChildClose( + child: ChildProcessWithoutNullStreams, + timeoutMs: number, +): Promise { + if ((child.exitCode !== null || child.signalCode !== null) + && child.stdin.destroyed && child.stdout.destroyed && child.stderr.destroyed) { + return Promise.resolve(true); + } + return new Promise((resolve) => { + let settled = false; + let timer: NodeJS.Timeout; + const finish = (closed: boolean) => { + if (settled) return; + settled = true; + clearTimeout(timer); + child.removeListener('close', onClose); + resolve(closed); + }; + const onClose = () => finish(true); + child.once('close', onClose); + timer = setTimeout(() => finish(false), timeoutMs); + }); +} + +async function removeTempDirWhenReleased(dir: string, timeoutMs: number): Promise { + const started = Date.now(); + let lastError: NodeJS.ErrnoException | null = null; + do { + try { + fs.rmSync(dir, { recursive: true, force: true }); + return; + } catch (error) { + lastError = error as NodeJS.ErrnoException; + if (!['EBUSY', 'EPERM', 'ENOTEMPTY'].includes(lastError.code ?? '')) throw error; + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } while (Date.now() - started <= timeoutMs); + throw lastError ?? new Error(`Timed out removing ${dir}`); +} + describe('Shared MCP daemon (issue #411)', () => { let tempDir: string; // the (possibly symlinked) path processes are spawned with let realRoot: string; // its canonical form — what the daemon keys paths on @@ -184,18 +226,35 @@ describe('Shared MCP daemon (issue #411)', () => { }); afterEach(async () => { + // Register close listeners before kill so already-fast Windows exits cannot + // race past the event that proves child stdio/process handles were released. + const proxyCloseWaits = servers.map((server) => waitChildClose(server.child, 5000)); + // Capture the detached daemon before killing proxies: some shutdown + // paths remove the pidfile while Windows still owns process/CWD handles. + const daemonPid = readLockPid(realRoot); killTree(...servers.map((s) => s.child)); + const proxyClosed = await Promise.all(proxyCloseWaits); + const failedProxyIndex = proxyClosed.findIndex((closed) => !closed); // The daemon is detached (not a tracked child) — reap it explicitly via the // pid it recorded, so a test can't leak a background daemon. Guard against // our own pid: the version-mismatch test plants `pid: process.pid` in the // lockfile, and we must never SIGKILL the vitest worker. - const daemonPid = readLockPid(realRoot); if (daemonPid && daemonPid !== process.pid && isAlive(daemonPid)) { try { process.kill(daemonPid, 'SIGKILL'); } catch { /* race */ } + if (!(await waitProcessExit(daemonPid, 5000))) { + throw new Error(`Detached daemon ${daemonPid} did not exit during teardown`); + } } - await new Promise((r) => setTimeout(r, 50)); + const failedProxyPid = failedProxyIndex === -1 + ? null + : servers[failedProxyIndex].child.pid ?? 'unknown'; servers.length = 0; - fs.rmSync(tempDir, { recursive: true, force: true }); + if (failedProxyIndex !== -1) { + throw new Error(`Proxy process ${failedProxyPid} did not close during teardown`); + } + // A detached daemon is not a ChildProcess we can await for `close`. Retry + // only transient Windows directory-handle races after its PID is gone. + await removeTempDirWhenReleased(tempDir, 5000); }); it('two invocations share ONE detached daemon; both attach as proxies', async () => {