feat(watcher): chokidar auto-refresh with debounced incremental reindex#52
feat(watcher): chokidar auto-refresh with debounced incremental reindex#52PatrickSys merged 3 commits intomasterfrom
Conversation
Adds a chokidar-based file watcher that runs in MCP server mode. Any source file change in the project root triggers a debounced incremental reindex automatically — no manual refresh_index needed. - src/core/file-watcher.ts: FileWatcherOptions interface + startFileWatcher() with configurable debounce (default 2 s, override via CODEBASE_CONTEXT_DEBOUNCE_MS) - src/index.ts: wires watcher after server.connect(), guards against concurrent reindexes with indexState.status check, cleans up on exit/SIGINT/SIGTERM - tests/file-watcher.test.ts: 3 vitest tests covering trigger, debounce coalescing, and stop() cancellation (real fs writes, no fake timers) - package.json: adds chokidar ^3.6.0 dependency Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f300961b73
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (indexState.status === 'indexing') { | ||
| if (process.env.CODEBASE_CONTEXT_DEBUG) { | ||
| console.error('[file-watcher] Index in progress — skipping auto-refresh'); | ||
| } | ||
| return; |
There was a problem hiding this comment.
Queue a follow-up refresh when indexing is busy
This branch drops file-change events whenever indexState.status === 'indexing', so edits made during a long-running index (including the startup full index) can be permanently missed if no later change occurs. In that scenario the watcher never schedules another run, leaving the served index stale relative to disk until a manual refresh or another edit happens; this undermines the new auto-refresh guarantee.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR wires a chokidar-based file watcher into the MCP server to enable automatic incremental reindexing whenever source files change, completing Phase 2d of the project roadmap. The core implementation in Two concerns worth addressing before merge:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| src/core/file-watcher.ts | New file — clean, focused implementation of a debounced chokidar watcher. Correct ignore patterns, proper timer cleanup in stop(), and awaitWriteFinish guards against partial-write events. No issues. |
| src/index.ts | Watcher wired correctly after server.connect(). Two concerns: (1) `parseInt |
| tests/file-watcher.test.ts | The third test ("stop() cancels a pending callback") has a timing flaw: it waits 150 ms before stopping, but the watcher's awaitWriteFinish.stabilityThreshold is 200 ms, so the 'add' event hasn't fired yet when stop() is called. The test passes vacuously, not by actually exercising debounce cancellation. |
| package.json | Adds chokidar ^3.6.0 to production dependencies. Version range and placement are correct. |
| pnpm-lock.yaml | Lock file updated correctly to reflect chokidar 3.6.0 and its transitive dependencies (anymatch, binary-extensions, glob-parent, is-binary-path, normalize-path, readdirp, fsevents optional). No concerns. |
Sequence Diagram
sequenceDiagram
participant OS as OS / Filesystem
participant CK as chokidar (file-watcher.ts)
participant DT as Debounce Timer
participant IDX as performIndexing()
participant ST as indexState
Note over OS,ST: Server startup
IDX->>ST: status = 'indexing'
Note over CK: startFileWatcher() called
CK-->>OS: watch(rootPath, { awaitWriteFinish: 200ms })
OS->>CK: file add/change/unlink event
CK->>DT: clearTimeout + setTimeout(debounceMs)
OS->>CK: rapid successive events
CK->>DT: clearTimeout + reset timer (coalesced)
DT->>IDX: onChanged() fires after debounceMs
alt indexState.status === 'indexing'
IDX-->>DT: skip (return early — changes lost)
else indexState.status !== 'indexing'
IDX->>ST: status = 'indexing'
IDX->>ST: status = 'ready' / 'error'
end
Note over OS,ST: Process shutdown
OS->>CK: SIGINT / SIGTERM / exit
CK->>DT: clearTimeout (cancel pending)
CK->>CK: watcher.close()
Last reviewed commit: f300961
| await fs.writeFile(path.join(tempDir, 'cancel.ts'), 'export const y = 99;'); | ||
| // Let chokidar detect the event but stop before debounce fires | ||
| await new Promise((resolve) => setTimeout(resolve, 150)); | ||
| stop(); |
There was a problem hiding this comment.
Test doesn't actually test debounce cancellation
The test waits 150 ms before calling stop(), but the watcher is configured with awaitWriteFinish: { stabilityThreshold: 200 }. This means chokidar will not emit the 'add' event until at least 200 ms after the write finishes. At the 150 ms mark, trigger() has never been called, no debounce timer has been set, and stop() merely discards a clearTimeout(undefined) no-op before closing the watcher.
callCount stays at 0 because the filesystem event was never delivered — not because the debounce timer was successfully cancelled. The test comment "Let chokidar detect the event but stop before debounce fires" is therefore incorrect; chokidar hasn't detected anything yet.
To actually test cancellation of a live debounce timer, the wait before stop() needs to be longer than stabilityThreshold (200 ms) so that trigger() has been invoked and the timer is running, but shorter than debounceMs (500 ms) so the callback hasn't fired yet. For example, ~300–350 ms would reliably land in that window:
await fs.writeFile(path.join(tempDir, 'cancel.ts'), 'export const y = 99;');
// Wait long enough for chokidar's awaitWriteFinish (200 ms) to fire trigger(),
// but short enough that the debounce (500 ms) hasn't elapsed yet.
await new Promise((resolve) => setTimeout(resolve, 300));
stop();
await new Promise((resolve) => setTimeout(resolve, debounceMs + 200));
expect(callCount).toBe(0);| if (indexState.status === 'indexing') { | ||
| if (process.env.CODEBASE_CONTEXT_DEBUG) { | ||
| console.error('[file-watcher] Index in progress — skipping auto-refresh'); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
File changes during initial indexing are silently and permanently dropped
performIndexing() on line 719 is fire-and-forget (not awaited), so the watcher can receive onChanged events while the initial full index is still running. The guard here correctly prevents stacking a second concurrent run, but changes detected during that window are discarded with no mechanism to catch up. Once the initial index completes, those changed files will never be re-examined unless the user manually calls refresh_index.
A simple mitigation is to track whether at least one change was skipped and trigger a follow-up incremental reindex after the initial run finishes:
let pendingChanges = false;
onChanged: () => {
if (indexState.status === 'indexing') {
pendingChanges = true;
if (process.env.CODEBASE_CONTEXT_DEBUG) {
console.error('[file-watcher] Index in progress — queuing auto-refresh');
}
return;
}
pendingChanges = false;
console.error('[file-watcher] Changes detected — incremental reindex starting');
performIndexing(true);
}Then, at the end of performIndexing, or via a statuschange hook, check pendingChanges and schedule a follow-up incremental run.
src/index.ts
Outdated
| if (process.env.CODEBASE_CONTEXT_DEBUG) console.error('[DEBUG] Server ready'); | ||
|
|
||
| // Auto-refresh: watch for file changes and trigger incremental reindex | ||
| const debounceMs = parseInt(process.env.CODEBASE_CONTEXT_DEBOUNCE_MS ?? '', 10) || 2000; |
There was a problem hiding this comment.
parseInt || 2000 prevents setting debounce to 0
parseInt('0', 10) returns 0, which is falsy, so 0 || 2000 evaluates to 2000. Any CODEBASE_CONTEXT_DEBOUNCE_MS=0 (or any value that parses to 0) silently falls back to the default. If near-instant refresh is ever needed for testing or tight workflows, this would be surprising.
A more explicit guard avoids the pitfall:
| const debounceMs = parseInt(process.env.CODEBASE_CONTEXT_DEBOUNCE_MS ?? '', 10) || 2000; | |
| const debounceMs = (() => { const v = parseInt(process.env.CODEBASE_CONTEXT_DEBOUNCE_MS ?? '', 10); return Number.isFinite(v) && v >= 0 ? v : 2000; })(); |
|
Addressed review comment re: dropped events during indexing: watcher now sets a queued flag when changes occur while indexing; once indexing completes successfully we run a single incremental refresh to catch any edits that occurred mid-index. Also made watcher change logs debug-only and added small init delay in watcher tests for flake resistance. |
|
Follow-up: addressed Greptile inline comments:
|
What
Adds a chokidar-based file watcher to MCP server mode (Phase 2d from
docs/TODO.md).When the server is running, any source file change in the project root triggers a
debounced incremental reindex automatically — the user no longer needs to call
refresh_indexmanually after editing files.Why
The incremental reindex path (
performIndexing(true)) already existed and worked.This PR simply wires filesystem events to it, making the MCP server self-updating
by default.
Changes
package.jsonchokidar ^3.6.0to dependenciessrc/core/file-watcher.tsstartFileWatcher()with debounce logic, returnsstop()cleanupsrc/index.tsserver.connect(), guard against concurrent reindexestests/file-watcher.test.tsBehaviour
CODEBASE_CONTEXT_DEBOUNCE_MS)node_modules/,.codebase-context/,.git/,dist/exit,SIGINT, andSIGTERMRelease notes
New: MCP server mode now auto-refreshes the index when source files change.
No configuration required. Set
CODEBASE_CONTEXT_DEBOUNCE_MSto tune the debounce window (default: 2000 ms).Test plan
pnpm build— TypeScript compiles without errorspnpm test— 243/243 tests pass (3 new file-watcher tests)pnpm format:check— Prettier clean🤖 Generated with Claude Code