diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index 8e0c25134..bc3aaec0d 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -30,7 +30,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). -- `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}`. 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). 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 now makes the acquire TIME OUT with a message naming the PID (the user removes the lock explicitly) rather than risk corruption. 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. 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`. - `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. diff --git a/src/core/pglite-lock.ts b/src/core/pglite-lock.ts index 9b00bfe7b..74c91aa45 100644 --- a/src/core/pglite-lock.ts +++ b/src/core/pglite-lock.ts @@ -16,6 +16,7 @@ import { mkdirSync, existsSync, readFileSync, writeFileSync, rmSync, statSync } from 'fs'; import { join } from 'path'; +import { parseGlobalFlags } from './cli-options.ts'; const LOCK_DIR_NAME = '.gbrain-lock'; const LOCK_FILE = 'lock'; @@ -24,6 +25,21 @@ const LOCK_FILE = 'lock'; // LIVE holder (embed jobs run for many minutes) is never mistaken for stale. const HEARTBEAT_INTERVAL_MS = 30_000; +class LiveServeLockError extends Error {} + +function isServeCommand(lockData: { subcommand?: unknown; command?: unknown }): boolean { + // New lock files store the command after the same global-flag parsing used + // by cli.ts. This survives paths with spaces and forms such as + // `gbrain --quiet serve` without confusing `gbrain search serve`. + if (typeof lockData.subcommand === 'string') return lockData.subcommand === 'serve'; + + const command = lockData.command; + if (typeof command !== 'string') return false; + const parts = command.trim().split(/\s+/); + // Backward compatibility for locks created before `subcommand` was stored. + return parts[0] === 'serve' || parts[1] === 'serve'; +} + // #2348: there is NO steal-on-stale-heartbeat anymore. A holder whose PID is // alive is NEVER reaped, regardless of how long its heartbeat has been stale. // PGLite/WASM is strictly single-writer; the heartbeat runs on the JS event @@ -32,9 +48,9 @@ const HEARTBEAT_INTERVAL_MS = 30_000; // Reaping it (the old #2058 grace window) let a second OS process open the same // data dir and corrupt the catalog + pgvector extension state (58P01 / // internal_load_library / `type "vector" does not exist`), recoverable only by -// wipe+restore. Only a DEAD PID is reaped now; a wedged-but-alive or PID-reused -// holder makes the acquire time out with a message naming the PID (the user -// removes the lock explicitly) rather than risk corruption. +// wipe+restore. Only a DEAD PID is reaped now. A live serve-tagged holder gets +// the immediate process-conflict explanation below; other wedged-but-alive or +// PID-reused holders time out. Neither path steals the lock. export interface LockHandle { lockDir: string; @@ -145,13 +161,25 @@ export async function acquireLock(dataDir: string | undefined, opts?: { timeoutM // Holder process is gone — reap and try to acquire. try { rmSync(lockDir, { recursive: true, force: true }); } catch { /* race condition, try again */ } } else { - // Live holder — wait and retry. If it is genuinely wedged (or its PID - // was reused by an unrelated process), the acquire times out below - // with a message naming the PID; we never force-steal a live holder. + if (isServeCommand(lockData)) { + throw new LiveServeLockError( + `GBrain's local database is already open through \`gbrain serve\` (MCP, PID ${lockPid}). ` + + `This brain uses PGLite, so a separate CLI process cannot open it at the same time. ` + + `Stop \`gbrain serve\`, then retry this CLI command. ` + + `Or keep it running and use its MCP tools instead. ` + + `A process with the recorded PID is still running, so GBrain will not remove ${lockDir} automatically.`, + ); + } + // Other live holders may be short-lived, so wait and retry. If one is + // genuinely wedged (or its PID was reused), the acquire times out; + // we never force-steal a live holder. await new Promise(r => setTimeout(r, 1000)); continue; } - } catch { + } catch (err) { + // A live MCP server is not a stale or corrupt lock. Surface the useful + // explanation without touching the lock it still owns. + if (err instanceof LiveServeLockError) throw err; // Corrupt lock file — remove it try { rmSync(lockDir, { recursive: true, force: true }); } catch { /* race condition */ } } @@ -169,6 +197,7 @@ export async function acquireLock(dataDir: string | undefined, opts?: { timeoutM acquired_at: now, refreshed_at: now, command: process.argv.slice(1).join(' '), + subcommand: parseGlobalFlags(process.argv.slice(2)).rest[0] ?? null, }), { mode: 0o644 }); const ownerToken = tokenOf({ pid: process.pid, acquired_at: now }); diff --git a/test/pglite-lock.test.ts b/test/pglite-lock.test.ts index 5d2f472b8..2850a0a94 100644 --- a/test/pglite-lock.test.ts +++ b/test/pglite-lock.test.ts @@ -109,7 +109,13 @@ describe('pglite-lock #2058 heartbeat + steal-grace', () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true, force: true }); }); - function writeHolder(fields: { pid: number; acquiredAgoMs: number; refreshedAgoMs: number }) { + function writeHolder(fields: { + pid: number; + acquiredAgoMs: number; + refreshedAgoMs: number; + command?: string; + subcommand?: string; + }) { const lockDir = join(TEST_DIR, '.gbrain-lock'); mkdirSync(lockDir, { recursive: true }); const now = Date.now(); @@ -117,10 +123,70 @@ describe('pglite-lock #2058 heartbeat + steal-grace', () => { pid: fields.pid, acquired_at: now - fields.acquiredAgoMs, refreshed_at: now - fields.refreshedAgoMs, - command: 'test holder', + command: fields.command ?? 'test holder', + ...(fields.subcommand === undefined ? {} : { subcommand: fields.subcommand }), })); } + test('a live gbrain serve owner with global flags fails fast with a clear explanation', async () => { + writeHolder({ + pid: process.pid, + acquiredAgoMs: 60_000, + refreshedAgoMs: 0, + command: '/path with spaces/gbrain/src/cli.ts --quiet serve', + subcommand: 'serve', + }); + + const startedAt = Date.now(); + await expect(acquireLock(TEST_DIR, { timeoutMs: 5_000 })).rejects.toThrow( + /already open through `gbrain serve`.*Stop `gbrain serve`, then retry this CLI command.*use its MCP tools instead.*will not remove/s, + ); + + expect(Date.now() - startedAt).toBeLessThan(1_000); + expect(existsSync(join(TEST_DIR, '.gbrain-lock'))).toBe(true); + }); + + test('legacy serve lock metadata is still recognized', async () => { + writeHolder({ + pid: process.pid, + acquiredAgoMs: 60_000, + refreshedAgoMs: 0, + command: '/path/to/gbrain/src/cli.ts serve', + }); + + await expect(acquireLock(TEST_DIR, { timeoutMs: 5_000 })).rejects.toThrow( + /already open through `gbrain serve`/, + ); + expect(existsSync(join(TEST_DIR, '.gbrain-lock'))).toBe(true); + }); + + test('a search for the word serve is not mistaken for the MCP server', async () => { + writeHolder({ + pid: process.pid, + acquiredAgoMs: 60_000, + refreshedAgoMs: 0, + command: '/compiled/gbrain search serve', + subcommand: 'search', + }); + + await expect(acquireLock(TEST_DIR, { timeoutMs: 100 })).rejects.toThrow(/Timed out/); + expect(existsSync(join(TEST_DIR, '.gbrain-lock'))).toBe(true); + }); + + test('a dead gbrain serve owner is still cleaned up automatically', async () => { + writeHolder({ + pid: 999999999, + acquiredAgoMs: 60_000, + refreshedAgoMs: 0, + command: '/path/to/gbrain/src/cli.ts serve', + subcommand: 'serve', + }); + + const lock = await acquireLock(TEST_DIR, { timeoutMs: 2_000 }); + expect(lock.acquired).toBe(true); + await releaseLock(lock); + }); + test('[REGRESSION] a LIVE holder with a fresh heartbeat is NOT stolen even when the lock is old', async () => { // The WAL-corruption bug: a >5min embed used to get its lock force-removed. // Now an alive holder that heartbeated recently is left alone regardless of