From 62ffee3218f42e97dd0245e0a8784eeafc7ffbd4 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Fri, 25 Sep 2026 07:26:12 +0000 Subject: [PATCH] fix(storage): rebuild a shared slot whose graph is missing (#3374) A publish interrupted between moving the checkout's graph into `.publish-` staging and renaming staging onto the commit dir left slot metadata at HEAD with no graph and no graphPath; the next reclaim deleted the orphaned staging. The up-to-date fast path never checked that the graph exists, so every later analyze reported "Already up to date" over a checkout with no index. A failed restore in publishSharedGraph's catch had the same outcome, and also deleted the staging dir holding the only copy of the graph. - run-analyze: for a shared-store slot, stat the graph the checkout reads (resolveGraphPath for the flat slot, the branch slot's own lbug otherwise) before the fast path; if it is missing, force a full rebuild. An incremental run would diff nothing into a fresh, empty database. Private .gitnexus indexes are unchanged: they only lose their graph by hand, and metadata-only fast-path fixtures rely on that path. - publishSharedGraph: when moving the staged graph back fails and it is still in staging, keep the staging dir, log its path, and skip this run's reclaim, which would otherwise delete it. The next analyze finds no graph and rebuilds. Co-Authored-By: Claude Opus 5.5 (1M context) --- gitnexus/src/core/run-analyze.ts | 17 ++- gitnexus/src/core/shared-store-analyze.ts | 26 +++- .../integration/shared-store-analyze.test.ts | 135 +++++++++++++++++- 3 files changed, 172 insertions(+), 6 deletions(-) diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 51e1a86bd..1126a3f65 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -33,7 +33,7 @@ import { import { PDG_EDGE_TYPES } from './lbug/pdg-emit-sink.js'; import path from 'path'; import fs from 'fs/promises'; -import { constants as fsConstants } from 'node:fs'; +import { constants as fsConstants, existsSync } from 'node:fs'; import { randomUUID } from 'node:crypto'; import { retryRename } from '../storage/fs-atomic.js'; import { acquireIndexLock, requireExclusiveIndexLock } from '../storage/index-lock.js'; @@ -170,6 +170,7 @@ import { } from '../storage/storage-resolver.js'; import { isSharedStoreDisabled, + resolveGraphPath, resolveSharedStore, storeRootOfCheckoutSlot, type SharedStoreLayout, @@ -2209,6 +2210,20 @@ async function runFullAnalysisInner( processDetectionBudget, ); + // A shared-store slot (#3352) can record HEAD with no graph behind it: a + // publish interrupted between its renames, or a commit graph reclaimed from + // under the pointer. Neither the fast path nor an incremental diff (which + // writes only changed files into a fresh, empty database) would restore it, + // so rebuild. Scoped to store slots: private `.gitnexus` indexes only lose + // their graph by hand, and their metadata-only fixtures rely on this path. + if (existingMeta && !options.force && storeRootOfCheckoutSlot(storagePath)) { + const graph = placement.branch ? lbugPath : resolveGraphPath(storagePath); + if (!existsSync(graph)) { + log('Shared store: this checkout has no graph; doing a full build.'); + options = { ...options, force: true }; + } + } + // ── Early-return: already up to date ────────────────────────────── if ( existingMeta && diff --git a/gitnexus/src/core/shared-store-analyze.ts b/gitnexus/src/core/shared-store-analyze.ts index 646f5bd86..0cffb4dfc 100644 --- a/gitnexus/src/core/shared-store-analyze.ts +++ b/gitnexus/src/core/shared-store-analyze.ts @@ -378,6 +378,7 @@ export const publishSharedGraph = async ( // Every pointer change and the reclaim that follows run under one publish // lock, so a concurrent reclaim never sees a half-recorded reference. await withStoreLock(layout, 'publish', async () => { + let keptStaging = false; if (shareable) { const target = commitGraphDir(layout, currentCommit, featureKeyOf(meta)); const targetGraph = path.join(target, LBUG_DIRECTORY); @@ -423,11 +424,27 @@ export const publishSharedGraph = async ( log(`Shared store: published commit graph ${currentCommit.slice(0, 12)}.`); } catch (err) { // Put the graph back so the slot stays usable as a private index. - await fs.rename(path.join(staging, LBUG_DIRECTORY), own).catch(() => {}); - await fs.rm(staging, { recursive: true, force: true }).catch(() => {}); - log( - `Shared store: could not publish (${(err as Error).message}); keeping a private graph.`, + const staged = path.join(staging, LBUG_DIRECTORY); + const restored = await fs.rename(staged, own).then( + () => true, + () => false, ); + if (restored || !(await exists(staged))) { + await fs.rm(staging, { recursive: true, force: true }).catch(() => {}); + log( + `Shared store: could not publish (${(err as Error).message}); keeping a private graph.`, + ); + } else { + // The staging dir now holds this checkout's only graph. Keep it, + // and skip this run's reclaim (which deletes unreferenced staging), + // so it can be moved back by hand; the next analyze of this + // checkout finds no graph and rebuilds. + keptStaging = true; + log( + `Shared store: could not publish (${(err as Error).message}) or restore the graph; ` + + `it is at ${staged}.`, + ); + } } } if (published) { @@ -438,6 +455,7 @@ export const publishSharedGraph = async ( delete meta.graphPath; await saveMeta(slot, meta); } + if (keptStaging) return; // Best effort: an unreadable store must not fail a finished analysis. try { const reclaimed = await reclaimSharedStoreLocked(layout.root); diff --git a/gitnexus/test/integration/shared-store-analyze.test.ts b/gitnexus/test/integration/shared-store-analyze.test.ts index c3c8fbcd0..cfa97fec2 100644 --- a/gitnexus/test/integration/shared-store-analyze.test.ts +++ b/gitnexus/test/integration/shared-store-analyze.test.ts @@ -2,7 +2,11 @@ import { execFileSync } from 'child_process'; import { existsSync } from 'fs'; import fs from 'fs/promises'; import path from 'path'; -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { pathToFileURL } from 'url'; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { CLASS_FRAMEWORK_ANNOTATIONS_FEATURE } from '../../src/core/analysis-features.js'; +import { resolveAnalyzerRunnerIdentity } from '../../src/core/analyzer-identity.js'; +import { SCHEMA_FINGERPRINT } from '../../src/core/lbug/schema.js'; import { ensurePrivateSharedGraph, featureKeyOf, @@ -18,6 +22,7 @@ import { import type { RepoMeta } from '../../src/storage/repo-meta.js'; import { commitGraphDir, + resolveGraphPath, resolveSharedStore, type SharedStoreLayout, } from '../../src/storage/shared-store.js'; @@ -351,6 +356,59 @@ describe('publishSharedGraph race (#3352)', () => { expect(existsSync(path.join(layoutOf(main).checkoutSlot, 'lbug'))).toBe(true); }); + /** Fail every rename onto one of `blocked`; others run for real. */ + const blockRenamesOnto = (blocked: ReadonlySet) => { + const realRename = fs.rename.bind(fs); + return vi + .spyOn(fs, 'rename') + .mockImplementation((from, to) => + blocked.has(String(to)) + ? Promise.reject(Object.assign(new Error('rename blocked'), { code: 'EIO' })) + : realRename(from, to), + ); + }; + + // #3374: the staging dir holds the checkout's only graph once putting it + // back fails; deleting it would leave metadata at HEAD with no graph. + it('keeps the staged graph when putting it back fails', async () => { + const { checkouts, head } = await setup(); + const [main] = checkouts; + const layout = layoutOf(main); + const slot = layout.checkoutSlot; + const meta = (await loadMeta(slot)) as RepoMeta; + const target = commitGraphDir(layout, head, featureKeyOf(meta)); + const spy = blockRenamesOnto(new Set([target, path.join(slot, 'lbug')])); + try { + await publishSharedGraph(layout, main, head, () => {}); + } finally { + spy.mockRestore(); + } + const staging = (await fs.readdir(layout.commitsDir)).filter((n) => n.startsWith('.publish-')); + expect(staging).toHaveLength(1); + expect(await fs.readFile(path.join(layout.commitsDir, staging[0], 'lbug'), 'utf-8')).toBe( + `graph from ${main}`, + ); + expect(await listCommitDirs(layout)).toEqual([]); + expect((await loadMeta(slot))?.graphPath).toBeUndefined(); + }); + + it('drops the staging dir when the graph never left the slot', async () => { + const { checkouts, head } = await setup(); + const [main] = checkouts; + const layout = layoutOf(main); + const slot = layout.checkoutSlot; + const meta = (await loadMeta(slot)) as RepoMeta; + const target = commitGraphDir(layout, head, featureKeyOf(meta)); + const spy = blockRenamesOnto(new Set([target])); + try { + await publishSharedGraph(layout, main, head, () => {}); + } finally { + spy.mockRestore(); + } + expect(await fs.readdir(layout.commitsDir)).toEqual([]); + expect(await fs.readFile(path.join(slot, 'lbug'), 'utf-8')).toBe(`graph from ${main}`); + }); + const seedFreshSlot = async (checkout: string): Promise => { const layout = layoutOf(checkout); await fs.rm(layout.checkoutSlot, { recursive: true, force: true }); @@ -450,3 +508,78 @@ describe('publishSharedGraph race (#3352)', () => { expect(await fs.readFile(path.join(target, 'lbug'), 'utf-8')).toBe('older published graph'); }); }); + +// #3374: a publish interrupted between its renames (or a reclaimed commit +// graph) leaves slot metadata at HEAD with no graph behind it. +describe('up-to-date fast path over a missing shared graph (#3374)', () => { + let tmpHome: Awaited>; + let tmpRepo: Awaited>; + let savedHome: string | undefined; + + beforeEach(async () => { + tmpHome = await createTempDir('gitnexus-test-shared-missing-home-'); + tmpRepo = await createTempDir('gitnexus-test-shared-missing-repo-'); + savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + }); + + afterEach(async () => { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpRepo.cleanup(); + await tmpHome.cleanup(); + }); + + it('rebuilds a slot whose metadata is at HEAD but whose graph is gone', async () => { + const root = await fs.realpath(tmpRepo.dbPath); + const main = path.join(root, 'main'); + await fs.mkdir(main); + git(main, 'init', '-q', '-b', 'main'); + git( + main, + '-c', + 'user.name=t', + '-c', + 'user.email=t@t', + 'commit', + '-q', + '--allow-empty', + '-m', + 'init', + ); + const wt = path.join(root, 'wt'); + git(main, 'worktree', 'add', '-q', '-b', 'wt', wt); + const slot = layoutOf(wt).checkoutSlot; + await fs.mkdir(slot, { recursive: true }); + await saveMeta(slot, { + repoPath: wt, + storagePath: slot, + lastCommit: git(wt, 'rev-parse', 'HEAD'), + indexedAt: new Date().toISOString(), + schemaFingerprint: SCHEMA_FINGERPRINT, + analysisFeatures: { + [CLASS_FRAMEWORK_ANNOTATIONS_FEATURE.id]: CLASS_FRAMEWORK_ANNOTATIONS_FEATURE.version, + }, + runnerIdentity: resolveAnalyzerRunnerIdentity( + pathToFileURL(path.resolve(__dirname, '../../src/core/run-analyze.ts')).href, + ), + // Same FTS mode as the run below, so only the missing graph can + // decide against the fast path. + capabilities: { + graph: { provider: 'ladybugdb', status: 'available' }, + fts: { provider: 'ladybugdb-fts', status: 'unavailable', skipReason: 'disabled-by-flag' }, + vectorSearch: { provider: 'exact-scan', status: 'unavailable', exactScanLimit: 0 }, + }, + }); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis( + wt, + { skipAgentsMd: true, skipSkills: true, skipFts: true }, + { onProgress: () => {} }, + ); + + expect(result.alreadyUpToDate).not.toBe(true); + expect(existsSync(resolveGraphPath(slot))).toBe(true); + }, 120_000); +});