diff --git a/CHANGELOG.md b/CHANGELOG.md index f5df24d12..60bdb5d0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,24 @@ All notable changes to GBrain will be documented in this file. +## [0.42.32.0] - 2026-06-07 + +**A single un-parseable note can no longer silently stop your brain from indexing anything new.** A page whose YAML frontmatter `title:` was a bare date (`title: 2024-06-01`) or number (`title: 1458`) parsed as a Date/number, not text — and the importer threw when it tried to lowercase it. That throw blocked the sync bookmark from advancing, so every later `gbrain sync` re-walked the whole repo, never reached HEAD, and quietly stopped indexing new commits. The page was committed and on GitHub, but `gbrain get` returned `page_not_found` with no surfaced error. + +Two fixes. First, a non-string title/slug/type now coerces deterministically at parse time — a YAML date becomes its UTC ISO string (`2024-06-01`), so the same page reads the same on every machine and the import never throws. Second, the importer gained a **bounded auto-skip safety valve**: a file that fails to import N consecutive syncs (default 3, `GBRAIN_SYNC_AUTOSKIP_AFTER`) is recorded and skipped so it can't wedge all indexing forever — while a *fresh* failure still fails closed (the bookmark holds and you're told what broke), and a repository history rewrite still hard-blocks even with `--skip-failed`. Skipped pages stay visible: `gbrain doctor` keeps warning until you fix them, and escalates to a hard failure when a real failure has blocked the bookmark past the staleness window. + +`gbrain doctor` now decides sync-failure severity through one shared rule on both the local and remote surfaces, so a stuck bookmark surfaces identically whether you run doctor on your own machine or against a remote brain. + +### Added +- **Bounded auto-skip sync ledger.** A file that fails N consecutive syncs (`GBRAIN_SYNC_AUTOSKIP_AFTER`, default 3; set `0` to disable) is auto-skipped so one poison file can't freeze indexing for the whole brain. Skips are per-source, survive crashes (the bookmark advances before anything is marked skipped), and self-heal — fix or delete the file and the next sync clears it. `gbrain doctor` lists what was skipped and why. + +### Fixed +- **Non-string frontmatter titles no longer wedge indexing (#1939).** `title: 2024-06-01` / `title: 1458` (and date/number `slug`/`type`) coerce to deterministic strings at parse time instead of throwing, so a handful of date-named notes can't silently stop your brain from indexing new commits. +- **`gbrain doctor` sync-failure severity is now consistent across surfaces (#1939).** Local and remote doctor share one decision: a stuck bookmark escalates to FAIL once it has blocked past the staleness window (or many files are blocking), while already-skipped pages stay a visible warning. + +### To take advantage of v0.42.32.0 +Upgrade and run `gbrain sync` once. Any pages that previously wedged the importer (bare date/number titles) now import on their own. If a file still genuinely can't parse, sync tells you which one; fix it, or let the auto-skip valve move past it after a few runs and watch for it in `gbrain doctor`. Tune the threshold with `GBRAIN_SYNC_AUTOSKIP_AFTER` (set `0` to keep the strict fail-closed behavior). + ## [0.42.31.0] - 2026-06-07 **You can now write typed graph edges with your own provenance straight from the CLI — `gbrain link-add a b --link-type relies-on --link-source citation-graph` — and an external edge-writer (a citation-graph ingester, an importer, a classifier) no longer needs a gbrain schema migration to register a new provenance.** Two ergonomics gaps for tools that compute edges out-of-band, filed by a downstream agent building a citation-graph ingester (#1941). diff --git a/TODOS.md b/TODOS.md index c03d34e25..4a280ffda 100644 --- a/TODOS.md +++ b/TODOS.md @@ -1828,22 +1828,18 @@ at plan time and got carved out: ## v0.40.3.0 follow-ups (v0.41+) -- [ ] **v0.41+: source-scope the `sync-failures.jsonl` log so `--skip-failed` works under `--parallel > 1`.** - v0.40.3.0 shipped `gbrain sync --all --parallel N` as a continuous worker pool - with per-source DB locks. The remaining unsafe path: `recordSyncFailures()` / - `acknowledgeSyncFailures()` in `src/core/sync.ts` write to a brain-global JSONL - file at `~/.gbrain/sync-failures.jsonl` with no per-source scope. Under parallel - sync, source A's `--skip-failed` ack can swallow source B's failures recorded - while B was still running. v0.40.3.0's safe interim: refuse to combine - `--skip-failed` / `--retry-failed` with `--parallel > 1` (loud error, paste-ready - hint pointing at `--parallel 1`). The proper fix: (1) extend the JSONL row - schema with a `source_id` field; (2) `recordSyncFailures(failures, sourceId)` - stamps the field; (3) `acknowledgeSyncFailures({sourceId})` filters acks to - one source's rows; (4) `unacknowledgedSyncFailures({sourceId})` reads the - subset. Drop the v0.40.3.0 restriction once source-scoped acks are - deterministic. Estimate: ~1-2 days. Filed during v0.40.3.0 plan review by - Codex outside-voice (decision D15 → B in the eng-review plan at - `~/.claude/plans/system-instruction-you-are-working-fluttering-grove.md`). +- [ ] **v0.41+: drop the `--skip-failed` / `--retry-failed` + `--parallel > 1` restriction now that the failure log is source-scoped.** + **Priority:** P3 + v0.42.32.0 (#1939) landed the source-scoping infrastructure this TODO asked + for: `src/core/sync-failure-ledger.ts` keys every row by `(source_id, path)`, + `recordFailures(sourceId, …)` stamps it, `acknowledgeFailures(sourceId)` / + `autoSkipFailures(sourceId, …)` filter to one source, and a cross-process + lock + atomic temp-rename (`withLedgerLock`) makes concurrent read-modify-write + safe. The remaining work is just to LIFT the v0.40.3.0 interim guard at + `src/commands/sync.ts:3078` (`parallelEligible && (skipFailed || retryFailed)` + → loud refuse) after adding a test that proves source-scoped acks stay + deterministic under `--all --parallel N`. Estimate: ~0.5 day. Originally filed + during the v0.40.3.0 plan review (Codex outside-voice, decision D15 → B). - [ ] **v0.41+ (optional): extend `checkSyncFreshness` to include `embedding_coverage_pct` per source.** v0.40.3.0 plan originally proposed adding a NEW doctor check diff --git a/VERSION b/VERSION index dfd694371..e8fb9eb62 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.42.31.0 +0.42.32.0 diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index 2bd2e2542..ef8f05295 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -34,7 +34,8 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/core/db.ts` — Connection management, schema initialization. `resolveSessionTimeouts()` returns `statement_timeout` + `idle_in_transaction_session_timeout` (defaults 5min each, env-overridable via `GBRAIN_STATEMENT_TIMEOUT`/`GBRAIN_IDLE_TX_TIMEOUT`/`GBRAIN_CLIENT_CHECK_INTERVAL`). Both `connect()` (module singleton) and `PostgresEngine.connect()` (worker pool) consume the result via postgres.js's `connection` option, sending GUCs as startup parameters that survive PgBouncer transaction mode (`setSessionDefaults` kept as a back-compat no-op shim). `connect()` returns `Promise` — `true` iff THIS call created the module singleton, `false` if it joined an existing one; the decision is atomic (no `await` between the `if (sql)` null-check and the synchronous `sql = postgres(...)` assignment), so two concurrent module connects can't both claim creation. `PostgresEngine` stores the return as its `_ownsModuleSingleton` token and only the creating engine may `db.disconnect()` the singleton — a borrower probe engine (lint/doctor config-lift) no-ops its disconnect, so its teardown can't null the connection the long-lived cycle owner is still using (the dream-cycle "connect() has not been called" failure). The module `sql` is only ever nulled by `db.disconnect()` (postgres.js auto-reconnects its own internal pool and never touches our reference). `disconnect()` snapshots + nulls `sql` before awaiting `s.end()` so a concurrent connect can't join a pool that's already closing. - `src/commands/migrate-engine.ts` — Bidirectional engine migration (`gbrain migrate --to supabase/pglite`). - `src/core/import-file.ts` — importFromFile + importFromContent (chunk + embed + tags). `importFromContent` and `importCodeFile` stamp `pages.embedding_signature` via `setPageEmbeddingSignature(slug, {sourceId, signature: currentEmbeddingSignature()})` when the import actually embedded (not `--no-embed`) so a model/dims swap is detectable as stale; `importCodeFile` only stamps when every chunk was freshly embedded this call (`needsEmbedIndexes.length === chunks.length`), mixed reuse-by-hash pages stay unstamped (`reindex --code --force` / `embed --stale` handle those). `importFromContent`'s tag reconciliation is ADD-ONLY: it only `addTag` (idempotent, ON CONFLICT DO NOTHING). The `tags` table has no provenance column and frontmatter tags are stripped from stored `pages.frontmatter` (markdown.ts:118), so a frontmatter-origin tag can't be distinguished from a DB-enrichment tag (auto-tag / dream synthesize / signal-detector) at re-import — deletion is unsafe (would wipe enrichment under `gbrain reindex --markdown`). Accepted trade-off: removing a tag from frontmatter no longer removes it from the DB on next sync (needs a `tag_source` provenance column). Pinned by `test/reindex-preserve-tags.test.ts` + `test/import-file.test.ts`. -- `src/core/sync.ts` — Pure sync functions (manifest parsing, filtering, slug conversion). Exported `pruneDir(name: string): boolean` is the single source of truth for descent-time directory exclusion across walkers — blocks `node_modules` (no leading dot, so naive walkers slipped through and inflated MISSING_OPEN counts via vendor packages), dot-prefix dirs, `ops/`, and `*.raw` sidecars; `isSyncable` applies it per path segment, and `walkMarkdownFiles` in `src/commands/extract.ts` + `listTextFiles` in `src/core/cycle/transcript-discovery.ts` consult it BEFORE recursing to save the IO of walking thousands of vendor files (closes #923 + #202). `manageGitignore` worktree discriminator matches the gitdir path segment (`/modules/` = submodule, `/worktrees/` = worktree, per Git's documented layout) so Conductor worktrees (first-class repos) get `.gitignore` management for storage-tiering (closes #889). `classifyErrorCode(errorMsg)` regex classifier with 12 codes (`SLUG_MISMATCH`, `YAML_PARSE`, `YAML_DUPLICATE_KEY`, `MISSING_OPEN`, `MISSING_CLOSE`, `NESTED_QUOTES`, `EMPTY_FRONTMATTER`, `NULL_BYTES`, `INVALID_UTF8`, `STATEMENT_TIMEOUT`, `FILE_TOO_LARGE`, `SYMLINK_NOT_ALLOWED`) plus `UNKNOWN`. `summarizeFailuresByCode(failures)` returns sorted `[{code, count}]`. `code?` optional field on `SyncFailure`, backfilled at ack time on older entries. `acknowledgeSyncFailures()` returns `AcknowledgeResult { count, summary }`. `MISSING_OPEN`/`MISSING_CLOSE`/`EMPTY_FRONTMATTER` regexes match the actual `markdown.ts:159-244` validator strings; `FILE_TOO_LARGE` covers `import-file.ts:199, 352, 401`; `SYMLINK_NOT_ALLOWED` covers `:347`. +- `src/core/sync.ts` — Pure sync functions (manifest parsing, filtering, slug conversion). Exported `pruneDir(name: string): boolean` is the single source of truth for descent-time directory exclusion across walkers — blocks `node_modules` (no leading dot, so naive walkers slipped through and inflated MISSING_OPEN counts via vendor packages), dot-prefix dirs, `ops/`, and `*.raw` sidecars; `isSyncable` applies it per path segment, and `walkMarkdownFiles` in `src/commands/extract.ts` + `listTextFiles` in `src/core/cycle/transcript-discovery.ts` consult it BEFORE recursing to save the IO of walking thousands of vendor files (closes #923 + #202). `manageGitignore` worktree discriminator matches the gitdir path segment (`/modules/` = submodule, `/worktrees/` = worktree, per Git's documented layout) so Conductor worktrees (first-class repos) get `.gitignore` management for storage-tiering (closes #889). The sync-failure ledger (failure store, error classifier, the shared bookmark gate, and the doctor severity rule) lives in `src/core/sync-failure-ledger.ts`; `sync.ts` re-exports `classifyErrorCode`, `summarizeFailuresByCode`, `loadSyncFailures`, `unacknowledgedSyncFailures`, `acknowledgeSyncFailures`, `recordSyncFailures`, `decideSyncFailureSeverity`, `applySyncFailureGate`, and the `SyncFailure` type for backward-compatible imports — see its entry below. +- `src/core/sync-failure-ledger.ts` — the bounded auto-skip sync failure ledger (issue #1939; formerly inline "Bug 9" in `sync.ts`). A LEAF module (imports only fs/path/crypto/config) so `sync.ts` can re-export it without a circular dependency. State lives in `~/.gbrain/sync-failures.jsonl`, one JSON object per line, keyed by `(source_id, path)` with a per-key `attempts` count and a 3-state machine: `open` (fresh/blocking) → `acknowledged` (human resolved via `gbrain sync --skip-failed`) or `auto_skipped` (chronic). `classifyErrorCode(errorMsg)` regex classifier with 12 codes (`SLUG_MISMATCH`, `YAML_PARSE`, `YAML_DUPLICATE_KEY`, `MISSING_OPEN`, `MISSING_CLOSE`, `NESTED_QUOTES`, `EMPTY_FRONTMATTER`, `NULL_BYTES`, `INVALID_UTF8`, `STATEMENT_TIMEOUT`, `FILE_TOO_LARGE`, `SYMLINK_NOT_ALLOWED`) plus `UNKNOWN` (also recognizes `PAGE_JUNK_PATTERN` from the content-sanity gate); `summarizeFailuresByCode(failures)` returns sorted `[{code, count}]`; `MISSING_OPEN`/`MISSING_CLOSE`/`EMPTY_FRONTMATTER` regexes match the `markdown.ts` validator strings, `FILE_TOO_LARGE` covers `import-file.ts:199, 352, 401`, `SYMLINK_NOT_ALLOWED` covers `:347`. All mutations run under `withLedgerLock` (cross-process file lock) with an atomic rename write. The auto-skip threshold resolves via `resolveAutoSkipThreshold()` from `GBRAIN_SYNC_AUTOSKIP_AFTER` (default `DEFAULT_AUTOSKIP_AFTER = 3`; `0` disables the valve = pure fail-closed). Two pure decision functions are the unit-test surface: `decideGateAction({fileFailures, sentinels, attemptsByPath, threshold, skipFailed})` returns `hard_block | block | advance | advance_then_autoskip` (sentinels like `` ALWAYS hard-block, even with `--skip-failed`, so a history rewrite can't auto-skip; any FRESH failure with `attempts < threshold` blocks fail-closed; only when ALL failures are chronic does it `advance_then_autoskip`), and `decideSyncFailureSeverity({entries, nowMs, failHours})` returns the `sync_failures` doctor status (`ok` when zero unresolved; `fail` when ≥10 OPEN-blocking or the oldest OPEN failure has blocked the bookmark past `failHours`; otherwise `warn` — `auto_skipped`-only rows stay WARN-visible regardless of count because the bookmark already advanced). `applySyncFailureGate(input)` is the one orchestrator BOTH sync paths (incremental + full/`runImport`) call: it records/clears ledger rows, runs `decideGateAction`, then executes effects in the crash-safe order (advance the bookmark FIRST via the injected `advance()` callback, THEN auto-skip the chronic set) so a crash can never mark a file skipped while leaving sync wedged. `isSkippablePath` rejects `<…>` sentinels. Pinned by `test/sync-failure-ledger.serial.test.ts` + `test/sync-failures.test.ts`. - `src/core/storage.ts` — Pluggable storage interface (S3, Supabase Storage, local). - `src/core/storage-config.ts` — Storage tiering: `loadStorageConfig` reads `gbrain.yml`, normalizes deprecated keys (`git_tracked`/`supabase_only`) to canonical (`db_tracked`/`db_only`) with once-per-process deprecation warning, and runs `normalizeAndValidateStorageConfig` (auto-fixes missing trailing `/`, throws `StorageConfigError` on tier overlap). Path-segment matcher: `media/x/` does NOT match `media/xerox/foo`. Uses a dedicated parser for the `gbrain.yml` shape rather than gray-matter (broken on delimiter-less YAML). - `src/core/disk-walk.ts` — `walkBrainRepo(repoPath)` returns `Map` from one recursive `readdirSync`. Skips dot-dirs, `node_modules`, non-`.md` files. Used by `gbrain storage status` to replace per-page `existsSync + statSync` (~400K syscalls on 200K-page brains → tens). @@ -273,7 +274,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/core/ai/model-resolver.ts:parseModelId` — gateway-side resolver accepts both colon and slash form (`provider:model` and `provider/model`) so a slash-form id resolves to the same recipe at every gateway entry point (chat / embed / rerank) instead of throwing `AIConfigError: model id must be in format provider:model`. Bare names without ANY separator still throw — gateway routing always needs an explicit provider. Pinned by `test/ai/model-resolver-slash.test.ts` including a `resolveRecipe` round-trip asserting slash form resolves to the same recipe object as colon form. - `src/commands/transcripts.ts` — `gbrain transcripts recent [--days N] [--full] [--json]`: recent raw `.txt` transcripts from the dream-cycle corpus dirs. Imports `listRecentTranscripts` from `src/core/transcripts.ts` (the same library the gated `get_recent_transcripts` MCP op uses). Local-only by construction — the CLI always runs with `ctx.remote=false`. - `src/commands/integrity.ts` — `gbrain integrity check|auto|review|extract`: bare-tweet detection, dead-link detection, three-bucket repair (auto-repair / review-queue / skip). `scanIntegrity()` is the shared library function called from `gbrain doctor` (sampled at limit=500) and `cmdCheck` (full scan). Batch-load fast path on Postgres uses a single SQL query (fixes the PgBouncer round-trip timeout, ~60s → ~6s), gated by `engine.kind === 'postgres'` at the call site so PGLite never enters batch; fallback `catch` logs at `GBRAIN_DEBUG=1`. Batch projection is `SELECT ... ORDER BY source_id, slug` (NOT `SELECT DISTINCT ON (slug)`, which collapsed same-slug-different-source pages into one scan) so multi-source brains scan each `(source, slug)` row independently. Sequential and auto-repair loops use `listAllPageRefs()` to enumerate `(slug, source_id)` pairs and thread `sourceId` to `getPage`; batch + sequential paths report the same page count on multi-source brains. -- `src/commands/doctor.ts` — `gbrain doctor [--json] [--fast] [--fix] [--dry-run] [--index-audit]`: health checks. Checks include `jsonb_integrity` + `markdown_body_completeness` (reliability), `schema_version` (fails loudly when `version=0`, routes to `gbrain apply-migrations --yes`), `queue_health` (Postgres-only: stalled-forever active jobs started_at > 1h, waiting-depth-per-name > threshold default 10 via `GBRAIN_QUEUE_WAITING_THRESHOLD`, and dead-lettered subagent jobs with `last_error` matching the `prompt_too_long` classifier in last 24h), `sync_failures` (`[CODE=N, ...]` breakdown for unacked-warn + acked-ok), `rls_event_trigger` (healthy `evtenabled` set is `('O','A')` only; fix hint `gbrain apply-migrations --force-retry 35`), `graph_coverage` (short-circuits to ok when `SELECT COUNT(*) FROM pages WHERE type IN ('entity','person','company','organization')` returns 0; WARN hint is `gbrain extract all`), `embedding_column_registry` (probes each declared column via Postgres `format_type(atttypid, atttypmod)` to catch dim mismatch with a paste-ready `gbrain config set embedding_columns '{...}'` hint, probes HNSW index presence via `pg_indexes`, computes default-column population via `COUNT(*) FILTER (WHERE IS NOT NULL) / COUNT(*)` warning below 90% except empty brains where chunk_count=0 short-circuits to ok; PGLite parity via `executeRaw`), and `skill_brain_first` (walks SKILL.md via `autoDetectSkillsDirReadOnly`, calls `analyzeSkillBrainFirst()` from `src/core/skill-brain-first.ts` per file with structured `Check.issues[]`; warn states `missing_brain_first`/`brain_first_typo`, ok states `compliant_callout`/`compliant_phase`/`compliant_position`/`exempt_frontmatter`/`no_external`; snapshot+diff audit at `~/.gbrain/audit/skill-brain-first-YYYY-Www.jsonl`). `--fix` delegates inlined cross-cutting rules to `> **Convention:** see [path](path).` callouts via `src/core/dry-fix.ts` (and MISSING_RULE_PATTERNS for the brain-first callout); `--fix --dry-run` previews. `--index-audit` (Postgres-only, informational, no auto-drop) reports zero-scan indexes from `pg_stat_user_indexes`. Every DB check runs under a progress phase; `markdown_body_completeness` runs under a 1s heartbeat. `runDoctor` uses `autoDetectSkillsDirReadOnly` (from `src/core/repo-root.ts`; install-path fallback so `cd ~ && gbrain doctor` finds bundled skills); `--fix` carries a D6 install-path safety gate that refuses auto-repair when `detected.source === 'install_path'` (would rewrite the bundled tree). The Lane D supervisor check at `doctor.ts:1011-1043` consumes `summarizeCrashes(events)` from `src/core/minions/handlers/supervisor-audit.ts` (warn at `>=1` real crash; ok message has `clean_exits_24h=N`; warn message has `runtime=A oom=B unknown=C legacy=D` per-cause breakdown) so OOM/runtime/unknown crashes are distinguishable from clean code=0 worker drains; cross-surface parity with `gbrain jobs supervisor status` is pinned by source-grep wiring assertions requiring the breakdown substrings in BOTH `doctor.ts` and `jobs.ts`. `checkSyncFreshness` (exported, in `runDoctor` local + `doctorReportRemote` thin-client) is a staleness probe: warns at 24h, fails at 72h or never-synced; future-`last_sync_at` warns ("clock skew") instead of falling through ok; env overrides `GBRAIN_SYNC_FRESHNESS_WARN_HOURS`/`GBRAIN_SYNC_FRESHNESS_FAIL_HOURS` (invalid fall back with once-per-process stderr warn via `_resolveSyncFreshnessHours`); failure messages embed `source.id` so the printed `gbrain sync --source ` matches. It has a `localOnly`-gated git short-circuit (`runDoctor` passes `localOnly: true`; `doctorReportRemote` runs in the HTTP MCP server `src/commands/serve-http.ts` and keeps default `false` so that path never walks DB-supplied `local_path` via subprocess — trust boundary). The local predicate mirrors sync's "do work?" gate (HEAD == `last_commit` AND working tree clean via `requireCleanWorkingTree: 'ignore-untracked'` so a quiet repo with only untracked dirs is `unchanged` not SEVERE, AND `chunker_version === CURRENT`); the inline SELECT carries `last_commit + chunker_version + newest_content_at`. The REMOTE path computes lag via `lagFromContentMs(newest_content_at, lastSync, now)` from the stored column, NO git subprocess; LOCAL fall-through and the `< 0` clock-skew check stay on raw wall-clock. Three-bucket count math populates `Check.details = {unchanged_count, synced_recently_count, stale_count}` with the invariant `sum === sources.length`. `checkCycleFreshness` is DELIBERATELY NOT git-short-circuited or content-relativized (`last_commit == HEAD` can't answer "did the full cycle complete?"; a sync can succeed while later cycle phases fail; different axis `last_full_cycle_at`). Pinned by `test/doctor.test.ts` (incl. IRON-RULE regression banning stale verb names, the sync_freshness boundary matrix, the D4 regression guard verifying git probes are NEVER called when `localOnly` is unset/false, the three-bucket invariant, and the untracked-folders / remote-never-shells-out trust-boundary cases). +- `src/commands/doctor.ts` — `gbrain doctor [--json] [--fast] [--fix] [--dry-run] [--index-audit]`: health checks. Checks include `jsonb_integrity` + `markdown_body_completeness` (reliability), `schema_version` (fails loudly when `version=0`, routes to `gbrain apply-migrations --yes`), `queue_health` (Postgres-only: stalled-forever active jobs started_at > 1h, waiting-depth-per-name > threshold default 10 via `GBRAIN_QUEUE_WAITING_THRESHOLD`, and dead-lettered subagent jobs with `last_error` matching the `prompt_too_long` classifier in last 24h), `sync_failures` (`[CODE=N, ...]` breakdown for unacked-warn + acked-ok; severity comes from the shared `decideSyncFailureSeverity` in `src/core/sync-failure-ledger.ts` so the LOCAL and REMOTE/thin-client doctor surfaces can never drift — a stuck bookmark escalates to FAIL once an OPEN failure has blocked past the staleness window or ≥10 files block, while already `auto_skipped` rows stay a visible WARN), `rls_event_trigger` (healthy `evtenabled` set is `('O','A')` only; fix hint `gbrain apply-migrations --force-retry 35`), `graph_coverage` (short-circuits to ok when `SELECT COUNT(*) FROM pages WHERE type IN ('entity','person','company','organization')` returns 0; WARN hint is `gbrain extract all`), `embedding_column_registry` (probes each declared column via Postgres `format_type(atttypid, atttypmod)` to catch dim mismatch with a paste-ready `gbrain config set embedding_columns '{...}'` hint, probes HNSW index presence via `pg_indexes`, computes default-column population via `COUNT(*) FILTER (WHERE IS NOT NULL) / COUNT(*)` warning below 90% except empty brains where chunk_count=0 short-circuits to ok; PGLite parity via `executeRaw`), and `skill_brain_first` (walks SKILL.md via `autoDetectSkillsDirReadOnly`, calls `analyzeSkillBrainFirst()` from `src/core/skill-brain-first.ts` per file with structured `Check.issues[]`; warn states `missing_brain_first`/`brain_first_typo`, ok states `compliant_callout`/`compliant_phase`/`compliant_position`/`exempt_frontmatter`/`no_external`; snapshot+diff audit at `~/.gbrain/audit/skill-brain-first-YYYY-Www.jsonl`). `--fix` delegates inlined cross-cutting rules to `> **Convention:** see [path](path).` callouts via `src/core/dry-fix.ts` (and MISSING_RULE_PATTERNS for the brain-first callout); `--fix --dry-run` previews. `--index-audit` (Postgres-only, informational, no auto-drop) reports zero-scan indexes from `pg_stat_user_indexes`. Every DB check runs under a progress phase; `markdown_body_completeness` runs under a 1s heartbeat. `runDoctor` uses `autoDetectSkillsDirReadOnly` (from `src/core/repo-root.ts`; install-path fallback so `cd ~ && gbrain doctor` finds bundled skills); `--fix` carries a D6 install-path safety gate that refuses auto-repair when `detected.source === 'install_path'` (would rewrite the bundled tree). The Lane D supervisor check at `doctor.ts:1011-1043` consumes `summarizeCrashes(events)` from `src/core/minions/handlers/supervisor-audit.ts` (warn at `>=1` real crash; ok message has `clean_exits_24h=N`; warn message has `runtime=A oom=B unknown=C legacy=D` per-cause breakdown) so OOM/runtime/unknown crashes are distinguishable from clean code=0 worker drains; cross-surface parity with `gbrain jobs supervisor status` is pinned by source-grep wiring assertions requiring the breakdown substrings in BOTH `doctor.ts` and `jobs.ts`. `checkSyncFreshness` (exported, in `runDoctor` local + `doctorReportRemote` thin-client) is a staleness probe: warns at 24h, fails at 72h or never-synced; future-`last_sync_at` warns ("clock skew") instead of falling through ok; env overrides `GBRAIN_SYNC_FRESHNESS_WARN_HOURS`/`GBRAIN_SYNC_FRESHNESS_FAIL_HOURS` (invalid fall back with once-per-process stderr warn via `_resolveSyncFreshnessHours`); failure messages embed `source.id` so the printed `gbrain sync --source ` matches. It has a `localOnly`-gated git short-circuit (`runDoctor` passes `localOnly: true`; `doctorReportRemote` runs in the HTTP MCP server `src/commands/serve-http.ts` and keeps default `false` so that path never walks DB-supplied `local_path` via subprocess — trust boundary). The local predicate mirrors sync's "do work?" gate (HEAD == `last_commit` AND working tree clean via `requireCleanWorkingTree: 'ignore-untracked'` so a quiet repo with only untracked dirs is `unchanged` not SEVERE, AND `chunker_version === CURRENT`); the inline SELECT carries `last_commit + chunker_version + newest_content_at`. The REMOTE path computes lag via `lagFromContentMs(newest_content_at, lastSync, now)` from the stored column, NO git subprocess; LOCAL fall-through and the `< 0` clock-skew check stay on raw wall-clock. Three-bucket count math populates `Check.details = {unchanged_count, synced_recently_count, stale_count}` with the invariant `sum === sources.length`. `checkCycleFreshness` is DELIBERATELY NOT git-short-circuited or content-relativized (`last_commit == HEAD` can't answer "did the full cycle complete?"; a sync can succeed while later cycle phases fail; different axis `last_full_cycle_at`). Pinned by `test/doctor.test.ts` (incl. IRON-RULE regression banning stale verb names, the sync_freshness boundary matrix, the D4 regression guard verifying git probes are NEVER called when `localOnly` is unset/false, the three-bucket invariant, and the untracked-folders / remote-never-shells-out trust-boundary cases). - `src/core/migrate.ts` — schema-migration runner. Owns the `MIGRATIONS` array (source of truth for schema DDL). `Migration` interface carries `sqlFor?: { postgres?, pglite? }` (engine-specific SQL overrides `sql`) and `transaction?: boolean` (false for `CREATE INDEX CONCURRENTLY`, which Postgres refuses in a transaction; ignored on PGLite). Key migrations: v14 (handler branches on `engine.kind` for CONCURRENTLY-on-Postgres with invalid-remnant pre-drop via `pg_index.indisvalid`, plain `CREATE INDEX` on PGLite); v15 (`minion_jobs.max_stalled` default 1→5 + backfill non-terminal rows); v24 `rls_backfill_missing_tables` (`sqlFor: { pglite: '' }` no-op — PGLite has no RLS engine, targets subagent tables absent from pglite-schema.ts); v30 `dream_verdicts (file_path TEXT, content_hash TEXT, worth_processing BOOL, reasons JSONB, judged_at TIMESTAMPTZ, PK(file_path, content_hash))` (RLS-enabled under BYPASSRLS; synthesize reads/writes to avoid re-judging); v35 auto-RLS event trigger `auto_rls_on_create_table` fires on `ddl_command_end` for `WHEN TAG IN ('CREATE TABLE','CREATE TABLE AS','SELECT INTO')` running `ALTER TABLE … ENABLE ROW LEVEL SECURITY` on new `public.*` tables (no FORCE) + one-time backfill on every existing `public.*` base table whose comment doesn't match `^GBRAIN:RLS_EXEMPT\s+reason=\S.{3,}` (per-table failure aborts the offending CREATE TABLE; no EXCEPTION wrap; PGLite no-op via `sqlFor.pglite: ''`; breaking change: intentionally-RLS-off public tables need the GBRAIN:RLS_EXEMPT comment before upgrade); v40 `pages_emotional_weight` (`pages.emotional_weight REAL NOT NULL DEFAULT 0.0`, column-only metadata-only); v46 `mcp_request_log_params_jsonb_normalize` (`UPDATE ... SET params = params::text::jsonb WHERE jsonb_typeof(params) = 'string'`, idempotent); v60-v65 six-migration chain wiring source-scoping into `oauth_clients` — v60 (`oauth_clients_source_id_fk`: `source_id TEXT` NULL→`'default'` backfill + FK to `sources(id) ON DELETE SET NULL`), v61 (`federated_read TEXT[] NOT NULL DEFAULT '{}'`), v62 (explicit-CASE backfill so `source_id IS NULL` → `'{}'`), v63 (fail-loud check every row's source_id is in its federated_read array), v64 (FK flipped to `ON DELETE RESTRICT`), v65 (GIN index for array-containment); v68 `eval_candidates_embedding_column` (`eval_candidates.embedding_column TEXT NULL` per-row provenance for `gbrain eval replay` to reproduce the same retrieval space; NULL-tolerant); v108 `pages_embedding_signature` (`pages.embedding_signature TEXT NULL` = `:` stamped via `setPageEmbeddingSignature`; GRANDFATHER — stale predicate is `embedding_signature IS NOT NULL AND embedding_signature <> $current` so NULL is NEVER stale and upgrade never re-embeds the whole corpus; no index; metadata-only); v109 `sources_newest_content_at` (`sources.newest_content_at TIMESTAMPTZ` durable newest-COMMIT HEAD committer time written by `writeSyncAnchor`, read by the REMOTE staleness path instead of shelling to git; mirror in pglite-schema.ts + schema.sql + bootstrap probe); v110 `page_aliases` (`(id, source_id, alias_norm, slug, ...)` with `UNIQUE (source_id, alias_norm, slug)` + lookup indexes on `(source_id, alias_norm)` and `(source_id, slug)`; `alias_norm` is `normalizeAlias()` output so WRITE/READ key on the same form; also in `src/core/pglite-schema.ts`); v111 `search_telemetry_rank1_columns` (`ADD COLUMN IF NOT EXISTS` on both engines: `sum_rank1_score`, `count_rank1`, three buckets `rank1_lt_solid`/`rank1_solid`/`rank1_high` on `search_telemetry` — aggregate not per-query rows so rank-1 median drift is bounded-growth; ALTERs right after v57 which created the table); v114 `links_link_source_check_kebab_regex` (#1941, opens `link_source` from the closed allowlist to a kebab-case format gate `^[a-z][a-z0-9]*(-[a-z0-9]+)*$` + `char_length<=64`; Postgres branch uses `NOT VALID` + `VALIDATE CONSTRAINT` with `transaction:false`, PGLite plain DROP+ADD; existing built-ins all satisfy the regex so VALIDATE never fails on existing data). - `src/core/progress.ts` — Shared bulk-action progress reporter. Writes to stderr. Modes: `auto` (TTY `\r`-rewriting; non-TTY plain lines), `human`, `json` (JSONL), `quiet`. Rate-gated by `minIntervalMs` and `minItems`. `startHeartbeat(reporter, note)` for single long queries. `child()` composes phase paths. Singleton SIGINT/SIGTERM coordinator emits `abort` events for every live phase. EPIPE defense on both sync throws and stream `'error'` events. Zero dependencies. `emitHumanLine` is prefix-aware — inside a `withSourcePrefix(id, ...)` scope from `src/core/console-prefix.ts` it prepends `[id] ` (and TTY-rewrite mode `\r\x1b[2K` carries the prefix inside the clear-to-EOL escape); `emitJson` is intentionally NOT prefixed so NDJSON consumers don't choke on a `[id] {...}` shape. - `src/core/console-prefix.ts` — `AsyncLocalStorage`-backed per-source line-prefix helper. Exports `withSourcePrefix(id, fn)` (runs `fn` with `id` as active prefix; nested wraps replace then restore), `getSourcePrefix()` (read-only accessor; test seam), and `slog(...)` / `serr(...)` (prefix-aware `console.log`/`console.error`). Embedded-newline-safe: a multi-line string under prefix `[foo]` emits `[foo] line1\n[foo] line2`. Outside a wrap, `slog`/`serr` fall through to bare `console.log`/`console.error` so single-source callers see identical output (back-compat invariant). Use `src.id` (slug-validated by `sources add`) NOT `src.name` (free-form) to defeat log-injection through newline/control-character names. Coverage: `src/commands/sync.ts` performSync + callees, `src/commands/embed.ts` runEmbedCore + helpers, `src/core/progress.ts` emitHumanLine. @@ -287,7 +288,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `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/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. Pinned by `test/import-checkpoint.test.ts` + `test/import-resume.test.ts` (incl. the SLUG_MISMATCH retry regression). +- `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). - `src/core/cycle.ts` — brain maintenance cycle primitive (9 phases). `runCycle(engine: BrainEngine | null, opts: CycleOpts): Promise` composes phases in semantic order: lint → backlinks → sync → synthesize → extract → patterns → recompute_emotional_weight → embed → orphans. `synthesize` runs after sync (cross-references see a fresh brain) and before extract (auto-link materializes its writes); `patterns` runs after extract so it reads a fresh graph (subagent put_page sets `ctx.remote=true` and skips auto-link/timeline by default, so extract is the canonical materialization); `recompute_emotional_weight` sees the union of `syncPagesAffected` + `synthesizeWrittenSlugs` incrementally, or all pages when neither anchor is set (full backfill via `gbrain dream --phase recompute_emotional_weight`). `CycleReport.schema_version: "1"` is stable; `totals` is additive (`pages_emotional_weight_recomputed`, `transcripts_processed`, `synth_pages_written`, `patterns_written`). Three callers: `gbrain dream` CLI, `gbrain autopilot` daemon inline path, the Minions `autopilot-cycle` handler. Coordination via `gbrain_cycle_locks` DB table + `~/.gbrain/cycle.lock` file lock with PID-liveness for PGLite. `yieldBetweenPhases` runs between phases; `yieldDuringPhase` is in-phase keepalive (synthesize/patterns renew the cycle-lock TTL during long waits). Engine nullable; lock-skip on read-only phase selections. `CycleOpts.signal?: AbortSignal` propagates the worker's abort signal with `checkAborted()` between every phase. `runPhaseSync` returns `pagesAffected` via `SyncPhaseResult` (threaded to `runPhaseExtract` as the 4th arg) and takes `willRunExtractPhase: boolean` setting `noExtract: phases.includes('extract')` so `gbrain dream --phase sync` doesn't silently lose extraction. `resolveSourceForDir(engine, brainDir)` threads `sourceId` to `performSync()` so sync reads the per-source `sources.last_commit` anchor (not the drift-prone global `config.sync.last_commit`). `CycleOpts.brainDir` is `string | null`; when null (checkout-less postgres/Supabase brain) the 6 filesystem phases (lint/backlinks/sync/synthesize/extract/patterns) skip with `details.reason: 'no_brain_dir'` and the DB-only phases run; `resolveSourceForDir` is null-tolerant. `cycleSourceId = opts.sourceId ?? resolveSourceForDir(engine, brainDir)` is the canonical per-source scope for `extract_facts`/`extract_atoms`/calibration so `gbrain dream --source repo-a` reconciles repo-a's facts even with no checkout (instead of scoping to `'default'` while stamping repo-a fresh). `deriveStatus` counts `edges_resolved`/`edges_ambiguous` as work so an edges-only cycle reports `ok` not `clean`; the `jobs.ts` `autopilot-cycle` + phase-wrapper handlers pass `null` (not `'.'`) when no repo is configured. Pinned by `test/dream-postgres.serial.test.ts` + `test/jobs-autopilot-cycle-braindir.serial.test.ts`. @@ -310,7 +311,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `skills/_friction-protocol.md` — shared cross-cutting convention skill (like `_brain-filing-rules.md`). Tells agents when to call `gbrain friction log` and how to choose a severity. Routes to friction CLI from any skill the claw-test exercises. - `scripts/check-progress-to-stdout.sh` — CI guard against regressing to `\r`-on-stdout progress. Wired into `bun run test` via `scripts/check-progress-to-stdout.sh && bun test` in package.json. - `docs/progress-events.md` — Canonical JSON event schema reference. Additive only. -- `src/core/markdown.ts` — Frontmatter parsing + body splitter. `splitBody` requires an explicit timeline sentinel (``, `--- timeline ---`, or `---` immediately before `## Timeline`/`## History`). Plain `---` in body text is a markdown horizontal rule, not a separator. `inferType` auto-types `/wiki/analysis/` → analysis, `/wiki/guides/` → guide, `/wiki/hardware/` → hardware, `/wiki/architecture/` → architecture, `/writing/` → writing (plus existing people/companies/deals/etc heuristics). +- `src/core/markdown.ts` — Frontmatter parsing + body splitter. `coerceFrontmatterString(v)` coerces a non-string `title`/`slug`/`type` to a deterministic string at parse time so a YAML-typed value never reaches `.toLowerCase()` and throws (the #1939 wedge: `title: 2024-06-01` parsed as a `Date`, `title: 1458` as a number, and the throw blocked the sync bookmark from advancing); a `Date` becomes its UTC ISO date (`2024-06-01`, machine-independent and matching the on-disk token, unlike `String(date)`), `null`/`undefined` become `''`, everything else uses `String()`. `splitBody` requires an explicit timeline sentinel (``, `--- timeline ---`, or `---` immediately before `## Timeline`/`## History`). Plain `---` in body text is a markdown horizontal rule, not a separator. `inferType` auto-types `/wiki/analysis/` → analysis, `/wiki/guides/` → guide, `/wiki/hardware/` → hardware, `/wiki/architecture/` → architecture, `/writing/` → writing (plus existing people/companies/deals/etc heuristics). - `scripts/check-jsonb-pattern.sh` — CI grep guard. Fails the build if anyone reintroduces (a) the `${JSON.stringify(x)}::jsonb` interpolation pattern (postgres.js v3 double-encodes it), or (b) `max_stalled INTEGER NOT NULL DEFAULT 1` in any schema source file (must be DEFAULT 5 to preserve SIGKILL-rescue). Wired into `bun test`. - `scripts/check-source-id-projection.sh` — CI grep guard for the multi-source bug class. Greps `src/core/postgres-engine.ts` + `src/core/pglite-engine.ts` for `SELECT.*FROM pages` projections matching the `rowToPage` feeder shape (id + slug + type + title) and fails if `source_id` is missing. `Page.source_id` is required at the type level; a projection dropping the column produces `Page` rows with `source_id: undefined` while TypeScript's `: string` lies about it. Wired into `bun run verify` + `bun run check:all`. - `docker-compose.ci.yml` + `scripts/ci-local.sh` — Local CI gate. `bun run ci:local` spins up `pgvector/pgvector:pg16` + `oven/bun:1` with named volumes (`gbrain-ci-pg-data`, `gbrain-ci-node-modules`, `gbrain-ci-bun-cache`), runs gitleaks on host, smoke-tests `scripts/run-e2e.sh` argv handling, runs unit tests with `DATABASE_URL` unset, then runs all 29 E2E files sequentially. `--diff` swaps in the diff-aware selector; `--no-pull` skips upstream pulls; `--clean` nukes named volumes. Postgres host port defaults to 5434; override with `GBRAIN_CI_PG_PORT=NNNN`. Stronger gate than PR CI's 2-file Tier 1 set. diff --git a/docs/guides/live-sync.md b/docs/guides/live-sync.md index 15f8f5f6e..fbf0fce6c 100644 --- a/docs/guides/live-sync.md +++ b/docs/guides/live-sync.md @@ -120,6 +120,17 @@ hashes match. If both a cron and `--watch` fire simultaneously, no conflict. server is down when a push happens, that sync is missed. Pair webhooks with a cron fallback that catches anything the webhook missed. +4. **A single un-parseable file can't wedge all indexing.** When a file fails + to import (malformed YAML frontmatter, an unquoted colon, etc.), sync holds + the bookmark and tells you exactly which file broke — a *fresh* failure + fails closed so nothing is silently dropped. But a file that fails the same + way `GBRAIN_SYNC_AUTOSKIP_AFTER` consecutive syncs (default 3, set `0` to + disable) is auto-skipped so the rest of the brain keeps indexing past it. + Skipped files don't disappear: `gbrain doctor` keeps warning until you fix + or delete them, and fixing the file clears it on the next sync. A repository + history rewrite still hard-blocks even with `--skip-failed`. Run + `gbrain sync --skip-failed` to acknowledge a known-bad set yourself. + ## How to Verify 1. **Edit a file and search for the change.** Edit a brain markdown file, diff --git a/llms-full.txt b/llms-full.txt index b19d484d5..018b531f1 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -2617,6 +2617,17 @@ hashes match. If both a cron and `--watch` fire simultaneously, no conflict. server is down when a push happens, that sync is missed. Pair webhooks with a cron fallback that catches anything the webhook missed. +4. **A single un-parseable file can't wedge all indexing.** When a file fails + to import (malformed YAML frontmatter, an unquoted colon, etc.), sync holds + the bookmark and tells you exactly which file broke — a *fresh* failure + fails closed so nothing is silently dropped. But a file that fails the same + way `GBRAIN_SYNC_AUTOSKIP_AFTER` consecutive syncs (default 3, set `0` to + disable) is auto-skipped so the rest of the brain keeps indexing past it. + Skipped files don't disappear: `gbrain doctor` keeps warning until you fix + or delete them, and fixing the file clears it on the next sync. A repository + history rewrite still hard-blocks even with `--skip-failed`. Run + `gbrain sync --skip-failed` to acknowledge a known-bad set yourself. + ## How to Verify 1. **Edit a file and search for the change.** Edit a brain markdown file, diff --git a/package.json b/package.json index 9fa1ccd32..b8b372042 100644 --- a/package.json +++ b/package.json @@ -143,5 +143,5 @@ "bun": ">=1.3.10" }, "license": "MIT", - "version": "0.42.31.0" + "version": "0.42.32.0" } diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index a5549b606..b59e6fc8b 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -569,29 +569,24 @@ export async function doctorReportRemote(engine: BrainEngine): Promise l.trim()); - for (const line of lines) { - try { - const entry = JSON.parse(line) as { acknowledged_at?: string | null }; - if (!entry.acknowledged_at) unacked++; - } catch { /* skip malformed line */ } - } - } - checks.push({ - name: 'sync_failures', - status: unacked === 0 ? 'ok' : 'warn', - message: unacked === 0 - ? 'No unacked failures' - : `${unacked} unacked failure(s) — run \`gbrain sync --skip-failed\` on the host to acknowledge`, - }); + const { loadSyncFailures, decideSyncFailureSeverity } = await import('../core/sync.ts'); + const entries = loadSyncFailures(); + const failHours = _resolveSyncFreshnessHours('GBRAIN_SYNC_FRESHNESS_FAIL_HOURS', 72); + const sev = decideSyncFailureSeverity({ entries, nowMs: Date.now(), failHours }); + const msg = + sev.unresolved === 0 + ? 'No unresolved sync failures' + : `${sev.unresolved} unresolved sync failure(s)` + + (sev.auto_skipped > 0 ? ` (${sev.auto_skipped} auto-skipped — pages NOT indexed)` : '') + + ` — run \`gbrain sync --skip-failed\` on the host to acknowledge`; + checks.push({ name: 'sync_failures', status: sev.status, message: msg }); } catch { checks.push({ name: 'sync_failures', status: 'ok', message: 'No failures recorded' }); } @@ -4396,19 +4391,25 @@ export async function buildChecks( // Without this doctor check, users see "sync blocked" and have no // surface showing which files to fix. try { - const { unacknowledgedSyncFailures, loadSyncFailures, summarizeFailuresByCode } = await import('../core/sync.ts'); - const unacked = unacknowledgedSyncFailures(); + const { unacknowledgedSyncFailures, loadSyncFailures, summarizeFailuresByCode, decideSyncFailureSeverity } = await import('../core/sync.ts'); const all = loadSyncFailures(); - if (unacked.length > 0) { - const codeSummary = summarizeFailuresByCode(unacked); + // issue #1939: "unresolved" = open + auto_skipped. Severity (ok/warn/fail) + // comes from the SAME shared decision the remote surface uses, so a stuck + // bookmark blocked past the fail cadence (or a large unresolved count) + // escalates to FAIL instead of staying a quiet WARN forever. + const unresolved = unacknowledgedSyncFailures(); + if (unresolved.length > 0) { + const failHours = _resolveSyncFreshnessHours('GBRAIN_SYNC_FRESHNESS_FAIL_HOURS', 72); + const sev = decideSyncFailureSeverity({ entries: all, nowMs: Date.now(), failHours }); + const codeSummary = summarizeFailuresByCode(unresolved); const codeBreakdown = codeSummary.map(s => `${s.code}=${s.count}`).join(', '); - const preview = unacked.slice(0, 3).map(f => `${f.path} (${f.error.slice(0, 60)})`).join('; '); + const preview = unresolved.slice(0, 3).map(f => `${f.path} (${f.error.slice(0, 60)})`).join('; '); // v0.40.3.0 T8b (D8 + D12 Bug 3): emit a single sync-retry-failed // step. sync-skip-failed is DELIBERATELY NOT emitted as a remediation // — auto-skipping failed syncs hides data loss. Operators can still // run `gbrain sync --skip-failed` manually. const { makeRemediationStep } = await import('../core/remediation-step.ts'); - const oldestTs = unacked.reduce( + const oldestTs = unresolved.reduce( (acc, f) => (acc === '' || f.ts < acc ? f.ts : acc), '', ); @@ -4417,18 +4418,20 @@ export async function buildChecks( job: 'sync-retry-failed', // Content-stable per codex D12 Bug 2: count + oldest_ts captures // the relevant state without using a real timestamp. - params: { failure_count: unacked.length, oldest_failure: oldestTs }, - severity: unacked.length >= 10 ? 'high' : 'medium', + params: { failure_count: unresolved.length, oldest_failure: oldestTs }, + severity: sev.status === 'fail' ? 'high' : 'medium', est_seconds: 30, est_usd_cost: 0, - rationale: `Retry ${unacked.length} unacked sync failure(s) (codes: ${codeBreakdown})`, + rationale: `Retry ${unresolved.length} unresolved sync failure(s) (codes: ${codeBreakdown})`, }); checks.push({ name: 'sync_failures', - status: 'warn', + status: sev.status, message: - `${unacked.length} unacknowledged sync failure(s) [${codeBreakdown}]. ${preview}` + - `${unacked.length > 3 ? `, and ${unacked.length - 3} more` : ''}. ` + + `${unresolved.length} unresolved sync failure(s) [${codeBreakdown}]` + + (sev.auto_skipped > 0 ? ` — ${sev.auto_skipped} auto-skipped (pages NOT indexed)` : '') + + `. ${preview}` + + `${unresolved.length > 3 ? `, and ${unresolved.length - 3} more` : ''}. ` + `Fix the file(s) and re-run 'gbrain sync', or use 'gbrain sync --skip-failed' to acknowledge.`, remediation: [retryStep], remediation_status: 'remediable', diff --git a/src/commands/import.ts b/src/commands/import.ts index 37d3d3dad..1d4575fcb 100644 --- a/src/commands/import.ts +++ b/src/commands/import.ts @@ -44,7 +44,7 @@ export interface RunImportResult { export async function runImport( engine: BrainEngine, args: string[], - opts: { commit?: string; strategy?: SyncStrategy; sourceId?: string } = {}, + opts: { commit?: string; strategy?: SyncStrategy; sourceId?: string; managedBookmark?: boolean } = {}, ): Promise { const noEmbed = args.includes('--no-embed'); const fresh = args.includes('--fresh'); @@ -438,13 +438,17 @@ export async function runImport( // Not a git repo or git not available } - if (gitHead) { + // issue #1939: when performFullSync drives runImport it owns the failure + // ledger + bookmark via the shared gate (applySyncFailureGate). Skipping the + // internal handling here prevents double-recording (which would double-count + // the auto-skip `attempts` streak) and a competing bookmark write. + if (gitHead && !opts.managedBookmark) { // Record failures into the central JSONL so doctor can surface them. // Use gitHead as the commit so a later sync can tell "same broken - // state as last time" from "new broken state." + // state as last time" from "new broken state." Source-scoped (#1939 #2). if (failures.length > 0) { - const { recordSyncFailures } = await import('../core/sync.ts'); - recordSyncFailures(failures, gitHead); + const { recordFailures } = await import('../core/sync.ts'); + recordFailures(opts.sourceId ?? 'default', failures, gitHead); } if (failures.length === 0) { await engine.setConfig('sync.last_commit', gitHead); diff --git a/src/commands/sync.ts b/src/commands/sync.ts index bee832d5f..e739e5339 100644 --- a/src/commands/sync.ts +++ b/src/commands/sync.ts @@ -11,10 +11,14 @@ import { isSyncable, unsyncableReason, resolveSlugForPath, - recordSyncFailures, unacknowledgedSyncFailures, acknowledgeSyncFailures, + loadSyncFailures, formatCodeBreakdown, + applySyncFailureGate, + isSkippablePath, + resolveAutoSkipThreshold, + DEFAULT_SOURCE_ID, } from '../core/sync.ts'; import { estimateTokens, CHUNKER_VERSION } from '../core/chunkers/code.ts'; import { @@ -1472,6 +1476,10 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise 0) { - recordSyncFailures(failedFiles, pin); - // Emit structured summary grouped by error code so the operator - // can see *why* files failed, not just how many. + // issue #1939 — gate the bookmark through the shared failure ledger. + // • Fresh failures still BLOCK (fail-closed): the next sync re-walks the + // diff and re-attempts. Escape hatch: --skip-failed. + // • A file that fails >= threshold consecutive syncs AUTO-SKIPS so a poison + // file can't wedge all indexing forever (recorded, surfaced by doctor). + // • A `` SENTINEL (history rewrite) HARD-BLOCKS even with + // --skip-failed — advancing would record a commit that no longer matches + // the indexed tree. + // `advance` is the bookmark write; the gate runs it ONLY when advancing, and + // ALWAYS before marking anything auto-skipped/acknowledged (crash-atomic). + const advance = async (): Promise => { + // v0.42.x (#1794): advance to the PINNED target (not live HEAD) — commits + // past the pin are the next sync's pin..HEAD diff. `commitTimeMs(pin)` stamps + // newest_content_at against the commit we drained to. `last_sync_at` is bumped + // HERE and ONLY here so the autopilot scheduler never sees a stuck source as + // "fresh". The checkpoint rows clear here — CONVERGENCE CONTRACT: sync + // convergence == IMPORT convergence; downstream extract/facts/embed is + // decoupled (its own resumable stale sweeps). + await writeSyncAnchor(engine, opts.sourceId, 'last_commit', pin, commitTimeMs(repoPath, pin)); + await engine.setConfig('sync.last_run', new Date().toISOString()); + await writeSyncAnchor(engine, opts.sourceId, 'repo_path', repoPath); + await writeChunkerVersion(engine, opts.sourceId, String(CHUNKER_VERSION)); + await clearOpCheckpoint(engine, ckpt.paths); + await clearOpCheckpoint(engine, ckpt.target); + }; + + // issue #1939 adversarial finding #1: a file that failed to parse (open ledger + // row) and is then deleted/renamed-away never re-enters failedFiles and never + // imports, so its row would never clear and would age doctor to a permanent + // FAIL. Treat removed paths as resolved so the ledger self-heals. + const resolvedPaths = [ + ...succeededPaths, + ...filtered.deleted, + ...filtered.renamed.map(r => r.from), + ]; + + const gate = await applySyncFailureGate({ + sourceId: opts.sourceId ?? DEFAULT_SOURCE_ID, + failedFiles, + succeededPaths: resolvedPaths, + commit: pin, + skipFailed: opts.skipFailed === true, + advance, + }); + + if (!gate.advanced) { const codeBreakdown = formatCodeBreakdown(failedFiles); - if (!opts.skipFailed) { + if (gate.sentinelBlocked) { serr( - `\nSync blocked: ${failedFiles.length} file(s) failed to parse:\n` + + `\nSync blocked: repository history changed during sync (force-push / reset).\n` + `${codeBreakdown}\n\n` + - `Fix the YAML frontmatter in the files above and re-run, or use ` + - `'gbrain sync --skip-failed' to acknowledge and move on.`, + `The pinned target is no longer an ancestor of HEAD; advancing would record ` + + `a commit that doesn't match the indexed tree. Re-run sync to re-pin against ` + + `current HEAD.`, ); - // Update last_run + repo_path (progress on infra) but NOT last_commit. - // v0.42.x (#1794): the checkpoint is INTENTIONALLY left in place — the - // banked `completed` set (flushed above) lets the next run skip the files - // already drained and re-attempt only the failures. - await engine.setConfig('sync.last_run', new Date().toISOString()); - await writeSyncAnchor(engine, opts.sourceId, 'repo_path', repoPath); - return { - status: 'blocked_by_failures', - fromCommit: lastCommit, - toCommit: pin, - added: filtered.added.length, - modified: filtered.modified.length, - deleted: filtered.deleted.length, - renamed: filtered.renamed.length, - chunksCreated, - embedded: 0, - pagesAffected, - failedFiles: failedFiles.length, - }; - } - // --skip-failed: acknowledge the now-recorded set and proceed. - const acked = acknowledgeSyncFailures(); - if (acked.count > 0) { + } else { + const fileFailCount = failedFiles.filter(f => isSkippablePath(f.path)).length; serr( - ` Acknowledged ${acked.count} failure(s) and advancing past them:\n` + - `${formatCodeBreakdown(acked.summary)}`, + `\nSync blocked: ${fileFailCount} file(s) failed to parse:\n` + + `${codeBreakdown}\n\n` + + `Fix the frontmatter and re-run, or use 'gbrain sync --skip-failed' to ` + + `acknowledge and move on. A file that keeps failing auto-skips after ` + + `${resolveAutoSkipThreshold()} consecutive syncs.`, ); } + // Update last_run + repo_path (progress on infra) but NOT last_commit. The + // checkpoint is INTENTIONALLY left in place — the banked completed set lets + // the next run skip the drained files and re-attempt only the failures. + await engine.setConfig('sync.last_run', new Date().toISOString()); + await writeSyncAnchor(engine, opts.sourceId, 'repo_path', repoPath); + return { + status: 'blocked_by_failures', + fromCommit: lastCommit, + toCommit: pin, + added: filtered.added.length, + modified: filtered.modified.length, + deleted: filtered.deleted.length, + renamed: filtered.renamed.length, + chunksCreated, + embedded: 0, + pagesAffected, + failedFiles: failedFiles.length, + }; } - // Update sync state AFTER all changes succeed (source-scoped when - // opts.sourceId is set, global config otherwise). v0.42.x (#1794): advance to - // the PINNED target (not live HEAD) — commits past the pin are the next - // sync's pin..HEAD diff. `commitTimeMs(pin)` stamps newest_content_at against - // the commit we actually drained to. `last_sync_at` is bumped HERE and ONLY - // here (inside writeSyncAnchor) — never on a killed partial — so the - // autopilot scheduler never sees a stuck source as "fresh". - await writeSyncAnchor(engine, opts.sourceId, 'last_commit', pin, commitTimeMs(repoPath, pin)); - await engine.setConfig('sync.last_run', new Date().toISOString()); - await writeSyncAnchor(engine, opts.sourceId, 'repo_path', repoPath); - // v0.20.0 Cathedral II Layer 12: persist the chunker version we just - // finished with so the next sync's up_to_date gate respects it. Only - // source-scoped syncs track this (see readChunkerVersion for rationale). - await writeChunkerVersion(engine, opts.sourceId, String(CHUNKER_VERSION)); - // v0.42.x (#1794): import drained the full lastCommit..pin range — the anchor - // is advanced and both checkpoint rows clear here. CONVERGENCE CONTRACT: sync - // convergence == IMPORT convergence. Downstream (extract/facts/embed below) - // is DELIBERATELY decoupled from the anchor: it is size-gated (inline only for - // small syncs) and otherwise handled by its own resumable sweeps - // (extract --stale watermark, embed --stale / embed-backfill, the extract_facts - // + conversation_facts_backfill cycle phases). Coupling a 44k-page facts/embed - // pass into the anchor gate would re-introduce the exact non-convergence this - // fix exists to kill. A kill mid-downstream just means the banked pages get - // their links/embeddings/facts from the next stale sweep. - await clearOpCheckpoint(engine, ckpt.paths); - await clearOpCheckpoint(engine, ckpt.target); + // Advanced. Surface what the gate did past the failures. + if (gate.acknowledged > 0) { + serr(` Acknowledged ${gate.acknowledged} failure(s) and advanced past them.`); + } + if (gate.autoSkipped.length > 0) { + serr( + `\n Auto-skipped ${gate.autoSkipped.length} file(s) that failed >= ` + + `${resolveAutoSkipThreshold()} consecutive syncs:\n` + + gate.autoSkipped.map(p => ` ${p}`).join('\n') + '\n' + + ` Bookmark advanced; these pages are NOT indexed and remain in ` + + `sync-failures.jsonl. 'gbrain doctor' will warn until they're fixed.`, + ); + } // Log ingest await engine.logIngest({ @@ -2263,55 +2304,80 @@ async function performFullSync( commit: headCommit, strategy: opts.strategy, sourceId: opts.sourceId, + // issue #1939: performFullSync owns the failure ledger + bookmark via the + // shared gate below; don't let runImport double-record or write its own. + managedBookmark: true, }); serr( `[gbrain phase] sync.fullsync.import done ${Date.now() - _fullImportT0}ms ` + `imported=${result.imported} skipped=${result.skipped} errors=${result.errors}`, ); - // Bug 9 — gate the full-sync bookmark on success. runImport already - // writes its own sync.last_commit conditionally (import.ts), but - // performFullSync is called on first-sync + force-full paths where - // the sync module owns the last_commit write. Respect the same gate. - if (result.failures.length > 0) { - recordSyncFailures(result.failures, headCommit); - const codeBreakdown = formatCodeBreakdown(result.failures); - if (!opts.skipFailed) { - serr( - `\nFull sync blocked: ${result.failures.length} file(s) failed:\n` + - `${codeBreakdown}\n\n` + - `Fix the YAML in those files and re-run, or use '--skip-failed'.`, - ); - await engine.setConfig('sync.last_run', new Date().toISOString()); - await writeSyncAnchor(engine, opts.sourceId, 'repo_path', repoPath); - return { - status: 'blocked_by_failures', - fromCommit: null, - toCommit: headCommit, - added: 0, modified: 0, deleted: 0, renamed: 0, - chunksCreated: result.chunksCreated, - embedded: 0, - pagesAffected: [], - failedFiles: result.failures.length, - }; - } - const acked = acknowledgeSyncFailures(); - if (acked.count > 0) { - serr( - ` Acknowledged ${acked.count} failure(s) and advancing past them:\n` + - `${formatCodeBreakdown(acked.summary)}`, - ); - } - } + // issue #1939 — gate the full-sync bookmark through the SAME shared ledger as + // the incremental path (Codex #6: a wedge here on first/forced sync was + // previously unreachable by the valve). A full re-import is authoritative for + // the whole tree, so any previously-tracked failing path that ISN'T failing + // now has been resolved → clear it (resets its auto-skip streak); current + // failures still climb their attempts. + const fullSourceId = opts.sourceId ?? DEFAULT_SOURCE_ID; + const fullFailureSet = new Set(result.failures.map(f => f.path)); + const fullSucceeded = loadSyncFailures() + .filter(e => e.source_id === fullSourceId && isSkippablePath(e.path) && !fullFailureSet.has(e.path)) + .map(e => e.path); + const advanceFull = async (): Promise => { + // Persist sync state so the next sync is incremental. Routed through + // writeSyncAnchor so --source pins the right sources row. + await writeSyncAnchor(engine, opts.sourceId, 'last_commit', headCommit, newestCommitMs(repoPath)); + await engine.setConfig('sync.last_run', new Date().toISOString()); + await writeSyncAnchor(engine, opts.sourceId, 'repo_path', repoPath); + await writeChunkerVersion(engine, opts.sourceId, String(CHUNKER_VERSION)); + }; - // Persist sync state so next sync is incremental (C1 fix: was missing). - // v0.18.0 Step 5: routed through writeSyncAnchor so --source pins it - // to the right sources row rather than the global config. - await writeSyncAnchor(engine, opts.sourceId, 'last_commit', headCommit, newestCommitMs(repoPath)); - await engine.setConfig('sync.last_run', new Date().toISOString()); - await writeSyncAnchor(engine, opts.sourceId, 'repo_path', repoPath); - // v0.20.0 Cathedral II Layer 12: persist chunker version for the gate. - await writeChunkerVersion(engine, opts.sourceId, String(CHUNKER_VERSION)); + const fullGate = await applySyncFailureGate({ + sourceId: fullSourceId, + failedFiles: result.failures, + succeededPaths: fullSucceeded, + commit: headCommit, + skipFailed: opts.skipFailed === true, + advance: advanceFull, + }); + + if (!fullGate.advanced) { + const codeBreakdown = formatCodeBreakdown(result.failures); + if (fullGate.sentinelBlocked) { + serr(`\nFull sync blocked: repository history changed during sync.\n${codeBreakdown}`); + } else { + const fileFailCount = result.failures.filter(f => isSkippablePath(f.path)).length; + serr( + `\nFull sync blocked: ${fileFailCount} file(s) failed:\n` + + `${codeBreakdown}\n\n` + + `Fix the YAML in those files and re-run, or use '--skip-failed'. A file ` + + `that keeps failing auto-skips after ${resolveAutoSkipThreshold()} consecutive syncs.`, + ); + } + await engine.setConfig('sync.last_run', new Date().toISOString()); + await writeSyncAnchor(engine, opts.sourceId, 'repo_path', repoPath); + return { + status: 'blocked_by_failures', + fromCommit: null, + toCommit: headCommit, + added: 0, modified: 0, deleted: 0, renamed: 0, + chunksCreated: result.chunksCreated, + embedded: 0, + pagesAffected: [], + failedFiles: result.failures.length, + }; + } + if (fullGate.acknowledged > 0) { + serr(` Acknowledged ${fullGate.acknowledged} failure(s) and advanced past them.`); + } + if (fullGate.autoSkipped.length > 0) { + serr( + `\n Auto-skipped ${fullGate.autoSkipped.length} file(s) that failed >= ` + + `${resolveAutoSkipThreshold()} consecutive syncs. These pages are NOT indexed; ` + + `'gbrain doctor' will warn until they're fixed.`, + ); + } // 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 diff --git a/src/core/content-sanity.ts b/src/core/content-sanity.ts index 1d516075a..ca5260162 100644 --- a/src/core/content-sanity.ts +++ b/src/core/content-sanity.ts @@ -376,14 +376,18 @@ export function assessContentSanity(opts: { // doesn't repeat the lowercase per literal. const bodyHead = body.slice(0, SCAN_HEAD_BYTES); const bodyHeadLower = bodyHead.toLowerCase(); - const titleLower = opts.title.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. + const title = String(opts.title ?? ''); + const titleLower = title.toLowerCase(); const junk_pattern_matches: string[] = []; for (const p of BUILT_IN_JUNK_PATTERNS) { const scope = p.applies_to ?? 'both'; let matched = false; if (scope === 'title' || scope === 'both') { - if (p.pattern.test(opts.title)) matched = true; + if (p.pattern.test(title)) matched = true; } if (!matched && (scope === 'body' || scope === 'both')) { if (p.pattern.test(bodyHead)) matched = true; diff --git a/src/core/enrichment/completeness.ts b/src/core/enrichment/completeness.ts index 8b91afd6d..b5e7997aa 100644 --- a/src/core/enrichment/completeness.ts +++ b/src/core/enrichment/completeness.ts @@ -112,7 +112,8 @@ function nonRedundancy(page: Page): number { } function hasTitle(page: Page): number { - return page.title && page.title.trim().length > 0 ? 1 : 0; + // Coerce (issue #1939): a malformed YAML date/number title could be non-string. + return String(page.title ?? '').trim().length > 0 ? 1 : 0; } function hasBody(page: Page): number { diff --git a/src/core/markdown.ts b/src/core/markdown.ts index 19cf0474f..9a6359b6b 100644 --- a/src/core/markdown.ts +++ b/src/core/markdown.ts @@ -49,6 +49,25 @@ export interface ParsedMarkdown { errors?: ParseValidationError[]; } +/** + * Coerce a raw YAML frontmatter value into a string. + * + * js-yaml parses unquoted scalars by type: `title: 2024-06-01` becomes a JS + * `Date`, `title: 1458` becomes a `number`. The old `(frontmatter.X as string)` + * cast was a compile-time lie — at runtime the value stayed a Date/number, so + * any downstream `.toLowerCase()` / `.trim()` threw and (via the importer's + * failure gate) could wedge sync indefinitely (issue #1939). + * + * Dates coerce to their UTC ISO date (`2024-06-01`) — deterministic across + * machines and matching the on-disk source token, unlike `String(date)` which + * renders a timezone-dependent long form. Everything else uses `String()`. + */ +export function coerceFrontmatterString(v: unknown): string { + if (v == null) return ''; + if (v instanceof Date) return v.toISOString().slice(0, 10); + return String(v); +} + /** * Parse a markdown file with YAML frontmatter into its components. * @@ -105,12 +124,12 @@ export function parseMarkdown( const { compiled_truth, timeline } = splitBody(body); - const type = (frontmatter.type as string) || ( + const type = coerceFrontmatterString(frontmatter.type) || ( opts?.activePack ? inferTypeFromPack(filePath, opts.activePack) : inferType(filePath) ); - const title = (frontmatter.title as string) || inferTitle(filePath); + const title = coerceFrontmatterString(frontmatter.title).trim() || inferTitle(filePath); const tags = extractTags(frontmatter); - const slug = (frontmatter.slug as string) || inferSlug(filePath); + const slug = coerceFrontmatterString(frontmatter.slug) || inferSlug(filePath); const cleanFrontmatter = { ...frontmatter }; delete cleanFrontmatter.type; diff --git a/src/core/sync-failure-ledger.ts b/src/core/sync-failure-ledger.ts new file mode 100644 index 000000000..3bc4e4c2e --- /dev/null +++ b/src/core/sync-failure-ledger.ts @@ -0,0 +1,764 @@ +// ───────────────────────────────────────────────────────────────── +// Sync failure ledger — issue #1939 (was "Bug 9" in src/core/sync.ts) +// ───────────────────────────────────────────────────────────────── +// +// When a sync run catches a per-file parse error (YAML with unquoted +// colons, malformed frontmatter, a non-string title, etc.), we record it +// here instead of just logging and moving on. Goals: +// 1. Gate the sync.last_commit bookmark advance in BOTH sync gates +// (incremental + full/runImport) through one shared policy. +// 2. Give a visible, per-(source,path) record of what failed, with the +// commit hash to re-attempt after fixing the source file. +// 3. `gbrain sync --skip-failed` acknowledges a known-bad set. +// 4. BOUNDED AUTO-SKIP: a file that fails N consecutive syncs is +// auto-skipped so a single poison file can't wedge ALL indexing +// forever — without ever silently dropping a FRESH failure, and +// never auto-skipping an infra sentinel like ``. +// +// This module is a LEAF (imports only fs/path/crypto/config) so it can be +// re-exported from sync.ts without a circular dependency. The state lives +// in `~/.gbrain/sync-failures.jsonl`, one JSON object per line. +// +// State machine (per (source_id, path)): +// +// recordFailures (file fails a sync run) +// │ attempts++ each consecutive run +// ▼ +// ┌──────┐ clearFailures (file imports OK) ┌─────────┐ +// │ open │ ───────────────────────────────────────│ removed │ +// └──────┘ └─────────┘ +// │ │ +// --skip-│ │ attempts >= threshold AND no fresh failures +// failed │ │ (after bookmark advance) +// ▼ ▼ +// ┌────────────┐ ┌──────────────┐ +// │acknowledged│ │ auto_skipped │ (still UNRESOLVED → doctor WARN +// │ (resolved) │ │ visible │ until the file imports cleanly) +// └────────────┘ └──────────────┘ +// +// `acknowledged` = ok (human resolved). `open` + `auto_skipped` = unresolved +// (doctor surfaces them). `` and any `<…>` sentinel is recorded but +// NEVER auto-skipped/acknowledged-to-advance — a history rewrite must hard-block. + +import { + existsSync as _existsSync, + readFileSync as _readFileSync, + writeFileSync as _writeFileSync, + mkdirSync as _mkdirSync, + renameSync as _renameSync, + openSync as _openSync, + closeSync as _closeSync, + unlinkSync as _unlinkSync, + statSync as _statSync, +} from 'fs'; +import { join as _joinPath } from 'path'; +import { gbrainPath as _gbrainPath } from './config.ts'; +import { createHash as _createHash } from 'crypto'; + +export const DEFAULT_SOURCE_ID = 'default'; +/** Reserved sentinel paths (e.g. ``) start with this; never file paths. */ +export const SENTINEL_PREFIX = '<'; +export const DEFAULT_AUTOSKIP_AFTER = 3; +const LOCK_STALE_MS = 30_000; +const LOCK_SPIN_MS = 50; +const LOCK_TIMEOUT_MS = 5_000; + +export type SyncFailureState = 'open' | 'acknowledged' | 'auto_skipped'; + +export interface SyncFailure { + /** Owning source (#1939 Codex #2 — failures must not merge across sources). */ + source_id: string; + path: string; + error: string; + /** Structured error code extracted from the error message. */ + code: string; + /** Most recent commit this path failed on. */ + commit: string; + line?: number; + /** ISO — start of the current unresolved streak. */ + first_seen: string; + /** ISO — last update. */ + ts: string; + /** Consecutive failed sync runs for (source_id, path). */ + attempts: number; + state: SyncFailureState; + resolved_at?: string; + // Legacy MIRROR fields, still WRITTEN for one release so any pre-#1939 + // reader of `acknowledged_at` keeps working. Derived from `state`. + acknowledged?: boolean; + acknowledged_at?: string | null; +} + +export interface AcknowledgeResult { + count: number; + summary: Array<{ code: string; count: number }>; +} + +/** A real importable file (not an infra sentinel like ``). */ +export function isSkippablePath(path: string): boolean { + return !path.startsWith(SENTINEL_PREFIX); +} + +/** + * Resolve the auto-skip threshold from `GBRAIN_SYNC_AUTOSKIP_AFTER` + * (default 3). `0` disables the valve entirely (pure fail-closed). + */ +export function resolveAutoSkipThreshold(): number { + const raw = process.env.GBRAIN_SYNC_AUTOSKIP_AFTER; + if (raw === undefined || raw === '') return DEFAULT_AUTOSKIP_AFTER; + const n = Number(raw); + if (!Number.isFinite(n) || n < 0) return DEFAULT_AUTOSKIP_AFTER; + return Math.floor(n); +} + +/** + * Best-effort extraction of a structured error code from a sync failure + * message. Order matters: DB-layer errors are checked BEFORE YAML-layer + * ones so Postgres `duplicate key` isn't mislabeled as a YAML duplicate-key. + */ +export function classifyErrorCode(errorMsg: string): string { + // SLUG_MISMATCH: thrown by importFromFile() at src/core/import-file.ts. + if (/slug.*does not match|SLUG_MISMATCH/i.test(errorMsg)) return 'SLUG_MISMATCH'; + + // DB-layer errors come BEFORE the YAML duplicate-key check. + if (/duplicate key value violates unique constraint|DB_DUPLICATE_KEY/i.test(errorMsg)) { + return 'DB_DUPLICATE_KEY'; + } + if (/canceling statement due to statement timeout|STATEMENT_TIMEOUT/i.test(errorMsg)) { + return 'STATEMENT_TIMEOUT'; + } + + // YAML / frontmatter patterns. + if (/YAML parse failed|YAML_PARSE/i.test(errorMsg)) return 'YAML_PARSE'; + if (/YAMLException|duplicated mapping key|YAML_DUPLICATE_KEY/i.test(errorMsg)) { + return 'YAML_DUPLICATE_KEY'; + } + if (/File is empty or whitespace-only|Frontmatter must start with ---|MISSING_OPEN/i.test(errorMsg)) { + return 'MISSING_OPEN'; + } + if (/No closing --- delimiter|Heading at line .* found inside frontmatter|MISSING_CLOSE/i.test(errorMsg)) { + return 'MISSING_CLOSE'; + } + if (/Frontmatter block is empty|EMPTY_FRONTMATTER/i.test(errorMsg)) return 'EMPTY_FRONTMATTER'; + if (/Content contains null bytes|NULL_BYTES|null byte/i.test(errorMsg)) return 'NULL_BYTES'; + if (/Nested double quotes|NESTED_QUOTES/i.test(errorMsg)) return 'NESTED_QUOTES'; + + // Generic fallbacks. + if (/invalid UTF-?8|INVALID_UTF8/i.test(errorMsg)) return 'INVALID_UTF8'; + if (/file too large|content too large|FILE_TOO_LARGE/i.test(errorMsg)) return 'FILE_TOO_LARGE'; + if (/skipping symlink|symlink|SYMLINK_NOT_ALLOWED/i.test(errorMsg)) return 'SYMLINK_NOT_ALLOWED'; + + // takes-v2 fence + holder grammar failures. + if (/TAKES_TABLE_MALFORMED|TAKES_ROW_NUM_COLLISION|TAKES_FENCE_UNBALANCED/i.test(errorMsg)) { + return 'TAKES_TABLE_MALFORMED'; + } + if (/TAKES_HOLDER_INVALID/i.test(errorMsg)) return 'TAKES_HOLDER_INVALID'; + + // Embedding error classification. + if (/embedding requires [A-Z][A-Z0-9_]+_API_KEY|EMBEDDING_NO_CREDS/i.test(errorMsg)) { + return 'EMBEDDING_NO_CREDS'; + } + if (/Anthropic has no embedding model|EMBEDDING_NO_TOUCHPOINT/i.test(errorMsg)) { + return 'EMBEDDING_NO_TOUCHPOINT'; + } + if (/\brate.?limit|\b429\b|too many requests|rate_limited|RateLimit/i.test(errorMsg)) { + return 'EMBEDDING_RATE_LIMIT'; + } + if (/insufficient_quota|quota exceeded|exceeded.*quota|credit balance is too low|billing|EMBEDDING_QUOTA/i.test(errorMsg)) { + return 'EMBEDDING_QUOTA'; + } + if (/maximum context length|max_tokens|context length|input too long|input length exceeds|tokens? exceed|too many tokens|EMBEDDING_OVERSIZE/i.test(errorMsg)) { + return 'EMBEDDING_OVERSIZE'; + } + + // content-sanity reject disposition. + if (/PAGE_JUNK_PATTERN/i.test(errorMsg)) return 'PAGE_JUNK_PATTERN'; + + return 'UNKNOWN'; +} + +/** Group failures by error code and return a sorted summary. */ +export function summarizeFailuresByCode( + failures: Array<{ error: string; code?: string }>, +): Array<{ code: string; count: number }> { + const counts: Record = {}; + for (const f of failures) { + const code = f.code ?? classifyErrorCode(f.error); + counts[code] = (counts[code] ?? 0) + 1; + } + return Object.entries(counts) + .sort(([, a], [, b]) => b - a) + .map(([code, count]) => ({ code, count })); +} + +/** + * Format a code-grouped summary as a human-readable multi-line string. + * Accepts either raw failures (summarized internally) or an already- + * summarized `{code, count}[]`. Empty input → empty string. + */ +export function formatCodeBreakdown( + input: Array<{ error: string; code?: string }> | Array<{ code: string; count: number }>, +): string { + const summary = + input.length > 0 && typeof (input[0] as { count?: unknown }).count === 'number' + ? (input as Array<{ code: string; count: number }>) + : summarizeFailuresByCode(input as Array<{ error: string; code?: string }>); + return summary.map(s => ` ${s.code}: ${s.count}`).join('\n'); +} + +function _failuresDir(): string { + return _gbrainPath(); +} + +export function syncFailuresPath(): string { + return _joinPath(_failuresDir(), 'sync-failures.jsonl'); +} + +function _ledgerKey(f: { source_id: string; path: string }): string { + // NUL separator can't appear in a source id or path. + return `${f.source_id}${f.path}`; +} + +// ─── State mirror ─────────────────────────────────────────────────── + +/** + * Keep the legacy `acknowledged`/`acknowledged_at` fields consistent with + * `state`. `auto_skipped` is intentionally NOT acknowledged (it is still an + * unindexed page) so even a pre-#1939 reader counting `!acknowledged_at` + * keeps surfacing it. + */ +function _applyMirror(f: SyncFailure): SyncFailure { + if (f.state === 'acknowledged') { + f.acknowledged = true; + f.acknowledged_at = f.resolved_at ?? f.ts; + } else { + f.acknowledged = false; + f.acknowledged_at = null; + } + return f; +} + +// ─── Load + normalize ──────────────────────────────────────────────── + +function _normalizeRow(raw: Record): SyncFailure { + const source_id = + typeof raw.source_id === 'string' && raw.source_id ? raw.source_id : DEFAULT_SOURCE_ID; + const error = String(raw.error ?? ''); + const code = typeof raw.code === 'string' && raw.code ? raw.code : classifyErrorCode(error); + const ts = typeof raw.ts === 'string' && raw.ts ? raw.ts : new Date(0).toISOString(); + const first_seen = + typeof raw.first_seen === 'string' && raw.first_seen ? raw.first_seen : ts; + let state: SyncFailureState; + if (raw.state === 'open' || raw.state === 'acknowledged' || raw.state === 'auto_skipped') { + state = raw.state; + } else { + state = raw.acknowledged === true || raw.acknowledged_at ? 'acknowledged' : 'open'; + } + const attempts = + typeof raw.attempts === 'number' && Number.isFinite(raw.attempts) && raw.attempts > 0 + ? Math.floor(raw.attempts) + : 1; + const row: SyncFailure = { + source_id, + path: String(raw.path ?? ''), + error, + code, + commit: String(raw.commit ?? ''), + line: typeof raw.line === 'number' ? raw.line : undefined, + first_seen, + ts, + attempts, + state, + resolved_at: + typeof raw.resolved_at === 'string' + ? raw.resolved_at + : typeof raw.acknowledged_at === 'string' + ? raw.acknowledged_at + : undefined, + }; + return _applyMirror(row); +} + +/** Merge legacy duplicate rows for one (source_id, path) into a single row. */ +function _mergeGroup(group: SyncFailure[]): SyncFailure { + const sorted = [...group].sort((a, b) => (a.ts < b.ts ? -1 : a.ts > b.ts ? 1 : 0)); + const latest = sorted[sorted.length - 1]; + const first_seen = sorted.reduce( + (m, r) => (r.first_seen && r.first_seen < m ? r.first_seen : m), + sorted[0].first_seen, + ); + const hasOpen = group.some(r => r.state === 'open'); + const hasAuto = group.some(r => r.state === 'auto_skipped'); + const state: SyncFailureState = hasOpen ? 'open' : hasAuto ? 'auto_skipped' : 'acknowledged'; + // attempts reconstruction: distinct commits is a proxy for distinct runs; + // never under-count below the largest recorded attempts. + const distinctCommits = new Set(group.map(r => r.commit)).size; + const maxAttempts = group.reduce((m, r) => Math.max(m, r.attempts), 0); + const attempts = Math.max(distinctCommits, maxAttempts, 1); + return _applyMirror({ ...latest, first_seen, state, attempts }); +} + +/** + * Read the ledger, normalizing every row (backfill source_id/state/attempts/ + * first_seen) and collapsing duplicate (source_id, path) rows. Skips malformed + * lines with a warning. Empty array if the file doesn't exist. + */ +export function loadSyncFailures(): SyncFailure[] { + const path = syncFailuresPath(); + if (!_existsSync(path)) return []; + const raw = _readFileSync(path, 'utf-8'); + const rows: SyncFailure[] = []; + for (const line of raw.split('\n')) { + const trimmed = line.trim(); + if (!trimmed) continue; + try { + rows.push(_normalizeRow(JSON.parse(trimmed) as Record)); + } catch { + console.warn(`[sync-failures] skipping malformed line: ${trimmed.slice(0, 120)}`); + } + } + const byKey = new Map(); + for (const r of rows) { + const k = _ledgerKey(r); + const arr = byKey.get(k); + if (arr) arr.push(r); + else byKey.set(k, [r]); + } + const out: SyncFailure[] = []; + for (const group of byKey.values()) { + out.push(group.length === 1 ? group[0] : _mergeGroup(group)); + } + return out; +} + +/** Unresolved failures (open + auto_skipped). */ +export function unacknowledgedSyncFailures(): SyncFailure[] { + return loadSyncFailures().filter(e => e.state !== 'acknowledged'); +} + +// ─── Concurrency: cross-process lock + atomic write ────────────────── + +function _sleepSync(ms: number): void { + // Synchronous sleep without busy-spin; works in Node + Bun. + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); +} + +function _acquireLock(lockPath: string): boolean { + const deadline = Date.now() + LOCK_TIMEOUT_MS; + // eslint-disable-next-line no-constant-condition + while (true) { + try { + _closeSync(_openSync(lockPath, 'wx')); + return true; + } catch (e) { + if ((e as NodeJS.ErrnoException)?.code !== 'EEXIST') return false; // best-effort + // Age-based stale break (prefer age over PID liveness, per db-lock learning). + try { + const st = _statSync(lockPath); + if (Date.now() - st.mtimeMs > LOCK_STALE_MS) { + try { _unlinkSync(lockPath); } catch { /* raced; retry */ } + continue; + } + } catch { + continue; // lock vanished underfoot; retry acquire + } + if (Date.now() >= deadline) return false; + _sleepSync(LOCK_SPIN_MS); + } + } +} + +/** + * Serialize a read-modify-write of the ledger across processes + * (`sync --all`, per-source cron). Falls back to best-effort (no lock) on + * acquire timeout so a sync is never deadlocked by a wedged lock holder. + */ +export function withLedgerLock(fn: () => T): T { + _mkdirSync(_failuresDir(), { recursive: true }); + const lockPath = syncFailuresPath() + '.lock'; + const got = _acquireLock(lockPath); + if (!got) { + console.warn('[sync-failures] could not acquire ledger lock; proceeding best-effort'); + } + try { + return fn(); + } finally { + if (got) { + try { _unlinkSync(lockPath); } catch { /* already gone */ } + } + } +} + +function _writeAll(entries: SyncFailure[]): void { + _mkdirSync(_failuresDir(), { recursive: true }); + const target = syncFailuresPath(); + const tmp = `${target}.tmp-${process.pid}`; + const body = entries.map(e => JSON.stringify(e)).join('\n'); + _writeFileSync(tmp, entries.length ? body + '\n' : ''); + _renameSync(tmp, target); // atomic on POSIX +} + +// ─── Mutations ─────────────────────────────────────────────────────── + +/** + * Record this run's failures AND clear succeeded paths in ONE locked + * transaction. Returns post-state attempts for each failing path. + * + * Upsert is keyed by (source_id, path) over OPEN rows: an existing open row + * increments `attempts` (consecutive); anything else (no row, or an + * acknowledged/auto_skipped row) starts a fresh open row at attempts 1 — a + * re-failure after a fix is a new streak (#1939 Codex #4). + */ +function _recordAndClear( + sourceId: string, + succeededPaths: string[], + failures: Array<{ path: string; error: string; line?: number }>, + commit: string, +): Map { + return withLedgerLock(() => { + const entries = loadSyncFailures(); + const byKey = new Map(); + for (const e of entries) byKey.set(_ledgerKey(e), e); + let mutated = false; + + // Resolve succeeded paths (success fully clears the row — #1939 Codex #4). + for (const p of succeededPaths) { + if (byKey.delete(_ledgerKey({ source_id: sourceId, path: p }))) mutated = true; + } + + const now = new Date().toISOString(); + for (const f of failures) { + const key = _ledgerKey({ source_id: sourceId, path: f.path }); + const ex = byKey.get(key); + const code = classifyErrorCode(f.error); + if (ex && ex.state === 'open') { + ex.attempts += 1; + ex.ts = now; + ex.commit = commit; + ex.error = f.error; + ex.code = code; + ex.line = f.line; + _applyMirror(ex); + } else { + byKey.set( + key, + _applyMirror({ + source_id: sourceId, + path: f.path, + error: f.error, + code, + commit, + line: f.line, + first_seen: now, + ts: now, + attempts: 1, + state: 'open', + }), + ); + } + mutated = true; + } + + // Skip the write (and avoid creating an empty ledger file) when a clean run + // touched nothing — e.g. succeeded paths that had no prior failure rows. + if (mutated) _writeAll([...byKey.values()]); + + const attempts = new Map(); + for (const f of failures) { + attempts.set( + f.path, + byKey.get(_ledgerKey({ source_id: sourceId, path: f.path }))?.attempts ?? 1, + ); + } + return attempts; + }); +} + +/** + * Public single-purpose recorder (no clear). Used by callers outside the + * sync gate (e.g. `gbrain import`). Increments attempts like the gate. + */ +export function recordFailures( + sourceId: string, + failures: Array<{ path: string; error: string; line?: number }>, + commit: string, +): void { + if (failures.length === 0) return; + _recordAndClear(sourceId, [], failures, commit); +} + +/** Remove ledger rows for the given (sourceId, paths) — used on success. */ +export function clearFailures(sourceId: string, paths: string[]): void { + if (paths.length === 0) return; + withLedgerLock(() => { + const entries = loadSyncFailures(); + const remove = new Set(paths.map(p => _ledgerKey({ source_id: sourceId, path: p }))); + const kept = entries.filter(e => !remove.has(_ledgerKey(e))); + if (kept.length !== entries.length) _writeAll(kept); + }); +} + +/** + * Acknowledge OPEN file failures (human `--skip-failed`). Scoped to one + * source when `sourceId` is given (never acks another source — #1939 Codex + * #2). Sentinels (``) are NEVER acknowledged this way. + */ +export function acknowledgeFailures(sourceId?: string): AcknowledgeResult { + return withLedgerLock(() => { + const entries = loadSyncFailures(); + const now = new Date().toISOString(); + let changed = 0; + const acked: SyncFailure[] = []; + for (const e of entries) { + if (e.state !== 'open') continue; + if (sourceId !== undefined && e.source_id !== sourceId) continue; + if (!isSkippablePath(e.path)) continue; + e.state = 'acknowledged'; + e.resolved_at = now; + _applyMirror(e); + changed++; + acked.push(e); + } + if (changed > 0) _writeAll(entries); + return { count: changed, summary: summarizeFailuresByCode(acked) }; + }); +} + +/** + * Mark the given chronic file paths `auto_skipped` (valve fired). Only OPEN, + * non-sentinel rows transition. Auto-skipped rows stay UNRESOLVED so doctor + * keeps warning until the file imports cleanly. + */ +export function autoSkipFailures(sourceId: string, paths: string[]): AcknowledgeResult { + if (paths.length === 0) return { count: 0, summary: [] }; + return withLedgerLock(() => { + const entries = loadSyncFailures(); + const target = new Set( + paths.filter(isSkippablePath).map(p => _ledgerKey({ source_id: sourceId, path: p })), + ); + const now = new Date().toISOString(); + let changed = 0; + const skipped: SyncFailure[] = []; + for (const e of entries) { + if (!target.has(_ledgerKey(e))) continue; + if (e.state !== 'open') continue; + e.state = 'auto_skipped'; + e.resolved_at = now; + _applyMirror(e); + changed++; + skipped.push(e); + } + if (changed > 0) _writeAll(entries); + return { count: changed, summary: summarizeFailuresByCode(skipped) }; + }); +} + +// ─── Legacy shims (re-exported from sync.ts for existing callers) ───── + +/** @deprecated use recordFailures(sourceId, …). Defaults to the host source. */ +export function recordSyncFailures( + failures: Array<{ path: string; error: string; line?: number }>, + commit: string, +): void { + recordFailures(DEFAULT_SOURCE_ID, failures, commit); +} + +/** @deprecated use acknowledgeFailures(sourceId). Acks ALL sources' open files. */ +export function acknowledgeSyncFailures(): AcknowledgeResult { + return acknowledgeFailures(undefined); +} + +// ─── Pure decisions (no side effects — the unit-test surface) ───────── + +export interface GateDecision { + action: 'hard_block' | 'block' | 'advance' | 'advance_then_autoskip'; + autoSkipPaths: string[]; +} + +/** + * Decide what the sync gate should do, given this run's failures and the + * current attempt counts. Pure — the caller executes effects in the safe + * order (advance THEN ack, so a crash can't mark a file skipped while the + * sync stays wedged — #1939 Codex #5). + * + * sentinels present → hard_block (ALWAYS, even --skip-failed) + * no file failures → advance + * --skip-failed → advance (ack handled post-advance) + * valve disabled (threshold<=0) → block (pure fail-closed) if failures + * any fresh (attempts=threshold) → advance_then_autoskip + */ +export function decideGateAction(args: { + fileFailures: Array<{ path: string }>; + sentinels: Array<{ path: string }>; + attemptsByPath: Map; + threshold: number; + skipFailed: boolean; +}): GateDecision { + if (args.sentinels.length > 0) return { action: 'hard_block', autoSkipPaths: [] }; + if (args.fileFailures.length === 0) return { action: 'advance', autoSkipPaths: [] }; + if (args.skipFailed) return { action: 'advance', autoSkipPaths: [] }; + if (args.threshold <= 0) return { action: 'block', autoSkipPaths: [] }; + + const chronic: string[] = []; + let fresh = 0; + for (const f of args.fileFailures) { + const a = args.attemptsByPath.get(f.path) ?? 1; + if (a >= args.threshold) chronic.push(f.path); + else fresh++; + } + if (fresh > 0) return { action: 'block', autoSkipPaths: [] }; + if (chronic.length > 0) return { action: 'advance_then_autoskip', autoSkipPaths: chronic }; + return { action: 'block', autoSkipPaths: [] }; +} + +export interface SeverityResult { + status: 'ok' | 'warn' | 'fail'; + unresolved: number; + open: number; + auto_skipped: number; +} + +/** + * Decide the `sync_failures` doctor severity from ledger entries. Pure; + * `nowMs` is injected for deterministic boundary tests. Both doctor surfaces + * (local + remote) call this so they can never drift (#1939 Codex #1). + * + * unresolved (open|auto_skipped) == 0 → ok + * oldest OPEN older than failHours, OR + * total unresolved >= 10 → fail + * else (incl. auto_skipped-only) → warn (stays visible) + * malformed ts → treated as not-old (never crashes doctor) + */ +export function decideSyncFailureSeverity(args: { + entries: SyncFailure[]; + nowMs: number; + failHours: number; +}): SeverityResult { + const unresolved = args.entries.filter( + e => e.state === 'open' || e.state === 'auto_skipped', + ); + const autoSkipped = unresolved.filter(e => e.state === 'auto_skipped').length; + const open = unresolved.length - autoSkipped; + if (unresolved.length === 0) { + return { status: 'ok', unresolved: 0, open: 0, auto_skipped: 0 }; + } + let oldestOpenMs = Infinity; + for (const e of unresolved) { + if (e.state !== 'open') continue; + const ms = Date.parse(e.ts); + if (Number.isFinite(ms)) oldestOpenMs = Math.min(oldestOpenMs, ms); + } + const blockedTooLong = + Number.isFinite(oldestOpenMs) && args.nowMs - oldestOpenMs > args.failHours * 3_600_000; + // FAIL keys off OPEN (blocking) failures only — many open, or one blocking the + // bookmark past the fail cadence. `auto_skipped` rows already advanced the + // bookmark (indexing is NOT wedged) so they stay WARN-visible regardless of + // count, matching the state-machine contract. (#1939 adversarial finding #3.) + const status: 'warn' | 'fail' = open >= 10 || blockedTooLong ? 'fail' : 'warn'; + return { status, unresolved: unresolved.length, open, auto_skipped: autoSkipped }; +} + +// ─── Shared gate orchestrator (incremental + full sync) ────────────── + +export interface SyncGateInput { + sourceId: string; + /** All per-file failures this run, including sentinels like ``. */ + failedFiles: Array<{ path: string; error: string; line?: number }>; + /** File paths that imported successfully this run (clears their rows). */ + succeededPaths: string[]; + /** Pin commit the run drained to (stamped on recorded failures). */ + commit: string; + skipFailed: boolean; + threshold?: number; + /** Advances the bookmark + clears checkpoints. Awaited BEFORE any ack. */ + advance: () => Promise | void; +} + +export interface SyncGateOutcome { + advanced: boolean; + sentinelBlocked: boolean; + fresh: number; + chronic: number; + autoSkipped: string[]; + acknowledged: number; +} + +/** + * The one gate both sync paths share (#1939 Codex #6). Records/clears the + * ledger under a single lock, decides, then on advance runs `advance()` + * FIRST and only marks auto_skipped/acknowledged afterwards (#1939 Codex #5). + */ +export async function applySyncFailureGate(input: SyncGateInput): Promise { + const threshold = input.threshold ?? resolveAutoSkipThreshold(); + const sentinels = input.failedFiles.filter(f => !isSkippablePath(f.path)); + const fileFailures = input.failedFiles.filter(f => isSkippablePath(f.path)); + + // Fast path: clean run touched no failures and no successes — nothing to + // reconcile in the ledger, just advance. + if (input.failedFiles.length === 0 && input.succeededPaths.length === 0) { + await input.advance(); + return { + advanced: true, + sentinelBlocked: false, + fresh: 0, + chronic: 0, + autoSkipped: [], + acknowledged: 0, + }; + } + + const attemptsByPath = _recordAndClear( + input.sourceId, + input.succeededPaths, + input.failedFiles, + input.commit, + ); + + const decision = decideGateAction({ + fileFailures, + sentinels, + attemptsByPath, + threshold, + skipFailed: input.skipFailed, + }); + + let fresh = 0; + let chronic = 0; + for (const f of fileFailures) { + if ((attemptsByPath.get(f.path) ?? 1) >= threshold && threshold > 0) chronic++; + else fresh++; + } + + if (decision.action === 'hard_block' || decision.action === 'block') { + return { + advanced: false, + sentinelBlocked: decision.action === 'hard_block', + fresh, + chronic, + autoSkipped: [], + acknowledged: 0, + }; + } + + // ATOMICITY: advance the bookmark BEFORE marking anything skipped/acked. + await input.advance(); + + let autoSkipped: string[] = []; + let acknowledged = 0; + if (input.skipFailed) { + acknowledged = acknowledgeFailures(input.sourceId).count; + } else if (decision.action === 'advance_then_autoskip') { + autoSkipped = decision.autoSkipPaths; + autoSkipFailures(input.sourceId, autoSkipped); + } + + return { + advanced: true, + sentinelBlocked: false, + fresh, + chronic, + autoSkipped, + acknowledged, + }; +} diff --git a/src/core/sync.ts b/src/core/sync.ts index 6604343a6..d92378bc8 100644 --- a/src/core/sync.ts +++ b/src/core/sync.ts @@ -468,282 +468,42 @@ export function resolveSlugForPath(filePath: string, repoPrefix?: string): strin } // ───────────────────────────────────────────────────────────────── -// Sync failure tracking — Bug 9 +// Sync failure ledger — moved to ./sync-failure-ledger.ts (issue #1939) // ───────────────────────────────────────────────────────────────── // -// When a sync run catches a per-file parse error (YAML with unquoted -// colons, malformed frontmatter, etc.), we record it here instead of just -// logging and moving on. Three goals: -// 1. Gate the sync.last_commit bookmark advance in all three sync paths -// (incremental, full/runImport, `gbrain import` git continuity). -// 2. Give users a visible record of what failed, with the commit hash -// they can use to re-attempt after fixing the source file. -// 3. Let `gbrain sync --skip-failed` acknowledge a known-bad set so -// repos with many broken files aren't permanently stuck. - -import { existsSync as _existsSync, readFileSync as _readFileSync, appendFileSync as _appendFileSync, mkdirSync as _mkdirSync } from 'fs'; -import { join as _joinPath } from 'path'; -import { gbrainPath as _gbrainPath } from './config.ts'; -import { createHash as _createHash } from 'crypto'; - -export interface SyncFailure { - path: string; - error: string; - /** Structured error code extracted from the error message. */ - code?: string; - commit: string; - line?: number; - ts: string; - acknowledged?: boolean; - acknowledged_at?: string; -} - -/** - * Best-effort extraction of a structured error code from a sync failure - * message. Matches known ParseValidationCode patterns (SLUG_MISMATCH, - * YAML_PARSE, etc.) and common DB / timeout errors. Returns 'UNKNOWN' - * when no pattern matches. - * - * Order matters: DB-layer errors are checked BEFORE YAML-layer ones so - * Postgres `duplicate key value violates unique constraint` doesn't get - * mislabeled as a YAML duplicate-key. Frontmatter patterns key off the - * canonical messages emitted by `collectValidationErrors()` in markdown.ts. - */ -export function classifyErrorCode(errorMsg: string): string { - // SLUG_MISMATCH: thrown by importFromFile() at src/core/import-file.ts:374. - if (/slug.*does not match|SLUG_MISMATCH/i.test(errorMsg)) return 'SLUG_MISMATCH'; - - // DB-layer errors come BEFORE the YAML duplicate-key check. Postgres unique- - // constraint violations contain "duplicate key" but are not a YAML problem. - if (/duplicate key value violates unique constraint|DB_DUPLICATE_KEY/i.test(errorMsg)) { - return 'DB_DUPLICATE_KEY'; - } - if (/canceling statement due to statement timeout|STATEMENT_TIMEOUT/i.test(errorMsg)) { - return 'STATEMENT_TIMEOUT'; - } - - // YAML / frontmatter patterns. These match either the canonical message - // strings in src/core/markdown.ts (collectValidationErrors) or the literal - // ParseValidationCode token, so they fire whether the caller stores the - // message or just the code. - if (/YAML parse failed|YAML_PARSE/i.test(errorMsg)) return 'YAML_PARSE'; - if (/YAMLException|duplicated mapping key|YAML_DUPLICATE_KEY/i.test(errorMsg)) { - return 'YAML_DUPLICATE_KEY'; - } - if (/File is empty or whitespace-only|Frontmatter must start with ---|MISSING_OPEN/i.test(errorMsg)) { - return 'MISSING_OPEN'; - } - if (/No closing --- delimiter|Heading at line .* found inside frontmatter|MISSING_CLOSE/i.test(errorMsg)) { - return 'MISSING_CLOSE'; - } - if (/Frontmatter block is empty|EMPTY_FRONTMATTER/i.test(errorMsg)) return 'EMPTY_FRONTMATTER'; - if (/Content contains null bytes|NULL_BYTES|null byte/i.test(errorMsg)) return 'NULL_BYTES'; - if (/Nested double quotes|NESTED_QUOTES/i.test(errorMsg)) return 'NESTED_QUOTES'; - - // Generic fallbacks. - if (/invalid UTF-?8|INVALID_UTF8/i.test(errorMsg)) return 'INVALID_UTF8'; - - // v0.22.12 additions: covers the four real production sites in src/core/import-file.ts - // (lines 199, 347, 352, 401) that previously bucketed to UNKNOWN. - if (/file too large|content too large|FILE_TOO_LARGE/i.test(errorMsg)) return 'FILE_TOO_LARGE'; - if (/skipping symlink|symlink|SYMLINK_NOT_ALLOWED/i.test(errorMsg)) return 'SYMLINK_NOT_ALLOWED'; - - // v0.32 takes-v2 additions: malformed fence rows + holder-grammar failures. - // TAKES_TABLE_MALFORMED and TAKES_ROW_NUM_COLLISION are produced by - // parseTakesFence (src/core/takes-fence.ts); TAKES_HOLDER_INVALID lands - // in v0.32 (EXP-4) when a holder doesn't match the world|brain|people/...| - // companies/... grammar. Wired into sync-failures.jsonl by the v0_28_0 - // migration's phaseBBackfill (one-time backfill emission). - if (/TAKES_TABLE_MALFORMED|TAKES_ROW_NUM_COLLISION|TAKES_FENCE_UNBALANCED/i.test(errorMsg)) { - return 'TAKES_TABLE_MALFORMED'; - } - if (/TAKES_HOLDER_INVALID/i.test(errorMsg)) return 'TAKES_HOLDER_INVALID'; - - // v0.41.6.0 D2: embedding error classification. Per-recipe verbatim shapes: - // native-openai → "OpenAI embedding requires OPENAI_API_KEY." - // native-google → "Google embedding requires GOOGLE_GENERATIVE_AI_API_KEY." - // openai-compat → "${recipe.name} embedding requires ${REQUIRED_ENV}." - // (Voyage AI / ZeroEntropy / DeepSeek / Together AI / - // DashScope / MiniMax / Zhipu AI all use this shape via - // defaultResolveAuth at src/core/ai/gateway.ts:250) - // EMBEDDING_NO_CREDS catches the missing-env case for every provider. The - // anthropic-no-touchpoint case ("Anthropic has no embedding model") is a - // misconfig, not a creds issue — bucketed separately so users don't get - // pointed at setting a key for a provider that doesn't offer embeddings. - if (/embedding requires [A-Z][A-Z0-9_]+_API_KEY|EMBEDDING_NO_CREDS/i.test(errorMsg)) { - return 'EMBEDDING_NO_CREDS'; - } - if (/Anthropic has no embedding model|EMBEDDING_NO_TOUCHPOINT/i.test(errorMsg)) { - return 'EMBEDDING_NO_TOUCHPOINT'; - } - // 429 status + textual rate-limit signals. AI SDK normalizes provider 429s - // into messages containing "rate limit" / "rate_limited" / "429". - if (/\brate.?limit|\b429\b|too many requests|rate_limited|RateLimit/i.test(errorMsg)) { - return 'EMBEDDING_RATE_LIMIT'; - } - // OpenAI: insufficient_quota / "exceeded your current quota". Anthropic: - // "credit balance is too low". Catch-all token: EMBEDDING_QUOTA. - if (/insufficient_quota|quota exceeded|exceeded.*quota|credit balance is too low|billing|EMBEDDING_QUOTA/i.test(errorMsg)) { - return 'EMBEDDING_QUOTA'; - } - // OpenAI: "maximum context length" / "too many tokens in request". Voyage: - // "input length exceeds". General: "max_tokens" / "context length". - if (/maximum context length|max_tokens|context length|input too long|input length exceeds|tokens? exceed|too many tokens|EMBEDDING_OVERSIZE/i.test(errorMsg)) { - return 'EMBEDDING_OVERSIZE'; - } - - // v0.41/v0.42 content-sanity gate. The ONLY throw path is junk under the - // `reject` disposition (the v0.42 default is `quarantine`, which lands the - // page hidden and never enters this classifier). The thrown - // ContentSanityBlockError embeds `PAGE_JUNK_PATTERN:` (see - // src/core/content-sanity.ts PAGE_JUNK_PATTERN_CODE). Soft-block (oversize) - // and flag (markup-heavy) also never throw — the page lands. So a - // PAGE_JUNK_PATTERN row in sync-failures.jsonl on a v0.42 brain means the - // operator opted into reject mode. - if (/PAGE_JUNK_PATTERN/i.test(errorMsg)) return 'PAGE_JUNK_PATTERN'; - - return 'UNKNOWN'; -} - -/** Group failures by error code and return a sorted summary. */ -export function summarizeFailuresByCode( - failures: Array<{ error: string; code?: string }>, -): Array<{ code: string; count: number }> { - const counts: Record = {}; - for (const f of failures) { - const code = f.code ?? classifyErrorCode(f.error); - counts[code] = (counts[code] ?? 0) + 1; - } - return Object.entries(counts) - .sort(([, a], [, b]) => b - a) - .map(([code, count]) => ({ code, count })); -} - -/** - * Format a code-grouped summary as a human-readable multi-line string for - * stderr / doctor output. Accepts either raw failures (which are summarized - * internally) or an already-summarized `{code, count}[]` shape (the return - * value of `summarizeFailuresByCode` or `AcknowledgeResult.summary`). - * Returns an empty string when the input is empty. - */ -export function formatCodeBreakdown( - input: Array<{ error: string; code?: string }> | Array<{ code: string; count: number }>, -): string { - // Distinguish by shape: summary entries have a numeric `count`. Empty array - // returns '' from either branch — both paths produce a 0-length join. - const summary = - input.length > 0 && typeof (input[0] as { count?: unknown }).count === 'number' - ? (input as Array<{ code: string; count: number }>) - : summarizeFailuresByCode(input as Array<{ error: string; code?: string }>); - return summary.map(s => ` ${s.code}: ${s.count}`).join('\n'); -} - -function _failuresDir(): string { - return _gbrainPath(); -} - -export function syncFailuresPath(): string { - return _joinPath(_failuresDir(), 'sync-failures.jsonl'); -} - -function _hashError(msg: string): string { - return _createHash('sha256').update(msg).digest('hex').slice(0, 12); -} - -function _dedupKey(f: { path: string; commit: string; error: string }): string { - return `${f.path}|${f.commit}|${_hashError(f.error)}`; -} - -/** - * Read the failures JSONL, skipping malformed lines with a warning to stderr. - * Returns empty array if the file doesn't exist. - */ -export function loadSyncFailures(): SyncFailure[] { - const path = syncFailuresPath(); - if (!_existsSync(path)) return []; - const raw = _readFileSync(path, 'utf-8'); - const out: SyncFailure[] = []; - for (const line of raw.split('\n')) { - const trimmed = line.trim(); - if (!trimmed) continue; - try { - out.push(JSON.parse(trimmed) as SyncFailure); - } catch { - console.warn(`[sync-failures] skipping malformed line: ${trimmed.slice(0, 120)}`); - } - } - return out; -} - -/** - * Append failure entries to the JSONL. Dedups by (path, commit, error-hash) — - * the same file failing with the same error on the same commit writes ONCE - * to the log, not once per sync run. - */ -export function recordSyncFailures( - failures: Array<{ path: string; error: string; line?: number }>, - commit: string, -): void { - if (failures.length === 0) return; - const existing = loadSyncFailures(); - const seen = new Set(existing.map(f => _dedupKey(f))); - - _mkdirSync(_failuresDir(), { recursive: true }); - const now = new Date().toISOString(); - for (const f of failures) { - const entry: SyncFailure = { - path: f.path, - error: f.error, - code: classifyErrorCode(f.error), - commit, - line: f.line, - ts: now, - }; - if (seen.has(_dedupKey(entry))) continue; - _appendFileSync(syncFailuresPath(), JSON.stringify(entry) + '\n'); - seen.add(_dedupKey(entry)); - } -} - -export interface AcknowledgeResult { - count: number; - summary: Array<{ code: string; count: number }>; -} - -/** - * Mark all unacknowledged failures as acknowledged. Used by - * `gbrain sync --skip-failed`. Returns count and a structured summary - * grouped by error code so the operator can see *why* files were skipped. - * - * We do not delete — acknowledged entries stay as historical record so - * doctor can still show them under a "previously skipped" bucket. - */ -export function acknowledgeSyncFailures(): AcknowledgeResult { - const entries = loadSyncFailures(); - if (entries.length === 0) return { count: 0, summary: [] }; - const now = new Date().toISOString(); - let changed = 0; - const newlyAcked: SyncFailure[] = []; - const updated = entries.map(e => { - if (e.acknowledged) return e; - changed++; - // Backfill code for entries that predate the code field. - const code = e.code ?? classifyErrorCode(e.error); - const acked = { ...e, code, acknowledged: true, acknowledged_at: now }; - newlyAcked.push(acked); - return acked; - }); - if (changed === 0) return { count: 0, summary: [] }; - _mkdirSync(_failuresDir(), { recursive: true }); - const fd = require('fs').writeFileSync; - fd(syncFailuresPath(), updated.map(e => JSON.stringify(e)).join('\n') + '\n'); - return { - count: changed, - summary: summarizeFailuresByCode(newlyAcked), - }; -} - -/** Return only unacknowledged failures. */ -export function unacknowledgedSyncFailures(): SyncFailure[] { - return loadSyncFailures().filter(f => !f.acknowledged); -} +// The failure store + bounded auto-skip valve now live in a leaf module so +// they can be unit-tested in isolation and shared by both sync gates without +// a circular import. Re-exported here so existing callers that +// `await import('../core/sync.ts')` for these symbols keep working. +export { + classifyErrorCode, + summarizeFailuresByCode, + formatCodeBreakdown, + syncFailuresPath, + loadSyncFailures, + unacknowledgedSyncFailures, + recordSyncFailures, + acknowledgeSyncFailures, + recordFailures, + clearFailures, + acknowledgeFailures, + autoSkipFailures, + withLedgerLock, + resolveAutoSkipThreshold, + isSkippablePath, + decideGateAction, + decideSyncFailureSeverity, + applySyncFailureGate, + DEFAULT_SOURCE_ID, + SENTINEL_PREFIX, + DEFAULT_AUTOSKIP_AFTER, +} from './sync-failure-ledger.ts'; +export type { + SyncFailure, + SyncFailureState, + AcknowledgeResult, + GateDecision, + SeverityResult, + SyncGateInput, + SyncGateOutcome, +} from './sync-failure-ledger.ts'; diff --git a/test/content-sanity.test.ts b/test/content-sanity.test.ts index d94bef96f..80e2aaa98 100644 --- a/test/content-sanity.test.ts +++ b/test/content-sanity.test.ts @@ -640,3 +640,28 @@ describe('assessContentSanity — confidence split (Q1=A)', () => { expect(r.shouldFlag).toBe(false); }); }); + +// issue #1939 — assessContentSanity 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. It must coerce defensively and never throw. +describe('issue #1939 — non-string title defensive coercion', () => { + const base = { compiled_truth: 'some prose body here', timeline: '' }; + + test('Date title does not throw', () => { + expect(() => + assessContentSanity({ ...base, title: new Date('2024-06-01') as unknown as string }), + ).not.toThrow(); + }); + + test('number title does not throw', () => { + expect(() => + assessContentSanity({ ...base, title: 1458 as unknown as string }), + ).not.toThrow(); + }); + + test('null/undefined title does not throw and yields a normal result', () => { + const r = assessContentSanity({ ...base, title: undefined as unknown as string }); + expect(r).toBeDefined(); + expect(typeof r.shouldQuarantine).toBe('boolean'); + }); +}); diff --git a/test/e2e/sync.test.ts b/test/e2e/sync.test.ts index e511d79c7..e8cc62631 100644 --- a/test/e2e/sync.test.ts +++ b/test/e2e/sync.test.ts @@ -553,4 +553,105 @@ describeE2E('E2E: sync --skip-failed structured summary loop (v0.22.12, issue #5 const finalSummary = summarizeFailuresByCode(failures); expect(finalSummary).toEqual([{ code: 'SLUG_MISMATCH', count: 2 }]); }); + + // issue #1939 CRITICAL REGRESSION: a page whose YAML title parses to a Date + // (or number) must import cleanly — pre-fix it threw in assessContentSanity + // and wedged the bookmark. This mirrors the apple-notes repro + // (sources/apple-notes/.../2024-06-01 8189165238.md). + test('date/number-titled page imports cleanly; bookmark advances; get returns it', async () => { + const { performSync } = await import('../../src/commands/sync.ts'); + const { loadSyncFailures } = await import('../../src/core/sync.ts'); + const engine = getEngine(); + + const beforeCommit = await engine.getConfig('sync.last_commit'); + // Bare-date title (→ Date) and bare-number title (→ number). + writeFileSync(join(repoPath, 'people/datey.md'), [ + '---', 'type: note', 'title: 2024-06-01', '---', '', 'Apple note body.', + ].join('\n')); + writeFileSync(join(repoPath, 'people/numbery.md'), [ + '---', 'type: note', 'title: 1458', '---', '', 'Another note.', + ].join('\n')); + execSync('git add -A && git commit -m "add date/number titled notes"', { cwd: repoPath, stdio: 'pipe' }); + + const result = await performSync(engine, { repoPath, noPull: true, noEmbed: true }); + expect(result.status).not.toBe('blocked_by_failures'); + + // Neither file landed as a failure. + const fails = loadSyncFailures().filter(f => f.path.includes('datey') || f.path.includes('numbery')); + expect(fails.length).toBe(0); + + // Bookmark advanced past the broken-but-now-fixed commit. + const afterCommit = await engine.getConfig('sync.last_commit'); + expect(afterCommit).not.toBe(beforeCommit); + + // The pages are retrievable, with deterministic coerced titles. + const datey = await engine.getPage('people/datey'); + expect(datey).not.toBeNull(); + expect(datey!.title).toBe('2024-06-01'); + const numbery = await engine.getPage('people/numbery'); + expect(numbery).not.toBeNull(); + expect(numbery!.title).toBe('1458'); + }); + + // issue #1939 valve: a genuinely un-importable file blocks for (threshold-1) + // syncs, then auto-skips on the Nth so it can't wedge indexing forever. + test('bounded auto-skip: poison file blocks then auto-skips, advancing the bookmark', async () => { + const { performSync } = await import('../../src/commands/sync.ts'); + const { loadSyncFailures } = await import('../../src/core/sync.ts'); + const engine = getEngine(); + const prevThreshold = process.env.GBRAIN_SYNC_AUTOSKIP_AFTER; + process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = '2'; + try { + const beforeCommit = await engine.getConfig('sync.last_commit'); + // slug-mismatch = a reliable per-file import failure. + writeFileSync(join(repoPath, 'people/poison.md'), [ + '---', 'type: person', 'title: Poison', 'slug: not-the-path-slug', '---', '', 'Body.', + ].join('\n')); + execSync('git add -A && git commit -m "add poison file"', { cwd: repoPath, stdio: 'pipe' }); + + // Attempt 1: blocks (attempts=1 < 2). + let result = await performSync(engine, { repoPath, noPull: true, noEmbed: true }); + expect(result.status).toBe('blocked_by_failures'); + expect(await engine.getConfig('sync.last_commit')).toBe(beforeCommit); + let poison = loadSyncFailures().find(f => f.path.includes('poison'))!; + expect(poison.state).toBe('open'); + expect(poison.attempts).toBe(1); + + // Attempt 2: attempts hits threshold, fresh==0 → auto-skip + advance. + result = await performSync(engine, { repoPath, noPull: true, noEmbed: true }); + expect(result.status).toBe('synced'); + expect(await engine.getConfig('sync.last_commit')).not.toBe(beforeCommit); + poison = loadSyncFailures().find(f => f.path.includes('poison'))!; + expect(poison.state).toBe('auto_skipped'); + } finally { + if (prevThreshold === undefined) delete process.env.GBRAIN_SYNC_AUTOSKIP_AFTER; + else process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = prevThreshold; + } + }); + + // issue #1939 adversarial finding #1: a parse-failed file that is later DELETED + // from the repo must not leave a permanent open ledger row (which would age + // doctor to FAIL forever). The incremental gate treats removed paths as resolved. + test('deleting a failed file clears its ledger row (self-heal, no stuck FAIL)', async () => { + const { performSync } = await import('../../src/commands/sync.ts'); + const { loadSyncFailures } = await import('../../src/core/sync.ts'); + const engine = getEngine(); + + writeFileSync(join(repoPath, 'people/gonepoison.md'), [ + '---', 'type: person', 'title: Gone', 'slug: wrong-derived-slug', '---', '', 'Body.', + ].join('\n')); + execSync('git add -A && git commit -m "add a file that fails to parse"', { cwd: repoPath, stdio: 'pipe' }); + + // Sync blocks; an open ledger row exists for the bad file. + let result = await performSync(engine, { repoPath, noPull: true, noEmbed: true }); + expect(result.status).toBe('blocked_by_failures'); + expect(loadSyncFailures().some(f => f.path.includes('gonepoison'))).toBe(true); + + // Delete the file and sync. The removed path is treated as resolved, so the + // ledger row is cleared and the bookmark advances. + execSync('git rm people/gonepoison.md && git commit -m "delete the bad file"', { cwd: repoPath, stdio: 'pipe' }); + result = await performSync(engine, { repoPath, noPull: true, noEmbed: true }); + expect(result.status).not.toBe('blocked_by_failures'); + expect(loadSyncFailures().some(f => f.path.includes('gonepoison'))).toBe(false); + }); }); diff --git a/test/fixtures/e5-lease-cap-ab/2026-05-24-baseline-dry-run.json b/test/fixtures/e5-lease-cap-ab/2026-05-24-baseline-dry-run.json index b725d6c8a..3a47320b0 100644 --- a/test/fixtures/e5-lease-cap-ab/2026-05-24-baseline-dry-run.json +++ b/test/fixtures/e5-lease-cap-ab/2026-05-24-baseline-dry-run.json @@ -40,4 +40,4 @@ "pr_gate_pass": false, "note": "controller does NOT meet PR gate; defaults stay OFF" } -} \ No newline at end of file +} diff --git a/test/markdown.test.ts b/test/markdown.test.ts index 2d6f165ba..2e485f521 100644 --- a/test/markdown.test.ts +++ b/test/markdown.test.ts @@ -302,3 +302,44 @@ Some content.`; expect(parseMarkdown('', 'projects/blog/writing/essay.md').type).toBe('writing'); }); }); + +// issue #1939 — js-yaml parses `title: 2024-06-01` as a Date and `title: 1458` +// as a number. The old `(frontmatter.title as string)` cast was a compile-time +// lie; at runtime downstream `.toLowerCase()` threw and wedged sync. Coercion +// must be non-throwing AND deterministic (UTC ISO for dates, no timezone drift). +describe('issue #1939 — non-string frontmatter coercion', () => { + test('date title coerces to its UTC ISO date string', () => { + const parsed = parseMarkdown('---\ntitle: 2024-06-01\n---\nbody\n', 'apple-notes/x.md'); + expect(parsed.title).toBe('2024-06-01'); + expect(typeof parsed.title).toBe('string'); + }); + + test('number title coerces to its string form', () => { + const parsed = parseMarkdown('---\ntitle: 1458\n---\nbody\n', 'apple-notes/x.md'); + expect(parsed.title).toBe('1458'); + }); + + test('date title is timezone-independent (UTC) — repro file shape', () => { + // sources/apple-notes/YC/Talks YC/2023-04-25 1458.md style page. + const parsed = parseMarkdown('---\ntitle: 2023-04-25\n---\nnotes\n', 'apple-notes/2023-04-25 1458.md'); + expect(parsed.title).toBe('2023-04-25'); // never "Mon Apr 24 2023 ...GMT-0700" + }); + + test('date/number slug + type coerce without throwing', () => { + const parsed = parseMarkdown('---\nslug: 2024-06-01\ntype: 2024\n---\nbody\n', 'x.md'); + expect(typeof parsed.slug).toBe('string'); + expect(parsed.slug).toBe('2024-06-01'); + expect(typeof parsed.type).toBe('string'); + }); + + test('missing/empty title falls back to inferred title (no throw)', () => { + const parsed = parseMarkdown('---\ntype: note\n---\nbody\n', 'people/alice-example.md'); + expect(typeof parsed.title).toBe('string'); + expect(parsed.title.length).toBeGreaterThan(0); + }); + + test('string title still passes through unchanged', () => { + const parsed = parseMarkdown('---\ntitle: A Normal Title\n---\nbody\n', 'x.md'); + expect(parsed.title).toBe('A Normal Title'); + }); +}); diff --git a/test/sync-failure-ledger.serial.test.ts b/test/sync-failure-ledger.serial.test.ts new file mode 100644 index 000000000..76eab4e3a --- /dev/null +++ b/test/sync-failure-ledger.serial.test.ts @@ -0,0 +1,377 @@ +/** + * issue #1939 — sync failure ledger + bounded auto-skip valve. + * + * Covers the correctness gates the /codex outside-voice review identified: + * #1 auto-skipped entries stay UNRESOLVED (doctor WARN), not hidden + * #2 (source_id, path) keying — failures never merge across sources + * #3 `` sentinel never auto-skips; always hard-blocks + * #4 success clears a path so `attempts` is truly consecutive + * #5 advance-before-ack atomicity (a throwing advance marks nothing) + * #7 legacy rows normalize + duplicates collapse deterministically + * #8 cross-process lock + atomic write (stale-lock break, no partial file) + */ + +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { mkdtempSync, rmSync, readFileSync, writeFileSync, mkdirSync, existsSync, utimesSync, openSync, closeSync } from 'fs'; +import { join, dirname } from 'path'; +import { tmpdir } from 'os'; + +let tmpHome: string; +const originalHome = process.env.HOME; +const originalThreshold = process.env.GBRAIN_SYNC_AUTOSKIP_AFTER; + +beforeEach(async () => { + tmpHome = mkdtempSync(join(tmpdir(), 'gbrain-ledger-')); + process.env.HOME = tmpHome; + delete process.env.GBRAIN_SYNC_AUTOSKIP_AFTER; + const { syncFailuresPath } = await import('../src/core/sync-failure-ledger.ts'); + try { rmSync(syncFailuresPath(), { force: true }); } catch { /* none */ } +}); + +afterEach(() => { + if (originalHome) process.env.HOME = originalHome; else delete process.env.HOME; + if (originalThreshold === undefined) delete process.env.GBRAIN_SYNC_AUTOSKIP_AFTER; + else process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = originalThreshold; + try { rmSync(tmpHome, { recursive: true, force: true }); } catch { /* ignore */ } +}); + +async function L() { + return import('../src/core/sync-failure-ledger.ts'); +} + +describe('#2 multi-source keying', () => { + test('same relative path in two sources keeps independent counters', async () => { + const { recordFailures, loadSyncFailures } = await L(); + recordFailures('alpha', [{ path: 'people/x.md', error: 'YAML parse failed' }], 'c1'); + recordFailures('alpha', [{ path: 'people/x.md', error: 'YAML parse failed' }], 'c2'); + recordFailures('beta', [{ path: 'people/x.md', error: 'YAML parse failed' }], 'c1'); + + const rows = loadSyncFailures(); + expect(rows.length).toBe(2); + const alpha = rows.find(r => r.source_id === 'alpha')!; + const beta = rows.find(r => r.source_id === 'beta')!; + expect(alpha.attempts).toBe(2); + expect(beta.attempts).toBe(1); + }); + + test('acknowledgeFailures(sourceId) only acks that source', async () => { + const { recordFailures, acknowledgeFailures, loadSyncFailures } = await L(); + recordFailures('alpha', [{ path: 'a.md', error: 'e' }], 'c1'); + recordFailures('beta', [{ path: 'b.md', error: 'e' }], 'c1'); + + const res = acknowledgeFailures('alpha'); + expect(res.count).toBe(1); + const rows = loadSyncFailures(); + expect(rows.find(r => r.source_id === 'alpha')!.state).toBe('acknowledged'); + expect(rows.find(r => r.source_id === 'beta')!.state).toBe('open'); + }); +}); + +describe('#4 success clears → consecutive attempts', () => { + test('clearFailures removes a path; a later failure restarts at 1', async () => { + const { recordFailures, clearFailures, loadSyncFailures } = await L(); + recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c1'); + recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c2'); + expect(loadSyncFailures()[0].attempts).toBe(2); + + clearFailures('s', ['a.md']); + expect(loadSyncFailures().length).toBe(0); + + recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c3'); + expect(loadSyncFailures()[0].attempts).toBe(1); + }); + + test('clearFailures is source-scoped', async () => { + const { recordFailures, clearFailures, loadSyncFailures } = await L(); + recordFailures('s1', [{ path: 'a.md', error: 'e' }], 'c1'); + recordFailures('s2', [{ path: 'a.md', error: 'e' }], 'c1'); + clearFailures('s1', ['a.md']); + const rows = loadSyncFailures(); + expect(rows.length).toBe(1); + expect(rows[0].source_id).toBe('s2'); + }); +}); + +describe('#3 sentinel never auto-skips', () => { + test('decideGateAction hard-blocks on a sentinel even when chronic + skipFailed', async () => { + const { decideGateAction } = await L(); + const d = decideGateAction({ + fileFailures: [], + sentinels: [{ path: '' }], + attemptsByPath: new Map(), + threshold: 3, + skipFailed: true, + }); + expect(d.action).toBe('hard_block'); + expect(d.autoSkipPaths).toEqual([]); + }); + + test('autoSkipFailures refuses to skip a sentinel path', async () => { + const { recordFailures, autoSkipFailures, loadSyncFailures } = await L(); + recordFailures('s', [{ path: '', error: 'history rewrite' }], 'c1'); + const res = autoSkipFailures('s', ['']); + expect(res.count).toBe(0); + expect(loadSyncFailures()[0].state).toBe('open'); + }); + + test('isSkippablePath', async () => { + const { isSkippablePath } = await L(); + expect(isSkippablePath('people/x.md')).toBe(true); + expect(isSkippablePath('')).toBe(false); + }); +}); + +describe('#7 legacy normalization + dup collapse', () => { + test('backfills source_id/state/attempts/first_seen on legacy rows', async () => { + const { loadSyncFailures, syncFailuresPath } = await L(); + mkdirSync(dirname(syncFailuresPath()), { recursive: true }); + writeFileSync( + syncFailuresPath(), + JSON.stringify({ path: 'a.md', error: 'YAML parse failed', commit: 'old', ts: '2025-01-01T00:00:00Z' }) + '\n' + + JSON.stringify({ path: 'b.md', error: 'e', commit: 'old', ts: '2025-01-02T00:00:00Z', acknowledged_at: '2025-02-01T00:00:00Z' }) + '\n', + ); + const rows = loadSyncFailures(); + const a = rows.find(r => r.path === 'a.md')!; + const b = rows.find(r => r.path === 'b.md')!; + expect(a.source_id).toBe('default'); + expect(a.state).toBe('open'); + expect(a.attempts).toBe(1); + expect(a.first_seen).toBe('2025-01-01T00:00:00Z'); + expect(b.state).toBe('acknowledged'); + }); + + test('collapses duplicate open rows for one (source,path) into one with attempts = distinct commits', async () => { + const { loadSyncFailures, syncFailuresPath } = await L(); + mkdirSync(dirname(syncFailuresPath()), { recursive: true }); + // Three legacy rows, same path, two distinct commits. + writeFileSync( + syncFailuresPath(), + [ + { path: 'a.md', error: 'e', commit: 'c1', ts: '2025-01-01T00:00:00Z' }, + { path: 'a.md', error: 'e', commit: 'c2', ts: '2025-01-02T00:00:00Z' }, + { path: 'a.md', error: 'e2', commit: 'c2', ts: '2025-01-03T00:00:00Z' }, + ].map(r => JSON.stringify(r)).join('\n') + '\n', + ); + const rows = loadSyncFailures(); + expect(rows.length).toBe(1); + expect(rows[0].attempts).toBe(2); // distinct commits c1, c2 + expect(rows[0].commit).toBe('c2'); // latest by ts + expect(rows[0].first_seen).toBe('2025-01-01T00:00:00Z'); + }); + + test('skips malformed lines', async () => { + const { loadSyncFailures, recordFailures, syncFailuresPath } = await L(); + recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c1'); + writeFileSync(syncFailuresPath(), readFileSync(syncFailuresPath(), 'utf-8') + 'NOT-JSON\n'); + expect(loadSyncFailures().length).toBe(1); + }); +}); + +describe('#1 severity — auto_skipped stays visible (WARN)', () => { + test('decideSyncFailureSeverity branches', async () => { + const { decideSyncFailureSeverity } = await L(); + const base = { source_id: 's', path: 'a.md', error: 'e', code: 'X', commit: 'c', first_seen: '', ts: '', attempts: 1 }; + const now = Date.parse('2026-06-07T00:00:00Z'); + const failHours = 72; + + // empty → ok + expect(decideSyncFailureSeverity({ entries: [], nowMs: now, failHours }).status).toBe('ok'); + + // one recent open → warn + expect(decideSyncFailureSeverity({ + entries: [{ ...base, state: 'open', ts: '2026-06-06T18:00:00Z' }], + nowMs: now, failHours, + }).status).toBe('warn'); + + // one OLD open (>72h) → fail + expect(decideSyncFailureSeverity({ + entries: [{ ...base, state: 'open', ts: '2026-06-01T00:00:00Z' }], + nowMs: now, failHours, + }).status).toBe('fail'); + + // 10 recent OPEN → fail (count of blocking failures) + const ten = Array.from({ length: 10 }, (_, i) => ({ ...base, path: `a${i}.md`, state: 'open' as const, ts: '2026-06-06T23:00:00Z' })); + expect(decideSyncFailureSeverity({ entries: ten, nowMs: now, failHours }).status).toBe('fail'); + + // 10 recent AUTO_SKIPPED → still warn (#3): the valve already advanced the + // bookmark, so indexing is not wedged. Visible, not gating. + const tenSkipped = Array.from({ length: 10 }, (_, i) => ({ ...base, path: `s${i}.md`, state: 'auto_skipped' as const, ts: '2026-06-06T23:00:00Z' })); + const skSev = decideSyncFailureSeverity({ entries: tenSkipped, nowMs: now, failHours }); + expect(skSev.status).toBe('warn'); + expect(skSev.auto_skipped).toBe(10); + + // auto_skipped only → warn (still visible), counted + const sev = decideSyncFailureSeverity({ + entries: [{ ...base, state: 'auto_skipped', ts: '2026-06-01T00:00:00Z' }], + nowMs: now, failHours, + }); + expect(sev.status).toBe('warn'); + expect(sev.auto_skipped).toBe(1); + expect(sev.unresolved).toBe(1); + + // acknowledged only → ok + expect(decideSyncFailureSeverity({ + entries: [{ ...base, state: 'acknowledged', ts: '2026-06-01T00:00:00Z' }], + nowMs: now, failHours, + }).status).toBe('ok'); + }); + + test('malformed ts never crashes (treated as not-old)', async () => { + const { decideSyncFailureSeverity } = await L(); + const sev = decideSyncFailureSeverity({ + entries: [{ source_id: 's', path: 'a.md', error: 'e', code: 'X', commit: 'c', first_seen: '', ts: 'not-a-date', attempts: 1, state: 'open' }], + nowMs: Date.now(), failHours: 72, + }); + expect(sev.status).toBe('warn'); + }); + + test('auto_skipped row keeps acknowledged_at null so legacy !acknowledged_at readers still count it', async () => { + const { recordFailures, autoSkipFailures, loadSyncFailures } = await L(); + recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c1'); + autoSkipFailures('s', ['a.md']); + const row = loadSyncFailures()[0]; + expect(row.state).toBe('auto_skipped'); + expect(row.acknowledged).toBe(false); + expect(row.acknowledged_at).toBeNull(); + }); +}); + +describe('decideGateAction — full branch table', () => { + test('all branches', async () => { + const { decideGateAction } = await L(); + const mk = (over: Partial[0]>) => decideGateAction({ + fileFailures: [], sentinels: [], attemptsByPath: new Map(), threshold: 3, skipFailed: false, ...over, + }); + + expect(mk({}).action).toBe('advance'); // no failures + expect(mk({ fileFailures: [{ path: 'a' }], skipFailed: true }).action).toBe('advance'); + expect(mk({ fileFailures: [{ path: 'a' }], threshold: 0 }).action).toBe('block'); // valve off + expect(mk({ fileFailures: [{ path: 'a' }], attemptsByPath: new Map([['a', 1]]) }).action).toBe('block'); // fresh + + const chronic = mk({ fileFailures: [{ path: 'a' }], attemptsByPath: new Map([['a', 3]]) }); + expect(chronic.action).toBe('advance_then_autoskip'); + expect(chronic.autoSkipPaths).toEqual(['a']); + + // mixed fresh + chronic → block (don't silently drop the fresh one) + expect(mk({ + fileFailures: [{ path: 'a' }, { path: 'b' }], + attemptsByPath: new Map([['a', 3], ['b', 1]]), + }).action).toBe('block'); + }); +}); + +describe('#5 + #6 applySyncFailureGate orchestration', () => { + test('advance_then_autoskip after threshold; advance runs before auto-skip mark', async () => { + process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = '3'; + const { applySyncFailureGate, loadSyncFailures } = await L(); + let advanceCalls = 0; + const run = () => applySyncFailureGate({ + sourceId: 's', failedFiles: [{ path: 'poison.md', error: 'YAML parse failed' }], + succeededPaths: [], commit: 'c', skipFailed: false, + advance: async () => { advanceCalls++; }, + }); + + let r = await run(); + expect(r.advanced).toBe(false); // attempts 1 → block + r = await run(); + expect(r.advanced).toBe(false); // attempts 2 → block + r = await run(); + expect(r.advanced).toBe(true); // attempts 3 → advance + auto-skip + expect(r.autoSkipped).toEqual(['poison.md']); + expect(advanceCalls).toBe(1); + expect(loadSyncFailures()[0].state).toBe('auto_skipped'); + }); + + test('a throwing advance() marks nothing auto_skipped (atomicity)', async () => { + process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = '1'; + const { applySyncFailureGate, loadSyncFailures } = await L(); + await expect(applySyncFailureGate({ + sourceId: 's', failedFiles: [{ path: 'poison.md', error: 'e' }], + succeededPaths: [], commit: 'c', skipFailed: false, + advance: async () => { throw new Error('db write failed'); }, + })).rejects.toThrow('db write failed'); + // Recorded as open, NOT auto_skipped — next run can retry. + const row = loadSyncFailures()[0]; + expect(row.state).toBe('open'); + }); + + test('sentinel hard-blocks even with skipFailed; no advance', async () => { + const { applySyncFailureGate } = await L(); + let advanced = false; + const r = await applySyncFailureGate({ + sourceId: 's', failedFiles: [{ path: '', error: 'history rewrite' }], + succeededPaths: [], commit: 'c', skipFailed: true, + advance: async () => { advanced = true; }, + }); + expect(r.advanced).toBe(false); + expect(r.sentinelBlocked).toBe(true); + expect(advanced).toBe(false); + }); + + test('succeeded paths clear prior failures even with no new failures', async () => { + const { recordFailures, applySyncFailureGate, loadSyncFailures } = await L(); + recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c1'); + expect(loadSyncFailures().length).toBe(1); + const r = await applySyncFailureGate({ + sourceId: 's', failedFiles: [], succeededPaths: ['a.md'], commit: 'c2', + skipFailed: false, advance: async () => {}, + }); + expect(r.advanced).toBe(true); + expect(loadSyncFailures().length).toBe(0); + }); + + test('skipFailed acknowledges and advances', async () => { + const { applySyncFailureGate, loadSyncFailures } = await L(); + const r = await applySyncFailureGate({ + sourceId: 's', failedFiles: [{ path: 'a.md', error: 'e' }], + succeededPaths: [], commit: 'c', skipFailed: true, advance: async () => {}, + }); + expect(r.advanced).toBe(true); + expect(r.acknowledged).toBe(1); + expect(loadSyncFailures()[0].state).toBe('acknowledged'); + }); +}); + +describe('#8 concurrency: lock + atomic write', () => { + test('atomic rewrite leaves valid JSON lines (no partial)', async () => { + const { recordFailures, syncFailuresPath } = await L(); + recordFailures('s', [{ path: 'a.md', error: 'e' }, { path: 'b.md', error: 'e' }], 'c1'); + const raw = readFileSync(syncFailuresPath(), 'utf-8'); + for (const line of raw.split('\n').filter(Boolean)) { + expect(() => JSON.parse(line)).not.toThrow(); + } + expect(existsSync(syncFailuresPath() + '.lock')).toBe(false); // lock released + }); + + test('a stale lock (old mtime) is broken so a mutation still proceeds', async () => { + const { recordFailures, syncFailuresPath, withLedgerLock } = await L(); + // Pre-create the data dir + a stale lock file. + mkdirSync(dirname(syncFailuresPath()), { recursive: true }); + const lockPath = syncFailuresPath() + '.lock'; + closeSync(openSync(lockPath, 'w')); + const old = new Date(Date.now() - 120_000); // 2 min ago > 30s stale window + utimesSync(lockPath, old, old); + + // Mutation should break the stale lock and succeed. + recordFailures('s', [{ path: 'a.md', error: 'e' }], 'c1'); + const { loadSyncFailures } = await L(); + expect(loadSyncFailures().length).toBe(1); + + // withLedgerLock returns the callback's value. + expect(withLedgerLock(() => 42)).toBe(42); + }); +}); + +describe('resolveAutoSkipThreshold', () => { + test('default 3, env override, 0 disables, invalid → default', async () => { + const { resolveAutoSkipThreshold } = await L(); + delete process.env.GBRAIN_SYNC_AUTOSKIP_AFTER; + expect(resolveAutoSkipThreshold()).toBe(3); + process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = '5'; + expect(resolveAutoSkipThreshold()).toBe(5); + process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = '0'; + expect(resolveAutoSkipThreshold()).toBe(0); + process.env.GBRAIN_SYNC_AUTOSKIP_AFTER = 'nonsense'; + expect(resolveAutoSkipThreshold()).toBe(3); + }); +}); diff --git a/test/sync-failures.test.ts b/test/sync-failures.test.ts index b50a47d87..63ccb6a29 100644 --- a/test/sync-failures.test.ts +++ b/test/sync-failures.test.ts @@ -40,7 +40,11 @@ afterEach(() => { }); describe('Bug 9 — sync-failures JSONL helpers', () => { - test('recordSyncFailures appends one line per failure with dedup', async () => { + // issue #1939: recordSyncFailures now upserts by (source_id, path) and + // increments `attempts` (consecutive failed runs) instead of appending a row + // per (path, commit, error). One row per failing path; the attempt counter + // drives the bounded auto-skip valve. + test('recordSyncFailures upserts per path, incrementing attempts', async () => { const { recordSyncFailures, loadSyncFailures, syncFailuresPath } = await import('../src/core/sync.ts'); recordSyncFailures([ @@ -51,21 +55,28 @@ describe('Bug 9 — sync-failures JSONL helpers', () => { expect(existsSync(syncFailuresPath())).toBe(true); const entries = loadSyncFailures(); expect(entries.length).toBe(2); - expect(entries[0].path).toBe('people/alice.md'); - expect(entries[0].commit).toBe('abc123def456'); - expect(entries[0].acknowledged).toBeUndefined(); + const alice = entries.find(e => e.path === 'people/alice.md')!; + expect(alice.commit).toBe('abc123def456'); + expect(alice.state).toBe('open'); + expect(alice.attempts).toBe(1); + expect(alice.acknowledged).toBe(false); - // Same failure on same commit should NOT re-append. + // Same path again (same commit) → still ONE row, attempts climbs. recordSyncFailures([ { path: 'people/alice.md', error: 'YAML: unexpected colon in title' }, ], 'abc123def456'); expect(loadSyncFailures().length).toBe(2); + expect(loadSyncFailures().find(e => e.path === 'people/alice.md')!.attempts).toBe(2); - // Different commit → new entry. + // Different commit → still upserted (not appended), attempts keeps climbing. recordSyncFailures([ { path: 'people/alice.md', error: 'YAML: unexpected colon in title' }, ], 'zzz999'); - expect(loadSyncFailures().length).toBe(3); + const after = loadSyncFailures(); + expect(after.length).toBe(2); + const alice2 = after.find(e => e.path === 'people/alice.md')!; + expect(alice2.attempts).toBe(3); + expect(alice2.commit).toBe('zzz999'); }); test('acknowledgeSyncFailures marks unacked entries, leaves acked alone', async () => { @@ -129,6 +140,18 @@ describe('Bug 9 — doctor surfaces sync failures', () => { expect(source).toContain('unacknowledgedSyncFailures'); expect(source).toContain("'gbrain sync --skip-failed'"); }); + + // issue #1939: BOTH doctor surfaces (local buildChecks + remote + // doctorReportRemote) must decide severity through the one shared helper so + // they can never drift. The remote surface must no longer hand-roll an + // `acknowledged_at` count (the old field-split that caused drift). + test('both doctor surfaces use the shared decideSyncFailureSeverity', async () => { + const source = await Bun.file(new URL('../src/commands/doctor.ts', import.meta.url)).text(); + const occurrences = source.split('decideSyncFailureSeverity').length - 1; + expect(occurrences).toBeGreaterThanOrEqual(2); // local + remote + // remote no longer counts `!entry.acknowledged_at` by hand. + expect(source).not.toContain('if (!entry.acknowledged_at) unacked++'); + }); }); describe('Bug 9 — sync.ts CLI flag wiring', () => { @@ -169,23 +192,25 @@ describe('Bug 9 — sync.ts CLI flag wiring', () => { expect(unacknowledgedSyncFailures().length).toBe(0); }); - test('performSync gates sync.last_commit on failedFiles.length', async () => { + test('performSync gates the bookmark through the shared failure ledger', async () => { const source = await Bun.file(new URL('../src/commands/sync.ts', import.meta.url)).text(); - // The gate exists and references the failure set. - expect(source).toContain('failedFiles.length > 0'); + // issue #1939: the gate is now the shared applySyncFailureGate orchestrator. + expect(source).toContain('applySyncFailureGate'); expect(source).toContain('blocked_by_failures'); }); - test('performFullSync gates on result.failures from runImport', async () => { + test('performFullSync routes failures through the same shared gate', async () => { const source = await Bun.file(new URL('../src/commands/sync.ts', import.meta.url)).text(); - expect(source).toContain('result.failures.length > 0'); + expect(source).toContain('result.failures'); + expect(source).toContain('applySyncFailureGate'); }); - test('runImport returns RunImportResult with failures list', async () => { + test('runImport returns RunImportResult and records via the ledger', async () => { const source = await Bun.file(new URL('../src/commands/import.ts', import.meta.url)).text(); expect(source).toContain('RunImportResult'); expect(source).toContain('failures: Array<{ path: string; error: string }>'); - expect(source).toContain('recordSyncFailures'); + // issue #1939: import records source-scoped via recordFailures (sourceId, …). + expect(source).toContain('recordFailures'); }); });