From 983c4df3745939be9ec7960d6e5d04d8bd6653a8 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Tue, 27 Oct 2026 18:03:50 +0000 Subject: [PATCH] fix(index): a deleted file git still lists is skipped, not reported as unreadable (#2928) A full index reads every path the scan lists, and `git ls-files` keeps listing a tracked file deleted from the working tree until the deletion is staged. So `codegraph init` and `codegraph index` stat'ed the missing file, got ENOENT and reported "1 files could not be read" with an errors.log entry. The errored file also kept the run from recording the commit it indexed, so `codegraph status` fell back to a full scan afterwards. The batch reader in indexAll and the single-file indexFile now take ENOENT and ENOTDIR to mean nothing is there: the file is skipped and counted as skipped, so the run reads as complete rather than PARTIAL. A symlink that points at nothing is skipped the same way, as the directory walk for projects without git already did. Every other read failure (EACCES, EIO, a folder where a file was) is still reported. On POSIX a path under a file fails realpath with ENOTDIR, which validatePathWithinRoot rejected as if it escaped the root; it is now judged like any missing leaf, by its nearest existing ancestor, so it reaches the reader and is skipped too. Discovery is unchanged, with no extra stat per tracked file. Validation: - index-gone-tracked-files.test.ts (new): all 5 cases fail on main's source and pass with the fix: committed and staged deletions, a folder replaced by a file, a dangling symlink, and EACCES still reported by both the full index and indexFiles. Mutating the single-file path, the skipped tally or the EACCES boundary fails the test each time. - security.test.ts: a path under an existing file validates, and one under a symlink to a file outside the root is still refused. - The issue's CLI repro: main prints "1 files could not be read" and writes errors.log; the fix prints "Indexed 1 files", writes no errors.log, and status reports nothing pending. Reported in PR #2910 by @xianzhi-xzw, who also filed issue #2904. This reimplements the fix independently, at read time rather than with an lstat of every tracked file during discovery. Fixes #2904 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/index-gone-tracked-files.test.ts | 132 +++++++++++++++++++++ __tests__/security.test.ts | 13 ++ src/extraction/index.ts | 25 +++- src/utils.ts | 15 ++- 5 files changed, 182 insertions(+), 4 deletions(-) create mode 100644 __tests__/index-gone-tracked-files.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 941ec1600..6076e3fb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph upgrade` now upgrades a global npm install when run from your home folder, which holds npm's global folder on Windows and with tools like nvm: before, it ran `npm install` without `-g` there, which left the global copy on the old version and put a second copy in your home folder. A copy installed in a project is now upgraded in that project wherever you run the command, with a warning if npm left it on the old version, and a copy installed with pnpm, yarn or bun gets the command that upgrades it instead of npm. Thanks @timlincool for the report and @maxmilian for the first fix. (#2477) - `codegraph status` no longer says the index is up to date while indexed files are missing their symbols: it now names files the parser couldn't read and files stored without their symbols (which `codegraph sync` repairs), `status --json` counts both, and `codegraph files --json` lists each file's recorded errors. Thanks @lipchey for the report. (#2336) - Running `codegraph status` while `codegraph index` rebuilds a project no longer makes the rebuild fail with "database is locked": status now only reads the index, and it says when a rebuild is running in another process, in its output and as `rebuildInProgress` in `status --json`. Thanks @sealad886 for the report, and @danusha2345 and @KissFeng for the fixes. (#2913) +- `codegraph init` and `codegraph index` no longer report a file you deleted as one that "could not be read" while git still lists it, which it does until the deletion is staged: such a file is now skipped, and so is a symlink that points at nothing. A file that is there but can't be read is still reported. Thanks @xianzhi-xzw for the report and the first fix. (#2904) - In CFML, a `` or `` block the parser can't read no longer marks its whole file as contributing nothing to the graph: `codegraph init` no longer warns about the file, and `codegraph status` no longer lists it among files whose symbols are missing, since its component, functions and calls are all indexed. The file's recorded errors now name the block and its line instead. Thanks @HarryMuc for the report. (#2654) - In Erlang, a comment inside an argument or parameter list, like the one in `add_to_includes( % give access to .hrl in app's src/` before its two arguments, no longer counts as an argument. The call now links to the function of its real arity, and a function whose parameter list holds a comment keeps its own arity. Before, the comment raised the arity by one, so the call linked to nothing. Re-index Erlang projects after upgrading. Thanks @Dshuishui. (#1619) - Indexing large Python projects is much faster again and needs less memory: since 1.6.2, resolving Python references re-read source files over and over, so a project the size of CPython took several times as long to index. The graph it builds is unchanged. Thanks @bompus for the report. (#2332) diff --git a/__tests__/index-gone-tracked-files.test.ts b/__tests__/index-gone-tracked-files.test.ts new file mode 100644 index 000000000..1c302a732 --- /dev/null +++ b/__tests__/index-gone-tracked-files.test.ts @@ -0,0 +1,132 @@ +/** + * A full index skips a file the scan lists but the disk no longer has (#2904). + * + * `git ls-files` keeps listing a tracked file deleted from the working tree + * until the deletion is staged, so the scan hands the reader a path with + * nothing behind it. That is not a file that "could not be read": the run + * skips it, still accounts for it (an unaccounted file reads as a PARTIAL + * index), and reports only listed files that are there but cannot be read. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as cp from 'child_process'; +import * as fs from 'fs'; +import * as fsp from 'fs/promises'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import type { IndexResult } from '../src/extraction'; + +// A pass-through `stat`, so one test can make a present file unreadable on +// every platform (chmod cannot do that on Windows). +vi.mock('fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, stat: vi.fn(actual.stat) }; +}); + +describe('full index of files git lists but the disk lacks (#2904)', () => { + let root: string; + let cg: CodeGraph | undefined; + const git = (...args: string[]) => + cp.execFileSync('git', args, { cwd: root, encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'] }); + const write = (rel: string, name = path.basename(rel, '.py')) => { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), `def ${name}():\n return 1\n`); + }; + const commit = () => { git('add', '-A'); git('commit', '-qm', 'fixture'); }; + const readErrors = (r: IndexResult) => r.errors.filter((e) => e.code === 'read_error').map((e) => e.filePath); + const metadata = (key: string): string | null => (cg as any).queries.getMetadata(key); + const indexedPaths = (): string[] => (cg as any).queries.getAllFilePaths().sort(); + + beforeEach(() => { + root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'cg-gone-tracked-'))); + git('init', '-q'); + git('config', 'user.email', 'test@example.invalid'); + git('config', 'user.name', 'Test'); + git('config', 'commit.gpgsign', 'false'); + fs.writeFileSync(path.join(root, '.gitignore'), '.codegraph/\n'); + }); + + afterEach(() => { + cg?.close(); + cg = undefined; + fs.rmSync(root, { recursive: true, force: true }); + }); + + /** A clean run over exactly `indexed`: nothing errored, unread or unaccounted. */ + const expectCleanRun = (result: IndexResult, indexed: string[]) => { + expect(readErrors(result)).toEqual([]); + expect(result.filesErrored).toBe(0); + expect(result.filesIndexed).toBe(indexed.length); + expect(result.errors.filter((e) => e.code === 'index_partial')).toEqual([]); + expect(metadata('index_state')).toBe('complete'); + expect(indexedPaths()).toEqual(indexed); + }; + + it.each(['committed', 'staged'] as const)('skips a %s file deleted with the deletion unstaged', async (state) => { + write('keep.py'); + write('gone.py'); + if (state === 'committed') commit(); else git('add', '-A'); + fs.unlinkSync(path.join(root, 'gone.py')); + expect(git('ls-files')).toContain('gone.py'); // the premise: git still lists it + + cg = CodeGraph.initSync(root); + expectCleanRun(await cg.indexAll(), ['keep.py']); + // A clean run records the commit it indexed, so `status` keeps its git + // fast path instead of falling back to a full scan. + if (state === 'committed') expect(metadata('indexed_at_commit')).toBe(git('rev-parse', 'HEAD').trim()); + expect(cg.getChangedFiles()).toEqual({ added: [], modified: [], removed: [] }); + }); + + it('skips a tracked file whose folder is now a file', async () => { + write('keep.py'); + write('previous/gone.py'); + commit(); + fs.rmSync(path.join(root, 'previous'), { recursive: true }); + fs.writeFileSync(path.join(root, 'previous'), 'a file where the folder was\n'); + expect(git('ls-files')).toContain('previous/gone.py'); + + cg = CodeGraph.initSync(root); + expectCleanRun(await cg.indexAll(), ['keep.py']); + }); + + it('skips a symlink that points at nothing, as the directory walk does', async (ctx) => { + write('keep.py'); + // File symlinks need Developer Mode or elevation on Windows. + try { fs.symlinkSync('missing-target.py', path.join(root, 'broken.py')); } catch { ctx.skip(); } + commit(); + + cg = CodeGraph.initSync(root); + expectCleanRun(await cg.indexAll(), ['keep.py']); + }); + + it('still reports a listed file that is there but cannot be read', async () => { + write('keep.py'); + write('locked.py'); + write('gone.py'); + commit(); + fs.unlinkSync(path.join(root, 'gone.py')); + const stat = vi.mocked(fsp.stat); + const realStat = stat.getMockImplementation()!; + stat.mockImplementation(((p: fs.PathLike, ...rest: unknown[]) => { + if (String(p).endsWith('locked.py')) { + return Promise.reject(Object.assign(new Error(`EACCES: permission denied, stat '${String(p)}'`), { code: 'EACCES' })); + } + return (realStat as (...a: unknown[]) => Promise)(p, ...rest); + }) as typeof fsp.stat); + try { + cg = CodeGraph.initSync(root); + const result = await cg.indexAll(); + expect(readErrors(result)).toEqual(['locked.py']); + expect(result.filesErrored).toBe(1); + expect(result.filesIndexed).toBe(1); + + // The single-file path draws the same line. + const files = await cg.indexFiles(['gone.py', 'locked.py']); + expect(readErrors(files)).toEqual(['locked.py']); + expect(files.filesErrored).toBe(1); + expect(files.filesSkipped).toBe(1); + } finally { + stat.mockImplementation(realStat); + } + }); +}); diff --git a/__tests__/security.test.ts b/__tests__/security.test.ts index 6c128b4c3..1048c7613 100644 --- a/__tests__/security.test.ts +++ b/__tests__/security.test.ts @@ -312,6 +312,19 @@ describe('Symlink escape prevention (#527)', () => { } }); + // POSIX reports ENOTDIR, not ENOENT, for a path under a FILE (a folder git + // still lists that is now a file, #2904). It is a missing leaf like any other: + // allowed inside the root, still refused below a link out of it. + it('judges a path under an existing file like any missing leaf', () => { + expect(validatePathWithinRoot(root, 'src/in.ts/child.ts')).toBe(path.join(root, 'src', 'in.ts', 'child.ts')); + expect(validatePathWithinRoot(root, 'src/in.ts/child.ts', { allowSymlinkEscape: true })).not.toBeNull(); + }); + + it('rejects a path under an in-repo symlink to an out-of-root FILE', () => { + if (!link(path.join(root, 'escape'), path.join(outside, 'pkg', 'secret.txt'))) return; + expect(validatePathWithinRoot(root, 'escape/child.txt')).toBeNull(); + }); + it('rejects every path under a root that does not exist', () => { expect(validatePathWithinRoot(path.join(root, 'gone'), 'src/in.ts')).toBeNull(); }); diff --git a/src/extraction/index.ts b/src/extraction/index.ts index f437cfd09..c93d26b00 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -28,7 +28,7 @@ import { detectLanguage, isSourceFile, isLanguageSupported, isFileLevelOnlyLangu 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'; +import { validatePathWithinRoot, normalizePath, isMissingPathError } from '../utils'; import ignore, { Ignore } from 'ignore'; import { detectFrameworks, getFrameworkResolver } from '../resolution/frameworks'; import { declaredDependencies } from '../resolution/frameworks/package-deps'; @@ -2579,6 +2579,14 @@ export class ExtractionOrchestrator { const content = bytes.toString('utf-8'); return { filePath: fp, content, stats, error: null as Error | null }; } catch (err) { + // Nothing at the path is not a file that could not be read (#2904): + // git keeps listing a tracked file deleted from the working tree + // until the deletion is staged, a symlink can point at nothing (the + // directory walk skips one quietly), and a file can go between the + // scan and this read. Skip it. EACCES, EIO and the like still report. + if (isMissingPathError(err)) { + return { filePath: fp, content: null as string | null, stats: null as fs.Stats | null, error: null as Error | null, gone: true }; + } return { filePath: fp, content: null as string | null, stats: null as fs.Stats | null, error: err as Error }; } }) @@ -2586,7 +2594,7 @@ export class ExtractionOrchestrator { // Dispatch each readable file into the bounded parse window; the window // stores results on the main thread as they arrive. - for (const { filePath, content, stats, error, skipped } of fileContents) { + for (const { filePath, content, stats, error, skipped, gone } of fileContents) { if (signal?.aborted) { aborted = true; break; } if (skipped) { @@ -2596,6 +2604,15 @@ export class ExtractionOrchestrator { continue; } + if (gone) { + // Listed by the scan but not on disk: skipped, stored as nothing. It + // still counts toward the tallies, or the run would read as PARTIAL. + processed++; + filesSkipped++; + onProgress?.({ phase: 'parsing', current: processed, total }); + continue; + } + if (error || content === null || stats === null) { processed++; filesErrored++; @@ -2921,6 +2938,10 @@ export class ExtractionOrchestrator { } content = read.bytes === null ? oversizeStamp(stats.size) : read.bytes.toString('utf8'); } catch (error) { + // Nothing at the path: nothing to index, and not a read failure (#2904). + if (isMissingPathError(error)) { + return { nodes: [], edges: [], unresolvedReferences: [], errors: [], durationMs: 0 }; + } return { nodes: [], edges: [], diff --git a/src/utils.ts b/src/utils.ts index d817003c8..0a7bca020 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -124,6 +124,16 @@ export function lexicalPathWithinRoot(projectRoot: string, filePath: string): st return isWithinDir(resolved, path.resolve(projectRoot)) ? resolved : null; } +/** + * Whether a filesystem error means nothing is at the path: ENOENT, or ENOTDIR, + * which POSIX reports instead when a folder on the way is a file (Windows + * reports ENOENT for both). + */ +export function isMissingPathError(err: unknown): boolean { + const code = (err as NodeJS.ErrnoException | null)?.code; + return code === 'ENOENT' || code === 'ENOTDIR'; +} + /** * Validate that a file path stays within the project root, resolving symlinks. * @@ -188,7 +198,8 @@ export function validatePathWithinRoot( // nearest existing ancestor before allowing a write path: otherwise // `project/.codegraph/ui -> /outside` plus a missing `trails/` directory // passes the lexical check and mkdir follows the symlink out of the root. - if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return null; + // A path under a file (ENOTDIR on POSIX) is a missing leaf too. + if (!isMissingPathError(err)) return null; } let ancestor = path.dirname(resolved); @@ -200,7 +211,7 @@ export function validatePathWithinRoot( } return null; } catch (err) { - if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return null; + if (!isMissingPathError(err)) return null; const parent = path.dirname(ancestor); if (parent === ancestor) return null; ancestor = parent;