From 0f6fdd981c231906d089e2d0c2b8fed159d10235 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sat, 26 Sep 2026 10:36:19 +0300 Subject: [PATCH 1/6] fix(lock): keep a live writer lock regardless of file age (#1959) --- __tests__/security.test.ts | 17 +++++++++++++++++ src/utils.ts | 13 ++++--------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/__tests__/security.test.ts b/__tests__/security.test.ts index 9aa3c9597c..30e845c182 100644 --- a/__tests__/security.test.ts +++ b/__tests__/security.test.ts @@ -66,6 +66,23 @@ describe('FileLock', () => { lock1.release(); }); + it('keeps a live holder protected after its lock file is older than two minutes (#1959)', () => { + const holder = new FileLock(lockPath); + const contender = new FileLock(lockPath); + holder.acquire(); + const old = new Date(Date.now() - 3 * 60_000); + fs.utimesSync(lockPath, old, old); + + try { + expect(() => contender.acquire()).toThrow(/locked by another process/); + expect(fs.readFileSync(lockPath, 'utf8')).toBe(String(process.pid)); + } finally { + holder.release(); + contender.release(); + } + expect(fs.existsSync(lockPath)).toBe(false); + }); + it('should detect and remove stale locks from dead processes', () => { // Write a lock file with a PID that doesn't exist // PID 99999999 is extremely unlikely to be a real process diff --git a/src/utils.ts b/src/utils.ts index dc860a3f49..cdc46e831f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -241,9 +241,6 @@ export class FileLock { private lockPath: string; private held = false; - /** Locks older than this are considered stale regardless of PID status */ - private static readonly STALE_TIMEOUT_MS = 2 * 60 * 1000; // 2 minutes - constructor(lockPath: string) { this.lockPath = lockPath; } @@ -257,18 +254,16 @@ export class FileLock { try { const content = fs.readFileSync(this.lockPath, 'utf-8').trim(); const pid = parseInt(content, 10); - const stat = fs.statSync(this.lockPath); - const lockAge = Date.now() - stat.mtimeMs; - - // Treat locks older than the timeout as stale, regardless of PID - if (lockAge < FileLock.STALE_TIMEOUT_MS && !isNaN(pid) && this.isProcessAlive(pid)) { + // A long sync can hold the lock for hours. Its age says nothing about + // whether the writer is still alive; only a dead PID is stale. + if (!isNaN(pid) && this.isProcessAlive(pid)) { throw new Error( `CodeGraph database is locked by another process (PID ${pid}). ` + `If this is stale, run 'codegraph unlock' or delete ${this.lockPath}` ); } - // Stale lock (dead process or timed out) - remove it + // Stale lock (dead process or malformed PID) - remove it fs.unlinkSync(this.lockPath); } catch (err) { if (err instanceof Error && err.message.includes('locked by another')) { From a5c4532b480a8da277cbab76e01ced1a1bd87c24 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sat, 26 Sep 2026 11:03:09 +0300 Subject: [PATCH 2/6] fix(watcher): re-arm after lock contention and reconcile before trusting index (#1959) --- __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 +++++++++++++++++++++++--- 7 files changed, 195 insertions(+), 19 deletions(-) diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 3a8f0345ec..46a3cc3d54 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -8593,6 +8593,29 @@ describe('Nested non-submodule git repos', () => { expect(ig.ignores('scratch/tmp.ts')).toBe(true); }); + it('filesystem fallback retains git info/exclude and core.excludesFile when ls-files fails (#1959)', async () => { + const { execFileSync } = await import('child_process'); + const root = path.join(tempDir, 'fallback-excludes-root'); + fs.mkdirSync(root, { recursive: true }); + execFileSync('git', ['init', '-q'], { cwd: root, stdio: 'pipe' }); + const globalExcludes = path.join(tempDir, 'fallback-global-excludes'); + fs.writeFileSync(globalExcludes, 'scratch/\n'); + execFileSync('git', ['config', 'core.excludesFile', globalExcludes], { cwd: root, stdio: 'pipe' }); + fs.writeFileSync(path.join(root, '.git', 'info', 'exclude'), 'worktrees/\n'); + fs.mkdirSync(path.join(root, 'scratch')); + fs.mkdirSync(path.join(root, 'worktrees')); + fs.writeFileSync(path.join(root, 'app.ts'), 'export const app = 1;\n'); + fs.writeFileSync(path.join(root, 'scratch', 'hidden.ts'), 'export const hidden = 1;\n'); + fs.writeFileSync(path.join(root, 'worktrees', 'hidden.ts'), 'export const hidden = 2;\n'); + + // rev-parse/config still work, but both ls-files and status fail as they + // would under a Git timeout. This exercises the real filesystem walk. + fs.writeFileSync(path.join(root, '.git', 'index'), 'not a git index'); + expect(() => execFileSync('git', ['ls-files'], { cwd: root, stdio: 'pipe' })).toThrow(); + expect(scanDirectory(root)).toEqual(['app.ts']); + expect(await scanDirectoryAsync(root)).toEqual(['app.ts']); + }); + it('buildScopeIgnore prunes dirs ignored only by a nested .gitignore (#1728)', async () => { const { execFileSync } = await import('child_process'); const git = (cwd: string, ...args: string[]) => diff --git a/__tests__/mcp-staleness-banner.test.ts b/__tests__/mcp-staleness-banner.test.ts index f72f456e03..fb72cdf5e8 100644 --- a/__tests__/mcp-staleness-banner.test.ts +++ b/__tests__/mcp-staleness-banner.test.ts @@ -20,7 +20,7 @@ * left untouched. */ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; @@ -72,6 +72,7 @@ describe('MCP staleness banner', () => { afterEach(() => { __setFsWatchForTests(null); // reset the injected fs.watch seam + vi.restoreAllMocks(); try { cg.unwatch(); } catch { /* ignore */ } try { cg.close(); } catch { /* ignore */ } if (fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true }); @@ -209,6 +210,26 @@ describe('MCP staleness banner', () => { // status renders the notice inline, so the auto-banner is not also prepended. expect(text.startsWith('⚠️')).toBe(false); }); + + it('distinguishes a re-armed but not-yet-caught-up watcher from a disabled one (#1959)', async () => { + vi.spyOn(cg, 'isWatcherDegraded').mockReturnValue(true); + vi.spyOn(cg, 'isWatcherRecovering').mockReturnValue(true); + + const search = await handler.execute('codegraph_search', { query: 'alphaOnly' }); + expect(search.content[0].text).toMatch(/auto-sync is RECOVERING/); + expect(search.content[0].text).not.toMatch(/auto-sync is DISABLED/); + + const status = await handler.execute('codegraph_status', {}); + expect(status.content[0].text).toContain('**Auto-sync recovering:**'); + expect(status.content[0].text).not.toContain('**Auto-sync disabled:**'); + }); + + it('asks the owned watcher to re-arm on the next MCP tool call (#1959)', async () => { + const rearm = vi.spyOn(cg, 'rearmWatcherAfterLockContention').mockReturnValue(false); + + await handler.execute('codegraph_status', {}); + expect(rearm).toHaveBeenCalledTimes(1); + }); }); describe('MCP staleness banner — matching whole paths (#1968)', () => { diff --git a/__tests__/watcher.test.ts b/__tests__/watcher.test.ts index b2285e45ad..6af1467a8f 100644 --- a/__tests__/watcher.test.ts +++ b/__tests__/watcher.test.ts @@ -344,6 +344,53 @@ describe('FileWatcher', () => { watcher.stop(); }); + + it('re-arms after lock contention and keeps the stale banner until a full catch-up (#1959)', async () => { + let finishCatchUp!: () => void; + const catchUp = new Promise(resolve => { finishCatchUp = resolve; }); + const syncFn = vi.fn().mockRejectedValue(new LockUnavailableError()); + const watcher = newWatcher(syncFn, { debounceMs: 25 }); + watcher.start(); + try { + await watcher.waitUntilReady(); + __emitWatchEventForTests(testDir, 'src/locked.ts'); + await waitFor(() => watcher.isDegraded() && !watcher.isActive(), 8000); + + syncFn.mockImplementation(async () => { + await catchUp; + return { filesChanged: 1, durationMs: 5 }; + }); + expect(watcher.rearmAfterLockContention()).toBe(true); + expect(watcher.isActive()).toBe(true); + expect(watcher.isDegraded()).toBe(true); + expect(watcher.rearmAfterLockContention()).toBe(false); + await waitFor(() => syncFn.mock.calls.length >= 7, 4000); + expect(syncFn.mock.calls.at(-1)?.[0]).toBeUndefined(); + expect(watcher.isDegraded()).toBe(true); + finishCatchUp(); + await waitFor(() => !watcher.isDegraded(), 4000); + expect(watcher.isActive()).toBe(true); + } finally { + finishCatchUp?.(); + watcher.stop(); + } + }); + + it('throttles a failed re-arm across tool calls (#1959)', async () => { + const syncFn = vi.fn().mockRejectedValue(new LockUnavailableError()); + const watcher = newWatcher(syncFn, { debounceMs: 25 }); + watcher.start(); + try { + await watcher.waitUntilReady(); + __emitWatchEventForTests(testDir, 'src/locked.ts'); + await waitFor(() => watcher.isDegraded() && !watcher.isActive(), 8000); + expect(watcher.rearmAfterLockContention()).toBe(true); + await waitFor(() => watcher.isDegraded() && !watcher.isActive(), 8000); + expect(watcher.rearmAfterLockContention()).toBe(false); + } finally { + watcher.stop(); + } + }); }); describe('persistent sync-failure degradation (#1127)', () => { @@ -371,6 +418,7 @@ describe('FileWatcher', () => { expect(syncFn.mock.calls.length).toBeGreaterThanOrEqual(6); // MAX_SYNC_FAILURE_RETRIES + 1 expect(watcher.isDegraded()).toBe(true); + expect(watcher.rearmAfterLockContention()).toBe(false); expect(onDegraded).toHaveBeenCalledTimes(1); expect(onDegraded).toHaveBeenCalledWith(expect.stringContaining('auto-sync disabled')); // The degrade reason carries the underlying error so the user can act. diff --git a/src/index.ts b/src/index.ts index 1a7c5814c0..62f0a84e82 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1144,12 +1144,11 @@ export class CodeGraph { } /** - * True once live watching has permanently degraded (OS watch-resource - * exhaustion, or a write lock held past the retry budget) and auto-sync is - * disabled until the next {@link watch} call. Distinct from `!isWatching()`: - * a stopped/never-started watcher is inactive but NOT degraded. MCP tools use - * this to surface a whole-index "results may be stale" notice, since - * `getPendingFiles()` goes empty once watching stops (#876). + * True once live watching has degraded, or while a re-armed watcher has not + * completed its full catch-up. Distinct from `!isWatching()`: a stopped or + * never-started watcher is inactive but NOT degraded. MCP tools use this for + * a whole-index stale notice, since pending files are lost when watching + * stops (#876, #1959). */ isWatcherDegraded(): boolean { return this.watcher?.isDegraded() ?? false; @@ -1160,6 +1159,16 @@ export class CodeGraph { return this.watcher?.getDegradedReason() ?? null; } + /** Re-arm a lock-degraded watcher; its stale state persists until a full sync. */ + rearmWatcherAfterLockContention(): boolean { + return this.watcher?.rearmAfterLockContention() ?? false; + } + + /** True while a re-armed watcher owes a full reconcile of missed changes. */ + isWatcherRecovering(): boolean { + return this.watcher?.isRecoveringFromLock() ?? false; + } + /** * Files seen by the file watcher since the last successful sync — * the per-file "stale" signal MCP tools attach to responses so an agent diff --git a/src/mcp/server-instructions.ts b/src/mcp/server-instructions.ts index 324a73f2d5..ab762f839c 100644 --- a/src/mcp/server-instructions.ts +++ b/src/mcp/server-instructions.ts @@ -64,7 +64,7 @@ calls; a grep/read exploration is dozens. - **Trust codegraph's results — don't re-verify them with grep.** They come from a full AST parse; re-checking with grep is slower, less accurate, and wastes context. - **Don't grep or Read first** to find or understand indexed code — ONE \`codegraph_explore\` returns the relevant symbols' source together in a single round-trip. Reach for raw \`Read\`/\`Grep\` only to confirm a specific detail codegraph didn't cover, or for what codegraph doesn't index (configs, docs). - **Don't reconstruct a flow by hand** — name the endpoints in one \`codegraph_explore\` and it surfaces the path between them, dynamic-dispatch hops included. -- **After editing, check the staleness banner.** When a tool response starts with "⚠️ Some files referenced below were edited since the last index sync…", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. A different, rarer banner — "⚠️ CodeGraph auto-sync is DISABLED…" — means live watching stopped entirely (the whole index is frozen, not just a few files); until it's resolved, Read files directly to confirm anything that may have changed. +- **After editing, check the staleness banner.** When a tool response starts with "⚠️ Some files referenced below were edited since the last index sync…", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. A different, rarer banner — "⚠️ CodeGraph auto-sync is DISABLED…" or "⚠️ CodeGraph auto-sync is RECOVERING…" — means the whole index may be stale; until the full catch-up completes, Read files directly to confirm anything that may have changed. - **A file flagged "⚠ changed on disk after the last index sync" drifted from its index** (most common on projects queried via \`projectPath\`, which have no live watcher). Codegraph never serves a possibly-mis-sliced body from such a file — it either shows the file's full CURRENT source (trust it as a Read) or omits the source with this flag. When the source was omitted, Read that specific file; line numbers referencing it elsewhere in the response may be shifted until that project's next sync. All unflagged files remain trustworthy. - **Source is re-served on every call by default**, including for fresh subagents and after context compaction. Cross-call dedup requires \`CODEGRAPH_EXPLORE_DEDUP=1\` and is only suitable for hosts that guarantee one durable context per connection. With that opt-in, **"Already sent earlier in this conversation"** points to exact, unchanged source returned by an earlier \`codegraph_explore\` in that context. Use that copy; don't re-fetch it and don't Read the file. The bytes it freed went into source you have not seen yet, elsewhere in the same response. diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 33a2003abf..2c48f4f40c 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -1114,6 +1114,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 */ @@ -2190,6 +2199,10 @@ export class ToolHandler { if (degraded) { const [head, ...tail] = result.content; if (!head || head.type !== 'text') return result; + if (cg.isWatcherRecovering?.()) { + const composed = `${formatRecoveringBanner()}\n\n${head.text}`; + return { ...result, content: [{ type: 'text', text: composed }, ...tail] }; + } let reason: string | null = null; try { reason = cg.getWatcherDegradedReason?.() ?? null; @@ -2298,6 +2311,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 @@ -6843,11 +6864,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 74257cc58f..0d1b13148c 100644 --- a/src/sync/watcher.ts +++ b/src/sync/watcher.ts @@ -48,6 +48,8 @@ import { watchDisabledReason } from './watch-policy'; * few cycles) stays under this; a long-lived external writer crosses it. */ const MAX_LOCK_RETRIES = 5; +/** A failed re-arm must not turn frequent MCP calls into a lock-polling loop. */ +const LOCK_REARM_COOLDOWN_MS = 30_000; /** * Number of consecutive GENERIC (non-lock) sync failures the watcher tolerates * before it degrades auto-sync. A deterministic failure — a tree-sitter @@ -278,12 +280,16 @@ export class FileWatcher { */ private inotifyLimitWarned = false; /** - * One-way latch: the reason live watching was permanently disabled at runtime + * The reason live watching was disabled at runtime * (watch-resource exhaustion, lock contention past the retry budget, or a * persistent generic sync failure past the retry budget), or null while - * healthy. Set by {@link degrade}; cleared only by a fresh start(). + * healthy. Set by {@link degrade}; a lock-contention recovery keeps it until + * a full reconciliation succeeds. */ private degradedReason: string | null = null; + private degradedByLock = false; + private recoveringFromLock = false; + private lastLockRearmMs = 0; /** Consecutive lock-contention retries for watcher-triggered syncs. */ private lockRetryCount = 0; /** Consecutive generic (non-lock) sync failures; reset only by a clean sync. */ @@ -380,6 +386,8 @@ export class FileWatcher { if (this.recursiveWatcher || this.dirWatchers.size > 0 || this.inert) return true; // Already watching this.stopped = false; this.degradedReason = null; + this.degradedByLock = false; + this.recoveringFromLock = false; this.lockRetryCount = 0; this.syncFailureRetryCount = 0; this.needsFullScan = false; @@ -518,7 +526,7 @@ export class FileWatcher { } private watchDirectory(dir: string, real: string, recursive: boolean): void { - if (this.stopped || this.degradedReason || this.inotifyLimitWarned) return; + if (this.stopped || (this.degradedReason && !this.recoveringFromLock) || this.inotifyLimitWarned) return; if (this.dirWatchers.has(dir)) return; // Recursive streams cost more than inotify entries. Keep a separate hard // ceiling while honouring a lower user-configured directory watch budget. @@ -745,15 +753,17 @@ export class FileWatcher { } /** - * Permanently disable live watching after a terminal runtime failure + * Disable live watching after a terminal runtime failure * (watch-resource exhaustion, lock contention past the retry budget, or a * persistent generic sync failure past the retry budget). * Idempotent: logs one actionable warning, fires {@link WatchOptions.onDegraded} * once, and stops the watcher. A subsequent start() clears the latch. */ - private degrade(reason: string, context: Record = {}): void { - if (this.degradedReason) return; + private degrade(reason: string, context: Record = {}, byLock = false): void { + if (this.degradedReason && !this.recoveringFromLock) return; this.degradedReason = reason; + this.degradedByLock = byLock; + this.recoveringFromLock = false; logWarn('File watcher disabled', { projectRoot: this.projectRoot, reason, ...context }); this.onDegraded?.(reason); this.stop(); @@ -774,7 +784,7 @@ export class FileWatcher { } /** - * Whether live watching has degraded permanently (until the next start()). + * Whether live watching has degraded or is still catching up after re-arm. * Distinct from {@link isActive}: a degraded watcher is inactive, but an * inactive watcher is not necessarily degraded (it may simply be stopped or * never started). Hosts use this to tell the user auto-sync is off. @@ -788,6 +798,41 @@ export class FileWatcher { return this.degradedReason; } + /** Watches are live again, but the full catch-up has not committed yet. */ + isRecoveringFromLock(): boolean { + return this.recoveringFromLock; + } + + /** + * Re-arm a watcher disabled by lock contention when a caller next uses the + * graph. Keep the whole-index stale banner until a FULL scan reconciles edits + * missed while watches were off. Resource exhaustion and deterministic sync + * failures are not automatically retried. Repeated unsuccessful re-arms are + * throttled so many MCP sessions cannot hammer a long-lived writer. + */ + rearmAfterLockContention(): boolean { + if (!this.degradedByLock || this.isActive()) return false; + const now = Date.now(); + if (this.lastLockRearmMs && now - this.lastLockRearmMs < LOCK_REARM_COOLDOWN_MS) return false; + const reason = this.degradedReason; + this.lastLockRearmMs = now; + if (!this.start()) { + // A disabled platform can reject start() without calling degrade(). + // Preserve the old stale-index signal in that case; a new resource + // failure has its own more precise degraded reason. + if (!this.degradedReason) { + this.degradedReason = reason; + this.degradedByLock = true; + } + return false; + } + this.degradedReason = reason; + this.recoveringFromLock = true; + this.needsFullScan = true; + this.scheduleSync(); + return true; + } + /** * Stop watching for file changes. */ @@ -797,6 +842,7 @@ export class FileWatcher { this.treeRefreshTimer = null; this.knownDirs.clear(); this.watchedRealDirs.clear(); + this.recoveringFromLock = false; if (this.debounceTimer) { clearTimeout(this.debounceTimer); @@ -947,6 +993,10 @@ export class FileWatcher { if (!scoped) this.needsFullScan = false; try { const result = await this.syncFn(scoped); + if (this.recoveringFromLock && !scoped && !this.needsFullScan) { + this.degradedReason = null; + this.recoveringFromLock = false; + } this.lockRetryCount = 0; // a clean sync clears any contention backoff this.syncFailureRetryCount = 0; // ...and any generic-failure backoff // Remove entries whose most recent event predates this sync — those @@ -980,7 +1030,8 @@ export class FileWatcher { 'CodeGraph file lock held by another process past the retry budget; ' + 'auto-sync disabled. Run `codegraph sync` once the other writer finishes ' + '(or install git sync hooks) to refresh the graph.', - { pendingFiles: this.pendingFiles.size, retryCount: this.lockRetryCount } + { pendingFiles: this.pendingFiles.size, retryCount: this.lockRetryCount }, + true ); } } else { From 37b0314f81cfb7d16877c11ee7f55effd0cf76f5 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sat, 26 Sep 2026 14:27:24 +0300 Subject: [PATCH 3/6] feat(mcp): report exact index freshness without blocking the daemon (#1959) --- __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 ++++++- 4 files changed, 157 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/__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 2c48f4f40c..eec9e3b6df 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -50,6 +50,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, @@ -1402,7 +1403,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: { @@ -6805,6 +6806,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 a3face0f462b9c75c4c029f6f9049fa88f00903f Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sat, 26 Sep 2026 14:35:39 +0300 Subject: [PATCH 4/6] fix(mcp): refuse drifted explore source after watcher degradation (#1959) --- __tests__/mcp-stale-refusal.test.ts | 71 +++++++++++++++++++++++++++++ src/mcp/server-instructions.ts | 2 +- src/mcp/tools.ts | 25 ++++++++-- 3 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 __tests__/mcp-stale-refusal.test.ts 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 ab762f839c..a332947d08 100644 --- a/src/mcp/server-instructions.ts +++ b/src/mcp/server-instructions.ts @@ -64,7 +64,7 @@ calls; a grep/read exploration is dozens. - **Trust codegraph's results — don't re-verify them with grep.** They come from a full AST parse; re-checking with grep is slower, less accurate, and wastes context. - **Don't grep or Read first** to find or understand indexed code — ONE \`codegraph_explore\` returns the relevant symbols' source together in a single round-trip. Reach for raw \`Read\`/\`Grep\` only to confirm a specific detail codegraph didn't cover, or for what codegraph doesn't index (configs, docs). - **Don't reconstruct a flow by hand** — name the endpoints in one \`codegraph_explore\` and it surfaces the path between them, dynamic-dispatch hops included. -- **After editing, check the staleness banner.** When a tool response starts with "⚠️ Some files referenced below were edited since the last index sync…", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. A different, rarer banner — "⚠️ CodeGraph auto-sync is DISABLED…" 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 eec9e3b6df..93e5499c4a 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -2124,7 +2124,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(); @@ -2134,7 +2134,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); @@ -2143,7 +2143,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 @@ -2151,9 +2151,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; @@ -2358,6 +2360,21 @@ export class ToolHandler { const raw = (pooled && projectPath) ? await this.queryPool!.run(toolName, { ...dispatchArgs, projectPath }) : 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 f2b462d2efbb6c707818c9640981f9622d0713b5 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sat, 26 Sep 2026 15:53:03 +0300 Subject: [PATCH 5/6] 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 --- __tests__/mcp-stale-refusal.test.ts | 29 +++++++++++++++++++++++- src/mcp/server-instructions.ts | 2 +- src/mcp/tools.ts | 35 ++++++++++++++++++++++++----- 3 files changed, 59 insertions(+), 7 deletions(-) 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 a332947d08..0234f5e6a6 100644 --- a/src/mcp/server-instructions.ts +++ b/src/mcp/server-instructions.ts @@ -64,7 +64,7 @@ calls; a grep/read exploration is dozens. - **Trust codegraph's results — don't re-verify them with grep.** They come from a full AST parse; re-checking with grep is slower, less accurate, and wastes context. - **Don't grep or Read first** to find or understand indexed code — ONE \`codegraph_explore\` returns the relevant symbols' source together in a single round-trip. Reach for raw \`Read\`/\`Grep\` only to confirm a specific detail codegraph didn't cover, or for what codegraph doesn't index (configs, docs). - **Don't reconstruct a flow by hand** — name the endpoints in one \`codegraph_explore\` and it surfaces the path between them, dynamic-dispatch hops included. -- **After editing, check the staleness banner.** When a tool response starts with "⚠️ Some files referenced below were edited since the last index sync…", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. A different, rarer banner — "⚠️ CodeGraph auto-sync is DISABLED…" 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 93e5499c4a..44d19e3d0a 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -2100,6 +2100,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 @@ -2161,6 +2172,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; @@ -2360,11 +2383,13 @@ export class ToolHandler { const raw = (pooled && projectPath) ? await this.queryPool!.run(toolName, { ...dispatchArgs, projectPath }) : 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. From 12b393f56a0c6f564768ee109e2d57c6d0b16e10 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Sun, 27 Sep 2026 09:12:55 -0500 Subject: [PATCH 6/6] fix(sync): recover index freshness after lock contention (#1959) Keep a live writer's lock regardless of its age. Re-arm lock-degraded watchers with throttling and retain stale state until full reconciliation succeeds. Validate contributing files through structured paths and hashes, refusing incomplete checks without tool errors. Report bounded freshness measurements and cover Git fallback exclusions. Co-authored-by: danusha2345 Co-authored-by: danusha2345 Co-Authored-By: Claude Opus 5.5 --- __tests__/mcp-freshness-bounds.test.ts | 91 +++++++++++++++++ __tests__/mcp-stale-refusal.test.ts | 60 +++++++++++- __tests__/mcp-status-freshness.test.ts | 8 +- __tests__/security.test.ts | 1 + src/mcp/answer-freshness.ts | 56 +++++++++++ src/mcp/index-freshness.ts | 8 +- src/mcp/server-instructions.ts | 2 +- src/mcp/tools.ts | 130 ++++++++++++------------- 8 files changed, 283 insertions(+), 73 deletions(-) create mode 100644 __tests__/mcp-freshness-bounds.test.ts create mode 100644 src/mcp/answer-freshness.ts diff --git a/__tests__/mcp-freshness-bounds.test.ts b/__tests__/mcp-freshness-bounds.test.ts new file mode 100644 index 0000000000..5710b4e137 --- /dev/null +++ b/__tests__/mcp-freshness-bounds.test.ts @@ -0,0 +1,91 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { createHash } from 'crypto'; +import { Readable } from 'stream'; +import { validateAnswerFiles } from '../src/mcp/answer-freshness'; +import { measurePendingChanges } from '../src/mcp/index-freshness'; + +vi.mock('fs', async importOriginal => { + const actual = await importOriginal(); + return { ...actual, createReadStream: vi.fn(actual.createReadStream) }; +}); + +const workers = vi.hoisted(() => [] as any[]); +vi.mock('worker_threads', async () => { + const { EventEmitter } = await import('events'); + return { Worker: class extends EventEmitter { + terminate = vi.fn(async () => 0); + constructor() { super(); workers.push(this); } + } }; +}); + +describe('bounded freshness validation (#1959)', () => { + let root: string | undefined; + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + workers.length = 0; + if (root) fs.rmSync(root, { recursive: true, force: true }); + root = undefined; + }); + + it('marks a file beyond the byte budget unchecked instead of fresh', async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-freshness-bounds-')); + const content = 'x'.repeat(9 * 1024 * 1024); + fs.writeFileSync(path.join(root, 'large.ts'), content); + const result = await validateAnswerFiles(root, [{ + path: 'large.ts', contentHash: createHash('sha256').update(content).digest('hex'), + }]); + expect(result).toEqual({ stale: [], unchecked: ['large.ts'] }); + }); + + it('bounds a stuck measurement, shares concurrent probes, and terminates its worker', async () => { + vi.useFakeTimers(); + const pending = measurePendingChanges('/freshness-timeout'); + expect(measurePendingChanges('/freshness-timeout')).toBe(pending); + expect(workers).toHaveLength(1); + await vi.advanceTimersByTimeAsync(8000); + expect(await pending).toBeNull(); + expect(workers[0].terminate).toHaveBeenCalledOnce(); + }); + + it('returns unknown on deadline even if worker termination is delayed', async () => { + vi.useFakeTimers(); + const pending = measurePendingChanges('/freshness-delayed-exit'); + let release!: () => void; + workers[0].terminate.mockImplementation(() => new Promise(resolve => { release = resolve; })); + await vi.advanceTimersByTimeAsync(8000); + expect(await pending).toBeNull(); + release(); + await Promise.resolve(); + }); + + it('aborts stalled file reads and reports all remaining paths as unchecked', async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-freshness-deadline-')); + fs.writeFileSync(path.join(root, 'stalled.ts'), 'export const x = 1;'); + vi.mocked(fs.createReadStream).mockImplementationOnce((_file, options) => new Readable({ + read() {}, signal: (options as { signal: AbortSignal }).signal, + }) as fs.ReadStream); + const result = await validateAnswerFiles(root, [ + { path: 'stalled.ts', contentHash: 'hash' }, + { path: 'next.ts', contentHash: 'hash' }, + ]); + expect(result).toEqual({ stale: [], unchecked: ['stalled.ts', 'next.ts'] }); + }); + + it('caps measurements across projects and releases the slots on failure', async () => { + const first = measurePendingChanges('/freshness-one'); + const second = measurePendingChanges('/freshness-two'); + expect(await measurePendingChanges('/freshness-three')).toBeNull(); + expect(workers).toHaveLength(2); + workers[0].emit('error', new Error('worker failed')); + workers[1].emit('message', { added: -1, modified: 0, removed: 0 }); + expect(await first).toBeNull(); + expect(await second).toBeNull(); + const retry = measurePendingChanges('/freshness-three'); + workers[2].emit('message', { added: 0, modified: 1, removed: 0 }); + expect(await retry).toEqual({ added: 0, modified: 1, removed: 0 }); + }); +}); diff --git a/__tests__/mcp-stale-refusal.test.ts b/__tests__/mcp-stale-refusal.test.ts index 929ca1a7e7..53362c7a29 100644 --- a/__tests__/mcp-stale-refusal.test.ts +++ b/__tests__/mcp-stale-refusal.test.ts @@ -1,19 +1,24 @@ -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import CodeGraph from '../src/index'; import { ExploreSessionState } from '../src/mcp/explore-session-state'; import { ToolHandler } from '../src/mcp/tools'; +import { QueryPool } from '../src/mcp/query-pool'; +import { Worker } from 'worker_threads'; import { __setFsWatchForTests } from '../src/sync/watcher'; describe('a degraded index refuses answers from changed files (#1959)', () => { let root: string; let cg: CodeGraph; let handler: ToolHandler; + let pool: QueryPool | undefined; beforeEach(async () => { root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-stale-refusal-')); + fs.mkdirSync(path.join(root, 'folder with spaces'), { recursive: true }); + fs.writeFileSync(path.join(root, 'folder with spaces/目录 🦀.ts'), 'export function unicodeSymbol() { return 1; }\n'); fs.writeFileSync(path.join(root, 'alpha.ts'), 'export function alphaOnly() { return 1; }\n'); fs.writeFileSync(path.join(root, 'beta.ts'), 'export function betaOnly() { return 2; }\n'); fs.writeFileSync( @@ -34,7 +39,10 @@ describe('a degraded index refuses answers from changed files (#1959)', () => { __setFsWatchForTests(null); }); - afterEach(() => { + afterEach(async () => { + await pool?.destroy(); + pool = undefined; + await handler.closeAll(); __setFsWatchForTests(null); try { cg.unwatch(); } catch { /* ignore */ } try { cg.close(); } catch { /* ignore */ } @@ -95,4 +103,52 @@ describe('a degraded index refuses answers from changed files (#1959)', () => { expect(unaffected.content[0].text).toContain('beta.ts'); expect(unaffected.content[0].text).not.toContain('cannot answer from this index'); }); + it.each([false, true])('preserves spaces and Unicode through provenance (pooled=%s)', async pooled => { + if (pooled) { + pool = new QueryPool({ root, size: 1, createWorker: () => new Worker( + path.resolve(__dirname, '../dist/mcp/query-worker.js'), { workerData: { root } }, + ) }); + handler.setQueryPool(pool); + await vi.waitFor(() => expect(pool!.ready).toBe(true), { timeout: 15000 }); + } + const relative = 'folder with spaces/目录 🦀.ts'; + const initial = await handler.execute('codegraph_search', { query: 'unicodeSymbol' }); + expect(initial.content[0].text).toContain(relative); + expect(initial).not.toHaveProperty('_cgAnswerFiles'); + const before = fs.statSync(path.join(root, relative)); + fs.writeFileSync(path.join(root, relative), 'export function unicodeSymbol() { return 9; }\n'); + fs.utimesSync(path.join(root, relative), before.atime, before.mtime); + const result = await handler.execute('codegraph_search', { query: 'unicodeSymbol' }); + expect(result.isError).toBeFalsy(); + expect(result.content[0].text).toContain('cannot answer from this index'); + expect(result.structuredContent).toEqual({ freshness: { stale: [relative], unchecked: [] } }); + expect(result).not.toHaveProperty('_cgAnswerFiles'); + }); + + it.each(['codegraph_search', 'codegraph_explore', 'codegraph_callers', 'codegraph_callees', 'codegraph_impact'])( + 'refuses deleted contributing files in %s', async tool => { + fs.unlinkSync(path.join(root, 'alpha.ts')); + const result = await handler.execute(tool, { query: 'alphaOnly', symbol: 'alphaOnly' }); + expect(result.isError).toBeFalsy(); + expect(result.content[0].text).toContain('cannot answer from this index'); + expect(result.content[0].text).toContain('alpha.ts'); + expect(result).not.toHaveProperty('_cgExploreEmission'); + }, + ); + + it('refuses rather than silently validating only the first 200 contributing files', async () => { + for (let i = 0; i < 201; i++) { + fs.writeFileSync(path.join(root, `caller${i}.ts`), + `import { alphaOnly } from './alpha'; export function caller${i}() { return alphaOnly(); }\n`); + } + await cg.indexAll(); + const raw = await handler.executeReadTool('codegraph_impact', { symbol: 'alphaOnly' }); + expect(raw._cgAnswerFiles!.length).toBeGreaterThan(200); + const result = await handler.execute('codegraph_impact', { symbol: 'alphaOnly' }); + expect(result.isError).toBeFalsy(); + expect(result.content[0].text).toContain('validation budget'); + expect((result.structuredContent!.freshness as { unchecked: string[] }).unchecked.length).toBeGreaterThan(0); + expect(result.content[0].text).not.toContain('**Impact'); + }); + }); diff --git a/__tests__/mcp-status-freshness.test.ts b/__tests__/mcp-status-freshness.test.ts index 9c7106ab37..19c14f7ce1 100644 --- a/__tests__/mcp-status-freshness.test.ts +++ b/__tests__/mcp-status-freshness.test.ts @@ -22,7 +22,7 @@ describe('MCP status freshness (#1959)', () => { 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: [] } }); + cg = CodeGraph.initSync(root); await cg.indexAll(); handler = new ToolHandler(cg); }); @@ -41,7 +41,11 @@ describe('MCP status freshness (#1959)', () => { 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; + const result = await handler.execute('codegraph_status', {}); + expect(result.structuredContent).toEqual({ freshness: { + lastIndexedAt: cg.getLastIndexedAt(), changes: { added: 1, modified: 1, removed: 1 }, complete: true, + } }); + const changed = result.content[0].text; expect(changed).toContain('**Changes since index:** 1 added, 1 modified, 1 removed'); }); diff --git a/__tests__/security.test.ts b/__tests__/security.test.ts index 30e845c182..8cc9d7aa12 100644 --- a/__tests__/security.test.ts +++ b/__tests__/security.test.ts @@ -425,6 +425,7 @@ describe('MCP Input Validation', () => { })); const fakeCg = { searchNodes: () => many, + getFile: () => undefined, // Search down-ranks generated files, and since #1500 that verdict comes // from the index (path convention ∪ content banner) rather than the // filename alone. No database here — none of these paths is generated. diff --git a/src/mcp/answer-freshness.ts b/src/mcp/answer-freshness.ts new file mode 100644 index 0000000000..297eab4728 --- /dev/null +++ b/src/mcp/answer-freshness.ts @@ -0,0 +1,56 @@ +import { createHash } from 'crypto'; +import { createReadStream } from 'fs'; +import { validatePathWithinRoot } from '../utils'; + +export interface AnswerFile { + path: string; + /** The indexed version used to build the answer, even across a worker hop. */ + contentHash: string | null; +} + +/** Limits apply to the entire answer; exceeding any of them refuses it. */ +const MAX_FILES = 200; +const MAX_BYTES = 8 * 1024 * 1024; +const MAX_MS = 250; + +export async function validateAnswerFiles(root: string, files: AnswerFile[]): Promise<{ + stale: string[]; + unchecked: string[]; +}> { + const stale: string[] = []; + const unchecked: string[] = []; + const signal = AbortSignal.timeout(MAX_MS); + let bytes = 0; + for (let i = 0; i < files.length; i++) { + const file = files[i]!; + if (i >= MAX_FILES || bytes >= MAX_BYTES || signal.aborted) { + unchecked.push(...files.slice(i).map(f => f.path)); + break; + } + if (!file.contentHash) { + unchecked.push(file.path); + continue; + } + try { + const hash = createHash('sha256'); + const absolute = validatePathWithinRoot(root, file.path); + if (!absolute) { stale.push(file.path); continue; } + const stream = createReadStream(absolute, { + encoding: 'utf8', highWaterMark: 64 * 1024, signal, + }); + let complete = true; + for await (const chunk of stream) { + bytes += Buffer.byteLength(chunk); + if (bytes > MAX_BYTES) { complete = false; break; } + hash.update(chunk); + } + if (!complete) unchecked.push(file.path); + else if (hash.digest('hex') !== file.contentHash) stale.push(file.path); + } catch { + // Timeout is unknown, never a clean bill of health. A missing or + // unreadable contributing file cannot support the indexed answer. + (signal.aborted ? unchecked : stale).push(file.path); + } + } + return { stale, unchecked }; +} diff --git a/src/mcp/index-freshness.ts b/src/mcp/index-freshness.ts index eaf40db7c0..ed6c66b02f 100644 --- a/src/mcp/index-freshness.ts +++ b/src/mcp/index-freshness.ts @@ -10,12 +10,15 @@ export interface PendingChangeCounts { /** `getChangedFiles()` may fall back to a huge filesystem scan (#1959). */ const MEASURE_TIMEOUT_MS = 8_000; +let liveMeasurements = 0; const active = new Map>(); export function measurePendingChanges(root: string): Promise { const key = path.resolve(root); const existing = active.get(key); if (existing) return existing; + // Status probes for many projects must not exhaust the shared daemon. + if (liveMeasurements >= 2) return Promise.resolve(null); const pending = runMeasurement(key).finally(() => active.delete(key)); active.set(key, pending); @@ -38,14 +41,17 @@ function runMeasurement(root: string): Promise { return Promise.resolve(null); } + liveMeasurements++; return new Promise(resolve => { let settled = false; const finish = (counts: PendingChangeCounts | null) => { if (settled) return; settled = true; clearTimeout(timer); + // A worker inside a synchronous Git call may take time to terminate. + // Return unknown on deadline, but keep its concurrency slot until exit. + void worker.terminate().catch(() => {}).finally(() => { liveMeasurements--; }); resolve(counts); - void worker.terminate().catch(() => {}); }; const timer = setTimeout(() => finish(null), MEASURE_TIMEOUT_MS); worker.once('message', (value: PendingChangeCounts | null) => { diff --git a/src/mcp/server-instructions.ts b/src/mcp/server-instructions.ts index 0234f5e6a6..80a6247a80 100644 --- a/src/mcp/server-instructions.ts +++ b/src/mcp/server-instructions.ts @@ -64,7 +64,7 @@ calls; a grep/read exploration is dozens. - **Trust codegraph's results — don't re-verify them with grep.** They come from a full AST parse; re-checking with grep is slower, less accurate, and wastes context. - **Don't grep or Read first** to find or understand indexed code — ONE \`codegraph_explore\` returns the relevant symbols' source together in a single round-trip. Reach for raw \`Read\`/\`Grep\` only to confirm a specific detail codegraph didn't cover, or for what codegraph doesn't index (configs, docs). - **Don't reconstruct a flow by hand** — name the endpoints in one \`codegraph_explore\` and it surfaces the path between them, dynamic-dispatch hops included. -- **After editing, check the staleness banner.** When a tool response starts with "⚠️ Some files referenced below were edited since the last index sync…", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. A different, rarer banner — "⚠️ CodeGraph auto-sync is DISABLED…" 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. +- **After editing, check the staleness banner.** When a tool response starts with "⚠️ Some files referenced below were edited since the last index sync…", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. A different, rarer banner — "⚠️ CodeGraph auto-sync is DISABLED…" or "⚠️ CodeGraph auto-sync is RECOVERING…" — means the whole index may be stale; until the full catch-up completes, Read files directly to confirm anything that may have changed. If a response refuses to answer from changed files or says freshness could not be verified within its budget, retry after sync or narrow the query; that refusal is not an error in the tool. - **A file flagged "⚠ changed on disk after the last index sync" drifted from its index** (most common on projects queried via \`projectPath\`, which have no live watcher). Codegraph never serves a possibly-mis-sliced body from such a file — it either shows the file's full CURRENT source (trust it as a Read) or omits the source with this flag. When the source was omitted, Read that specific file; line numbers referencing it elsewhere in the response may be shifted until that project's next sync. All unflagged files remain trustworthy. - **Source is re-served on every call by default**, including for fresh subagents and after context compaction. Cross-call dedup requires \`CODEGRAPH_EXPLORE_DEDUP=1\` and is only suitable for hosts that guarantee one durable context per connection. With that opt-in, **"Already sent earlier in this conversation"** points to exact, unchanged source returned by an earlier \`codegraph_explore\` in that context. Use that copy; don't re-fetch it and don't Read the file. The bytes it freed went into source you have not seen yet, elsewhere in the same response. diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 44d19e3d0a..c6c9fea386 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -51,6 +51,7 @@ import { } from '../graph/named-symbol-flow'; import { getUpdateNotice } from '../upgrade/update-check'; import { measurePendingChanges } from './index-freshness'; +import { validateAnswerFiles, type AnswerFile } from './answer-freshness'; import { ExploreDiagnostics } from './explore-diagnostics'; import { EXPLORE_EMISSION_KEY, @@ -1196,6 +1197,9 @@ export interface ToolResult { * {@link EXPLORE_EMISSION_KEY}; the two must stay in sync. */ _cgExploreEmission?: ExploreEmission; + /** Internal structured provenance, preserved by query workers and stripped by execute. */ + _cgAnswerFiles?: AnswerFile[]; + structuredContent?: Record; } /** @@ -2100,18 +2104,6 @@ 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 * renderers slice CURRENT bytes at INDEXED line ranges; when the file @@ -2135,7 +2127,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, forceHash = false): boolean { + private isFileStaleOnDisk(cg: CodeGraph, relPath: string, content?: string): boolean { let root: string; try { root = cg.getProjectRoot(); @@ -2145,7 +2137,7 @@ export class ToolHandler { const key = `${root}\0${relPath}`; const now = Date.now(); const hit = this.driftCache.get(key); - if (!forceHash && hit && now - hit.at < ToolHandler.DRIFT_TTL_MS) return hit.stale; + if (hit && now - hit.at < ToolHandler.DRIFT_TTL_MS) return hit.stale; let stale = false; try { const rec = cg.getFile(relPath); @@ -2154,7 +2146,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 (forceHash || st.size !== rec.size || Math.floor(st.mtimeMs) !== Math.floor(rec.modifiedAt)) { + if (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 @@ -2162,26 +2154,21 @@ 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 = forceHash; + stale = false; } 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 answerResult(cg: CodeGraph, text: string, paths: Iterable): ToolResult { + const result = this.textResult(text); + result._cgAnswerFiles = [...new Set(paths)].map(file => ({ + path: file, + contentHash: cg.getFile(file)?.contentHash ?? null, + })); + return result; } private withStalenessNotice(result: ToolResult, projectPath?: string): ToolResult { @@ -2380,24 +2367,28 @@ export class ToolHandler { // keep the main handler's workspace-specific not-indexed guidance. const pooled = !!(this.queryPool && this.queryPool.healthy && this.queryPool.ready); const projectPath = pooled ? args.projectPath ?? this.cg?.getProjectRoot() : undefined; + const wasDegraded = project.isWatcherDegraded?.(); const raw = (pooled && projectPath) ? await this.queryPool!.run(toolName, { ...dispatchArgs, projectPath }) : await this.executeReadTool(toolName, dispatchArgs); - 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.' - ); + const answeredFrom = raw._cgAnswerFiles; + delete raw._cgAnswerFiles; + if (!raw.isError && answeredFrom && (wasDegraded || project.isWatcherDegraded?.())) { + const validation = await validateAnswerFiles(project.getProjectRoot(), answeredFrom); + if (validation.stale.length || validation.unchecked.length) { + // Rejected source must not enter the session's emission history. + const lines = ['⚠️ CodeGraph cannot answer from this index:']; + if (validation.stale.length) { + lines.push('These files changed or became unavailable after their last sync:', + ...validation.stale.map(file => `- ${file}`)); + } + if (validation.unchecked.length) { + lines.push(`Freshness could not be verified within the validation budget for ${validation.unchecked.length} files:`, + ...validation.unchecked.slice(0, 20).map(file => `- ${file}`)); + if (validation.unchecked.length > 20) lines.push(`- … ${validation.unchecked.length - 20} more (narrow the query)`); + } + lines.push('Retry after a successful codegraph sync, or narrow the query.'); + return { ...this.textResult(lines.join('\n')), structuredContent: { freshness: validation } }; } } // Record + STRIP before anything else touches the result: the emission is @@ -2565,7 +2556,7 @@ export class ToolHandler { }); const formatted = this.formatSearchResults(ranked); - return this.textResult(this.truncateOutput(formatted)); + return this.answerResult(cg, this.truncateOutput(formatted), ranked.map(r => r.node.filePath)); } /** @@ -2606,6 +2597,7 @@ export class ToolHandler { } const { groups, filteredOut } = this.groupDefinitions(allMatches.nodes, fileFilter); + const answerPaths = new Set(groups.flat().map(node => node.filePath)); const filterNote = filteredOut ? `\n\n> **Note:** no definition of "${symbol}" matches file "${fileFilter}" — showing all definitions instead.` : ''; @@ -2619,6 +2611,7 @@ export class ToolHandler { if (!seen.has(c.node.id)) { seen.add(c.node.id); callers.push(c.node); + answerPaths.add(c.node.filePath); const label = this.edgeLabel(c.edge); if (label) labels.set(c.node.id, label); } @@ -2631,7 +2624,7 @@ export class ToolHandler { if (groups.length === 1) { const { callers, labels } = collect(groups[0]!); if (callers.length === 0) { - return this.textResult(`No callers found for "${symbol}"${allMatches.note}${filterNote}`); + return this.answerResult(cg, `No callers found for "${symbol}"${allMatches.note}${filterNote}`, answerPaths); } // A successful `file` narrowing makes the multi-symbol aggregation note // stale — suppress it. @@ -2642,7 +2635,7 @@ export class ToolHandler { ? `\n\n> Showing ${limit} of ${callers.length} callers; pass \`limit\` (up to 100) to widen.` : ''; const formatted = this.formatNodeList(callers.slice(0, limit), `Callers of ${symbol}`, labels) + cut + note + filterNote; - return this.textResult(this.truncateOutput(formatted)); + return this.answerResult(cg, this.truncateOutput(formatted), answerPaths); } // Multiple DISTINCT definitions (#764): one section per definition so an @@ -2667,7 +2660,7 @@ export class ToolHandler { lines.push(`- … +${callers.length - limit} more (pass \`limit\` to widen)`); } } - return this.textResult(this.truncateOutput(lines.join('\n') + filterNote)); + return this.answerResult(cg, this.truncateOutput(lines.join('\n') + filterNote), answerPaths); } /** @@ -2687,6 +2680,7 @@ export class ToolHandler { } const { groups, filteredOut } = this.groupDefinitions(allMatches.nodes, fileFilter); + const answerPaths = new Set(groups.flat().map(node => node.filePath)); const filterNote = filteredOut ? `\n\n> **Note:** no definition of "${symbol}" matches file "${fileFilter}" — showing all definitions instead.` : ''; @@ -2700,6 +2694,7 @@ export class ToolHandler { if (!seen.has(c.node.id)) { seen.add(c.node.id); callees.push(c.node); + answerPaths.add(c.node.filePath); const label = this.edgeLabel(c.edge); if (label) labels.set(c.node.id, label); } @@ -2711,7 +2706,7 @@ export class ToolHandler { if (groups.length === 1) { const { callees, labels } = collect(groups[0]!); if (callees.length === 0) { - return this.textResult(`No callees found for "${symbol}"${allMatches.note}${filterNote}`); + return this.answerResult(cg, `No callees found for "${symbol}"${allMatches.note}${filterNote}`, answerPaths); } // A successful `file` narrowing makes the multi-symbol aggregation note // stale — suppress it. @@ -2722,7 +2717,7 @@ export class ToolHandler { ? `\n\n> Showing ${limit} of ${callees.length} callees; pass \`limit\` (up to 100) to widen.` : ''; const formatted = this.formatNodeList(callees.slice(0, limit), `Callees of ${symbol}`, labels) + cut + note + filterNote; - return this.textResult(this.truncateOutput(formatted)); + return this.answerResult(cg, this.truncateOutput(formatted), answerPaths); } // Multiple DISTINCT definitions (#764): per-definition sections. @@ -2745,7 +2740,7 @@ export class ToolHandler { lines.push(`- … +${callees.length - limit} more (pass \`limit\` to widen)`); } } - return this.textResult(this.truncateOutput(lines.join('\n') + filterNote)); + return this.answerResult(cg, this.truncateOutput(lines.join('\n') + filterNote), answerPaths); } /** @@ -2765,6 +2760,7 @@ export class ToolHandler { } const { groups, filteredOut } = this.groupDefinitions(allMatches.nodes, fileFilter); + const answerPaths = new Set(groups.flat().map(node => node.filePath)); const filterNote = filteredOut ? `\n\n> **Note:** no definition of "${symbol}" matches file "${fileFilter}" — showing all definitions instead.` : ''; @@ -2777,6 +2773,7 @@ export class ToolHandler { const impact = cg.getImpactRadius(node.id, depth); for (const [id, n] of impact.nodes) { mergedNodes.set(id, n); + answerPaths.add(n.filePath); } for (const e of impact.edges) { const key = `${e.source}->${e.target}:${e.kind}`; @@ -2792,7 +2789,7 @@ export class ToolHandler { // Single definition (or same-file overloads): the familiar merged report. if (groups.length === 1) { const formatted = this.formatImpact(symbol, impactOf(groups[0]!)) + (fileFilter && !filteredOut ? "" : allMatches.note) + filterNote; - return this.textResult(this.truncateOutput(formatted)); + return this.answerResult(cg, this.truncateOutput(formatted), answerPaths); } // Multiple DISTINCT definitions (#764): a blast radius PER definition — @@ -2809,7 +2806,7 @@ export class ToolHandler { this.formatImpact(`${head.qualifiedName} (${head.filePath}${line})`, impactOf(group)) ); } - return this.textResult(this.truncateOutput(sections.join('\n') + filterNote)); + return this.answerResult(cg, this.truncateOutput(sections.join('\n') + filterNote), answerPaths); } /** @@ -3672,9 +3669,9 @@ export class ToolHandler { const empty = `No relevant code found for "${query}"${missNote}${explanation}`; // Still an explore call, so it is still recorded: an empty answer spends a // call against the tier budget even though it emits no source. - return this.exploreResult(empty, { + return { ...this.textResult(empty), [EXPLORE_EMISSION_KEY]: { projectRoot, query, files: [], sourceBytes: 0, responseBytes: empty.length, - }); + } }; } // Graph-aware glue: findRelevantContext builds the subgraph from name/text @@ -6361,23 +6358,20 @@ export class ToolHandler { }); sourceBytes += emitted.bytes; } - return this.exploreResult(finalText, { + // Include graph-only and omitted/deleted files, not just emitted source. + const answerPaths = new Set(fileGroups.keys()); + for (const id of [...flow.pathNodeIds, ...flow.namedNodeIds]) { + const node = cg.getNode(id); + if (node) answerPaths.add(node.filePath); + } + const result = this.answerResult(cg, finalText, answerPaths); + result[EXPLORE_EMISSION_KEY] = { projectRoot, query, files: emittedFiles, sourceBytes, responseBytes: finalText.length, - }); - } - - /** - * An explore response plus the record of what it emitted (CG-17). The record - * rides the result only as far as {@link execute}, which files it into the - * calling session's state and deletes it — see {@link EXPLORE_EMISSION_KEY}. - */ - private exploreResult(text: string, emission: ExploreEmission): ToolResult { - const result = this.textResult(text); - result[EXPLORE_EMISSION_KEY] = emission; + }; return result; } @@ -6945,7 +6939,9 @@ export class ToolHandler { } } - return this.textResult(lines.join('\n')); + return { ...this.textResult(lines.join('\n')), structuredContent: { + freshness: { lastIndexedAt, changes, complete: changes !== null }, + } }; } /**