diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d8ab6f79..689d0bd9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- `codegraph status` now detects source changes introduced by commits, pulls, merges, and branch switches instead of incorrectly reporting a stale index as up to date. + - C, C++, Objective-C and Rust unions are now indexed as first-class `union` nodes. A `union` declaration previously produced no symbol at all, so it never appeared in search or `codegraph_explore`, and anything attached to it disappeared with it — in Rust, every `impl SomeTrait for MyUnion` lost its edge, the methods from that impl were left pointing at a type the graph did not contain, and asking which types implement a trait quietly skipped the union ones. A union-shaped dispatch table in C now resolves its function pointers like a struct-shaped one. A `typedef union { … } Name;` in C keeps the typedef's name and is no longer mistaken for a plain type alias. Thanks @ctype-lab. Re-index after upgrading to pick up unions in existing projects. (#1515) - A long-lived index no longer drifts away from what a fresh `codegraph index` would produce. When a file gained or lost a symbol, references to that name in files the sync never touched kept pointing at the definition that was correct before the change, and — because nothing distinguished two same-named definitions — the winner could come down to the order files happened to be written, which differs between a full index and a sync. On this project's own repository, replaying 80 commits through `sync` left 5.7% of connections wrong; it is now 1.3%, and the wrong-answers-still-being-asserted half drops by 99.7%. Since call edges are what flow questions follow and what `codegraph_explore` ranks files by, this quietly degraded answers as an index aged, with nothing to indicate it. Syncing is unchanged in speed, and an edit that only changes a function's body does no extra work at all. Set `CODEGRAPH_NO_REBIND=1` to opt out. diff --git a/__tests__/status-json.test.ts b/__tests__/status-json.test.ts index 292ca209f..1095c01b8 100644 --- a/__tests__/status-json.test.ts +++ b/__tests__/status-json.test.ts @@ -86,6 +86,29 @@ describe('codegraph status --json — CI fields (#329)', () => { expect(ms).toBeGreaterThanOrEqual(before - 1000); expect(ms).toBeLessThanOrEqual(after + 1000); }); + + it('status --json reports source files added by a clean commit', async () => { + execFileSync('git', ['init'], { cwd: tempDir, stdio: 'pipe' }); + execFileSync('git', ['config', 'user.email', 'test@test.com'], { cwd: tempDir }); + execFileSync('git', ['config', 'user.name', 'Test'], { cwd: tempDir }); + fs.writeFileSync(path.join(tempDir, 'a.ts'), 'export const x = 1;\n'); + execFileSync('git', ['add', '-A'], { cwd: tempDir }); + execFileSync('git', ['commit', '-m', 'initial'], { cwd: tempDir, stdio: 'pipe' }); + + const cg = CodeGraph.initSync(tempDir); + await cg.indexAll(); + cg.close(); + + fs.writeFileSync(path.join(tempDir, 'new.ts'), 'export const newlyCommitted = 2;\n'); + execFileSync('git', ['add', '-A'], { cwd: tempDir }); + execFileSync('git', ['commit', '-m', 'add source'], { cwd: tempDir, stdio: 'pipe' }); + expect( + execFileSync('git', ['status', '--porcelain'], { cwd: tempDir, encoding: 'utf-8' }) + ).toBe(''); + + const out = runStatusJson(tempDir); + expect(out.pendingChanges).toEqual({ added: 1, modified: 0, removed: 0 }); + }); }); describe('index completeness marker (index_state)', () => { diff --git a/__tests__/sync.test.ts b/__tests__/sync.test.ts index f26c05e1f..84494112f 100644 --- a/__tests__/sync.test.ts +++ b/__tests__/sync.test.ts @@ -302,13 +302,49 @@ describe('Sync Module', () => { expect(result.filesRemoved).toBe(0); expect(result.changedFilePaths).toBeUndefined(); }); + + it('should detect changes introduced by clean commits', async () => { + fs.writeFileSync( + path.join(testDir, 'src', 'new.ts'), + `export function newFunc() { return 42; }` + ); + fs.writeFileSync( + path.join(testDir, 'src', 'index.ts'), + `export function hello() { return 'committed'; }` + ); + git('add', '-A'); + git('commit', '-m', 'add and modify sources'); + + expect( + execFileSync('git', ['status', '--porcelain'], { cwd: testDir, encoding: 'utf-8' }) + ).toBe(''); + expect(cg.getChangedFiles()).toEqual({ + added: ['src/new.ts'], + modified: ['src/index.ts'], + removed: [], + }); + + await cg.sync(); + fs.unlinkSync(path.join(testDir, 'src', 'index.ts')); + git('add', '-A'); + git('commit', '-m', 'remove source'); + + expect( + execFileSync('git', ['status', '--porcelain'], { cwd: testDir, encoding: 'utf-8' }) + ).toBe(''); + expect(cg.getChangedFiles()).toEqual({ + added: [], + modified: [], + removed: ['src/index.ts'], + }); + }); }); - // Incremental sync's git fast path used to consume `git status` output without - // the ignore matcher the full index applies — so a committed dependency dir - // (built-in default exclude) or a tracked file under a .gitignored dir would - // leak into the index via `sync`, then vanish on the next `index --force`. The - // git fast path must exclude exactly what the full scan does. (#766) + // Change detection once consumed `git status` through a separate path without + // the full index's ignore matcher. A committed dependency dir or tracked file + // under a .gitignored dir could then appear as pending even though filesystem + // reconciliation would not index it. Both paths must use the same visibility + // rules. (#766) describe('Incremental sync honors the ignore matcher (#766)', () => { let testDir: string; let cg: CodeGraph; diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 22108d1d1..6ac4f1be5 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -1079,119 +1079,6 @@ function getGitVisibleFiles(rootDir: string): Set | null { } } -/** - * Result of git-based change detection. - * Returns null when git is unavailable (non-git project or command failure), - * signaling the caller to fall back to full filesystem scan. - */ -interface GitChanges { - modified: string[]; // M, MM, AM — files to re-hash + re-index - added: string[]; // ?? — new untracked files to index - deleted: string[]; // D — files to remove from DB -} - -/** - * Use `git status` to detect changed files instead of scanning every file. - * Returns null on failure so callers fall back to full scan. - * - * Recurses into embedded repos — the untracked kind (#193: the parent's status - * collapses them to an opaque `?? subdir/` entry) always, and the gitignored - * kind (#514: they never appear in the parent's status at all) only for - * directories opted in via `codegraph.json` `includeIgnored` (#622, #699) — - * running `git status` inside each, so changes in a multi-repo workspace sync - * without a full rescan. By default a gitignored dir is left alone, matching the - * full-index scan (#970, #976). Deleting an ENTIRE embedded repo dir is the one - * case this cannot see (the child status that would report the deletions is gone - * with it); a full `codegraph index` reconciles that. - */ -function getGitChangedFiles(rootDir: string): GitChanges | null { - try { - 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)); - return changes; - } catch { - return null; - } -} - -function collectGitStatus(repoDir: string, prefix: string, out: GitChanges, overrides?: Record, includeIgnored: Ignore | null = null, exclude: Ignore | null = null): void { - const output = execFileSync( - 'git', - ['status', '--porcelain', '--no-renames'], - { cwd: repoDir, encoding: 'utf-8', timeout: 10000, maxBuffer: 50 * 1024 * 1024, stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true } - ); - - // This repo's own ignore rules — built-in defaults (#407) plus its .gitignore. - // Change detection must exclude the SAME files the full index does, but git - // status hides neither: it ignores nothing for *tracked* paths, and the - // built-in defaults aren't gitignore at all. Without this filter a committed - // vendor/ dir, or a tracked file under a .gitignored dir, surfaces here as a - // change — so `codegraph status` (which reads getChangedFiles) reports a - // pending edit the full index never tracks and `sync` never clears. Matching - // repo-relative `rel` at each recursion level mirrors getGitVisibleFiles' - // ScopeIgnore: every embedded repo is judged by ITS OWN rules, never the - // 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; - } - - const filePath = normalizePath(prefix + rel); - if (!isSourceFile(filePath, overrides)) continue; - - 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; - } - - // 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; - // 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 (statusCode === '??') { - out.added.push(filePath); - } else { - // M, MM, AM, A (staged), etc. — treat as modified - out.modified.push(filePath); - } - } - - // Recurse embedded repos found under untracked dirs (at the dir itself or - // nested deeper). Gitignored dirs are walked only for the directories the - // project opted in via `includeIgnored`; by default `.gitignore` is respected - // and they are left alone (#970, #976), mirroring the full-index scan. - for (const rel of untrackedDirs) { - for (const repoRel of findNestedGitRepos(path.join(repoDir, rel), rel)) { - collectGitStatus(path.join(repoDir, repoRel), prefix + repoRel, out, overrides, includeIgnored, exclude); - } - } - for (const rel of findIgnoredEmbeddedRepos(repoDir, includeIgnored, prefix)) { - collectGitStatus(path.join(repoDir, rel), prefix + rel, out, overrides, includeIgnored, exclude); - } -} - /** * Recursively scan a directory for source files. * @@ -2797,53 +2684,10 @@ export class ExtractionOrchestrator { /** * Get files that have changed since last index. - * Uses git status as a fast path when available, falling back to full scan. + * Reconciles the filesystem against the index using the same stat/hash + * strategy as sync, so clean commits and branch changes are visible too. */ getChangedFiles(): { added: string[]; modified: string[]; removed: string[] } { - const gitChanges = getGitChangedFiles(this.rootDir); - - if (gitChanges) { - // === Git fast path === - const added: string[] = []; - const modified: string[] = []; - const removed: string[] = []; - - // Deleted files — only report if tracked in DB - for (const filePath of gitChanges.deleted) { - 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); - let content: string; - 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); - } - } - - return { added, modified, removed }; - } - - // === Fallback: full scan (non-git project or git failure) === const currentFiles = new Set(scanDirectory(this.rootDir)); const trackedFiles = this.queries.getAllFiles(); @@ -2859,7 +2703,9 @@ export class ExtractionOrchestrator { // Find removed files for (const tracked of trackedFiles) { - if (!currentFiles.has(tracked.path)) { + // `git ls-files` includes tracked paths deleted from disk until the + // deletion is staged, so set membership alone is not sufficient. + if (!currentFiles.has(tracked.path) || !fs.existsSync(path.join(this.rootDir, tracked.path))) { removed.push(tracked.path); } } @@ -2867,6 +2713,22 @@ export class ExtractionOrchestrator { // Find added and modified files for (const filePath of currentFiles) { const fullPath = path.join(this.rootDir, filePath); + const tracked = trackedMap.get(filePath); + + // Match sync's cheap pre-filter. Git updates mtimes for paths written by + // checkout, merge and pull, while new paths have no indexed record. + if (tracked) { + try { + const stat = fs.statSync(fullPath); + if (stat.size === tracked.size && Math.floor(stat.mtimeMs) === Math.floor(tracked.modifiedAt)) { + continue; + } + } catch (error) { + logDebug('Skipping unstattable file while detecting changes', { filePath, error: String(error) }); + continue; + } + } + let content: string; try { content = fs.readFileSync(fullPath, 'utf-8'); @@ -2876,7 +2738,6 @@ export class ExtractionOrchestrator { } const contentHash = hashContent(content); - const tracked = trackedMap.get(filePath); if (!tracked) { added.push(filePath);