fix(sync): acknowledge auto-skipped failures (#3829) (#3891)

Wave-assembled from PR #3891 by @RerankerGuo. Conflict resolution: KEY_FILES.md
kept master's newer migrate-engine/import-file/sync entries; the PR's 3-state
machine wording re-applied onto master's sync-failure-ledger entry.

Co-Authored-By: RerankerGuo <121015044+RerankerGuo@users.noreply.github.com>
This commit is contained in:
test
2026-08-13 12:18:13 -07:00
committed by Sina Matian
co-authored by RerankerGuo
parent 136fc109c1
commit fd0e371d5b
3 changed files with 26 additions and 7 deletions
+1 -1
View File
@@ -53,7 +53,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this
- `src/commands/migrate-engine.ts` — Bidirectional engine migration (`gbrain migrate --to supabase/pglite`). Copies the complete source catalog FIRST (`copyMigrationSources` — every `sources` row incl. archived rows and sync/routing metadata, `ON CONFLICT (id) DO UPDATE`, `default` ordered first) so every page write has a valid `pages.source_id` FK parent and the target preserves per-source behavior; pages copy afterward, tracked in the resume manifest by composite `(source_id, slug)` key. Link copy preserves each row's `to_source_id` (falling back to the origin source only for legacy rows without it), and failed-target filtering uses that same target composite key, so cross-source links migrate without being rebound to the origin source. The resume manifest is target-aware: `migrationTargetId(config)` hashes `(engine, locator)` (`database_url` for Postgres, resolved `database_path` for PGLite) and `manifestMatchesTarget` requires `schema_version === 2` plus a matching `target_id` — a legacy engine-only manifest, or one from a DIFFERENT target of the same engine kind, starts fresh instead of skipping "completed" pages the new target never received. Pinned by `test/migrate-engine-resume.test.ts` (manifest identity) + `test/e2e/migrate-engine-sources-postgres.test.ts` (source catalog lands before overlapping-slug pages, PGLite → real Postgres) + `test/e2e/multi-source-bug-class.test.ts` (cross-source links).
- `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`. identity-based dedup pre-check at `:427-490`. Calls `engine.findDuplicatePage?.(sourceId, {hash, frontmatterId})` (optional `?` so test doubles compile). Posture: SKIP when `frontmatter.id` matches (true external duplicate from overlapping ingest roots), WARN-ALWAYS on content_hash collision with different/missing `frontmatter.id` (templates and daily logs may legitimately share text), FAIL CLOSED on lookup error, bypass via `--force-rechunk`. Soft-deleted pages excluded at the engine layer so tombstones don't block legitimate re-imports under new slugs. Pinned by `test/import-dedup-frontmatter-id.test.ts` (11 cases). `importFromContent` is the narrow waist every ingest path passes through (`gbrain import`, `gbrain sync`, `put_page` MCP, `/ingest` webhook). It runs a three-tier content-quality disposition via `assessContentSanity` from `src/core/content-sanity.ts` BEFORE chunking: (1) high-confidence junk (built-in Cloudflare/CAPTCHA interstitial patterns + operator literals) → QUARANTINE (stamps the `quarantine` frontmatter marker, writes ZERO chunks, hides the page from search) OR REJECT (throw → sync-failure) when `content_sanity.junk_disposition` is `reject`; (2) fuzzy markup-heavy (prose-vs-markup ratio above `content_sanity.max_markup_ratio`, warn-tier byte window, code pages exempt) → `content_flag:markup_heavy` marker (page stays fully searchable, marker rides search results + get_page to warn the agent); (3) oversize → `embed_skip` soft-block via `buildEmbedSkipMarker()` PLUS a `content_flag:oversized` marker, AND deletes any pre-existing chunks in the same transaction so search can't surface stale chunks. Gate-owned markers (`quarantine`, `content_flag`) are STRIPPED from untrusted (remote MCP, `ctx.remote !== false`) frontmatter so a write-scoped client can't hide pages or forge the warning channel; markers are excluded from `content_hash` so a flagged page doesn't re-embed every sync. `gbrain import` honors `errors > 0` for non-zero exit. `classifyErrorCode` in `src/core/sync.ts` recognizes the `PAGE_JUNK_PATTERN` code so sync-failures.jsonl grouping bins these. `extractEntityRefs` (canonical; matches both `[Name](people/slug)` markdown links and Obsidian `[[people/slug|Name]]` wikilinks), `extractPageLinks`, `inferLinkType` heuristics (attended/works_at/invested_in/founded/advises/source/mentions), `parseTimelineEntries`, `isAutoLinkEnabled` config helper. Link candidates match any dir-shaped path (#2576; existence-checked at persist). Used by extract.ts, operations.ts auto-link post-hook, and backlinks.ts. Pinned by `test/import-file-content-sanity.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), `vendor`/`dist`/`build`/`venv`, dot-prefix dirs, and `*.raw` sidecars — NOT `ops/`, which is ordinary user content (#2404; the bundled daily-task-manager stores `ops/tasks` there); `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/<name>` = submodule, `/worktrees/<name>` = 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. `isSyncable` factored through private `classifySync(path, opts): SyncableReason | null`; exported companion `unsyncableReason(path, opts)` returns the same tagged reason or null when syncable. `SYNC_SKIP_FILES` is a named export (the four canonical metafile basenames `schema.md`, `index.md`, `log.md`, `README.md`). `SyncableReason` union: `'metafile' | 'strategy' | 'pruned-dir' | 'include-glob-miss' | 'exclude-glob-hit'`. The `commands/sync.ts` cleanup loop guards on `unsyncableReason(path)` being `'metafile'` OR `'pruned-dir'` (#2404) so previously-indexed metafile pages AND deliberately put-created pages under pruned dirs survive every re-sync. Does NOT cover `manifest.deleted` (the upstream filter already strips metafiles). Pinned by `test/sync-isSyncable-shape.test.ts` (15 cases, duality contract) + `test/sync-metafile-skip.serial.test.ts` (3 PGLite cases incl. the renamed `.md → .txt` negative). `pruneDir`: `pruneDir(name, parentDir?)` extended with optional `parentDir`. When provided, additionally rejects directories containing `.git` as a FILE — the git submodule gitfile pattern (regular repos have `.git` as a DIRECTORY; submodules as a file pointing into the parent's `.git/modules/`). Sync + extract walkers thread `parentDir` so the gitfile-as-FILE check fires per descend step. Best-effort: `statSync` failures fall through and treat as a normal dir. Closes the phantom-import bug class where syncing a worktree-with-submodules walked into submodule trees. Pinned by `test/sync-walker-submodule.test.ts`.
- `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 `<head>` 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/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) → `auto_skipped` (chronic, still doctor-visible) or `acknowledged` (human resolved via `gbrain sync --skip-failed` from either unresolved state). `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 `<head>` 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). Also carries `DERIVE_PHASE_DB_ONLY_DEFAULTS` (`life/events/`, `atoms/`, `extracts/`, `dream-cycle-summaries/`) + `effectiveDbOnlyDirs` — the engine's derive-phase output prefixes treated as implicitly-declared db_only by the `undeclared_db_only_pages` doctor check but deliberately NOT merged into `loadStorageConfig` (a global merge would auto-gitignore those dirs and silently kill ingestion for brains that file-back them, the #2788 class) — and `findDbOnlyCollisions` (pure collector-output vs db_only overlap detector shared by the `db_only_collector_collision` doctor check and sync's `manageGitignore` warning). Pinned by `test/storage-config.test.ts` + `test/doctor-silent-death-checks.test.ts`.
- `src/core/disk-walk.ts``walkBrainRepo(repoPath)` returns `Map<slug, {size, mtimeMs}>` 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).