Two coupled changes addressing the issue's underlying ask — "how does the
agent know when the index lags" — without resorting to a static wait.
Per-file staleness banner
-------------------------
FileWatcher now tracks per-path `pendingFiles` (path, firstSeenMs,
lastSeenMs, indexing) — events since the last successful sync, cleared
only after a sync whose `syncStartedMs >= lastSeenMs` commits. Chokidar
initial-scan events are gated behind a `ready` flag (with `waitUntilReady()`
exposed so tests can deterministically wait through it) so a fresh startup
doesn't falsely flag every existing file as pending.
ToolHandler now wraps every code-returning response (search, context,
callers, callees, impact, trace, explore, node, files) with
`withStalenessNotice`: intersects "files referenced in the response" with
`getPendingFiles()` and emits a hybrid signal —
* banner at the top for files referenced AND pending (with edit age +
indexing/pending-sync state, telling the agent to Read those specific
files directly; the rest of the response stays fresh and codegraph
stays authoritative for it),
* compact footer for pending files elsewhere in the project not
referenced above (capped at 5).
Cost is one boolean check + N substring matches when pending; zero
allocation when idle. `codegraph_status` surfaces the same data as a
first-class `### Pending sync:` section so the agent can ask "is the index
caught up?" in one call.
Cross-project quirk: when an agent passes `projectPath` matching the
default session's project, the staleness wrapper switches from the cached
cross-project CodeGraph (no watcher) to the default one (with watcher) so
the signal still fires. Same fix applied to `handleStatus`.
CODEGRAPH_WATCH_DEBOUNCE_MS
---------------------------
MCP `serve --mcp` now reads `CODEGRAPH_WATCH_DEBOUNCE_MS` and forwards it
to `cg.watch({ debounceMs })`. Clamped to [100ms, 60s]; out-of-range or
non-numeric values fall back to the FileWatcher default (2000ms). Active
value is logged to stderr on watcher startup so it's discoverable. The
docs in `server-instructions.ts`, `installer/instructions-template.ts`,
and `.cursor/rules/codegraph.mdc` no longer claim "~500ms"; they now
describe the banner mechanism instead — since per-file staleness replaces
the "wait N ms" guidance entirely, the docs become accurate at any
debounce value.
Validation
----------
* 847 unit/integration tests pass (added 15 new ones — pending-file
tracking, banner/footer routing, status section, env-var parsing).
* Direct MCP probe through a real `codegraph serve --mcp` process: edit a
file, query within the debounce window, banner fires naming the
edited file with edit-age.
* Real Claude TUI session via `scripts/agent-eval/itrun.sh` with
`CODEGRAPH_WATCH_DEBOUNCE_MS=10000`: agent edits `math.ts`, calls
`codegraph_explore`, reads the banner, **and discloses it unprompted in
its final reply**: "note: symbol index is mid-sync for the new `divide`,
but the source it returned is verbatim from disk."
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
4a4a37d135
commit
b48170e69f
@@ -0,0 +1,47 @@
|
||||
/**
|
||||
* CODEGRAPH_WATCH_DEBOUNCE_MS env override (issue #403).
|
||||
*
|
||||
* Lets users tune the watcher quiet window from MCP-launched configs without
|
||||
* editing the agent's command line — formatter-on-save chains and large
|
||||
* generated outputs benefit from a longer window. Clamped to [100ms, 60s];
|
||||
* out-of-range / non-numeric values fall back to the FileWatcher default
|
||||
* (2000ms) rather than throwing or silently capping a likely typo.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { parseDebounceEnv } from '../src/mcp/engine';
|
||||
|
||||
describe('parseDebounceEnv', () => {
|
||||
it('returns undefined for unset / empty values', () => {
|
||||
expect(parseDebounceEnv(undefined)).toBeUndefined();
|
||||
expect(parseDebounceEnv('')).toBeUndefined();
|
||||
expect(parseDebounceEnv(' ')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('accepts integer values inside [100, 60000]', () => {
|
||||
expect(parseDebounceEnv('100')).toBe(100);
|
||||
expect(parseDebounceEnv('2000')).toBe(2000);
|
||||
expect(parseDebounceEnv('5000')).toBe(5000);
|
||||
expect(parseDebounceEnv('60000')).toBe(60000);
|
||||
});
|
||||
|
||||
it('rejects out-of-range values (returns undefined, lets default win)', () => {
|
||||
expect(parseDebounceEnv('0')).toBeUndefined();
|
||||
expect(parseDebounceEnv('50')).toBeUndefined(); // below 100
|
||||
expect(parseDebounceEnv('99')).toBeUndefined();
|
||||
expect(parseDebounceEnv('60001')).toBeUndefined(); // above 60s
|
||||
expect(parseDebounceEnv('-500')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('rejects non-integer / non-numeric values', () => {
|
||||
expect(parseDebounceEnv('abc')).toBeUndefined();
|
||||
expect(parseDebounceEnv('500.5')).toBeUndefined();
|
||||
expect(parseDebounceEnv('NaN')).toBeUndefined();
|
||||
expect(parseDebounceEnv('Infinity')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('accepts scientific notation that resolves to an in-range integer', () => {
|
||||
// Number('1e3') === 1000, Number.isInteger(1000) === true. Power users
|
||||
// who write debounce as 1e3 should not be surprised; the clamp still applies.
|
||||
expect(parseDebounceEnv('1e3')).toBe(1000);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,153 @@
|
||||
/**
|
||||
* Per-file staleness banner on MCP tool responses (issue #403).
|
||||
*
|
||||
* The watcher tracks every file event since the last successful sync; the
|
||||
* tool dispatcher intersects "files referenced in this response" with that
|
||||
* pending set and prepends a banner ("⚠️ Some files referenced below were
|
||||
* edited since the last index sync…") plus an optional footer ("(Note: N
|
||||
* file(s) elsewhere in this project are pending index sync…)").
|
||||
*
|
||||
* No auto-flush, no static wait — the response is instant and the agent
|
||||
* decides whether to Read the specific stale file. These tests exercise
|
||||
* the full real path: real watcher + real CodeGraph index + real
|
||||
* ToolHandler.execute().
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
import CodeGraph from '../src/index';
|
||||
import { ToolHandler } from '../src/mcp/tools';
|
||||
|
||||
function waitFor(condition: () => boolean, timeoutMs = 5000, intervalMs = 50): Promise<void> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const start = Date.now();
|
||||
const tick = () => {
|
||||
if (condition()) return resolve();
|
||||
if (Date.now() - start > timeoutMs) return reject(new Error('waitFor timed out'));
|
||||
setTimeout(tick, intervalMs);
|
||||
};
|
||||
tick();
|
||||
});
|
||||
}
|
||||
|
||||
describe('MCP staleness banner', () => {
|
||||
let testDir: string;
|
||||
let cg: CodeGraph;
|
||||
let handler: ToolHandler;
|
||||
|
||||
beforeEach(async () => {
|
||||
testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-stale-banner-'));
|
||||
fs.mkdirSync(path.join(testDir, 'src'));
|
||||
// Three isolated files with no cross-references — keeps each test's
|
||||
// "which path does the response mention?" assertion unambiguous. If the
|
||||
// files shared imports/calls, codegraph_search responses would surface
|
||||
// multiple file paths and the banner-vs-footer split would be racy.
|
||||
fs.writeFileSync(
|
||||
path.join(testDir, 'src', 'alpha-only.ts'),
|
||||
'export function alphaOnly() { return 1; }\n',
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(testDir, 'src', 'bravo-only.ts'),
|
||||
'export function bravoOnly() { return 2; }\n',
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(testDir, 'src', 'charlie-only.ts'),
|
||||
'export function charlieOnly() { return 3; }\n',
|
||||
);
|
||||
|
||||
cg = CodeGraph.initSync(testDir, { config: { include: ['**/*.ts'], exclude: [] } });
|
||||
await cg.indexAll();
|
||||
handler = new ToolHandler(cg);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
try { cg.unwatch(); } catch { /* ignore */ }
|
||||
try { cg.close(); } catch { /* ignore */ }
|
||||
if (fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('prepends a stale banner when the response references a pending file', async () => {
|
||||
// Long debounce so the edit lingers in pendingFiles while we query.
|
||||
cg.watch({ debounceMs: 4000 });
|
||||
await cg.waitUntilWatcherReady();
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(testDir, 'src', 'alpha-only.ts'),
|
||||
'export function alphaOnly() { return 99; }\n',
|
||||
);
|
||||
await waitFor(() => cg.getPendingFiles().some((p) => p.path === 'src/alpha-only.ts'), 8000);
|
||||
|
||||
const res = await handler.execute('codegraph_search', { query: 'alphaOnly' });
|
||||
expect(res.isError).toBeFalsy();
|
||||
const text = res.content[0].text;
|
||||
|
||||
// Banner shape: warning glyph + filename + actionable instruction.
|
||||
expect(text.startsWith('⚠️')).toBe(true);
|
||||
expect(text).toContain('src/alpha-only.ts');
|
||||
expect(text).toMatch(/edited \d+ms ago/);
|
||||
expect(text).toMatch(/Read them directly/);
|
||||
// The actual result must still follow the banner.
|
||||
expect(text).toMatch(/alphaOnly/);
|
||||
});
|
||||
|
||||
it('uses the footer (not the banner) when pending files are not referenced', async () => {
|
||||
cg.watch({ debounceMs: 4000 });
|
||||
await cg.waitUntilWatcherReady();
|
||||
|
||||
// Edit bravo-only.ts but search for the alphaOnly symbol, whose hit is
|
||||
// only in alpha-only.ts. The two files share no imports/calls so the
|
||||
// response text won't mention bravo-only.ts.
|
||||
fs.writeFileSync(
|
||||
path.join(testDir, 'src', 'bravo-only.ts'),
|
||||
'export function bravoOnly() { return 22; }\n',
|
||||
);
|
||||
await waitFor(() => cg.getPendingFiles().some((p) => p.path === 'src/bravo-only.ts'), 8000);
|
||||
|
||||
const res = await handler.execute('codegraph_search', { query: 'alphaOnly' });
|
||||
const text = res.content[0].text;
|
||||
|
||||
expect(text.startsWith('⚠️')).toBe(false);
|
||||
expect(text).toMatch(/elsewhere in this project are pending index sync/);
|
||||
expect(text).toContain('src/bravo-only.ts');
|
||||
});
|
||||
|
||||
it('drops the banner once the sync completes and clears the pending entry', async () => {
|
||||
cg.watch({ debounceMs: 200 });
|
||||
await cg.waitUntilWatcherReady();
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(testDir, 'src', 'alpha-only.ts'),
|
||||
'export function alphaOnly() { return 7; }\n',
|
||||
);
|
||||
await waitFor(() => cg.getPendingFiles().length === 0, 5000);
|
||||
|
||||
const res = await handler.execute('codegraph_search', { query: 'alphaOnly' });
|
||||
const text = res.content[0].text;
|
||||
expect(text.startsWith('⚠️')).toBe(false);
|
||||
expect(text).not.toMatch(/elsewhere in this project are pending index sync/);
|
||||
});
|
||||
|
||||
it('lists pending files under "Pending sync" in codegraph_status', async () => {
|
||||
cg.watch({ debounceMs: 4000 });
|
||||
await cg.waitUntilWatcherReady();
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(testDir, 'src', 'charlie-only.ts'),
|
||||
'export function charlieOnly() { return 33; }\n',
|
||||
);
|
||||
await waitFor(() => cg.getPendingFiles().some((p) => p.path === 'src/charlie-only.ts'), 8000);
|
||||
|
||||
const res = await handler.execute('codegraph_status', {});
|
||||
const text = res.content[0].text;
|
||||
expect(text).toContain('### Pending sync:');
|
||||
expect(text).toContain('src/charlie-only.ts');
|
||||
// Status embeds the info first-class, so the auto-banner is suppressed.
|
||||
expect(text.startsWith('⚠️')).toBe(false);
|
||||
});
|
||||
|
||||
it('returns zero pending files when no watcher is active', () => {
|
||||
expect(cg.getPendingFiles()).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -198,6 +198,94 @@ describe('FileWatcher', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('pending file tracking (#403)', () => {
|
||||
it('should expose edited paths via getPendingFiles before sync fires', async () => {
|
||||
// Slow debounce — events arrive but sync hasn't run yet.
|
||||
const syncFn = vi.fn().mockResolvedValue({ filesChanged: 1, durationMs: 10 });
|
||||
const watcher = new FileWatcher(testDir, syncFn, { debounceMs: 2000 });
|
||||
watcher.start();
|
||||
// Deterministic boundary: wait for chokidar's initial scan to complete
|
||||
// so any late initial-scan events have fired before we assert. A bare
|
||||
// sleep is flaky under test-parallelism load.
|
||||
await watcher.waitUntilReady();
|
||||
|
||||
expect(watcher.getPendingFiles()).toEqual([]);
|
||||
|
||||
fs.writeFileSync(path.join(testDir, 'src', 'pending.ts'), 'export const p = 1;');
|
||||
|
||||
// Allow chokidar to emit, but DON'T let the 2s debounce fire.
|
||||
await waitFor(() => watcher.getPendingFiles().length > 0, 3000);
|
||||
|
||||
const pending = watcher.getPendingFiles();
|
||||
const paths = pending.map((p) => p.path);
|
||||
expect(paths).toContain('src/pending.ts');
|
||||
const entry = pending.find((p) => p.path === 'src/pending.ts')!;
|
||||
expect(entry.firstSeenMs).toBeGreaterThan(0);
|
||||
expect(entry.lastSeenMs).toBeGreaterThanOrEqual(entry.firstSeenMs);
|
||||
// No sync running yet → indexing flag is false.
|
||||
expect(entry.indexing).toBe(false);
|
||||
|
||||
watcher.stop();
|
||||
});
|
||||
|
||||
it('should clear an entry only after a successful sync absorbing that edit', async () => {
|
||||
const syncFn = vi.fn().mockResolvedValue({ filesChanged: 1, durationMs: 10 });
|
||||
const watcher = new FileWatcher(testDir, syncFn, { debounceMs: 200 });
|
||||
watcher.start();
|
||||
await watcher.waitUntilReady();
|
||||
|
||||
fs.writeFileSync(path.join(testDir, 'src', 'fresh.ts'), 'export const f = 1;');
|
||||
|
||||
// Watcher saw the change → pendingFiles has the entry. Longer windows
|
||||
// here because chokidar event delivery on macOS slows under heavy
|
||||
// parallel test-suite load (4× slower than isolation).
|
||||
await waitFor(() => watcher.getPendingFiles().some((p) => p.path === 'src/fresh.ts'), 8000);
|
||||
|
||||
// Wait through debounce + sync; the entry should drop out.
|
||||
await waitFor(() => syncFn.mock.calls.length > 0, 8000);
|
||||
await waitFor(() => !watcher.getPendingFiles().some((p) => p.path === 'src/fresh.ts'), 8000);
|
||||
|
||||
expect(watcher.getPendingFiles()).toEqual([]);
|
||||
watcher.stop();
|
||||
});
|
||||
|
||||
it('should keep entries unchanged when sync fails (rescheduled work sees the same set)', async () => {
|
||||
// First post-settle sync rejects, second resolves. The initial-scan
|
||||
// sync (triggered by chokidar's pre-existing add events) is allowed to
|
||||
// resolve cleanly so it doesn't consume one of our scripted outcomes.
|
||||
const syncFn = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({ filesChanged: 0, durationMs: 1 }) // initial scan
|
||||
.mockRejectedValueOnce(new Error('boom')) // first real edit fails
|
||||
.mockResolvedValueOnce({ filesChanged: 1, durationMs: 10 }); // retry succeeds
|
||||
const onSyncError = vi.fn();
|
||||
const watcher = new FileWatcher(testDir, syncFn, { debounceMs: 200, onSyncError });
|
||||
watcher.start();
|
||||
// Wait through chokidar `ready` AND the initial-scan-triggered sync, so
|
||||
// the next sync corresponds to the explicit edit below.
|
||||
await watcher.waitUntilReady();
|
||||
await waitFor(() => syncFn.mock.calls.length >= 1, 5000);
|
||||
await new Promise((r) => setTimeout(r, 100));
|
||||
|
||||
fs.writeFileSync(path.join(testDir, 'src', 'will-fail.ts'), 'export const wf = 1;');
|
||||
|
||||
// Wait for the sync that handles the explicit edit to reject.
|
||||
await waitFor(() => onSyncError.mock.calls.length > 0, 5000);
|
||||
|
||||
// The file is STILL in pendingFiles — failure didn't drop it.
|
||||
const after = watcher.getPendingFiles();
|
||||
expect(after.some((p) => p.path === 'src/will-fail.ts')).toBe(true);
|
||||
|
||||
// Retry resolves; entry clears.
|
||||
await waitFor(
|
||||
() => !watcher.getPendingFiles().some((p) => p.path === 'src/will-fail.ts'),
|
||||
5000,
|
||||
);
|
||||
|
||||
watcher.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe('callbacks', () => {
|
||||
it('should call onSyncComplete after successful sync', async () => {
|
||||
const syncFn = vi.fn().mockResolvedValue({ filesChanged: 2, durationMs: 50 });
|
||||
|
||||
Reference in New Issue
Block a user