fix(mcp): make the file watcher honour workspace filters and harden the shared engine - #25
Merged
Merged
Conversation
…d engine Generated files kept out of the initial index were let straight back in by the MCP file watcher, and each event they caused cost several full-graph scans. On an 8 GB machine running several agent sessions that showed up as sustained watcher work and memory pressure (#23). Watcher - One exclusion rule. The watcher carried its own ten-entry SKIP_DIRS list and was never given the index config, so --exclude and .codegraphignore applied to the initial index only. is_excluded_entry() now states the indexer's rule once; the indexer's walk and a new WorkspaceFilter both use it. The watcher checks every ancestor of an event path, since an event names only the file and the walk only ever reached a file through its parents. default_exclude_dirs is a strict superset of the old SKIP_DIRS, so nothing the watcher skipped before is admitted now. - One pass per batch. process_changes ran a full-graph property("path") query for every deleted file, vanished file, changed file, its dependents and each dependent. It now builds one path-to-nodes map per batch, and a delete for a file the index never held takes no lock at all. Vector re-embedding is batched the same way (update_files_vectors), replacing a full node walk per changed file. - Symlinked workspaces. The indexer stores paths as the workspace was given; FSEvents reports the resolved location (/var -> /private/var, or any symlinked folder). Every lookup by event path missed, so on such a workspace an edit re-parsed a file without removing its old symbols and a delete removed nothing: the graph only grew. Present in 0.20.1. Event paths are now translated into the indexed form at intake, and the filter matches roots both as given and resolved. - Orphan vectors. remove_file_vectors ran before the nodes were deleted and only drops vectors whose node is already gone, so it could not see the file it was called for. One prune_orphan_vectors after the batch covers deletions and the vectors left behind by re-parsing, which nothing cleaned up before. - A delete-only batch now rebuilds the indexes too. This is index hygiene: search resolves results against the graph, so deleted symbols were not visible before either. Graph-only - index_workspace and the daemon-attach path initialised the memory manager, which loads the embedding model, before anything checked --graph-only. The existing comment on the graph-only branch already promised the model would never load; it now doesn't. Memory tools are unavailable in graph-only mode, the state the RAM gate already leaves them in on low-memory hosts. Shared engine - Resource settings. An auto-spawned engine was passed only the model name, so a client's --exclude, --max-files and --graph-only were dropped. engine_args() forwards the engine-level settings, EngineConfig carries graph_only, and a graph-only engine no longer loads the shared model. --profile is deliberately not forwarded: it filters one client's tool list, and a shared engine would impose it on every other client. - One load per workspace. Two attaches to a cold workspace both built a full backend, each indexing and starting a watcher, and the loser returned its own unregistered copy, serving that connection from it for its whole life. The registry now holds a per-workspace OnceCell. - One engine per socket. Concurrent auto-starts were guarded only by a socket probe taken before a model load that can take minutes; the second engine then removed the first one's live socket to bind its own, leaving the first running and unreachable. Reproduced: two clients, two engines. An exclusive lock (std File::try_lock) is now taken before anything is loaded and held for the engine's lifetime. Verified end to end against the shipped 0.20.1 binary and this build, each watcher test with a positive control (a fresh, non-excluded file must still be picked up). Unit tests cover the filter (including an explicit symlink, since Linux CI has no /var -> /private/var), most-specific-root selection, max depth, the engine lock and the forwarded arguments. Not addressed: .gitignore is honoured by neither the watcher nor the initial index, which stay consistent; supporting it means adding the ignore crate. Reported and diagnosed by Christopher Schulze in #23, whose analysis of the watcher filter, the per-event scans, graph-only ordering and the shared engine's dropped settings was accurate on every point. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017rVbt7rENTwXkdHt3Bpgb5
🔍 CodeGraph PR Review8 files changed (+658/−234, 35 functions) · Risk: 🔴 high Blast radius58 direct callers affected (20 breaking) across
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
The developer wanted to fix GitHub issue #23, where the MCP file watcher ignored workspace exclusion filters (--exclude, .codegraphignore), loaded the embedding model despite --graph-only, did repeated full-graph path scans per event, forwarded only --embedding-model to the auto-spawned shared engine, and allowed concurrent clients to start duplicate engines or load a workspace twice. They want to remain the sole maintainer, so they explicitly rejected merging the outside contributor's PR #24. They asked for the fix to be implemented independently and included alongside the other fixes. The work went on its own branch (fix/watcher-honours-workspace-filters) off main, with the reporter credited for the report and diagnosis. It also covers related pre-existing bugs found while testing, such as symlinked workspaces never removing stale symbols. The developer's final instruction was to commit this change and push it through the no-mistakes gate.
What Changed
is_excluded_entry()and a newWorkspaceFilter. That means--exclude,.codegraphignoreand the default skip dirs now apply to watched events, checked against every ancestor directory of the event path. Each batch is handled with one path-to-nodes map and one batched re-embed (update_files_vectors) instead of several full-graph scans per event. Orphan vectors are pruned after each batch./var->/private/var) are now translated back to the indexed form. Before this, edits added symbols without removing the old ones, and deletes removed nothing. Deleted-directory events in such workspaces are matched too.--graph-onlyno longer loads the embedding model through the memory manager, so memory tools are unavailable in that mode (README and tool-calling guide updated). An auto-spawned shared engine now receives--exclude,--max-filesand--graph-onlyalong with--embedding-model;--profileis deliberately not forwarded. The engine also holds an exclusive socket lock for its whole lifetime, and loads each workspace once through a per-workspaceOnceCell, so concurrent clients can no longer start duplicate engines or load a workspace twice.Reported and diagnosed by Christopher Schulze in #23.
🤖 Generated with Claude Code
Risk Assessment
✅ Low: The fix-round change is small and correct: it adds a check of the raw event path against the canonical root, which locates resolved-form deletes without needing the filesystem, and the new regression test fails without the fix.
Testing
I built the target and base binaries and drove both live. The watcher scenarios used stdio MCP on a workspace opened through an explicit symlink under /var -> /private/var. They covered excluded writes,.codegraphignorewrites, a file edit and a whole-directory delete. I also ran--run-tooland--servewith--graph-only, and started four--connectclients at once against an unused socket. Every scenario passes on the target, and on base each one shows the original bug. The change's own focused unit tests also pass. The evidence is CLI transcripts and server logs in the evidence directory; there is no UI surface. I removed the temporary binaries, and the worktree is clean.Evidence: Watcher transcript, target (symlinked workspace)
after writes: kept_ -> [kept_new, kept_one]; cached_ -> []; genfn_ -> []; edit_ -> [edit_renamed] after rm -rf src/old: old_ -> []Evidence: Watcher transcript, base (bug reproduced)
after writes: cached_ -> [cached_later (/private/...)]; genfn_ -> [genfn_later]; edit_ -> [edit_original, edit_renamed] after rm -rf src/old: old_ -> [old_a, old_b]Evidence: Server log, target watcher run
Evidence: Concurrent --connect, target
engine processes running for this socket: 1 <bin> --serve --socket ... --embedding-model bge-small --max-files 777 --exclude cache --graph-onlyEvidence: Concurrent --connect, base
engine processes running for this socket: 3 (each started with only --embedding-model bge-small); clients see excluded cached_initialEvidence: --serve --graph-only log, target
Evidence: --serve --graph-only log, base
Evidence: --run-tool --graph-only logs (target/base)
Evidence: Driver scripts
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
crates/codegraph-server/src/indexer.rs:129- Deleting a directory in a symlinked workspace still leaves its symbols in the graph, which is the same 'graph only grows' bug this change says it fixes.WorkspaceFilter::locatebuilds the resolved form of an event path by canonicalizing the path's parent directory. It then compares that resolved form with the canonical root, and compares the raw event path only with the root as given. Example: the workspace is opened as/var/folders/x/wsand the user runsrm -rf src/old. FSEvents reports/private/var/folders/x/ws/src/old/a.rs, but by intake timesrc/oldis gone, socanonicalize()fails andresolvedis None. The raw path does not start with/var/folders/x/ws, soadmits()returns false andis_watchabledrops the event. The nodes for every file under the deleted directory are never removed. The same happens when FSEvents reports such a delete as a modify, the 'vanished' path. Fix: also compare the raw event path against the canonical root ((Some(path), canonical)in the candidate list). FSEvents already reports resolved paths, so this needs no filesystem access and works after the parent is deleted. Adding a test that deletes a whole subdirectory under the symlinked root would cover it.🔧 Fix applied.
✅ Re-checked - no issues remain.
crates/codegraph-server/src/main.rs- This was already the case on base and the change does not touch it. A--connectclient started with--profile coreis still shown all 42 tools. The engine-args comment says the profile filters the tools one client is shown, but in--connectmode nothing on either side applies it. This may be worth a follow-up issue.cargo build -p codegraph-serverat the target commit and at base 64f4f30 (base taken fromgit archiveinto a temporary directory), so the two binaries could be comparedpython3 drive_watcher.py <bin> <symlinked-ws> <real-dir> <log> --graph-only --exclude cache: drives--mcpover stdio, writes to included,--excluded and.codegraphignored paths, edits a file, runsrm -rf src/old, and checks the results withcodegraph_symbol_search. Run against both binaries.codegraph-server --graph-only -w <ws> --run-tool codegraph_symbol_searchon target and base, checking the logs for MemoryManager initialisationpython3 drive_engine.py <bin> <ws> <sock> <home> 4: starts 4--connectclients at the same time against an unused socket with--graph-only --exclude cache --max-files 777 --profile core, then lists the--serveprocesses withpsand their argv. Run against both binaries.codegraph-server --serve --socket <sock> --graph-only -w <ws>on target and base, checking the logs for a model loadcargo test -p codegraph-server --lib -- workspace_filter lock_testsandcargo test -p codegraph-server --bin codegraph-server engine_args(the change's own focused tests, all pass)✅ **Document** - passed
✅ No issues found.
crates/codegraph-server/src/indexer.rs:290- These problems were already there before this change, and the files this change touched are clean.cargo fmt --checkreports drift in about 60 files in other crates (language parsers, codegraph-harness, codegraph-memory).cargo clippy -p codegraph-serverreports warnings in lines this change did not touch: indexer.rs:290-292 (doc list indentation), indexer.rs:587 (type_complexity), server.rs:1403/4027/4074/4173, plus unused imports in the parser crates. A separate repo-widecargo fmt --alland clippy cleanup commit would clear them without mixing unrelated churn into this fix.✅ **Push** - passed
✅ No issues found.