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); +});