diff --git a/gitnexus/scripts/cross-platform-tests.ts b/gitnexus/scripts/cross-platform-tests.ts index 4cc74fd87..07b10fe25 100644 --- a/gitnexus/scripts/cross-platform-tests.ts +++ b/gitnexus/scripts/cross-platform-tests.ts @@ -290,6 +290,11 @@ const NATIVE_ADDON_SMOKE = [ // Filesystem behavior tests — exercise operations that vary across // platforms (CRLF, symlinks, permissions, temp dirs) const FILESYSTEM = [ + // The durable ParsedFile store's prune tolerates a chunk directory it cannot + // delete (#3204). The failures that motivate it — held handles, read-only + // mounts — are Windows- and macOS-flavored, and the permission-based case + // skips itself where chmod cannot block a delete, so run it everywhere. + 'test/unit/parsedfile-store.test.ts', 'test/integration/filesystem-walker.test.ts', 'test/integration/watch-filesystem.test.ts', 'test/integration/markdown-processor-crlf.test.ts', diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index 443c13289..01d8a0862 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -26,6 +26,7 @@ import { computeChunkHash, loadParseCacheChunk, persistParseCacheChunk, + markParseCacheChunkStale, PARSE_CACHE_VERSION, packParseCacheChunks, } from '../../../storage/parse-cache.js'; @@ -37,6 +38,7 @@ import { loadDurableParsedFileIndex, prepareDurableParsedFileChunk, durableChunkHasShards, + durableChunkHasStaleShards, } from '../../../storage/parsedfile-store.js'; import type { ParseWorkerResult } from '../workers/parse-worker.js'; import { DEFAULT_PDG_MAX_FUNCTION_LINES } from '../cfg/collect.js'; @@ -932,8 +934,12 @@ export async function runChunkedParseAndResolve( * Chunk hashes whose durable ParsedFile directory could not be reset. The * old generation's shards are still on disk, so a warm hit would union * stale shards with the new ones. Treated exactly like a quarantined chunk: - * skip the parse-cache write so the next run re-dispatches into a clean - * directory rather than trusting a generation we could not clear. + * skip the parse-cache write AND, when shards from that generation are + * still on disk, retire the hash (#3204): the old `.v8` is not carried + * forward and the directory is dropped from the durable index, so the next + * run re-dispatches. Kept as its own set rather than + * read back off `staleKeys`, which is a superset — it is what selects the + * warn below over the quarantine branch's dev-only log. */ const durablePrepareFailures = new Set(); @@ -1073,7 +1079,11 @@ export async function runChunkedParseAndResolve( // Persist raw results for this chunk hash (skipping when any chunk file // was worker-quarantined, so the narrower rawResults isn't cached under // the full-chunk key — see the original inline note / U20.U2). - if (parseCache && p.chunkHash && rawResults.length > 0) { + // `rawResults.length > 0` guards the WRITE only. A quarantined chunk + // often returns nothing at all (the worker died on it), and that chunk + // still has to be retired — otherwise its pre-existing `.v8` is copied + // forward at save time (#3204). + if (parseCache && p.chunkHash) { const quarantineSet = new Set(workerPool?.getQuarantinedPaths?.() ?? []); const chunkHadQuarantine = p.chunkFiles.some((f) => quarantineSet.has(f.path)); const durableGenerationStale = durablePrepareFailures.has(p.chunkHash); @@ -1084,6 +1094,10 @@ export async function runChunkedParseAndResolve( 'so its shards may be stale; next run will re-dispatch it', ); } else if (chunkHadQuarantine) { + // This chunk's durable directory now holds only this run's NARROWER + // shards, so a warm hit would replay the full-coverage `.v8` over + // partial ParsedFiles (#3204). + markParseCacheChunkStale(parseCache, p.chunkHash); if (isDev) { const quarantinedInChunk = p.chunkFiles.filter((f) => quarantineSet.has(f.path)).length; logger.info( @@ -1092,7 +1106,7 @@ export async function runChunkedParseAndResolve( `next run will rediscover (${p.chunkHash.slice(0, 8)})`, ); } - } else { + } else if (rawResults.length > 0) { await persistParseCacheChunk(parseCache, p.chunkHash, rawResults); if (isDev) { logger.info( @@ -1142,6 +1156,22 @@ export async function runChunkedParseAndResolve( // directory on write, so at worst the old generation lingers. // Caught per chunk so one failure cannot abort the others. durablePrepareFailures.add(miss.chunkHash); + // Retire ONLY when a generation nobody cleared is still on disk. + // `prepareDurableParsedFileChunk` is rm-then-mkdir: an rm failure + // leaves the old shards to be unioned into a warm hit, but an rm + // that succeeded before a failing mkdir leaves nothing — the + // workers recreate the directory and write a clean generation, so + // retiring there would discard a good `.v8` for no safety gain + // (and a correlated burst would discard the whole shared cache). + // Retire here rather than at finalize: that branch sits behind + // `rawResults.length > 0`, so a chunk whose worker round returns + // nothing would keep its stale entry. + if ( + parseCache && + (await durableChunkHasStaleShards(durableParsedFileDir, miss.chunkHash)) + ) { + markParseCacheChunkStale(parseCache, miss.chunkHash); + } logger.warn( { err, chunkHash: miss.chunkHash.slice(0, 8) }, 'parsedfile-cache: could not reset durable chunk generation; ' + diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 2f89e3c4b..30b54ece4 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -4312,9 +4312,11 @@ async function runFullAnalysisInner( // Prune the durable ParsedFile store to EXACTLY the parse cache's // surviving keys (#2038 warm-cache coverage), so the two content-addressed // stores stay coherent: a chunk is "cached" iff both its parse-cache shard - // and its durable shards exist. A quarantined chunk (in usedKeys but with - // no parse-cache shard) drops its durable subdir here and re-dispatches - // next run. Same try/catch — a durable-store write failure must never + // and its durable shards exist. A retired chunk — worker-quarantined, or + // one whose failed durable reset left an uncleared generation behind + // (#3204) — is filtered out of `savedKeys`, so it drops out of the + // durable index here and re-dispatches next run. Same try/catch — a + // durable-store write must never // break an otherwise successful run (next run treats it as a miss). await mergeStagedDurableParsedFileStore( storagePath, diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 0ce2b7dab..560c273e5 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -896,6 +896,16 @@ export interface ParseCache { * Transient — never serialized to disk. */ usedKeys: Set; + /** + * Hashes this run decided it cannot vouch for — its durable generation could + * not be reset, or its chunk was worker-quarantined (#3204). `saveParseCache` + * refuses them, so neither a pre-existing `.v8` nor the chunk's durable + * directory survives into the next run. Kept separate from `usedKeys` + * because the orchestrator re-adds keys to that set after the parse phase + * (#2106 sibling fold), which would undo a deletion. + * Transient — never serialized to disk. + */ + staleKeys?: Set; /** * When set, chunk payloads are loaded from / flushed to sharded files on * demand instead of retaining every chunk in `entries` for the whole run @@ -1125,6 +1135,24 @@ export const persistParseCacheChunk = async ( cache.entries.set(chunkHash, slim); }; +/** + * Retire a chunk this run cannot vouch for — its durable ParsedFile generation + * could not be reset, or its chunk was worker-quarantined (#3204). + * + * `saveParseCache` refuses a stale key, so no pre-existing `.v8` is copied + * forward and the durable store — pruned to exactly the keys that save + * returns — drops the chunk in the same step. The two deletes matter because + * `loadParseCacheChunk` reads `entries` and `onDiskKeys` and does NOT consult + * `staleKeys`: without them a second lookup of the same hash inside this run + * would still serve the retired shard. + */ +export const markParseCacheChunkStale = (cache: ParseCache, chunkHash: string): void => { + cache.staleKeys ??= new Set(); + cache.staleKeys.add(chunkHash); + cache.entries.delete(chunkHash); + cache.onDiskKeys?.delete(chunkHash); +}; + const loadLegacyParseCache = async (storagePath: string): Promise => { const cachePath = getLegacyCachePath(storagePath); try { @@ -1210,7 +1238,14 @@ export const saveParseCache = async (storagePath: string, cache: ParseCache): Pr await fs.rm(tmpDir, { recursive: true, force: true }); await fs.mkdir(tmpDir, { recursive: true }); - const keys = [...cache.usedKeys].filter(isValidChunkCacheKey).sort(); + // A stale key is dropped here rather than at the failure site: the + // orchestrator folds sibling-branch keys back into `usedKeys` after the parse + // phase (#2106), so this is the last point that sees the final key set. The + // exclusion also reaches the durable store, which prunes to the keys this + // function returns — both stores drop the chunk together (#3204). + const keys = [...cache.usedKeys] + .filter((key) => isValidChunkCacheKey(key) && !cache.staleKeys?.has(key)) + .sort(); // Track hashes whose shard was actually written/copied this save. A hash can // be in `usedKeys` without a backing shard — its in-memory serialize threw, or // its on-disk copy failed/was-absent (e.g. a worker-quarantined chunk added to diff --git a/gitnexus/src/storage/parsedfile-store.ts b/gitnexus/src/storage/parsedfile-store.ts index 68ed2e06e..58c336c95 100644 --- a/gitnexus/src/storage/parsedfile-store.ts +++ b/gitnexus/src/storage/parsedfile-store.ts @@ -619,6 +619,22 @@ export const prepareDurableParsedFileChunk = async ( await fs.mkdir(dir, { recursive: true }); }; +/** + * Does this chunk still hold shards from a generation nobody cleared? + * + * Asked only after {@link prepareDurableParsedFileChunk} rejected, to tell its + * two failure modes apart (#3204). The `rm` failing leaves the previous + * generation in place, and a warm hit would union it with whatever this run's + * workers write — that chunk must be retired. The `rm` succeeding and the + * `mkdir` then failing leaves NO directory: the workers recreate it and write + * a clean generation, so retiring would throw away a good cache entry for + * nothing. Absent or empty ⇒ nothing to distrust. + */ +export const durableChunkHasStaleShards = async ( + durableDir: string, + chunkHash: string, +): Promise => (await listV8Shards(durableChunkDir(durableDir, chunkHash))).length > 0; + /** * Synchronous durable-shard writer for use INSIDE a parse worker, alongside * {@link persistParsedFileShardSync}. Writes the SAME bytes to a content-addressed @@ -728,8 +744,9 @@ export const loadDurableParsedFileIndex = async ( * Prune the durable store to `keepKeys` and rewrite its index. `keepKeys` must * be the parse cache's surviving on-disk keys (so the two stores stay coherent: * a chunk is "cached" iff BOTH its parse-cache shard and its durable shards - * exist; a quarantined chunk — no parse-cache shard — drops its durable subdir - * here and re-dispatches next run). Only chunks whose envelopes all validate + * exist; a chunk retired by `markParseCacheChunkStale` — worker-quarantined, or + * holding a durable generation that could not be reset — is absent from + * `keepKeys`, so it drops its durable subdir here and re-dispatches next run). Only chunks whose envelopes all validate * are indexed, together with their exact persisted path coverage (never vouch * for a missing/corrupt shard). The index write is tmp+rename atomic. */ @@ -745,6 +762,8 @@ export const pruneAndSaveDurableParsedFileStore = async ( return; // nothing written this run } const survivors: Record = {}; + const undeletable: string[] = []; + let firstRemoveError: unknown; for (const name of entries) { if (name === DURABLE_INDEX_FILENAME) continue; const full = path.join(durableDir, name); @@ -771,7 +790,29 @@ export const pruneAndSaveDurableParsedFileStore = async ( /* not a readable dir → drop below */ } } - await fs.rm(full, { recursive: true, force: true }); + // The causes that break `prepareDurableParsedFileChunk` — permissions, a + // locked file, a read-only mount — break this rm too (#3204). Dropping the + // entry from the index is what makes the chunk unreachable; losing the + // directory is a cleanup bonus. Never let one of them abort the loop and + // cost every remaining chunk its index entry. + try { + await fs.rm(full, { recursive: true, force: true }); + } catch (err) { + // `name` reached the drop branch precisely because it is not a live + // chunk key, so it may be any stray directory — report it as an entry. + undeletable.push(name); + firstRemoveError ??= err; + } + } + if (undeletable.length > 0) { + // One line per RUN, not per directory: a store-wide cause (read-only mount, + // wrong ownership) hits every non-survivor, and thousands of warns would + // bury the message that matters. + logger.warn( + { err: firstRemoveError, count: undeletable.length, firstEntry: undeletable[0] }, + 'parsedfile-cache: could not remove pruned durable chunk directories; ' + + 'they are excluded from the index and will be re-attempted next run', + ); } const idx: DurableParsedFileIndex = { version, entries: survivors }; const tmp = path.join(durableDir, `${DURABLE_INDEX_FILENAME}.tmp`); @@ -810,7 +851,20 @@ export const mergeStagedDurableParsedFileStore = async ( if (name === DURABLE_INDEX_FILENAME) continue; const from = path.join(stagedDir, name); const to = path.join(liveDir, name); - await replaceDurableChunkDir(from, to); + // Same reasoning as the prune's per-entry guard, on the loop that runs + // BEFORE it (#3204): this overlay targets the same live chunk directories, + // so the causes that break a reset break a replacement too. Letting one + // throw here would skip the prune entirely — the durable index would not be + // rewritten this run, and a retired chunk would keep its directory. + try { + await replaceDurableChunkDir(from, to); + } catch (err) { + logger.warn( + { err, entry: name }, + 'parsedfile-cache: could not publish a staged durable chunk; ' + + 'it stays uncached and will re-dispatch next run', + ); + } } await pruneAndSaveDurableParsedFileStore(liveDir, version, keepKeys); }; @@ -824,6 +878,12 @@ const replaceDurableChunkDir = async (from: string, to: string): Promise = /* dest exists, or the rename is cross-device */ } const backup = `${to}.replacing`; + // Deliberately NOT best-effort. If a non-empty backup survives, the + // `fs.rename(to, backup)` below cannot overwrite it and is swallowed as + // "dest was missing", so the `fs.cp` fallback would merge the staged + // generation INTO the live directory — manufacturing exactly the old+new + // union this fix exists to prevent. Let it throw; the caller's per-entry + // guard keeps one such chunk from costing the others their prune. await fs.rm(backup, { recursive: true, force: true }); let backedUp = false; try { @@ -847,6 +907,8 @@ const replaceDurableChunkDir = async (from: string, to: string): Promise = throw err; } if (backedUp) { - await fs.rm(backup, { recursive: true, force: true }); + // The new generation is already in place; an undeletable backup is litter, + // not a failure. The next prune re-attempts it. + await fs.rm(backup, { recursive: true, force: true }).catch(() => {}); } }; diff --git a/gitnexus/test/integration/parse-impl-quarantine-cache-skip.test.ts b/gitnexus/test/integration/parse-impl-quarantine-cache-skip.test.ts index 949f0d75e..e662603ea 100644 --- a/gitnexus/test/integration/parse-impl-quarantine-cache-skip.test.ts +++ b/gitnexus/test/integration/parse-impl-quarantine-cache-skip.test.ts @@ -78,6 +78,7 @@ import { fileContentHash, packParseCacheChunks, } from '../../src/storage/parse-cache.js'; +import type { ParseCache } from '../../src/storage/parse-cache.js'; import type { ParseWorkerResult } from '../../src/core/ingestion/workers/parse-worker.js'; /** @@ -257,7 +258,7 @@ describe('U20: parse-impl quarantine + chunk-cache integration (PR #1693 Codex f const expectedChunkHash = hashPacks(scanned).poison; - const parseCache = { + const parseCache: ParseCache = { version: 'test', entries: new Map(), usedKeys: new Set(), @@ -323,6 +324,9 @@ describe('U20: parse-impl quarantine + chunk-cache integration (PR #1693 Codex f // against a fresh-quarantine pool. expect(parseCache.entries.has(expectedChunkHash)).toBe(false); expect(parseCache.usedKeys.has(expectedChunkHash)).toBe(true); + // #3204: `usedKeys` alone would let `saveParseCache` copy a pre-existing + // shard forward, so the skipped chunk is also retired from the save. + expect(parseCache.staleKeys?.has(expectedChunkHash)).toBe(true); for (const hash of hashPacks(scanned).others) { expect(parseCache.entries.has(hash)).toBe(true); } @@ -338,7 +342,7 @@ describe('U20: parse-impl quarantine + chunk-cache integration (PR #1693 Codex f })); const expectedChunkHash = hashPacks(scanned).poison; - const parseCache = { + const parseCache: ParseCache = { version: 'test', entries: new Map(), usedKeys: new Set(), diff --git a/gitnexus/test/unit/parse-impl-warm-cache-parsedfile-coverage.test.ts b/gitnexus/test/unit/parse-impl-warm-cache-parsedfile-coverage.test.ts index c01baaca2..efd2f9fb9 100644 --- a/gitnexus/test/unit/parse-impl-warm-cache-parsedfile-coverage.test.ts +++ b/gitnexus/test/unit/parse-impl-warm-cache-parsedfile-coverage.test.ts @@ -34,8 +34,10 @@ import { pathToFileURL } from 'node:url'; // Partial mock: lets one test make prepareDurableParsedFileChunk fail without // touching the worker-side persist path (which shares the same directory). +// Receives the chunk hash so a test can fail the reset for ONE chunk while its +// siblings reset normally — the shape a correlated-vs-isolated failure needs. const prepareOverride = vi.hoisted(() => ({ - impl: undefined as undefined | (() => Promise), + impl: undefined as undefined | ((durableDir: string, chunkHash: string) => Promise), })); const persistOverride = vi.hoisted(() => ({ impl: undefined as undefined | (() => Promise), @@ -46,7 +48,7 @@ vi.mock('../../src/storage/parsedfile-store.js', async (importOriginal) => { ...real, prepareDurableParsedFileChunk: (durableDir: string, chunkHash: string) => prepareOverride.impl - ? prepareOverride.impl() + ? prepareOverride.impl(durableDir, chunkHash) : real.prepareDurableParsedFileChunk(durableDir, chunkHash), persistParsedFileChunk: ( storagePath: string, @@ -78,6 +80,7 @@ import { clearParsedFileStore, } from '../../src/storage/parsedfile-store.js'; import type { ParseWorkerResult } from '../../src/core/ingestion/workers/parse-worker.js'; +import type { ParseCache } from '../../src/storage/parse-cache.js'; import type { ParsedFile } from 'gitnexus-shared'; // A structurally-minimal ParsedFile. `loadParsedFilesForPaths` keys on @@ -309,7 +312,7 @@ describe('parse-impl warm-cache ParsedFile coverage (#2038)', () => { return { path: rel, size: fs.statSync(full).size }; }; - const newCache = () => ({ + const newCache = (): ParseCache => ({ version: PARSE_CACHE_VERSION, entries: new Map(), usedKeys: new Set(), @@ -401,6 +404,279 @@ describe('parse-impl warm-cache ParsedFile coverage (#2038)', () => { expect(cache.onDiskKeys.size + cache.entries.size).toBe(0); }); + // #3204: the chunk above had no prior generation. When one EXISTS, skipping + // the write is not enough — `saveParseCache` copies the pre-existing `.v8` + // forward from `usedKeys`, and the durable prune then keeps the mixed + // directory because it prunes to exactly those saved keys. + const seedThenFailReset = async ( + rel: string, + source: string, + ): Promise<{ + file: { path: string; size: number }; + chunkHash: string; + warm: ReturnType; + }> => { + const file = writeFile(rel, source); + const chunkHash = computeChunkHash([ + { filePath: file.path, contentHash: fileContentHash(source) }, + ]); + const cold = newCache(); + await run(cold, [file]); // miss → populates the parse cache + durable shards + await persistCaches(cold); + + // Corrupt (do not delete) one durable shard: the coherence gate then + // re-dispatches while the previous generation stays on disk, which is the + // only way a chunk is both a live `.v8` entry and a miss in one run. + const chunkDir = path.join(getDurableParsedFileDir(storageDir), chunkHash); + const shard = fs.readdirSync(chunkDir).find((name) => name.endsWith('.v8')); + if (!shard) throw new Error('expected a durable shard to corrupt'); + fs.writeFileSync(path.join(chunkDir, shard), Buffer.from([0, 1, 2])); + + const { loadParseCache } = await import('../../src/storage/parse-cache.js'); + const warm = (await loadParseCache(storageDir)) as ReturnType; + expect(warm.onDiskKeys.has(chunkHash)).toBe(true); // the old generation is live + fs.rmSync(markerPath, { force: true }); + + prepareOverride.impl = () => Promise.reject(new Error('EACCES: simulated cache failure')); + try { + await run(warm, [file]); + } finally { + prepareOverride.impl = undefined; + } + expect(fs.existsSync(markerPath)).toBe(true); // the gate did fall through + return { file, chunkHash, warm }; + }; + + const readSavedIndexKeys = (): string[] => { + const raw = fs.readFileSync(path.join(storageDir, 'parse-cache', 'index.json'), 'utf-8'); + return (JSON.parse(raw) as { keys: string[] }).keys; + }; + + it('drops a pre-existing cache entry when the durable generation could not be reset', async () => { + const { chunkHash, warm } = await seedThenFailReset( + 'src/stale-carryforward.ts', + 'export function carried() { return 1; }\n', + ); + + const { saveParseCache, pruneCache } = await import('../../src/storage/parse-cache.js'); + pruneCache(warm, warm.usedKeys); + const saved = await saveParseCache(storageDir, warm); + + expect(saved).not.toContain(chunkHash); + expect(readSavedIndexKeys()).not.toContain(chunkHash); + expect(fs.existsSync(path.join(storageDir, 'parse-cache', `${chunkHash}.v8`))).toBe(false); + + await pruneAndSaveDurableParsedFileStore( + getDurableParsedFileDir(storageDir), + PARSE_CACHE_VERSION, + new Set(saved), + ); + const { loadDurableParsedFileIndex } = await import('../../src/storage/parsedfile-store.js'); + const durable = await loadDurableParsedFileIndex( + getDurableParsedFileDir(storageDir), + PARSE_CACHE_VERSION, + ); + expect(durable.has(chunkHash)).toBe(false); + }); + + it('keeps the chunk excluded when a post-parse merge re-adds its key', async () => { + // run-analyze folds sibling-branch keys into usedKeys AFTER the parse phase + // (#2106), and retains every loaded key when a sibling meta is unreadable. + // Invalidation has to outlive both, which is why it is filtered at save. + const { chunkHash, warm } = await seedThenFailReset( + 'src/stale-readd.ts', + 'export function readded() { return 1; }\n', + ); + + // Model both merges: the sibling fold re-adds the key, and the + // unreadable-meta fallback unions `entries.keys()` back into `usedKeys`. + // Either one would resurrect the chunk if invalidation were a deletion + // from `usedKeys` instead of a filter at save time. + warm.usedKeys.add(chunkHash); + warm.entries.set(chunkHash, [] as unknown as ParseWorkerResult[]); + warm.onDiskKeys?.add(chunkHash); + for (const key of warm.entries.keys()) warm.usedKeys.add(key); + + const { saveParseCache } = await import('../../src/storage/parse-cache.js'); + const saved = await saveParseCache(storageDir, warm); + + expect(saved).not.toContain(chunkHash); + expect(readSavedIndexKeys()).not.toContain(chunkHash); + }); + + it('retires the chunk when the failed reset left the old generation on disk', async () => { + const { chunkHash, warm } = await seedThenFailReset( + 'src/stale-marking-site.ts', + 'export function marked() { return 1; }\n', + ); + + expect(warm.staleKeys?.has(chunkHash)).toBe(true); + // `onDiskKeys` carried this hash from the loaded index, so clearing it is + // observable; `entries` is empty on the sharded path, which is why the + // in-memory half is proved by the legacy-cache case below instead. + expect(warm.onDiskKeys?.has(chunkHash)).toBe(false); + }); + + it('clears an in-memory entry for a retired chunk', async () => { + // `markParseCacheChunkStale` also drops `entries`, which only matters for a + // legacy (non-sharded) cache whose payloads live in memory. Drive the + // helper directly — the sharded pipeline never populates `entries`, so the + // pipeline test above cannot observe this half. + const { markParseCacheChunkStale, saveParseCache } = + await import('../../src/storage/parse-cache.js'); + const cache = newCache(); + const chunkHash = 'a'.repeat(64); + cache.entries.set(chunkHash, [] as unknown as ParseWorkerResult[]); + cache.onDiskKeys?.add(chunkHash); + cache.usedKeys.add(chunkHash); + + markParseCacheChunkStale(cache, chunkHash); + + expect(cache.entries.has(chunkHash)).toBe(false); + expect(cache.onDiskKeys?.has(chunkHash)).toBe(false); + expect(await saveParseCache(storageDir, cache)).not.toContain(chunkHash); + }); + + it('does NOT retire a chunk when the failed reset left no old generation', async () => { + // `prepareDurableParsedFileChunk` is rm-then-mkdir. When the rm succeeded + // and the mkdir failed there is nothing stale to protect against — the + // workers recreate the directory and write a clean generation — so + // retiring would throw away a good cache entry. Only an rm failure, which + // leaves shards behind, justifies retirement. + const source = 'export function mkdirOnly() { return 1; }\n'; + const file = writeFile('src/mkdir-only.ts', source); + const chunkHash = computeChunkHash([ + { filePath: file.path, contentHash: fileContentHash(source) }, + ]); + const cold = newCache(); + await run(cold, [file]); + await persistCaches(cold); + + // Corrupt a shard so the coherence gate re-dispatches, then model the + // rm-succeeded/mkdir-failed shape: the directory is gone when prepare throws. + const chunkDir = path.join(getDurableParsedFileDir(storageDir), chunkHash); + const shard = fs.readdirSync(chunkDir).find((name) => name.endsWith('.v8')); + if (!shard) throw new Error('expected a durable shard to corrupt'); + fs.writeFileSync(path.join(chunkDir, shard), Buffer.from([0, 1, 2])); + + const { loadParseCache, saveParseCache } = await import('../../src/storage/parse-cache.js'); + const warm = (await loadParseCache(storageDir)) as ParseCache; + prepareOverride.impl = async (durableDir, hash) => { + fs.rmSync(path.join(durableDir, hash), { recursive: true, force: true }); + throw new Error('EMFILE: simulated mkdir failure after a clean rm'); + }; + try { + await run(warm, [file]); + } finally { + prepareOverride.impl = undefined; + } + + expect(warm.staleKeys?.has(chunkHash) ?? false).toBe(false); + expect(await saveParseCache(storageDir, warm)).toContain(chunkHash); + }); + + it('still saves a chunk whose durable generation reset succeeded', async () => { + const f = writeFile('src/healthy.ts', 'export function healthy() { return 1; }\n'); + const chunkHash = computeChunkHash([ + { + filePath: f.path, + contentHash: fileContentHash('export function healthy() { return 1; }\n'), + }, + ]); + const cache = newCache(); + + await run(cache, [f]); + const { saveParseCache } = await import('../../src/storage/parse-cache.js'); + const saved = await saveParseCache(storageDir, cache); + + expect(saved).toContain(chunkHash); + expect(cache.staleKeys?.has(chunkHash) ?? false).toBe(false); + expect(fs.existsSync(path.join(storageDir, 'parse-cache', `${chunkHash}.v8`))).toBe(true); + const { loadDurableParsedFileIndex: loadIdx } = + await import('../../src/storage/parsedfile-store.js'); + await pruneAndSaveDurableParsedFileStore( + getDurableParsedFileDir(storageDir), + PARSE_CACHE_VERSION, + new Set(saved), + ); + expect( + (await loadIdx(getDurableParsedFileDir(storageDir), PARSE_CACHE_VERSION)).has(chunkHash), + ).toBe(true); + }); + + it('re-dispatches on the run after a failed reset instead of taking a warm hit', async () => { + const { file, chunkHash, warm } = await seedThenFailReset( + 'src/stale-nextrun.ts', + 'export function nextRun() { return 1; }\n', + ); + await persistCaches(warm); + + const { loadParseCache } = await import('../../src/storage/parse-cache.js'); + const third = (await loadParseCache(storageDir)) as ReturnType; + expect(third.onDiskKeys.has(chunkHash)).toBe(false); + fs.rmSync(markerPath, { force: true }); + + await run(third, [file]); + + expect(fs.existsSync(markerPath)).toBe(true); + }); + + it('retires only the failing chunk — a sibling chunk stays warm through the next run', async () => { + // The single-chunk tests cannot tell "retires the failing chunk" from + // "retires everything": one chunk plus a global spawn marker look the same + // either way. Two chunks, one failure, and a per-chunk assertion can. + const aSrc = 'export function a() { return 1; }\n'; + const bSrc = 'export function b() { return 2; }\n'; + const a = writeFile('src/a.ts', aSrc); + const b = writeFile('src/b.ts', bSrc); + const hashOf = (rel: string, src: string): string => + computeChunkHash([{ filePath: rel, contentHash: fileContentHash(src) }]); + const aHash = hashOf(a.path, aSrc); + const bHash = hashOf(b.path, bSrc); + + // chunkByteBudget 1 forces one file per chunk, so a and b hash distinctly. + const cold = newCache(); + await run(cold, [a, b], 1); + await persistCaches(cold); + + // Corrupt only a's durable shard: a re-dispatches, b still hits warm. + const aDir = path.join(getDurableParsedFileDir(storageDir), aHash); + const aShard = fs.readdirSync(aDir).find((name) => name.endsWith('.v8')); + if (!aShard) throw new Error('expected a durable shard for a'); + fs.writeFileSync(path.join(aDir, aShard), Buffer.from([0, 1, 2])); + + const { loadParseCache } = await import('../../src/storage/parse-cache.js'); + const warm = (await loadParseCache(storageDir)) as ParseCache; + prepareOverride.impl = (_durableDir, hash) => + hash === aHash + ? Promise.reject(new Error('EACCES: simulated cache failure')) + : Promise.resolve(); + try { + await run(warm, [a, b], 1); + } finally { + prepareOverride.impl = undefined; + } + await persistCaches(warm); + + // b survived the save; only a was retired. + expect(warm.staleKeys?.has(aHash)).toBe(true); + expect(warm.staleKeys?.has(bHash) ?? false).toBe(false); + + // Actually perform the next run. An index entry alone does not exercise the + // warm-hit/coherence path, so the claim in the title has to be paid for. + // Run each chunk on its own so the single global spawn marker is + // unambiguous about WHICH chunk re-dispatched. + const bOnly = (await loadParseCache(storageDir)) as ParseCache; + fs.rmSync(markerPath, { force: true }); + await run(bOnly, [b], 1); + expect(fs.existsSync(markerPath)).toBe(false); // sibling served warm + + const aOnly = (await loadParseCache(storageDir)) as ParseCache; + fs.rmSync(markerPath, { force: true }); + await run(aOnly, [a], 1); + expect(fs.existsSync(markerPath)).toBe(true); // retired chunk re-parsed + }); + it('retains worker ParsedFiles when the main-thread run-store write fails', async () => { const f = writeFile( 'src/persist-fallback.ts', diff --git a/gitnexus/test/unit/parsedfile-store.test.ts b/gitnexus/test/unit/parsedfile-store.test.ts index ad012bd58..79187f73a 100644 --- a/gitnexus/test/unit/parsedfile-store.test.ts +++ b/gitnexus/test/unit/parsedfile-store.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; -import { promises as nodeFsPromises } from 'node:fs'; +import { promises as nodeFsPromises, existsSync } from 'node:fs'; import v8 from 'node:v8'; import { mkdtemp, rm, readdir, readFile, writeFile } from 'fs/promises'; import { tmpdir } from 'os'; @@ -18,6 +18,7 @@ import { prepareDurableParsedFileChunk, pruneAndSaveDurableParsedFileStore, mergeStagedDurableParsedFileStore, + loadDurableParsedFileIndex, } from '../../src/storage/parsedfile-store.js'; /** @@ -908,6 +909,52 @@ describe('parsedfile-store receiverChain sanitation', () => { } }); + // chmod is the only lever that makes a real `fs.rm` reject here, and root + // ignores directory write permission while Windows treats the mode bits as a + // near no-op. Skipping is honest; a green vacuous run is not. + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'keeps pruning and still writes the index when one chunk directory cannot be removed', + async () => { + // #3204: the non-survivor delete targets the same directory whose reset + // may have failed, so the causes that break the reset (permissions, a + // locked file, a read-only mount) break this rm too. One undeletable + // directory must not cost every other chunk its index entry. + const dir = await mkdtemp(path.join(tmpdir(), 'pf-rm-fail-')); + const durableDir = getDurableParsedFileDir(dir); + const undeletable = '3'.repeat(64); + const keep = '4'.repeat(64); + try { + await prepareDurableParsedFileChunk(durableDir, undeletable); + persistDurableParsedFileShardSync(durableDir, undeletable, 1, 0, [ + makeParsedFile('gone.c'), + ]); + await prepareDurableParsedFileChunk(durableDir, keep); + persistDurableParsedFileShardSync(durableDir, keep, 1, 0, [makeParsedFile('keep.c')]); + + // Clearing write permission on the chunk directory makes its shards + // un-unlinkable, so the recursive rm of that directory rejects while the + // store root stays writable for the index rewrite. + const doomed = path.join(durableDir, undeletable); + await nodeFsPromises.chmod(doomed, 0o555); + await expect( + pruneAndSaveDurableParsedFileStore(durableDir, 'v-test', new Set([keep])), + ).resolves.toBeUndefined(); + + // Assert the premise: the directory SURVIVED, i.e. the rm really did + // reject. Without this the test passes wherever the delete succeeds and + // would stay green if the try/catch were reverted. + expect(existsSync(doomed)).toBe(true); + + const index = await loadDurableParsedFileIndex(durableDir, 'v-test'); + expect(index.has(keep)).toBe(true); + expect(index.has(undeletable)).toBe(false); + } finally { + await nodeFsPromises.chmod(path.join(durableDir, undeletable), 0o755).catch(() => {}); + await rm(dir, { recursive: true, force: true }); + } + }, + ); + it('overlays staged durable chunks onto the live store without dropping live-only keys', async () => { const live = await mkdtemp(path.join(tmpdir(), 'pf-live-')); const staged = await mkdtemp(path.join(tmpdir(), 'pf-stg-'));