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) <noreply@anthropic.com>
This commit is contained in:
Gergo Magyar 2026-09-24 16:00:12 +00:00
parent 2be2701234
commit b62b94874d
4 changed files with 62 additions and 8 deletions

View file

@ -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<void> => {
const ensurePrivateGraph = async (copy = true): Promise<void> => {
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 ────────────

View file

@ -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<boolean> => {
export const ensurePrivateSharedGraph = async (
slot: string,
log: Log,
opts: { copy?: boolean } = {},
): Promise<boolean> => {
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);

View file

@ -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);

View file

@ -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);