From 3c61e25503d09038dc021f4635ca7ae3babf1432 Mon Sep 17 00:00:00 2001 From: Time Attakc <89218912+time-attack@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:10:57 -0700 Subject: [PATCH] fix(reindex-frontmatter): reuse the connected engine instead of self-deadlocking on the PGLite lock (#1963) (#3558) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review: survived two independent refuters — the only PR of 32 reviewed this way to do so. The bug: `gbrain reindex-frontmatter` and `gbrain backfill ` were 100% dead on PGLite. cli.ts takes the data-dir lock, the command modules built a second engine on the same dir, and acquireLock never reaps a live PID — 30s timeout, exit 1, with the error naming the waiting process itself as the holder. Reproduced on the parent commit at 33.2s; passes in 5.0s with the fix. Root-cause fix at the dispatch layer, not a softening of the lock, and the sibling census confirmed these were the only two affected callers. Postgres path verified before merge (it was the review's one open gap, since the bug is PGLite-only and all verification had gone there while the change itself is connection-teardown ownership). Against real Postgres 16 + pgvector: reindex-frontmatter and all three registered backfills exit 0 with zero residual connections, zero advisory locks, and zero cycle-lock rows — byte-identical output and identical teardown to master on the same database, confirming the change is behavior-neutral there. Merged tree re-verified after rebase: typecheck clean, pglite-lock + reindex-frontmatter 16 pass, llms bundle fresh, 23/23 CI green. --- CHANGELOG.md | 15 +++ VERSION | 2 +- docs/architecture/KEY_FILES.md | 4 +- package.json | 2 +- src/cli.ts | 16 ++- src/commands/backfill.ts | 22 ++-- src/commands/reindex-frontmatter.ts | 55 +++----- ...ex-frontmatter-pglite-spawn.serial.test.ts | 123 ++++++++++++++++++ 8 files changed, 184 insertions(+), 55 deletions(-) create mode 100644 test/reindex-frontmatter-pglite-spawn.serial.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 35338925f..a10053137 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ All notable changes to GBrain will be documented in this file. +## [0.42.68.1] - 2026-07-30 + +**If you run `gbrain reindex-frontmatter` or `gbrain backfill` on the default embedded database, they now work. Until this release both failed every time, after waiting 30 seconds.** + +The embedded database allows one process at a time, and holds a lock to enforce it. These two commands opened a second connection to the same database from inside the process that already held that lock, then waited for a lock that could never be released — because the thing holding it was the waiting process itself. The wait ran its full 30 seconds and the command exited with an error naming a blocking process that was, in fact, itself. Both commands now reuse the connection that is already open. + +Nothing changes for brains on Postgres, where a second connection was always allowed. + +## To take advantage of v0.42.68.1 + +Nothing to undo — the commands failed without writing anything. Just run whichever you needed: +```bash +gbrain reindex-frontmatter +``` + ## [0.42.67.0] - 2026-07-28 **If you develop GBrain on Windows, the test and check commands now actually run. Until this release they were quietly doing almost nothing.** diff --git a/VERSION b/VERSION index a706b0945..b0b77a1b5 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.42.67.0 +0.42.68.1 \ No newline at end of file diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index 37d5de938..d19436479 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -32,7 +32,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/commands/doctor.ts` extension — `graph_signals_coverage` check wired into both `runDoctor` (local) and `doctorReportRemote` (HTTP/JSON thin-client path). Reads `search.graph_signals` config first, falls back to mode default; silent `ok` when disabled. Computes inbound link coverage on the page set; warns at <10% with `gbrain extract all` fix hint; `ok` at ≥30% ("fire on most queries") and 10-29% ("fire occasionally"), each with the percentage embedded. Pinned by cases in `test/doctor.test.ts`. - `src/core/engine-factory.ts` — Engine factory with dynamic imports (`'pglite'` | `'postgres'`). - `src/core/pglite-engine.ts` — PGLite (embedded Postgres 17.5 via WASM) implementation, all BrainEngine methods. `listLinkSources({sourceId?, sourceIds?})` returns distinct `link_source` provenances + counts (`ORDER BY count DESC, link_source ASC NULLS LAST`; scalar + federated scoped; parity with postgres-engine.ts) powering `gbrain link-sources`. `addLinksBatch`/`addTimelineEntriesBatch`/`addTakesBatch` pass the whole batch as one JSONB document via `jsonb_to_recordset(($1::jsonb)->'rows')` (bound through `executeRawJsonb` with a `{ rows }` wrapper; rows built by the shared `src/core/batch-rows.ts` helpers, NUL-stripped), and are `batchRetry`-wrapped. `connect()` wraps `PGlite.create()` in a try/catch that emits an actionable error (macOS 26.3 WASM bug #223, points at `gbrain doctor`); the lock is released on failure so the next process can retry cleanly. `searchKeyword`/`searchKeywordChunks` multiply `ts_rank` by the source-factor CASE at chunk grain; `searchVector` is a two-stage CTE — inner CTE keeps `ORDER BY cc.embedding <=> vec` so HNSW stays usable, outer SELECT re-ranks by `raw_score * source_factor`, inner LIMIT scales with offset to preserve pagination. `searchTakes`/`searchTakesVector` take full `SearchOpts` and apply the standard source-scope predicates (federated `sourceIds[]` wins over scalar `sourceId`, via the joined page's `source_id`) alongside the holder allow-list — parity SQL in postgres-engine.ts; pinned by `test/e2e/think-source-isolation-pglite.test.ts`. `initSchema()` calls `applyForwardReferenceBootstrap()` BEFORE replaying SCHEMA_SQL — probes for forward-referenced state the embedded blob needs (`pages.source_id`, `links.link_source`, `links.origin_page_id`, `content_chunks.symbol_name`, `content_chunks.language`, `sources` FK target, plus `files.source_id`, `files.page_id`, `oauth_clients.source_id`, `oauth_clients.federated_read`, `sources.archived`, `sources.archived_at`, `sources.archive_expires_at`, `timeline_entries.event_page_id` — column-only, migration v121 stays the source of truth for its FK + indexes) and adds only what's missing; threads the DDL connection from `initSchema` so probes run inside the advisory-lock scope; no-op on fresh installs and modern brains (closes the upgrade-wedge bug class #239/#243/#266/#357/#366/#374/#375/#378/#395/#396/#1018/#974/#820). `getBrainScore` returns 100/100 with full breakdown (35/25/15/15/10) when `pageCount === 0` (vacuous truth — empty brain has no coverage problem); Pinned by `test/brain-score-breakdown.test.ts` empty-brain assertion + `test/doctor-report-remote.serial.test.ts`. `disconnect()` uses snapshot+early-null (snapshot `_db`/`_lock`, null instance fields BEFORE any `await` so a concurrent `connect()` can't see a partial mid-close state) wrapped in try/finally guaranteeing lock-release even if `db.close()` throws; KEEPS close-then-release order (release-then-close was rejected: it would widen the window where a sibling process connects to a still-closing brain); Pinned by `test/pglite-engine-disconnect.serial.test.ts`. `PGlite.create()` runs inside `preservingProcessExitCode` (#2084): PGLite's Emscripten runtime writes its own status into `process.exitCode` (99 at create; in-memory brains run initdb whose status lands on a later tick; the exit status at close — and assigning `undefined` cannot clear a prior value), which would otherwise silently clobber an errored op's exit 1 back to 0. The wrapper keeps the global tidy for external readers; `db.close()` stays unwrapped (its 0-write is baseline behavior test runners depend on). The CLI's exit verdict is immune either way — it lives in the gbrain-owned channel in `cli-force-exit.ts` and never reads `process.exitCode` back. Exports `classifyPgliteInitError(message): 'bunfs' | 'macos-26-3' | 'unknown'` + `buildPgliteInitErrorMessage(verdict, original)` routing the catch-block hint by failure shape (`bunfs` matches literal `$$bunfs` OR `ENOENT[\s\S]*pglite\.data` co-occurrence, surfaces a paste-ready `bun upgrade` + Node fallback; `macos-26-3` keeps the #223 link; `unknown` falls through); Pinned by `test/pglite-init-classifier.test.ts`. Implements `deletePages(slugs, {sourceId})` + `resolveSlugsByPaths(paths, {sourceId})` via `slug = ANY($1::text[])` array-param binding, caller-chunking primitive throwing when input exceeds `DELETE_BATCH_SIZE`, `deletePages` returns `RETURNING slug` rows so callers filter `pagesAffected` to confirmed deletes. Implements the embedding-signature stale-detection quartet — `sumStaleChunkChars({sourceId?, signature?})`, `setPageEmbeddingSignature(slug, {sourceId?, signature})`, `invalidateStaleSignatureEmbeddings({signature, sourceId?})`, widened `countStaleChunks({sourceId?, signature?})` (the `signature` opt widens via `JOIN pages p ... WHERE cc.embedding IS NULL OR (p.embedding_signature IS NOT NULL AND p.embedding_signature <> $signature)`, NULL grandfathered); parity SQL with postgres-engine.ts. PGLite-specific DDL (pgvector, pg_trgm, triggers). Engine-path helper dependencies (`retry`, ontology, recency decay) avoid dynamic `import()`; the only lazy dynamic imports are `ai/gateway.ts` in `initSchema` and `_upsertChunksOnce`, line-marked because their local catches preserve compiled-default and stored-config fallbacks that eager module evaluation would bypass. -- `src/core/pglite-lock.ts` — advisory data-dir lock so two processes never open the same embedded PGLite (single-connection WASM Postgres) at once. Atomic `mkdir` of `.gbrain-lock/` + a `lock` file carrying `{pid, acquired_at, refreshed_at, command, subcommand}`. A held lock HEARTBEATS its `refreshed_at` every 30s (`.unref()`ed timer; informational). A waiting acquirer reaps a holder ONLY when its PID is dead — a LIVE holder is NEVER stolen, regardless of how stale its heartbeat is (#2348). A live `gbrain serve` holder is identified from the parsed `subcommand` and reported immediately with separate CLI-retry and MCP-tool choices; other live holders keep the bounded wait. The heartbeat runs on the JS event loop, which is blocked during long synchronous WASM imports/CHECKPOINTs, so a working `dream`/`embed` holder can look stale while alive; the old steal-on-stale-heartbeat grace let a second OS process open the same data dir and corrupt the catalog + pgvector extension (58P01 / `internal_load_library` / `type "vector" does not exist`), recoverable only by wipe+restore. A wedged-but-alive or PID-reused holder is never stolen: serve-tagged holders report immediately, while other holders time out with a message naming the PID. Each holder carries an ownership token (`:`); the heartbeat and `releaseLock` verify the on-disk lock is STILL theirs before touching it. In-memory engines take no lock. Pinned by `test/pglite-lock.test.ts`. A corrupted store surfaces a `reinit-pglite` recovery hint via `classifyPgliteInitError`'s `corrupt` verdict in `pglite-engine.ts`. +- `src/core/pglite-lock.ts` — advisory data-dir lock so two processes never open the same embedded PGLite (single-connection WASM Postgres) at once. Atomic `mkdir` of `.gbrain-lock/` + a `lock` file carrying `{pid, acquired_at, refreshed_at, command, subcommand}`. A held lock HEARTBEATS its `refreshed_at` every 30s (`.unref()`ed timer; informational). A waiting acquirer reaps a holder ONLY when its PID is dead — a LIVE holder is NEVER stolen, regardless of how stale its heartbeat is (#2348). A live `gbrain serve` holder is identified from the parsed `subcommand` and reported immediately with separate CLI-retry and MCP-tool choices; other live holders keep the bounded wait. The heartbeat runs on the JS event loop, which is blocked during long synchronous WASM imports/CHECKPOINTs, so a working `dream`/`embed` holder can look stale while alive; the old steal-on-stale-heartbeat grace let a second OS process open the same data dir and corrupt the catalog + pgvector extension (58P01 / `internal_load_library` / `type "vector" does not exist`), recoverable only by wipe+restore. A wedged-but-alive or PID-reused holder is never stolen: serve-tagged holders report immediately, while other holders time out with a message naming the PID. Each holder carries an ownership token (`:`); the heartbeat and `releaseLock` verify the on-disk lock is STILL theirs before touching it. In-memory engines take no lock. There is deliberately NO same-process reentrancy or same-PID special case: a second `acquireLock` from the process that already holds the lock waits out the timeout like any other live holder (#1963 was this shape — a command double-connecting a second engine on the same data dir; the fix is to reuse the connected engine at the dispatch layer, never to soften the lock). Pinned by `test/pglite-lock.test.ts`. A corrupted store surfaces a `reinit-pglite` recovery hint via `classifyPgliteInitError`'s `corrupt` verdict in `pglite-engine.ts`. - `src/core/postgres-engine.ts` — Postgres + pgvector implementation (Supabase / self-hosted). `addLinksBatch`/`addTimelineEntriesBatch`/`addTakesBatch` pass the batch as one JSONB document — `INSERT ... SELECT FROM jsonb_to_recordset(($1::jsonb)->'rows') AS v(...) JOIN pages ...` bound through `executeRawJsonb({ rows })` — which encodes arbitrary free text safely (the old `unnest(${arr}::text[])` array-literal path crashed Postgres with "malformed array literal" on calendar/Zoom context, gbrain#1861) and sidesteps the 65535-parameter cap; takes declares native recordset column types (`page_id int, weight real, active boolean, …`) so no per-element casts; all three are `batchRetry`-wrapped. `searchKeyword`/`searchVector` scope `statement_timeout` via `sql.begin` + `SET LOCAL` so the GUC dies with the transaction instead of leaking across the pooled postgres.js connection. `getEmbeddingsByChunkIds` uses `tryParseEmbedding` so one corrupt row skips+warns instead of killing the query. `searchKeyword`/`searchKeywordChunks`/`searchVector` apply source-aware ranking by inlining the source-factor CASE and `NOT (col LIKE …)` hard-exclude from `src/core/search/sql-ranking.ts`; `searchVector` is a two-stage CTE (HNSW-safe inner ORDER BY, source-boost re-rank in outer SELECT) carrying `p.source_id` inner→outer. `_savedConfig` retains the connect config; `reconnect()` tears down + recreates the pool from saved config (called by supervisor watchdog after 3 consecutive health-check failures, and by `batchRetry` on a retryable connection error). Concurrent callers share one in-flight `_reconnectPromise` (they await the single reconnect rather than racing a half-rebuilt pool); ownership re-samples through the atomic `db.connect()` token on the connect leg. `reconnect(ctx?)` accepts the triggering error and records a pool-recovery audit event (`reap_detected`/`reconnect_other`/`reconnect_succeeded`/`reconnect_failed`) for the `pool_reap_health` doctor check. `executeRaw` is a single-statement passthrough — no per-call retry (unsound for non-idempotent statements; recovery is supervisor-driven). `connect()` applies `resolveSessionTimeouts()` from `db.ts` as connection-time startup parameters (`statement_timeout`, `idle_in_transaction_session_timeout`) so orphan pgbouncer backends can't hold locks for hours. `countStaleChunks()`+`listStaleChunks()` server-side-filter on `embedding IS NULL` for `embed --stale` (eliminates ~76 MB/call client-side pull); `upsertChunks()` resets both `embedding` AND `embedded_at` to NULL when chunk_text changes without a new embedding. `initSchema()` calls `applyForwardReferenceBootstrap()` BEFORE replaying SCHEMA_SQL on the same probe set as PGLite (extended for column-only forward-reference cases: `files.source_id`, `files.page_id`, `oauth_clients.source_id`, `oauth_clients.federated_read`, `sources.archived`/`archived_at`/`archive_expires_at`, `timeline_entries.event_page_id`); the entire probe path runs on the DDL connection threaded from `initSchema` (closing a concurrent-bootstrap race for Supabase pooler users); closes #1018/#974/#820. `disconnect()` is idempotent — `_connectionStyle` tracks whether the engine owns its pool (worker engines) or shares the module-level singleton; second call on an instance-pool engine is a no-op rather than clobbering the singleton; and a module-style engine only calls `db.disconnect()` when it owns the singleton (`_ownsModuleSingleton`, set from the `db.connect()` creation token), so a borrower probe engine's teardown leaves the cycle owner's connection intact. Pinned by `test/e2e/postgres-engine-disconnect-idempotency.test.ts` + `test/postgres-engine-singleton-ownership.test.ts`. `getBrainScore` empty-brain parity with PGLite — 100/100 with breakdown 35/25/15/15/10 when `pageCount === 0` (both engines must agree to keep `doctor-report-remote.serial.test.ts` deterministic). Implements `deletePages(slugs, {sourceId}): Promise` via `DELETE FROM pages WHERE slug = ANY($1::text[]) AND source_id = $2 RETURNING slug` (single round-trip; caller chunks); `resolveSlugsByPaths` does `SELECT slug, source_path FROM pages WHERE source_path = ANY($1::text[]) AND source_id = $2`; FK cascades through `content_chunks`/`links`/`tags`/`raw_data`/`timeline_entries`/`page_versions`, `files.page_id`+`links.origin_page_id` go SET NULL; throws when input exceeds `DELETE_BATCH_SIZE` (from `src/core/engine-constants.ts`); both short-circuit on empty input. Implements the embedding-signature stale-detection quartet (`sumStaleChunkChars`, `setPageEmbeddingSignature`, `invalidateStaleSignatureEmbeddings`, widened `countStaleChunks`, all accept optional `signature` extending "stale" to model/dims-swap drift via the `pages.embedding_signature` JOIN, NULL grandfathered; the `embedding IS NULL` server-side filter is preserved as the no-signature fast path); Pinned by `test/e2e/engine-parity.test.ts`. Retry classifiers, ontology/recency helpers, and disconnect/pool-recovery audit writers bind statically; only the two `ai/gateway.ts` fallback lookups stay lazy and line-marked, in parity with PGLite. - `src/core/cjk.ts` — Single source of truth for CJK detection. Exports `CJK_RANGES_REGEX`, `CJK_SLUG_CHARS` (character-class fragment for embedding inside other regexes), `CJK_SENTENCE_DELIMITERS` (`。！？`), `CJK_CLAUSE_DELIMITERS` (`；：，、`), `CJK_DENSITY_THRESHOLD = 0.30`, `hasCJK(s)`, `countCJKAwareWords(s)` (30% density threshold — English docs with one Japanese term stay whitespace-tokenized; Chinese-dominant docs get char-counted), and `escapeLikePattern(s)` (escapes `%`, `_`, `\\` for `ILIKE ... ESCAPE '\\'`). BMP-only ranges (Han / Hiragana / Katakana / Hangul Syllables). Consumers: `expansion.ts`, `sync.ts:slugifySegment`, `operations.ts:validatePageSlug + validateFilename`, `chunkers/recursive.ts:countWords + DELIMITERS`, `pglite-engine.ts:searchKeyword + searchKeywordChunks`. - `src/core/audit-slug-fallback.ts` — Weekly ISO-week-rotated audit JSONL at `~/.gbrain/audit/slug-fallback-YYYY-Www.jsonl`. `logSlugFallback(slug, sourcePath)` fires when `importFromFile` falls back to a frontmatter slug because `slugifyPath` returned empty (emoji / Thai / Arabic / non-CJK exotic-script filenames). `readRecentSlugFallbacks(days)` reads the last N days for `gbrain doctor`'s `slug_fallback_audit` check. Honors `GBRAIN_AUDIT_DIR` via the shared `resolveAuditDir()`. Separate surface from `sync-failures.jsonl` — that file carries bookmark-gating semantics that info events shouldn't trigger. @@ -205,7 +205,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/commands/import.ts` — `gbrain import [--source-id ]`: page import with the path-set checkpoint. `--source-id ` routes pages to the named source (resolved via `resolveSourceWithTier()` at the boundary; consistent across `import`, `extract`, `graph-query`, `sources current`). Pinned by `test/import-source-id.test.ts`. - `src/commands/graph-query.ts` — `gbrain graph-query [--type T] [--depth N] [--direction in|out|both] [--include-foreign]`: typed-edge relationship traversal (renders indented tree). Foreign-edge footer always present (`X foreign edges (use --include-foreign to traverse)`) so cross-source edges never disappear silently; `--include-foreign` widens the SQL filter to walk them. Pinned by `test/graph-query.test.ts`. - `src/commands/sources.ts` — `gbrain sources {list,add,remove,archive,restore,archived,purge,current,status,audit}`. `current [--json]` calls `resolveSourceWithTier()` and prints `source_id`, `tier` (`flag | env | dotfile | local_path | brain_default | seed_default`), and optional `detail` (decision table in `skills/conventions/brain-routing.md`). `status [--json]` — read-only per-source dashboard (last sync, staleness, page count, embedding coverage, unacked failures); thin wrapper around `buildSyncStatusReport` + `printSyncStatusReport` from `src/commands/sync.ts`; `--json` emits stable `{schema_version: 1, sources, ...}` on stdout; filters input to `local_path IS NOT NULL AND archived IS NOT TRUE`. `audit [--json]` — read-only dry-run disk scan for size distribution + would-blocks + junk-pattern hits WITHOUT touching the DB; walks `sources.local_path`, reads each markdown file, runs `assessContent()` from `src/core/content-sanity.ts`, aggregates by verdict (`ok | warn_oversize | hard_block_junk_pattern`). The live `runStatus` health table gains a `BACKFILL` column between `EMBED` and `FAILS` (`active(N)` beats `queued(N)` beats `idle`, from `SourceMetrics.backfill_active` / `backfill_queued` in `src/core/source-health.ts`) so operators see deferred `embed-backfill` minion work after `sync --all` exits 0; `jobCountsBySource` in `source-health.ts` widens its `minion_jobs` SQL with two `COUNT(*) FILTER (WHERE name = 'embed-backfill' AND ...)` aggregates (best-effort, all-0 on pre-minions brains). Pinned by `test/content-sanity.test.ts`, `test/import-file-content-sanity.test.ts`, `test/source-health.test.ts`. -- `src/commands/reindex-frontmatter.ts` — `gbrain reindex-frontmatter`. Query path wrapped in the standard `withEngine(...)` lifecycle so `engine.connect()` runs before the first SQL call. Pinned by `test/reindex-frontmatter-connect.test.ts`. +- `src/commands/reindex-frontmatter.ts` — `gbrain reindex-frontmatter`. `reindexFrontmatterCli(engine, args)` takes the ALREADY-CONNECTED engine from cli.ts's dispatch (#1963); it must never build/connect its own engine — a second connect on the same PGLite data dir self-deadlocks on the data-dir lock (this process already holds it) and timed out 100% of the time on PGLite. Same rule applies to `runBackfillCommand(engine, args)` in `src/commands/backfill.ts` and any future command dispatched from cli.ts's engine-connected switch. Pinned by `test/reindex-frontmatter-connect.test.ts` (library path) and `test/reindex-frontmatter-pglite-spawn.serial.test.ts` (CLI dispatch seam, both commands). - `src/core/source-config-sql.ts` + `src/core/sources-load.ts` — canonical recovery for historical non-object `sources.config` values. The application reader unwraps nested JSON strings and merges recoverable array fragments left-to-right; the shared SQL expression mirrors that policy atomically for both engines, source config updates, archive/restore, and the paste-ready `source_config_shape` doctor repair. `localFederatedSourceIds` reads config through the same parser so stdio/CLI federation cannot silently disagree with `sources list`. Invalid fragments degrade to `{}` rather than throwing. Pinned by `test/sources-load.test.ts`, `test/list-all-sources.test.ts`, `test/local-federated-search-scope.test.ts`, `test/destructive-guard.test.ts`, and `test/doctor-source-config-shape.test.ts`. - `src/core/source-resolver.ts` — 6-tier source resolution. `resolveSourceWithTier(engine, explicit, cwd)` returns `{ source_id, tier: SourceTier, detail? }` alongside `resolveSourceId()` (unchanged). `SOURCE_TIER_NAMES = ['flag', 'env', 'dotfile', 'local_path', 'sole_non_default', 'brain_default', 'seed_default']` (7 entries; order matches priority). Tier `sole_non_default` slots between `local_path` and `brain_default`: when NO `sources.default` config is set AND exactly one registered source has `local_path` AND isn't `'default'`, auto-route to it; archived sources excluded (try/catch for pre-v34 brains); private `pickSoleNonDefaultSource(engine)` shared by both resolver entry points so they cannot drift. Exported `formatSoleNonDefaultNudge(sourceId): string | null` builds the user-facing stderr nudge (null when `GBRAIN_NO_SOLE_NON_DEFAULT_NUDGE=1`). `src/commands/sync.ts:1497-1519` calls `resolveSourceWithTier` unconditionally so the tier fires; `src/commands/import.ts:96-128` mirrors with the tier-gated nudge. Consumed by `gbrain sources current`, `import --source-id`, `extract --source-id`, and the `source_routing_health` doctor check. Pinned by `test/source-resolver-with-tier.test.ts` (`withEnv()` per test-isolation lint), `test/source-resolver-sole-non-default.test.ts` (14 cases), `test/sync-sole-non-default-routing.test.ts` (3 PGLite cases driving real `runSync`). - `src/core/sync.ts` extension — `isSyncable` factored through private `classifySync(path, opts): SyncableReason | null`; exported companion `unsyncableReason(path, opts)` returns the same tagged reason or null when syncable. `SYNC_SKIP_FILES` is a named export (the four canonical metafile basenames `schema.md`, `index.md`, `log.md`, `README.md`). `SyncableReason` union: `'metafile' | 'strategy' | 'pruned-dir' | 'include-glob-miss' | 'exclude-glob-hit'`. The `commands/sync.ts` cleanup loop guards on `unsyncableReason(path)` being `'metafile'` OR `'pruned-dir'` (#2404) so previously-indexed metafile pages AND deliberately put-created pages under pruned dirs survive every re-sync. Does NOT cover `manifest.deleted` (the upstream filter already strips metafiles). Pinned by `test/sync-isSyncable-shape.test.ts` (15 cases, duality contract) + `test/sync-metafile-skip.serial.test.ts` (3 PGLite cases incl. the renamed `.md → .txt` negative). diff --git a/package.json b/package.json index 0fc26109f..8f167e3ee 100644 --- a/package.json +++ b/package.json @@ -147,7 +147,7 @@ "bun": ">=1.3.10" }, "license": "MIT", - "version": "0.42.67.0", + "version": "0.42.68.1", "overrides": { "@hono/node-server": "^2.0.5", "fast-uri": "^3.1.4", diff --git a/src/cli.ts b/src/cli.ts index 8965a2d70..a044e171a 100755 --- a/src/cli.ts +++ b/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 ?" diff --git a/src/commands/backfill.ts b/src/commands/backfill.ts index 1d07d4623..b13b73e68 100644 --- a/src/commands/backfill.ts +++ b/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 { +/** + * #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 ` on PGLite timed out + * after 30s. Engine lifecycle belongs to cli.ts's connect + teardown. + */ +export async function runBackfillCommand(engine: BrainEngine, args: string[]): Promise { const cli = parseArgs(args); if (cli.help) { printHelp(); return; } @@ -144,20 +151,10 @@ export async function runBackfillCommand(args: string[]): Promise { 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 { 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); } diff --git a/src/commands/reindex-frontmatter.ts b/src/commands/reindex-frontmatter.ts index 0571731ae..7d83a4995 100644 --- a/src/commands/reindex-frontmatter.ts +++ b/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 { +/** + * 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 { 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 { } } - 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); } diff --git a/test/reindex-frontmatter-pglite-spawn.serial.test.ts b/test/reindex-frontmatter-pglite-spawn.serial.test.ts new file mode 100644 index 000000000..5458c757e --- /dev/null +++ b/test/reindex-frontmatter-pglite-spawn.serial.test.ts @@ -0,0 +1,123 @@ +/** + * #1963 regression test: `gbrain reindex-frontmatter` (and `gbrain backfill + * `, 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, + 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; + 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 ` 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); +});