Skip to content

fix(status): detect committed source changes with a clean working tree #1540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
WhoJay0609 wants to merge 1 commit into
base: main
Choose a base branch
from
+87 −165
Open

fix(status): detect committed source changes with a clean working tree #1540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
23 changes: 23 additions & 0 deletions __tests__/status-json.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)', () => {
Expand Down
46 changes: 41 additions & 5 deletions __tests__/sync.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
181 changes: 21 additions & 160 deletions src/extraction/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1079,119 +1079,6 @@ function getGitVisibleFiles(rootDir: string): Set<string> | 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<string, Language>, 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.
*
Expand Down Expand Up @@ -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();

Expand All @@ -2859,14 +2703,32 @@ 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);
}
}

// 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');
Expand All @@ -2876,7 +2738,6 @@ export class ExtractionOrchestrator {
}

const contentHash = hashContent(content);
const tracked = trackedMap.get(filePath);

if (!tracked) {
added.push(filePath);
Expand Down