Przeglądaj źródła

fix(status): preserve pending changes across commits and restores (#1878)

Preserve the indexed-commit approach and regression cases from #1848, with dirty-path retention, commit-race guards, NUL Git paths, and failed-I/O recovery.

Fixes #1829.

Co-authored-by: Eric Minish <eric.minish@gmail.com>
Colby Mchenry 6 dni temu
rodzic
commit
48711140e7
5 zmienionych plików z 511 dodań i 49 usunięć
  1. 1 0
      CHANGELOG.md
  2. 148 0
      __tests__/git-index-currency.test.ts
  3. 162 0
      __tests__/sync.test.ts
  4. 189 48
      src/extraction/index.ts
  5. 11 1
      src/index.ts

+ 1 - 0
CHANGELOG.md

@@ -159,6 +159,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 - Daemon startup and cleanup now preserve live legacy PID-only locks while still reclaiming dead or identity-disproved records, preventing two writers from serving the same project.
 - Incremental sync now keeps edge rebinding crash-safe: replacing a resolved edge with its recovery reference commits atomically, so an interruption cannot permanently remove the relationship.
+- Status now detects committed but unindexed changes and restored edits without scanning every source file; thanks @inth3shadows. (#1829)
 
 - The prompt hook no longer injects unrelated projects when run from your home directory or a broader directory containing a stray workspace manifest. (#1454)
 

+ 148 - 0
__tests__/git-index-currency.test.ts

@@ -0,0 +1,148 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import * as cp from 'child_process';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import { CodeGraph } from '../src';
+
+// Wrap only I/O entry points for deterministic failure injection; all other
+// calls, files, parser work and SQLite remain real.
+vi.mock('child_process', async importOriginal => {
+  const actual = await importOriginal<typeof import('child_process')>();
+  return { ...actual, execFileSync: vi.fn(actual.execFileSync) };
+});
+vi.mock('fs', async importOriginal => {
+  const actual = await importOriginal<typeof import('fs')>();
+  return { ...actual, readFileSync: vi.fn(actual.readFileSync) };
+});
+
+describe('git index currency across commits and restores (#1829)', () => {
+  let root: string;
+  let cg: CodeGraph;
+  const git = (...args: string[]) => cp.execFileSync('git', args, { cwd: root, encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'] });
+  const write = (name: string, symbol: string) => fs.writeFileSync(path.join(root, name), `export function ${symbol}() { return 1; }\n`);
+  const commit = () => { git('add', '-A'); git('commit', '-m', 'change'); return git('rev-parse', 'HEAD').trim(); };
+  const metadata = () => (cg as any).queries;
+  const symbols = (name: string) => cg.searchNodes(name).map(r => r.node.name);
+  const clean = () => expect(cg.getChangedFiles()).toEqual({ added: [], modified: [], removed: [] });
+
+  beforeEach(async () => {
+    root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-git-currency-'));
+    git('init'); git('config', 'user.email', 'test@example.invalid'); git('config', 'user.name', 'Test');
+    fs.writeFileSync(path.join(root, '.gitignore'), '.codegraph/\n');
+    write('source.ts', 'original'); commit();
+    cg = CodeGraph.initSync(root);
+    expect((await cg.indexAll()).success).toBe(true);
+  });
+  afterEach(() => { vi.restoreAllMocks(); cg?.close(); fs.rmSync(root, { recursive: true, force: true }); });
+
+  it.each(['index', 'sync', 'scoped'] as const)('sees a restored indexed dirty edit after %s', async mode => {
+    write('source.ts', 'dirtyVersion');
+    if (mode === 'index') await cg.indexAll();
+    else await cg.sync(mode === 'scoped' ? { paths: ['source.ts'] } : {});
+    expect(symbols('dirtyVersion')).toContain('dirtyVersion');
+    git('restore', 'source.ts');
+    expect(git('status', '--porcelain')).toBe('');
+    // Reopen proves that dirty candidates survive beyond one engine instance.
+    cg.close(); cg = CodeGraph.openSync(root);
+    expect(cg.getChangedFiles().modified).toEqual(['source.ts']);
+    await cg.sync();
+    expect(symbols('original')).toContain('original');
+    expect(symbols('dirtyVersion')).not.toContain('dirtyVersion'); clean();
+  });
+
+  it.each(['index', 'sync'] as const)('does not claim a commit made during %s maintenance', async mode => {
+    const oldHead = git('rev-parse', 'HEAD').trim();
+    write('source.ts', 'firstEdit');
+    const db = (cg as any).db;
+    const maintain = db.runMaintenance.bind(db);
+    const spy = vi.spyOn(db, 'runMaintenance').mockImplementationOnce(async () => {
+      write('source.ts', 'lateCommit'); commit();
+      return maintain();
+    });
+    if (mode === 'index') await cg.indexAll(); else await cg.sync();
+    expect(spy).toHaveBeenCalledOnce();
+    expect(metadata().getMetadata('indexed_at_commit')).toBe(oldHead);
+    expect(cg.getChangedFiles().modified).toEqual(['source.ts']);
+    await cg.sync(); expect(symbols('lateCommit')).toContain('lateCommit'); clean();
+  });
+
+  it('handles non-ASCII and quoted committed paths without git text escaping', async () => {
+    const names = ['тест.ts', 'space name.ts'];
+    if (process.platform !== 'win32') names.push('quote"name.ts');
+    for (const file of names) write(file, 'newSymbol');
+    commit();
+    expect(cg.getChangedFiles().added.sort()).toEqual(names.sort());
+    await cg.sync(); clean();
+    fs.renameSync(path.join(root, 'тест.ts'), path.join(root, 'renamed.ts')); commit();
+    expect(cg.getChangedFiles()).toEqual({ added: ['renamed.ts'], modified: [], removed: ['тест.ts'] });
+  });
+
+  it('falls back when git diff fails instead of claiming a clean index', () => {
+    write('new.ts', 'newSymbol'); commit();
+    const real = cp.execFileSync;
+    let injected = 0;
+    vi.spyOn(cp, 'execFileSync').mockImplementation(((file: string, args: string[], options: any) => {
+      if (file === 'git' && args[0] === 'diff') { injected++; throw new Error('Injected git diff timeout'); }
+      return real(file, args, options);
+    }) as typeof cp.execFileSync);
+    expect(cg.getChangedFiles().added).toEqual(['new.ts']);
+    expect(injected).toBeGreaterThan(0);
+  });
+
+  it('does not advance the commit stamp after a scoped sync', async () => {
+    const oldHead = metadata().getMetadata('indexed_at_commit');
+    write('one.ts', 'one'); write('two.ts', 'two'); commit();
+    await cg.sync({ paths: ['one.ts'] });
+    expect(metadata().getMetadata('indexed_at_commit')).toBe(oldHead);
+    expect(cg.getChangedFiles().added).toEqual(['two.ts']);
+    await cg.sync(); clean();
+  });
+
+  it('retains a restored dirty path when a later scoped sync touches another file', async () => {
+    write('source.ts', 'dirtyVersion'); await cg.sync();
+    git('restore', 'source.ts'); write('other.ts', 'other');
+    await cg.sync({ paths: ['other.ts'] });
+    expect(cg.getChangedFiles()).toEqual({ added: [], modified: ['source.ts'], removed: [] });
+    await cg.sync(); expect(symbols('original')).toContain('original'); clean();
+  });
+
+  it('does not call a recreated committed deletion removed when the current bytes match the DB', async () => {
+    fs.unlinkSync(path.join(root, 'source.ts')); commit();
+    write('source.ts', 'original');
+    clean();
+    write('source.ts', 'replacement');
+    expect(cg.getChangedFiles()).toEqual({ added: [], modified: ['source.ts'], removed: [] });
+    await cg.sync(); expect(symbols('replacement')).toContain('replacement'); clean();
+  });
+
+  it('retains deleted untracked files as candidates after they were indexed', async () => {
+    write('untracked.ts', 'temporary'); await cg.sync();
+    fs.unlinkSync(path.join(root, 'untracked.ts'));
+    expect(cg.getChangedFiles().removed).toEqual(['untracked.ts']);
+    await cg.sync(); expect(symbols('temporary')).not.toContain('temporary'); clean();
+  });
+
+  it('never advances freshness after a failed full index', async () => {
+    write('new.ts', 'newSymbol'); commit();
+    const controller = new AbortController(); controller.abort();
+    expect((await cg.indexAll({ signal: controller.signal })).success).toBe(false);
+    expect(cg.getChangedFiles().added).toEqual(['new.ts']);
+  });
+
+  it('keeps a committed path pending when sync cannot read it', async () => {
+    write('new.ts', 'newSymbol'); commit();
+    const real = fs.readFileSync;
+    let injected = 0;
+    vi.spyOn(fs, 'readFileSync').mockImplementation(((file: any, ...args: any[]) => {
+      if (String(file) === path.join(root, 'new.ts')) { injected++; throw new Error('Injected transient read error'); }
+      return (real as any)(file, ...args);
+    }) as typeof fs.readFileSync);
+    await cg.sync();
+    expect(injected).toBeGreaterThan(0);
+    expect(symbols('newSymbol')).not.toContain('newSymbol');
+    vi.restoreAllMocks();
+    expect(cg.getChangedFiles().added).toEqual(['new.ts']);
+    await cg.sync(); clean();
+  });
+});

+ 162 - 0
__tests__/sync.test.ts

@@ -881,3 +881,165 @@ describe('Scoped sync parity (#watcher-scoped)', () => {
     expect(cg.searchNodes('gamma').length).toBe(1);
   });
 });
+
+// A change that is COMMITTED but not yet indexed used to read as zero pending
+// changes: getChangedFiles' git fast path built its candidate list from
+// `git status --porcelain`, and committing is exactly what removes a file from
+// that output. The hash comparison below it was correct and simply never
+// reached. Committed work is now sourced from `git diff <indexed commit> HEAD`.
+// (#1829)
+describe('committed-but-unindexed changes (#1829)', () => {
+  let testDir: string;
+  let cg: CodeGraph;
+
+  const git = (...args: string[]) =>
+    execFileSync('git', args, { cwd: testDir, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] });
+
+  beforeEach(async () => {
+    testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-1829-'));
+    git('init');
+    git('config', 'user.email', 'test@test.com');
+    git('config', 'user.name', 'Test');
+
+    fs.mkdirSync(path.join(testDir, 'src'));
+    fs.writeFileSync(path.join(testDir, 'src', 'one.ts'), `export function alpha() { return 1; }`);
+    git('add', '-A');
+    git('commit', '-m', 'initial');
+
+    cg = CodeGraph.initSync(testDir, { config: { include: ['**/*.ts'], exclude: [] } });
+    await cg.indexAll();
+  });
+
+  afterEach(() => {
+    if (cg) cg.destroy();
+    if (fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true });
+  });
+
+  it('sees a committed NEW file (git status shows nothing; the DB has no row)', async () => {
+    fs.writeFileSync(path.join(testDir, 'src', 'two.ts'), `export function beta() { return 2; }`);
+    git('add', '-A');
+    git('commit', '-m', 'add two');
+
+    const changes = cg.getChangedFiles();
+    expect(changes.added).toContain('src/two.ts');
+
+    // status and sync must agree — the whole point is that the number a user
+    // reads matches the work that is actually outstanding.
+    const result = await cg.sync();
+    expect(result.filesAdded).toBe(1);
+    expect(cg.searchNodes('beta').length).toBeGreaterThan(0);
+  });
+
+  it('sees a committed MODIFICATION to an already-tracked file', async () => {
+    fs.writeFileSync(path.join(testDir, 'src', 'one.ts'), `export function alphaRenamed() { return 99; }`);
+    git('add', '-A');
+    git('commit', '-m', 'edit one');
+
+    expect(cg.getChangedFiles().modified).toContain('src/one.ts');
+    const result = await cg.sync();
+    expect(result.filesModified).toBe(1);
+    expect(cg.searchNodes('alphaRenamed').length).toBeGreaterThan(0);
+  });
+
+  it('sees a committed DELETE', async () => {
+    fs.writeFileSync(path.join(testDir, 'src', 'two.ts'), `export function beta() { return 2; }`);
+    git('add', '-A');
+    git('commit', '-m', 'add two');
+    await cg.sync();
+
+    fs.rmSync(path.join(testDir, 'src', 'two.ts'));
+    git('add', '-A');
+    git('commit', '-m', 'remove two');
+
+    expect(cg.getChangedFiles().removed).toContain('src/two.ts');
+    const result = await cg.sync();
+    expect(result.filesRemoved).toBe(1);
+  });
+
+  it('reports zero once the sync has absorbed the commit (the stamp advances)', async () => {
+    fs.writeFileSync(path.join(testDir, 'src', 'two.ts'), `export function beta() { return 2; }`);
+    git('add', '-A');
+    git('commit', '-m', 'add two');
+    await cg.sync();
+
+    const after = cg.getChangedFiles();
+    expect(after.added).toHaveLength(0);
+    expect(after.modified).toHaveLength(0);
+    expect(after.removed).toHaveLength(0);
+  });
+
+  it('counts a file once when it was committed AND edited again since', async () => {
+    // The same path now reaches the candidate list from both sources — the
+    // committed diff and `git status`. It is still one changed file.
+    fs.writeFileSync(path.join(testDir, 'src', 'one.ts'), `export function alpha() { return 2; }`);
+    git('add', '-A');
+    git('commit', '-m', 'edit one');
+    fs.writeFileSync(path.join(testDir, 'src', 'one.ts'), `export function alpha() { return 3; }`);
+
+    const changes = cg.getChangedFiles();
+    expect(changes.modified.filter((f) => f === 'src/one.ts')).toHaveLength(1);
+    expect(changes.added).toHaveLength(0);
+
+    const result = await cg.sync();
+    expect(result.filesModified).toBe(1);
+  });
+
+  it('sees a committed RENAME as a removal plus an add', async () => {
+    // `--no-renames` on the committed diff is deliberate: the index keys files
+    // by path, so a rename IS a removal and an add, and pairing them up would
+    // only have to be taken apart again.
+    fs.renameSync(path.join(testDir, 'src', 'one.ts'), path.join(testDir, 'src', 'renamed.ts'));
+    git('add', '-A');
+    git('commit', '-m', 'rename one');
+
+    const changes = cg.getChangedFiles();
+    expect(changes.removed).toContain('src/one.ts');
+    expect(changes.added).toContain('src/renamed.ts');
+
+    const result = await cg.sync();
+    expect(result.filesRemoved).toBe(1);
+    expect(result.filesAdded).toBe(1);
+    expect(cg.searchNodes('alpha').every((r) => r.node.filePath !== 'src/one.ts')).toBe(true);
+  });
+
+  it('still filters committed changes by the rules the full index uses', async () => {
+    // vendor/ is a built-in exclude git knows nothing about. Sourcing candidates
+    // from `git diff` must not smuggle in files `git status` would have had
+    // filtered out (#766) — same classifier, both sources.
+    fs.mkdirSync(path.join(testDir, 'vendor'));
+    fs.writeFileSync(path.join(testDir, 'vendor', 'lib.ts'), `export function vendored() { return 1; }`);
+    git('add', '-A');
+    git('commit', '-m', 'add vendor');
+
+    const changes = cg.getChangedFiles();
+    expect(changes.added).not.toContain('vendor/lib.ts');
+    expect(changes.modified).not.toContain('vendor/lib.ts');
+  });
+
+  it('falls back to the full scan when history moved under the index', async () => {
+    // A rebase/gc/shallow clone can leave the stamped commit unreachable. The
+    // fast path cannot diff against a commit that is gone, so the (correct,
+    // slower) full scan has to answer instead of silently reporting zero.
+    fs.writeFileSync(path.join(testDir, 'src', 'two.ts'), `export function beta() { return 2; }`);
+    git('add', '-A');
+    git('commit', '-m', 'add two');
+    (cg as unknown as { queries: { setMetadata(k: string, v: string): void } })
+      .queries.setMetadata('indexed_at_commit', '0'.repeat(40));
+
+    expect(cg.getChangedFiles().added).toContain('src/two.ts');
+  });
+
+  it('an index with no stamp still answers correctly (pre-#1829 index upgrading)', async () => {
+    (cg as unknown as { queries: { setMetadata(k: string, v: string): void } })
+      .queries.setMetadata('indexed_at_commit', '');
+    fs.writeFileSync(path.join(testDir, 'src', 'two.ts'), `export function beta() { return 2; }`);
+    git('add', '-A');
+    git('commit', '-m', 'add two');
+
+    expect(cg.getChangedFiles().added).toContain('src/two.ts');
+
+    // ...and it self-heals: the sync writes a stamp, so the next read is clean.
+    await cg.sync();
+    expect(cg.getChangedFiles().added).toHaveLength(0);
+  });
+});

+ 189 - 48
src/extraction/index.ts

@@ -126,6 +126,8 @@ export interface SyncResult {
   nodesUpdated: number;
   durationMs: number;
   changedFilePaths?: string[];
+  /** Paths not absorbed because reading or extraction failed; retain for status/retry. */
+  failedFilePaths?: string[];
   /**
    * Symbol names whose set of definitions this sync CHANGED — names the synced
    * files gained or lost, as the symmetric difference of their `file\0name`
@@ -1287,20 +1289,93 @@ interface GitChanges {
  * case this cannot see (the child status that would report the deletions is gone
  * with it); a full `codegraph index` reconciles that.
  */
-export function getGitChangedFiles(rootDir: string): GitChanges | null {
+export function getGitChangedFiles(rootDir: string, sinceCommit?: string | null): GitChanges | null {
   try {
+    // `git status` only ever describes the WORKING TREE, so a change that has
+    // been committed leaves no entry and never enters the candidate set — the
+    // hash comparison in getChangedFiles is correct but is never reached for
+    // it, and `pendingChanges` reads 0 while the index is genuinely behind
+    // (#1829). `sinceCommit` — the commit the index was last brought up to
+    // date at — adds the other half: what has been committed since. Callers
+    // that hold no such stamp still get exactly what they always did, the
+    // working-tree changes.
     const changes: GitChanges = { modified: [], added: [], deleted: [] };
     // Custom extension → language overrides from the project's codegraph.json,
     // so change detection sees the same custom-extension files the full index does.
     const overrides = loadExtensionOverrides(rootDir);
-    collectGitStatus(rootDir, '', changes, overrides, loadIncludeIgnoredMatcher(rootDir), loadExcludeMatcher(rootDir));
+    collectGitStatus(rootDir, '', changes, overrides, loadIncludeIgnoredMatcher(rootDir), loadExcludeMatcher(rootDir), sinceCommit ?? undefined);
     return changes;
   } catch {
     return null;
   }
 }
 
-function collectGitStatus(repoDir: string, prefix: string, out: GitChanges, overrides?: Record<string, Language>, includeIgnored: Ignore | null = null, exclude: Ignore | null = null): void {
+/**
+ * Metadata key: the commit the index was last brought up to date at. Written by
+ * a full index AND by every successful sync — unlike the extraction stamp, which
+ * a sync must not advance because it only touches a subset of files. This one is
+ * about the tree; failed file paths remain explicit retry candidates. (#1829)
+ */
+export const INDEXED_AT_COMMIT_KEY = 'indexed_at_commit';
+
+/** HEAD's commit sha, or null in a non-git repo or one with no commits yet. */
+export function getGitHeadSha(rootDir: string): string | null {
+  try {
+    return execFileSync('git', ['rev-parse', 'HEAD'], {
+      cwd: rootDir, encoding: 'utf-8', timeout: 5000, stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true,
+    }).trim() || null;
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * NUL-delimited status/path pairs for every path committed
+ * between `sinceCommit` and HEAD. Empty when the stamp IS HEAD, which is the
+ * common case — one cheap git call on the hot path.
+ */
+function gitCommittedChangesSince(repoDir: string, sinceCommit: string): string[] {
+  // NUL framing preserves Unicode, quotes, tabs and newlines in Git paths.
+  // Let command failures reach getGitChangedFiles: [] would falsely mean clean.
+  const out = execFileSync('git', ['diff', '--relative', '--name-status', '--no-renames', '-z', sinceCommit, 'HEAD', '--', '.'], {
+    cwd: repoDir, encoding: 'utf-8', timeout: 10000, maxBuffer: 50 * 1024 * 1024, stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true,
+  });
+  return out.split('\0');
+}
+
+/**
+ * Can an INDEX trust the git fast path, given the commit it was built at?
+ *
+ * False means "fall back to the full scan" — the expensive path that compares
+ * every file on disk against the DB, and the only correct read when git cannot
+ * say what happened between the stamp and now:
+ *
+ *  - stamp present but unknown to this repo (rebase, gc, shallow clone, a stamp
+ *    from a different checkout) — history moved under the index.
+ *  - stamp absent while the repo HAS commits — an index built before stamping
+ *    existed. One full scan; the next sync stamps it and the fast path returns.
+ *
+ * A repo with NO commits keeps the fast path with or without a stamp: every
+ * file is untracked, so `git status` already sees all of them. Callers with no
+ * index behind them (the exported `getGitChangedFiles`) never ask this — a
+ * working-tree diff is the whole of what they wanted. (#1829)
+ */
+export function canTrustGitFastPath(rootDir: string, sinceCommit?: string | null): boolean {
+  const head = getGitHeadSha(rootDir);
+  if (head == null) return true; // no commits (or not a git repo — caller handles that)
+  if (!sinceCommit) return false;
+  if (sinceCommit === head) return true;
+  try {
+    execFileSync('git', ['cat-file', '-e', `${sinceCommit}^{commit}`], {
+      cwd: rootDir, encoding: 'utf-8', timeout: 5000, stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true,
+    });
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+function collectGitStatus(repoDir: string, prefix: string, out: GitChanges, overrides?: Record<string, Language>, includeIgnored: Ignore | null = null, exclude: Ignore | null = null, sinceCommit?: string): void {
   const output = execFileSync(
     'git',
     // `-uall` lists individual untracked files instead of collapsing an
@@ -1309,7 +1384,7 @@ function collectGitStatus(repoDir: string, prefix: string, out: GitChanges, over
     // below). Nested untracked git repos still collapse to `?? repo/` even
     // with `-uall` — git never crosses a repo boundary — so the recursion
     // still handles them. (#1213)
-    ['status', '--porcelain', '--no-renames', '-uall'],
+    ['status', '--porcelain', '--no-renames', '-z', '-uall'],
     { cwd: repoDir, encoding: 'utf-8', timeout: 10000, maxBuffer: 50 * 1024 * 1024, stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true }
   );
 
@@ -1325,46 +1400,66 @@ function collectGitStatus(repoDir: string, prefix: string, out: GitChanges, over
   // parent's. (#766)
   const ig = buildDefaultIgnore(repoDir);
 
-  const untrackedDirs: string[] = [];
-  for (const line of output.split('\n')) {
-    if (line.length < 4) continue; // Minimum: "XY file"
-
-    const statusCode = line.substring(0, 2);
-    const rel = normalizePath(line.substring(3));
-
-    // Untracked directory entries (trailing slash) may hide an embedded repo —
-    // collect for the recursion below instead of treating as a file.
-    if (statusCode === '??' && rel.endsWith('/')) {
-      untrackedDirs.push(rel);
-      continue;
-    }
-
+  // One classifier for both candidate sources below, so a committed change is
+  // filtered by exactly the rules a working-tree change is (#766, #999, #1829).
+  const classify = (statusCode: string, rel: string): void => {
     const filePath = normalizePath(prefix + rel);
-    if (!isSourceFile(filePath, overrides)) continue;
+    if (!isSourceFile(filePath, overrides)) return;
 
     if (statusCode.includes('D')) {
       // Deletions stay unfiltered: getChangedFiles acts on one only when the
       // path is already tracked in the DB, where removal is always correct — and
       // that lets a newly-excluded dir's stale rows clean themselves up. (#766)
       out.deleted.push(filePath);
-      continue;
+      return;
     }
 
     // Added (`??`) / modified files inside an excluded dir must not enter the
     // index — match against the repo-relative path, same as the full scan. (#766)
-    if (ig.ignores(rel)) continue;
+    if (ig.ignores(rel)) return;
     // User `codegraph.json` `exclude` (#999) is project-root-relative, so it's
     // matched against the full path — sync must not re-add a tracked file the
     // full index now keeps out. Deletions above stay unfiltered so a file that
     // WAS indexed before an exclude was added still cleans itself out.
-    if (exclude && exclude.ignores(filePath)) continue;
+    if (exclude && exclude.ignores(filePath)) return;
 
     if (statusCode === '??') {
       out.added.push(filePath);
     } else {
-      // M, MM, AM, A (staged), etc. — treat as modified
+      // M, MM, AM, A (staged), etc. — treat as modified. getChangedFiles
+      // re-decides added-vs-modified from the DB, so a committed `A` that the
+      // index never saw still lands in `added`.
       out.modified.push(filePath);
     }
+  };
+
+  const untrackedDirs: string[] = [];
+  for (const line of output.split('\0')) {
+    if (line.length < 4) continue; // Minimum: "XY file"
+
+    const statusCode = line.substring(0, 2);
+    const rel = normalizePath(line.substring(3));
+
+    // Untracked directory entries (trailing slash) may hide an embedded repo —
+    // collect for the recursion below instead of treating as a file.
+    if (statusCode === '??' && rel.endsWith('/')) {
+      untrackedDirs.push(rel);
+      continue;
+    }
+
+    classify(statusCode, rel);
+  }
+
+  // Committed but unindexed: everything between the commit this index was last
+  // brought up to date at and HEAD. `git status` cannot see these — committing
+  // is precisely what removes a file from its output — so without this pass a
+  // `git commit` makes a real pending change read as zero (#1829). The stamp
+  // belongs to the ROOT repo, so the embedded-repo recursion below passes none.
+  if (sinceCommit) {
+    const fields = gitCommittedChangesSince(repoDir, sinceCommit);
+    for (let i = 0; i + 1 < fields.length; i += 2) {
+      classify(`${fields[i]!.charAt(0)} `, normalizePath(fields[i + 1]!));
+    }
   }
 
   // Recurse embedded repos found under untracked dirs (at the dir itself or
@@ -2957,6 +3052,7 @@ export class ExtractionOrchestrator {
     });
 
     const filesToIndex: string[] = [];
+    const failedFilePaths: string[] = [];
     // === Filesystem reconcile (git-independent) ===
     // The source of truth for "what changed" is the filesystem vs the indexed
     // state — never git. We enumerate the current source files and reconcile
@@ -3082,6 +3178,7 @@ export class ExtractionOrchestrator {
           }
         } catch (error) {
           logDebug('Skipping unstattable file during sync', { filePath, error: String(error) });
+          failedFilePaths.push(filePath);
           continue;
         }
       }
@@ -3092,6 +3189,7 @@ export class ExtractionOrchestrator {
         content = fs.readFileSync(fullPath, 'utf-8');
       } catch (error) {
         logDebug('Skipping unreadable file during sync', { filePath, error: String(error) });
+        failedFilePaths.push(filePath);
         continue;
       }
       const contentHash = hashContent(content);
@@ -3133,6 +3231,7 @@ export class ExtractionOrchestrator {
       });
 
       const result = await this.indexFile(filePath);
+      if (result.errors.some(e => e.severity === 'error')) failedFilePaths.push(filePath);
       nodesUpdated += result.nodes.length;
 
       const pause = backpressure?.();
@@ -3166,16 +3265,67 @@ export class ExtractionOrchestrator {
       nodesUpdated,
       durationMs: Date.now() - startTime,
       changedFilePaths: changedFilePaths.length > 0 ? changedFilePaths : undefined,
+      ...(failedFilePaths.length > 0 ? { failedFilePaths } : {}),
       definitionDelta: definitionDelta.length > 0 ? definitionDelta : undefined,
     };
   }
 
+  private indexedDirtyPaths(stamp: string | null): string[] | null {
+    try {
+      const state = JSON.parse(this.queries.getMetadata('indexed_dirty_paths') ?? 'null');
+      if (!state || state.commit !== (stamp ?? '') || !Array.isArray(state.paths)) return null;
+      if (!state.paths.every((p: unknown) => typeof p === 'string' && p.length > 0 &&
+        !path.isAbsolute(p) && !p.split('/').includes('..'))) return null;
+      return state.paths;
+    } catch { return null; }
+  }
+
+  /** Capture before file reads. In-flight/failed full writes must not claim freshness. */
+  beginGitIndexState(full: boolean): { head: string; stamp: string; dirty: string[] | null } {
+    const head = getGitHeadSha(this.rootDir) ?? '';
+    const stamp = this.queries.getMetadata(INDEXED_AT_COMMIT_KEY) ?? '';
+    const prior = this.indexedDirtyPaths(stamp);
+    const status = getGitChangedFiles(this.rootDir);
+    const dirty = status ? [...new Set([
+      ...(full ? [] : prior ?? []), ...status.added, ...status.modified, ...status.deleted,
+    ])] : null;
+    if (full || prior === null || dirty === null) {
+      this.queries.setMetadata(INDEXED_AT_COMMIT_KEY, '');
+      this.queries.setMetadata('indexed_dirty_paths', '');
+    } else {
+      // Scoped writes leave the commit alone and retain dirty paths before the
+      // first write, so a crash cannot forget an indexed uncommitted edit.
+      this.queries.setMetadata('indexed_dirty_paths', JSON.stringify({ commit: stamp, paths: dirty }));
+    }
+    return { head, stamp, dirty };
+  }
+
+  finishGitIndexState(snapshot: { head: string; stamp: string; dirty: string[] | null }, full: boolean, retries: string[] = []): void {
+    const after = getGitChangedFiles(this.rootDir);
+    if (!snapshot.dirty || !after) return;
+    const commit = full ? snapshot.head : snapshot.stamp;
+    const paths = [...new Set([...snapshot.dirty, ...after.added, ...after.modified, ...after.deleted, ...retries])].sort();
+    // The embedded commit makes a torn pair fail closed: readers reject a dirty
+    // set that doesn't match the separately stored commit. Write the set first.
+    this.queries.setMetadata('indexed_dirty_paths', JSON.stringify({ commit, paths }));
+    if (full) this.queries.setMetadata(INDEXED_AT_COMMIT_KEY, commit);
+  }
+
   /**
    * Get files that have changed since last index.
    * Uses git status as a fast path when available, falling back to full scan.
    */
   getChangedFiles(): { added: string[]; modified: string[]; removed: string[] } {
-    const gitChanges = getGitChangedFiles(this.rootDir);
+    // The commit this index was last brought up to date at. Absent on an index
+    // built before stamping existed — getGitChangedFiles then declines the fast
+    // path and the full scan below answers correctly, once, until a sync or a
+    // full index writes the stamp. (#1829)
+    let sinceCommit: string | null = null;
+    try { sinceCommit = this.queries.getMetadata(INDEXED_AT_COMMIT_KEY) ?? null; } catch { /* advisory */ }
+    const dirtyPaths = this.indexedDirtyPaths(sinceCommit);
+    const gitChanges = dirtyPaths !== null && canTrustGitFastPath(this.rootDir, sinceCommit)
+      ? getGitChangedFiles(this.rootDir, sinceCommit)
+      : null;
 
     if (gitChanges) {
       // === Git fast path ===
@@ -3183,36 +3333,27 @@ export class ExtractionOrchestrator {
       const modified: string[] = [];
       const removed: string[] = [];
 
-      // Deleted files — only report if tracked in DB
-      for (const filePath of gitChanges.deleted) {
+      // Git supplies candidates, never the verdict. A committed deletion may
+      // have been recreated locally; a previously indexed dirty path may have
+      // vanished from git status after restore. Classify current disk vs DB once.
+      const candidates = new Set([...gitChanges.deleted, ...gitChanges.modified, ...gitChanges.added, ...dirtyPaths!]);
+      const scope = this.scopedSyncMatcher();
+      const overrides = loadExtensionOverrides(this.rootDir);
+      for (const filePath of candidates) {
         const tracked = this.queries.getFileByPath(filePath);
-        if (tracked) {
-          removed.push(filePath);
-        }
-      }
-
-      // Modified + added files — read + hash, compare with DB. Untracked (`??`)
-      // files stay untracked in git even after indexing, so they must be
-      // hash-compared like modified files instead of always counting as added —
-      // otherwise status reports them as pending forever. (See issue #206.)
-      for (const filePath of [...gitChanges.modified, ...gitChanges.added]) {
         const fullPath = path.join(this.rootDir, filePath);
+        if (!isSourceFile(filePath, overrides) || scope.ignores(filePath) || !fs.existsSync(fullPath)) {
+          if (tracked) removed.push(filePath);
+          continue;
+        }
         let content: string;
-        try {
-          content = fs.readFileSync(fullPath, 'utf-8');
-        } catch (error) {
+        try { content = fs.readFileSync(fullPath, 'utf-8'); }
+        catch (error) {
           logDebug('Skipping unreadable file while detecting changes', { filePath, error: String(error) });
           continue;
         }
-
-        const contentHash = hashContent(content);
-        const tracked = this.queries.getFileByPath(filePath);
-
-        if (!tracked) {
-          added.push(filePath);
-        } else if (tracked.contentHash !== contentHash) {
-          modified.push(filePath);
-        }
+        if (!tracked) added.push(filePath);
+        else if (tracked.contentHash !== hashContent(content)) modified.push(filePath);
       }
 
       return { added, modified, removed };

+ 11 - 1
src/index.ts

@@ -517,6 +517,7 @@ export class CodeGraph {
         walValve.start();
       }
       try {
+        const gitState = this.orchestrator.beginGitIndexState(true);
         const before = this.queries.getNodeAndEdgeCount();
         // Mark the index as in-flight BEFORE any writes: a run killed
         // mid-index (OOM, SIGKILL, the #850 liveness watchdog) leaves this
@@ -695,6 +696,11 @@ export class CodeGraph {
           } catch { /* metadata is advisory — never fail an index over it */ }
         }
 
+        if (result.success && result.filesErrored === 0 &&
+          (result.filesDiscovered === undefined || result.filesIndexed + result.filesSkipped >= result.filesDiscovered)) {
+          this.orchestrator.finishGitIndexState(gitState, true);
+        }
+
         // Reconcile the scan's ground truth against what the pipeline
         // accounted for. A shortfall means files were silently dropped
         // (observed in the wild: a run under heavy load came up 37 files
@@ -822,6 +828,9 @@ export class CodeGraph {
         // timer-driven PASSIVE checkpoints ran, and a query-pool reader could
         // pin frames while the WAL grew without a bound.
         const backpressure = walValve ? () => walValve!.backpressure() : undefined;
+        const fullReconcile = !options.paths || options.paths.length === 0;
+        const gitState = this.orchestrator.beginGitIndexState(fullReconcile);
+
         const result = await this.orchestrator.sync(options.onProgress, options.paths, backpressure);
 
         // Fold the store phase's WAL BEFORE the post-store reads below
@@ -1023,11 +1032,12 @@ export class CodeGraph {
         // A killed full index leaves this marker at `indexing`. Sync repairs
         // missing files, pending refs, and (on open) dropped indexes, so a
         // successful recovery must also close the metadata state (#1556).
-        const fullReconcile = !options.paths || options.paths.length === 0;
         if (fullReconcile && this.getIndexState() === 'indexing') {
           try { this.queries.setMetadata('index_state', 'complete'); } catch { /* advisory */ }
         }
 
+        this.orchestrator.finishGitIndexState(gitState, fullReconcile, result.failedFilePaths);
+
         return result;
       } finally {
         // Mirror indexAll's teardown: stop the valve, then restore the