diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 3a8f0345ec..46a3cc3d54 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -8593,6 +8593,29 @@ describe('Nested non-submodule git repos', () => { expect(ig.ignores('scratch/tmp.ts')).toBe(true); }); + it('filesystem fallback retains git info/exclude and core.excludesFile when ls-files fails (#1959)', async () => { + const { execFileSync } = await import('child_process'); + const root = path.join(tempDir, 'fallback-excludes-root'); + fs.mkdirSync(root, { recursive: true }); + execFileSync('git', ['init', '-q'], { cwd: root, stdio: 'pipe' }); + const globalExcludes = path.join(tempDir, 'fallback-global-excludes'); + fs.writeFileSync(globalExcludes, 'scratch/\n'); + execFileSync('git', ['config', 'core.excludesFile', globalExcludes], { cwd: root, stdio: 'pipe' }); + fs.writeFileSync(path.join(root, '.git', 'info', 'exclude'), 'worktrees/\n'); + fs.mkdirSync(path.join(root, 'scratch')); + fs.mkdirSync(path.join(root, 'worktrees')); + fs.writeFileSync(path.join(root, 'app.ts'), 'export const app = 1;\n'); + fs.writeFileSync(path.join(root, 'scratch', 'hidden.ts'), 'export const hidden = 1;\n'); + fs.writeFileSync(path.join(root, 'worktrees', 'hidden.ts'), 'export const hidden = 2;\n'); + + // rev-parse/config still work, but both ls-files and status fail as they + // would under a Git timeout. This exercises the real filesystem walk. + fs.writeFileSync(path.join(root, '.git', 'index'), 'not a git index'); + expect(() => execFileSync('git', ['ls-files'], { cwd: root, stdio: 'pipe' })).toThrow(); + expect(scanDirectory(root)).toEqual(['app.ts']); + expect(await scanDirectoryAsync(root)).toEqual(['app.ts']); + }); + it('buildScopeIgnore prunes dirs ignored only by a nested .gitignore (#1728)', async () => { const { execFileSync } = await import('child_process'); const git = (cwd: string, ...args: string[]) => diff --git a/__tests__/mcp-freshness-bounds.test.ts b/__tests__/mcp-freshness-bounds.test.ts new file mode 100644 index 0000000000..5710b4e137 --- /dev/null +++ b/__tests__/mcp-freshness-bounds.test.ts @@ -0,0 +1,91 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { createHash } from 'crypto'; +import { Readable } from 'stream'; +import { validateAnswerFiles } from '../src/mcp/answer-freshness'; +import { measurePendingChanges } from '../src/mcp/index-freshness'; + +vi.mock('fs', async importOriginal => { + const actual = await importOriginal(); + return { ...actual, createReadStream: vi.fn(actual.createReadStream) }; +}); + +const workers = vi.hoisted(() => [] as any[]); +vi.mock('worker_threads', async () => { + const { EventEmitter } = await import('events'); + return { Worker: class extends EventEmitter { + terminate = vi.fn(async () => 0); + constructor() { super(); workers.push(this); } + } }; +}); + +describe('bounded freshness validation (#1959)', () => { + let root: string | undefined; + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + workers.length = 0; + if (root) fs.rmSync(root, { recursive: true, force: true }); + root = undefined; + }); + + it('marks a file beyond the byte budget unchecked instead of fresh', async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-freshness-bounds-')); + const content = 'x'.repeat(9 * 1024 * 1024); + fs.writeFileSync(path.join(root, 'large.ts'), content); + const result = await validateAnswerFiles(root, [{ + path: 'large.ts', contentHash: createHash('sha256').update(content).digest('hex'), + }]); + expect(result).toEqual({ stale: [], unchecked: ['large.ts'] }); + }); + + it('bounds a stuck measurement, shares concurrent probes, and terminates its worker', async () => { + vi.useFakeTimers(); + const pending = measurePendingChanges('/freshness-timeout'); + expect(measurePendingChanges('/freshness-timeout')).toBe(pending); + expect(workers).toHaveLength(1); + await vi.advanceTimersByTimeAsync(8000); + expect(await pending).toBeNull(); + expect(workers[0].terminate).toHaveBeenCalledOnce(); + }); + + it('returns unknown on deadline even if worker termination is delayed', async () => { + vi.useFakeTimers(); + const pending = measurePendingChanges('/freshness-delayed-exit'); + let release!: () => void; + workers[0].terminate.mockImplementation(() => new Promise(resolve => { release = resolve; })); + await vi.advanceTimersByTimeAsync(8000); + expect(await pending).toBeNull(); + release(); + await Promise.resolve(); + }); + + it('aborts stalled file reads and reports all remaining paths as unchecked', async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-freshness-deadline-')); + fs.writeFileSync(path.join(root, 'stalled.ts'), 'export const x = 1;'); + vi.mocked(fs.createReadStream).mockImplementationOnce((_file, options) => new Readable({ + read() {}, signal: (options as { signal: AbortSignal }).signal, + }) as fs.ReadStream); + const result = await validateAnswerFiles(root, [ + { path: 'stalled.ts', contentHash: 'hash' }, + { path: 'next.ts', contentHash: 'hash' }, + ]); + expect(result).toEqual({ stale: [], unchecked: ['stalled.ts', 'next.ts'] }); + }); + + it('caps measurements across projects and releases the slots on failure', async () => { + const first = measurePendingChanges('/freshness-one'); + const second = measurePendingChanges('/freshness-two'); + expect(await measurePendingChanges('/freshness-three')).toBeNull(); + expect(workers).toHaveLength(2); + workers[0].emit('error', new Error('worker failed')); + workers[1].emit('message', { added: -1, modified: 0, removed: 0 }); + expect(await first).toBeNull(); + expect(await second).toBeNull(); + const retry = measurePendingChanges('/freshness-three'); + workers[2].emit('message', { added: 0, modified: 1, removed: 0 }); + expect(await retry).toEqual({ added: 0, modified: 1, removed: 0 }); + }); +}); diff --git a/__tests__/mcp-stale-refusal.test.ts b/__tests__/mcp-stale-refusal.test.ts new file mode 100644 index 0000000000..53362c7a29 --- /dev/null +++ b/__tests__/mcp-stale-refusal.test.ts @@ -0,0 +1,154 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { ExploreSessionState } from '../src/mcp/explore-session-state'; +import { ToolHandler } from '../src/mcp/tools'; +import { QueryPool } from '../src/mcp/query-pool'; +import { Worker } from 'worker_threads'; +import { __setFsWatchForTests } from '../src/sync/watcher'; + +describe('a degraded index refuses answers from changed files (#1959)', () => { + let root: string; + let cg: CodeGraph; + let handler: ToolHandler; + let pool: QueryPool | undefined; + + beforeEach(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-stale-refusal-')); + fs.mkdirSync(path.join(root, 'folder with spaces'), { recursive: true }); + fs.writeFileSync(path.join(root, 'folder with spaces/目录 🦀.ts'), 'export function unicodeSymbol() { return 1; }\n'); + fs.writeFileSync(path.join(root, 'alpha.ts'), 'export function alphaOnly() { return 1; }\n'); + fs.writeFileSync(path.join(root, 'beta.ts'), 'export function betaOnly() { return 2; }\n'); + fs.writeFileSync( + path.join(root, 'gamma.ts'), + "import { alphaOnly } from './alpha';\nexport function gammaUses() { return alphaOnly(); }\n" + ); + cg = CodeGraph.initSync(root); + await cg.indexAll(); + handler = new ToolHandler(cg); + + __setFsWatchForTests(() => { + const err = new Error('too many open files') as NodeJS.ErrnoException; + err.code = 'EMFILE'; + throw err; + }); + expect(cg.watch()).toBe(false); + expect(cg.isWatcherDegraded()).toBe(true); + __setFsWatchForTests(null); + }); + + afterEach(async () => { + await pool?.destroy(); + pool = undefined; + await handler.closeAll(); + __setFsWatchForTests(null); + try { cg.unwatch(); } catch { /* ignore */ } + try { cg.close(); } catch { /* ignore */ } + fs.rmSync(root, { recursive: true, force: true }); + }); + + it('names a changed file without serving its result, but keeps unaffected source available', async () => { + const alphaPath = path.join(root, 'alpha.ts'); + const primed = await handler.execute('codegraph_explore', { query: 'alphaOnly' }); + expect(primed.content[0].text).toContain('export function alphaOnly'); + const before = fs.statSync(alphaPath); + fs.writeFileSync(alphaPath, 'export function alphaOnly() { return 9; }\n'); + // Identical size and indexed mtime: the last-mile guard must hash bytes, + // not trust metadata or a prior two-second drift-cache verdict. + fs.utimesSync(alphaPath, before.atime, before.mtime); + + const session = new ExploreSessionState(); + const refused = await handler.execute('codegraph_explore', { query: 'alphaOnly' }, session); + expect(refused.isError).toBeFalsy(); + expect(refused.content[0].text).toContain('alpha.ts'); + expect(refused.content[0].text).toContain('cannot answer from this index'); + expect(refused.content[0].text).not.toContain('export function alphaOnly'); + expect(session.view().projects).toEqual([]); + + const unaffected = await handler.execute('codegraph_explore', { query: 'betaOnly' }, session); + expect(unaffected.content[0].text).toContain('export function betaOnly'); + expect(unaffected.content[0].text).toContain('auto-sync is DISABLED'); + + // Let the ordinary sync see a definite metadata change, then ensure the + // same session receives the source it was not shown before. + fs.utimesSync(alphaPath, before.atime, new Date(before.mtimeMs + 2000)); + await cg.sync(); + const refreshed = await handler.execute('codegraph_explore', { query: 'alphaOnly' }, session); + expect(refreshed.content[0].text).toContain('export function alphaOnly'); + expect(refreshed.content[0].text).toContain('return 9'); + expect(refreshed.content[0].text).not.toContain('cannot answer from this index'); + }); + + it('refuses a graph answer that names a changed file, and serves one that does not', async () => { + const callers = await handler.execute('codegraph_callers', { symbol: 'alphaOnly' }); + expect(callers.content[0].text).toContain('gammaUses'); + + fs.writeFileSync( + path.join(root, 'gamma.ts'), + "import { alphaOnly } from './alpha';\nexport function gammaUses() { return 0; }\n" + ); + + const refused = await handler.execute('codegraph_callers', { symbol: 'alphaOnly' }); + expect(refused.isError).toBeFalsy(); + expect(refused.content[0].text).toContain('cannot answer from this index'); + expect(refused.content[0].text).toContain('- gamma.ts'); + expect(refused.content[0].text).not.toContain('gammaUses'); + + const search = await handler.execute('codegraph_search', { query: 'gammaUses' }); + expect(search.content[0].text).toContain('cannot answer from this index'); + + const unaffected = await handler.execute('codegraph_search', { query: 'betaOnly' }); + expect(unaffected.content[0].text).toContain('beta.ts'); + expect(unaffected.content[0].text).not.toContain('cannot answer from this index'); + }); + it.each([false, true])('preserves spaces and Unicode through provenance (pooled=%s)', async pooled => { + if (pooled) { + pool = new QueryPool({ root, size: 1, createWorker: () => new Worker( + path.resolve(__dirname, '../dist/mcp/query-worker.js'), { workerData: { root } }, + ) }); + handler.setQueryPool(pool); + await vi.waitFor(() => expect(pool!.ready).toBe(true), { timeout: 15000 }); + } + const relative = 'folder with spaces/目录 🦀.ts'; + const initial = await handler.execute('codegraph_search', { query: 'unicodeSymbol' }); + expect(initial.content[0].text).toContain(relative); + expect(initial).not.toHaveProperty('_cgAnswerFiles'); + const before = fs.statSync(path.join(root, relative)); + fs.writeFileSync(path.join(root, relative), 'export function unicodeSymbol() { return 9; }\n'); + fs.utimesSync(path.join(root, relative), before.atime, before.mtime); + const result = await handler.execute('codegraph_search', { query: 'unicodeSymbol' }); + expect(result.isError).toBeFalsy(); + expect(result.content[0].text).toContain('cannot answer from this index'); + expect(result.structuredContent).toEqual({ freshness: { stale: [relative], unchecked: [] } }); + expect(result).not.toHaveProperty('_cgAnswerFiles'); + }); + + it.each(['codegraph_search', 'codegraph_explore', 'codegraph_callers', 'codegraph_callees', 'codegraph_impact'])( + 'refuses deleted contributing files in %s', async tool => { + fs.unlinkSync(path.join(root, 'alpha.ts')); + const result = await handler.execute(tool, { query: 'alphaOnly', symbol: 'alphaOnly' }); + expect(result.isError).toBeFalsy(); + expect(result.content[0].text).toContain('cannot answer from this index'); + expect(result.content[0].text).toContain('alpha.ts'); + expect(result).not.toHaveProperty('_cgExploreEmission'); + }, + ); + + it('refuses rather than silently validating only the first 200 contributing files', async () => { + for (let i = 0; i < 201; i++) { + fs.writeFileSync(path.join(root, `caller${i}.ts`), + `import { alphaOnly } from './alpha'; export function caller${i}() { return alphaOnly(); }\n`); + } + await cg.indexAll(); + const raw = await handler.executeReadTool('codegraph_impact', { symbol: 'alphaOnly' }); + expect(raw._cgAnswerFiles!.length).toBeGreaterThan(200); + const result = await handler.execute('codegraph_impact', { symbol: 'alphaOnly' }); + expect(result.isError).toBeFalsy(); + expect(result.content[0].text).toContain('validation budget'); + expect((result.structuredContent!.freshness as { unchecked: string[] }).unchecked.length).toBeGreaterThan(0); + expect(result.content[0].text).not.toContain('**Impact'); + }); + +}); diff --git a/__tests__/mcp-staleness-banner.test.ts b/__tests__/mcp-staleness-banner.test.ts index f72f456e03..fb72cdf5e8 100644 --- a/__tests__/mcp-staleness-banner.test.ts +++ b/__tests__/mcp-staleness-banner.test.ts @@ -20,7 +20,7 @@ * left untouched. */ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; @@ -72,6 +72,7 @@ describe('MCP staleness banner', () => { afterEach(() => { __setFsWatchForTests(null); // reset the injected fs.watch seam + vi.restoreAllMocks(); try { cg.unwatch(); } catch { /* ignore */ } try { cg.close(); } catch { /* ignore */ } if (fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true }); @@ -209,6 +210,26 @@ describe('MCP staleness banner', () => { // status renders the notice inline, so the auto-banner is not also prepended. expect(text.startsWith('⚠️')).toBe(false); }); + + it('distinguishes a re-armed but not-yet-caught-up watcher from a disabled one (#1959)', async () => { + vi.spyOn(cg, 'isWatcherDegraded').mockReturnValue(true); + vi.spyOn(cg, 'isWatcherRecovering').mockReturnValue(true); + + const search = await handler.execute('codegraph_search', { query: 'alphaOnly' }); + expect(search.content[0].text).toMatch(/auto-sync is RECOVERING/); + expect(search.content[0].text).not.toMatch(/auto-sync is DISABLED/); + + const status = await handler.execute('codegraph_status', {}); + expect(status.content[0].text).toContain('**Auto-sync recovering:**'); + expect(status.content[0].text).not.toContain('**Auto-sync disabled:**'); + }); + + it('asks the owned watcher to re-arm on the next MCP tool call (#1959)', async () => { + const rearm = vi.spyOn(cg, 'rearmWatcherAfterLockContention').mockReturnValue(false); + + await handler.execute('codegraph_status', {}); + expect(rearm).toHaveBeenCalledTimes(1); + }); }); describe('MCP staleness banner — matching whole paths (#1968)', () => { diff --git a/__tests__/mcp-status-freshness.test.ts b/__tests__/mcp-status-freshness.test.ts new file mode 100644 index 0000000000..19c14f7ce1 --- /dev/null +++ b/__tests__/mcp-status-freshness.test.ts @@ -0,0 +1,64 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { execFileSync } from 'child_process'; +import CodeGraph from '../src/index'; +import { measurePendingChanges } from '../src/mcp/index-freshness'; +import { ToolHandler } from '../src/mcp/tools'; + +describe('MCP status freshness (#1959)', () => { + let root: string; + let cg: CodeGraph; + let handler: ToolHandler; + + beforeEach(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-status-freshness-')); + fs.writeFileSync(path.join(root, 'modify.ts'), 'export const modify = 1;\n'); + fs.writeFileSync(path.join(root, 'remove.ts'), 'export const remove = 1;\n'); + const git = (...args: string[]) => execFileSync('git', args, { cwd: root, stdio: 'pipe' }); + git('init', '-q'); + git('config', 'user.name', 'CodeGraph Test'); + git('config', 'user.email', 'codegraph-test@example.invalid'); + git('add', 'modify.ts', 'remove.ts'); + git('commit', '-qm', 'baseline'); + cg = CodeGraph.initSync(root); + await cg.indexAll(); + handler = new ToolHandler(cg); + }); + + afterEach(() => { + try { cg.close(); } catch { /* ignore */ } + fs.rmSync(root, { recursive: true, force: true }); + }); + + it('reports the latest indexed file and exact change counts', async () => { + const initial = (await handler.execute('codegraph_status', {})).content[0].text; + expect(initial).toMatch(/\*\*Latest file indexed:\*\* \d{4}-\d\d-\d\dT/); + expect(initial).toContain('**Changes since index:** 0 added, 0 modified, 0 removed'); + + fs.writeFileSync(path.join(root, 'modify.ts'), 'export const modify = 42;\n'); + fs.unlinkSync(path.join(root, 'remove.ts')); + fs.writeFileSync(path.join(root, 'add.ts'), 'export const added = 1;\n'); + + const result = await handler.execute('codegraph_status', {}); + expect(result.structuredContent).toEqual({ freshness: { + lastIndexedAt: cg.getLastIndexedAt(), changes: { added: 1, modified: 1, removed: 1 }, complete: true, + } }); + const changed = result.content[0].text; + expect(changed).toContain('**Changes since index:** 1 added, 1 modified, 1 removed'); + }); + + it('returns unknown rather than a false zero when the measurement cannot open an index', async () => { + expect(await measurePendingChanges(path.join(root, 'missing'))).toBeNull(); + }); + + it('counts edits committed after the index even when the working tree is clean', async () => { + fs.writeFileSync(path.join(root, 'modify.ts'), 'export const modify = 99;\n'); + execFileSync('git', ['add', 'modify.ts'], { cwd: root, stdio: 'pipe' }); + execFileSync('git', ['commit', '-qm', 'changed'], { cwd: root, stdio: 'pipe' }); + + const status = (await handler.execute('codegraph_status', {})).content[0].text; + expect(status).toContain('**Changes since index:** 0 added, 1 modified, 0 removed'); + }); +}); diff --git a/__tests__/security.test.ts b/__tests__/security.test.ts index 9aa3c9597c..8cc9d7aa12 100644 --- a/__tests__/security.test.ts +++ b/__tests__/security.test.ts @@ -66,6 +66,23 @@ describe('FileLock', () => { lock1.release(); }); + it('keeps a live holder protected after its lock file is older than two minutes (#1959)', () => { + const holder = new FileLock(lockPath); + const contender = new FileLock(lockPath); + holder.acquire(); + const old = new Date(Date.now() - 3 * 60_000); + fs.utimesSync(lockPath, old, old); + + try { + expect(() => contender.acquire()).toThrow(/locked by another process/); + expect(fs.readFileSync(lockPath, 'utf8')).toBe(String(process.pid)); + } finally { + holder.release(); + contender.release(); + } + expect(fs.existsSync(lockPath)).toBe(false); + }); + 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 @@ -408,6 +425,7 @@ describe('MCP Input Validation', () => { })); const fakeCg = { searchNodes: () => many, + getFile: () => undefined, // Search down-ranks generated files, and since #1500 that verdict comes // from the index (path convention ∪ content banner) rather than the // filename alone. No database here — none of these paths is generated. diff --git a/__tests__/watcher.test.ts b/__tests__/watcher.test.ts index b2285e45ad..6af1467a8f 100644 --- a/__tests__/watcher.test.ts +++ b/__tests__/watcher.test.ts @@ -344,6 +344,53 @@ describe('FileWatcher', () => { watcher.stop(); }); + + it('re-arms after lock contention and keeps the stale banner until a full catch-up (#1959)', async () => { + let finishCatchUp!: () => void; + const catchUp = new Promise(resolve => { finishCatchUp = resolve; }); + const syncFn = vi.fn().mockRejectedValue(new LockUnavailableError()); + const watcher = newWatcher(syncFn, { debounceMs: 25 }); + watcher.start(); + try { + await watcher.waitUntilReady(); + __emitWatchEventForTests(testDir, 'src/locked.ts'); + await waitFor(() => watcher.isDegraded() && !watcher.isActive(), 8000); + + syncFn.mockImplementation(async () => { + await catchUp; + return { filesChanged: 1, durationMs: 5 }; + }); + expect(watcher.rearmAfterLockContention()).toBe(true); + expect(watcher.isActive()).toBe(true); + expect(watcher.isDegraded()).toBe(true); + expect(watcher.rearmAfterLockContention()).toBe(false); + await waitFor(() => syncFn.mock.calls.length >= 7, 4000); + expect(syncFn.mock.calls.at(-1)?.[0]).toBeUndefined(); + expect(watcher.isDegraded()).toBe(true); + finishCatchUp(); + await waitFor(() => !watcher.isDegraded(), 4000); + expect(watcher.isActive()).toBe(true); + } finally { + finishCatchUp?.(); + watcher.stop(); + } + }); + + it('throttles a failed re-arm across tool calls (#1959)', async () => { + const syncFn = vi.fn().mockRejectedValue(new LockUnavailableError()); + const watcher = newWatcher(syncFn, { debounceMs: 25 }); + watcher.start(); + try { + await watcher.waitUntilReady(); + __emitWatchEventForTests(testDir, 'src/locked.ts'); + await waitFor(() => watcher.isDegraded() && !watcher.isActive(), 8000); + expect(watcher.rearmAfterLockContention()).toBe(true); + await waitFor(() => watcher.isDegraded() && !watcher.isActive(), 8000); + expect(watcher.rearmAfterLockContention()).toBe(false); + } finally { + watcher.stop(); + } + }); }); describe('persistent sync-failure degradation (#1127)', () => { @@ -371,6 +418,7 @@ describe('FileWatcher', () => { expect(syncFn.mock.calls.length).toBeGreaterThanOrEqual(6); // MAX_SYNC_FAILURE_RETRIES + 1 expect(watcher.isDegraded()).toBe(true); + expect(watcher.rearmAfterLockContention()).toBe(false); expect(onDegraded).toHaveBeenCalledTimes(1); expect(onDegraded).toHaveBeenCalledWith(expect.stringContaining('auto-sync disabled')); // The degrade reason carries the underlying error so the user can act. diff --git a/src/index.ts b/src/index.ts index 1a7c5814c0..62f0a84e82 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1144,12 +1144,11 @@ export class CodeGraph { } /** - * True once live watching has permanently degraded (OS watch-resource - * exhaustion, or a write lock held past the retry budget) and auto-sync is - * disabled until the next {@link watch} call. Distinct from `!isWatching()`: - * a stopped/never-started watcher is inactive but NOT degraded. MCP tools use - * this to surface a whole-index "results may be stale" notice, since - * `getPendingFiles()` goes empty once watching stops (#876). + * True once live watching has degraded, or while a re-armed watcher has not + * completed its full catch-up. Distinct from `!isWatching()`: a stopped or + * never-started watcher is inactive but NOT degraded. MCP tools use this for + * a whole-index stale notice, since pending files are lost when watching + * stops (#876, #1959). */ isWatcherDegraded(): boolean { return this.watcher?.isDegraded() ?? false; @@ -1160,6 +1159,16 @@ export class CodeGraph { return this.watcher?.getDegradedReason() ?? null; } + /** Re-arm a lock-degraded watcher; its stale state persists until a full sync. */ + rearmWatcherAfterLockContention(): boolean { + return this.watcher?.rearmAfterLockContention() ?? false; + } + + /** True while a re-armed watcher owes a full reconcile of missed changes. */ + isWatcherRecovering(): boolean { + return this.watcher?.isRecoveringFromLock() ?? false; + } + /** * Files seen by the file watcher since the last successful sync — * the per-file "stale" signal MCP tools attach to responses so an agent diff --git a/src/mcp/answer-freshness.ts b/src/mcp/answer-freshness.ts new file mode 100644 index 0000000000..297eab4728 --- /dev/null +++ b/src/mcp/answer-freshness.ts @@ -0,0 +1,56 @@ +import { createHash } from 'crypto'; +import { createReadStream } from 'fs'; +import { validatePathWithinRoot } from '../utils'; + +export interface AnswerFile { + path: string; + /** The indexed version used to build the answer, even across a worker hop. */ + contentHash: string | null; +} + +/** Limits apply to the entire answer; exceeding any of them refuses it. */ +const MAX_FILES = 200; +const MAX_BYTES = 8 * 1024 * 1024; +const MAX_MS = 250; + +export async function validateAnswerFiles(root: string, files: AnswerFile[]): Promise<{ + stale: string[]; + unchecked: string[]; +}> { + const stale: string[] = []; + const unchecked: string[] = []; + const signal = AbortSignal.timeout(MAX_MS); + let bytes = 0; + for (let i = 0; i < files.length; i++) { + const file = files[i]!; + if (i >= MAX_FILES || bytes >= MAX_BYTES || signal.aborted) { + unchecked.push(...files.slice(i).map(f => f.path)); + break; + } + if (!file.contentHash) { + unchecked.push(file.path); + continue; + } + try { + const hash = createHash('sha256'); + const absolute = validatePathWithinRoot(root, file.path); + if (!absolute) { stale.push(file.path); continue; } + const stream = createReadStream(absolute, { + encoding: 'utf8', highWaterMark: 64 * 1024, signal, + }); + let complete = true; + for await (const chunk of stream) { + bytes += Buffer.byteLength(chunk); + if (bytes > MAX_BYTES) { complete = false; break; } + hash.update(chunk); + } + if (!complete) unchecked.push(file.path); + else if (hash.digest('hex') !== file.contentHash) stale.push(file.path); + } catch { + // Timeout is unknown, never a clean bill of health. A missing or + // unreadable contributing file cannot support the indexed answer. + (signal.aborted ? unchecked : stale).push(file.path); + } + } + return { stale, unchecked }; +} diff --git a/src/mcp/index-freshness-worker.ts b/src/mcp/index-freshness-worker.ts new file mode 100644 index 0000000000..06d9787372 --- /dev/null +++ b/src/mcp/index-freshness-worker.ts @@ -0,0 +1,23 @@ +/** Exact CLI-parity change count, isolated from the MCP transport event loop. */ +import { parentPort, workerData } from 'worker_threads'; + +if (parentPort) { + const port = parentPort; + let cg: import('../index').default | null = null; + let counts: { added: number; modified: number; removed: number } | null = null; + try { + const CodeGraph = (require('../index') as typeof import('../index')).default; + cg = CodeGraph.openSync((workerData as { root: string }).root); + const changes = cg.getChangedFiles(); + counts = { + added: changes.added.length, + modified: changes.modified.length, + removed: changes.removed.length, + }; + } catch { + // A failed or timed-out measurement must never masquerade as zero changes. + } finally { + try { cg?.close(); } catch { /* the worker is exiting */ } + } + port.postMessage(counts); +} diff --git a/src/mcp/index-freshness.ts b/src/mcp/index-freshness.ts new file mode 100644 index 0000000000..ed6c66b02f --- /dev/null +++ b/src/mcp/index-freshness.ts @@ -0,0 +1,66 @@ +import { existsSync } from 'fs'; +import * as path from 'path'; +import { Worker } from 'worker_threads'; + +export interface PendingChangeCounts { + added: number; + modified: number; + removed: number; +} + +/** `getChangedFiles()` may fall back to a huge filesystem scan (#1959). */ +const MEASURE_TIMEOUT_MS = 8_000; +let liveMeasurements = 0; +const active = new Map>(); + +export function measurePendingChanges(root: string): Promise { + const key = path.resolve(root); + const existing = active.get(key); + if (existing) return existing; + // Status probes for many projects must not exhaust the shared daemon. + if (liveMeasurements >= 2) return Promise.resolve(null); + + const pending = runMeasurement(key).finally(() => active.delete(key)); + active.set(key, pending); + return pending; +} + +function runMeasurement(root: string): Promise { + // The compiled sibling is beside us in production; Vitest loads src/ but + // builds dist/ before the tests, so use that copy for the worker there. + const sibling = path.join(__dirname, 'index-freshness-worker.js'); + const workerFile = existsSync(sibling) + ? sibling + : path.resolve(__dirname, '../../dist/mcp/index-freshness-worker.js'); + if (!existsSync(workerFile)) return Promise.resolve(null); + + let worker: Worker; + try { + worker = new Worker(workerFile, { workerData: { root } }); + } catch { + return Promise.resolve(null); + } + + liveMeasurements++; + return new Promise(resolve => { + let settled = false; + const finish = (counts: PendingChangeCounts | null) => { + if (settled) return; + settled = true; + clearTimeout(timer); + // A worker inside a synchronous Git call may take time to terminate. + // Return unknown on deadline, but keep its concurrency slot until exit. + void worker.terminate().catch(() => {}).finally(() => { liveMeasurements--; }); + resolve(counts); + }; + const timer = setTimeout(() => finish(null), MEASURE_TIMEOUT_MS); + worker.once('message', (value: PendingChangeCounts | null) => { + const valid = value && typeof value === 'object' && ['added', 'modified', 'removed'].every( + key => Number.isSafeInteger(value[key as keyof PendingChangeCounts]) && value[key as keyof PendingChangeCounts] >= 0, + ); + finish(valid ? value : null); + }); + worker.once('error', () => finish(null)); + worker.once('exit', () => finish(null)); + }); +} diff --git a/src/mcp/server-instructions.ts b/src/mcp/server-instructions.ts index 324a73f2d5..80a6247a80 100644 --- a/src/mcp/server-instructions.ts +++ b/src/mcp/server-instructions.ts @@ -64,7 +64,7 @@ calls; a grep/read exploration is dozens. - **Trust codegraph's results — don't re-verify them with grep.** They come from a full AST parse; re-checking with grep is slower, less accurate, and wastes context. - **Don't grep or Read first** to find or understand indexed code — ONE \`codegraph_explore\` returns the relevant symbols' source together in a single round-trip. Reach for raw \`Read\`/\`Grep\` only to confirm a specific detail codegraph didn't cover, or for what codegraph doesn't index (configs, docs). - **Don't reconstruct a flow by hand** — name the endpoints in one \`codegraph_explore\` and it surfaces the path between them, dynamic-dispatch hops included. -- **After editing, check the staleness banner.** When a tool response starts with "⚠️ Some files referenced below were edited since the last index sync…", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. A different, rarer banner — "⚠️ CodeGraph auto-sync is DISABLED…" — means live watching stopped entirely (the whole index is frozen, not just a few files); until it's resolved, Read files directly to confirm anything that may have changed. +- **After editing, check the staleness banner.** When a tool response starts with "⚠️ Some files referenced below were edited since the last index sync…", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. A different, rarer banner — "⚠️ CodeGraph auto-sync is DISABLED…" or "⚠️ CodeGraph auto-sync is RECOVERING…" — means the whole index may be stale; until the full catch-up completes, Read files directly to confirm anything that may have changed. If a response refuses to answer from changed files or says freshness could not be verified within its budget, retry after sync or narrow the query; that refusal is not an error in the tool. - **A file flagged "⚠ changed on disk after the last index sync" drifted from its index** (most common on projects queried via \`projectPath\`, which have no live watcher). Codegraph never serves a possibly-mis-sliced body from such a file — it either shows the file's full CURRENT source (trust it as a Read) or omits the source with this flag. When the source was omitted, Read that specific file; line numbers referencing it elsewhere in the response may be shifted until that project's next sync. All unflagged files remain trustworthy. - **Source is re-served on every call by default**, including for fresh subagents and after context compaction. Cross-call dedup requires \`CODEGRAPH_EXPLORE_DEDUP=1\` and is only suitable for hosts that guarantee one durable context per connection. With that opt-in, **"Already sent earlier in this conversation"** points to exact, unchanged source returned by an earlier \`codegraph_explore\` in that context. Use that copy; don't re-fetch it and don't Read the file. The bytes it freed went into source you have not seen yet, elsewhere in the same response. diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 33a2003abf..c6c9fea386 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -50,6 +50,8 @@ import { resolveNamedSymbolFlow, } from '../graph/named-symbol-flow'; import { getUpdateNotice } from '../upgrade/update-check'; +import { measurePendingChanges } from './index-freshness'; +import { validateAnswerFiles, type AnswerFile } from './answer-freshness'; import { ExploreDiagnostics } from './explore-diagnostics'; import { EXPLORE_EMISSION_KEY, @@ -1114,6 +1116,15 @@ export function formatDegradedBanner(reason: string | null): string { ); } +/** Re-armed watches are not proof of freshness until their full scan commits. */ +export function formatRecoveringBanner(): string { + return ( + '⚠️ CodeGraph auto-sync is RECOVERING — file watching restarted after lock contention, ' + + 'but the full index catch-up has not completed. Read files directly to confirm ' + + 'current content before relying on these results.' + ); +} + /** * MCP Tool definition */ @@ -1186,6 +1197,9 @@ export interface ToolResult { * {@link EXPLORE_EMISSION_KEY}; the two must stay in sync. */ _cgExploreEmission?: ExploreEmission; + /** Internal structured provenance, preserved by query workers and stripped by execute. */ + _cgAnswerFiles?: AnswerFile[]; + structuredContent?: Record; } /** @@ -1393,7 +1407,7 @@ export const tools: ToolDefinition[] = [ }, { name: 'codegraph_status', - description: 'Index health check (files / nodes / edges). Skip unless debugging.', + description: 'Index health check: files, nodes, edges, last indexed time, and added/modified/removed counts. Skip unless debugging.', inputSchema: { type: 'object', properties: { @@ -2090,7 +2104,6 @@ export class ToolHandler { */ private driftCache = new Map(); private static readonly DRIFT_TTL_MS = 2000; - /** * On-disk drift check for a single indexed file (issue #1474). The code * renderers slice CURRENT bytes at INDEXED line ranges; when the file @@ -2149,6 +2162,15 @@ export class ToolHandler { return stale; } + private answerResult(cg: CodeGraph, text: string, paths: Iterable): ToolResult { + const result = this.textResult(text); + result._cgAnswerFiles = [...new Set(paths)].map(file => ({ + path: file, + contentHash: cg.getFile(file)?.contentHash ?? null, + })); + return result; + } + private withStalenessNotice(result: ToolResult, projectPath?: string): ToolResult { if (result.isError) return result; @@ -2190,6 +2212,10 @@ export class ToolHandler { if (degraded) { const [head, ...tail] = result.content; if (!head || head.type !== 'text') return result; + if (cg.isWatcherRecovering?.()) { + const composed = `${formatRecoveringBanner()}\n\n${head.text}`; + return { ...result, content: [{ type: 'text', text: composed }, ...tail] }; + } let reason: string | null = null; try { reason = cg.getWatcherDegradedReason?.() ?? null; @@ -2298,6 +2324,14 @@ export class ToolHandler { if (typeof check === 'object' && check !== undefined) return check; } + const project = await this.getCodeGraph(args.projectPath as string | undefined); + // Recover a watcher disabled by prolonged lock contention on the next call. + // The stale banner remains until the watcher finishes its full scan; + // frequent calls cannot bypass its cooldown (#1959). + if (project.rearmWatcherAfterLockContention?.()) { + process.stderr.write('[CodeGraph MCP] Re-armed file watcher; full catch-up pending.\n'); + } + // codegraph_status reports watcher state (pending files, degraded mode, // worktree warning) and embeds its own sections — it must run on the MAIN // thread against the watched default instance, so it is NEVER off-loaded to @@ -2333,9 +2367,30 @@ export class ToolHandler { // keep the main handler's workspace-specific not-indexed guidance. const pooled = !!(this.queryPool && this.queryPool.healthy && this.queryPool.ready); const projectPath = pooled ? args.projectPath ?? this.cg?.getProjectRoot() : undefined; + const wasDegraded = project.isWatcherDegraded?.(); const raw = (pooled && projectPath) ? await this.queryPool!.run(toolName, { ...dispatchArgs, projectPath }) : await this.executeReadTool(toolName, dispatchArgs); + const answeredFrom = raw._cgAnswerFiles; + delete raw._cgAnswerFiles; + if (!raw.isError && answeredFrom && (wasDegraded || project.isWatcherDegraded?.())) { + const validation = await validateAnswerFiles(project.getProjectRoot(), answeredFrom); + if (validation.stale.length || validation.unchecked.length) { + // Rejected source must not enter the session's emission history. + const lines = ['⚠️ CodeGraph cannot answer from this index:']; + if (validation.stale.length) { + lines.push('These files changed or became unavailable after their last sync:', + ...validation.stale.map(file => `- ${file}`)); + } + if (validation.unchecked.length) { + lines.push(`Freshness could not be verified within the validation budget for ${validation.unchecked.length} files:`, + ...validation.unchecked.slice(0, 20).map(file => `- ${file}`)); + if (validation.unchecked.length > 20) lines.push(`- … ${validation.unchecked.length - 20} more (narrow the query)`); + } + lines.push('Retry after a successful codegraph sync, or narrow the query.'); + return { ...this.textResult(lines.join('\n')), structuredContent: { freshness: validation } }; + } + } // Record + STRIP before anything else touches the result: the emission is // internal bookkeeping and must never reach the client, whether or not a // caller passed session state. @@ -2501,7 +2556,7 @@ export class ToolHandler { }); const formatted = this.formatSearchResults(ranked); - return this.textResult(this.truncateOutput(formatted)); + return this.answerResult(cg, this.truncateOutput(formatted), ranked.map(r => r.node.filePath)); } /** @@ -2542,6 +2597,7 @@ export class ToolHandler { } const { groups, filteredOut } = this.groupDefinitions(allMatches.nodes, fileFilter); + const answerPaths = new Set(groups.flat().map(node => node.filePath)); const filterNote = filteredOut ? `\n\n> **Note:** no definition of "${symbol}" matches file "${fileFilter}" — showing all definitions instead.` : ''; @@ -2555,6 +2611,7 @@ export class ToolHandler { if (!seen.has(c.node.id)) { seen.add(c.node.id); callers.push(c.node); + answerPaths.add(c.node.filePath); const label = this.edgeLabel(c.edge); if (label) labels.set(c.node.id, label); } @@ -2567,7 +2624,7 @@ export class ToolHandler { if (groups.length === 1) { const { callers, labels } = collect(groups[0]!); if (callers.length === 0) { - return this.textResult(`No callers found for "${symbol}"${allMatches.note}${filterNote}`); + return this.answerResult(cg, `No callers found for "${symbol}"${allMatches.note}${filterNote}`, answerPaths); } // A successful `file` narrowing makes the multi-symbol aggregation note // stale — suppress it. @@ -2578,7 +2635,7 @@ export class ToolHandler { ? `\n\n> Showing ${limit} of ${callers.length} callers; pass \`limit\` (up to 100) to widen.` : ''; const formatted = this.formatNodeList(callers.slice(0, limit), `Callers of ${symbol}`, labels) + cut + note + filterNote; - return this.textResult(this.truncateOutput(formatted)); + return this.answerResult(cg, this.truncateOutput(formatted), answerPaths); } // Multiple DISTINCT definitions (#764): one section per definition so an @@ -2603,7 +2660,7 @@ export class ToolHandler { lines.push(`- … +${callers.length - limit} more (pass \`limit\` to widen)`); } } - return this.textResult(this.truncateOutput(lines.join('\n') + filterNote)); + return this.answerResult(cg, this.truncateOutput(lines.join('\n') + filterNote), answerPaths); } /** @@ -2623,6 +2680,7 @@ export class ToolHandler { } const { groups, filteredOut } = this.groupDefinitions(allMatches.nodes, fileFilter); + const answerPaths = new Set(groups.flat().map(node => node.filePath)); const filterNote = filteredOut ? `\n\n> **Note:** no definition of "${symbol}" matches file "${fileFilter}" — showing all definitions instead.` : ''; @@ -2636,6 +2694,7 @@ export class ToolHandler { if (!seen.has(c.node.id)) { seen.add(c.node.id); callees.push(c.node); + answerPaths.add(c.node.filePath); const label = this.edgeLabel(c.edge); if (label) labels.set(c.node.id, label); } @@ -2647,7 +2706,7 @@ export class ToolHandler { if (groups.length === 1) { const { callees, labels } = collect(groups[0]!); if (callees.length === 0) { - return this.textResult(`No callees found for "${symbol}"${allMatches.note}${filterNote}`); + return this.answerResult(cg, `No callees found for "${symbol}"${allMatches.note}${filterNote}`, answerPaths); } // A successful `file` narrowing makes the multi-symbol aggregation note // stale — suppress it. @@ -2658,7 +2717,7 @@ export class ToolHandler { ? `\n\n> Showing ${limit} of ${callees.length} callees; pass \`limit\` (up to 100) to widen.` : ''; const formatted = this.formatNodeList(callees.slice(0, limit), `Callees of ${symbol}`, labels) + cut + note + filterNote; - return this.textResult(this.truncateOutput(formatted)); + return this.answerResult(cg, this.truncateOutput(formatted), answerPaths); } // Multiple DISTINCT definitions (#764): per-definition sections. @@ -2681,7 +2740,7 @@ export class ToolHandler { lines.push(`- … +${callees.length - limit} more (pass \`limit\` to widen)`); } } - return this.textResult(this.truncateOutput(lines.join('\n') + filterNote)); + return this.answerResult(cg, this.truncateOutput(lines.join('\n') + filterNote), answerPaths); } /** @@ -2701,6 +2760,7 @@ export class ToolHandler { } const { groups, filteredOut } = this.groupDefinitions(allMatches.nodes, fileFilter); + const answerPaths = new Set(groups.flat().map(node => node.filePath)); const filterNote = filteredOut ? `\n\n> **Note:** no definition of "${symbol}" matches file "${fileFilter}" — showing all definitions instead.` : ''; @@ -2713,6 +2773,7 @@ export class ToolHandler { const impact = cg.getImpactRadius(node.id, depth); for (const [id, n] of impact.nodes) { mergedNodes.set(id, n); + answerPaths.add(n.filePath); } for (const e of impact.edges) { const key = `${e.source}->${e.target}:${e.kind}`; @@ -2728,7 +2789,7 @@ export class ToolHandler { // Single definition (or same-file overloads): the familiar merged report. if (groups.length === 1) { const formatted = this.formatImpact(symbol, impactOf(groups[0]!)) + (fileFilter && !filteredOut ? "" : allMatches.note) + filterNote; - return this.textResult(this.truncateOutput(formatted)); + return this.answerResult(cg, this.truncateOutput(formatted), answerPaths); } // Multiple DISTINCT definitions (#764): a blast radius PER definition — @@ -2745,7 +2806,7 @@ export class ToolHandler { this.formatImpact(`${head.qualifiedName} (${head.filePath}${line})`, impactOf(group)) ); } - return this.textResult(this.truncateOutput(sections.join('\n') + filterNote)); + return this.answerResult(cg, this.truncateOutput(sections.join('\n') + filterNote), answerPaths); } /** @@ -3608,9 +3669,9 @@ export class ToolHandler { const empty = `No relevant code found for "${query}"${missNote}${explanation}`; // Still an explore call, so it is still recorded: an empty answer spends a // call against the tier budget even though it emits no source. - return this.exploreResult(empty, { + return { ...this.textResult(empty), [EXPLORE_EMISSION_KEY]: { projectRoot, query, files: [], sourceBytes: 0, responseBytes: empty.length, - }); + } }; } // Graph-aware glue: findRelevantContext builds the subgraph from name/text @@ -6297,23 +6358,20 @@ export class ToolHandler { }); sourceBytes += emitted.bytes; } - return this.exploreResult(finalText, { + // Include graph-only and omitted/deleted files, not just emitted source. + const answerPaths = new Set(fileGroups.keys()); + for (const id of [...flow.pathNodeIds, ...flow.namedNodeIds]) { + const node = cg.getNode(id); + if (node) answerPaths.add(node.filePath); + } + const result = this.answerResult(cg, finalText, answerPaths); + result[EXPLORE_EMISSION_KEY] = { projectRoot, query, files: emittedFiles, sourceBytes, responseBytes: finalText.length, - }); - } - - /** - * An explore response plus the record of what it emitted (CG-17). The record - * rides the result only as far as {@link execute}, which files it into the - * calling session's state and deletes it — see {@link EXPLORE_EMISSION_KEY}. - */ - private exploreResult(text: string, emission: ExploreEmission): ToolResult { - const result = this.textResult(text); - result[EXPLORE_EMISSION_KEY] = emission; + }; return result; } @@ -6784,6 +6842,18 @@ export class ToolHandler { `**Database size:** ${(stats.dbSizeBytes / 1024 / 1024).toFixed(2)} MB`, ); + // Exact CLI-parity change counts are measured on a worker: Git or the + // filesystem fallback can stall on a large/busy checkout, but status must + // not block the shared daemon's transport (#1959). Unknown is never zero. + const lastIndexedAt = cg.getLastIndexedAt(); + const changes = await measurePendingChanges(cg.getProjectRoot()); + lines.push( + `**Latest file indexed:** ${lastIndexedAt == null ? 'never' : new Date(lastIndexedAt).toISOString()}`, + changes + ? `**Changes since index:** ${changes.added} added, ${changes.modified} modified, ${changes.removed} removed` + : '**Changes since index:** unknown (measurement timed out or failed; do not assume the index is current)', + ); + // Surface the active SQLite backend (node:sqlite, Node's built-in real // SQLite — full WAL + FTS5, no native build). lines.push(`**Backend:** node:sqlite (Node built-in) — full WAL + FTS5`); @@ -6843,11 +6913,14 @@ export class ToolHandler { // but the index is frozen — call that out explicitly here, the one place an // agent asks "is the index caught up?". if (cg.isWatcherDegraded()) { + const recovering = cg.isWatcherRecovering(); lines.push( '', - '**Auto-sync disabled:**', - `- ${cg.getWatcherDegradedReason() ?? 'live file watching stopped'}`, - '- The index is frozen; Read files directly for current content.' + recovering ? '**Auto-sync recovering:**' : '**Auto-sync disabled:**', + recovering + ? '- File watching restarted; full index catch-up has not completed.' + : `- ${cg.getWatcherDegradedReason() ?? 'live file watching stopped'}`, + '- The index may be stale; Read files directly for current content.' ); } @@ -6866,7 +6939,9 @@ export class ToolHandler { } } - return this.textResult(lines.join('\n')); + return { ...this.textResult(lines.join('\n')), structuredContent: { + freshness: { lastIndexedAt, changes, complete: changes !== null }, + } }; } /** diff --git a/src/sync/watcher.ts b/src/sync/watcher.ts index 74257cc58f..0d1b13148c 100644 --- a/src/sync/watcher.ts +++ b/src/sync/watcher.ts @@ -48,6 +48,8 @@ import { watchDisabledReason } from './watch-policy'; * few cycles) stays under this; a long-lived external writer crosses it. */ const MAX_LOCK_RETRIES = 5; +/** A failed re-arm must not turn frequent MCP calls into a lock-polling loop. */ +const LOCK_REARM_COOLDOWN_MS = 30_000; /** * Number of consecutive GENERIC (non-lock) sync failures the watcher tolerates * before it degrades auto-sync. A deterministic failure — a tree-sitter @@ -278,12 +280,16 @@ export class FileWatcher { */ private inotifyLimitWarned = false; /** - * One-way latch: the reason live watching was permanently disabled at runtime + * The reason live watching was 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 - * healthy. Set by {@link degrade}; cleared only by a fresh start(). + * healthy. Set by {@link degrade}; a lock-contention recovery keeps it until + * a full reconciliation succeeds. */ private degradedReason: string | null = null; + private degradedByLock = false; + private recoveringFromLock = false; + private lastLockRearmMs = 0; /** Consecutive lock-contention retries for watcher-triggered syncs. */ private lockRetryCount = 0; /** Consecutive generic (non-lock) sync failures; reset only by a clean sync. */ @@ -380,6 +386,8 @@ export class FileWatcher { if (this.recursiveWatcher || this.dirWatchers.size > 0 || this.inert) return true; // Already watching this.stopped = false; this.degradedReason = null; + this.degradedByLock = false; + this.recoveringFromLock = false; this.lockRetryCount = 0; this.syncFailureRetryCount = 0; this.needsFullScan = false; @@ -518,7 +526,7 @@ export class FileWatcher { } private watchDirectory(dir: string, real: string, recursive: boolean): void { - if (this.stopped || this.degradedReason || this.inotifyLimitWarned) return; + if (this.stopped || (this.degradedReason && !this.recoveringFromLock) || this.inotifyLimitWarned) return; if (this.dirWatchers.has(dir)) return; // Recursive streams cost more than inotify entries. Keep a separate hard // ceiling while honouring a lower user-configured directory watch budget. @@ -745,15 +753,17 @@ export class FileWatcher { } /** - * Permanently disable live watching after a terminal runtime failure + * 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). * Idempotent: logs one actionable warning, fires {@link WatchOptions.onDegraded} * once, and stops the watcher. A subsequent start() clears the latch. */ - private degrade(reason: string, context: Record = {}): void { - if (this.degradedReason) return; + private degrade(reason: string, context: Record = {}, byLock = false): void { + if (this.degradedReason && !this.recoveringFromLock) return; this.degradedReason = reason; + this.degradedByLock = byLock; + this.recoveringFromLock = false; logWarn('File watcher disabled', { projectRoot: this.projectRoot, reason, ...context }); this.onDegraded?.(reason); this.stop(); @@ -774,7 +784,7 @@ export class FileWatcher { } /** - * Whether live watching has degraded permanently (until the next start()). + * Whether live watching has degraded or is still catching up after re-arm. * Distinct from {@link isActive}: a degraded watcher is inactive, but an * inactive watcher is not necessarily degraded (it may simply be stopped or * never started). Hosts use this to tell the user auto-sync is off. @@ -788,6 +798,41 @@ export class FileWatcher { return this.degradedReason; } + /** Watches are live again, but the full catch-up has not committed yet. */ + isRecoveringFromLock(): boolean { + return this.recoveringFromLock; + } + + /** + * Re-arm a watcher disabled by lock contention when a caller next uses the + * graph. Keep the whole-index stale banner until a FULL scan reconciles edits + * missed while watches were off. Resource exhaustion and deterministic sync + * failures are not automatically retried. Repeated unsuccessful re-arms are + * throttled so many MCP sessions cannot hammer a long-lived writer. + */ + rearmAfterLockContention(): boolean { + if (!this.degradedByLock || this.isActive()) return false; + const now = Date.now(); + if (this.lastLockRearmMs && now - this.lastLockRearmMs < LOCK_REARM_COOLDOWN_MS) return false; + const reason = this.degradedReason; + this.lastLockRearmMs = now; + if (!this.start()) { + // A disabled platform can reject start() without calling degrade(). + // Preserve the old stale-index signal in that case; a new resource + // failure has its own more precise degraded reason. + if (!this.degradedReason) { + this.degradedReason = reason; + this.degradedByLock = true; + } + return false; + } + this.degradedReason = reason; + this.recoveringFromLock = true; + this.needsFullScan = true; + this.scheduleSync(); + return true; + } + /** * Stop watching for file changes. */ @@ -797,6 +842,7 @@ export class FileWatcher { this.treeRefreshTimer = null; this.knownDirs.clear(); this.watchedRealDirs.clear(); + this.recoveringFromLock = false; if (this.debounceTimer) { clearTimeout(this.debounceTimer); @@ -947,6 +993,10 @@ export class FileWatcher { if (!scoped) this.needsFullScan = false; try { const result = await this.syncFn(scoped); + if (this.recoveringFromLock && !scoped && !this.needsFullScan) { + this.degradedReason = null; + this.recoveringFromLock = false; + } this.lockRetryCount = 0; // a clean sync clears any contention backoff this.syncFailureRetryCount = 0; // ...and any generic-failure backoff // Remove entries whose most recent event predates this sync — those @@ -980,7 +1030,8 @@ export class FileWatcher { '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 } + { pendingFiles: this.pendingFiles.size, retryCount: this.lockRetryCount }, + true ); } } else { diff --git a/src/utils.ts b/src/utils.ts index dc860a3f49..cdc46e831f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -241,9 +241,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; } @@ -257,18 +254,16 @@ export class FileLock { 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; - - // Treat locks older than the timeout as stale, regardless of PID - if (lockAge < FileLock.STALE_TIMEOUT_MS && !isNaN(pid) && this.isProcessAlive(pid)) { + // A long sync can hold the lock for hours. Its age says nothing about + // whether the writer is still alive; only a dead PID is stale. + if (!isNaN(pid) && 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 + // Stale lock (dead process or malformed PID) - remove it fs.unlinkSync(this.lockPath); } catch (err) { if (err instanceof Error && err.message.includes('locked by another')) {