Skip to content

Commit 7d58420

Browse files
jaredwrayclaude
andauthored
fix(file-entry-cache): restore v8 reconcile and change-detection semantics (#1648) (#1649)
* fix(file-entry-cache): restore v8 reconcile and change-detection semantics (#1648) file-entry-cache v11 diverged from v8 in three ways that broke ESLint's use case: 1. reconcile() updated cache entries for *every* tracked file, not just those inspected via getFileDescriptor(). This revalidated files that were never looked at this run, so a file that changed on disk but was not part of the run kept its stale (valid) cache entry. reconcile() now only updates files that were visited during the session and drops ones that were deleted. 2. getFileDescriptor() returned `changed: false` on repeated calls for the same file because it overwrote the persisted meta on every read. Change detection now compares against a baseline snapshot taken as of the last load/reconcile, so a file keeps reporting `changed: true` until the cache is reconciled. 3. create() threw when the cache file contained invalid JSON. It now starts with an empty cache and overwrites the file on the next reconcile(), matching v8. Adds regression tests covering all three scenarios and updates existing tests that relied on the previous (incorrect) same-session dedup behavior to reconcile before re-checking. * test(file-entry-cache): cover reconcile() guard for externally-removed entries Covers the defensive branch in reconcile() that skips a session-tracked key when it no longer exists in the underlying cache, restoring 100% patch coverage. * fix(file-entry-cache): harden create() and reconcile() per code review - create(): only treat parse errors (SyntaxError) as an invalid cache and fall back to an empty cache; re-throw unexpected IO/permission errors so a valid cache is not silently discarded and overwritten. - reconcile(): persist the consistent size/mtime/hash snapshot captured when the file was inspected instead of re-stat'ing only size/mtime (which left the hash stale and could mask a change made between getFileDescriptor() and reconcile() in useModifiedTime mode). statSync is now used only to detect deletion. Adds regression tests for both behaviors. * fixing some small issues * minor bug fixes --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c5eb62c commit 7d58420

4 files changed

Lines changed: 419 additions & 18 deletions

File tree

packages/file-entry-cache/README.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,12 @@ let fileDescriptor = cache.getFileDescriptor('./src/file.txt');
6161
console.log(fileDescriptor.changed); // true as it is the first time
6262
console.log(fileDescriptor.key); // './src/file.txt' (stored as provided)
6363

64+
// Repeated calls keep reporting `changed: true` until you persist the state
65+
// with reconcile(); only then does the file become the cached baseline.
66+
cache.reconcile();
67+
6468
fileDescriptor = cache.getFileDescriptor('./src/file.txt');
65-
console.log(fileDescriptor.changed); // false as it has not changed
69+
console.log(fileDescriptor.changed); // false as it has not changed since reconcile()
6670

6771
// do something to change the file
6872
fs.writeFileSync('./src/file.txt', 'new data foo bar');

packages/file-entry-cache/src/index.ts

Lines changed: 106 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,23 @@ export function create(
128128
if (cacheDirectory) {
129129
const cachePath = `${cacheDirectory}/${cacheId}`;
130130
if (fs.existsSync(cachePath)) {
131-
fileEntryCache.cache = createFlatCacheFile(cachePath, opts.cache);
131+
try {
132+
fileEntryCache.cache = createFlatCacheFile(cachePath, opts.cache);
133+
} catch (error) {
134+
// If the cache file content cannot be parsed (e.g. corrupted,
135+
// non-JSON, or a legacy/foreign format the parser rejects), start
136+
// with an empty cache. The existing file is overwritten on the next
137+
// reconcile() rather than throwing. Parse failures surface as a
138+
// SyntaxError (malformed JSON) or a TypeError (valid JSON whose shape
139+
// the flatted parser rejects, e.g. a top-level object instead of the
140+
// expected array). Genuine IO/permission failures (e.g. EISDIR/EACCES)
141+
// are re-thrown so valid cache data is not silently discarded.
142+
if (error instanceof SyntaxError || error instanceof TypeError) {
143+
fileEntryCache.cache = new FlatCache(opts.cache);
144+
} else {
145+
throw error;
146+
}
147+
}
132148
}
133149
}
134150

@@ -150,6 +166,15 @@ export class FileEntryCache {
150166
private _logger?: ILogger;
151167
private _useAbsolutePathAsKey = false;
152168
private _useModifiedTime = true;
169+
/**
170+
* Snapshot of the persisted meta for each key as of the last load/reconcile.
171+
* Change detection compares against this baseline (not the working cache) so
172+
* that repeated `getFileDescriptor()` calls keep reporting a file as changed
173+
* until the cache is reconciled. The set of keys also tracks which files were
174+
* visited during the current session so that `reconcile()` only updates those.
175+
*/
176+
private _originalMeta: Map<string, FileDescriptorMeta | undefined> =
177+
new Map();
153178

154179
/**
155180
* Create a new FileEntryCache instance
@@ -203,6 +228,9 @@ export class FileEntryCache {
203228
*/
204229
public set cache(cache: FlatCache) {
205230
this._cache = cache;
231+
// The baseline is derived from the cache, so reset it when the cache is
232+
// replaced. It will be re-snapshotted lazily on the next getFileDescriptor.
233+
this._originalMeta = new Map();
206234
}
207235

208236
/**
@@ -263,6 +291,13 @@ export class FileEntryCache {
263291

264292
/**
265293
* Set the current working directory
294+
*
295+
* Note: when relative paths are used as cache keys (the default), `cwd` must
296+
* stay stable across a `getFileDescriptor()` / `reconcile()` cycle. Relative
297+
* keys are resolved against the *current* `cwd` each time, so changing it
298+
* mid-run can cause `reconcile()` to resolve a key to a different (missing)
299+
* path and drop the entry. Use absolute keys (`useAbsolutePathAsKey: true`)
300+
* if `cwd` must change during a run.
266301
* @param {string} value - The value to set
267302
*/
268303
public set cwd(value: string) {
@@ -368,6 +403,7 @@ export class FileEntryCache {
368403
*/
369404
public destroy() {
370405
this._cache.destroy();
406+
this._originalMeta = new Map();
371407
}
372408

373409
/**
@@ -378,18 +414,55 @@ export class FileEntryCache {
378414
public removeEntry(filePath: string): void {
379415
const key = this.createFileKey(filePath);
380416
this._cache.removeKey(key);
417+
this._originalMeta.delete(key);
381418
}
382419

383420
/**
384421
* Reconcile the cache
385422
* @method reconcile
386423
*/
387424
public reconcile(): void {
388-
const { items } = this._cache;
389-
for (const item of items) {
390-
const fileDescriptor = this.getFileDescriptor(item.key);
391-
if (fileDescriptor.notFound) {
392-
this._cache.removeKey(item.key);
425+
// Prune entries for files that have been deleted from disk. This mirrors
426+
// v8's removeNotFoundFiles() and sweeps ALL cache keys (not only the ones
427+
// visited this session) so stale entries for deleted files do not
428+
// accumulate over time. It only REMOVES missing files; it never refreshes
429+
// the meta of an existing entry, so it does not reintroduce the
430+
// "reconcile() revalidates every file" bug (#1648).
431+
for (const key of [...this._cache.keys()]) {
432+
try {
433+
fs.statSync(this.getAbsolutePath(key));
434+
} catch (error) {
435+
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
436+
// The file no longer exists; drop it from the cache.
437+
this._cache.removeKey(key);
438+
this._originalMeta.delete(key);
439+
} else {
440+
// Any other failure (e.g. EACCES/EIO, or the path-traversal guard
441+
// firing when restrictAccessToCwd is toggled) means we could not
442+
// confirm the file's state. Keep the entry rather than discarding
443+
// valid cached data, and surface the error instead of silently
444+
// dropping it.
445+
this._logger?.error(
446+
{ key, error },
447+
"reconcile: unable to stat file; keeping cached entry",
448+
);
449+
}
450+
}
451+
}
452+
453+
// Promote the inspected meta to the baseline for each file visited via
454+
// getFileDescriptor() this session, so that subsequent getFileDescriptor()
455+
// calls compare against the freshly reconciled state. Only visited entries
456+
// are touched. The inspected meta already holds a consistent size/mtime/hash
457+
// snapshot, so it is promoted as-is rather than re-stat'ing (which would
458+
// refresh size/mtime but not hash, leaving the baseline inconsistent).
459+
for (const key of [...this._originalMeta.keys()]) {
460+
const meta = this._cache.getKey<FileDescriptorMeta>(key);
461+
if (meta) {
462+
this._originalMeta.set(key, { ...meta });
463+
} else {
464+
// The entry was removed during the session or by the prune above.
465+
this._originalMeta.delete(key);
393466
}
394467
}
395468

@@ -499,35 +572,49 @@ export class FileEntryCache {
499572
};
500573
}
501574

502-
// If the file is not in the cache, add it
503-
if (!metaCache) {
575+
// Snapshot the baseline (the persisted state as of the last load/reconcile)
576+
// the first time this key is seen in the current session. Change detection
577+
// compares against this baseline rather than the working cache so that a
578+
// file reported as changed keeps reporting as changed until reconcile().
579+
if (!this._originalMeta.has(result.key)) {
580+
this._originalMeta.set(
581+
result.key,
582+
metaCache ? { ...metaCache } : undefined,
583+
);
584+
}
585+
586+
const baseline = this._originalMeta.get(result.key);
587+
588+
// If there is no baseline, the file is new (or has not been reconciled yet)
589+
// and is therefore considered changed.
590+
if (baseline === undefined) {
504591
result.changed = true;
505592
this._cache.setKey(result.key, result.meta);
506593
this._logger?.debug({ filePath }, "File not in cache, marked as changed");
507594
return result;
508595
}
509596

510597
// If the file is in the cache, check if the file has changed
511-
if (useModifiedTimeValue && metaCache?.mtime !== result.meta?.mtime) {
598+
if (useModifiedTimeValue && baseline.mtime !== result.meta?.mtime) {
512599
result.changed = true;
513600
this._logger?.debug(
514-
{ filePath, oldMtime: metaCache.mtime, newMtime: result.meta.mtime },
601+
{ filePath, oldMtime: baseline.mtime, newMtime: result.meta.mtime },
515602
"File changed: mtime differs",
516603
);
517604
}
518605

519-
if (metaCache?.size !== result.meta?.size) {
606+
if (baseline.size !== result.meta?.size) {
520607
result.changed = true;
521608
this._logger?.debug(
522-
{ filePath, oldSize: metaCache.size, newSize: result.meta.size },
609+
{ filePath, oldSize: baseline.size, newSize: result.meta.size },
523610
"File changed: size differs",
524611
);
525612
}
526613

527-
if (useCheckSumValue && metaCache?.hash !== result.meta?.hash) {
614+
if (useCheckSumValue && baseline.hash !== result.meta?.hash) {
528615
result.changed = true;
529616
this._logger?.debug(
530-
{ filePath, oldHash: metaCache.hash, newHash: result.meta.hash },
617+
{ filePath, oldHash: baseline.hash, newHash: result.meta.hash },
531618
"File changed: hash differs",
532619
);
533620
}
@@ -733,6 +820,11 @@ export class FileEntryCache {
733820
const meta = this._cache.getKey(key);
734821
this._cache.removeKey(key);
735822
this._cache.setKey(newKey, meta);
823+
// Keep the change-detection baseline aligned with the renamed key.
824+
if (this._originalMeta.has(key)) {
825+
this._originalMeta.set(newKey, this._originalMeta.get(key));
826+
this._originalMeta.delete(key);
827+
}
736828
}
737829
}
738830
}

0 commit comments

Comments
 (0)