diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index 62c3fae05..22bb97fbf 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -50,7 +50,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/core/sources-ops.ts` — Multi-source registration + clone-lifecycle ops (`addSource`, `recloneIfMissing`, `defaultCloneDir`, `isOwnedClone`, `unownedHint`). **Reclone-ownership invariant (must-never-violate): gbrain may only delete/re-clone a clone it created, NEVER a user working tree.** `recloneIfMissing` deletes `local_path`, so it gates on `isOwnedClone(src)` and throws a `SourceOpError('unmanaged_path', ...)` BEFORE any filesystem op when ownership is unprovable — fail-closed. Ownership is proven by `config.managed_clone === true` (written by `addSource`'s `--url` path, covering default-location and `--clone-dir` clones) OR `local_path === defaultCloneDir(id)` (back-compat for pre-marker clones, via exact normalized-path equality, symlink-free). A row with `remote_url` + an unowned `local_path` (a user-registered working tree, e.g. `sources add --path`) is refused untouched; re-add with `--url` to regain auto-reclone. The reclone is EXDEV-safe: clone into a SIBLING temp of `local_path` (not the shared `clones/.tmp`, which may sit on a different mount than a `--clone-dir` target), then swap (move old aside → move new in → drop old) so `local_path` is never left missing-and-unrecoverable; on swap failure the original is restored, and if restore fails the error names the `aside` path so it's never reflexively deleted. A TOCTOU re-check re-confirms ownership immediately before the destructive move and rejects a symlink leaf swapped in after the entry check (`symlink_escape`). `unownedHint(src, state)` is the shared recovery message used by both the core error and the `gbrain sync --source` CLI error; `gbrain sources restore` special-cases `unmanaged_path` to print "DB row restored; gbrain syncs this path read-only" instead of the misleading "try sync to recover" guidance. `SourceOpErrorCode` includes `unmanaged_path`. Pinned by `test/sources-ops.test.ts`, `test/sources-resync-recovery.test.ts`. - `src/core/utils.ts` — Shared SQL utilities extracted from postgres-engine.ts. Exports `parseEmbedding(value)` (throws on unknown input, used by migration + ingest paths where data integrity matters) and `tryParseEmbedding(value)` (returns `null` + warns once per process, used by search/rescore paths where availability matters more than strictness). `isUndefinedColumnError(err)` predicate — pattern-matches Postgres SQLSTATE 42703 / "column ... does not exist" with engine-driver shape variation tolerated; replaces bare `catch {}` blocks in `oauth-provider.ts` so genuine errors (lock timeout, network blip, permission denied) propagate while column-missing falls through to the legacy fallback. `validateSourceId(id)` throws on anything outside `^[a-z0-9_-]+$`, used by the per-source disk-layout fix in patterns.ts/synthesize.ts before any `join(brainDir, '.sources', source_id, slug+'.md')` so source_id can't traverse out of brainDir. `rowToSearchResult` projects email `message_id` / `thread_id` metadata and exposes `source_subject` only when a non-empty Message-ID proves the page is an email, so generated page titles never become authoritative email subjects. `rowToPage` populates the required `Page.source_id` from the SELECT projection (`scripts/check-source-id-projection.sh` enforces every projection feeding `rowToPage` includes the column). - `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 the pool end so a concurrent connect can't join a pool that's already closing. The end routes through `endPoolBounded(pool)` (#1972) — a gbrain-owned `Promise.race` of `pool.end({ timeout: POOL_END_TIMEOUT_SECONDS })` against a hard timer — so a PgBouncer transaction-mode drain that never settles can't hang teardown — the #2084 contract (finishCliTeardown's computed-deadline backstop + flushThenExit's fence-and-grace exit in cli-force-exit.ts) bounds it and delivers pending stdout before exit. `connection-manager.ts` ends its direct + read pools concurrently through the same helper so the per-pool bounds don't stack. -- `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. 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). +- `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/` = 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. `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 `` 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`. diff --git a/src/commands/migrate-engine.ts b/src/commands/migrate-engine.ts index 3d1e2af18..11f46d7ed 100644 --- a/src/commands/migrate-engine.ts +++ b/src/commands/migrate-engine.ts @@ -80,6 +80,10 @@ export function manifestMatchesTarget(manifest: MigrateManifest, targetId: strin return manifest.schema_version === 2 && manifest.target_id === targetId; } +function makeManifestKey(sourceId: string, slug: string): string { + return sourceId === 'default' ? slug : `${sourceId}::${slug}`; +} + function loadManifest(): MigrateManifest | null { const path = getManifestPath(); if (!existsSync(path)) return null; @@ -151,6 +155,25 @@ export async function copyMigrationSources(source: BrainEngine, target: BrainEng } } +export async function copyPageLinksToTarget( + source: BrainEngine, + target: BrainEngine, + page: Page, + failedKeys: ReadonlySet = new Set(), +): Promise { + const links = await source.getLinks(page.slug, { sourceId: page.source_id }); + for (const link of links) { + const toSourceId = link.to_source_id ?? page.source_id; + if (failedKeys.has(makeManifestKey(toSourceId, link.to_slug))) continue; + await target.addLink( + link.from_slug, link.to_slug, + link.context, link.link_type, + undefined, undefined, undefined, + { fromSourceId: page.source_id, toSourceId }, + ); + } +} + /** * postgres.js's UNDEFINED_VALUE guard rejects any bound parameter that is JS * `undefined` — unlike PGLite, it will not silently treat it as SQL NULL. @@ -564,8 +587,6 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[] // entries were bare slugs; we keep treating those as default-source for // back-compat resume. const completedSet = new Set(manifest?.completed_slugs || []); - const makeManifestKey = (sourceId: string, slug: string): string => - sourceId === 'default' ? slug : `${sourceId}::${slug}`; if (!manifest) { manifest = { completed_slugs: [], @@ -680,17 +701,7 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[] progress.tick(1); continue; } - const sourceOpts = { sourceId: page.source_id }; - const links = await sourceEngine.getLinks(page.slug, sourceOpts); - for (const link of links) { - if (failedKeys.has(makeManifestKey(page.source_id, link.to_slug))) continue; - await targetEngine.addLink( - link.from_slug, link.to_slug, - link.context, link.link_type, - undefined, undefined, undefined, - { fromSourceId: page.source_id, toSourceId: page.source_id }, - ); - } + await copyPageLinksToTarget(sourceEngine, targetEngine, page, failedKeys); progress.tick(1); } progress.finish(); diff --git a/test/e2e/multi-source-bug-class.test.ts b/test/e2e/multi-source-bug-class.test.ts index a3c12d3fb..51128e3c0 100644 --- a/test/e2e/multi-source-bug-class.test.ts +++ b/test/e2e/multi-source-bug-class.test.ts @@ -33,7 +33,10 @@ import { PGLiteEngine } from '../../src/core/pglite-engine.ts'; import { resetPgliteState } from '../helpers/reset-pglite.ts'; import { validateSourceId } from '../../src/core/utils.ts'; import { extractTakesFromDb } from '../../src/core/cycle/extract-takes.ts'; -import { copyMigrationSources } from '../../src/commands/migrate-engine.ts'; +import { + copyMigrationSources, + copyPageLinksToTarget, +} from '../../src/commands/migrate-engine.ts'; let engine: PGLiteEngine; let migrationTarget: PGLiteEngine; @@ -124,6 +127,68 @@ describe('multi-source bug class', () => { expect(await migrationTarget.getPage('people/alice', { sourceId: 'media-corpus' })).not.toBeNull(); }); + test('migration preserves a cross-source link target source (#3859)', async () => { + await copyMigrationSources(engine, migrationTarget); + const pages = await engine.listPages({ limit: 100 }); + for (const page of pages) { + await migrationTarget.putPage(page.slug, { + type: page.type, + title: page.title, + compiled_truth: page.compiled_truth, + timeline: page.timeline, + frontmatter: page.frontmatter, + }, { sourceId: page.source_id }); + } + await engine.addLink( + 'media/x/post-123', + 'concepts/widget', + 'cross-source migration', + 'references', + undefined, + undefined, + undefined, + { fromSourceId: 'media-corpus', toSourceId: 'default' }, + ); + + const sourcePage = await engine.getPage( + 'media/x/post-123', + { sourceId: 'media-corpus' }, + ); + await copyPageLinksToTarget(engine, migrationTarget, sourcePage!); + + const copied = await migrationTarget.getLinks( + 'media/x/post-123', + { sourceId: 'media-corpus' }, + ); + expect(copied).toHaveLength(1); + expect(copied[0]!.to_source_id).toBe('default'); + + await engine.addLink( + 'people/alice', + 'concepts/widget', + 'failed cross-source target', + 'references', + undefined, + undefined, + undefined, + { fromSourceId: 'media-corpus', toSourceId: 'default' }, + ); + const filteredSourcePage = await engine.getPage( + 'people/alice', + { sourceId: 'media-corpus' }, + ); + await copyPageLinksToTarget( + engine, + migrationTarget, + filteredSourcePage!, + new Set(['concepts/widget']), + ); + expect(await migrationTarget.getLinks( + 'people/alice', + { sourceId: 'media-corpus' }, + )).toHaveLength(0); + }); + test('listAllPageRefs returns one row per (slug, source_id), ordered (F11)', async () => { const refs = await engine.listAllPageRefs(); // 4 rows: alice@default, alice@media-corpus, widget@default, post-123@media-corpus