From b62b94874ddf4102bd20f29072afd80d71eec3ce Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Thu, 24 Sep 2026 16:00:12 +0000 Subject: [PATCH] fix(storage): lock the embed-job graph copy and skip it on forced rebuilds (#3352) The server embed job copies a shared checkout's graph under the slot's index lock, so a CLI analyze in another process cannot interleave. A forced rebuild with no embeddings to carry over drops the pointer without copying a graph it would discard unread. Co-Authored-By: Claude Opus 5.5 (1M context) --- gitnexus/src/core/run-analyze.ts | 13 +++++++-- gitnexus/src/core/shared-store-analyze.ts | 13 +++++++-- gitnexus/src/server/api.ts | 16 +++++++++-- .../integration/shared-store-seed.test.ts | 28 +++++++++++++++++++ 4 files changed, 62 insertions(+), 8 deletions(-) diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index a0a7dee60..61e4f87fc 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -1400,9 +1400,11 @@ async function runFullAnalysisInner( // Shared-store pointer slots (#3352) get a private graph just before the // first graph open: here for the paths that open it before the up-to-date // check, and below once that check falls through. - const ensurePrivateGraph = async (): Promise => { + const ensurePrivateGraph = async (copy = true): Promise => { if (!writeTarget.sharedStore || placement.branch) return; - if (!(await ensurePrivateSharedGraph(metaDir, log))) options = { ...options, force: true }; + if (!(await ensurePrivateSharedGraph(metaDir, log, { copy }))) { + options = { ...options, force: true }; + } // Later dirty-flag writes spread the in-memory metadata; keep them from // re-recording the pointer this slot just left. delete loadedMeta?.graphPath; @@ -2362,7 +2364,12 @@ async function runFullAnalysisInner( } await ensureWritableStorage(); - await ensurePrivateGraph(); + // A forced rebuild reads the old graph only to carry embeddings over; with + // none to carry, copying the shared graph would be thrown away unread. + const forcedRebuildReadsOldGraph = + resumeEmbeddingCheckpoint || + _deriveEmbeddingMode(options, existingMeta?.stats?.embeddings ?? 0).shouldLoadCache; + await ensurePrivateGraph(!options.force || forcedRebuildReadsOldGraph); delete existingMeta?.graphPath; // ── Cache embeddings from existing index before rebuild ──────────── diff --git a/gitnexus/src/core/shared-store-analyze.ts b/gitnexus/src/core/shared-store-analyze.ts index bc2302e25..5bf862a0d 100644 --- a/gitnexus/src/core/shared-store-analyze.ts +++ b/gitnexus/src/core/shared-store-analyze.ts @@ -261,18 +261,25 @@ export const seedSharedSlot = async ( /** * Turn a pointer slot into a private one before analyze opens or writes the - * graph. Returns false when the pointed-at shared graph cannot be copied + * graph. With `copy: false` the slot just stops pointing and the caller builds + * its own graph from scratch. Returns false when the pointed-at shared graph cannot be copied * (garbage-collected or unreadable): the slot's file hashes then describe a * graph that is not there, and the caller must do a full build. Caller holds * the slot's index lock. */ -export const ensurePrivateSharedGraph = async (slot: string, log: Log): Promise => { +export const ensurePrivateSharedGraph = async ( + slot: string, + log: Log, + opts: { copy?: boolean } = {}, +): Promise => { const own = path.join(slot, LBUG_DIRECTORY); const pointed = resolveGraphPath(slot); if (pointed === own) return true; const meta = await loadMeta(slot); if (!meta) return true; - if (!(await exists(own))) { + // `copy: false` — the caller rebuilds from scratch and reads nothing from + // the old graph, so only the pointer is dropped. + if (opts.copy !== false && !(await exists(own))) { const started = Date.now(); try { await cloneGraphFile(pointed, own); diff --git a/gitnexus/src/server/api.ts b/gitnexus/src/server/api.ts index 79b3077a0..a30092586 100644 --- a/gitnexus/src/server/api.ts +++ b/gitnexus/src/server/api.ts @@ -8,6 +8,7 @@ * CORS is restricted to localhost, private/LAN networks, and the deployed site. */ +import { acquireIndexLock, requireExclusiveIndexLock } from '../storage/index-lock.js'; import { ensurePrivateSharedGraph } from '../core/shared-store-analyze.js'; import { resolveGraphPath } from '../storage/shared-store.js'; import { reclaimAfterSlotRemoval } from '../storage/shared-store-lifecycle.js'; @@ -2132,8 +2133,19 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => try { // Writes go to the slot's own graph; a shared-store checkout // reading an immutable commit graph (#3352) takes a private copy. - if (!(await ensurePrivateSharedGraph(storagePath, () => {}))) { - throw new Error('The shared graph this repository reads is gone. Re-run analyze.'); + // The in-memory repo lock only serializes this server; a CLI + // analyze in another process guards the slot with the index lock. + const slotLock = await acquireIndexLock(storagePath); + try { + requireExclusiveIndexLock( + slotLock, + `Cannot acquire the index lock at ${storagePath}; refusing to copy the shared graph.`, + ); + if (!(await ensurePrivateSharedGraph(storagePath, () => {}))) { + throw new Error('The shared graph this repository reads is gone. Re-run analyze.'); + } + } finally { + slotLock.release(); } const lbugPath = path.join(storagePath, LBUG_DIRECTORY); const ftsSession = await loadFtsSession(storagePath); diff --git a/gitnexus/test/integration/shared-store-seed.test.ts b/gitnexus/test/integration/shared-store-seed.test.ts index 7f712bd89..191e362ac 100644 --- a/gitnexus/test/integration/shared-store-seed.test.ts +++ b/gitnexus/test/integration/shared-store-seed.test.ts @@ -176,6 +176,26 @@ describe('shared store seeding (#3352)', () => { expect(await queryNames(graphOf(orphan))).toEqual(['zeta']); }, 240_000); + it('a forced rebuild of a pointer slot builds without copying the shared graph', async () => { + const wt = addWorktree('wt-a'); + await analyze(main); + await analyze(wt); + const shared = graphOf(wt); + expect(path.dirname(path.dirname(shared))).toBe(layoutOf(wt).commitsDir); + + const logs: string[] = []; + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + await runFullAnalysis( + wt, + { force: true }, + { onProgress: () => {}, onLog: (m) => logs.push(m) }, + ); + + expect(logs.some((m) => m.startsWith('Shared store: copied the shared graph'))).toBe(false); + expect(await queryNames(graphOf(wt))).toEqual(['alpha', 'beta']); + expect(existsSync(shared)).toBe(true); + }, 240_000); + it('matches a from-scratch build after seeding and updating (R9)', async () => { const wt = addWorktree('wt-a'); await analyze(main); @@ -241,6 +261,14 @@ describe('ensurePrivateSharedGraph', () => { expect((await fs.readdir(slot)).filter((n) => n.startsWith('lbug'))).toEqual([]); }); + it('with copy: false drops the pointer without copying', async () => { + const { slot, graph } = await pointerSlot(); + await fs.writeFile(graph, 'shared graph bytes'); + expect(await ensurePrivateSharedGraph(slot, () => {}, { copy: false })).toBe(true); + expect(existsSync(path.join(slot, 'lbug'))).toBe(false); + expect((await loadMeta(slot))?.graphPath).toBeUndefined(); + }); + it('is a no-op for a slot that already owns its graph', async () => { const { slot } = await pointerSlot(); const meta = await loadMeta(slot);