diff --git a/CHANGELOG.md b/CHANGELOG.md index e3f1ecaa0..3a1f1ba82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,60 @@ All notable changes to GBrain will be documented in this file. +## [0.42.37.0] - 2026-06-08 + +**Cross-source reads now honor the caller's grant everywhere, a single bad frontmatter value no longer wedges a whole `lint`/`sync` run, and a handful of long-standing papercuts are gone.** A triage of the open issue backlog pulled the highest-impact bugs into one wave. + +The headline is a source-isolation hardening pass. Every read that can be scoped to a source now resolves through one shared, fail-closed trust+grant check, so a remote client only ever sees the sources it was granted — whether it asks for one source, all sources, or reads a page by exact slug. Reads route the same way across query, the code-intel traversals, image search, and `get_page`. Legacy bearer tokens now carry the source grant an operator already stored on them, instead of being pinned to `default`. + +On ingestion, a non-string frontmatter value (a bare number or date in `title:`, `slug:`, or `type:`) used to throw partway through and abort the entire run — so one malformed file could stop a whole brain from linting or syncing. Now those values are coerced to a usable string (a bare date `2024-06-01` becomes a real slug, not a crash), and `gbrain lint` flags the un-quoted field by name so you can clean it up. + +Plus: `gbrain embed --catch-up` runs to completion instead of stopping after the first batch (and tells you when chunks genuinely can't be embedded); the frontmatter pre-commit hook actually matches `.md`/`.mdx` files now instead of silently doing nothing; the skill catalog shows the real description for skills that write it as a YAML block scalar; and `getConfig` retries through a transient connection blip instead of silently falling back to defaults. + +### Fixed +- **Source-scoped reads honor the caller's grant across every read op (gbrain#1924, #1371, #1393).** One shared resolver replaces the per-op scope logic: a remote caller's "all sources" request is bounded to its grant, an out-of-grant source is refused, and `get_page`'s exact-slug path is scoped like every other read (both engines). +- **Legacy bearer tokens carry their stored source grant (gbrain#1336).** Tokens with an operator-set source grant read across exactly those sources instead of being limited to `default`. +- **Non-string frontmatter no longer aborts `lint`/`sync` (gbrain#1883, #1658, #1556, #1948).** Title/slug/type are coerced to usable strings instead of throwing mid-run, and `gbrain lint` reports the un-quoted field by name. +- **`embed --catch-up` runs to completion (gbrain#1946).** The mode no longer stops after one batch, and surfaces chunks that can't be embedded instead of looking like a clean finish. +- **Frontmatter pre-commit hook matches `.md`/`.mdx` files (gbrain#1840).** The installed hook was a silent no-op; it now validates staged markdown on commit. +- **Skill catalog shows block-scalar descriptions (gbrain#1711).** Skills written with `description: |` show their real text instead of a stray indicator. +- **`getConfig` retries on a transient connection blip (gbrain#1603)** instead of silently falling through to defaults (which surfaced as the wrong search mode / empty output on remote Postgres). + +### To take advantage of v0.42.37.0 + +`gbrain upgrade`. No configuration needed. If `gbrain lint` now flags a `frontmatter-non-string-field` on a page, quote the value in that page's frontmatter (e.g. `title: "123"`). Reinstall the pre-commit hook with `gbrain frontmatter install-hook` to pick up the fixed matcher. + +## [0.42.36.0] - 2026-06-08 + +**A huge `gbrain sync` that keeps getting killed now converges instead of restarting from zero.** On a high-write source — hundreds of thousands of files, a generator committing faster than each sync can drain — a full sync that ran past its launching session's timeout (SIGTERM) would lose 100% of its progress and re-import the entire backlog on the next run, forever. The bookmark never advanced, the source went quietly stale for hours while the importer burned CPU the whole time, and competing hourly launches stole each other's lock and raced. This release makes a large sync **resumable, durable, and single-flight** so it banks what it imports and picks up where it left off. + +Progress is now banked into an append-only checkpoint as files drain, written through a direct session connection so it survives connection-pool exhaustion (the exact condition that used to silently drop every checkpoint write). The write is a delta — one row per drained file — instead of rewriting the whole completed-set each flush, so banking stays cheap even at hundreds of thousands of files. The bookmark still only advances on true completion, so a killed run resumes from the checkpoint rather than re-walking from zero. And the per-source lock now heartbeats through the direct pool and refuses to steal a holder that's alive and actively refreshing — so a long sync that overruns into the next scheduled run is skipped, not break-locked into a thrashing race. + +### Fixed +- **Resumable sync survives pool exhaustion (gbrain#1794).** Checkpoint reads/writes route through the direct session pool with bounded retry; `EMAXCONNSESSION` / `too_many_connections` are now classified retryable. A killed run banks its progress and the next run skips already-drained files. +- **Guaranteed final flush on every exit path.** A cooperative timeout, an external SIGTERM (one-shot no-retry flush via the cleanup registry, ordered before lock release), and a clean finish all bank the in-flight delta. The bookmark is never advanced on a partial. +- **Fail-loud instead of burning CPU.** If checkpoint persistence fails repeatedly (pool genuinely dead), the run aborts with a `checkpoint_unavailable` partial rather than importing work it can never bank. Every partial/blocked exit now logs how many files were banked, so a killed run is never misread as total loss. +- **Lock thrash eliminated.** The import loop yields the event loop so the lock-refresh heartbeat fires mid-import; takeover refuses to steal a recently-refreshed (alive-but-starved) holder; a bare `gbrain sync` (no `--source`) now uses the refreshing lock too; and a cron sync that collides with a running one is reported as a skip, not a phase failure. + +### Added +- **Append-only checkpoint storage** (`op_checkpoint_paths`, migration v115): one row per drained path; O(delta) writes instead of O(N²) full-set rewrites over a large sync. + +### To take advantage of v0.42.36.0 +- Nothing to do. `gbrain sync` is resumable by default — a killed sync now banks its progress and the next run converges. Five env knobs tune cadence, fail-loud threshold, event-loop yield, and lock-steal grace if you need them at incident time; see the "Sync resumability + lock tuning" section in CLAUDE.md. + +## [0.42.35.0] - 2026-06-07 + +**A bookmark left pointing at a rewritten-away commit no longer freezes your brain in an endless full re-walk.** When a source's history is rewritten — a force-push, a `master`→`main` consolidation, a squash — the commit gbrain recorded as "last synced" can fall outside the branch's current history. The old guard treated that the same as a missing commit and fell back to re-importing the entire repository on every run. On a large brain with a cross-region database that full walk never finishes inside the sync timeout, so the bookmark never advanced and the source went quietly stale with no error surfaced. + +The fix is a smaller, exact diff. `git diff A..B` compares two trees and does not require A to be an ancestor of B, so when the recorded commit's object is still on disk (the common case right after a rewrite) gbrain now diffs directly against it and imports only the real delta — the changed files, not the whole tree. A clear `[sync] last_commit … history rewritten` line marks the recovery. Only when the commit object is genuinely gone does sync fall back to a full reconcile, and that reconcile now also purges pages whose source files were removed — so a full sync is finally authoritative for deletes, not just imports (manually authored `put_page` pages and metafiles are never swept). Sibling of the v0.42.32.0 silent-staleness fix (gbrain#1939); closes gbrain#1970. + +### Fixed +- **Sync recovers from an unreachable `last_commit` instead of full-walking forever (gbrain#1970).** A bookmark orphaned by a history rewrite is now diffed tree-to-tree directly when its object is still present, importing only the changed files; an oversized or failed diff degrades to a full reconcile instead of throwing. Only a truly-absent (gc'd) object forces a full reconcile. +- **A full sync now purges deleted files.** `performFullSync` reconciles deletions — pages whose backing file is gone are removed (gated to file-backed pages via `source_path`; manual `put_page` pages and metafiles are spared). This makes both the object-absent recovery path and every `--full` sync authoritative for deletes, not just imports. +- **Rename to an unsyncable path deletes the stale page.** A syncable file renamed to a non-syncable destination (which git reports as a rename, not a delete) now removes the old page instead of leaving it orphaned. + +### To take advantage of v0.42.35.0 +- Nothing to do. The next `gbrain sync` after upgrading self-heals a stuck bookmark automatically; watch for the one-line `[sync] last_commit … history rewritten` recovery message. If a source has been stale since a force-push or branch consolidation, this is the release that unsticks it. ## [0.42.34.0] - 2026-06-07 **Relationship questions now get relationship answers.** Ask "who invested in widget-co", "who introduced me to alice-example", or "what connects fund-a and fund-b" and gbrain resolves the named entity and walks its typed-edge graph (`invested_in`, `works_at`, `founded`, `attended`, `advises`, …) to surface the answer — even when no single page mentions both sides. Until now the graph only re-ranked results that keyword/vector search had already found; a relationship that lived purely in the edges (an investor whose page never names the company) was invisible. It now enters retrieval as a first-class candidate. diff --git a/CLAUDE.md b/CLAUDE.md index 6645fe69a..56d63874d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -370,6 +370,24 @@ For background tasks (`run_in_background: true`), the harness captures the exit file separately — use it via the bg task's `.exit` file, not the streamed output. +## Sync resumability + lock tuning (v0.42.x, #1794) + +`gbrain sync` is resumable and converges under pool exhaustion + repeated kills. +Progress banks into the append-only `op_checkpoint_paths` table (one row per drained +path, written via the direct session pool so it survives `EMAXCONNSESSION`); a killed +run resumes from the checkpoint and `last_commit` only advances on true completion. The +per-source lock heartbeats through the direct pool and refuses to steal a live, +recently-refreshed holder. Five env knobs tune it (all env-only, incident-time escape +hatches — no config-dashboard surface by design): + +| Env var | Default | What it does | +|---|---|---| +| `GBRAIN_SYNC_CHECKPOINT_EVERY` | 1000 | Flush the checkpoint every N drained files. | +| `GBRAIN_SYNC_CHECKPOINT_SECONDS` | 10 | Also flush every N seconds (whichever comes first) — bounds worst-case loss regardless of throughput. Flush also fires after the first file. | +| `GBRAIN_SYNC_MAX_CHECKPOINT_FAILURES` | 3 | Consecutive failed flushes (each already retried ~12s) before the run aborts with `reason: 'checkpoint_unavailable'` instead of importing work it can never bank. | +| `GBRAIN_SYNC_YIELD_EVERY` | 64 | Yield the event loop (`setTimeout(0)`, NOT `setImmediate` — Bun starves the timers phase under a tight setImmediate loop) every N files so the lock-refresh `setInterval` heartbeat fires mid-import. | +| `GBRAIN_LOCK_STEAL_GRACE_SECONDS` | derived (~600 at 30min TTL) | A holder that refreshed within this window is NOT stolen even if its TTL lapsed (starved-but-alive). Dead holders stop refreshing, age past the grace, and become stealable; TTL stays the backstop. | + ## Build `bun build --compile --outfile bin/gbrain src/cli.ts` diff --git a/VERSION b/VERSION index 5f3886e1f..6f19cb7c6 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.42.34.0 +0.42.37.0 diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index e5186de7a..ef7ccc392 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -288,7 +288,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/core/embedding-dim-check.ts` extension — facts.embedding dim drift surface. `readFactsEmbeddingDim(engine): Promise` covers both `vector(N)` and `halfvec(N)` shapes (migration v40 falls back to `vector` on pgvector < 0.7); regex ordering is halfvec-before-vector (substring "vec" appears in "halfvec"; naive `/vector/i` would shadow). `buildFactsAlterRecipe(dims, configured, type)` emits the paste-ready `DROP INDEX IF EXISTS idx_facts_embedding_hnsw; ALTER TABLE facts ALTER COLUMN embedding TYPE halfvec(N) USING embedding::halfvec(N); CREATE INDEX idx_facts_embedding_hnsw ON facts USING hnsw (embedding halfvec_cosine_ops) WHERE ...` flow (NOT bare REINDEX, which doesn't rewrite the index after a column-type change). `assertFactsEmbeddingDimMatchesConfig(engine)` is the preflight — throws `FactsEmbeddingDimMismatchError` (tagged `tag: 'FACTS_EMBEDDING_DIM_MISMATCH'` for parity with the worker-pool MUST_ABORT semantics) when configured dim ≠ column width; cached per-engine via `WeakMap`; PGLite engines silently skip. Doctor check `facts_embedding_width_consistency` (registered after `embedding_width_consistency`) reuses the same helpers with an identical ALTER recipe. Pinned by `test/embedding-dim-check-facts.test.ts`. - `src/core/postgres-engine.ts` extension — `insertFact` + `insertFacts` no longer hardcode `tx.unsafe(\`'${embedLit}'::vector\`)` for the embedding column. `resolveFactsEmbeddingCast()` (private) probes `pg_attribute` once per engine instance (cached in `_factsEmbeddingCastSuffix`) and returns `'::halfvec'` when migration v40 created the column as halfvec, else `'::vector'`; both insert paths use the cached suffix so the cast matches the actual column type (works on older pgvector that lacks implicit auto-cast). Test seam `__resetFactsEmbeddingCastCacheForTest()` clears the per-engine cache. - `src/core/cycle.ts` + `src/core/cycle/extract-atoms.ts` + `src/core/cycle/synthesize-concepts.ts` + `src/commands/extract.ts` + `src/commands/doctor.ts` + `src/core/op-checkpoint.ts` extensions — six daily-driver ops fixes. (1) Batch idempotency: `atomsExistingForHashes(engine, sourceId, hashes[])` (exported from `src/core/cycle/extract-atoms.ts`) replaces the per-hash loop (7K individual queries) with one batched SQL roundtrip returning already-extracted `content_hash16` values; fail-open (SQL error → empty set, extraction proceeds); powered by migration v104 `pages_atom_source_hash_idx` (partial expression index on `frontmatter->>'source_hash'` for atom rows where `deleted_at IS NULL`; Postgres `CREATE INDEX CONCURRENTLY` with invalid-remnant pre-drop, PGLite plain). (2) Shorter cycle lock TTL + active in-phase refresh: `LOCK_TTL_MINUTES = 5` (was 30); `buildYieldDuringPhase(lock, outer)` (exported, with `LockHandle`) calls `lock.refresh()` + any external hook on every fire, throttled to 30s via `maybeYield`, firing both in the main loop AND immediately after every `await chat(...)`; `synthesize_concepts` uses the same throttled hook. A crashed cycle releases its lock 6x faster while a healthy long-running cycle keeps it alive (residual: a single `await chat()` past 5 min can expire the lock mid-await — TODO-OPS-2). (3) Progress wiring: `progress?: ProgressReporter` opt on `ExtractAtomsOpts` and `SynthesizeConceptsOpts`; cycle.ts passes its phase-level reporter down (NOT a child reporter, which would collide on `cycle.extract_atoms.extract_atoms.work`); phases only call `tick()`/`heartbeat()`, cycle.ts owns `start()`/`finish()`. (4) `by-mention` resume: `mentionsFingerprint({source, type, since, gazetteerHash})` in `src/core/op-checkpoint.ts` — the gazetteer hash is load-bearing (adding entity pages mid-pause shifts the hash → new fingerprint → fresh scan against the new gazetteer, never silent skip); `gbrain extract links --by-mention` resumes via `op_checkpoints` with `flushAndCheckpoint` ordering (links flush to DB FIRST, page keys commit to checkpoint SECOND, persist THIRD, so a crash mid-batch leaves the page un-checkpointed and resume re-scans it); persist every 1000 items OR 30s; clean exit clears the checkpoint; `--dry-run` skips both load and write. (5) `sync_consolidation` doctor check (multi-source brains see a paste-ready `gbrain sync --all --parallel 4 --workers 4 --skip-failed`; single-source "not applicable"; SQL errors return `warn` via the check's own try/catch). (6) Test-isolation: `test/cycle-last-full-cycle-at.test.ts` + `test/schema-cli.test.ts` use per-test `GBRAIN_HOME=tempdir`. Pinned by `test/cycle/extract-atoms-batch.test.ts`, `test/cycle/cycle-lock-ttl.test.ts` (regression pin on `LOCK_TTL_MINUTES === 5`), `test/op-checkpoint-mentions-fingerprint.test.ts`, `test/cycle/extract-atoms-progress.test.ts`, `test/cycle/synthesize-concepts-progress.test.ts`, `test/cycle/yield-during-phase-refresh.test.ts`, `test/cycle/yield-during-phase-throttle.test.ts`, `test/extract-by-mention-resume.test.ts`, `test/doctor-sync-consolidation.test.ts`. Companion `sync --all` recipe block in `skills/cron-scheduler/SKILL.md`. -- `src/commands/sync.ts` — `gbrain sync` CLI + the `performSync` / `performFullSync` library entrypoints (consumed by the autopilot cycle and the Minion sync handler). `performSync` runs under a writer lock: per-source `gbrain-sync:` whenever `opts.sourceId` is set, wrapped in `withRefreshingLock` from `src/core/db-lock.ts` so long-running sources (250K+ chunks) don't lose the lock at the 30-min TTL mid-run; legacy single-default-source brains keep the bare `tryAcquireDbLock(SYNC_LOCK_ID)` path; `SyncOpts.lockId?: string` is the explicit override. This lock-identity invariant prevents a `sync --all` per-source worker racing `sync --source foo` on the global lock from corrupting the same source. `performSync` throws a typed `SyncLockBusyError` when the writer lock is held; the Minion `sync` handler (`src/commands/jobs.ts`) catches it and marks the job SKIPPED (not failed) so a cron/autopilot tick defers to the holder without polluting crash metrics. `performSyncInner` is RESUMABLE (incremental path): it drains a PINNED target commit (`lastCommit..pin`), banking drained file paths in two `op_checkpoints` rows (both keyed by `syncFingerprint({sourceId, lastCommit})` from `src/core/op-checkpoint.ts` — an `op:'sync'` paths row + an `op:'sync-target'` target row) via `recordCompleted` every `GBRAIN_SYNC_CHECKPOINT_EVERY` (default 1000) files, and advances `last_commit`/`last_sync_at` ONLY at full import completion. A sync killed mid-import banks its progress, leaves the anchor unmoved (the source stays correctly stale to the autopilot scheduler — `last_sync_at` is never bumped on a partial), and the next run `resumeFilter`s the same fixed diff to skip done files. The pin is the checkpoint's stored target when still reachable from HEAD, else live HEAD (a history rewrite / reset re-pins); completion advances to the pin, NOT live HEAD, so commits landing past the pin are a clean next-sync diff — this closes the cross-run staleness window. After import a pin-reachability gate (`git merge-base --is-ancestor pin HEAD`) replaces the old strict head-drift gate: forward commits on top of the pin (e.g. a background enrich process committing to the same repo every ~2 min) are SAFE and no longer block the run; only a real rewrite (pin not an ancestor of HEAD) blocks and discards the checkpoint. A file added in `lastCommit..pin` but gone from disk (deleted by a commit after the pin) is SKIPPED and marked completed, not recorded as a failure. Downstream extract/facts/embed are size-gated: inline only for `totalChanges <= 100`; large syncs defer to the resumable `extract --stale` watermark + `embed --stale`/backfill + the facts cycle phases, so a 44K-page facts/embed pass never re-blocks import convergence (sync convergence == import convergence). Worker engines wrap in try/finally so disconnect always fires; both PGLite-detection sites use `engine.kind === 'pglite'`. CLI accepts `--workers N` (alias `--concurrency N`) validated via `parseWorkers` (explicit bypasses the file-count floor; auto path defers to `autoConcurrency()`). The newest-first descending-lex order uses `sortNewestFirst(addsAndMods)` from `src/core/sort-newest-first.ts` (shared with `gbrain import`). `gbrain sync --all` runs a continuous worker pool: `parseWorkers`-validated `--parallel N` (default `min(sourceCount, --workers, DEFAULT_PARALLEL_SOURCES=4)`), long-lived async workers pulling from a shared FIFO queue (no head-of-line blocking), per-source `withSourcePrefix(src.id, ...)` so every `slog`/`serr` line carries `[]`; `--skip-failed`/`--retry-failed` reject when combined with `--parallel > 1` (the brain-global `sync-failures.jsonl` has no per-source scope); a connection-budget stderr warning fires when `parallel × workers × 2 > 16` (the `× 2 per-file pool` factor: each per-file worker opens its own `PostgresEngine` with `poolSize=2`). Exports `resolveParallelism`, `syncOneSource`, `buildSyncStatusReport`, `printSyncStatusReport`, `SyncStatusReport` back the `gbrain sources status` dashboard. `--json` envelope `{schema_version: 1, sources, parallel, ok_count, error_count}` on stdout; human banners route to stderr via `humanSink` so `jq` parses cleanly. Exit matrix: 0 all ok, 1 any error, 2 cost-prompt-not-confirmed. The dashboard SQL is `content_chunks ch JOIN pages pg ON pg.id = ch.page_id WHERE pg.deleted_at IS NULL` with `archived = false` at the caller; embedding column resolved via `resolveEmbeddingColumn(undefined, cfg)` from `src/core/search/embedding-column.ts` so Voyage/multimodal/non-default-column brains count against the column they use; errors propagate (no swallow-catch). The sync delete loop is interleaved per-batch resolve+delete using `engine.resolveSlugsByPaths` + `engine.deletePages` from `src/core/engine.ts` (73K-delete commit: ~146K SQL round-trips → ~292, closing the cascade-staleness class where one big-delete commit jammed every other source's sync); per-batch try-catch decomposes batch DELETE failures to per-slug `deletePage` fallback, unrecoverable per-slug failures land in `failedFiles`; `pagesAffected` filters to confirmed-deleted slugs. The rename loop gets the same batched slug-resolve. `resolveSlugByPathOrSourcePath` at `sync.ts:267` delegates to `engine.resolveSlugsByPaths` when `sourceId` is set, keeping legacy `executeRaw` fallback for the no-sourceId path. `failedFiles` is hoisted to the top of `performSyncInner` so both delete-decompose and import loops feed the same bookmark gate. The `sync --all` cost gate is mode-aware via `willEmbedSynchronously` + `shouldBlockSync` from `src/core/embedding.ts` (federated_v2 resolved once up front): the DEFERRED path (v2 on, parallel) is INFORMATIONAL (embedding goes to per-source `embed-backfill` jobs with their own `$X/source/24h` cap, default $25 via `SPEND_CAP_CONFIG_KEY` from `embed-backfill-submit.ts`; prints the cap + backlog `sumStaleChunkChars({signature})` priced via `estimateCostFromChars` at `currentEmbeddingPricePerMTok()` + queued-job count, NEVER exits 2); the INLINE path BLOCKS only when the new-content estimate exceeds `sync.cost_gate_min_usd` (default $0.50). The new-content estimate (`estimateInlineNewTokens`) contributes ZERO for sources provably unchanged since last sync (`isSourceUnchangedSinceSync` HEAD==last_commit + clean tree AND `chunker_version === CURRENT`) and the full-tree ceiling for changed sources; the pre-existing stale backlog is shown informationally but NEVER gated on (sync doesn't sweep it inline). Helpers `resolveCostGateFloorUsd(engine)` (fail-open $0.50, accepts 0) + `resolveBackfillCapUsd(engine)`. JSON envelopes gain `mode` + `gate` discriminators (`dry_run | deferred_notice | below_floor | confirmation_required`); `SyncStatusReportSource` gains `backfill_queued`/`backfill_active`/`backfill_last_completed_at`; cost previews read `getEmbeddingModelName()` (no hardcoded OpenAI). `SyncOpts.noSchemaPack` (CLI `--no-schema-pack`, threaded through `performSync` AND `syncOneSource`) skips `loadActivePack` so pages fall back to legacy prefix typing — an escape hatch when a suspect pack regex wedges a sync. A per-file BEGIN heartbeat `if (process.env.GBRAIN_SYNC_TRACE) serr('[sync] begin import: ')` fires BEFORE `importFile` (the `progress.tick` fires only AFTER) so a stuck file is a begin-line with no matching completion. Triage doc: `docs/architecture/serve-sync-concurrency.md` (PGLite single-writer serve↔sync contention + the `GBRAIN_SYNC_TRACE` + `--no-schema-pack` recipes). Pinned by `test/e2e/sync-status-pglite.test.ts` (IRON-RULE: PGLite seeds 2 sources × pages × chunks, soft-deletes 1 page, archives 1 source, validates the SQL excludes both AND uses the active embedding column), `test/sync-cost-gate.serial.test.ts`, `test/sync-cost-preview.test.ts`. Runaway-sync protection: `resolveSyncHardDeadline(args, {isTty, env, defaultNonTtySec?})` resolves a wall-clock hard deadline (precedence `--no-hard-deadline` > `--hard-deadline ` > `--timeout `(non-`--all`, which auto-arms the backstop) > `GBRAIN_SYNC_MAX_RUNTIME_SECONDS` env > non-TTY default 3600s > none; `HARD_DEADLINE_GRACE_SEC=30`). `src/cli.ts` installs the out-of-band watchdog (see `src/core/process-watchdog.ts`) for the sync command BEFORE `connectEngine` and disposes it in the dispatch `finally`, so even an event-loop-starved sync — or a connect-phase hang — is SIGTERM-then-SIGKILLed by the deadline instead of orphaning under cron. `runSync` registers a SIGINT handler that aborts an interrupt `AbortController` composed via `composeAbortSignals(...)` (an `AbortSignal.any` wrapper over the defined signals) with the per-source `--timeout` signal, so Ctrl-C returns a clean `partial` and releases the lock through the normal `finally` (process-cleanup.ts owns SIGTERM lock-release; the watchdog owns the hard kill). `withRefreshingLock` `unref()`s its refresh `setInterval`. The spin's own root cause is not yet pinned (leading lead is catastrophic-backtracking in a pack link-inference regex, bounded by the redos-guard); the watchdog heartbeat plus the existing `[gbrain phase]` breadcrumbs are the diagnosis surface. Pinned by `test/sync-hard-deadline.test.ts` (resolution precedence + `composeAbortSignals`). +- `src/commands/sync.ts` — `gbrain sync` CLI + the `performSync` / `performFullSync` library entrypoints (consumed by the autopilot cycle and the Minion sync handler). `performSync` runs under a writer lock: per-source `gbrain-sync:` whenever `opts.sourceId` is set, wrapped in `withRefreshingLock` from `src/core/db-lock.ts` so long-running sources (250K+ chunks) don't lose the lock at the 30-min TTL mid-run; the bare no-source path uses the SAME refreshing lock (#1794 — it was previously a non-refreshing `tryAcquireDbLock`, stealable mid-run during an incident); `SyncOpts.lockId?: string` is the explicit override. The lock refresh AND its health probe route through the DIRECT session pool so Supavisor transaction-pool exhaustion (`EMAXCONNSESSION`) can't kill renewal; takeover is heartbeat-aware (it will NOT steal a holder whose `last_refreshed_at` is within `GBRAIN_LOCK_STEAL_GRACE_SECONDS`, defending an alive-but-starved holder); the import loop yields the event loop every `GBRAIN_SYNC_YIELD_EVERY` files (`setTimeout(0)`, not `setImmediate` — Bun starves the timers phase) so the refresh `setInterval` heartbeat fires mid-import. This lock-identity invariant prevents a `sync --all` per-source worker racing `sync --source foo` on the global lock from corrupting the same source. `performSync` throws a typed `SyncLockBusyError` when the writer lock is held; the Minion `sync` handler (`src/commands/jobs.ts`) catches it and marks the job SKIPPED (not failed) so a cron/autopilot tick defers to the holder without polluting crash metrics. `performSyncInner` is RESUMABLE (incremental path): it drains a PINNED target commit (`lastCommit..pin`), banking drained file paths via `appendCompleted` (append-only delta into the `op_checkpoint_paths` child table, migration v115 — one row per path, O(delta) not the old O(N²) full-array rewrite), keyed by `syncFingerprint({sourceId, lastCommit})` from `src/core/op-checkpoint.ts` (paths under `op:'sync'`; the pinned target under `op:'sync-target'`), and advances `last_commit`/`last_sync_at` ONLY at full import completion. Checkpoint writes route through the DIRECT session pool + bounded retry so they survive `EMAXCONNSESSION`; the flush cadence is first-file then every `GBRAIN_SYNC_CHECKPOINT_EVERY` (default 1000) files OR `GBRAIN_SYNC_CHECKPOINT_SECONDS` (default 10s), with a race-safe `pendingCheckpointPaths` delta (single-flight swap, re-merge on failure) under parallel workers; a SIGTERM banks the in-flight delta via a no-retry one-shot (`appendCompletedOnce`, ordered before lock release through `registerCleanup`); and sustained flush failure aborts the run with `reason:'checkpoint_unavailable'` after `GBRAIN_SYNC_MAX_CHECKPOINT_FAILURES` consecutive fails rather than importing work it can never bank (every partial/blocked exit logs the banked-file count). A sync killed mid-import banks its progress, leaves the anchor unmoved (the source stays correctly stale to the autopilot scheduler — `last_sync_at` is never bumped on a partial), and the next run `resumeFilter`s the same fixed diff to skip done files. The pin is the checkpoint's stored target when still reachable from HEAD, else live HEAD (a history rewrite / reset re-pins); completion advances to the pin, NOT live HEAD, so commits landing past the pin are a clean next-sync diff — this closes the cross-run staleness window. After import a pin-reachability gate (`git merge-base --is-ancestor pin HEAD`) replaces the old strict head-drift gate: forward commits on top of the pin (e.g. a background enrich process committing to the same repo every ~2 min) are SAFE and no longer block the run; only a real rewrite (pin not an ancestor of HEAD) blocks and discards the checkpoint. A file added in `lastCommit..pin` but gone from disk (deleted by a commit after the pin) is SKIPPED and marked completed, not recorded as a failure. Downstream extract/facts/embed are size-gated: inline only for `totalChanges <= 100`; large syncs defer to the resumable `extract --stale` watermark + `embed --stale`/backfill + the facts cycle phases, so a 44K-page facts/embed pass never re-blocks import convergence (sync convergence == import convergence). Worker engines wrap in try/finally so disconnect always fires; both PGLite-detection sites use `engine.kind === 'pglite'`. CLI accepts `--workers N` (alias `--concurrency N`) validated via `parseWorkers` (explicit bypasses the file-count floor; auto path defers to `autoConcurrency()`). The newest-first descending-lex order uses `sortNewestFirst(addsAndMods)` from `src/core/sort-newest-first.ts` (shared with `gbrain import`). `gbrain sync --all` runs a continuous worker pool: `parseWorkers`-validated `--parallel N` (default `min(sourceCount, --workers, DEFAULT_PARALLEL_SOURCES=4)`), long-lived async workers pulling from a shared FIFO queue (no head-of-line blocking), per-source `withSourcePrefix(src.id, ...)` so every `slog`/`serr` line carries `[]`; `--skip-failed`/`--retry-failed` reject when combined with `--parallel > 1` (the brain-global `sync-failures.jsonl` has no per-source scope); a connection-budget stderr warning fires when `parallel × workers × 2 > 16` (the `× 2 per-file pool` factor: each per-file worker opens its own `PostgresEngine` with `poolSize=2`). Exports `resolveParallelism`, `syncOneSource`, `buildSyncStatusReport`, `printSyncStatusReport`, `SyncStatusReport` back the `gbrain sources status` dashboard. `--json` envelope `{schema_version: 1, sources, parallel, ok_count, error_count}` on stdout; human banners route to stderr via `humanSink` so `jq` parses cleanly. Exit matrix: 0 all ok, 1 any error, 2 cost-prompt-not-confirmed. The dashboard SQL is `content_chunks ch JOIN pages pg ON pg.id = ch.page_id WHERE pg.deleted_at IS NULL` with `archived = false` at the caller; embedding column resolved via `resolveEmbeddingColumn(undefined, cfg)` from `src/core/search/embedding-column.ts` so Voyage/multimodal/non-default-column brains count against the column they use; errors propagate (no swallow-catch). The sync delete loop is interleaved per-batch resolve+delete using `engine.resolveSlugsByPaths` + `engine.deletePages` from `src/core/engine.ts` (73K-delete commit: ~146K SQL round-trips → ~292, closing the cascade-staleness class where one big-delete commit jammed every other source's sync); per-batch try-catch decomposes batch DELETE failures to per-slug `deletePage` fallback, unrecoverable per-slug failures land in `failedFiles`; `pagesAffected` filters to confirmed-deleted slugs. The rename loop gets the same batched slug-resolve; a rename whose destination is un-syncable folds the source path into the delete set so the old page can't orphan (#1970). An entry-time bookmark-reachability guard distinguishes a gc'd anchor (`cat-file` fails → `performFullSync`) from a history-rewrite anchor that is merely no-longer-an-ancestor but still on disk: the latter is diffed tree-to-tree directly (`git diff lastCommit..pin` is an endpoint-tree compare, ancestry not required) so a force-push / `master`→`main` consolidation imports only the real delta instead of re-walking the whole tree forever (#1970); an oversized or failed diff degrades to `performFullSync`. `performFullSync` is itself authoritative for deletes — after an advancing full import it purges file-backed pages (`source_path != null` AND strategy-aware `isSyncable`) whose source file no longer exists, sparing `put_page`/manual pages (null `source_path`) and metafiles. `resolveSlugByPathOrSourcePath` at `sync.ts:267` delegates to `engine.resolveSlugsByPaths` when `sourceId` is set, keeping legacy `executeRaw` fallback for the no-sourceId path. `failedFiles` is hoisted to the top of `performSyncInner` so both delete-decompose and import loops feed the same bookmark gate. The `sync --all` cost gate is mode-aware via `willEmbedSynchronously` + `shouldBlockSync` from `src/core/embedding.ts` (federated_v2 resolved once up front): the DEFERRED path (v2 on, parallel) is INFORMATIONAL (embedding goes to per-source `embed-backfill` jobs with their own `$X/source/24h` cap, default $25 via `SPEND_CAP_CONFIG_KEY` from `embed-backfill-submit.ts`; prints the cap + backlog `sumStaleChunkChars({signature})` priced via `estimateCostFromChars` at `currentEmbeddingPricePerMTok()` + queued-job count, NEVER exits 2); the INLINE path BLOCKS only when the new-content estimate exceeds `sync.cost_gate_min_usd` (default $0.50). The new-content estimate (`estimateInlineNewTokens`) contributes ZERO for sources provably unchanged since last sync (`isSourceUnchangedSinceSync` HEAD==last_commit + clean tree AND `chunker_version === CURRENT`) and the full-tree ceiling for changed sources; the pre-existing stale backlog is shown informationally but NEVER gated on (sync doesn't sweep it inline). Helpers `resolveCostGateFloorUsd(engine)` (fail-open $0.50, accepts 0) + `resolveBackfillCapUsd(engine)`. JSON envelopes gain `mode` + `gate` discriminators (`dry_run | deferred_notice | below_floor | confirmation_required`); `SyncStatusReportSource` gains `backfill_queued`/`backfill_active`/`backfill_last_completed_at`; cost previews read `getEmbeddingModelName()` (no hardcoded OpenAI). `SyncOpts.noSchemaPack` (CLI `--no-schema-pack`, threaded through `performSync` AND `syncOneSource`) skips `loadActivePack` so pages fall back to legacy prefix typing — an escape hatch when a suspect pack regex wedges a sync. A per-file BEGIN heartbeat `if (process.env.GBRAIN_SYNC_TRACE) serr('[sync] begin import: ')` fires BEFORE `importFile` (the `progress.tick` fires only AFTER) so a stuck file is a begin-line with no matching completion. Triage doc: `docs/architecture/serve-sync-concurrency.md` (PGLite single-writer serve↔sync contention + the `GBRAIN_SYNC_TRACE` + `--no-schema-pack` recipes). Pinned by `test/e2e/sync-status-pglite.test.ts` (IRON-RULE: PGLite seeds 2 sources × pages × chunks, soft-deletes 1 page, archives 1 source, validates the SQL excludes both AND uses the active embedding column), `test/sync-cost-gate.serial.test.ts`, `test/sync-cost-preview.test.ts`. Runaway-sync protection: `resolveSyncHardDeadline(args, {isTty, env, defaultNonTtySec?})` resolves a wall-clock hard deadline (precedence `--no-hard-deadline` > `--hard-deadline ` > `--timeout `(non-`--all`, which auto-arms the backstop) > `GBRAIN_SYNC_MAX_RUNTIME_SECONDS` env > non-TTY default 3600s > none; `HARD_DEADLINE_GRACE_SEC=30`). `src/cli.ts` installs the out-of-band watchdog (see `src/core/process-watchdog.ts`) for the sync command BEFORE `connectEngine` and disposes it in the dispatch `finally`, so even an event-loop-starved sync — or a connect-phase hang — is SIGTERM-then-SIGKILLed by the deadline instead of orphaning under cron. `runSync` registers a SIGINT handler that aborts an interrupt `AbortController` composed via `composeAbortSignals(...)` (an `AbortSignal.any` wrapper over the defined signals) with the per-source `--timeout` signal, so Ctrl-C returns a clean `partial` and releases the lock through the normal `finally` (process-cleanup.ts owns SIGTERM lock-release; the watchdog owns the hard kill). `withRefreshingLock` `unref()`s its refresh `setInterval`. The spin's own root cause is not yet pinned (leading lead is catastrophic-backtracking in a pack link-inference regex, bounded by the redos-guard); the watchdog heartbeat plus the existing `[gbrain phase]` breadcrumbs are the diagnosis surface. Pinned by `test/sync-hard-deadline.test.ts` (resolution precedence + `composeAbortSignals`). - `src/commands/import.ts` — `gbrain import` CLI + `runImport` library entrypoint. Uses a path-set checkpoint via `src/core/import-checkpoint.ts` (the walk still applies `sortNewestFirst()` for embed-cost ordering, but checkpoint correctness no longer depends on sort order). A file enters `completed: Set` only when its `processFile` returns success (including content-hash short-circuit no-ops); failed files never enter the set so the next run retries them automatically with no manual `~/.gbrain/import-checkpoint.json` delete. This closes three classes: parallel-import-with-slow-worker dropping the slow file on crash-resume (the slow file isn't in `completed` until its own `processFile` resolves), failed-file-bumps-counter-past-itself (failures don't add to `completed`), and sort-flip-drops-newest-N-on-cross-version-resume (order is no longer part of the checkpoint). Old positional checkpoints are detected and discarded with a stderr line on first resume (re-walking is cheap because `content_hash` short-circuits unchanged files). Checkpoint persists every 100 successful adds, not every 100 processed files. The `managedBookmark` opt (set by `performFullSync` when `runImport` is the full-sync engine) suppresses `runImport`'s own `sync.last_commit` advance so the shared `applySyncFailureGate` (`src/core/sync-failure-ledger.ts`) owns the bookmark + failure-ledger gating on that path — one gate decides advance/block/auto-skip across both sync paths. Pinned by `test/import-checkpoint.test.ts` + `test/import-resume.test.ts` (incl. the SLUG_MISMATCH retry regression). - `src/core/import-checkpoint.ts` — `loadCheckpoint(brainDir)`, `saveCheckpoint(brainDir, completed)`, `resumeFilter(files, completed, brainDir)`, `clearCheckpoint()`, plus the `ImportCheckpoint` type. Path-set format `{schema_version, brainDir, completed: string[]}`. Atomic write via `.tmp` + `rename()` so a mid-write crash never leaves a partial JSON. `loadCheckpoint` returns `null` on: missing file, malformed JSON, brainDir mismatch (ran against a different brain), and the old positional format (logged to stderr before discard). `resumeFilter` returns `{toProcess, skippedCount}` — pure, no I/O, deterministic. `clearCheckpoint` is no-op-on-missing for clean-exit cleanup. Honors `GBRAIN_HOME` via `gbrainPath()` so `withEnv({GBRAIN_HOME: tmpdir})` test isolation works without monkey-patching fs. Best-effort persistence — `saveCheckpoint` logs warnings on write errors but never throws. - `src/core/sort-newest-first.ts` — single source of truth for the descending-lex sort that `gbrain import` and `gbrain sync` both apply. Mutates in place (Array.prototype.sort semantics), returns the same array reference for fluent chaining. Empty/single-element inputs short-circuit. Future ordering changes flip one line here instead of touching two CLI commands. Pinned by `test/sort-newest-first.test.ts` (descending order, mixed prefixes, empty, single-element, in-place-mutation contract). @@ -370,7 +370,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/commands/report.ts` — Structured report saver (audit trail for maintenance/enrichment) - `src/core/destructive-guard.ts` — three-layer protection against accidental data loss. `assessDestructiveImpact(engine, sourceId)` counts pages/chunks/embeddings/files for a source. `checkDestructiveConfirmation(impact, opts)` is the fail-closed gate (`--confirm-destructive` required when data is present; `--yes` alone is rejected). `softDeleteSource` / `restoreSource` / `listArchivedSources` / `purgeExpiredSources` drive the source-level archive lifecycle via `sources.archived BOOLEAN`, `archived_at TIMESTAMPTZ`, `archive_expires_at TIMESTAMPTZ`. Page-level analog: `BrainEngine.softDeletePage` / `restorePage` / `purgeDeletedPages` plus `pages.deleted_at TIMESTAMPTZ` and a partial purge index. The MCP `delete_page` op rewires to `softDeletePage`; ops `restore_page` (`scope: write`) and `purge_deleted_pages` (`scope: admin`, `localOnly: true`) round out the surface. Search visibility (`buildVisibilityClause` in `src/core/search/sql-ranking.ts`) hides soft-deleted pages and archived sources from `searchKeyword` / `searchKeywordChunks` / `searchVector` in both engines. The autopilot cycle's `purge` phase calls `purgeExpiredSources` + `engine.purgeDeletedPages(72)` so the 72h TTL is real. - `src/commands/pages.ts` — `gbrain pages purge-deleted [--older-than HOURS|Nd] [--dry-run] [--json]` operator escape hatch. Mirror of `gbrain sources purge` for the page-level lifecycle. Hard-deletes pages whose `deleted_at` is older than the cutoff; cascades to content_chunks/page_links/chunk_relations. -- `src/core/op-checkpoint.ts` — DB-backed checkpoint primitive for long-running ops. Migration v67 introduces `op_checkpoints (op TEXT, fingerprint TEXT, completed_keys JSONB, updated_at TIMESTAMPTZ, PK(op, fingerprint))`. Per-op fingerprint helpers (`embedFingerprint`, `extractFingerprint`, `reindexFingerprint`, `integrityFingerprint`, `purgeFingerprint`) compute `sha8(canonical-JSON(relevant-params))` so re-running with the same params resumes from `completed_keys` and re-running with different params (e.g. `--limit 100` vs `--limit 200`) starts fresh. Cross-worker safe on Postgres (DB row, no file-lock race); PGLite degrades gracefully. Replaces per-op file-backed JSON checkpoints scattered across `import.ts`, `embed.ts`, `reindex.ts`. The 7-day TTL GC runs in the cycle's `purge` phase. Pinned by `test/op-checkpoint.test.ts` (~15 cases incl. per-op fingerprint scoping). `import-checkpoint.ts` was NOT migrated to this primitive — both checkpoint systems coexist without conflict; migrating requires async-propagating 4 sync call sites in `src/commands/import.ts` and rewriting 18 tests, deferred. +- `src/core/op-checkpoint.ts` — DB-backed checkpoint primitive for long-running ops. Migration v67 introduces `op_checkpoints (op TEXT, fingerprint TEXT, completed_keys JSONB, updated_at TIMESTAMPTZ, PK(op, fingerprint))`. Per-op fingerprint helpers (`embedFingerprint`, `extractFingerprint`, `reindexFingerprint`, `integrityFingerprint`, `purgeFingerprint`) compute `sha8(canonical-JSON(relevant-params))` so re-running with the same params resumes from `completed_keys` and re-running with different params (e.g. `--limit 100` vs `--limit 200`) starts fresh. Cross-worker safe on Postgres (DB row, no file-lock race); PGLite degrades gracefully. Replaces per-op file-backed JSON checkpoints scattered across `import.ts`, `embed.ts`, `reindex.ts`. The 7-day TTL GC runs in the cycle's `purge` phase. All writes (`recordCompleted`, `clearOpCheckpoint`) route through `engine.executeRawDirect` + `withRetry(BULK_RETRY_OPTS)` so they survive Supavisor pool exhaustion, and `recordCompleted` returns `boolean` (banked vs failed-after-retries) — the 9 non-sync consumers keep its REPLACE-into-`completed_keys` semantics. Resumable sync uses the additive `appendCompleted(key, deltaKeys)` / `appendCompletedOnce` (the latter no-retry for the SIGTERM path) which INSERT a delta into the `op_checkpoint_paths` child table (migration v115: `(op, fingerprint, path)` PK, FK to `op_checkpoints` ON DELETE CASCADE) via a single writable-CTE `unnest($3::text[])` write — O(delta), killing the old O(N²) full-set rewrite. `loadOpCheckpoint` returns the `UNION ALL` of legacy `completed_keys` + child-table paths (deduped in JS), so an in-flight upgrade loses nothing. `syncFingerprint({sourceId, lastCommit})` keys the sync rows. Pinned by `test/op-checkpoint.test.ts` (incl. delta-append, union read, cascade clear, durable-write boolean). `import-checkpoint.ts` was NOT migrated to this primitive — both checkpoint systems coexist without conflict; migrating requires async-propagating 4 sync call sites in `src/commands/import.ts` and rewriting 18 tests, deferred. - `src/core/brain-score-recommendations.ts` — pure data layer consumed by both `gbrain doctor --remediation-plan` / `--remediate` and `gbrain features`. `computeRecommendations(checks, opts)` returns `Remediation[]` with stable `id`, content-hash `idempotency_key`, `severity`, `est_seconds`, `est_usd_cost`, `depends_on` (references stable ids, not check names — so plan order is reproducible). `classifyChecks(report)` triages every doctor check three-state into `remediable | human_only | blocked` (`human_only` covers RLS warnings and other human-judgment gates; `blocked` covers dependency chains where a parent check failed). `maxReachableScore(checks)` computes the ceiling for empty/under-configured brains (no entity pages → graph_coverage caps at 70; no embedding key → embedding_coverage caps at 60). Cost estimates pull from `anthropic-pricing.ts` (synthesize/patterns/consolidate) and `embedding-pricing.ts` (embed jobs). Pinned by `test/brain-score-recommendations.test.ts` (~27 cases incl. determinism, content-hash idempotency, DB-backed checkpoint provenance, three-state triage). - `src/commands/doctor.ts` extension — `--remediation-plan [--json] [--target-score N]` prints what would run (stable `id`, `idempotency_key`, `severity`, `est_seconds`, `est_usd_cost`, `depends_on`); `--remediate [--yes] [--target-score N] [--max-usd N]` submits each plan step as a Minion job in dependency order, re-checking score between steps. `--target-score N` defaults to 90; refuses to start when target exceeds `maxReachableScore()` and lists what's missing. `--max-usd N` is the cron-safety guard — submission refuses when the plan's `est_total_usd_cost` exceeds the cap. JSON envelope adds a `Check.remediation` field (additive, schema_version unchanged). Pinned by tests in `test/doctor.test.ts`. - `src/commands/jobs.ts` extension — registers 11 Minion handlers: `reindex`, `repair-jsonb`, `orphans`, `integrity`, `purge`, `synthesize` (PROTECTED), `patterns` (PROTECTED), `consolidate` (PROTECTED), `extract_facts`, `resolve_symbol_edges`, `recompute_emotional_weight`. Phase wrappers delegate to `runCycle({phases:[name]})` so `src/core/cycle.ts` stays the single source of truth for phase semantics. The standalone `sync` handler passes `noExtract: true` to match `runPhaseSync`'s contract (doctor's remediation plan emitting `[sync, extract]` would otherwise double-extract). diff --git a/docs/integrations/pre-commit.md b/docs/integrations/pre-commit.md index f683161b2..fc0453fdf 100644 --- a/docs/integrations/pre-commit.md +++ b/docs/integrations/pre-commit.md @@ -7,7 +7,7 @@ brain source's repo that runs `gbrain frontmatter validate` against staged ## What the hook catches -The same seven validation classes the `frontmatter-guard` skill and +The same eight validation classes the `frontmatter-guard` skill and `gbrain doctor`'s `frontmatter_integrity` subcheck report: | Code | What it catches | @@ -18,6 +18,7 @@ The same seven validation classes the `frontmatter-guard` skill and | `SLUG_MISMATCH` | `slug:` in frontmatter doesn't match path-derived slug | | `NULL_BYTES` | Binary corruption (`\x00`) anywhere in the content | | `NESTED_QUOTES` | `title: "outer "inner" outer"` shape that breaks YAML | +| `NON_STRING_FIELD` | `title`/`type`/`slug` is an unquoted non-string scalar (`title: 123`) | | `EMPTY_FRONTMATTER` | `---` ... `---` with nothing meaningful between | ## Install diff --git a/llms-full.txt b/llms-full.txt index 8b3579bdc..558367394 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -518,6 +518,24 @@ For background tasks (`run_in_background: true`), the harness captures the exit file separately — use it via the bg task's `.exit` file, not the streamed output. +## Sync resumability + lock tuning (v0.42.x, #1794) + +`gbrain sync` is resumable and converges under pool exhaustion + repeated kills. +Progress banks into the append-only `op_checkpoint_paths` table (one row per drained +path, written via the direct session pool so it survives `EMAXCONNSESSION`); a killed +run resumes from the checkpoint and `last_commit` only advances on true completion. The +per-source lock heartbeats through the direct pool and refuses to steal a live, +recently-refreshed holder. Five env knobs tune it (all env-only, incident-time escape +hatches — no config-dashboard surface by design): + +| Env var | Default | What it does | +|---|---|---| +| `GBRAIN_SYNC_CHECKPOINT_EVERY` | 1000 | Flush the checkpoint every N drained files. | +| `GBRAIN_SYNC_CHECKPOINT_SECONDS` | 10 | Also flush every N seconds (whichever comes first) — bounds worst-case loss regardless of throughput. Flush also fires after the first file. | +| `GBRAIN_SYNC_MAX_CHECKPOINT_FAILURES` | 3 | Consecutive failed flushes (each already retried ~12s) before the run aborts with `reason: 'checkpoint_unavailable'` instead of importing work it can never bank. | +| `GBRAIN_SYNC_YIELD_EVERY` | 64 | Yield the event loop (`setTimeout(0)`, NOT `setImmediate` — Bun starves the timers phase under a tight setImmediate loop) every N files so the lock-refresh `setInterval` heartbeat fires mid-import. | +| `GBRAIN_LOCK_STEAL_GRACE_SECONDS` | derived (~600 at 30min TTL) | A holder that refreshed within this window is NOT stolen even if its TTL lapsed (starved-but-alive). Dead holders stop refreshing, age past the grace, and become stealable; TTL stays the backstop. | + ## Build `bun build --compile --outfile bin/gbrain src/cli.ts` diff --git a/package.json b/package.json index 7d6f87318..fbd6e4e2d 100644 --- a/package.json +++ b/package.json @@ -143,5 +143,5 @@ "bun": ">=1.3.10" }, "license": "MIT", - "version": "0.42.34.0" + "version": "0.42.37.0" } diff --git a/skills/frontmatter-guard/SKILL.md b/skills/frontmatter-guard/SKILL.md index 2f9e43ff8..6c0744945 100644 --- a/skills/frontmatter-guard/SKILL.md +++ b/skills/frontmatter-guard/SKILL.md @@ -24,7 +24,7 @@ mutating: true ## Contract This skill guarantees: -- Every brain page is scanned against the seven canonical frontmatter validation classes +- Every brain page is scanned against the eight canonical frontmatter validation classes - Mechanical errors (nested quotes, missing closing `---`, null bytes, slug mismatch) are auto-repairable on demand with `.bak` backups - Validation logic is shared with `gbrain doctor`'s `frontmatter_integrity` subcheck — single source of truth - Reports per source (gbrain is multi-source since v0.18.0); never silently audits the wrong root @@ -50,6 +50,7 @@ Without a guard, these accumulate silently until `gbrain sync` chokes or search | `SLUG_MISMATCH` | Frontmatter `slug:` differs from path-derived slug | Yes (removes the field) | | `NULL_BYTES` | Binary corruption (`\x00`) | Yes | | `NESTED_QUOTES` | `title: "outer "inner" outer"` shape | Yes | +| `NON_STRING_FIELD` | `title`/`type`/`slug` is an unquoted non-string scalar (e.g. `title: 123`, `slug: 2024-06-01`) | No (quote the value) | | `EMPTY_FRONTMATTER` | Open + close present but nothing between | No (needs human) | ## Phases diff --git a/src/commands/embed.ts b/src/commands/embed.ts index 08685dc69..3afefab03 100644 --- a/src/commands/embed.ts +++ b/src/commands/embed.ts @@ -629,15 +629,20 @@ async function embedAllStale( const CONCURRENCY = parseInt(process.env.GBRAIN_EMBED_CONCURRENCY || '20', 10); // D3 + D3a + D8: wall-clock budget. 30 min default; env override. - // v0.41.18.0 (A13): --catch-up removes the wall-clock cap entirely so the - // handler runs until countStaleChunks() returns 0. Use Number.MAX_SAFE_INTEGER - // (effectively unbounded) instead of the 30-min default. The AbortController - // still wraps for SIGINT propagation; just the timer never fires. - const BUDGET_MS = staleOpts?.catchUp - ? Number.MAX_SAFE_INTEGER + // #1946: --catch-up removes the wall-clock cap. The prior code set BUDGET_MS = + // Number.MAX_SAFE_INTEGER and passed it to setTimeout — but setTimeout's delay + // is a 32-bit signed int, so MAX_SAFE_INTEGER (9e15) overflows and the timer + // fires almost immediately, aborting catch-up after a single batch. The fix is + // to NOT arm the timer in catch-up at all: the keyset pass below terminates on + // its own (the (page_id, chunk_index) cursor advances monotonically), and + // SIGINT / worker-abort still propagate via externalSignal. + const BUDGET_MS: number | null = staleOpts?.catchUp + ? null : parseInt(process.env.GBRAIN_EMBED_TIME_BUDGET_MS || `${30 * 60 * 1000}`, 10); const budgetController = new AbortController(); - const budgetTimer = setTimeout(() => budgetController.abort(), BUDGET_MS); + const budgetTimer = BUDGET_MS != null + ? setTimeout(() => budgetController.abort(), BUDGET_MS) + : undefined; const budgetSignal = budgetController.signal; // #1737: the effective signal fires when EITHER the internal wall-clock // budget OR the caller's abort (worker timeout / lock loss / SIGTERM) fires. @@ -660,6 +665,10 @@ async function embedAllStale( let afterUpdatedAt: string | null = null; let totalChunksLoaded = 0; let budgetExitNotified = false; + // #1946 (OV2a): track chunks that errored out so a catch-up pass that finishes + // with stale chunks still remaining (un-embeddable for a non-transient reason) + // surfaces that loudly instead of looking like a clean run. + let embedFailures = 0; try { // eslint-disable-next-line no-constant-condition @@ -746,6 +755,7 @@ async function embedAllStale( // Budget/abort-fired cancellations are expected on the way out; don't // spam per-page "Error embedding" lines when we're shutting down. if (effectiveSignal.aborted) return; + embedFailures++; serr(`\n Error embedding ${slug}: ${e instanceof Error ? e.message : e}`); } totalProcessedPages++; @@ -772,10 +782,23 @@ async function embedAllStale( if (batch.length < PAGE_SIZE) break; } } finally { - clearTimeout(budgetTimer); + if (budgetTimer) clearTimeout(budgetTimer); } slog(`Embedded ${result.embedded} chunks across ${totalProcessedPages} pages`); + + // #1946 (OV2a): a catch-up pass that completed without being aborted but left + // chunks unembedded means those chunks are stuck (a non-transient embed + // failure), not that we ran out of time. Surface it loudly so it doesn't read + // as a clean run — re-running won't help until the underlying failure is fixed. + if (staleOpts?.catchUp && !effectiveSignal.aborted && embedFailures > 0) { + const remaining = await engine.countStaleChunks( + signature ? { signature, ...(sourceId ? { sourceId } : {}) } : (sourceId ? { sourceId } : undefined), + ); + if (remaining > 0) { + serr(`\n [embed] catch-up finished but ${remaining} chunk(s) remain stale after ${embedFailures} embed failure(s). These are not embeddable as-is; re-running won't clear them until the underlying error is resolved.`); + } + } } /** diff --git a/src/commands/frontmatter-install-hook.ts b/src/commands/frontmatter-install-hook.ts index 1477d28c1..6224a982f 100644 --- a/src/commands/frontmatter-install-hook.ts +++ b/src/commands/frontmatter-install-hook.ts @@ -39,7 +39,7 @@ if ! command -v gbrain >/dev/null 2>&1; then exit 0 fi -staged=$(git diff --cached --name-only --diff-filter=ACM | grep -E '\\\\.mdx?$' || true) +staged=$(git diff --cached --name-only --diff-filter=ACM | grep -E '\\.mdx?$' || true) [ -z "$staged" ] && exit 0 failed=0 diff --git a/src/commands/lint.ts b/src/commands/lint.ts index 2714c841d..e14e05456 100644 --- a/src/commands/lint.ts +++ b/src/commands/lint.ts @@ -46,6 +46,7 @@ const FRONTMATTER_RULE_NAMES: Record = { SLUG_MISMATCH: 'frontmatter-slug-mismatch', NULL_BYTES: 'frontmatter-null-bytes', NESTED_QUOTES: 'frontmatter-nested-quotes', + NON_STRING_FIELD: 'frontmatter-non-string-field', EMPTY_FRONTMATTER: 'frontmatter-empty', }; diff --git a/src/commands/sync.ts b/src/commands/sync.ts index 164389db7..e503b4e6c 100644 --- a/src/commands/sync.ts +++ b/src/commands/sync.ts @@ -47,11 +47,9 @@ import { clampWorkersForConnectionBudget, } from '../core/sync-concurrency.ts'; import { - tryAcquireDbLock, withRefreshingLock, LockUnavailableError, syncLockId, - SYNC_LOCK_ID, } from '../core/db-lock.ts'; import { withSourcePrefix, @@ -69,11 +67,14 @@ import { sortNewestFirst } from '../core/sort-newest-first.ts'; import { loadOpCheckpoint, recordCompleted, + appendCompleted, + appendCompletedOnce, clearOpCheckpoint, resumeFilter, syncFingerprint, type OpCheckpointKey, } from '../core/op-checkpoint.ts'; +import { registerCleanup } from '../core/process-cleanup.ts'; /** * v0.42.x (#1794) -- resumable incremental sync checkpoint. @@ -109,6 +110,8 @@ function syncCheckpointKeys( * import work on a kill (cheap -- content_hash short-circuits the re-import). */ const SYNC_CHECKPOINT_EVERY_DEFAULT = 1000; +const SYNC_CHECKPOINT_SECONDS_DEFAULT = 10; +const SYNC_MAX_CHECKPOINT_FAILURES_DEFAULT = 3; function resolveSyncCheckpointEvery(): number { const raw = process.env.GBRAIN_SYNC_CHECKPOINT_EVERY; @@ -117,6 +120,46 @@ function resolveSyncCheckpointEvery(): number { return Number.isInteger(n) && n > 0 ? n : SYNC_CHECKPOINT_EVERY_DEFAULT; } +/** + * v0.42.x (#1794): time-based flush ceiling. Bank the checkpoint at least every + * N seconds regardless of file throughput, so a kill loses at most ~N seconds of + * import work even when `checkpointEvery` files haven't accumulated yet. + */ +function resolveSyncCheckpointSeconds(): number { + const raw = process.env.GBRAIN_SYNC_CHECKPOINT_SECONDS; + if (!raw) return SYNC_CHECKPOINT_SECONDS_DEFAULT; + const n = Number(raw); + return Number.isInteger(n) && n > 0 ? n : SYNC_CHECKPOINT_SECONDS_DEFAULT; +} + +/** + * v0.42.x (#1794): how many consecutive checkpoint-flush failures (each already + * retried by withRetry across the ~12s Supavisor recovery window) before the + * sync aborts rather than burning CPU importing work it can never bank. + */ +function resolveSyncMaxCheckpointFailures(): number { + const raw = process.env.GBRAIN_SYNC_MAX_CHECKPOINT_FAILURES; + if (!raw) return SYNC_MAX_CHECKPOINT_FAILURES_DEFAULT; + const n = Number(raw); + return Number.isInteger(n) && n > 0 ? n : SYNC_MAX_CHECKPOINT_FAILURES_DEFAULT; +} + +const SYNC_YIELD_EVERY_DEFAULT = 64; + +/** + * v0.42.x (#1794): how many imported files between event-loop yields. The import + * loop is CPU-heavy (chunking, hashing); without yielding it starves the + * `withRefreshingLock` setInterval heartbeat, so the lock's `last_refreshed_at` + * never bumps, its TTL lapses mid-run, and a competing launch steals the live + * lock (the #1794 thrash). A `setImmediate` every N files lets the timer fire. + */ +function resolveSyncYieldEvery(): number { + const raw = process.env.GBRAIN_SYNC_YIELD_EVERY; + if (!raw) return SYNC_YIELD_EVERY_DEFAULT; + const n = Number(raw); + return Number.isInteger(n) && n > 0 ? n : SYNC_YIELD_EVERY_DEFAULT; +} + /** * v0.42.7 (#1696, D5): which terminal sync statuses warrant the end-of-sync * extraction-lag nudge. Fires on every non-error completion — crucially @@ -158,7 +201,15 @@ export interface SyncResult { * cron operators can disambiguate timeout vs pull-timeout in monitoring. */ filesImported?: number; - reason?: 'timeout' | 'pull_timeout'; + reason?: 'timeout' | 'pull_timeout' | 'checkpoint_unavailable'; + /** + * v0.42.x (#1794): cumulative file paths durably banked to the checkpoint + * across THIS run + prior resumed runs. Surfaced on every partial/blocked + * exit so an operator who kills a sync can see progress was banked — instead + * of reading only `last_commit` (unchanged by design) and concluding "lost + * everything," the exact misdiagnosis in the #1794 recurrence report. + */ + bankedFiles?: number; } /** @@ -737,32 +788,20 @@ export async function performSync(engine: BrainEngine, opts: SyncOpts): Promise< const lockKey = opts.lockId ?? syncLockId(opts.sourceId ?? 'default'); - // When `opts.sourceId` is set OR `opts.lockId` is explicitly overridden, - // use the TTL-refreshing lock so long sources stay safe. The default - // path (no sourceId, no lockId) keeps the bare tryAcquireDbLock for - // bit-for-bit back-compat with single-default-source brains. - const usePerSourcePath = opts.lockId !== undefined || opts.sourceId !== undefined; - - if (usePerSourcePath) { - try { - return await withRefreshingLock(engine, lockKey, () => performSyncInner(engine, opts)); - } catch (err) { - if (err instanceof LockUnavailableError) { - throw new SyncLockBusyError(await formatLockBusyMessage(engine, lockKey), lockKey); - } - throw err; - } - } - - // Legacy global-lock path (single-default-source brains). - const lockHandle = await tryAcquireDbLock(engine, lockKey); - if (!lockHandle) { - throw new SyncLockBusyError(await formatLockBusyMessage(engine, lockKey), lockKey); - } + // v0.42.x (#1794): ALL non-skipLock syncs use the TTL-refreshing lock — the + // bare `gbrain sync` path (no --source/--lockId) included. The pre-v0.42 code + // gave that path a NON-refreshing tryAcquireDbLock, so a long hand-run sync + // (exactly what you'd run during an incident on the 204K brain) could have its + // lock TTL lapse and be stolen mid-run. withRefreshingLock keeps the heartbeat + // alive (the import loop's event-loop yields ensure the timer fires), and the + // heartbeat-aware takeover refuses to steal a live, refreshing holder. try { - return await performSyncInner(engine, opts); - } finally { - try { await lockHandle.release(); } catch { /* best-effort release */ } + return await withRefreshingLock(engine, lockKey, () => performSyncInner(engine, opts)); + } catch (err) { + if (err instanceof LockUnavailableError) { + throw new SyncLockBusyError(await formatLockBusyMessage(engine, lockKey), lockKey); + } + throw err; } } @@ -1015,7 +1054,8 @@ function buildPartialResult(opts: { modified: number; deleted: number; renamed: number; - reason: 'timeout' | 'pull_timeout'; + reason: 'timeout' | 'pull_timeout' | 'checkpoint_unavailable'; + bankedFiles?: number; }): SyncResult { return { status: 'partial', @@ -1030,6 +1070,7 @@ function buildPartialResult(opts: { pagesAffected: opts.pagesAffected, filesImported: opts.filesImported, reason: opts.reason, + bankedFiles: opts.bankedFiles, }; } @@ -1246,21 +1287,50 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise isSyncable(r.from, syncOpts) && !isSyncable(r.to, syncOpts)) + .map(r => r.from); const filtered: SyncManifest = { added: manifest.added.filter(p => isSyncable(p, syncOpts)), modified: manifest.modified.filter(p => isSyncable(p, syncOpts)), - deleted: manifest.deleted.filter(p => isSyncable(p, syncOpts)), + deleted: unique([ + ...manifest.deleted.filter(p => isSyncable(p, syncOpts)), + ...renamedToUnsyncable, + ]), renamed: manifest.renamed.filter(r => isSyncable(r.to, syncOpts)), }; @@ -1467,25 +1564,82 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise(completedPaths); + const pendingCheckpointPaths = new Set(); + const checkpointSeconds = resolveSyncCheckpointSeconds(); + const maxFlushFailures = resolveSyncMaxCheckpointFailures(); let sinceFlush = 0; + let lastFlushAt = Date.now(); + let consecutiveFlushFailures = 0; + let bankedFiles = completedPaths.length; + let flushing = false; + let checkpointDead = false; + // Assigned at registration (after the pinPersisted gate); called on every + // normal return so a later operation's SIGTERM doesn't fire this stale flush. + let deregisterCheckpointCleanup: () => void = () => {}; const flushCheckpoint = async (): Promise => { - // `[...completed]` is a synchronous snapshot (atomic under concurrent - // worker `completed.add`), so a parallel flush can't observe a torn set. - await recordCompleted(engine, ckpt.paths, [...completed]); + if (pendingCheckpointPaths.size === 0 || flushing) return; + flushing = true; + // Synchronous swap (atomic under single-threaded JS): take the current + // pending set as this flush's batch; workers accumulate into a fresh set. + const batch = [...pendingCheckpointPaths]; + pendingCheckpointPaths.clear(); + try { + const ok = await appendCompleted(engine, ckpt.paths, batch); + if (ok) { + consecutiveFlushFailures = 0; + bankedFiles += batch.length; + } else { + // Not durably banked — re-merge so the next flush retries this batch. + for (const p of batch) pendingCheckpointPaths.add(p); + if (++consecutiveFlushFailures >= maxFlushFailures) checkpointDead = true; + } + } finally { + flushing = false; + } + }; + // v0.42.x (#1794): yield the event loop every N files so the refreshing-lock + // heartbeat timer can fire mid-import (otherwise the CPU loop starves it and + // the live lock gets stolen — the thrash this fixes). + const yieldEvery = resolveSyncYieldEvery(); + let sinceYield = 0; + const maybeYield = async (): Promise => { + if (++sinceYield >= yieldEvery) { + sinceYield = 0; + // setTimeout(0), NOT setImmediate: the lock-refresh heartbeat is a + // setInterval (timers phase). In Bun a tight setImmediate loop starves + // the timers phase, so the heartbeat would never fire. setTimeout(0) + // enters the timers phase where setInterval callbacks also run. + await new Promise((r) => setTimeout(r, 0)); + } }; const markCompleted = async (path: string): Promise => { completed.add(path); - if (++sinceFlush >= checkpointEvery) { + pendingCheckpointPaths.add(path); + const dueByCount = ++sinceFlush >= checkpointEvery; + const dueByTime = Date.now() - lastFlushAt >= checkpointSeconds * 1000; + const firstFile = completed.size === 1; // bank early on a fresh run + if (dueByCount || dueByTime || firstFile) { sinceFlush = 0; + lastFlushAt = Date.now(); await flushCheckpoint(); } }; @@ -1503,18 +1657,25 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise - buildPartialResult({ + // v0.41.13.0 (T2 + D-V3-1): closure for the partial-return path. + // v0.42.x (#1794): now ASYNC — it banks the unflushed delta before returning + // so a clean --timeout/SIGINT abort doesn't drop the last sub-cadence batch + // (best-effort; skipped when checkpointDead — the pool is gone). `reason` is + // overridden to 'checkpoint_unavailable' when the checkpoint died, and + // `bankedFiles` is surfaced so a killed run shows banked progress instead of + // looking like total loss. toCommit reports the PINNED target; last_commit is + // never advanced on a partial (the next run resumes from the checkpoint). + const partial = async (reason: 'timeout' | 'pull_timeout'): Promise => { + deregisterCheckpointCleanup(); + if (!checkpointDead) { + try { await flushCheckpoint(); } catch { /* best effort — we're aborting */ } + } + const banked = bankedFiles; + serr( + `[sync] banked ${banked} file(s) this run; next 'gbrain sync' resumes from ` + + `the checkpoint (last_commit unchanged at ${(lastCommit ?? '').slice(0, 8)}).`, + ); + return buildPartialResult({ fromCommit: lastCommit, toCommit: pin, filesImported, @@ -1524,8 +1685,30 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise { + await appendCompletedOnce(engine, ckpt.paths, [...pendingCheckpointPaths]); + }); // Per-file progress on stderr so agents see each step of a big sync. // Phases: sync.deletes, sync.renames, sync.imports. @@ -1598,7 +1781,7 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise; @@ -1725,7 +1909,7 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise= importsToDo.length) break; await importOnePath(eng, importsToDo[idx]); @@ -1960,7 +2146,7 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise relative(repoPath, abs)), + ); + const rows = await engine.executeRaw<{ slug: string; source_path: string | null }>( + `SELECT slug, source_path FROM pages WHERE source_id = $1 AND source_path IS NOT NULL AND deleted_at IS NULL`, + [sid], + ); + const staleSlugs = rows + .filter(r => r.source_path != null + && isSyncable(r.source_path, reconcileSyncOpts) + && !current.has(r.source_path)) + .map(r => r.slug); + if (staleSlugs.length > 0) { + const deleteScopedOpts = { sourceId: sid }; + for (let i = 0; i < staleSlugs.length; i += DELETE_BATCH_SIZE) { + const batch = staleSlugs.slice(i, i + DELETE_BATCH_SIZE); + try { + const deleted = await engine.deletePages(batch, deleteScopedOpts); + reconciledDeletes += deleted.length; + } catch { + // Per-slug fallback on a batch blip (mirrors the incremental delete + // loop). A stale page that won't delete is best-effort, not fatal. + for (const slug of batch) { + try { await engine.deletePage(slug, deleteScopedOpts); reconciledDeletes++; } + catch { /* best-effort */ } + } + } + } + if (reconciledDeletes > 0) { + slog(` Reconciled ${reconciledDeletes} stale page(s) whose source file was removed.`); + } + } + } + // Full sync doesn't track pagesAffected, so fall back to embed --stale. // v0.37 fix wave (Lane D.3 + CDX2-8): switched to runEmbedCore for the // same reason as the incremental path — surface dim-mismatch via hint @@ -2421,7 +2690,7 @@ async function performFullSync( toCommit: headCommit, added: result.imported, modified: 0, - deleted: 0, + deleted: reconciledDeletes, renamed: 0, chunksCreated: result.chunksCreated, embedded, diff --git a/src/core/content-sanity.ts b/src/core/content-sanity.ts index ca5260162..17a3616c9 100644 --- a/src/core/content-sanity.ts +++ b/src/core/content-sanity.ts @@ -376,9 +376,9 @@ export function assessContentSanity(opts: { // doesn't repeat the lowercase per literal. const bodyHead = body.slice(0, SCAN_HEAD_BYTES); const bodyHeadLower = bodyHead.toLowerCase(); - // Defensive coercion (issue #1939): this is a pure exported fn; lint.ts and - // import-file both pass `parsed.title`, which a malformed YAML date/number - // title could make non-string. Never throw on a bad title. + // Defensive coercion (issue #1939 / #1883 / #1658): this is a pure exported fn; + // lint.ts and import-file both pass `parsed.title`, which a malformed YAML + // date/number title could make non-string. Never throw on a bad title. const title = String(opts.title ?? ''); const titleLower = title.toLowerCase(); diff --git a/src/core/cycle.ts b/src/core/cycle.ts index 0660d940b..a4dce8ffe 100644 --- a/src/core/cycle.ts +++ b/src/core/cycle.ts @@ -905,6 +905,21 @@ async function runPhaseSync( pagesAffected: result.pagesAffected, }; } catch (e) { + // v0.42.x (#1794): a single-flight collision — another sync already holds + // the per-source lock — is NOT a phase failure. The other run is doing the + // work; surfacing 'fail' would paint a healthy cron contention red and (with + // the heartbeat-aware takeover) this is now the expected outcome when a long + // sync overruns into the next cron tick. Report it as a skip. + const { SyncLockBusyError } = await import('../commands/sync.ts'); + if (e instanceof SyncLockBusyError) { + return { + phase: 'sync', + status: 'skipped', + duration_ms: 0, + summary: 'sync already in progress elsewhere — skipped', + details: { syncStatus: 'lock_busy' }, + }; + } return { phase: 'sync', status: 'fail', diff --git a/src/core/db-lock.ts b/src/core/db-lock.ts index dd5de5dc7..436b59e16 100644 --- a/src/core/db-lock.ts +++ b/src/core/db-lock.ts @@ -42,6 +42,30 @@ const DEFAULT_TTL_MINUTES = 30; */ export const HOLDER_TAKEOVER_GRACE_MS = 60_000; +/** + * v0.42.x (#1794): heartbeat-aware steal grace. A holder whose + * `last_refreshed_at` is within this window is treated as ALIVE and is NOT + * stolen even if its `ttl_expires_at` has lapsed — defending a live, actively + * refreshing holder whose refresh tick was briefly starved (the #1794 thrash, + * where a CPU-bound import let the TTL expire and a competing launch stole the + * live lock). A genuinely dead holder stops refreshing, ages past the grace, + * and becomes stealable again (TTL stays the ultimate backstop). Derived from + * the TTL so it scales with the refresh cadence; override with + * GBRAIN_LOCK_STEAL_GRACE_SECONDS. + */ +export const DEFAULT_STEAL_GRACE_SECONDS = 600; + +export function resolveStealGraceSeconds(ttlMinutes: number): number { + const raw = process.env.GBRAIN_LOCK_STEAL_GRACE_SECONDS; + if (raw) { + const n = Number(raw); + if (Number.isInteger(n) && n > 0) return n; + } + // Refresh fires ~ttl/6; protect a holder that refreshed within ~2 ticks. + const refreshSec = Math.max(15, (ttlMinutes * 60) / 6); + return Math.max(Math.floor(refreshSec * 2), 60); +} + /** * Liveness classification of a lock holder, from the perspective of the * current host. Shared by `isHolderDeadLocally` (auto-takeover in @@ -130,6 +154,9 @@ export async function tryAcquireDbLock( ): Promise { const pid = process.pid; const host = hostname(); + // v0.42.x (#1794): a holder that refreshed within this window is protected + // from the ON CONFLICT steal even if its TTL lapsed (starved-but-alive). + const stealGraceSeconds = resolveStealGraceSeconds(ttlMinutes); // Engine-agnostic: prefer the engine's raw escape hatch (`sql` for postgres-js, // `db.query` for PGLite). Mirrors cycle.ts's pattern so behavior stays identical. @@ -166,6 +193,8 @@ export async function tryAcquireDbLock( ttl_expires_at = NOW() + ${ttl}::interval, last_refreshed_at = NOW() WHERE gbrain_cycle_locks.ttl_expires_at < NOW() + AND (gbrain_cycle_locks.last_refreshed_at IS NULL + OR gbrain_cycle_locks.last_refreshed_at < NOW() - ${stealGraceSeconds} * INTERVAL '1 second') RETURNING id `; if (rows.length === 0) return null; @@ -179,14 +208,16 @@ export async function tryAcquireDbLock( id: lockId, refresh: async () => { // v0.41.13.0: bump BOTH ttl_expires_at AND last_refreshed_at. - // Without last_refreshed_at, --max-age would steal healthy locks - // whose acquired_at is old but whose holder is alive and refreshing. - await sql` - UPDATE gbrain_cycle_locks - SET ttl_expires_at = NOW() + ${ttl}::interval, - last_refreshed_at = NOW() - WHERE id = ${lockId} AND holder_pid = ${pid} - `; + // v0.42.x (#1794): route through the DIRECT session pool, not the + // transaction pool, so a Supavisor pooler exhaustion (EMAXCONNSESSION) + // can't kill the heartbeat and let the live lock get stolen. + await engine.executeRawDirect( + `UPDATE gbrain_cycle_locks + SET ttl_expires_at = NOW() + ($1)::interval, + last_refreshed_at = NOW() + WHERE id = $2 AND holder_pid = $3`, + [ttl, lockId, pid], + ); }, release: async () => { deregister(); @@ -211,8 +242,10 @@ export async function tryAcquireDbLock( ttl_expires_at = NOW() + $4::interval, last_refreshed_at = NOW() WHERE gbrain_cycle_locks.ttl_expires_at < NOW() + AND (gbrain_cycle_locks.last_refreshed_at IS NULL + OR gbrain_cycle_locks.last_refreshed_at < NOW() - $5 * INTERVAL '1 second') RETURNING id`, - [lockId, pid, host, ttl], + [lockId, pid, host, ttl, stealGraceSeconds], ); if (rows.length === 0) return null; const deregister = registerCleanup(`db-lock:${lockId}`, async () => { @@ -736,27 +769,25 @@ export async function withRefreshingLock( const interval = setInterval(() => { void (async () => { try { - // A4 heartbeat: SELECT 1 against the engine's connection pool. - // Honest limit: this checks a connection is responsive in general, - // not the SPECIFIC backend running `work()`. The full X1 fix - // (lock-refresh on the work-pinned connection via withReservedConnection) - // is layered in by callers that pass the work backend's sql in. - // For migrate.ts (transactional DDL), the engine.transaction() path - // pins the backend; the heartbeat against engine.sql is a useful - // proxy for "Postgres is reachable" even if it can race the actual - // backend's wedge state. Lane B's primary win is the auto-refresh - // itself; the precise-backend-bind heartbeat is a Lane B follow-up. - const probe = engineSelectOne(engine); + // v0.42.x (#1794, V1): the refresh IS the heartbeat. handle.refresh() + // routes through the DIRECT session pool (postgres), so it survives a + // transaction-pool exhaustion (EMAXCONNSESSION) that would otherwise + // kill renewal and let the live lock be stolen. The pre-v0.42 code first + // probed `SELECT 1` on the READ pool and clearInterval'd on probe + // failure — that's exactly how an exhausted read pool stopped renewal + // even though the lock was alive. We no longer gate renewal on read-pool + // health, and we do NOT clearInterval on a transient failure: a blip + // self-heals on the next tick; the TTL is the backstop if the pool stays + // genuinely dead (at which point a steal is correct). const timeout = new Promise((_, reject) => - setTimeout(() => reject(new Error('heartbeat_timeout')), heartbeatTimeoutMs) + setTimeout(() => reject(new Error('refresh_timeout')), heartbeatTimeoutMs) ); - await Promise.race([probe, timeout]); - await handle.refresh(); + await Promise.race([handle.refresh(), timeout]); + healthOk = true; } catch (err) { const msg = err instanceof Error ? err.message : String(err); - process.stderr.write(`[lock-refresh] ${lockId}: ${msg}; lock will auto-expire\n`); + process.stderr.write(`[lock-refresh] ${lockId}: ${msg}; will retry next tick\n`); healthOk = false; - clearInterval(interval); } })(); }, refreshIntervalMs); @@ -778,24 +809,6 @@ export async function withRefreshingLock( } } -/** Internal: SELECT 1 on the engine's connection. */ -async function engineSelectOne(engine: BrainEngine): Promise { - const maybePG = engine as unknown as { sql?: (...args: unknown[]) => Promise }; - const maybePGLite = engine as unknown as { - db?: { query: (sql: string) => Promise<{ rows: unknown[] }> }; - }; - if (engine.kind === 'postgres' && maybePG.sql) { - const sql = maybePG.sql as any; - await sql`SELECT 1`; - return; - } - if (engine.kind === 'pglite' && maybePGLite.db) { - await maybePGLite.db.query('SELECT 1'); - return; - } - throw new Error(`Unknown engine kind for heartbeat: ${engine.kind}`); -} - /** * v0.41 Eng D9 (codex pass-2 #7 + #8) — per-tick election convenience. * diff --git a/src/core/markdown.ts b/src/core/markdown.ts index 9a6359b6b..79cef3df0 100644 --- a/src/core/markdown.ts +++ b/src/core/markdown.ts @@ -10,6 +10,7 @@ export type ParseValidationCode = | 'SLUG_MISMATCH' | 'NULL_BYTES' | 'NESTED_QUOTES' + | 'NON_STRING_FIELD' | 'EMPTY_FRONTMATTER'; export interface ParseValidationError { @@ -124,6 +125,13 @@ export function parseMarkdown( const { compiled_truth, timeline } = splitBody(body); + // #1948/#1939: frontmatter values can be non-strings (YAML coerces `title: 123` + // → number, a bare date → Date). The `as string` cast used to lie: a truthy + // non-string flowed downstream typed as string and crashed the first + // `.toLowerCase()` (content-sanity), aborting the whole lint/sync run. + // coerceFrontmatterString turns a scalar/date into a usable string (a date slug + // `2024-06-01` is legitimate); the NON_STRING_FIELD lint finding below still + // surfaces the un-quoted field so it can be cleaned up. const type = coerceFrontmatterString(frontmatter.type) || ( opts?.activePack ? inferTypeFromPack(filePath, opts.activePack) : inferType(filePath) ); @@ -308,6 +316,21 @@ function collectValidationErrors( }); } } + + // 8. NON_STRING_FIELD (#1948) — title/type/slug declared as a non-string YAML + // scalar (e.g. `title: 123`, `slug: 2024`). The parser coerces title to a + // string and falls back to inference for type/slug, but lint surfaces the + // malformed frontmatter so it gets fixed rather than silently rewritten. + // Pre-fix the slug validator above `typeof`-skipped these, hiding them. + for (const field of ['title', 'type', 'slug'] as const) { + const v = ctx.parsedFrontmatter[field]; + if (v != null && typeof v !== 'string') { + errors.push({ + code: 'NON_STRING_FIELD', + message: `Frontmatter "${field}" should be a string but is ${typeof v} (${JSON.stringify(v)}); quote the value (e.g. ${field}: "${String(v)}").`, + }); + } + } } /** diff --git a/src/core/migrate.ts b/src/core/migrate.ts index 929f8a368..2e31e60ad 100644 --- a/src/core/migrate.ts +++ b/src/core/migrate.ts @@ -5160,6 +5160,32 @@ export const MIGRATIONS: Migration[] = [ `, }, }, + { + version: 115, + name: 'op_checkpoint_paths_append_table', + // #1794 cathedral: append-only delta storage for op checkpoints. The parent + // op_checkpoints.completed_keys JSONB was rewritten in full on every flush — + // O(N^2) write bytes over a 204K-file sync. This child table banks one row + // per completed path; sync's appendCompleted INSERTs only the delta. The FK + // ON DELETE CASCADE makes clearOpCheckpoint + the 7-day purge drop children + // automatically. Created empty so the composite-PK index build is instant; + // no CONCURRENTLY / transaction:false needed (mirrors v75 op_checkpoints). + // The PK (op,fingerprint,path) btree's (op,fingerprint) prefix serves every + // read/delete, so no separate index. Keep in sync with src/schema.sql, + // src/core/pglite-schema.ts, src/core/schema-embedded.ts. + idempotent: true, + sql: ` + CREATE TABLE IF NOT EXISTS op_checkpoint_paths ( + op TEXT NOT NULL, + fingerprint TEXT NOT NULL, + path TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + PRIMARY KEY (op, fingerprint, path), + CONSTRAINT op_checkpoint_paths_parent_fk + FOREIGN KEY (op, fingerprint) REFERENCES op_checkpoints (op, fingerprint) ON DELETE CASCADE + ); + `, + }, ]; export const LATEST_VERSION = MIGRATIONS.length > 0 diff --git a/src/core/op-checkpoint.ts b/src/core/op-checkpoint.ts index 0aa490668..a338d20b6 100644 --- a/src/core/op-checkpoint.ts +++ b/src/core/op-checkpoint.ts @@ -1,5 +1,52 @@ import { createHash } from 'crypto'; import type { BrainEngine } from './engine.ts'; +import { withRetry, BULK_RETRY_OPTS, RetryAbortError } from './retry.ts'; + +/** Max paths per append-INSERT round-trip; bounds the param-array size. */ +const APPEND_CHUNK = 1000; + +/** + * Single writable-CTE statement (one round-trip): ensure the parent + * op_checkpoints row exists (FK target) and bump its updated_at so the 7-day + * purge tracks activity, then INSERT the delta child rows. `ON CONFLICT DO + * NOTHING` makes re-appending an already-banked path a no-op. $3 binds a JS + * string[] to a Postgres text[] (NOT a jsonb param), which postgres.js + PGLite + * both handle natively — so it sidesteps executeRawJsonb's array rejection. + */ +const APPEND_PATHS_SQL = `WITH parent AS ( + INSERT INTO op_checkpoints (op, fingerprint, completed_keys, updated_at) + VALUES ($1, $2, '[]'::jsonb, now()) + ON CONFLICT (op, fingerprint) DO UPDATE SET updated_at = now() +) +INSERT INTO op_checkpoint_paths (op, fingerprint, path) +SELECT $1, $2, unnest($3::text[]) +ON CONFLICT (op, fingerprint, path) DO NOTHING`; + +/** + * v0.42.x (#1794): every checkpoint write routes through the DIRECT session + * pool (`executeRawDirect`) wrapped in `withRetry(BULK_RETRY_OPTS)`. Rationale: + * under Supavisor transaction-pooler exhaustion (`EMAXCONNSESSION` / SQLSTATE + * 53300) the write competes with import workers for the same dead pool. The + * direct pool bypasses that, and retry rides out the 5-10s recovery window. + * Returns `true` if the write landed, `false` if it failed after retries (the + * caller — sync's fail-loud counter — decides whether to abort). A + * `RetryAbortError` (signal mid-sleep) is re-thrown, NOT counted as a failure. + */ +async function durableWrite( + engine: BrainEngine, + key: OpCheckpointKey, + label: string, + fn: () => Promise, +): Promise { + try { + await withRetry(fn, BULK_RETRY_OPTS); + return true; + } catch (e) { + if (e instanceof RetryAbortError) throw e; + console.error(`[op-checkpoint] ${label} failed (${key.op}, ${key.fingerprint}):`, (e as Error).message); + return false; + } +} /** * Shared checkpoint primitive for long-running ops (embed, extract, lint, @@ -69,17 +116,25 @@ export async function loadOpCheckpoint( key: OpCheckpointKey, ): Promise { try { - const rows = await engine.executeRaw<{ completed_keys: unknown }>( - `SELECT completed_keys FROM op_checkpoints - WHERE op = $1 AND fingerprint = $2`, + // v0.42.x (#1794): union the new append-only child rows (op_checkpoint_paths) + // with the legacy `completed_keys` JSONB array (recordCompleted consumers + + // pre-upgrade rows). UNION ALL — not UNION — because the JS Set below already + // dedupes, so we skip a server-side dedup sort over up to 204K rows on every + // resume. `jsonb_array_elements_text` expands the legacy array server-side, + // which also removes the old postgres.js-vs-PGLite string/array handling. + const rows = await engine.executeRaw<{ ckey: unknown }>( + `SELECT path AS ckey FROM op_checkpoint_paths + WHERE op = $1 AND fingerprint = $2 + UNION ALL + SELECT jsonb_array_elements_text(completed_keys) AS ckey FROM op_checkpoints + WHERE op = $1 AND fingerprint = $2`, [key.op, key.fingerprint], ); - if (rows.length === 0) return []; - const raw = rows[0]?.completed_keys; - // postgres.js returns JSONB as JS arrays; PGLite returns strings. Handle both. - const parsed = typeof raw === 'string' ? JSON.parse(raw) : raw; - if (!Array.isArray(parsed)) return []; - return parsed.filter((k): k is string => typeof k === 'string'); + const set = new Set(); + for (const r of rows) { + if (typeof r.ckey === 'string') set.add(r.ckey); + } + return [...set]; } catch (e) { console.error(`[op-checkpoint] load failed (${key.op}, ${key.fingerprint}):`, (e as Error).message); return []; @@ -98,22 +153,72 @@ export async function recordCompleted( engine: BrainEngine, key: OpCheckpointKey, keys: string[], -): Promise { - try { - // Sorted serialization keeps diff-based debug output stable and tests - // deterministic across insertion order shuffles. - const sorted = [...keys].sort(); - await engine.executeRaw( +): Promise { + // REPLACE semantics (kept deliberately — #1794 V3). Callers like + // extract-conversation-facts serialize a MUTABLE map through here and rely on + // stale keys being REMOVED; an append would make them unremovable. The full + // set lands in the parent `completed_keys` JSONB column via a single UPSERT — + // exactly as before. JSON.stringify into `$3::jsonb` is correct (the text→jsonb + // cast yields a proper array; NOT the double-encode trap, which is the template + // form). Sync uses `appendCompleted` (below) instead, never this. + const sorted = [...keys].sort(); + return durableWrite(engine, key, 'write', () => + engine.executeRawDirect( `INSERT INTO op_checkpoints (op, fingerprint, completed_keys, updated_at) VALUES ($1, $2, $3::jsonb, now()) ON CONFLICT (op, fingerprint) DO UPDATE SET completed_keys = EXCLUDED.completed_keys, updated_at = now()`, [key.op, key.fingerprint, JSON.stringify(sorted)], - ); + )); +} + +/** + * v0.42.x (#1794): ADDITIVE delta append — used ONLY by resumable sync. INSERTs + * just the new paths into `op_checkpoint_paths` (one row each) instead of + * rewriting the whole set, killing the O(N²) write amplification of a 204K-file + * sync. A single writable-CTE statement (one round-trip) ensures the parent + * `op_checkpoints` row exists (FK target) and bumps its `updated_at` so the + * 7-day purge tracks activity, then inserts the children. `ON CONFLICT DO + * NOTHING` makes re-appending an already-banked path a no-op, so a cold resume + * that re-sends a banked batch costs nothing. Returns false if any chunk's write + * fails after retries (caller's fail-loud counter decides whether to abort). + */ +export async function appendCompleted( + engine: BrainEngine, + key: OpCheckpointKey, + deltaKeys: string[], +): Promise { + if (deltaKeys.length === 0) return true; + for (let i = 0; i < deltaKeys.length; i += APPEND_CHUNK) { + const chunk = deltaKeys.slice(i, i + APPEND_CHUNK); + const ok = await durableWrite(engine, key, 'append', () => + engine.executeRawDirect(APPEND_PATHS_SQL, [key.op, key.fingerprint, chunk])); + if (!ok) return false; + } + return true; +} + +/** + * v0.42.x (#1794): NO-RETRY single-shot variant of appendCompleted for the + * SIGTERM cleanup path. The process-cleanup registry kills callbacks at a 3s + * deadline, which is shorter than withRetry's ~12s budget — a retrying flush + * would be cut off mid-retry and bank nothing. This does ONE direct write of + * the whole delta (no chunking, no retry) so it banks what it can inside the + * shutdown window. Best-effort: returns false (logged) on failure. + */ +export async function appendCompletedOnce( + engine: BrainEngine, + key: OpCheckpointKey, + deltaKeys: string[], +): Promise { + if (deltaKeys.length === 0) return true; + try { + await engine.executeRawDirect(APPEND_PATHS_SQL, [key.op, key.fingerprint, deltaKeys]); + return true; } catch (e) { - console.error(`[op-checkpoint] write failed (${key.op}, ${key.fingerprint}):`, (e as Error).message); - /* non-fatal: lost checkpoint just means re-walk on next run */ + console.error(`[op-checkpoint] sigterm-append failed (${key.op}, ${key.fingerprint}):`, (e as Error).message); + return false; } } @@ -128,15 +233,21 @@ export async function clearOpCheckpoint( engine: BrainEngine, key: OpCheckpointKey, ): Promise { - try { - await engine.executeRaw( + // Delete the parent (FK ON DELETE CASCADE drops the child rows), then a + // belt-and-suspenders child delete for any rows whose parent was somehow + // absent. Both routed through the direct pool + retry so a clean-exit clear + // survives pool exhaustion (a swallowed clear would make the next run skip + // already-cleared files). + await durableWrite(engine, key, 'clear', () => + engine.executeRawDirect( `DELETE FROM op_checkpoints WHERE op = $1 AND fingerprint = $2`, [key.op, key.fingerprint], - ); - } catch (e) { - console.error(`[op-checkpoint] clear failed (${key.op}, ${key.fingerprint}):`, (e as Error).message); - /* non-fatal */ - } + )); + await durableWrite(engine, key, 'clear-children', () => + engine.executeRawDirect( + `DELETE FROM op_checkpoint_paths WHERE op = $1 AND fingerprint = $2`, + [key.op, key.fingerprint], + )); } /** @@ -351,6 +462,9 @@ export async function purgeStaleCheckpoints( ttlDays = 7, ): Promise { try { + // Delete stale parents; the op_checkpoint_paths FK (ON DELETE CASCADE) + // drops their child rows automatically. The FK also guarantees no child + // can exist without a parent, so there are no orphans to sweep separately. const rows = await engine.executeRaw<{ count: string | number }>( `WITH deleted AS ( DELETE FROM op_checkpoints diff --git a/src/core/operations.ts b/src/core/operations.ts index 8739a2232..aa6f41e41 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -425,6 +425,91 @@ export function sourceScopeOpts(ctx: OperationContext): { sourceId?: string; sou return {}; } +/** + * Resolve a per-call requested source scope against the caller's trust + grant. + * FAIL-CLOSED: anything not strictly `ctx.remote === false` is untrusted. + * + * This is the SINGLE resolver for every read op that accepts a per-call + * `source_id` / `all_sources` parameter (query, code_callers, code_callees, + * get_page, search_by_image, code_blast, code_flow). Inlining the `__all__` + * branch per handler is the bug class that leaked cross-source reads (#1924, + * #1371): a remote client could pass `source_id: '__all__'` to opt out of its + * grant, or pass an explicit out-of-grant `source_id` that was never checked. + * + * - `__all__` / `all_sources`: + * trusted local (remote === false) → `{}` (spans the whole brain) + * remote → the caller's grant (sourceScopeOpts) + * - explicit `source_id`: + * remote + federated grant that doesn't include it → permission_denied + * otherwise → `{ sourceId }` + * - neither → the caller's grant (sourceScopeOpts). + * + * `code_traversal_cache_clear` is intentionally NOT a caller — it is localOnly + * and carries its own destructive D8 all_sources guard. + */ +export function resolveRequestedScope( + ctx: OperationContext, + sourceIdParam: string | undefined, + allSourcesParam = false, +): { sourceId?: string; sourceIds?: string[] } { + const wantsAll = allSourcesParam || sourceIdParam === '__all__'; + if (wantsAll) { + return ctx.remote === false ? {} : sourceScopeOpts(ctx); + } + if (sourceIdParam !== undefined) { + const allowed = ctx.auth?.allowedSources; + if (ctx.remote !== false && allowed && allowed.length > 0 && !allowed.includes(sourceIdParam)) { + throw new OperationError( + 'permission_denied', + `source '${sourceIdParam}' is outside your granted sources`, + 'Request access to this source, or omit source_id to search within your grant.', + ); + } + return { sourceId: sourceIdParam }; + } + return sourceScopeOpts(ctx); +} + +/** + * Code-intel adapter for `resolveRequestedScope`. Graph traversal + * (code_callers/code_callees/code_blast/code_flow) is single-source by design — + * the engine APIs and the traversal cache key take ONE `sourceId` string, not a + * federated array. So this collapses the resolver's output to `{allSources, + * sourceId}`, fail-closed: + * + * - resolver → one source (scalar or single-element grant) → that source + * - resolver → multi-source grant (federated remote client) → reject: ask the + * caller to specify which granted source (we must not silently span all) + * - resolver → empty scope → `allSources` ONLY for trusted local callers; a + * remote caller with no source in scope is denied, never widened to all. + */ +export function resolveCodeIntelScope( + ctx: OperationContext, + sourceIdParam: string | undefined, + allSourcesParam = false, +): { allSources: boolean; sourceId?: string } { + const scope = resolveRequestedScope(ctx, sourceIdParam, allSourcesParam); + if (scope.sourceId) return { allSources: false, sourceId: scope.sourceId }; + if (scope.sourceIds && scope.sourceIds.length === 1) { + return { allSources: false, sourceId: scope.sourceIds[0] }; + } + if (scope.sourceIds && scope.sourceIds.length > 1) { + throw new OperationError( + 'invalid_params', + 'Code traversal runs against a single source. Specify source_id (one of your granted sources).', + 'Pass source_id=.', + ); + } + // Empty scope: span everything only for trusted local callers; a remote caller + // that reached here has no source in scope and must NOT get cross-source results. + if (ctx.remote === false) return { allSources: true, sourceId: undefined }; + throw new OperationError( + 'permission_denied', + 'No source in scope for this request.', + 'Specify source_id, or check your granted sources.', + ); +} + /** * T4/D5 — resolve a per-call search-mode override. Honored ONLY for trusted/ * local callers (ctx.remote === false) so a remote OAuth client can't escalate @@ -524,19 +609,14 @@ const get_page: Operation = { const slug = p.slug as string; const fuzzy = (p.fuzzy as boolean) || false; const includeDeleted = (p.include_deleted as boolean) === true; - // v0.31.8 (D20): thread ctx.sourceId through read-side ops. Only pass - // sourceId when it's set on ctx — when unset (local CLI default chain - // resolves to no source), the engine two-branch query falls through to - // the cross-source view, preserving pre-v0.31.8 behavior. MCP callers - // (stdio + HTTP) populate ctx.sourceId via the transport layer. - const sourceOpts = ctx.sourceId ? { sourceId: ctx.sourceId } : {}; - // v0.41.13 #1436: fuzzy resolveSlugs ALSO needs source scope — pre-fix - // it was unscoped, so a remote `get_page` with `fuzzy: true` could - // return candidates from sources outside ctx.auth.allowedSources / - // ctx.sourceId. sourceScopeOpts(ctx) is the canonical precedence - // ladder (federated array > scalar > nothing) shared with every other - // read-side handler. - const fuzzyScope = sourceScopeOpts(ctx); + // #1393: route BOTH the exact-match read and the fuzzy resolveSlugs through + // the canonical precedence ladder (federated array > scalar > nothing). The + // exact path previously used scalar `ctx.sourceId` only, so a remote client + // with a federated `allowedSources` grant (and no single ctx.sourceId) got + // an UNSCOPED exact lookup — a cross-source read of any page by slug. getPage + // now honors sourceIds[] (both engines), so the same scope closes both paths. + const sourceOpts = sourceScopeOpts(ctx); + const fuzzyScope = sourceOpts; let page = await ctx.engine.getPage(slug, { includeDeleted, ...sourceOpts }); let resolved_slug: string | undefined; @@ -1398,7 +1478,7 @@ const query: Operation = { source_id: { type: 'string', description: - "v0.34: scope search to a single source. Defaults to OperationContext.sourceId (set from CLI --source / GBRAIN_SOURCE / .gbrain-source dotfile). Pass '__all__' to force cross-source search in multi-source brains.", + "v0.34: scope search to a single source. Defaults to OperationContext.sourceId (set from CLI --source / GBRAIN_SOURCE / .gbrain-source dotfile). Pass '__all__' to span every source for trusted local callers; for remote callers '__all__' spans only your granted sources.", }, cross_modal: { type: 'string', @@ -1448,15 +1528,14 @@ const query: Operation = { typeof p.embedding_column === 'string' && p.embedding_column.length > 0 ? (p.embedding_column as string) : undefined; - // Explicit per-call source_id must win over ctx.sourceId. The special - // __all__ value opts out of source filtering for local cross-source search. + // Explicit per-call source_id must win over ctx.sourceId. `__all__` spans + // every source for trusted local callers, but only the caller's granted + // sources for remote callers (resolveRequestedScope is the single + // trust+grant resolver shared by every source-scoped read op). This scope + // is spread into BOTH the image-similarity searchVector path and the text + // hybridSearch path below, so both honor the same grant. const sourceIdParam = typeof p.source_id === 'string' ? p.source_id : undefined; - const querySourceScope = - sourceIdParam !== undefined - ? sourceIdParam === '__all__' - ? {} - : { sourceId: sourceIdParam } - : sourceScopeOpts(ctx); + const querySourceScope = resolveRequestedScope(ctx, sourceIdParam); // v0.27.1: image-similarity branch. Bypasses hybridSearch (which is // text-only); embeds the image via embedMultimodal and runs a direct @@ -3790,21 +3869,17 @@ const code_callers: Operation = { params: { symbol: { type: 'string', required: true, description: 'Symbol to find callers of (bare or qualified name).' }, limit: { type: 'number', description: 'Max edges returned. Default 100.' }, - source_id: { type: 'string', description: "Scope to a single source. Defaults to ctx.sourceId; pass '__all__' to force cross-source." }, - all_sources: { type: 'boolean', description: 'Force cross-source search (equivalent to source_id=__all__).' }, + source_id: { type: 'string', description: "Scope to a single source. Defaults to ctx.sourceId; '__all__' spans every source for trusted local callers, your granted sources for remote callers." }, + all_sources: { type: 'boolean', description: 'Span sources (equivalent to source_id=__all__): every source locally, your grant remotely.' }, }, scope: 'read', handler: async (ctx, p) => { const symbol = p.symbol as string; const limit = (p.limit as number) ?? 100; - const allSourcesParam = p.all_sources === true; const sourceIdParam = typeof p.source_id === 'string' ? p.source_id : undefined; - const allSources = allSourcesParam || sourceIdParam === '__all__'; - const sourceId = allSources - ? undefined - : sourceIdParam !== undefined - ? sourceIdParam - : ctx.sourceId; + // Single trust+grant resolver: remote callers can't span sources outside + // their grant, and `__all__` collapses to their grant (not the whole brain). + const { allSources, sourceId } = resolveCodeIntelScope(ctx, sourceIdParam, p.all_sources === true); const edges = await ctx.engine.getCallersOf(symbol, { limit, allSources, @@ -3825,21 +3900,16 @@ const code_callees: Operation = { params: { symbol: { type: 'string', required: true, description: 'Symbol to find callees of (bare or qualified name).' }, limit: { type: 'number', description: 'Max edges returned. Default 100.' }, - source_id: { type: 'string', description: "Scope to a single source. Defaults to ctx.sourceId; pass '__all__' to force cross-source." }, - all_sources: { type: 'boolean', description: 'Force cross-source search.' }, + source_id: { type: 'string', description: "Scope to a single source. Defaults to ctx.sourceId; '__all__' spans every source for trusted local callers, your granted sources for remote callers." }, + all_sources: { type: 'boolean', description: 'Span sources: every source locally, your grant remotely.' }, }, scope: 'read', handler: async (ctx, p) => { const symbol = p.symbol as string; const limit = (p.limit as number) ?? 100; - const allSourcesParam = p.all_sources === true; const sourceIdParam = typeof p.source_id === 'string' ? p.source_id : undefined; - const allSources = allSourcesParam || sourceIdParam === '__all__'; - const sourceId = allSources - ? undefined - : sourceIdParam !== undefined - ? sourceIdParam - : ctx.sourceId; + // Single trust+grant resolver (see code_callers). + const { allSources, sourceId } = resolveCodeIntelScope(ctx, sourceIdParam, p.all_sources === true); const edges = await ctx.engine.getCalleesOf(symbol, { limit, allSources, @@ -3910,6 +3980,7 @@ const code_blast: Operation = { depth: { type: 'number', description: 'Hop cap (default 5, max 8)' }, max_nodes: { type: 'number', description: 'Result-set cap (default 200)' }, exact: { type: 'boolean', description: 'Skip bare-name disambiguation; treat symbol as exact qualified name' }, + source_id: { type: 'string', description: 'Source to traverse. Defaults to ctx.sourceId; federated clients with multiple granted sources must specify one.' }, }, scope: 'read', handler: async (ctx, p) => { @@ -3919,14 +3990,20 @@ const code_blast: Operation = { const depth = Math.min((p.depth as number) ?? 5, 8); const max_nodes = Math.min((p.max_nodes as number) ?? 200, 200); const exact = (p.exact as boolean) ?? false; + // Single trust+grant resolver: a remote federated client can't traverse a + // source outside its grant (pre-fix this scoped by bare ctx.sourceId only). + // Falls back to ctx.sourceId (a required string) for the trusted-local case, + // exactly preserving pre-fix local behavior. + const { sourceId: scopedSourceId } = resolveCodeIntelScope(ctx, typeof p.source_id === 'string' ? p.source_id : undefined); + const sourceId = scopedSourceId ?? ctx.sourceId; return getCachedOrCompute( ctx.engine, - { symbol_qualified: symbol, depth, source_id: ctx.sourceId }, + { symbol_qualified: symbol, depth, source_id: sourceId }, () => runRecursiveWalk(ctx.engine, symbol, { direction: 'callers', depth, maxNodes: max_nodes, - sourceId: ctx.sourceId, + sourceId, exact, }), ); @@ -3942,6 +4019,7 @@ const code_flow: Operation = { depth: { type: 'number', description: 'Hop cap (default 8, max 12)' }, max_nodes: { type: 'number', description: 'Result-set cap (default 200)' }, exact: { type: 'boolean', description: 'Skip bare-name disambiguation' }, + source_id: { type: 'string', description: 'Source to traverse. Defaults to ctx.sourceId; federated clients with multiple granted sources must specify one.' }, }, scope: 'read', handler: async (ctx, p) => { @@ -3951,14 +4029,17 @@ const code_flow: Operation = { const depth = Math.min((p.depth as number) ?? 8, 12); const max_nodes = Math.min((p.max_nodes as number) ?? 200, 200); const exact = (p.exact as boolean) ?? false; + // Single trust+grant resolver (see code_blast). + const { sourceId: scopedSourceId } = resolveCodeIntelScope(ctx, typeof p.source_id === 'string' ? p.source_id : undefined); + const sourceId = scopedSourceId ?? ctx.sourceId; return getCachedOrCompute( ctx.engine, - { symbol_qualified: symbol + ':flow', depth, source_id: ctx.sourceId }, + { symbol_qualified: symbol + ':flow', depth, source_id: sourceId }, () => runRecursiveWalk(ctx.engine, symbol, { direction: 'callees', depth, maxNodes: max_nodes, - sourceId: ctx.sourceId, + sourceId, exact, }), ); @@ -3979,6 +4060,9 @@ const code_traversal_cache_clear: Operation = { scope: 'admin', localOnly: true, handler: async (ctx, p) => { + // INTENTIONAL exemption from resolveRequestedScope: this is a localOnly + // admin/destructive op with its own D8 all_sources guard. The read-side + // trust+grant resolver does not apply here (no remote caller reaches it). const { clearTraversalCache } = await import('./code-intel/traversal-cache.ts'); const sourceId = (p.source_id as string | undefined) ?? ctx.sourceId; const allSources = (p.all_sources as boolean) ?? false; @@ -4011,7 +4095,7 @@ const search_by_image: Operation = { query: { type: 'string', description: 'Optional text refinement; runs hybrid intersect via D13 weighted RRF.' }, limit: { type: 'number', description: 'Max results (default 20)' }, offset: { type: 'number', description: 'Skip first N results (for pagination)' }, - source_id: { type: 'string', description: "Scope to a single source. Defaults to ctx.sourceId. '__all__' opts out." }, + source_id: { type: 'string', description: "Scope to a single source. Defaults to ctx.sourceId. '__all__' spans every source for trusted local callers, your granted sources for remote callers." }, }, scope: 'read', // NOT localOnly: remote MCP callers can pass image_url or image_data @@ -4061,13 +4145,12 @@ const search_by_image: Operation = { { maxBytes: cap }, ); - // Resolve source-scope (D5 canonical thread). - const resolvedSourceId = - sourceIdParam !== undefined - ? sourceIdParam === '__all__' - ? undefined - : sourceIdParam - : ctx.sourceId; + // Resolve source-scope through the single trust+grant resolver. Pre-fix + // this branch computed resolvedSourceId then spread sourceScopeOpts(ctx) + // after it (double-application: the spread silently won, and `__all__` + // didn't opt out for local callers with ctx.sourceId set). One resolver, + // one spread — `__all__` spans the brain only for trusted local callers. + const imageSourceScope = resolveRequestedScope(ctx, sourceIdParam); const { searchByImage } = await import('./search/by-image.ts'); const results = await searchByImage( @@ -4077,8 +4160,7 @@ const search_by_image: Operation = { limit: (p.limit as number) || 20, offset: (p.offset as number) || 0, query: queryRefinement, - sourceId: resolvedSourceId, - ...sourceScopeOpts(ctx), + ...imageSourceScope, }, ); diff --git a/src/core/pglite-engine.ts b/src/core/pglite-engine.ts index ca727cd5f..0a94630e6 100644 --- a/src/core/pglite-engine.ts +++ b/src/core/pglite-engine.ts @@ -830,13 +830,19 @@ export class PGLiteEngine implements BrainEngine { } // Pages CRUD - async getPage(slug: string, opts?: { sourceId?: string; includeDeleted?: boolean }): Promise { + async getPage(slug: string, opts?: { sourceId?: string; sourceIds?: string[]; includeDeleted?: boolean }): Promise { // v0.26.5: hide soft-deleted by default; opt-in via opts.includeDeleted. const includeDeleted = opts?.includeDeleted === true; const sourceId = opts?.sourceId; + const sourceIds = opts?.sourceIds; const where: string[] = ['slug = $1']; const params: unknown[] = [slug]; - if (sourceId) { + // #1393: federated grant (sourceIds[]) wins over scalar sourceId so the + // exact-match read honors allowedSources, not just one source. + if (sourceIds && sourceIds.length > 0) { + params.push(sourceIds); + where.push(`source_id = ANY($${params.length}::text[])`); + } else if (sourceId) { params.push(sourceId); where.push(`source_id = $${params.length}`); } diff --git a/src/core/pglite-schema.ts b/src/core/pglite-schema.ts index 80fb0a973..44c3cc418 100644 --- a/src/core/pglite-schema.ts +++ b/src/core/pglite-schema.ts @@ -931,6 +931,20 @@ CREATE TABLE IF NOT EXISTS op_checkpoints ( CREATE INDEX IF NOT EXISTS op_checkpoints_updated_at_idx ON op_checkpoints (updated_at); +-- #1794: append-only delta storage (one row per completed path). FK cascade +-- drops children with the parent. PK prefix (op,fingerprint) serves all reads. +-- Mirrors migration v115 + src/schema.sql. Placed after op_checkpoints so the +-- FK target exists in the top-to-bottom replay. +CREATE TABLE IF NOT EXISTS op_checkpoint_paths ( + op TEXT NOT NULL, + fingerprint TEXT NOT NULL, + path TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + PRIMARY KEY (op, fingerprint, path), + CONSTRAINT op_checkpoint_paths_parent_fk + FOREIGN KEY (op, fingerprint) REFERENCES op_checkpoints (op, fingerprint) ON DELETE CASCADE +); + -- ============================================================ -- migration_impact_log (v0.41.18.0 — gbrain onboard wave) -- ============================================================ diff --git a/src/core/postgres-engine.ts b/src/core/postgres-engine.ts index 569b2f0fb..cb7bcb14a 100644 --- a/src/core/postgres-engine.ts +++ b/src/core/postgres-engine.ts @@ -897,13 +897,21 @@ export class PostgresEngine implements BrainEngine { } // Pages CRUD - async getPage(slug: string, opts?: { sourceId?: string; includeDeleted?: boolean }): Promise { + async getPage(slug: string, opts?: { sourceId?: string; sourceIds?: string[]; includeDeleted?: boolean }): Promise { const sql = this.sql; const includeDeleted = opts?.includeDeleted === true; const sourceId = opts?.sourceId; - // v0.26.5: default hides soft-deleted rows. Compose with optional sourceId + const sourceIds = opts?.sourceIds; + // v0.26.5: default hides soft-deleted rows. Compose with optional source // filter via fragment chaining (postgres.js supports sql`` composition). - const sourceCondition = sourceId ? sql`AND source_id = ${sourceId}` : sql``; + // #1393: a federated grant (sourceIds[]) takes precedence over scalar + // sourceId so the exact-match read honors allowedSources, not just one source. + const sourceCondition = + sourceIds && sourceIds.length > 0 + ? sql`AND source_id = ANY(${sourceIds}::text[])` + : sourceId + ? sql`AND source_id = ${sourceId}` + : sql``; const deletedCondition = includeDeleted ? sql`` : sql`AND deleted_at IS NULL`; const rows = await sql` SELECT id, source_id, slug, type, title, compiled_truth, timeline, frontmatter, content_hash, created_at, updated_at, deleted_at, @@ -4849,9 +4857,26 @@ export class PostgresEngine implements BrainEngine { // Config async getConfig(key: string): Promise { - const sql = this.sql; - const rows = await sql`SELECT value FROM config WHERE key = ${key}`; - return rows.length > 0 ? (rows[0].value as string) : null; + // #1603: a transient pooler drop on this read used to throw / fall through + // to defaults silently — which on remote Postgres surfaces as the wrong + // search mode/knobs and empty-stdout queries. Retry-with-reconnect using the + // same tuned opts as the bulk writers. No auditSite: this is a single-row + // read, not a bulk write, so it must not emit batch-retry audit rows. + // `this.sql` is a getter, so each attempt sees the pool rebuilt by reconnect. + const opts = this.getBulkRetryOpts(); + return withRetry( + async () => { + const rows = await this.sql`SELECT value FROM config WHERE key = ${key}`; + return rows.length > 0 ? (rows[0].value as string) : null; + }, + { + maxRetries: opts.maxRetries, + delayMs: opts.delayMs, + delayMaxMs: opts.delayMaxMs, + jitter: BULK_RETRY_OPTS.jitter, + reconnect: (ctx) => this.reconnect(ctx), + }, + ); } async setConfig(key: string, value: string): Promise { diff --git a/src/core/retry-matcher.ts b/src/core/retry-matcher.ts index 46afac4d4..c25605e8d 100644 --- a/src/core/retry-matcher.ts +++ b/src/core/retry-matcher.ts @@ -31,6 +31,16 @@ const CONN_PATTERNS = [ // explicit match it was only accidentally caught by /connection.*closed/i. // Match the message form too for wrappers that fold the code into the text. /CONNECTION_ENDED/i, + // v0.42.x (#1794): Supavisor transaction-pooler session exhaustion. When all + // upstream connections are checked out the pooler rejects new sessions + // ("MaxClientsInSessionMode" / EMAXCONNSESSION) and Postgres raises SQLSTATE + // 53300 (too_many_connections). Both are transient under load — without a + // retry, the checkpoint write (and every other pool-contending write) is + // dropped during the exact spike #1794's resumable sync must survive. + /EMAXCONNSESSION/i, + /too many clients already/i, + /max.*clients?.*in session mode/i, + /remaining connection slots are reserved/i, ]; interface PgError { @@ -102,6 +112,10 @@ export function isRetryableConnError(err: unknown): boolean { // v0.42.5.0 (issue #1678): postgres.js's library-level connection-ended // code. Not an 08xxx SQLSTATE, so the /^08/ test above misses it. if (code === 'CONNECTION_ENDED') return true; + // v0.42.x (#1794): SQLSTATE 53300 too_many_connections — pool/pooler + // exhaustion. Starts with 53 not 08, so the /^08/ test above misses it. + // Transient: the spike clears as in-flight queries release connections. + if (code === '53300') return true; // v0.41.2.1: typed-shape match for gbrain's own GBrainError // (problem === 'No database connection'). Avoids brittle string match // when the error wrapper is gbrain-internal. diff --git a/src/core/schema-embedded.ts b/src/core/schema-embedded.ts index a4c291fd4..dc288eabd 100644 --- a/src/core/schema-embedded.ts +++ b/src/core/schema-embedded.ts @@ -675,6 +675,18 @@ CREATE TABLE IF NOT EXISTS op_checkpoints ( CREATE INDEX IF NOT EXISTS op_checkpoints_updated_at_idx ON op_checkpoints (updated_at); +-- #1794: append-only delta storage (one row per completed path). FK cascade +-- drops children with the parent. Mirrors migration v115 + src/schema.sql. +CREATE TABLE IF NOT EXISTS op_checkpoint_paths ( + op TEXT NOT NULL, + fingerprint TEXT NOT NULL, + path TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + PRIMARY KEY (op, fingerprint, path), + CONSTRAINT op_checkpoint_paths_parent_fk + FOREIGN KEY (op, fingerprint) REFERENCES op_checkpoints (op, fingerprint) ON DELETE CASCADE +); + -- migration_impact_log moved BELOW minion_jobs (was here, lines 645-676) -- because its \`job_id BIGINT REFERENCES minion_jobs(id)\` FK requires -- minion_jobs to exist FIRST during SCHEMA_SQL replay. v0.41.25.0 fix. diff --git a/src/core/skill-catalog.ts b/src/core/skill-catalog.ts index 237283521..31d6fe9e0 100644 --- a/src/core/skill-catalog.ts +++ b/src/core/skill-catalog.ts @@ -326,12 +326,48 @@ function availableBrainTools(ctx: OperationContext): string[] { // Frontmatter projection + description // --------------------------------------------------------------------------- -/** Parse a single-line `description:` from raw frontmatter (best-effort). */ +/** + * Parse a `description:` from raw frontmatter (best-effort). + * + * #1711: handles YAML block scalars. `description: |` (literal) and + * `description: >` (folded), with optional chomping/indent indicators + * (`|-`, `>+`, `|2`), are parsed by reading the following indented lines and + * folding them into a single line for the one-line catalog. Pre-fix the regex + * captured the bare `|`/`>` indicator as the description, so block-scalar skills + * showed a literal "|" in the catalog instead of their text. + */ function parseDescriptionField(raw: string): string | undefined { - const m = raw.match(/^description:\s*["']?(.+?)["']?\s*$/m); - if (!m) return undefined; - const v = m[1].trim(); - return v.length > 0 ? v : undefined; + const lines = raw.split('\n'); + for (let i = 0; i < lines.length; i++) { + const m = lines[i].match(/^description:[ \t]*(.*)$/); + if (!m) continue; + const inline = m[1].trim(); + + // Block scalar indicator (`|`, `>`, with optional chomp `+`/`-` and indent digit). + if (/^[|>][+-]?\d*$/.test(inline)) { + const block: string[] = []; + let baseIndent: number | null = null; + for (let j = i + 1; j < lines.length; j++) { + const line = lines[j]; + if (line.trim().length === 0) { block.push(''); continue; } + const indent = line.length - line.trimStart().length; + if (baseIndent === null) { + if (indent === 0) break; // no indented continuation + baseIndent = indent; + } + if (indent < baseIndent) break; // dedent ends the block + block.push(line.slice(baseIndent)); + } + // Catalog descriptions are one line; collapse literal/folded whitespace. + const folded = block.join(' ').replace(/\s+/g, ' ').trim(); + return folded.length > 0 ? folded : undefined; + } + + // Inline scalar: strip a matching pair of surrounding quotes. + const unq = inline.replace(/^(['"])([\s\S]*)\1$/, '$2').trim(); + return unq.length > 0 ? unq : undefined; + } + return undefined; } /** Strip the leading `---\n...\n---` fence; return the prose body. */ diff --git a/src/core/types.ts b/src/core/types.ts index 16488b5ff..300e6880b 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -330,6 +330,12 @@ export interface PageFilters { export interface GetPageOpts { /** Filter to a specific source. When omitted, getPage returns the first slug match across sources (pre-existing semantics). */ sourceId?: string; + /** + * Filter to a federated set of sources (the caller's allowedSources grant). + * Takes precedence over `sourceId` when non-empty. Closes the #1393 leak: the + * get_page exact path must honor a federated grant, not just scalar sourceId. + */ + sourceIds?: string[]; /** Include soft-deleted pages. Default false. See PageFilters.includeDeleted. */ includeDeleted?: boolean; } diff --git a/src/mcp/http-transport.ts b/src/mcp/http-transport.ts index 78a13f038..c1ccb3152 100644 --- a/src/mcp/http-transport.ts +++ b/src/mcp/http-transport.ts @@ -29,6 +29,7 @@ import { createHash } from 'crypto'; import type { BrainEngine } from '../core/engine.ts'; import { buildToolDefs } from './tool-defs.ts'; import { operations } from '../core/operations.ts'; +import type { AuthInfo } from '../core/operations.ts'; import { VERSION } from '../version.ts'; import { dispatchToolCall } from './dispatch.ts'; import { buildDefaultLimiters, type RateLimiter } from './rate-limit.ts'; @@ -74,6 +75,36 @@ interface AuthResult { * for narrower scoping. */ sourceId?: string; + /** + * #1336: AuthInfo carrying the legacy token's stored federated_read grant + * (`permissions.source_id` array). Threaded so `sourceScopeOpts` can scope + * read ops to the operator-granted sources instead of just scalar `sourceId`. + * Bounded to the stored grant — never widened to "all". + */ + auth?: AuthInfo; +} + +/** + * #1336: derive a legacy bearer token's source scope from its stored + * `permissions.source_id`. An ARRAY value is a federated_read grant → + * `allowedSources` (scoped reads across exactly those sources). A STRING value + * scopes the scalar floor. Anything else → 'default' (preserves pre-v0.34 + * behavior). NEVER widened to "all": an empty/garbage value keeps the 'default' + * floor and no federated grant. + */ +export function parseLegacyTokenScope(rawSource: unknown): { sourceId: string; allowedSources?: string[] } { + if (Array.isArray(rawSource)) { + const allowedSources = (rawSource as unknown[]).filter(s => typeof s === 'string' && s.length > 0) as string[]; + if (allowedSources.length > 0) { + // Scalar floor: the first granted source (write authority); reads span the array. + return { sourceId: allowedSources[0], allowedSources }; + } + return { sourceId: 'default' }; + } + if (typeof rawSource === 'string' && rawSource.length > 0) { + return { sourceId: rawSource }; + } + return { sourceId: 'default' }; } /** Read up to `cap` bytes off req.body. Returns null if cap exceeded. */ @@ -193,21 +224,29 @@ export async function startHttpTransport(opts: HttpTransportOptions) { .catch(() => { /* fire-and-forget */ }); // v0.28: extract per-token takes-holder allow-list. Fail-safe default // is ['world'] — a token with no permissions row sees public claims only. - const perms = (row as { permissions?: { takes_holders?: unknown } }).permissions; + const perms = (row as { permissions?: { takes_holders?: unknown; source_id?: unknown } }).permissions; const allowList = Array.isArray(perms?.takes_holders) ? (perms!.takes_holders as unknown[]).filter(h => typeof h === 'string') as string[] : ['world']; + // #1336: honor the operator-set source grant stored on the token. + const { sourceId, allowedSources } = parseLegacyTokenScope(perms?.source_id); + const auth: AuthInfo = { + token, + clientId: rowId, + clientName: rowName, + scopes: [], + sourceId, + ...(allowedSources ? { allowedSources } : {}), + }; return { ok: true, tokenId: rowId, tokenName: rowName, takesHoldersAllowList: allowList, // v0.34.1 (#861, D13): legacy bearer tokens default to 'default' - // source. Preserves the pre-v0.34 effective behavior of the - // serve-http fallback chain that was removed for OAuth clients - // (migration v60 backfills oauth_clients.source_id). This path - // is for the older v0.22.7 access_tokens transport. - sourceId: 'default', + // source unless the token carries an explicit grant (#1336 above). + sourceId, + auth, }; } catch { return { ok: false }; @@ -362,6 +401,9 @@ export async function startHttpTransport(opts: HttpTransportOptions) { remote: true, takesHoldersAllowList: auth.takesHoldersAllowList, sourceId: auth.sourceId, + // #1336: thread the token's federated_read grant so read ops scope + // to the operator-granted sources via sourceScopeOpts. + auth: auth.auth, }); const status = result.isError ? 'error' : 'success'; logRequest(auth.tokenName!, `tools/call:${toolName}`, status, Date.now() - startedMs); diff --git a/src/schema.sql b/src/schema.sql index 38d2c2f55..a900c6d1a 100644 --- a/src/schema.sql +++ b/src/schema.sql @@ -683,6 +683,21 @@ CREATE TABLE IF NOT EXISTS op_checkpoints ( CREATE INDEX IF NOT EXISTS op_checkpoints_updated_at_idx ON op_checkpoints (updated_at); +-- #1794: append-only delta storage. One row per completed path; sync's +-- appendCompleted INSERTs only the delta instead of rewriting the whole +-- completed_keys JSONB array each flush (O(N^2) -> O(delta)). FK cascade drops +-- children with the parent (clearOpCheckpoint + 7-day purge). PK prefix +-- (op,fingerprint) serves all reads; no separate index. Mirrors migration v115. +CREATE TABLE IF NOT EXISTS op_checkpoint_paths ( + op TEXT NOT NULL, + fingerprint TEXT NOT NULL, + path TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + PRIMARY KEY (op, fingerprint, path), + CONSTRAINT op_checkpoint_paths_parent_fk + FOREIGN KEY (op, fingerprint) REFERENCES op_checkpoints (op, fingerprint) ON DELETE CASCADE +); + -- migration_impact_log moved BELOW minion_jobs (was here, lines 645-676) -- because its `job_id BIGINT REFERENCES minion_jobs(id)` FK requires -- minion_jobs to exist FIRST during SCHEMA_SQL replay. v0.41.25.0 fix. diff --git a/test/db-lock-heartbeat-takeover.test.ts b/test/db-lock-heartbeat-takeover.test.ts new file mode 100644 index 000000000..fff0d4381 --- /dev/null +++ b/test/db-lock-heartbeat-takeover.test.ts @@ -0,0 +1,162 @@ +/** + * #1794 — heartbeat-aware lock takeover + direct-pool refresh. + * + * The lock-thrash fix: a holder that refreshed within the steal-grace window is + * NOT stolen even if its TTL lapsed (starved-but-alive), while a holder that + * stopped refreshing past the grace IS stolen. These tests isolate the ON + * CONFLICT grace logic by holding with `process.pid` (alive) so the dead-pid + * auto-takeover path can't fire — a successful steal therefore proves the + * grace/last_refreshed_at predicate did it. + * + * Plus the commit-8 causal-mechanism check: setImmediate yields let a + * setInterval tick fire during a busy loop (the reason the import loop's + * maybeYield keeps the refresh heartbeat alive). + */ + +import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; +import { hostname } from 'os'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { + tryAcquireDbLock, + resolveStealGraceSeconds, + DEFAULT_STEAL_GRACE_SECONDS, +} from '../src/core/db-lock.ts'; + +let engine: PGLiteEngine; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({ database_url: '' }); + await engine.initSchema(); +}); + +afterAll(async () => { + await engine.disconnect(); +}); + +beforeEach(async () => { + await engine.executeRaw(`DELETE FROM gbrain_cycle_locks WHERE id LIKE 'test-hb-%'`); +}); + +const LOCAL = hostname(); + +/** + * Seed a TTL-EXPIRED lock held by an ALIVE pid (process.pid), with a chosen + * last_refreshed_at age (or NULL). Alive holder → dead-pid auto-takeover can't + * fire, so the only reclaim path is the ON CONFLICT grace predicate. + */ +async function seedExpiredLock(id: string, refreshedSecondsAgo: number | null): Promise { + if (refreshedSecondsAgo === null) { + await engine.executeRaw( + `INSERT INTO gbrain_cycle_locks (id, holder_pid, holder_host, acquired_at, ttl_expires_at, last_refreshed_at) + VALUES ($1, $2, $3, NOW() - INTERVAL '1 hour', NOW() - INTERVAL '5 minutes', NULL)`, + [id, process.pid, LOCAL], + ); + } else { + await engine.executeRaw( + `INSERT INTO gbrain_cycle_locks (id, holder_pid, holder_host, acquired_at, ttl_expires_at, last_refreshed_at) + VALUES ($1, $2, $3, NOW() - INTERVAL '1 hour', NOW() - INTERVAL '5 minutes', NOW() - ($4 || ' seconds')::interval)`, + [id, process.pid, LOCAL, String(refreshedSecondsAgo)], + ); + } +} + +async function refreshedAge(id: string): Promise { + const rows = await engine.executeRaw<{ last_refreshed_at: string | null }>( + `SELECT last_refreshed_at FROM gbrain_cycle_locks WHERE id = $1`, + [id], + ); + const v = rows[0]?.last_refreshed_at ?? null; + return v ? new Date(v) : null; +} + +describe('resolveStealGraceSeconds', () => { + test('derives ~2 refresh ticks from the TTL (30min → 600s)', () => { + // refresh ~ttl/6 = 5min = 300s; *2 = 600s. + expect(resolveStealGraceSeconds(30)).toBe(DEFAULT_STEAL_GRACE_SECONDS); + expect(resolveStealGraceSeconds(30)).toBe(600); + }); + + test('floors at 60s for tiny TTLs', () => { + expect(resolveStealGraceSeconds(1)).toBe(60); + }); + + test('env override wins', () => { + process.env.GBRAIN_LOCK_STEAL_GRACE_SECONDS = '123'; + try { + expect(resolveStealGraceSeconds(30)).toBe(123); + } finally { + delete process.env.GBRAIN_LOCK_STEAL_GRACE_SECONDS; + } + }); + + test('bad env override falls back to derived', () => { + process.env.GBRAIN_LOCK_STEAL_GRACE_SECONDS = 'nope'; + try { + expect(resolveStealGraceSeconds(30)).toBe(600); + } finally { + delete process.env.GBRAIN_LOCK_STEAL_GRACE_SECONDS; + } + }); +}); + +describe('heartbeat-aware takeover (ON CONFLICT grace predicate)', () => { + test('FRESH holder is NOT stolen even with an expired TTL', async () => { + // ttl expired 5min ago, but refreshed 1s ago → inside the 600s grace. + await seedExpiredLock('test-hb-fresh', 1); + const handle = await tryAcquireDbLock(engine, 'test-hb-fresh', 30); + expect(handle).toBeNull(); + }); + + test('STALE-refresh holder IS stolen (refresh older than the grace)', async () => { + // ttl expired AND last refresh 20min ago (> 600s grace) → stealable, even + // though the holder pid is alive (proves the grace path, not auto-takeover). + await seedExpiredLock('test-hb-stale', 1200); + const handle = await tryAcquireDbLock(engine, 'test-hb-stale', 30); + expect(handle).not.toBeNull(); + await handle!.release(); + }); + + test('NULL last_refreshed_at (pre-v98 row) IS stolen on TTL expiry', async () => { + await seedExpiredLock('test-hb-null', null); + const handle = await tryAcquireDbLock(engine, 'test-hb-null', 30); + expect(handle).not.toBeNull(); + await handle!.release(); + }); + + test('a reclaimed handle refresh() bumps last_refreshed_at (direct pool path)', async () => { + await seedExpiredLock('test-hb-bump', 1200); + const handle = await tryAcquireDbLock(engine, 'test-hb-bump', 30); + expect(handle).not.toBeNull(); + const before = await refreshedAge('test-hb-bump'); + // Small real delay so NOW() advances measurably between acquire and refresh. + await new Promise((r) => setTimeout(r, 20)); + await handle!.refresh(); + const after = await refreshedAge('test-hb-bump'); + expect(before).not.toBeNull(); + expect(after).not.toBeNull(); + expect(after!.getTime()).toBeGreaterThan(before!.getTime()); + await handle!.release(); + }); +}); + +describe('event-loop yield keeps timers alive (commit 8 mechanism)', () => { + test('setTimeout(0) yields let a setInterval heartbeat fire during a busy loop', async () => { + let ticks = 0; + const iv = setInterval(() => { ticks++; }, 2); + try { + // Mirror the import loop's maybeYield: setTimeout(0) enters the timers + // phase, so the setInterval heartbeat can fire mid-loop. (A setImmediate + // loop starves the timers phase in Bun — the reason maybeYield uses + // setTimeout, not setImmediate.) Bound by wall-clock so the 2ms interval + // has real time to fire. + const start = Date.now(); + while (Date.now() - start < 40) { + await new Promise((r) => setTimeout(r, 0)); + } + } finally { + clearInterval(iv); + } + expect(ticks).toBeGreaterThan(0); + }); +}); diff --git a/test/frontmatter-install-hook.test.ts b/test/frontmatter-install-hook.test.ts index 4f11177a0..baf928fe8 100644 --- a/test/frontmatter-install-hook.test.ts +++ b/test/frontmatter-install-hook.test.ts @@ -53,6 +53,25 @@ describe('frontmatter install-hook (B13)', () => { } }); + test('#1840 — generated hook matches .md/.mdx (single-backslash regex, not over-escaped)', () => { + installHook(tmp, false); + const content = readFileSync(join(tmp, '.githooks', 'pre-commit'), 'utf8'); + // The shell must see `grep -E '\.mdx?$'`. Pre-fix it emitted `'\\.mdx?$'` + // (literal backslash), so the hook matched nothing and silently no-opped. + expect(content).toContain("grep -E '\\.mdx?$'"); + expect(content).not.toContain("grep -E '\\\\.mdx?$'"); + + // Prove the emitted pattern actually selects markdown files. Extract the + // exact pattern between the single quotes and run it through ripgrep-free + // JS regex parity (POSIX ERE `\.mdx?$` == JS `/\.mdx?$/`). + const m = content.match(/grep -E '([^']+)'/); + expect(m).not.toBeNull(); + const re = new RegExp(m![1]); + expect(re.test('notes/thing.md')).toBe(true); + expect(re.test('notes/thing.mdx')).toBe(true); + expect(re.test('notes/thing.txt')).toBe(false); + }); + test('installHook refuses to clobber existing hook without --force', () => { const hooksDir = join(tmp, '.githooks'); mkdirSync(hooksDir, { recursive: true }); diff --git a/test/frontmatter-non-string-fields.test.ts b/test/frontmatter-non-string-fields.test.ts new file mode 100644 index 000000000..2370f41bc --- /dev/null +++ b/test/frontmatter-non-string-fields.test.ts @@ -0,0 +1,83 @@ +/** + * Non-string frontmatter title/type/slug (#1883, #1658, #1556, #1948). + * + * A YAML scalar like `title: 123` parses to a NUMBER. Pre-fix the parser cast it + * `as string`, so a non-string flowed downstream typed as string and crashed the + * first `.toLowerCase()` in content-sanity — aborting the whole lint/sync run + * brain-wide (root trigger behind the never-converging-sync reports #1794/#1939). + * + * Fix: coerce title to a string at the parser; for slug/type fall back to + * inference (never fabricate a "123" slug); guard content-sanity defensively; + * and surface the malformed frontmatter via a lint NON_STRING_FIELD finding. + */ +import { describe, test, expect } from 'bun:test'; +import { parseMarkdown } from '../src/core/markdown.ts'; +import { assessContentSanity } from '../src/core/content-sanity.ts'; + +const fm = (body: string) => `---\n${body}\n---\nbody text here\n`; + +describe('parseMarkdown coerces/guards non-string frontmatter', () => { + test('numeric title is coerced to a string (intent preserved)', () => { + const p = parseMarkdown(fm('title: 123'), 'notes/thing.md'); + expect(typeof p.title).toBe('string'); + expect(p.title).toBe('123'); + }); + + test('boolean title is coerced to a string', () => { + const p = parseMarkdown(fm('title: false'), 'notes/thing.md'); + expect(p.title).toBe('false'); + }); + + test('missing title falls back to inferred title (string)', () => { + const p = parseMarkdown(fm('type: note'), 'notes/My Thing.md'); + expect(typeof p.title).toBe('string'); + expect(p.title.length).toBeGreaterThan(0); + }); + + test('non-string slug is coerced to a usable string (date slugs are legitimate)', () => { + // YAML parses `2024-06-01` as a Date; coerceFrontmatterString → "2024-06-01". + const p = parseMarkdown(fm('slug: 2024-06-01'), 'notes/real-slug.md'); + expect(typeof p.slug).toBe('string'); + expect(p.slug).toBe('2024-06-01'); + }); + + test('numeric type is coerced to a string (never crashes downstream)', () => { + const p = parseMarkdown(fm('type: 5\ntitle: ok'), 'notes/thing.md'); + expect(typeof p.type).toBe('string'); + }); + + test('valid string fields pass through unchanged', () => { + const p = parseMarkdown(fm('title: Real Title\ntype: concept\nslug: my-slug'), 'x.md'); + expect(p.title).toBe('Real Title'); + expect(p.type).toBe('concept'); + expect(p.slug).toBe('my-slug'); + }); +}); + +describe('lint surfaces non-string frontmatter (NON_STRING_FIELD)', () => { + test('numeric title produces a NON_STRING_FIELD validation error', () => { + const p = parseMarkdown(fm('title: 123'), 'notes/thing.md', { validate: true }); + const codes = (p.errors ?? []).map(e => e.code); + expect(codes).toContain('NON_STRING_FIELD'); + }); + + test('all-string frontmatter produces no NON_STRING_FIELD error', () => { + const p = parseMarkdown(fm('title: Fine\ntype: note'), 'notes/thing.md', { validate: true }); + const codes = (p.errors ?? []).map(e => e.code); + expect(codes).not.toContain('NON_STRING_FIELD'); + }); +}); + +describe('assessContentSanity never throws on a non-string title (belt-and-suspenders)', () => { + test('numeric title does not crash the sanity pass', () => { + expect(() => + assessContentSanity({ compiled_truth: 'hello world', timeline: '', title: 123 as unknown as string }), + ).not.toThrow(); + }); + + test('undefined title does not crash the sanity pass', () => { + expect(() => + assessContentSanity({ compiled_truth: 'hello world', timeline: '', title: undefined as unknown as string }), + ).not.toThrow(); + }); +}); diff --git a/test/get-page-federated-scope.test.ts b/test/get-page-federated-scope.test.ts new file mode 100644 index 000000000..a21bc89c0 --- /dev/null +++ b/test/get-page-federated-scope.test.ts @@ -0,0 +1,91 @@ +/** + * #1393 — get_page exact-match path honors the federated source grant. + * + * Pre-fix the exact path used scalar `ctx.sourceId` only: + * const sourceOpts = ctx.sourceId ? { sourceId: ctx.sourceId } : {}; + * A remote OAuth client with a federated `allowedSources` grant (and no single + * ctx.sourceId) therefore got an UNSCOPED exact lookup — a cross-source read of + * any page by slug. The fuzzy path was already scoped (#1436); this closes the + * exact path by (a) routing it through sourceScopeOpts and (b) teaching + * engine.getPage to honor a `sourceIds[]` array (both engines). + */ +import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { resetPgliteState } from './helpers/reset-pglite.ts'; +import { operations, OperationError, type OperationContext } from '../src/core/operations.ts'; + +let engine: PGLiteEngine; +const get_page = operations.find(o => o.name === 'get_page')!; + +function ctxOf(overrides: Partial = {}): OperationContext { + return { + engine: engine as any, + config: {} as any, + logger: console as any, + dryRun: false, + remote: true, + sourceId: 'default', + ...overrides, + }; +} + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); +}, 60_000); + +afterAll(async () => { + if (engine) await engine.disconnect(); +}, 60_000); + +beforeEach(async () => { + await resetPgliteState(engine); + await engine.executeRaw(`INSERT INTO sources (id, name, local_path) VALUES ('alpha', 'alpha', '/tmp/alpha') ON CONFLICT (id) DO NOTHING`); + await engine.executeRaw(`INSERT INTO sources (id, name, local_path) VALUES ('beta', 'beta', '/tmp/beta') ON CONFLICT (id) DO NOTHING`); + // Distinct slugs per source so an exact lookup can leak across the boundary. + await engine.putPage('secret/beta-doc', { + type: 'note', title: 'Beta secret', compiled_truth: 'beta-only content', frontmatter: {}, + }, { sourceId: 'beta' }); + await engine.putPage('shared/alpha-doc', { + type: 'note', title: 'Alpha doc', compiled_truth: 'alpha content', frontmatter: {}, + }, { sourceId: 'alpha' }); +}); + +describe('engine.getPage honors sourceIds[] (federated grant)', () => { + test('sourceIds[] matching the page returns it', async () => { + const page = await engine.getPage('secret/beta-doc', { sourceIds: ['alpha', 'beta'] }); + expect(page?.title).toBe('Beta secret'); + }); + + test('sourceIds[] NOT containing the page returns null', async () => { + const page = await engine.getPage('secret/beta-doc', { sourceIds: ['alpha'] }); + expect(page).toBeNull(); + }); + + test('sourceIds[] takes precedence over scalar sourceId', async () => { + // scalar says alpha, array says beta-only — array wins, page found. + const page = await engine.getPage('secret/beta-doc', { sourceId: 'alpha', sourceIds: ['beta'] }); + expect(page?.title).toBe('Beta secret'); + }); +}); + +describe('get_page handler closes the cross-source exact-read leak', () => { + test('remote client granted only [alpha] CANNOT read a beta-only slug', async () => { + const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['alpha'] } as any }); + // Pre-fix this returned the beta page (leak). Now it is scoped out → 404. + await expect(get_page.handler(ctx, { slug: 'secret/beta-doc' })).rejects.toBeInstanceOf(OperationError); + }); + + test('remote client granted [alpha, beta] CAN read the beta slug', async () => { + const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['alpha', 'beta'] } as any }); + const page: any = await get_page.handler(ctx, { slug: 'secret/beta-doc' }); + expect(page.title).toBe('Beta secret'); + }); + + test('remote client granted only [alpha] CAN read its own alpha slug', async () => { + const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['alpha'] } as any }); + const page: any = await get_page.handler(ctx, { slug: 'shared/alpha-doc' }); + expect(page.title).toBe('Alpha doc'); + }); +}); diff --git a/test/legacy-token-federated-scope.test.ts b/test/legacy-token-federated-scope.test.ts new file mode 100644 index 000000000..3cf1547b9 --- /dev/null +++ b/test/legacy-token-federated-scope.test.ts @@ -0,0 +1,45 @@ +/** + * #1336 — legacy bearer tokens honor their stored federated_read grant. + * + * Pre-fix the legacy access_tokens path hardcoded `sourceId: 'default'` and never + * populated `allowedSources`, so a token whose `permissions.source_id` granted + * multiple sources could not read across them (and MCP reads silently returned + * empty in non-default brains). The grant is now parsed and threaded. + * + * Bounded by design: never widened to "all" — an empty/garbage value keeps the + * 'default' floor and no federated grant. + */ +import { describe, test, expect } from 'bun:test'; +import { parseLegacyTokenScope } from '../src/mcp/http-transport.ts'; + +describe('parseLegacyTokenScope', () => { + test('array grant → allowedSources (federated read) with first as scalar floor', () => { + expect(parseLegacyTokenScope(['dept-x', 'shared'])).toEqual({ sourceId: 'dept-x', allowedSources: ['dept-x', 'shared'] }); + }); + + test('single-element array → that source as both floor and grant', () => { + expect(parseLegacyTokenScope(['only'])).toEqual({ sourceId: 'only', allowedSources: ['only'] }); + }); + + test('string grant → scalar source, no federated array', () => { + expect(parseLegacyTokenScope('team-a')).toEqual({ sourceId: 'team-a' }); + }); + + test('absent grant → default floor, never widened', () => { + expect(parseLegacyTokenScope(undefined)).toEqual({ sourceId: 'default' }); + expect(parseLegacyTokenScope(null)).toEqual({ sourceId: 'default' }); + }); + + test('empty array → default floor, no grant (NOT "all")', () => { + expect(parseLegacyTokenScope([])).toEqual({ sourceId: 'default' }); + }); + + test('garbage (number / empty string) → default floor', () => { + expect(parseLegacyTokenScope(123)).toEqual({ sourceId: 'default' }); + expect(parseLegacyTokenScope('')).toEqual({ sourceId: 'default' }); + }); + + test('array with non-string junk is filtered to valid sources', () => { + expect(parseLegacyTokenScope(['a', 5, '', 'b'])).toEqual({ sourceId: 'a', allowedSources: ['a', 'b'] }); + }); +}); diff --git a/test/op-checkpoint.test.ts b/test/op-checkpoint.test.ts index 550d08f67..6c821e38c 100644 --- a/test/op-checkpoint.test.ts +++ b/test/op-checkpoint.test.ts @@ -4,6 +4,7 @@ import { resetPgliteState } from './helpers/reset-pglite.ts'; import { loadOpCheckpoint, recordCompleted, + appendCompleted, clearOpCheckpoint, resumeFilter, purgeStaleCheckpoints, @@ -142,6 +143,88 @@ describe('loadOpCheckpoint / recordCompleted / clearOpCheckpoint', () => { }); }); +// #1794: append-only delta storage (op_checkpoint_paths). recordCompleted keeps +// REPLACE semantics for the 9 non-sync consumers; appendCompleted is the +// additive path sync uses to avoid O(N²) full-set rewrites. +describe('appendCompleted (delta) + union read', () => { + async function pathRowCount(op: string, fp: string): Promise { + const rows = await engine.executeRaw<{ n: string | number }>( + `SELECT count(*)::text AS n FROM op_checkpoint_paths WHERE op = $1 AND fingerprint = $2`, + [op, fp], + ); + return Number(rows[0]?.n ?? 0); + } + + test('appendCompleted returns true and load reflects the delta', async () => { + const key = { op: 'sync', fingerprint: 'fp-append' }; + expect(await appendCompleted(engine, key, ['a.md', 'b.md'])).toBe(true); + expect((await loadOpCheckpoint(engine, key)).sort()).toEqual(['a.md', 'b.md']); + }); + + test('re-appending an already-banked path inserts 0 new rows (delta, not full rewrite)', async () => { + const key = { op: 'sync', fingerprint: 'fp-delta' }; + await appendCompleted(engine, key, ['a.md', 'b.md']); + expect(await pathRowCount('sync', 'fp-delta')).toBe(2); + // Second flush re-sends one banked + one new path; ON CONFLICT DO NOTHING + // means only the genuinely-new row lands. + await appendCompleted(engine, key, ['b.md', 'c.md']); + expect(await pathRowCount('sync', 'fp-delta')).toBe(3); + expect((await loadOpCheckpoint(engine, key)).sort()).toEqual(['a.md', 'b.md', 'c.md']); + }); + + test('empty delta is a no-op (returns true, writes nothing)', async () => { + const key = { op: 'sync', fingerprint: 'fp-empty' }; + expect(await appendCompleted(engine, key, [])).toBe(true); + expect(await pathRowCount('sync', 'fp-empty')).toBe(0); + }); + + test('union read across legacy completed_keys array AND appended child rows', async () => { + // Simulates an in-flight upgrade: a pre-existing parent row carries the + // legacy array, then the new code appends child rows to the same key. + const key = { op: 'sync', fingerprint: 'fp-union' }; + await engine.executeRaw( + `INSERT INTO op_checkpoints (op, fingerprint, completed_keys, updated_at) + VALUES ('sync', 'fp-union', '["legacy-1","legacy-2"]'::jsonb, now())`, + ); + await appendCompleted(engine, key, ['new-1']); + expect((await loadOpCheckpoint(engine, key)).sort()).toEqual(['legacy-1', 'legacy-2', 'new-1']); + }); + + test('clearOpCheckpoint cascades to child rows', async () => { + const key = { op: 'sync', fingerprint: 'fp-clear' }; + await appendCompleted(engine, key, ['a.md', 'b.md']); + expect(await pathRowCount('sync', 'fp-clear')).toBe(2); + await clearOpCheckpoint(engine, key); + expect(await pathRowCount('sync', 'fp-clear')).toBe(0); + expect(await loadOpCheckpoint(engine, key)).toEqual([]); + }); + + test('recordCompleted still REPLACES (sync appendCompleted does not)', async () => { + // Guards V3: recordCompleted must remove stale keys, not append them. + const key = { op: 'embed', fingerprint: 'fp-replace' }; + await recordCompleted(engine, key, ['x', 'y']); + await recordCompleted(engine, key, ['x']); + expect((await loadOpCheckpoint(engine, key)).sort()).toEqual(['x']); + }); + + test('purge of a stale parent cascades to its child rows', async () => { + // The FK guarantees children always have a parent, so deleting the stale + // parent cascade-drops its children. (A standalone orphan is impossible to + // create — the FK rejects it — so there is no separate orphan sweep.) + await engine.executeRaw( + `INSERT INTO op_checkpoints (op, fingerprint, completed_keys, updated_at) + VALUES ('sync', 'fp-stale', '[]'::jsonb, now() - interval '10 days')`, + ); + await engine.executeRaw( + `INSERT INTO op_checkpoint_paths (op, fingerprint, path, created_at) + VALUES ('sync', 'fp-stale', 'old.md', now() - interval '10 days')`, + ); + const purged = await purgeStaleCheckpoints(engine, 7); + expect(purged).toBe(1); // counts the parent; child cascades silently + expect(await pathRowCount('sync', 'fp-stale')).toBe(0); + }); +}); + describe('resumeFilter (pure)', () => { test('empty completed returns all', () => { expect(resumeFilter(['a', 'b', 'c'], [])).toEqual(['a', 'b', 'c']); diff --git a/test/retry-matcher.test.ts b/test/retry-matcher.test.ts index 5b8c33a31..da066ef82 100644 --- a/test/retry-matcher.test.ts +++ b/test/retry-matcher.test.ts @@ -93,6 +93,34 @@ describe('isRetryableConnError', () => { const err = { problem: 'No database connection', message: 'instance pool torn down' }; expect(isRetryableConnError(err)).toBe(true); }); + + // #1794: Supavisor session-pool exhaustion (EMAXCONNSESSION) + Postgres + // SQLSTATE 53300 too_many_connections. Transient under load — must retry so + // the resumable-sync checkpoint write survives the spike instead of being + // dropped (which is how #1794 lost 100% of progress). + test('matches EMAXCONNSESSION via message', () => { + expect(isRetryableConnError(new Error('EMAXCONNSESSION: max clients in session mode'))).toBe(true); + }); + + test('matches SQLSTATE 53300 too_many_connections via code', () => { + expect(isRetryableConnError(pgError('53300', 'too many connections for role'))).toBe(true); + }); + + test('matches "too many clients already" message', () => { + expect(isRetryableConnError(new Error('sorry, too many clients already'))).toBe(true); + }); + + test('matches reserved-slots message', () => { + expect( + isRetryableConnError(new Error('remaining connection slots are reserved for non-replication superuser connections')) + ).toBe(true); + }); + + // 53300 must NOT accidentally widen to other 53xxx (e.g. 53400 + // configuration_limit_exceeded is not a transient pool blip). + test('does NOT match unrelated 53xxx codes', () => { + expect(isRetryableConnError(pgError('53400', 'configuration limit exceeded'))).toBe(false); + }); }); describe('isRetryableError', () => { diff --git a/test/skill-catalog-block-scalar.test.ts b/test/skill-catalog-block-scalar.test.ts new file mode 100644 index 000000000..4f53ebc85 --- /dev/null +++ b/test/skill-catalog-block-scalar.test.ts @@ -0,0 +1,49 @@ +/** + * #1711 — skill catalog parses YAML block-scalar `description:` fields. + * + * Pre-fix `parseDescriptionField` matched `description: |` with a greedy regex + * and captured the bare block indicator (`|` / `>`) as the description, so a + * skill written with `description: |` showed a literal "|" in the catalog + * instead of its text. + */ +import { describe, test, expect } from 'bun:test'; +import { oneLineDescription } from '../src/core/skill-catalog.ts'; + +describe('oneLineDescription — block scalars', () => { + test('literal block scalar (|) folds indented lines into the description', () => { + const raw = ['name: demo', 'description: |', ' First line of the description.', ' Second line continues it.'].join('\n'); + const out = oneLineDescription(raw, 'body fallback'); + expect(out).toBe('First line of the description. Second line continues it.'); + expect(out).not.toContain('|'); + }); + + test('folded block scalar (>) is parsed too', () => { + const raw = ['description: >', ' Folded description', ' across two lines.'].join('\n'); + expect(oneLineDescription(raw, 'fallback')).toBe('Folded description across two lines.'); + }); + + test('chomping/indent indicators (|-, >+, |2) are recognized', () => { + const raw = ['description: |-', ' Trimmed block scalar.'].join('\n'); + expect(oneLineDescription(raw, 'fallback')).toBe('Trimmed block scalar.'); + }); + + test('block scalar with no indented continuation falls back to body prose', () => { + const raw = ['description: |', 'name: next-key'].join('\n'); + expect(oneLineDescription(raw, 'Body prose line')).toBe('Body prose line'); + }); +}); + +describe('oneLineDescription — inline scalars still work', () => { + test('plain inline description', () => { + expect(oneLineDescription('description: A plain one-liner', 'fallback')).toBe('A plain one-liner'); + }); + + test('quoted inline description strips surrounding quotes', () => { + expect(oneLineDescription('description: "Quoted desc"', 'fallback')).toBe('Quoted desc'); + expect(oneLineDescription("description: 'Single quoted'", 'fallback')).toBe('Single quoted'); + }); + + test('absent description falls back to first prose line', () => { + expect(oneLineDescription('name: x', 'The first prose line.')).toBe('The first prose line.'); + }); +}); diff --git a/test/source-scope-resolver.test.ts b/test/source-scope-resolver.test.ts new file mode 100644 index 000000000..e5b9d3671 --- /dev/null +++ b/test/source-scope-resolver.test.ts @@ -0,0 +1,123 @@ +/** + * Source-isolation trust+grant resolver (#1924, #1371, #1393). + * + * The cross-source leak class: a remote OAuth client scoped to one source could + * pass `source_id: "__all__"` (or an explicit out-of-grant source_id) to read + * sources it was never granted. Every source-scoped read op now routes through + * ONE resolver. These tests pin the trust+grant matrix at the unit level so a + * future per-handler "optimization" that re-inlines the `__all__` branch fails + * loudly here. + */ +import { describe, test, expect } from 'bun:test'; +import { + resolveRequestedScope, + resolveCodeIntelScope, + OperationError, + type OperationContext, +} from '../src/core/operations.ts'; + +function ctxOf(overrides: Partial = {}): OperationContext { + return { + engine: {} as any, + config: {} as any, + logger: console as any, + dryRun: false, + remote: true, + sourceId: 'default', + ...overrides, + }; +} + +describe('resolveRequestedScope — __all__ / all_sources', () => { + test('trusted local + __all__ spans every source (empty scope)', () => { + const scope = resolveRequestedScope(ctxOf({ remote: false, sourceId: 'a' }), '__all__'); + expect(scope).toEqual({}); + }); + + test('remote + __all__ collapses to the caller grant, NOT the whole brain', () => { + const ctx = ctxOf({ remote: true, sourceId: 'a', auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a', 'b'] } as any }); + const scope = resolveRequestedScope(ctx, '__all__'); + expect(scope).toEqual({ sourceIds: ['a', 'b'] }); + }); + + test('remote + __all__ with single-source grant scopes to that one source', () => { + const ctx = ctxOf({ remote: true, sourceId: 'a', auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a'] } as any }); + expect(resolveRequestedScope(ctx, '__all__')).toEqual({ sourceIds: ['a'] }); + }); + + test('remote + __all__ with no federated grant falls back to scalar sourceId (never empty)', () => { + const ctx = ctxOf({ remote: true, sourceId: 'a' }); + expect(resolveRequestedScope(ctx, '__all__')).toEqual({ sourceId: 'a' }); + }); + + test('all_sources=true is treated identically to __all__', () => { + const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['x'] } as any }); + expect(resolveRequestedScope(ctx, undefined, true)).toEqual({ sourceIds: ['x'] }); + }); +}); + +describe('resolveRequestedScope — explicit source_id', () => { + test('remote + explicit source_id OUTSIDE the grant is rejected', () => { + const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a'] } as any }); + expect(() => resolveRequestedScope(ctx, 'b')).toThrow(OperationError); + try { + resolveRequestedScope(ctx, 'b'); + } catch (e) { + expect((e as OperationError).code).toBe('permission_denied'); + } + }); + + test('remote + explicit source_id INSIDE the grant is allowed', () => { + const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a', 'b'] } as any }); + expect(resolveRequestedScope(ctx, 'b')).toEqual({ sourceId: 'b' }); + }); + + test('trusted local + explicit source_id is allowed even with no grant', () => { + expect(resolveRequestedScope(ctxOf({ remote: false }), 'anything')).toEqual({ sourceId: 'anything' }); + }); + + test('remote with no federated grant array can pass an explicit source_id (scalar-floor model)', () => { + // allowedSources undefined → no federated restriction to enforce; the scalar + // sourceId path governs. (Empty [] is treated the same as undefined.) + expect(resolveRequestedScope(ctxOf({ remote: true }), 'z')).toEqual({ sourceId: 'z' }); + const emptyGrant = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: [] } as any }); + expect(resolveRequestedScope(emptyGrant, 'z')).toEqual({ sourceId: 'z' }); + }); +}); + +describe('resolveRequestedScope — default (no param)', () => { + test('falls back to the canonical sourceScopeOpts ladder (federated array wins)', () => { + const ctx = ctxOf({ remote: true, sourceId: 'a', auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a', 'b'] } as any }); + expect(resolveRequestedScope(ctx, undefined)).toEqual({ sourceIds: ['a', 'b'] }); + }); + + test('falls back to scalar sourceId when no federated grant', () => { + expect(resolveRequestedScope(ctxOf({ remote: true, sourceId: 'a' }), undefined)).toEqual({ sourceId: 'a' }); + }); +}); + +describe('resolveCodeIntelScope — single-source code traversal', () => { + test('scalar sourceId → that source, allSources false', () => { + expect(resolveCodeIntelScope(ctxOf({ remote: true, sourceId: 'a' }), undefined)).toEqual({ allSources: false, sourceId: 'a' }); + }); + + test('single-element federated grant → that one source', () => { + const ctx = ctxOf({ remote: true, auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['only'] } as any }); + expect(resolveCodeIntelScope(ctx, '__all__')).toEqual({ allSources: false, sourceId: 'only' }); + }); + + test('multi-source federated grant → rejected (must specify one)', () => { + const ctx = ctxOf({ remote: true, sourceId: 'a', auth: { token: 't', clientId: 'c', scopes: [], allowedSources: ['a', 'b'] } as any }); + expect(() => resolveCodeIntelScope(ctx, '__all__')).toThrow(OperationError); + }); + + test('trusted local + all → allSources true (spans the brain)', () => { + // ctx.sourceId is empty so the resolver yields {} → trusted-local allSources. + expect(resolveCodeIntelScope(ctxOf({ remote: false, sourceId: '' }), '__all__')).toEqual({ allSources: true, sourceId: undefined }); + }); + + test('remote with no source in scope is denied, never widened to all', () => { + const ctx = ctxOf({ remote: true, sourceId: '' }); + expect(() => resolveCodeIntelScope(ctx, '__all__')).toThrow(OperationError); + }); +}); diff --git a/test/sync.test.ts b/test/sync.test.ts index 5a6bf9596..acac5e348 100644 --- a/test/sync.test.ts +++ b/test/sync.test.ts @@ -670,3 +670,244 @@ describe('sync auto-embed arguments', () => { expect(buildAutoEmbedArgs(['people/alice'])).toEqual(['--slugs', 'people/alice']); }); }); + +// #1970: sync silently full-walks forever when last_commit is unreachable. +// The bookmark can point at a commit orphaned by a history rewrite (force-push, +// master→main consolidation, squash). The old guard sent BOTH "object missing" +// AND "not an ancestor" to a blind full re-walk that never advanced the bookmark. +// The fix: only a truly-absent object forces a full reconcile; a present-but- +// non-ancestor bookmark is diffed tree-to-tree directly (`git diff A..B` needs +// no ancestry). Plus F-A (full-sync delete reconcile), F-B (oversized-diff +// fallback), F-C (rename-to-unsyncable deletes the old page). +describe('#1970: unreachable last_commit bookmark recovery', () => { + let engine: PGLiteEngine; + const repos: string[] = []; + + beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); + }); + + afterAll(async () => { + await engine.disconnect(); + }); + + beforeEach(async () => { + await resetPgliteState(engine); + }); + + afterEach(() => { + while (repos.length) { + const d = repos.pop(); + if (d) rmSync(d, { recursive: true, force: true }); + } + }); + + function personMd(title: string, body: string): string { + return ['---', 'type: person', `title: ${title}`, '---', '', body].join('\n'); + } + + /** Create a temp git repo seeded with the given files + an initial commit. */ + function mkRepo(files: Record): string { + const dir = mkdtempSync(join(tmpdir(), 'gbrain-1970-')); + repos.push(dir); + execSync('git init', { cwd: dir, stdio: 'pipe' }); + execSync('git config user.email "test@test.com"', { cwd: dir, stdio: 'pipe' }); + execSync('git config user.name "Test"', { cwd: dir, stdio: 'pipe' }); + for (const [rel, content] of Object.entries(files)) { + mkdirSync(join(dir, rel, '..'), { recursive: true }); + writeFileSync(join(dir, rel), content); + } + execSync('git add -A && git commit -m "initial"', { cwd: dir, stdio: 'pipe' }); + return dir; + } + + const SYNC_OPTS = { noPull: true, noEmbed: true, noExtract: true, sourceId: 'default' } as const; + + async function bookmark(): Promise { + const rows = await engine.executeRaw<{ last_commit: string | null }>( + `SELECT last_commit FROM sources WHERE id = 'default'`, + ); + return rows[0]?.last_commit ?? null; + } + + async function captureLog(fn: () => Promise): Promise<{ result: T; out: string }> { + const lines: string[] = []; + const origLog = console.log; + console.log = (...args: unknown[]) => { lines.push(args.map(String).join(' ')); }; + try { + const result = await fn(); + return { result, out: lines.join('\n') }; + } finally { + console.log = origLog; + } + } + + test('orphan-present (not an ancestor): diffs tree-to-tree, imports only the delta, advances bookmark', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const repo = mkRepo({ + 'people/alice.md': personMd('Alice', 'Alice is a person.'), + 'people/bob.md': personMd('Bob', 'Bob is a person.'), + }); + + const first = await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + expect(first.status).toBe('first_sync'); + const orphan = await bookmark(); + expect(orphan).not.toBeNull(); + + // Rewrite history: amend the only commit (adds delta.md). The previous tip + // is now orphaned but still on disk — cat-file succeeds, is-ancestor fails. + writeFileSync(join(repo, 'people/carol.md'), personMd('Carol', 'Carol joins.')); + execSync('git add -A && git commit --amend -m "amended with carol"', { cwd: repo, stdio: 'pipe' }); + + // Sanity: the stored bookmark is present but no longer an ancestor of HEAD. + expect(execSync(`git cat-file -t ${orphan}`, { cwd: repo }).toString().trim()).toBe('commit'); + let isAncestor = true; + try { execSync(`git merge-base --is-ancestor ${orphan} HEAD`, { cwd: repo, stdio: 'pipe' }); } + catch { isAncestor = false; } + expect(isAncestor).toBe(false); + + const { result, out } = await captureLog(() => performSync(engine, { repoPath: repo, ...SYNC_OPTS })); + + // Incremental diff path (status 'synced'), NOT a full re-walk ('first_sync'). + expect(result.status).toBe('synced'); + expect(result.added).toBe(1); + expect(out).toContain('not an ancestor of HEAD'); + expect(await engine.getPage('people/carol')).not.toBeNull(); + // Bookmark advanced off the orphan onto the rewritten HEAD. + const advanced = await bookmark(); + expect(advanced).not.toBe(orphan); + expect(advanced).toBe(execSync('git rev-parse HEAD', { cwd: repo }).toString().trim()); + }); + + test('orphan-absent (object gc\'d): falls back to a full reconcile', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const repo = mkRepo({ 'people/alice.md': personMd('Alice', 'Alice is a person.') }); + + await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + // Simulate an orphaned-AND-pruned bookmark: a valid-shaped SHA with no object. + await engine.executeRaw( + `UPDATE sources SET last_commit = $1 WHERE id = 'default'`, + ['deadbeefdeadbeefdeadbeefdeadbeefdeadbeef'], + ); + writeFileSync(join(repo, 'people/bob.md'), personMd('Bob', 'Bob is a person.')); + execSync('git add -A && git commit -m "add bob"', { cwd: repo, stdio: 'pipe' }); + + const result = await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + // Object absent → authoritative full reconcile. + expect(result.status).toBe('first_sync'); + expect(await engine.getPage('people/bob')).not.toBeNull(); + expect(await engine.getPage('people/alice')).not.toBeNull(); + }); + + test('divergence: a file present in the orphan tree but dropped from HEAD is deleted', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const repo = mkRepo({ + 'people/alice.md': personMd('Alice', 'Alice is a person.'), + 'people/bob.md': personMd('Bob', 'Bob is a person.'), + }); + await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + expect(await engine.getPage('people/bob')).not.toBeNull(); + + // Rewrite the tip: drop bob, edit alice. Orphans the prior tip (still on disk). + execSync('git rm people/bob.md', { cwd: repo, stdio: 'pipe' }); + writeFileSync(join(repo, 'people/alice.md'), personMd('Alice', 'Alice was corrected.')); + execSync('git add -A && git commit --amend -m "drop bob, edit alice"', { cwd: repo, stdio: 'pipe' }); + + const result = await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + expect(result.status).toBe('synced'); + expect(await engine.getPage('people/bob')).toBeNull(); // deleted + const alice = await engine.getPage('people/alice'); + expect(alice!.compiled_truth).toContain('corrected'); // updated + }); + + test('F-C: a rename whose destination is unsyncable deletes the old page', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const repo = mkRepo({ 'people/carol.md': personMd('Carol', 'Carol is a person.') }); + await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + expect(await engine.getPage('people/carol')).not.toBeNull(); + + // git mv keeps content identical → classified as a 100% rename (R100). + // The destination .txt is unsyncable, so without the F-C fix the old page + // would linger (the rename drops out of both `renamed` and `deleted`). + execSync('git mv people/carol.md people/carol.txt', { cwd: repo, stdio: 'pipe' }); + execSync('git commit -m "rename carol to txt"', { cwd: repo, stdio: 'pipe' }); + + const result = await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + expect(result.status).toBe('synced'); + expect(await engine.getPage('people/carol')).toBeNull(); + }); + + test('F-A: full reconcile purges stale file-backed pages but spares manual + metafile pages', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const repo = mkRepo({ + 'people/alice.md': personMd('Alice', 'Alice is a person.'), + 'people/bob.md': personMd('Bob', 'Bob is a person.'), + }); + await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + + // A manually-curated page (put_page) — source_path stays NULL. + await engine.putPage('manual/note', { + type: 'note', title: 'Manual Note', compiled_truth: 'Hand-authored, not from a file.', + }, { sourceId: 'default' }); + // A metafile-backed page (e.g. an older import or direct put_page of log.md). + // Its source_path is unsyncable, so the reconcile must NOT delete it (#1433). + await engine.putPage('people/log', { + type: 'note', title: 'Log', compiled_truth: 'metafile page', source_path: 'people/log.md', + }, { sourceId: 'default' }); + + // Delete bob's backing file, then force a full reconcile. + execSync('git rm people/bob.md', { cwd: repo, stdio: 'pipe' }); + execSync('git commit -m "remove bob"', { cwd: repo, stdio: 'pipe' }); + + const result = await performSync(engine, { repoPath: repo, full: true, ...SYNC_OPTS }); + expect(result.status).toBe('first_sync'); + expect(result.deleted).toBeGreaterThanOrEqual(1); + + expect(await engine.getPage('people/bob')).toBeNull(); // stale file-backed → purged + expect(await engine.getPage('people/alice')).not.toBeNull(); // still present → kept + expect(await engine.getPage('manual/note')).not.toBeNull(); // null source_path → spared + expect(await engine.getPage('people/log')).not.toBeNull(); // metafile source_path → spared + }); + + test('F-B: an undiffable-but-present bookmark falls back to a full reconcile instead of throwing', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const repo = mkRepo({ 'people/alice.md': personMd('Alice', 'Alice is a person.') }); + await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + + // A blob SHA: cat-file -t succeeds ("blob", so objectPresent=true), but + // `git diff ..HEAD` errors — the same failure shape as an oversized + // post-rewrite diff hitting git()'s timeout/buffer limits. Must fall back, + // not throw. + const blob = execSync('git rev-parse HEAD:people/alice.md', { cwd: repo }).toString().trim(); + await engine.executeRaw(`UPDATE sources SET last_commit = $1 WHERE id = 'default'`, [blob]); + + const result = await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + expect(result.status).toBe('first_sync'); // fell back cleanly + expect(await engine.getPage('people/alice')).not.toBeNull(); + }); + + test('convergence: after orphan recovery, a later commit syncs incrementally to up_to_date', async () => { + const { performSync } = await import('../src/commands/sync.ts'); + const repo = mkRepo({ 'people/alice.md': personMd('Alice', 'Alice is a person.') }); + await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + + // Orphan + recover. + writeFileSync(join(repo, 'people/bob.md'), personMd('Bob', 'Bob is a person.')); + execSync('git add -A && git commit --amend -m "amended with bob"', { cwd: repo, stdio: 'pipe' }); + const recovered = await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + expect(recovered.status).toBe('synced'); + + // A subsequent ordinary commit now syncs incrementally (bookmark is sane). + writeFileSync(join(repo, 'people/carol.md'), personMd('Carol', 'Carol joins.')); + execSync('git add -A && git commit -m "add carol"', { cwd: repo, stdio: 'pipe' }); + const next = await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + expect(next.status).toBe('synced'); + expect(next.added).toBe(1); + + // No further changes → up_to_date (converged). + const settled = await performSync(engine, { repoPath: repo, ...SYNC_OPTS }); + expect(settled.status).toBe('up_to_date'); + }); +});