diff --git a/CHANGELOG.md b/CHANGELOG.md index 75535a0bbd..cbc994864d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,11 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### MCP / indexing +- While auto-sync is off, `codegraph_search`, `codegraph_callers`, `codegraph_callees` and `codegraph_impact` likewise refuse an answer that names a file changed since its last sync, and name that file instead. (#1959) +- While auto-sync is off, `codegraph_explore` no longer answers from files that changed since their last sync: it names them so they can be read directly, and keeps answering from files that did not change. (#1959) +- `codegraph_status` over MCP now reports when files were last indexed and how many were added, changed or removed since, computed without blocking other requests, so a frozen index shows up as numbers rather than only a banner. (#1959) +- After a long stretch of contention for the index lock, the next MCP call restarts file watching and runs a full catch-up instead of leaving auto-sync off for the rest of the session; answers say the index may be stale until the catch-up finishes. (#1959) +- File watching no longer drops the full re-scan a removed directory asks for when that sync fails, so the deleted files leave the index instead of lingering. (#1964) - Daemon startup and cleanup now preserve live legacy PID-only locks while still reclaiming dead or identity-disproved records, preventing two writers from serving the same project. - Incremental sync now keeps edge rebinding crash-safe: replacing a resolved edge with its recovery reference commits atomically, so an interruption cannot permanently remove the relationship. - Status now detects committed but unindexed changes and restored edits without scanning every source file; thanks @inth3shadows. (#1829) diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 304ea5746a..ad517b9dca 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -8248,6 +8248,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-stale-refusal.test.ts b/__tests__/mcp-stale-refusal.test.ts new file mode 100644 index 0000000000..929ca1a7e7 --- /dev/null +++ b/__tests__/mcp-stale-refusal.test.ts @@ -0,0 +1,98 @@ +import { afterEach, beforeEach, describe, expect, it } 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 { __setFsWatchForTests } from '../src/sync/watcher'; + +describe('a degraded index refuses answers from changed files (#1959)', () => { + let root: string; + let cg: CodeGraph; + let handler: ToolHandler; + + beforeEach(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-stale-refusal-')); + 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(() => { + __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'); + }); +}); diff --git a/__tests__/mcp-staleness-banner.test.ts b/__tests__/mcp-staleness-banner.test.ts index 4b51e12318..36cf575fca 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,4 +210,24 @@ 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); + }); }); diff --git a/__tests__/mcp-status-freshness.test.ts b/__tests__/mcp-status-freshness.test.ts new file mode 100644 index 0000000000..9c7106ab37 --- /dev/null +++ b/__tests__/mcp-status-freshness.test.ts @@ -0,0 +1,60 @@ +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, { config: { include: ['**/*.ts'], exclude: [] } }); + 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 changed = (await handler.execute('codegraph_status', {})).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__/watcher.test.ts b/__tests__/watcher.test.ts index 6b87a55ca9..e0a5407ab7 100644 --- a/__tests__/watcher.test.ts +++ b/__tests__/watcher.test.ts @@ -322,6 +322,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)', () => { @@ -349,6 +396,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. @@ -968,6 +1016,78 @@ describe('FileWatcher', () => { expect(calls[0]).toBeUndefined(); }); + it.each([ + ['lock contention', () => new LockUnavailableError()], + ['sync failure', () => new Error('injected sync failure')], + ])('retries a full-only directory removal after %s (#1964)', async (_case, failure) => { + const syncFn = vi.fn() + .mockRejectedValueOnce(failure()) + .mockResolvedValue({ filesChanged: 0, durationMs: 5 }); + const watcher = newWatcher(syncFn, { debounceMs: 25 }); + watcher.start(); + try { + await watcher.waitUntilReady(); + __emitWatchEventForTests(testDir, 'src/removed-dir'); + await waitFor(() => syncFn.mock.calls.length >= 2, 4000); + expect(syncFn.mock.calls.map(call => call[0])).toEqual([undefined, undefined]); + } finally { + watcher.stop(); + } + }); + + it('follows a scoped sync with a full scan when a directory is removed mid-sync (#1964)', async () => { + let release!: () => void; + const firstRun = new Promise(resolve => { release = resolve; }); + const calls: (string[] | undefined)[] = []; + const syncFn: SyncFn = async paths => { + calls.push(paths); + if (calls.length === 1) await firstRun; + return { filesChanged: 0, durationMs: 5 }; + }; + const watcher = newWatcher(syncFn, { debounceMs: 25 }); + watcher.start(); + try { + await watcher.waitUntilReady(); + fs.writeFileSync(path.join(testDir, 'src', 'a.ts'), 'export const a = 1;'); + __emitWatchEventForTests(testDir, 'src/a.ts'); + await waitFor(() => calls.length === 1, 4000); + __emitWatchEventForTests(testDir, 'src/removed-dir'); + release(); + await waitFor(() => calls.length >= 2, 4000); + expect(calls).toEqual([['src/a.ts'], undefined]); + } finally { + release(); + watcher.stop(); + } + }); + + it('follows a scoped sync with a full scan when scope changes mid-sync (#1964)', async () => { + let release!: () => void; + const firstRun = new Promise(resolve => { release = resolve; }); + const calls: (string[] | undefined)[] = []; + const syncFn: SyncFn = async paths => { + calls.push(paths); + if (calls.length === 1) await firstRun; + return { filesChanged: 0, durationMs: 5 }; + }; + const watcher = newWatcher(syncFn, { debounceMs: 25 }); + watcher.start(); + try { + await watcher.waitUntilReady(); + fs.writeFileSync(path.join(testDir, 'src', 'a.ts'), 'export const a = 1;'); + __emitWatchEventForTests(testDir, 'src/a.ts'); + await waitFor(() => calls.length === 1, 4000); + fs.writeFileSync(path.join(testDir, '.gitignore'), 'src/ignored/\n'); + __emitWatchEventForTests(testDir, '.gitignore'); + release(); + await waitFor(() => calls.length >= 2, 4000); + expect(calls).toEqual([['src/a.ts'], undefined]); + } finally { + release(); + watcher.stop(); + } + }); + it('a lone file event fires on the quick window, well before the full debounce', async () => { const calls: (string[] | undefined)[] = []; const syncFn: SyncFn = async (paths?: string[]) => { diff --git a/src/index.ts b/src/index.ts index 7b8ef2ca78..8e902ec2ad 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1115,12 +1115,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; @@ -1131,6 +1130,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/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..eaf40db7c0 --- /dev/null +++ b/src/mcp/index-freshness.ts @@ -0,0 +1,60 @@ +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; +const active = new Map>(); + +export function measurePendingChanges(root: string): Promise { + const key = path.resolve(root); + const existing = active.get(key); + if (existing) return existing; + + 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); + } + + return new Promise(resolve => { + let settled = false; + const finish = (counts: PendingChangeCounts | null) => { + if (settled) return; + settled = true; + clearTimeout(timer); + resolve(counts); + void worker.terminate().catch(() => {}); + }; + 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 4cb22150d5..362407a4ed 100644 --- a/src/mcp/server-instructions.ts +++ b/src/mcp/server-instructions.ts @@ -63,7 +63,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, Read the paths it names; 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 42fdd22884..835d304c14 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -49,6 +49,7 @@ import { resolveNamedSymbolFlow, } from '../graph/named-symbol-flow'; import { getUpdateNotice } from '../upgrade/update-check'; +import { measurePendingChanges } from './index-freshness'; import { ExploreDiagnostics } from './explore-diagnostics'; import { EXPLORE_EMISSION_KEY, @@ -1074,6 +1075,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 */ @@ -1353,7 +1363,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: { @@ -1941,6 +1951,17 @@ export class ToolHandler { */ private driftCache = new Map(); private static readonly DRIFT_TTL_MS = 2000; + /** + * Tools whose answer is read off the graph of the files it names, and so is + * refused like explore's when one of those files changed while auto-sync was + * off (#1959). codegraph_node is absent on purpose: its drift gate already + * serves a changed file's current bytes instead of the indexed slice (#1474). + */ + private static readonly GRAPH_ANSWER_TOOLS = new Set([ + 'codegraph_search', 'codegraph_callers', 'codegraph_callees', 'codegraph_impact', + ]); + /** Bounds the per-call hashing a degraded-index check may do. */ + private static readonly MAX_ANSWER_PATHS = 200; /** * On-disk drift check for a single indexed file (issue #1474). The code @@ -1965,7 +1986,7 @@ export class ToolHandler { * are handled by the existing not-found paths, and a wrong "stale" flag * would needlessly push the agent back to Read. */ - private isFileStaleOnDisk(cg: CodeGraph, relPath: string, content?: string): boolean { + private isFileStaleOnDisk(cg: CodeGraph, relPath: string, content?: string, forceHash = false): boolean { let root: string; try { root = cg.getProjectRoot(); @@ -1975,7 +1996,7 @@ export class ToolHandler { const key = `${root}\0${relPath}`; const now = Date.now(); const hit = this.driftCache.get(key); - if (hit && now - hit.at < ToolHandler.DRIFT_TTL_MS) return hit.stale; + if (!forceHash && hit && now - hit.at < ToolHandler.DRIFT_TTL_MS) return hit.stale; let stale = false; try { const rec = cg.getFile(relPath); @@ -1984,7 +2005,7 @@ export class ToolHandler { const st = statSync(absPath); // Same freshness test as the sync fast path (extraction/index.ts): // equal size + equal floored mtime ⇒ unchanged, no read needed. - if (st.size !== rec.size || Math.floor(st.mtimeMs) !== Math.floor(rec.modifiedAt)) { + if (forceHash || st.size !== rec.size || Math.floor(st.mtimeMs) !== Math.floor(rec.modifiedAt)) { const data = content ?? readFileSync(absPath, 'utf-8'); // Must stay byte-identical to extraction's `hashContent` (sha256 over // the utf-8 string) — the identical-rewrite test in @@ -1992,14 +2013,28 @@ export class ToolHandler { // to keep the extraction module off the MCP startup path. stale = createHash('sha256').update(data).digest('hex') !== rec.contentHash; } + } else if (forceHash) { + stale = true; // deleted/inaccessible since this response was rendered } } catch { - stale = false; + stale = forceHash; } this.driftCache.set(key, { at: now, stale }); return stale; } + /** Indexed files a graph tool's answer names as locations (#1959). */ + private indexedPathsIn(cg: CodeGraph, result: ToolResult): string[] { + const head = result.content[0]; + if (!head || head.type !== 'text') return []; + const found = new Set(); + for (const [token] of head.text.matchAll(/[\w@$+\-./]+\.\w+/g)) { + if (found.size >= ToolHandler.MAX_ANSWER_PATHS) break; + if (!found.has(token) && cg.getFile(token)) found.add(token); + } + return [...found]; + } + private withStalenessNotice(result: ToolResult, projectPath?: string): ToolResult { if (result.isError) return result; @@ -2043,6 +2078,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; @@ -2143,6 +2182,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 @@ -2176,6 +2223,23 @@ export class ToolHandler { const raw = (this.queryPool && this.queryPool.healthy && this.queryPool.ready) ? await this.queryPool.run(toolName, dispatchArgs) : await this.executeReadTool(toolName, dispatchArgs); + if (project.isWatcherDegraded?.()) { + // Explore reports the files it rendered; the graph tools name theirs as + // `path:line` locations in the text (#1959). + const answeredFrom = toolName === 'codegraph_explore' + ? raw[EXPLORE_EMISSION_KEY]?.files?.map(file => file.path) ?? [] + : ToolHandler.GRAPH_ANSWER_TOOLS.has(toolName) ? this.indexedPathsIn(project, raw) : []; + const stalePaths = answeredFrom.filter(file => this.isFileStaleOnDisk(project, file, undefined, true)); + if (stalePaths.length > 0) { + // Do not show graph/source derived from changed files, and do not + // record this rejected emission as source the session has seen. + return this.textResult( + '⚠️ CodeGraph cannot answer from this index: these files changed after their last sync:\n' + + stalePaths.map(file => `- ${file}`).join('\n') + + '\nRead those files directly or retry after a successful codegraph sync.' + ); + } + } // 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. @@ -6552,6 +6616,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`); @@ -6611,11 +6687,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.' ); } diff --git a/src/sync/watcher.ts b/src/sync/watcher.ts index 5ea6436b6c..063b1d2444 100644 --- a/src/sync/watcher.ts +++ b/src/sync/watcher.ts @@ -47,6 +47,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 @@ -277,12 +279,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. */ @@ -370,6 +376,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; @@ -480,7 +488,7 @@ export class FileWatcher { // watches to a watcher that is shutting down. `inotifyLimitWarned` does the // same after ENOSPC — the kernel budget is gone, so stop trying the rest of // the tree (every add would fail) while keeping the watches already set. - if (this.stopped || this.degradedReason || this.inotifyLimitWarned) return; + if (this.stopped || (this.degradedReason && !this.recoveringFromLock) || this.inotifyLimitWarned) return; if (this.dirWatchers.has(dir)) return; if (this.dirWatchers.size >= maxDirWatches()) { if (!this.dirCapWarned) { @@ -718,15 +726,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(); @@ -747,7 +757,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. @@ -761,11 +771,47 @@ 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. */ stop(): void { this.stopped = true; + this.recoveringFromLock = false; if (this.debounceTimer) { clearTimeout(this.debounceTimer); @@ -913,6 +959,10 @@ export class FileWatcher { try { const result = await this.syncFn(scoped); if (!scoped) this.needsFullScan = false; + if (this.recoveringFromLock && !scoped) { + 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 @@ -945,7 +995,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 { @@ -986,8 +1037,10 @@ export class FileWatcher { // 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 - // has given up. - if (this.pendingFiles.size > 0 && !this.stopped) { + // has given up. A directory removal whose full sync failed adds no + // pending file, only `needsFullScan` — it still owes the full reconcile + // it asked for (#1964). + if ((this.pendingFiles.size > 0 || this.needsFullScan) && !this.stopped) { const retryCount = Math.max(this.lockRetryCount, this.syncFailureRetryCount); if (retryCount > 0) { const retryDelayMs = Math.min(