Skip to content

Commit 9a2d7cd

Browse files
anvansterclaude
andcommitted
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
1 parent 64f4f30 commit 9a2d7cd

6 files changed

Lines changed: 629 additions & 232 deletions

File tree

‎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: 249 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,155 @@ 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. Both sides are compared resolved and as given.
133+
///
134+
/// The event path is resolved through its parent directory, which still
135+
/// exists when the file itself was just deleted. The most specific root
136+
/// wins when workspace folders nest.
137+
fn locate(&self, path: &Path) -> Option<(PathBuf, PathBuf, &IndexConfig, &globset::GlobSet)> {
138+
let resolved = path
139+
.parent()
140+
.and_then(|parent| parent.canonicalize().ok())
141+
.zip(path.file_name())
142+
.map(|(parent, name)| parent.join(name));
143+
144+
let mut best: Option<(usize, PathBuf, PathBuf, &IndexConfig, &globset::GlobSet)> = None;
145+
for (given, canonical, config, exclude_set) in &self.roots {
146+
for (candidate, root) in [(Some(path), given), (resolved.as_deref(), canonical)] {
147+
let Some(candidate) = candidate else { continue };
148+
let Ok(relative) = candidate.strip_prefix(root) else {
149+
continue;
150+
};
151+
let depth = root.components().count();
152+
if best.as_ref().is_none_or(|(d, ..)| depth > *d) {
153+
best = Some((
154+
depth,
155+
given.clone(),
156+
relative.to_path_buf(),
157+
config,
158+
exclude_set,
159+
));
160+
}
161+
}
162+
}
163+
best.map(|(_, root, relative, config, exclude_set)| (root, relative, config, exclude_set))
164+
}
165+
}
166+
18167
/// Configuration for a single indexing run.
19168
#[derive(Debug, Clone)]
20169
pub struct IndexConfig {
@@ -492,33 +641,12 @@ impl Indexer {
492641

493642
let path = entry.path();
494643

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-
}
644+
let is_dir = path.is_dir();
645+
if is_excluded_entry(config, &exclude_set, &path, is_dir) {
646+
continue;
500647
}
501648

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-
649+
if is_dir {
522650
let (t, p, s, child_by_lang, child_errors) = self
523651
.index_directory(graph, &path, config, depth + 1, counter.clone())
524652
.await;
@@ -532,12 +660,6 @@ impl Indexer {
532660
*parser_errors.entry(lang).or_insert(0) += count;
533661
}
534662
} 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-
541663
// Skip files that exceed the configurable size limit
542664
if let Ok(metadata) = std::fs::metadata(&path) {
543665
if metadata.len() > config.max_file_size_bytes {
@@ -662,6 +784,102 @@ mod tests {
662784
);
663785
}
664786

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

0 commit comments

Comments
 (0)