Skip to content

Commit 1e6084d

Browse files
anvansterclaude
andauthored
fix(mcp): make the file watcher honour workspace filters and harden the shared engine (#25)
* fix(mcp): make the watcher honour workspace filters; harden the shared 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 * no-mistakes(review): Locate deleted-directory events in symlinked workspaces * no-mistakes(document): Document watcher ignore rules and graph-only memory tools --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 64f4f30 commit 1e6084d

8 files changed

Lines changed: 658 additions & 234 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ one tool and exits without the MCP stdio handshake — ideal for scripting.
9797
| `--full-body-embedding` | `true` | Embed full function body (~50 lines) for better semantic search and duplicate detection |
9898
| `--max-files <n>` | 5000 | Maximum files to index |
9999
| `--profile <name>` | `all` | Filter the exposed MCP tool surface to a named subset (see below) |
100-
| `--graph-only` | off | Skip embedding generation — build the graph and serve structural tools only. No ONNX model load, 10-50× faster indexing. Semantic search unavailable. For CI / one-shot graph queries. |
100+
| `--graph-only` | off | Skip embedding generation — build the graph and serve structural tools only. No ONNX model load, 10-50× faster indexing. Semantic search and memory tools unavailable. For CI / one-shot graph queries. |
101101
| `--run-tool <name>` | — | One-shot mode: index, run a single tool, print its result, exit. No MCP handshake. Pair with `--tool-args '<json>'`. |
102102

103103
#### `--embedding-model static` — model2vec fast indexing

‎crates/codegraph-server/src/ai_query/engine.rs‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,24 @@ impl QueryEngine {
812812
/// Re-embed only symbols from a specific file path.
813813
/// Called on did_save to incrementally update embeddings without rebuilding all.
814814
pub async fn update_file_vectors(&self, file_path: &str) {
815+
self.update_files_vectors(&[file_path.to_string()]).await;
816+
}
817+
818+
/// Re-embed the symbols of several files in one pass over the graph.
819+
///
820+
/// Calling [`Self::update_file_vectors`] once per file walks every node once
821+
/// per file, so a burst of N changed files cost N full graph scans - one of
822+
/// the per-event costs behind issue #23. This walks the graph once and
823+
/// embeds everything that matched in one batch.
824+
pub async fn update_files_vectors(&self, file_paths: &[String]) {
825+
if file_paths.is_empty() {
826+
return;
827+
}
828+
let label = match file_paths {
829+
[only] => only.clone(),
830+
many => format!("{} files", many.len()),
831+
};
832+
let file_path = label.as_str();
815833
let engine = match self.vector_engine.read().await.clone() {
816834
Some(e) => e,
817835
None => {
@@ -841,8 +859,13 @@ impl QueryEngine {
841859
continue;
842860
}
843861

862+
// Matched by suffix in either direction, as before: stored paths and
863+
// event paths are not guaranteed to agree on being absolute.
844864
let path = node_props::path(node);
845-
if !path.ends_with(file_path) && !file_path.ends_with(path) {
865+
if !file_paths
866+
.iter()
867+
.any(|fp| path.ends_with(fp.as_str()) || fp.ends_with(path))
868+
{
846869
continue;
847870
}
848871

‎crates/codegraph-server/src/indexer.rs‎

Lines changed: 275 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,161 @@ use std::path::{Path, PathBuf};
1515
use std::sync::Arc;
1616
use tokio::sync::{Mutex, RwLock};
1717

18+
/// Whether one directory entry is excluded from the graph.
19+
///
20+
/// This is the single statement of the rule. The indexer applies it to every
21+
/// entry as it walks, and [`WorkspaceFilter`] applies it to every component of
22+
/// a path the file watcher reports. The watcher used to carry its own shorter
23+
/// list instead, so `--exclude`, `.codegraphignore` and most of the default
24+
/// exclusions kept generated files out of the initial index and then let every
25+
/// later write to them straight back in (issue #23).
26+
pub(crate) fn is_excluded_entry(
27+
config: &IndexConfig,
28+
exclude_set: &globset::GlobSet,
29+
path: &Path,
30+
is_dir: bool,
31+
) -> bool {
32+
let Some(name) = path.file_name() else {
33+
return true;
34+
};
35+
let name = name.to_string_lossy();
36+
if name.starts_with('.') {
37+
return true;
38+
}
39+
if exclude_set.is_match(path) {
40+
return true;
41+
}
42+
is_dir
43+
&& (config.exclude_dirs.iter().any(|d| d == name.as_ref())
44+
|| exclude_set.is_match(name.as_ref()))
45+
}
46+
47+
/// Decides whether the file watcher may admit a path, by the same rule the
48+
/// indexer used to build the graph.
49+
///
50+
/// The indexer only reaches a file after every directory above it has passed,
51+
/// because it recurses. A watcher event arrives as one bare path with no such
52+
/// history, so the rule is applied to each ancestor in turn - otherwise a file
53+
/// under an excluded directory would be judged only by its own name.
54+
///
55+
/// Built per workspace root because `.codegraphignore` is read per root, as
56+
/// the indexer does. It is read once, when the watcher starts: editing
57+
/// `.codegraphignore` takes effect on the next start, like `--exclude`.
58+
pub(crate) struct WorkspaceFilter {
59+
/// `(root as given, root canonicalised, config, exclude globs)`.
60+
roots: Vec<(PathBuf, PathBuf, IndexConfig, globset::GlobSet)>,
61+
}
62+
63+
impl WorkspaceFilter {
64+
pub(crate) fn new(roots: &[PathBuf], base: &IndexConfig) -> Self {
65+
let roots = roots
66+
.iter()
67+
.map(|root| {
68+
let mut config = base.clone();
69+
config.extend_from_codegraphignore(root);
70+
let exclude_set = config.build_exclude_set();
71+
let canonical = root.canonicalize().unwrap_or_else(|_| root.clone());
72+
(root.clone(), canonical, config, exclude_set)
73+
})
74+
.collect();
75+
Self { roots }
76+
}
77+
78+
/// Whether `path`, a file, belongs in the graph.
79+
///
80+
/// Deleted files cannot be stat'd, so the size limit is only applied to a
81+
/// file that still exists; every other check depends on the path alone.
82+
/// That keeps a delete for a file the index never held from being admitted
83+
/// on a technicality, and a delete for one it did hold from being refused.
84+
pub(crate) fn admits(&self, path: &Path) -> bool {
85+
let Some((root, relative, config, exclude_set)) = self.locate(path) else {
86+
return false;
87+
};
88+
89+
let components: Vec<_> = relative.components().collect();
90+
// Directories above the file, matching the walk's depth accounting.
91+
if components.len().saturating_sub(1) > config.max_depth as usize {
92+
return false;
93+
}
94+
95+
let mut current = root;
96+
for (index, component) in components.iter().enumerate() {
97+
current.push(component);
98+
let is_dir = index + 1 < components.len();
99+
if is_excluded_entry(config, exclude_set, &current, is_dir) {
100+
return false;
101+
}
102+
}
103+
104+
match std::fs::metadata(path) {
105+
Ok(metadata) => metadata.len() <= config.max_file_size_bytes,
106+
Err(_) => true,
107+
}
108+
}
109+
110+
/// `path` spelled the way the indexer stored it.
111+
///
112+
/// The indexer records each file under the workspace root as it was given,
113+
/// while FSEvents reports the resolved location. On a workspace reached
114+
/// through a symlink the two never match, so every lookup by event path
115+
/// missed: an edit re-parsed the file without removing its old symbols, a
116+
/// delete removed nothing, and the graph only ever grew. Translating at
117+
/// intake gives every later step one spelling.
118+
pub(crate) fn indexed_form(&self, path: &Path) -> PathBuf {
119+
match self.locate(path) {
120+
Some((root, relative, _, _)) => root.join(relative),
121+
None => path.to_path_buf(),
122+
}
123+
}
124+
125+
/// The workspace root `path` falls under, in the form it was given, and
126+
/// `path` relative to it.
127+
///
128+
/// Roots and event paths do not reliably share a spelling. A workspace
129+
/// opened as `/var/folders/...` or through any symlinked directory is
130+
/// reported by FSEvents at its resolved location, `/private/var/folders/...`,
131+
/// so a plain prefix test rejected every event and the watcher admitted
132+
/// nothing at all. The event path is compared, as reported, against both
133+
/// spellings of the root; that needs no filesystem access, so it still
134+
/// works when the file's directory was deleted along with it.
135+
///
136+
/// A path reported in unresolved form is additionally resolved through its
137+
/// parent directory. The most specific root wins when workspace folders
138+
/// nest.
139+
fn locate(&self, path: &Path) -> Option<(PathBuf, PathBuf, &IndexConfig, &globset::GlobSet)> {
140+
let resolved = path
141+
.parent()
142+
.and_then(|parent| parent.canonicalize().ok())
143+
.zip(path.file_name())
144+
.map(|(parent, name)| parent.join(name));
145+
146+
let mut best: Option<(usize, PathBuf, PathBuf, &IndexConfig, &globset::GlobSet)> = None;
147+
for (given, canonical, config, exclude_set) in &self.roots {
148+
for (candidate, root) in [
149+
(Some(path), given),
150+
(Some(path), canonical),
151+
(resolved.as_deref(), canonical),
152+
] {
153+
let Some(candidate) = candidate else { continue };
154+
let Ok(relative) = candidate.strip_prefix(root) else {
155+
continue;
156+
};
157+
let depth = root.components().count();
158+
if best.as_ref().is_none_or(|(d, ..)| depth > *d) {
159+
best = Some((
160+
depth,
161+
given.clone(),
162+
relative.to_path_buf(),
163+
config,
164+
exclude_set,
165+
));
166+
}
167+
}
168+
}
169+
best.map(|(_, root, relative, config, exclude_set)| (root, relative, config, exclude_set))
170+
}
171+
}
172+
18173
/// Configuration for a single indexing run.
19174
#[derive(Debug, Clone)]
20175
pub struct IndexConfig {
@@ -492,33 +647,12 @@ impl Indexer {
492647

493648
let path = entry.path();
494649

495-
// Skip hidden files and directories
496-
if let Some(name) = path.file_name() {
497-
if name.to_string_lossy().starts_with('.') {
498-
continue;
499-
}
650+
let is_dir = path.is_dir();
651+
if is_excluded_entry(config, &exclude_set, &path, is_dir) {
652+
continue;
500653
}
501654

502-
if path.is_dir() {
503-
let dir_name = path
504-
.file_name()
505-
.map(|n| n.to_string_lossy().to_string())
506-
.unwrap_or_default();
507-
508-
// Skip hardcoded exclude directories
509-
if config.exclude_dirs.iter().any(|e| e == &dir_name) {
510-
continue;
511-
}
512-
513-
// Skip directories matching user-configured exclude globs
514-
let path_str = path.to_string_lossy();
515-
if exclude_set.is_match(path_str.as_ref())
516-
|| exclude_set.is_match(dir_name.as_str())
517-
{
518-
tracing::info!("Skipping {:?}: matched exclude pattern", path);
519-
continue;
520-
}
521-
655+
if is_dir {
522656
let (t, p, s, child_by_lang, child_errors) = self
523657
.index_directory(graph, &path, config, depth + 1, counter.clone())
524658
.await;
@@ -532,12 +666,6 @@ impl Indexer {
532666
*parser_errors.entry(lang).or_insert(0) += count;
533667
}
534668
} else if path.is_file() {
535-
// Skip files matching exclude globs
536-
let path_str = path.to_string_lossy();
537-
if exclude_set.is_match(path_str.as_ref()) {
538-
continue;
539-
}
540-
541669
// Skip files that exceed the configurable size limit
542670
if let Ok(metadata) = std::fs::metadata(&path) {
543671
if metadata.len() > config.max_file_size_bytes {
@@ -662,6 +790,122 @@ mod tests {
662790
);
663791
}
664792

793+
fn filter_config(exclude_dirs: &[&str], max_depth: u32) -> IndexConfig {
794+
IndexConfig {
795+
exclude_dirs: exclude_dirs.iter().map(|d| d.to_string()).collect(),
796+
exclude_patterns: vec![],
797+
max_file_size_bytes: 1024 * 1024,
798+
max_depth,
799+
max_files: 1000,
800+
}
801+
}
802+
803+
#[test]
804+
fn workspace_filter_excludes_what_the_indexer_excludes() {
805+
let tmp = tempfile::tempdir().unwrap();
806+
let root = tmp.path().to_path_buf();
807+
std::fs::create_dir_all(root.join("src/cache")).unwrap();
808+
let filter =
809+
WorkspaceFilter::new(std::slice::from_ref(&root), &filter_config(&["cache"], 20));
810+
811+
assert!(filter.admits(&root.join("src/lib.rs")));
812+
// An excluded directory excludes everything beneath it, however deep -
813+
// a watcher event names only the file, so every ancestor is checked.
814+
assert!(!filter.admits(&root.join("src/cache/gen.rs")));
815+
assert!(!filter.admits(&root.join("cache/deep/er/gen.rs")));
816+
// Hidden entries, at any level, as the indexer's walk skips them.
817+
assert!(!filter.admits(&root.join(".hidden.rs")));
818+
assert!(!filter.admits(&root.join("src/.git/x.rs")));
819+
// Outside every workspace root.
820+
assert!(!filter.admits(Path::new("/somewhere/else/lib.rs")));
821+
}
822+
823+
#[test]
824+
fn workspace_filter_reads_codegraphignore_per_root() {
825+
let tmp = tempfile::tempdir().unwrap();
826+
let root = tmp.path().to_path_buf();
827+
std::fs::write(root.join(".codegraphignore"), "**/generated/**\n").unwrap();
828+
let filter = WorkspaceFilter::new(std::slice::from_ref(&root), &filter_config(&[], 20));
829+
830+
assert!(!filter.admits(&root.join("src/generated/out.rs")));
831+
assert!(filter.admits(&root.join("src/handwritten.rs")));
832+
}
833+
834+
#[test]
835+
fn workspace_filter_enforces_max_depth() {
836+
let tmp = tempfile::tempdir().unwrap();
837+
let root = tmp.path().to_path_buf();
838+
let filter = WorkspaceFilter::new(std::slice::from_ref(&root), &filter_config(&[], 1));
839+
840+
assert!(filter.admits(&root.join("a/file.rs")));
841+
assert!(!filter.admits(&root.join("a/b/file.rs")));
842+
}
843+
844+
/// A workspace reached through a symlink is reported by the OS at its
845+
/// resolved location. An explicit symlink is used rather than relying on
846+
/// macOS's `/var` -> `/private/var`, which Linux CI would not exercise.
847+
#[cfg(unix)]
848+
#[test]
849+
fn workspace_filter_matches_events_at_the_resolved_path() {
850+
let tmp = tempfile::tempdir().unwrap();
851+
let real = tmp.path().join("real");
852+
std::fs::create_dir_all(real.join("src/cache")).unwrap();
853+
let link = tmp.path().join("link");
854+
std::os::unix::fs::symlink(&real, &link).unwrap();
855+
856+
let filter =
857+
WorkspaceFilter::new(std::slice::from_ref(&link), &filter_config(&["cache"], 20));
858+
let resolved = real.canonicalize().unwrap();
859+
860+
// Admitted and excluded by the same rule whichever spelling arrives.
861+
assert!(filter.admits(&resolved.join("src/lib.rs")));
862+
assert!(!filter.admits(&resolved.join("src/cache/gen.rs")));
863+
864+
// Translated back to the spelling the indexer stored, so lookups by
865+
// path find the file's existing nodes instead of duplicating them.
866+
assert_eq!(
867+
filter.indexed_form(&resolved.join("src/lib.rs")),
868+
link.join("src/lib.rs")
869+
);
870+
assert_eq!(
871+
filter.indexed_form(&link.join("src/lib.rs")),
872+
link.join("src/lib.rs")
873+
);
874+
}
875+
876+
/// Deleting a directory removes the parent an event path would be resolved
877+
/// through, so a resolved-form path must be located without touching disk.
878+
#[cfg(unix)]
879+
#[test]
880+
fn workspace_filter_locates_files_in_a_deleted_directory() {
881+
let tmp = tempfile::tempdir().unwrap();
882+
let real = tmp.path().join("real");
883+
std::fs::create_dir_all(real.join("src/old")).unwrap();
884+
std::fs::write(real.join("src/old/a.rs"), "fn a() {}\n").unwrap();
885+
let link = tmp.path().join("link");
886+
std::os::unix::fs::symlink(&real, &link).unwrap();
887+
888+
let filter = WorkspaceFilter::new(std::slice::from_ref(&link), &filter_config(&[], 20));
889+
let deleted = real.canonicalize().unwrap().join("src/old/a.rs");
890+
std::fs::remove_dir_all(real.join("src/old")).unwrap();
891+
892+
assert!(filter.admits(&deleted));
893+
assert_eq!(filter.indexed_form(&deleted), link.join("src/old/a.rs"));
894+
}
895+
896+
#[test]
897+
fn workspace_filter_prefers_the_most_specific_root() {
898+
let tmp = tempfile::tempdir().unwrap();
899+
let outer = tmp.path().to_path_buf();
900+
let inner = outer.join("inner");
901+
std::fs::create_dir_all(&inner).unwrap();
902+
// Only the inner root excludes `vendor`.
903+
std::fs::write(inner.join(".codegraphignore"), "**/vendor/**\n").unwrap();
904+
let filter = WorkspaceFilter::new(&[outer.clone(), inner.clone()], &filter_config(&[], 20));
905+
906+
assert!(!filter.admits(&inner.join("vendor/x.rs")));
907+
}
908+
665909
#[test]
666910
fn extend_from_codegraphignore_appends_patterns() {
667911
let tmp = tempfile::tempdir().expect("tmp");

0 commit comments

Comments
 (0)