fix(storage): address seventh review round on the shared store (#3374)

- clean --gc aborts when the registry cannot be read instead of treating
  every member as orphaned; with no registry file it collects only
  members whose checkout directory is gone
- seeding from a repository-local index re-reads its metadata under the
  index lock, so the copied graph and the saved metadata match
- clean --gc skips a store another collector removed meanwhile, and
  reports "no shared stores" when stores/ holds only stray files

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

gitnexus/src/cli/clean.ts

@@ -174,15 +174,23 @@ const collectSharedStores = async (force: boolean): Promise<void> => {
     if (err.code === 'ENOENT') return [] as string[];
     throw err;
   });
-  if (names.length === 0) {
+  // Stray files (`.DS_Store`) are not stores, and a symlink is not followed:
+  // reclaim deletes under the root it is given. A store another collector
+  // removed meanwhile is simply gone.
+  const roots: string[] = [];
+  for (const name of names) {
+    const root = path.join(storesDir, name);
+    const stat = await fs.lstat(root).catch((err: NodeJS.ErrnoException) => {
+      if (err.code === 'ENOENT') return null;
+      throw err;
+    });
+    if (stat?.isDirectory()) roots.push(root);
+  }
+  if (roots.length === 0) {
     console.log(t('clean.gc.none'));
     return;
   }
-  for (const name of names) {
-    const root = path.join(storesDir, name);
-    // Stray files (`.DS_Store`) are not stores, and a symlink is not followed:
-    // reclaim deletes under the root it is given.
-    if (!(await fs.lstat(root)).isDirectory()) continue;
+  for (const root of roots) {
     // Without --force this is a preview: same selection, nothing deleted.
     const result = await reclaimSharedStore(root, { gc: true, dryRun: !force });
     console.log(

gitnexus/src/core/shared-store-analyze.ts

@@ -182,18 +182,26 @@ const seedFromLocalIndex = async (
   log: Log,
 ): Promise<boolean> => {
   const sourceGraph = path.join(source, LBUG_DIRECTORY);
-  const meta = await loadMeta(source);
-  if (!meta || meta.incrementalInProgress || !meta.lastCommit) return false;
-  if (!(await exists(sourceGraph))) return false;
-  if (commitDistanceToHead(repoPath, meta.lastCommit) === null) return false;
+  const usable = (m: RepoMeta | null): m is RepoMeta =>
+    !!m &&
+    !m.incrementalInProgress &&
+    !!m.lastCommit &&
+    commitDistanceToHead(repoPath, m.lastCommit) !== null;
+  if (!usable(await loadMeta(source)) || !(await exists(sourceGraph))) return false;
   let lock;
   try {
     lock = await acquireIndexLock(source, { timeoutMs: 2_000 });
   } catch {
     return false; // another analyze is writing it; seed from scratch instead
   }
+  let meta: RepoMeta;
   try {
     if (lock.lockFree || (await inspectLbugSidecars(sourceGraph)).kind !== 'clean') return false;
+    // Re-read under the lock: an analyze that finished while this waited
+    // rewrote both, and the copied graph must match the saved metadata.
+    const locked = await loadMeta(source);
+    if (!usable(locked)) return false;
+    meta = locked;
     await fs.mkdir(slot, { recursive: true });
     try {
       await cloneGraphFile(sourceGraph, path.join(slot, LBUG_DIRECTORY));

gitnexus/src/storage/shared-store-lifecycle.ts

@@ -16,7 +16,7 @@ import { acquireIndexLock, requireExclusiveIndexLock, type IndexLockHandle } fro
 import {
   canonicalizePath,
   findRegistryEntryByRepoPath,
-  readRegistry,
+  readRegistryStrictIfPresent,
   registryPathEquals,
 } from './repo-manager.js';
 import { loadMeta } from './repo-meta.js';
@@ -78,17 +78,22 @@ const listDirStrict = (dir: string): Promise<string[]> =>
  * gone, or its entry moved elsewhere (`--no-share`, sharing turned off). The
  * registry is the membership record for opted-in clones and for a main
  * checkout whose last worktree was removed, so identity alone cannot decide.
- * A slot with no attributable `repoPath` is never collected.
+ * A slot with no attributable `repoPath` is never collected. An unreadable
+ * registry aborts; without a registry file only slots whose checkout is gone
+ * are collected.
  */
 const orphanMembers = async (slots: string[]): Promise<Set<string>> => {
-  const entries = await readRegistry();
+  const entries = await readRegistryStrictIfPresent();
   const orphans = new Set<string>();
   for (const slot of slots) {
     const meta = await loadMeta(slot);
     if (!meta?.repoPath) continue;
-    const entry = existsSync(meta.repoPath)
-      ? findRegistryEntryByRepoPath(entries, meta.repoPath)
-      : undefined;
+    if (!existsSync(meta.repoPath)) {
+      orphans.add(slot);
+      continue;
+    }
+    if (!entries) continue;
+    const entry = findRegistryEntryByRepoPath(entries, meta.repoPath);
     if (
       !entry ||
       !registryPathEquals(canonicalizePath(entry.storagePath), canonicalizePath(slot))

gitnexus/test/integration/shared-store-clean.test.ts

@@ -170,6 +170,32 @@ describe('shared store clean (#3352)', () => {
     expect(existsSync(path.join(decoy, 'commits', 'ddddddd-4444444444444444'))).toBe(true);
   }, 240_000);
 
+  it('clean --gc reports no stores when stores/ holds only stray files', async () => {
+    await fs.mkdir(path.join(tmpHome.dbPath, 'stores'), { recursive: true });
+    await fs.writeFile(path.join(tmpHome.dbPath, 'stores', '.DS_Store'), 'x');
+    const logs = await cleanIn(main, { gc: true });
+    expect(logs).toContain('No shared stores to collect.');
+  });
+
+  it('clean --gc skips a store removed by a concurrent collector', async () => {
+    const gone = path.join(tmpHome.dbPath, 'stores', 'repo-0123456789ab');
+    await fs.mkdir(gone, { recursive: true });
+    const realLstat = fs.lstat;
+    const lstat = vi.spyOn(fs, 'lstat').mockImplementation((async (
+      target: string,
+      ...rest: unknown[]
+    ) => {
+      if (String(target) === gone) throw Object.assign(new Error('gone'), { code: 'ENOENT' });
+      return (realLstat as (...a: unknown[]) => Promise<unknown>)(target, ...rest);
+    }) as typeof fs.lstat);
+    try {
+      const logs = await cleanIn(main, { gc: true, force: true });
+      expect(logs).toContain('No shared stores to collect.');
+    } finally {
+      lstat.mockRestore();
+    }
+  });
+
   it('clean --gc without --force previews and deletes nothing', async () => {
     await analyze(wtA);
     await fs.writeFile(path.join(wtB, 'b.ts'), 'export function beta() { return 2; }\n');
@@ -340,6 +366,23 @@ describe('reclaimSharedStore', () => {
     expect(existsSync(path.join(dir, 'store.json'))).toBe(false);
   });
 
+  it('aborts instead of collecting members when the registry cannot be read', async () => {
+    const slot = await member('wt-000000000000', { repoPath: tmpHome.dbPath });
+    await fs.writeFile(path.join(tmpHome.dbPath, 'registry.json'), '{not json');
+    await expect(reclaimSharedStore(layout().root, { gc: true })).rejects.toThrow();
+    expect(existsSync(slot)).toBe(true);
+  });
+
+  it('without a registry file collects only members whose checkout is gone', async () => {
+    const live = await member('wt-000000000000', { repoPath: tmpHome.dbPath });
+    const dead = await member('wt-111111111111', {
+      repoPath: path.join(tmpHome.dbPath, 'deleted-worktree'),
+    });
+    await reclaimSharedStore(layout().root, { gc: true });
+    expect(existsSync(live)).toBe(true);
+    expect(existsSync(dead)).toBe(false);
+  });
+
   it('reports a graph it cannot delete instead of failing', async () => {
     const orphan = await commitGraph('ccccccc-3333333333333333');
     await member('wt-000000000000', {});