From 6e23d59d8db374d15ebf4e46f34fdd0054615916 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sat, 26 Sep 2026 15:51:00 +0300 Subject: [PATCH 1/5] fix(watcher): keep a full scan owed after a failed sync (#1964) A directory removal adds no pending file, only needsFullScan. The retry after a failed sync (lock contention or a generic failure) was gated on pending files alone, so the owed full reconcile was dropped and the removed directory's files kept their nodes. Reschedule while needsFullScan is set as well. A regression test also pins the mid-sync scope-change case, which already follows a scoped pass with a full scan. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/watcher.test.ts | 72 +++++++++++++++++++++++++++++++++++++++ src/sync/watcher.ts | 6 ++-- 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75535a0bbd..1b28013cbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### MCP / indexing +- 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__/watcher.test.ts b/__tests__/watcher.test.ts index 6b87a55ca9..a457468b92 100644 --- a/__tests__/watcher.test.ts +++ b/__tests__/watcher.test.ts @@ -968,6 +968,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/sync/watcher.ts b/src/sync/watcher.ts index 5ea6436b6c..920a8a69c2 100644 --- a/src/sync/watcher.ts +++ b/src/sync/watcher.ts @@ -986,8 +986,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( From 28cb771318f2b56f6ecdfe9e4408d57d82a06f2f Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sat, 26 Sep 2026 11:03:09 +0300 Subject: [PATCH 2/5] fix(watcher): re-arm after lock contention and reconcile before trusting index (#1959) --- CHANGELOG.md | 1 + __tests__/extraction.test.ts | 23 +++++++++ __tests__/mcp-staleness-banner.test.ts | 23 ++++++++- __tests__/watcher.test.ts | 48 ++++++++++++++++++ src/index.ts | 21 +++++--- src/mcp/server-instructions.ts | 2 +- src/mcp/tools.ts | 30 ++++++++++-- src/sync/watcher.ts | 67 +++++++++++++++++++++++--- 8 files changed, 196 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b28013cbd..0007d7fd12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### MCP / indexing +- 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. 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-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__/watcher.test.ts b/__tests__/watcher.test.ts index a457468b92..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. 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/server-instructions.ts b/src/mcp/server-instructions.ts index 4cb22150d5..600fad28bd 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. - **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..c3c81c5351 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -1074,6 +1074,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 */ @@ -2043,6 +2052,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 +2156,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 @@ -6611,11 +6632,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 920a8a69c2..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 { From 6f2d6914f54ed6780c496cab5a9ecec95282c144 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sat, 26 Sep 2026 14:27:24 +0300 Subject: [PATCH 3/5] feat(mcp): report exact index freshness without blocking the daemon (#1959) --- CHANGELOG.md | 1 + __tests__/mcp-status-freshness.test.ts | 60 ++++++++++++++++++++++++++ src/mcp/index-freshness-worker.ts | 23 ++++++++++ src/mcp/index-freshness.ts | 60 ++++++++++++++++++++++++++ src/mcp/tools.ts | 15 ++++++- 5 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 __tests__/mcp-status-freshness.test.ts create mode 100644 src/mcp/index-freshness-worker.ts create mode 100644 src/mcp/index-freshness.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0007d7fd12..a902ead4fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### MCP / indexing +- `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. 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/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/tools.ts b/src/mcp/tools.ts index c3c81c5351..8d9c24cfb8 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, @@ -1362,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: { @@ -6573,6 +6574,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`); From fa4dcc3c594c1c87f324a02a5ac62818c43d0351 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sat, 26 Sep 2026 14:35:39 +0300 Subject: [PATCH 4/5] fix(mcp): refuse drifted explore source after watcher degradation (#1959) --- CHANGELOG.md | 1 + __tests__/mcp-stale-refusal.test.ts | 71 +++++++++++++++++++++++++++++ src/mcp/server-instructions.ts | 2 +- src/mcp/tools.ts | 25 ++++++++-- 4 files changed, 94 insertions(+), 5 deletions(-) create mode 100644 __tests__/mcp-stale-refusal.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a902ead4fe..09738d087e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### MCP / indexing +- 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) diff --git a/__tests__/mcp-stale-refusal.test.ts b/__tests__/mcp-stale-refusal.test.ts new file mode 100644 index 0000000000..e27cf910ae --- /dev/null +++ b/__tests__/mcp-stale-refusal.test.ts @@ -0,0 +1,71 @@ +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('degraded explore refuses changed source (#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'); + 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'); + }); +}); diff --git a/src/mcp/server-instructions.ts b/src/mcp/server-instructions.ts index 600fad28bd..6c7534341e 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…" 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. +- **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 an exploration 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 8d9c24cfb8..8bd9f85f1e 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -1975,7 +1975,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(); @@ -1985,7 +1985,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); @@ -1994,7 +1994,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 @@ -2002,9 +2002,11 @@ 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; @@ -2198,6 +2200,21 @@ 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 (toolName === 'codegraph_explore' && project.isWatcherDegraded?.()) { + const emission = raw[EXPLORE_EMISSION_KEY]; + const stalePaths = emission?.files + ?.filter(file => this.isFileStaleOnDisk(project, file.path, undefined, true)) + .map(file => file.path) ?? []; + 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. From c8724970163d8fbd53f834bc717e63bed5f97686 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sat, 26 Sep 2026 15:53:03 +0300 Subject: [PATCH 5/5] fix(mcp): refuse graph answers that name a drifted file after watcher degradation (#1959) Explore already refuses to render source from a file that changed while auto-sync was off. search, callers, callees and impact answer from the same frozen graph but kept replying, naming the changed file as if its edges were current. They now take the same refusal: the indexed files an answer names as locations are hashed against the index, and a drifted one is named instead of the answer. codegraph_node keeps its own drift gate, which serves the changed file's current bytes. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/mcp-stale-refusal.test.ts | 29 +++++++++++++++++++++++- src/mcp/server-instructions.ts | 2 +- src/mcp/tools.ts | 35 ++++++++++++++++++++++++----- 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09738d087e..cbc994864d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,7 @@ 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) diff --git a/__tests__/mcp-stale-refusal.test.ts b/__tests__/mcp-stale-refusal.test.ts index e27cf910ae..929ca1a7e7 100644 --- a/__tests__/mcp-stale-refusal.test.ts +++ b/__tests__/mcp-stale-refusal.test.ts @@ -7,7 +7,7 @@ import { ExploreSessionState } from '../src/mcp/explore-session-state'; import { ToolHandler } from '../src/mcp/tools'; import { __setFsWatchForTests } from '../src/sync/watcher'; -describe('degraded explore refuses changed source (#1959)', () => { +describe('a degraded index refuses answers from changed files (#1959)', () => { let root: string; let cg: CodeGraph; let handler: ToolHandler; @@ -16,6 +16,10 @@ describe('degraded explore refuses changed source (#1959)', () => { 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); @@ -68,4 +72,27 @@ describe('degraded explore refuses changed source (#1959)', () => { 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/src/mcp/server-instructions.ts b/src/mcp/server-instructions.ts index 6c7534341e..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…" 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 an exploration refuses to answer from changed files, Read the paths it names; that refusal is not an error in the tool. +- **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 8bd9f85f1e..835d304c14 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -1951,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 @@ -2012,6 +2023,18 @@ export class ToolHandler { 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; @@ -2200,11 +2223,13 @@ 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 (toolName === 'codegraph_explore' && project.isWatcherDegraded?.()) { - const emission = raw[EXPLORE_EMISSION_KEY]; - const stalePaths = emission?.files - ?.filter(file => this.isFileStaleOnDisk(project, file.path, undefined, true)) - .map(file => file.path) ?? []; + 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.