Ver Fonte

fix(sync): refresh the watcher's scope when codegraph.json or a .gitignore changes (#1590) (#1594)

Fixes #1590.

## What was wrong

The live file watcher built its scope matcher — built-in defaults + `.gitignore` + the `codegraph.json` `exclude`/`include` rules — once in `start()` and kept it for the watcher's lifetime. The MCP server is long-lived, so a `codegraph.json` created or edited after it started was invisible to the watcher, while `codegraph sync` (a fresh process with a fresh matcher) honoured it immediately. From the user's side: the CLI removed a newly excluded file, and the daemon re-indexed it a few seconds later, which reads as "`exclude` doesn't work". As the report points out, `extensions` on the very same config file *was* read live (its loader is mtime-cached), so two fields of one file behaved differently.

There was a second half to it. The watcher's scoped fast path hands the exact edited paths to sync, and that path stat'ed and re-parsed them without consulting the scope matcher at all — so the stale view of scope leaked straight into the index.

## What this does

**Watcher — rebuild on a scope change, then reconcile in full.** An event for the root `codegraph.json` or `.gitignore` rebuilds the matcher, marks the next sync as a full reconcile, and schedules it. A scope change has no per-file events: newly excluded files must be *removed* from the index and newly included ones *added*, and only the scan-diff (which builds its own fresh matcher) knows which those are. Two ordering details are deliberate:

- the two root files are checked *before* the matcher is consulted, so a user pattern that happens to cover them (`*.json`, `.*`) can't hide their own edits;
- a nested `.gitignore` (an embedded child repo's own rules, or a subdirectory rule the git-backed scan honours) is checked *after* the matcher, so the thousands of package-local `.gitignore`s an `npm install` writes under an ignored `node_modules/` can never trigger a rebuild storm.

Rebuilding runs embedded-repo discovery (one `git ls-files`), which is fine per config edit and never happens per event. Replacing the field serves both watch strategies: the recursive handler and the per-directory `shouldIgnoreDir` walk read it on every call.

**Scoped sync — re-check the paths it was handed.** The orchestrator now runs scoped paths through the same scope matcher and source-extension gate the full walk applies. An out-of-scope path is treated as absent: removed if tracked, never parsed on trust. The matcher is memoized on the mtimes of the two root files it derives from (two `stat`s per sync while nothing changed), so the scoped path keeps skipping O(repo) work — paying embedded-repo discovery per sync would defeat its whole point.

## Tests

- `watcher.test.ts` — a `codegraph.json` edit schedules a full sync, after which an edit inside the newly excluded tree is dropped by the live matcher (not pending, no sync) while an in-scope edit still syncs scoped; a root `.gitignore` edit behaves the same; a nested `.gitignore` forces a full sync; a `.gitignore` under `node_modules/` schedules nothing; dropping the exclude again readmits the tree.
- `sync.test.ts` — end-to-end through `CodeGraph`: a scoped sync of a path that `codegraph.json` now excludes removes it (`filesRemoved: 1`, nothing parsed — the symbol added to the file never appears), stays out on a repeat, and is re-added through the same scoped path once the exclude is dropped.
- All five new tests fail on `main`; the `node_modules` guard passes both ways as expected.
- Full suite: 189 files, 3184 passed / 9 skipped.
- CLI half of the issue's repro (init with `exclude`, edit the config + the file, `codegraph sync`): the newly excluded file is removed and its new symbol never enters the index.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01LxZj6W6Y1SHXwvpT3uwJpK
Colby Mchenry há 1 semana atrás
pai
commit
cf1b0e341a
5 ficheiros alterados com 265 adições e 8 exclusões
  1. 1 0
      CHANGELOG.md
  2. 29 0
      __tests__/sync.test.ts
  3. 128 0
      __tests__/watcher.test.ts
  4. 54 2
      src/extraction/index.ts
  5. 53 6
      src/sync/watcher.ts

+ 1 - 0
CHANGELOG.md

@@ -73,6 +73,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - Files skipped because they are too large or repeatedly fail to parse are now recorded with the reason, so unchanged rejected files are no longer rediscovered and retried on every status check and sync — and a later successful parse of such a file replaces the record with its real symbols. Thanks @netbrah for the exceptional failure analysis behind this batch, and @danusha2345 for the fixes. (#1557)
 - C/C++ function-pointer analysis now bounds its compiled-pattern caches, so very large repositories can no longer exhaust the JavaScript engine's regular-expression code space during indexing. (#1559)
 - JSX rendering analysis now runs only on JavaScript-family files, so JSX-looking strings in C/C++ (or any other language) no longer create impossible call edges — in pure-C projects and in mixed-language monorepos alike. (#1560)
+- Editing `codegraph.json`'s `exclude` or `include` (or a `.gitignore`) while the MCP server is running now takes effect immediately. Previously the running file watcher kept the scope it had when it started, so a newly excluded file was removed by `codegraph sync` and then quietly re-added by the watcher seconds later — which looked like `exclude` not working at all — until the server was restarted. A scope change now refreshes the watcher and triggers a full reconcile, and a changed file the watcher hands to sync is re-checked against the current scope first, so the CLI and the live server can no longer disagree about what belongs in the index. Thanks @K1nG11. (#1590)
 - Indexing no longer crashes the whole process — a segmentation fault with no message and no partial index — on a C/C++ (or any other) file with extremely deep nesting, such as the parser stress-test fixtures in the clang and gcc test suites or a fuzzer corpus. Such a file is now handed to the fallback parser and recorded with a parse warning while the rest of the repository indexes normally. Thanks @apollo600 for the exact diagnosis. (#1581)
 - Calls to the methods of an exported object-literal constant — `export const api = { call() { … } }` used as a module's namespace, a common way to organize a TypeScript API surface — now resolve to the method, both in the defining file and through imports. Previously such a call linked to nothing (or to the constant itself), so `codegraph callers` and impact analysis reported zero callers for methods that are called from everywhere. Re-index after upgrading to pick up the edges. Thanks @IAliceBobI for the precise report and root-cause. (#1573)
 - Methods implemented in a generic or lifetime-parameterized `impl` block (`impl<T> Source for BufSource<T>`, `impl<'a> Iterator for Parents<'a>`) are now recorded under the implementing type instead of the trait. Previously such a method could not be found by its type — "who calls `BufSource::read`" had no answer — and it collided with the trait's own declaration, which could even invent a call-graph edge out of an impl body that contains no call at all. Impls on a reference (`impl Trait for &Foo`) and on a module-qualified type (`impl Trait for m::Foo`) are attributed to their type too. Re-index after upgrading. Thanks @Dshuishui. (#1588) (Rust)

+ 29 - 0
__tests__/sync.test.ts

@@ -851,4 +851,33 @@ describe('Scoped sync parity (#watcher-scoped)', () => {
     // b.ts untouched and still present
     expect(cg.searchNodes('beta').length).toBeGreaterThan(0);
   });
+
+  it('a scoped path that codegraph.json now excludes is removed, never re-parsed (#1590)', async () => {
+    // The daemon's watcher hands sync the exact edited path. If the project's
+    // scope changed underneath it, that path must be treated the way the full
+    // scan treats it — out of scope, hence gone — never parsed on trust.
+    const cfg = path.join(testDir, 'codegraph.json');
+    fs.writeFileSync(cfg, JSON.stringify({ exclude: ['src/b.ts'] }));
+    fs.writeFileSync(path.join(testDir, 'src', 'b.ts'), `export function beta() { return 2; }\nexport function gamma() { return 3; }`);
+    const scoped = await cg.sync({ paths: ['src/b.ts'] });
+    expect(scoped.filesRemoved).toBe(1);
+    expect(scoped.filesModified).toBe(0);
+    expect(scoped.filesAdded).toBe(0);
+    expect(cg.searchNodes('gamma').length).toBe(0);
+    expect(cg.searchNodes('beta').filter((r) => r.node.filePath === 'src/b.ts').length).toBe(0);
+    // Idempotent: the file stays out on a repeat scoped sync.
+    const again = await cg.sync({ paths: ['src/b.ts'] });
+    expect(again.filesRemoved).toBe(0);
+    expect(again.filesAdded).toBe(0);
+
+    // Dropping the exclude readmits it through the same scoped path. The
+    // scope matcher is mtime-keyed, so give the rewrite a distinct mtime even
+    // on a coarse-timestamp filesystem.
+    fs.writeFileSync(cfg, JSON.stringify({}));
+    const later = new Date(Date.now() + 5000);
+    fs.utimesSync(cfg, later, later);
+    const readmitted = await cg.sync({ paths: ['src/b.ts'] });
+    expect(readmitted.filesAdded).toBe(1);
+    expect(cg.searchNodes('gamma').length).toBe(1);
+  });
 });

+ 128 - 0
__tests__/watcher.test.ts

@@ -545,6 +545,134 @@ describe('FileWatcher', () => {
     });
   });
 
+  describe('scope config refresh (#1590)', () => {
+    // The matcher used to be built once in start() and kept for the watcher's
+    // lifetime, so a `codegraph.json` written AFTER the daemon started was
+    // invisible to the live watcher while `codegraph sync` honoured it: the
+    // CLI removed a newly excluded file and the watcher re-added it.
+    it('a codegraph.json edit rebuilds the matcher and forces a full sync', async () => {
+      const syncFn = vi.fn().mockResolvedValue({ filesChanged: 0, durationMs: 0 });
+      const watcher = newWatcher(syncFn, { debounceMs: 100 });
+      watcher.start();
+      await watcher.waitUntilReady();
+
+      // Scope the project after the watcher is already running.
+      fs.mkdirSync(path.join(testDir, 'skipme'));
+      fs.writeFileSync(path.join(testDir, 'skipme', 'b.ts'), 'export const b = 1;\n');
+      fs.writeFileSync(path.join(testDir, 'codegraph.json'), JSON.stringify({ exclude: ['skipme/'] }));
+      __emitWatchEventForTests(testDir, 'codegraph.json');
+
+      // The config edit schedules a FULL sync (no scoped path list): only the
+      // scan-diff can find the files the new scope drops or admits.
+      await waitFor(() => syncFn.mock.calls.length > 0);
+      expect(syncFn.mock.calls.length).toBe(1);
+      expect(syncFn.mock.calls[0]![0]).toBeUndefined();
+      expect(watcher.getPendingFiles()).toEqual([]);
+      await new Promise((r) => setTimeout(r, 50)); // let runSync settle
+
+      // An edit inside the newly excluded tree is dropped by the LIVE matcher:
+      // not pending, and no sync scheduled for it.
+      __emitWatchEventForTests(testDir, 'skipme/b.ts');
+      expect(watcher.getPendingFiles().map((p) => p.path)).not.toContain('skipme/b.ts');
+      await new Promise((r) => setTimeout(r, 300)); // > debounce
+      expect(syncFn.mock.calls.length).toBe(1);
+
+      // In-scope edits still sync, scoped to the edited path as before.
+      __emitWatchEventForTests(testDir, 'src/index.ts');
+      await waitFor(() => syncFn.mock.calls.length > 1);
+      expect(syncFn.mock.calls[1]![0]).toEqual(['src/index.ts']);
+
+      watcher.stop();
+    });
+
+    it('a root .gitignore edit is a scope change too', async () => {
+      const syncFn = vi.fn().mockResolvedValue({ filesChanged: 0, durationMs: 0 });
+      const watcher = newWatcher(syncFn, { debounceMs: 100 });
+      watcher.start();
+      await watcher.waitUntilReady();
+
+      fs.mkdirSync(path.join(testDir, 'gen'));
+      fs.writeFileSync(path.join(testDir, 'gen', 'out.ts'), 'export const g = 1;\n');
+      fs.writeFileSync(path.join(testDir, '.gitignore'), 'gen/\n');
+      __emitWatchEventForTests(testDir, '.gitignore');
+
+      await waitFor(() => syncFn.mock.calls.length > 0);
+      expect(syncFn.mock.calls[0]![0]).toBeUndefined();
+      await new Promise((r) => setTimeout(r, 50));
+
+      __emitWatchEventForTests(testDir, 'gen/out.ts');
+      expect(watcher.getPendingFiles().map((p) => p.path)).not.toContain('gen/out.ts');
+      await new Promise((r) => setTimeout(r, 300));
+      expect(syncFn.mock.calls.length).toBe(1);
+
+      watcher.stop();
+    });
+
+    it('a nested .gitignore inside the scope forces a full sync', async () => {
+      const syncFn = vi.fn().mockResolvedValue({ filesChanged: 0, durationMs: 0 });
+      const watcher = newWatcher(syncFn, { debounceMs: 100 });
+      watcher.start();
+      await watcher.waitUntilReady();
+
+      fs.mkdirSync(path.join(testDir, 'sub'));
+      fs.writeFileSync(path.join(testDir, 'sub', '.gitignore'), 'build/\n');
+      __emitWatchEventForTests(testDir, 'sub/.gitignore');
+
+      await waitFor(() => syncFn.mock.calls.length > 0);
+      expect(syncFn.mock.calls[0]![0]).toBeUndefined();
+
+      watcher.stop();
+    });
+
+    it('a .gitignore under an ignored tree (npm install churn) schedules nothing', async () => {
+      const syncFn = vi.fn().mockResolvedValue({ filesChanged: 0, durationMs: 0 });
+      const watcher = newWatcher(syncFn, { debounceMs: 100 });
+      watcher.start();
+      await watcher.waitUntilReady();
+
+      fs.mkdirSync(path.join(testDir, 'node_modules', 'pkg'), { recursive: true });
+      fs.writeFileSync(path.join(testDir, 'node_modules', 'pkg', '.gitignore'), 'lib/\n');
+      __emitWatchEventForTests(testDir, 'node_modules/pkg/.gitignore');
+
+      await new Promise((r) => setTimeout(r, 300));
+      expect(syncFn).not.toHaveBeenCalled();
+
+      watcher.stop();
+    });
+
+    it('removing the exclude again readmits the tree', async () => {
+      const syncFn = vi.fn().mockResolvedValue({ filesChanged: 0, durationMs: 0 });
+      const watcher = newWatcher(syncFn, { debounceMs: 100 });
+      watcher.start();
+      await watcher.waitUntilReady();
+
+      fs.mkdirSync(path.join(testDir, 'skipme'));
+      fs.writeFileSync(path.join(testDir, 'skipme', 'b.ts'), 'export const b = 1;\n');
+      const cfg = path.join(testDir, 'codegraph.json');
+      fs.writeFileSync(cfg, JSON.stringify({ exclude: ['skipme/'] }));
+      __emitWatchEventForTests(testDir, 'codegraph.json');
+      await waitFor(() => syncFn.mock.calls.length > 0);
+      await new Promise((r) => setTimeout(r, 50));
+      __emitWatchEventForTests(testDir, 'skipme/b.ts');
+      expect(watcher.getPendingFiles().map((p) => p.path)).not.toContain('skipme/b.ts');
+
+      // Drop the exclude. The loader is mtime-keyed, so make sure the second
+      // write carries a distinct mtime even on a coarse-timestamp filesystem.
+      fs.writeFileSync(cfg, JSON.stringify({}));
+      const later = new Date(Date.now() + 5000);
+      fs.utimesSync(cfg, later, later);
+      __emitWatchEventForTests(testDir, 'codegraph.json');
+      await waitFor(() => syncFn.mock.calls.length > 1);
+      expect(syncFn.mock.calls[1]![0]).toBeUndefined();
+      await new Promise((r) => setTimeout(r, 50));
+
+      __emitWatchEventForTests(testDir, 'skipme/b.ts');
+      expect(watcher.getPendingFiles().map((p) => p.path)).toContain('skipme/b.ts');
+
+      watcher.stop();
+    });
+  });
+
   describe('pending file tracking (#403)', () => {
     it('should expose edited paths via getPendingFiles before sync fires', async () => {
       // Slow debounce — pending entries are visible until the debounce fires.

+ 54 - 2
src/extraction/index.ts

@@ -27,7 +27,7 @@ import { StoreWriter, StoreBundle, finalizeStoreBundle } from './store-writer';
 import { materializeKernelResult } from './kernel';
 import { detectGeneratedFile } from './generated-detection';
 import { detectLanguage, isSourceFile, isLanguageSupported, isFileLevelOnlyLanguage, initGrammars, loadGrammarsForLanguages, readGrammarWasmBytes } from './grammars';
-import { loadExtensionOverrides, loadIncludeIgnoredPatterns, loadExcludePatterns, loadIncludePatterns } from '../project-config';
+import { loadExtensionOverrides, loadIncludeIgnoredPatterns, loadExcludePatterns, loadIncludePatterns, PROJECT_CONFIG_FILENAME } from '../project-config';
 import { isCodeGraphDataDir } from '../directory';
 import { logDebug, logWarn } from '../errors';
 import { validatePathWithinRoot, normalizePath } from '../utils';
@@ -1448,12 +1448,48 @@ export class ExtractionOrchestrator {
    * hasn't run yet so single-file re-index paths can detect on the spot.
    */
   private detectedFrameworkNames: string[] | null = null;
+  /**
+   * Scope matcher for SCOPED syncs, memoized on the mtimes of the two root
+   * files it is derived from (`codegraph.json`, `.gitignore`). See
+   * {@link scopedSyncMatcher}.
+   */
+  private scopedMatcher: { key: string; matcher: ScopeIgnore } | null = null;
 
   constructor(rootDir: string, queries: QueryBuilder) {
     this.rootDir = rootDir;
     this.queries = queries;
   }
 
+  /**
+   * The scope matcher a scoped sync applies to the paths it was handed — the
+   * same `buildScopeIgnore` the full scan uses, so an explicitly-passed path
+   * that is OUT of scope (a user `exclude` in `codegraph.json`, a `.gitignore`
+   * rule, a built-in default) is treated exactly as the full walk would treat
+   * it: absent, hence removed if tracked, never parsed (#1590).
+   *
+   * Memoized on the root config + root `.gitignore` mtimes: building the
+   * matcher runs embedded-repo discovery (`git ls-files`), which would defeat
+   * the scoped path's whole point (skipping O(repo) work) if paid per sync.
+   * Two `stat`s per sync while nothing changed. An embedded repo created
+   * between config edits joins the scoped matcher on the next full sync, the
+   * same lifecycle the watcher's own matcher already has.
+   */
+  private scopedSyncMatcher(): ScopeIgnore {
+    const key = [PROJECT_CONFIG_FILENAME, '.gitignore']
+      .map((name) => {
+        try {
+          return String(fs.statSync(path.join(this.rootDir, name)).mtimeMs);
+        } catch {
+          return '-';
+        }
+      })
+      .join('|');
+    if (this.scopedMatcher && this.scopedMatcher.key === key) return this.scopedMatcher.matcher;
+    const matcher = buildScopeIgnore(this.rootDir);
+    this.scopedMatcher = { key, matcher };
+    return matcher;
+  }
+
   /**
    * Build a filesystem-backed ResolutionContext sufficient for framework
    * detection. Graph-query methods (getNodesByName etc.) return empty because
@@ -2700,7 +2736,23 @@ export class ExtractionOrchestrator {
       // reads `filesChecked === 0 && durationMs === 0` as the
       // lock-unavailable signature (#449).
       const unique = [...new Set(scopedPaths)];
-      currentFiles = unique.filter((p) => fs.existsSync(path.join(this.rootDir, p)));
+      // A scoped path is "present" only if it exists AND is in scope — the
+      // same two gates the full walk applies (source extension, scope
+      // matcher). Without the scope gate a caller's stale view of scope
+      // leaked straight into the index: the watcher re-parsed a file the
+      // user had just excluded in `codegraph.json` while `codegraph sync`
+      // removed it (#1590). Out-of-scope paths fall out of `currentFiles`,
+      // so a tracked one takes the removal branch below, exactly as a full
+      // sync would treat it. (`include`-forced paths pass: ScopeIgnore
+      // applies the include precedence itself.)
+      const scope = this.scopedSyncMatcher();
+      const overrides = loadExtensionOverrides(this.rootDir);
+      currentFiles = unique.filter(
+        (p) =>
+          isSourceFile(p, overrides) &&
+          !scope.ignores(p) &&
+          fs.existsSync(path.join(this.rootDir, p))
+      );
       trackedFiles = [];
       for (const p of unique) {
         const rec = this.queries.getFileByPath(p);

+ 53 - 6
src/sync/watcher.ts

@@ -34,7 +34,7 @@
 import * as fs from 'fs';
 import * as path from 'path';
 import { isSourceFile, buildScopeIgnore, type ScopeIgnore } from '../extraction';
-import { loadExtensionOverrides } from '../project-config';
+import { loadExtensionOverrides, PROJECT_CONFIG_FILENAME } from '../project-config';
 import { logDebug, logWarn } from '../errors';
 import { normalizePath } from '../utils';
 import { isCodeGraphDataDir } from '../directory';
@@ -328,11 +328,13 @@ export class FileWatcher {
    * deterministically gate on watcher readiness.
    */
   private readyWaiters: Array<() => void> = [];
-  // The shared scope matcher (built-in defaults + project .gitignore, with
-  // embedded child repos matched by their OWN rules — #514), built once at
-  // start(). Same source of truth the indexer uses, so watcher scope can
-  // never diverge from index scope. An embedded repo created after start()
-  // joins the scope on the next watcher restart / re-index.
+  // The shared scope matcher (built-in defaults + project .gitignore + the
+  // `codegraph.json` exclude/include rules, with embedded child repos matched
+  // by their OWN rules — #514), built at start() and REBUILT whenever one of
+  // the files it is derived from changes (see `refreshScope`, #1590). Same
+  // source of truth the indexer uses, so watcher scope can never diverge from
+  // index scope. An embedded repo created after start() joins the scope on
+  // the next scope refresh / watcher restart / re-index.
   private ignoreMatcher: ScopeIgnore | null = null;
 
   private readonly projectRoot: string;
@@ -573,7 +575,24 @@ export class FileWatcher {
   private handleChange(rel: string): void {
     if (!rel || rel === '.' || rel.startsWith('..')) return;
     if (this.isAlwaysIgnored(rel)) return;
+    // The two root files the scope matcher is derived from are handled BEFORE
+    // the matcher is consulted: a user `exclude` pattern that happens to cover
+    // them (`*.json`, `.*`) must not be able to hide their own edits (#1590).
+    if (rel === PROJECT_CONFIG_FILENAME || rel === '.gitignore') {
+      this.refreshScope(rel);
+      return;
+    }
     if (this.ignoreMatcher && this.ignoreMatcher.ignores(rel)) return;
+    // A nested `.gitignore` (an embedded child repo's own rules, #514, or a
+    // subdirectory rule the git-backed full scan honors) is only a scope
+    // change when it sits INSIDE the current scope — checked after the matcher
+    // on purpose, so the thousands of package-local `.gitignore`s an
+    // `npm install` writes under an ignored `node_modules/` never trigger a
+    // rebuild storm.
+    if (rel.endsWith('/.gitignore')) {
+      this.refreshScope(rel);
+      return;
+    }
     if (!isSourceFile(rel, loadExtensionOverrides(this.projectRoot))) {
       this.maybeScheduleForRemovedDir(rel);
       return;
@@ -591,6 +610,34 @@ export class FileWatcher {
     this.scheduleSync();
   }
 
+  /**
+   * A scope-defining file changed (`codegraph.json`, a `.gitignore`): rebuild
+   * the ignore matcher and make the next sync a FULL reconcile (#1590).
+   *
+   * The matcher used to be built once in `start()` and kept for the watcher's
+   * lifetime — in a long-lived MCP daemon that meant a `codegraph.json`
+   * created or edited after startup was invisible to the live watcher, while
+   * `codegraph sync` (a fresh process) honoured it immediately: the CLI
+   * removed a newly excluded file and the watcher re-added it seconds later.
+   * `loadExtensionOverrides()` on the same filter line was already read live
+   * (mtime-cached), so two fields of the same config file disagreed.
+   *
+   * Rebuilding costs one `git ls-files` pass (embedded-repo discovery), which
+   * is fine per config edit — never per event. Replacing the field is enough
+   * for both strategies: the recursive handler and the per-directory
+   * `shouldIgnoreDir` walk read `this.ignoreMatcher` on every call. The full
+   * scan is required because a scope change has no per-file events: newly
+   * excluded files must be REMOVED from the index and newly included ones
+   * added, and only the scan-diff (which builds its own fresh matcher) knows
+   * which those are.
+   */
+  private refreshScope(rel: string): void {
+    logDebug('Scope config changed; rebuilding watcher scope', { file: rel });
+    this.ignoreMatcher = buildScopeIgnore(this.projectRoot);
+    this.needsFullScan = true;
+    this.scheduleSync();
+  }
+
   /**
    * A deleted DIRECTORY arrives as one event on the directory's own path —
    * no source extension, so the source-file filter drops it, and the files