fix(reindex-frontmatter): reuse the connected engine instead of self-deadlocking on the PGLite lock (#1963)

cli.ts's dispatch connects the primary engine (taking the PGLite data-dir
lock), then reindexFrontmatterCli built and connected a SECOND engine on
the same data dir. acquireLock never reaps a live PID — and the recorded
holder was this very process — so every 'gbrain reindex-frontmatter' on
PGLite spun the full 30s lock timeout and exited 1, reporting itself as
the blocking process. (The issue's statement_timeout theory was a red
herring: the backfill itself finishes in milliseconds.)

'gbrain backfill <kind>' had the identical double-connect and is fixed
the same way: both commands now take the already-connected engine from
the dispatch switch, and cli.ts's shared finally owns teardown.

Deliberately NOT changed: pglite-lock.ts. Making the lock reentrant (or
special-casing our own PID) would contradict the pinned #2348 semantics
— a live-PID holder is never stolen, PID-reuse included — and the root
cause is the second connect, not the lock.

Regression test spawns the real CLI against a fresh PGLite home
(init → import → reindex-frontmatter → backfill effective_date); on
master it fails after the 30s self-deadlock, with the fix it passes in
seconds.

Fixes #1963

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/cli.ts

@@ -2253,16 +2253,24 @@ async function handleCliOnly(command: string, args: string[]) {
         //
         // v0.30.1: still works; canonical entrypoint is now `gbrain backfill
         // effective_date`. This command stays as a thin alias for back-compat.
+        //
+        // #1963: pass the already-connected engine. The command used to build
+        // + connect its OWN engine here, which self-deadlocked on the PGLite
+        // data-dir lock (this process already holds it via connectEngine
+        // above) — 30s spin, then exit 1, on every PGLite invocation.
         const { reindexFrontmatterCli } = await import('./commands/reindex-frontmatter.ts');
-        await reindexFrontmatterCli(args);
-        return; // reindexFrontmatterCli handles its own engine lifecycle
+        await reindexFrontmatterCli(engine, args);
+        break;
       }
       case 'backfill': {
         // v0.30.1: first-class generic backfill command. Subcommand dispatch
         // is inside runBackfillCommand (kind | list | --help).
+        // #1963: same double-connect class as reindex-frontmatter — reuse the
+        // connected engine instead of building a second one on the same
+        // PGLite data dir.
         const { runBackfillCommand } = await import('./commands/backfill.ts');
-        await runBackfillCommand(args);
-        return;
+        await runBackfillCommand(engine, args);
+        break;
       }
       case 'code-callers': {
         // v0.20.0 Cathedral II Layer 10 (C4): "who calls <symbol>?"

src/commands/backfill.ts

@@ -16,10 +16,10 @@
  * always reserving 1 connection for HNSW + heartbeat + doctor probes.
  */
 
+import type { BrainEngine } from '../core/engine.ts';
 import { resolveDirectPoolSize } from '../core/connection-manager.ts';
 import { listBackfills, getBackfill } from '../core/backfill-registry.ts';
 import { runBackfill, clearBackfillCheckpoint } from '../core/backfill-base.ts';
-import { loadConfig, toEngineConfig } from '../core/config.ts';
 
 interface BackfillArgs {
   kind?: string;
@@ -114,7 +114,14 @@ function clampConcurrency(requested: number | undefined): { effective: number; w
   return { effective: requested };
 }
 
-export async function runBackfillCommand(args: string[]): Promise<void> {
+/**
+ * #1963 (same class as reindex-frontmatter): takes the ALREADY-CONNECTED
+ * engine from cli.ts's dispatch. Building a second engine here deadlocked on
+ * the PGLite data-dir lock (cli.ts's `connectEngine()` already holds it in
+ * this same process) — every `gbrain backfill <kind>` on PGLite timed out
+ * after 30s. Engine lifecycle belongs to cli.ts's connect + teardown.
+ */
+export async function runBackfillCommand(engine: BrainEngine, args: string[]): Promise<void> {
   const cli = parseArgs(args);
   if (cli.help) { printHelp(); return; }
 
@@ -144,20 +151,10 @@ export async function runBackfillCommand(args: string[]): Promise<void> {
     process.exit(2);
   }
 
-  const config = loadConfig();
-  if (!config) {
-    console.error('No brain configured. Run: gbrain init');
-    process.exit(2);
-  }
-
   // X5 admission control — clamp concurrency to direct-pool capacity.
   const { effective: concurrency, warning } = clampConcurrency(cli.concurrency);
   if (warning) console.warn(warning);
 
-  const { createEngine } = await import('../core/engine-factory.ts');
-  const engine = await createEngine(toEngineConfig(config));
-  await engine.connect(toEngineConfig(config));
-
   if (cli.fresh) {
     await clearBackfillCheckpoint(engine, reg.spec.name);
     console.log(`Cleared checkpoint for backfill.${reg.spec.name}`);
@@ -192,7 +189,6 @@ export async function runBackfillCommand(args: string[]): Promise<void> {
   if (result.cappedByMaxRows) console.log(`  ⚠️  Capped by --max-rows; more remain.`);
   if (result.cappedByErrors) console.log(`  ⚠️  Capped by --max-errors at ${result.errors}.`);
 
-  await engine.disconnect();
   if (result.cappedByErrors) process.exit(1);
 }
 

src/commands/reindex-frontmatter.ts

@@ -151,8 +151,17 @@ export async function runReindexFrontmatter(
   };
 }
 
-/** CLI entrypoint. Argv shape matches reindex-code for consistency. */
-export async function reindexFrontmatterCli(args: string[]): Promise<void> {
+/**
+ * CLI entrypoint. Argv shape matches reindex-code for consistency.
+ *
+ * #1963: takes the ALREADY-CONNECTED engine from cli.ts's dispatch instead of
+ * building its own. The old self-managed `createEngine()+connect()` here was a
+ * same-process double-connect: cli.ts's `connectEngine()` already held the
+ * PGLite data-dir lock, so the second `connect()` spun the full 30s lock
+ * timeout waiting on its own process and the command always exited 1 on
+ * PGLite. The engine lifecycle (connect + teardown) belongs to cli.ts.
+ */
+export async function reindexFrontmatterCli(engine: BrainEngine, args: string[]): Promise<void> {
   const opts: ReindexFrontmatterOpts = {};
   for (let i = 0; i < args.length; i++) {
     const a = args[i];
@@ -173,37 +182,15 @@ export async function reindexFrontmatterCli(args: string[]): Promise<void> {
     }
   }
 
-  const { createEngine } = await import('../core/engine-factory.ts');
-  const { loadConfig, toEngineConfig } = await import('../core/config.ts');
-  const cfg = loadConfig();
-  if (!cfg) {
-    console.error('No gbrain config; run `gbrain init` first.');
-    process.exit(1);
-  }
-  const engineConfig = toEngineConfig(cfg);
-  const engine = await createEngine(engineConfig);
-  // v0.37.7.0 #1225: createEngine() only constructs; callers MUST connect
-  // before any executeRaw call. Pre-fix, the first query in countAffected
-  // crashed with "PGLite not connected. Call connect() first." even on
-  // --dry-run. initSchema is idempotent on a current schema, costs ~1ms.
-  await engine.connect(engineConfig);
-  await engine.initSchema();
-
-  try {
-    const result = await runReindexFrontmatter(engine, opts);
-    if (opts.json) {
-      console.log(JSON.stringify(result, null, 2));
-    } else {
-      const noun = result.status === 'dry_run' ? 'would update' : 'updated';
-      console.error(
-        `\nReindex ${result.status}: examined=${result.examined} ${noun}=${result.updated} ` +
-        `fallback=${result.fallback} dur=${result.durationSec.toFixed(1)}s`,
-      );
-    }
-    if (result.status === 'cancelled') process.exit(1);
-  } finally {
-    if ('disconnect' in engine && typeof engine.disconnect === 'function') {
-      await engine.disconnect();
-    }
+  const result = await runReindexFrontmatter(engine, opts);
+  if (opts.json) {
+    console.log(JSON.stringify(result, null, 2));
+  } else {
+    const noun = result.status === 'dry_run' ? 'would update' : 'updated';
+    console.error(
+      `\nReindex ${result.status}: examined=${result.examined} ${noun}=${result.updated} ` +
+      `fallback=${result.fallback} dur=${result.durationSec.toFixed(1)}s`,
+    );
   }
+  if (result.status === 'cancelled') process.exit(1);
 }

test/reindex-frontmatter-pglite-spawn.serial.test.ts

@@ -0,0 +1,123 @@
+/**
+ * #1963 regression test: `gbrain reindex-frontmatter` (and `gbrain backfill
+ * <kind>`, same class) on a PGLite brain.
+ *
+ * Pre-fix, cli.ts's dispatch connected the primary engine (taking the PGLite
+ * data-dir lock), then `reindexFrontmatterCli` / `runBackfillCommand` built
+ * and connected a SECOND engine on the same data dir. `acquireLock` never
+ * reaps a live PID — and the named holder was this very process — so the
+ * command spun the full 30s lock timeout and exited 1 with "Timed out waiting
+ * for PGLite data-dir lock", 100% of the time on PGLite. The fix passes the
+ * already-connected engine through instead of double-connecting.
+ *
+ * Spawn-level on purpose: the bug lives in the CLI dispatch seam, which
+ * in-process unit tests of `runReindexFrontmatter` (see
+ * reindex-frontmatter-connect.test.ts) can never reach.
+ *
+ * Single-test design mirrors apply-migrations-pglite-spawn.serial.test.ts:
+ * each `bun run src/cli.ts` spawn pays a cold-start cost on CI, so one test
+ * walks the whole lifecycle. Serial because it spawns subprocesses + writes a
+ * tmpdir.
+ */
+import { describe, test, expect } from 'bun:test';
+import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs';
+import { join } from 'path';
+import { tmpdir } from 'os';
+
+const REPO = new URL('..', import.meta.url).pathname.replace(/\/$/, '');
+
+async function runCli(
+  args: string[],
+  env: Record<string, string>,
+  timeoutMs: number,
+): Promise<{ exitCode: number; stdout: string; stderr: string }> {
+  // Scrub inherited GBRAIN_* so a developer's shell config (embedding model,
+  // pace mode, …) can't change what the spawned CLI does.
+  const base = Object.fromEntries(
+    Object.entries(process.env).filter(([k]) => !k.startsWith('GBRAIN_')),
+  ) as Record<string, string>;
+  const proc = Bun.spawn(['bun', 'run', `${REPO}/src/cli.ts`, ...args], {
+    cwd: REPO,
+    env: { ...base, ...env },
+    stdout: 'pipe',
+    stderr: 'pipe',
+  });
+  const killer = setTimeout(() => {
+    try { proc.kill('SIGKILL'); } catch { /* already dead */ }
+  }, timeoutMs);
+  try {
+    const [stdout, stderr, exitCode] = await Promise.all([
+      new Response(proc.stdout).text(),
+      new Response(proc.stderr).text(),
+      proc.exited,
+    ]);
+    return { exitCode, stdout, stderr };
+  } finally {
+    clearTimeout(killer);
+  }
+}
+
+describe('reindex-frontmatter + backfill on PGLite (#1963 double-connect)', () => {
+  test('init → import → reindex-frontmatter --yes → backfill effective_date --dry-run (all exit 0, no lock timeout)', async () => {
+    const home = mkdtempSync(join(tmpdir(), 'gbrain-1963-'));
+    const notes = mkdtempSync(join(tmpdir(), 'gbrain-1963-notes-'));
+    try {
+      mkdirSync(join(home, '.gbrain'), { recursive: true });
+      writeFileSync(
+        join(home, '.gbrain', 'config.json'),
+        JSON.stringify({
+          engine: 'pglite',
+          database_path: join(home, '.gbrain', 'brain.pglite'),
+          embedding_dimensions: 1536,
+        }) + '\n',
+      );
+      for (let i = 1; i <= 3; i++) {
+        writeFileSync(
+          join(notes, `note${i}.md`),
+          `---\neffective_date: 2025-01-0${i}\n---\n# note ${i}\n\nbody ${i}\n`,
+        );
+      }
+      const env = { HOME: home, GBRAIN_HOME: home };
+
+      const init = await runCli(['init', '--migrate-only'], env, 120_000);
+      if (init.exitCode !== 0) {
+        console.error('--- init stdout ---\n' + init.stdout);
+        console.error('--- init stderr ---\n' + init.stderr);
+      }
+      expect(init.exitCode).toBe(0);
+
+      const imp = await runCli(['import', notes, '--no-embed'], env, 120_000);
+      if (imp.exitCode !== 0) {
+        console.error('--- import stdout ---\n' + imp.stdout);
+        console.error('--- import stderr ---\n' + imp.stderr);
+      }
+      expect(imp.exitCode).toBe(0);
+
+      // Pre-fix: exits 1 after the 30s PGLite lock timeout, naming ITSELF as
+      // the holder. Post-fix: reuses cli.ts's connected engine and succeeds.
+      const reindex = await runCli(['reindex-frontmatter', '--yes', '--json'], env, 120_000);
+      const reindexOut = reindex.stdout + reindex.stderr;
+      if (reindex.exitCode !== 0) {
+        console.error('--- reindex-frontmatter stdout ---\n' + reindex.stdout);
+        console.error('--- reindex-frontmatter stderr ---\n' + reindex.stderr);
+      }
+      expect(reindexOut).not.toMatch(/Timed out waiting for PGLite/);
+      expect(reindex.exitCode).toBe(0);
+      expect(reindex.stdout).toMatch(/"status":\s*"ok"/);
+
+      // `gbrain backfill <kind>` had the identical double-connect. dry-run
+      // still connects, so pre-fix it hit the same 30s timeout.
+      const backfill = await runCli(['backfill', 'effective_date', '--dry-run'], env, 120_000);
+      const backfillOut = backfill.stdout + backfill.stderr;
+      if (backfill.exitCode !== 0) {
+        console.error('--- backfill stdout ---\n' + backfill.stdout);
+        console.error('--- backfill stderr ---\n' + backfill.stderr);
+      }
+      expect(backfillOut).not.toMatch(/Timed out waiting for PGLite/);
+      expect(backfill.exitCode).toBe(0);
+    } finally {
+      try { rmSync(home, { recursive: true, force: true }); } catch { /* best effort */ }
+      try { rmSync(notes, { recursive: true, force: true }); } catch { /* best effort */ }
+    }
+  }, 480_000);
+});