diff --git a/TODOS.md b/TODOS.md index 68018e141..4054554f5 100644 --- a/TODOS.md +++ b/TODOS.md @@ -2279,10 +2279,25 @@ at plan time and got carved out: via `buildPerSourceBindings`. Document workaround: register source-scoped OAuth clients. -- [ ] **v0.41+: T20 — extends-chain merging in registry.ts.** - `registry.ts:167` documents the gap. Implementing full child-wins - merge cascades through every consumer of `manifest.page_types`. ~1 - day CC. +- [x] **v0.41+: T20 — extends-chain merging in registry.ts.** DONE (#1749). + `resolvePack` now merges parent → child (child-wins) for the six + ingest/query-shaping fields (`page_types`, `link_types`, + `frontmatter_links`, `enrichable_types`, `filing_rules`, `takes_kinds`) + plus `borrow_from` materialization, in `src/core/schema-pack/merge.ts`. + The cascade was transparent (consumers already read `resolved.manifest`), + not per-consumer. `phases`/`calibration_domains` deliberately excluded — + see the P3 follow-up below. + +- [ ] **P3: explicit opt-in to inherit `phases` / `calibration_domains`.** + T20 excludes these two from the child-wins merge because they gate real + cycle execution (`cycle.ts` `packDeclaresPhase`) and the manifest + contract says each pack declares its own participation explicitly — + auto-inheriting would silently make a child run cycle phases it never + requested. Multi-level lens packs (`gbrain-everything`) therefore still + re-declare them by hand. If that redeclaration becomes painful, add an + explicit manifest flag (e.g. `inherit_phases: true`) so a pack author + opts in consciously. Depends on: T20 (landed). Start in + `src/core/schema-pack/merge.ts` (`mergeInheritedManifest`). - [ ] **v0.41+: T21 — comment-preserving YAML emitter.** v0.40.7.0 emitter does NOT preserve comments. Authors who care diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index d26ea811d..2e86cc25c 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -478,7 +478,8 @@ for the full plan + 21 captured design decisions. Key files (v0.40.7.0 additions): - `src/core/schema-pack/pack-lock.ts` — Atomic `O_CREAT|O_EXCL` per-pack lock. DELIBERATELY NOT the `existsSync + writeFileSync` TOCTOU shape from `src/core/page-lock.ts`. Default 60s TTL, refresh every 10s while `withPackLock(fn)` runs, `--force` semantics = "steal stale lock" NOT "skip locking." Lock path per-pack so two packs never block each other. - `src/core/schema-pack/mutate-audit.ts` — ISO-week JSONL at `~/.gbrain/audit/schema-mutations-YYYY-Www.jsonl`. Privacy-redacted: type names → sha8, prefixes → first slug segment only, matches `candidate-audit.ts` privacy posture. Logs BOTH success AND failure events so the `schema_pack_writability` doctor check has signal. `summarizeMutations()` is the cross-surface parity primitive. -- `src/core/schema-pack/registry.ts` extensions — `invalidatePackCache(name?)` walks the extends-chain reverse-graph (editing a parent pack must not leave children stale). `tryCachedPack(name)` TTL-gated fast path: inside `STAT_TTL_MS` (default 1000ms, env `GBRAIN_PACK_STAT_TTL_MS`) returns cached without statting; outside the window it stats every file in the chain and cascade-invalidates on mtime change (cross-process detection). +- `src/core/schema-pack/registry.ts` extensions — `resolvePack` walks the `extends` chain (depth cap via `EXTENDS_DEPTH_WARN` / `EXTENDS_DEPTH_HARD_CAP`), RETAINS each ancestor manifest, materializes `borrow_from`, and composes all of it into `resolved.manifest` through `mergeInheritedManifest`. Every downstream consumer reads `resolved.manifest`, so doing the merge here is what makes inheritance visible without per-consumer wiring. `borrow_from` is selective (only the named `types` / `link_types`, and only from the target's OWN declarations), non-transitive, and fail-closed — a missing target throws `UnknownPackError` via `loadByName`, matching the extends path; an omitted category borrows none of it. The alias graph + closure hash are computed on the MERGED manifest, so a cross-pack alias cycle surfaces as `AliasCycleError` at resolve. `manifest_sha8` / `packIdentity` stay the CHILD's own bytes — a parent edit does not move the child's identity, so the invalidation path is what keeps a child honest. `invalidatePackCache(name?)` walks the extends-chain reverse-graph (editing a parent pack must not leave children stale). `tryCachedPack(name)` TTL-gated fast path: inside `STAT_TTL_MS` (default 1000ms, env `GBRAIN_PACK_STAT_TTL_MS`) returns cached without statting; outside the window it stats every TRACKED file — the extends chain PLUS every borrowed pack — and cascade-invalidates on mtime change (cross-process detection), so editing a borrowed pack invalidates its borrowers. Pinned by `test/schema-pack-registry.test.ts` + `test/schema-pack-merge.test.ts`. +- `src/core/schema-pack/merge.ts` — the pure child-wins composition helper behind `resolvePack`. `mergeInheritedManifest(ancestorsBaseFirst, child, borrowed)` returns the fully-composed manifest; precedence is child → borrowed → nearest parent … → base. SIX ingest/query-shaping fields inherit: `page_types`, `link_types`, `frontmatter_links`, `enrichable_types`, `filing_rules`, `takes_kinds`. `phases` + `calibration_domains` are DELIBERATELY child-only — they gate real cycle execution (`cycle.ts` `packDeclaresPhase`), so inheriting them would silently run phases a pack never declared; `mapping_rules`, `migration_from`, `extends`, `borrow_from`, and the identity fields are child-only too (all ride the `...child` spread). `mergePageTypes` carries the ordering contract `inferTypeFromPack` depends on (first-`path_prefix`-match-wins, array order): the BASE (root, `extends: null`) pack is the ordered foundation/tail; an override of a base type keeps the base POSITION (`Map.set` updates the value, keeps insertion order) so base's curated priority survives; a genuinely-new type from ANY non-base layer — child, borrowed, or a middle pack — is PREPENDED nearest-first, so a more-derived prefix wins regardless of chain depth. `mergeByKey` keeps the first occurrence per key walking highest-precedence-first (the order-insensitive keyed fields); `frontmatter_links` keys on `page_type\x00link_type` — a NUL, not a space, because both are unconstrained strings and a space-join would collide `{"a b","c"}` with `{"a","b c"}`. `mergeUnion` backs `takes_kinds`: UNION not replace, because the Zod default makes an omitted field indistinguishable from an explicit one — so a child can ADD kinds but CANNOT narrow below base ∪ parent. Pure + deterministic: no disk, no engine. Pinned by `test/schema-pack-merge.test.ts`. - `src/core/schema-pack/best-effort.ts` — `loadActivePackBestEffort(ctx)` returns `ResolvedPack | null`. Single source of truth for the T1.5 wiring sites. `null` means EMPTY FILTER (NOT hardcoded defaults — closes the silent-violation bug class). - `src/core/schema-pack/lint-rules.ts` — 12 pure rule functions. `withMutation`'s pre-write validation gate composes the 10 file-plane rules; the 2 DB-aware rules (`extractable_empty_corpus`, `mutation_count_anomaly`) need an engine. Single source of truth consumed by CLI lint + MCP `schema_lint` + the pre-write validation gate. New file-plane rule `link_regex_catastrophic_backtrack` — advisory ReDoS pre-screen flagging the classic nested-quantifier shapes (`(a+)+`, `(a*)*`, `(a+)*`, `(\w+)+`) in a link_type's `inference.regex` via `NESTED_QUANTIFIER_RE`. WARNING not error: a hard reject would disable the whole pack on upgrade (pages fall back to legacy typing). The runtime input-length cap in `redos-guard.ts` is the actual safety net; this rule tells the pack author to fix the pattern. - `src/core/schema-pack/redos-guard.ts` + `src/core/schema-pack/link-inference.ts` — ReDoS hardening for pack inference regexes. `redos-guard.ts` adds `MAX_REGEX_INPUT_CHARS` (default 64_000, env `GBRAIN_MAX_REGEX_INPUT_CHARS`) — a hard input-length cap, the real runtime safety net (catastrophic backtracking needs a long input; a link-extraction `context` is normally a sentence or short paragraph). Over the cap, `runRegexBounded` throws the tagged `RegexInputTooLargeError` and the regex is skipped (degrade-to-mentions) without entering the `node:vm`. `link-inference.ts:inferLinkTypeFromPack` no-budget branch (test contexts) now routes through `runRegexBounded` so the input-length cap + per-regex vm timeout (`PER_REGEX_TIMEOUT_MS = 50`) apply on every path (previously this branch ran `new RegExp(pattern).test(context)` unbounded — the one ReDoS hole with no timeout). Defensive hardening + diagnostics; the deterministic ~3100-file sync-wedge root cause remains open. Pinned by `test/redos-hardening.test.ts` + `test/schema-pack-lint-rules.test.ts`. diff --git a/docs/architecture/lens-packs.md b/docs/architecture/lens-packs.md index bdd108280..0d486f185 100644 --- a/docs/architecture/lens-packs.md +++ b/docs/architecture/lens-packs.md @@ -75,6 +75,15 @@ Meta-pack stacking creator + investor + engineer via the v0.38 preserved — this IS the active pack; the registry walks extends + borrow to materialize the merged view. +**Merge contract (T20 / #1749).** `resolvePack` merges parent → child +(child-wins) for the six ingest/query-shaping fields: `page_types`, +`link_types`, `frontmatter_links`, `enrichable_types`, `filing_rules`, +and `takes_kinds` (unioned — a child cannot narrow it). `phases` and +`calibration_domains` are **NOT** inherited: they gate cycle execution, +so each pack must declare its own participation explicitly. That is why +`gbrain-everything` re-declares all its phases and all 7 +`calibration_domains` — inheritance does not carry them. + Activate via `gbrain config set schema_pack gbrain-everything` and calibration_profile produces all 7 domain scorecards in one JSONB. diff --git a/docs/architecture/schema-packs.md b/docs/architecture/schema-packs.md index 0f3f9d163..bb11888e2 100644 --- a/docs/architecture/schema-packs.md +++ b/docs/architecture/schema-packs.md @@ -145,7 +145,7 @@ api_version: gbrain-schema-pack-v1 name: my-pack version: 0.0.1 gbrain_min_version: 0.39.0 -extends: gbrain-base # inherits everything from base; add overrides below +extends: gbrain-base # inherits base's TYPES (see Merge contract below); add overrides description: | My personal pack. @@ -170,6 +170,34 @@ enrichable_types: [] filing_rules: [] ``` +## Merge contract (`extends` + `borrow_from`) + +`resolvePack` composes a pack against its `extends` chain (and any +`borrow_from` targets) into the `resolved.manifest` every consumer reads +(T20 / #1749). The rules: + +- **Six fields inherit, child-wins:** `page_types`, `link_types`, + `frontmatter_links`, `enrichable_types`, `filing_rules`, and `takes_kinds`. + A child value with the same key (type name, link name, etc.) overrides the + parent's; keys the child doesn't declare come through from the parent. +- **`page_types` ordering:** overrides of a base type keep the base's declared + position (base's `inferType` prefix priority is authoritative); a genuinely + new type — from the child, a `borrow_from`, or a middle pack in the chain — + is prepended nearest-first, so a more-derived type's `path_prefix` wins + regardless of how deep the chain is. +- **`takes_kinds` is UNION, not replace** — it carries a Zod default, so an + omitted field is indistinguishable from an explicit one. A child can ADD + kinds but **cannot narrow** `takes_kinds` below base ∪ parent. If you need a + smaller set, don't `extends` a pack that declares the larger one. +- **`phases` and `calibration_domains` are NOT inherited** (child-only). They + gate real cycle execution, so each pack must declare its own participation + explicitly — inheriting them would silently make a child run phases it never + requested. This is why `gbrain-everything` re-declares all its phases and + calibration domains by hand. See `lens-packs.md` for the worked example. +- **`borrow_from` is selective + non-transitive + fail-closed:** it pulls only + the named `types`/`link_types` from the target's OWN declarations (omitting a + category borrows none of it); a missing target throws `UnknownPackError`. + ## Recovery + revert The single-PR cathedral is hard to revert atomically. Per codex finding diff --git a/src/core/schema-pack/index.ts b/src/core/schema-pack/index.ts index 585ff13f9..1b1a72422 100644 --- a/src/core/schema-pack/index.ts +++ b/src/core/schema-pack/index.ts @@ -45,6 +45,14 @@ export { computeAliasClosureHash, } from './closure.ts'; +export { + type BorrowedTypes, + mergeByKey, + mergeUnion, + mergePageTypes, + mergeInheritedManifest, +} from './merge.ts'; + export { type SourceClosureBinding, buildPerSourceBindings, diff --git a/src/core/schema-pack/merge.ts b/src/core/schema-pack/merge.ts new file mode 100644 index 000000000..a4265bbfc --- /dev/null +++ b/src/core/schema-pack/merge.ts @@ -0,0 +1,187 @@ +// v0.42 schema-pack inheritance merge (T20 / issue #1749). +// +// resolvePack walks the `extends` chain and resolves `borrow_from`, then +// hands the ancestor manifests + child + borrowed types to this pure +// helper to produce the fully-composed `resolved.manifest`. Every +// downstream consumer reads `resolved.manifest`, so doing the merge once +// here is what makes inheritance transparent to the ~dozen call sites that +// read page_types / link_types / filing_rules / etc. +// +// Precedence, highest → lowest: child → borrowed → nearest parent … base. +// +// Scope — the SIX ingest/query-shaping fields inherit: +// page_types, link_types, frontmatter_links, enrichable_types, +// filing_rules, takes_kinds. +// `phases` and `calibration_domains` are deliberately NOT inherited — they +// gate real cycle execution (cycle.ts `packDeclaresPhase`) and the manifest +// contract says each pack declares its own participation explicitly. They +// stay whatever the CHILD declared (child-only), same as before this change. +// `mapping_rules`, `migration_from`, `extends`, `borrow_from`, and every +// identity field (name/version/…) are child-only too. +// +// page_types ordering (inferType path_prefix precedence) +// ┌───────────────────────────────────────────────────────────────────┐ +// │ inferTypeFromPack (markdown.ts) is FIRST-path_prefix-match-wins in │ +// │ array order, and gbrain-base orders its types by priority on │ +// │ purpose. So: │ +// │ • the BASE (root, extends:null) pack is the ordered foundation — │ +// │ it forms the tail, in its declared order. │ +// │ • a NEW type from ANY non-base layer (child, borrowed, or a │ +// │ middle pack in the extends chain) is PREPENDED, nearest-first │ +// │ (child → borrowed → nearest parent … → farthest middle parent), │ +// │ so a more-derived type's prefix wins. This makes a type's │ +// │ priority independent of chain depth: `thesis` (declared by │ +// │ gbrain-investor) wins the same whether investor is the active │ +// │ pack (2-level) or a middle pack under gbrain-everything. │ +// │ • an OVERRIDE of an existing BASE type keeps the base POSITION │ +// │ (only its value changes) so base's curated priority is intact. │ +// └───────────────────────────────────────────────────────────────────┘ + +import type { SchemaPackManifest, PackPageType, PackLinkType } from './manifest-v1.ts'; + +/** Types pulled from `borrow_from` targets (already name-filtered by resolvePack). */ +export interface BorrowedTypes { + page_types: PackPageType[]; + link_types: PackLinkType[]; +} + +/** + * Merge keyed records child-wins: walk layers highest-precedence-first and + * keep the FIRST occurrence of each key. Used for the order-insensitive + * fields (link_types, frontmatter_links, enrichable_types, filing_rules) — + * these are keyed lookups, so array order carries no behavior. + */ +export function mergeByKey( + layersHighToLow: ReadonlyArray>, + keyFn: (item: T) => string, +): T[] { + const seen = new Set(); + const out: T[] = []; + for (const layer of layersHighToLow) { + for (const item of layer) { + const k = keyFn(item); + if (seen.has(k)) continue; + seen.add(k); + out.push(item); + } + } + return out; +} + +/** + * Order-preserving union across layers (dedup by value identity). Used for + * `takes_kinds`. UNION (not replace) because Zod applies the default + * `['fact','take','bet','hunch']` at parse time, so an omitted field is + * indistinguishable from an explicit one — replace-semantics would let a + * child that omits `takes_kinds` wipe the parent's. Consequence: a child + * cannot NARROW takes_kinds below base ∪ parent (documented constraint). + */ +export function mergeUnion(layers: ReadonlyArray>): T[] { + const seen = new Set(); + const out: T[] = []; + for (const layer of layers) { + for (const item of layer) { + if (seen.has(item)) continue; + seen.add(item); + out.push(item); + } + } + return out; +} + +/** + * Merge page_types with the ordering contract above. The BASE (root) pack is + * the ordered foundation (tail); overrides of a base type keep the base + * position; genuinely-new types from ANY non-base layer are prepended + * nearest-first so a more-derived type's prefix wins in inferType regardless + * of chain depth. + */ +export function mergePageTypes( + ancestorsBaseFirst: ReadonlyArray, + borrowedPageTypes: ReadonlyArray, + child: SchemaPackManifest, +): PackPageType[] { + // Split the extends chain: the root (extends:null) pack is the ordered + // foundation; everything above it (middle parents) contributes overrides + + // new types like child/borrowed do. ancestorsBaseFirst is [root … nearest]. + const base = ancestorsBaseFirst[0]; + const middleParentsBaseFirst = ancestorsBaseFirst.slice(1); + + // 1. Foundation map from the base pack, in declared order. Map.set() on an + // existing key UPDATES the value but KEEPS the insertion position, so an + // override of a base type stays in the base's curated priority slot. + const byName = new Map(); + if (base) for (const pt of base.page_types) byName.set(pt.name, pt); + + // 2. Value overrides of base types, applied lowest→highest precedence + // (farthest middle parent → nearest parent → borrowed → child) so the + // highest-precedence value wins for any type that exists in the base. + const overrideLayersLowToHigh: ReadonlyArray> = [ + ...middleParentsBaseFirst.map(p => p.page_types), + borrowedPageTypes, + child.page_types, + ]; + for (const layer of overrideLayersLowToHigh) { + for (const pt of layer) if (byName.has(pt.name)) byName.set(pt.name, pt); + } + + // 3. Genuinely-new types (absent from the base foundation), prepended + // nearest-first: child → borrowed → nearest parent … → farthest middle + // parent. Deduped by name, so the nearer layer wins a name declared new + // in more than one place. + const newLayersHighToLow: ReadonlyArray> = [ + child.page_types, + borrowedPageTypes, + ...[...middleParentsBaseFirst].reverse().map(p => p.page_types), + ]; + const seenNew = new Set(); + const prepended: PackPageType[] = []; + for (const layer of newLayersHighToLow) { + for (const pt of layer) { + if (byName.has(pt.name) || seenNew.has(pt.name)) continue; + seenNew.add(pt.name); + prepended.push(pt); + } + } + return [...prepended, ...byName.values()]; +} + +/** + * Compose the resolved manifest from the extends ancestors (base-first), + * the child, and the resolved `borrow_from` types. Pure + deterministic. + * Identity fields and the child-only fields come from `child` via spread; + * the six inheritable fields are overwritten with their merged values. + */ +export function mergeInheritedManifest( + ancestorsBaseFirst: ReadonlyArray, + child: SchemaPackManifest, + borrowed: BorrowedTypes, +): SchemaPackManifest { + // Ancestors highest-precedence-first (nearest parent … base) for the + // keyed merges. Child sits above all ancestors; borrowed sits between + // child and the ancestors (only page_types + link_types). + const ancestorsHighToLow = [...ancestorsBaseFirst].reverse(); + const ancLink = ancestorsHighToLow.map(a => a.link_types); + const ancFront = ancestorsHighToLow.map(a => a.frontmatter_links); + const ancEnrich = ancestorsHighToLow.map(a => a.enrichable_types); + const ancFiling = ancestorsHighToLow.map(a => a.filing_rules); + const ancTakes = ancestorsHighToLow.map(a => a.takes_kinds); + + return { + // Keeps identity fields AND the child-only fields (phases, + // calibration_domains, mapping_rules, migration_from, extends, + // borrow_from) exactly as the child declared them. + ...child, + page_types: mergePageTypes(ancestorsBaseFirst, borrowed.page_types, child), + link_types: mergeByKey([child.link_types, borrowed.link_types, ...ancLink], lt => lt.name), + frontmatter_links: mergeByKey( + [child.frontmatter_links, ...ancFront], + // NUL delimiter, not a space: page_type/link_type are unconstrained + // strings, so a space-join would collide {"a b","c"} with {"a","b c"}. + fl => `${fl.page_type}\x00${fl.link_type}`, + ), + enrichable_types: mergeByKey([child.enrichable_types, ...ancEnrich], et => et.type), + filing_rules: mergeByKey([child.filing_rules, ...ancFiling], fr => fr.kind), + takes_kinds: mergeUnion([child.takes_kinds, ...ancTakes]), + }; +} diff --git a/src/core/schema-pack/registry.ts b/src/core/schema-pack/registry.ts index 09b60fb21..d4c161ccf 100644 --- a/src/core/schema-pack/registry.ts +++ b/src/core/schema-pack/registry.ts @@ -55,6 +55,7 @@ import { statSync } from 'node:fs'; import type { SchemaPackManifest } from './manifest-v1.ts'; import { computeManifestSha8, packIdentity } from './manifest-v1.ts'; import { computeAliasClosureHash, buildAliasGraph, type AliasGraph } from './closure.ts'; +import { mergeInheritedManifest, type BorrowedTypes } from './merge.ts'; export const EXTENDS_DEPTH_WARN = 4 as const; export const EXTENDS_DEPTH_HARD_CAP = 8 as const; @@ -254,10 +255,12 @@ export async function resolvePack( return existing.resolved; } - // Walk extends chain to enforce depth cap AND collect names for the - // cache snapshot (codex C6 — child cache entry must remember every - // parent so invalidatePackCache(parentName) can cascade). + // Walk extends chain to enforce depth cap, collect names for the cache + // snapshot (codex C6 — child cache entry must remember every parent so + // invalidatePackCache(parentName) can cascade), AND retain each ancestor + // manifest so we can merge parent content child-wins (T20 / #1749). const chain: string[] = [manifest.name]; + const ancestorsNearestFirst: SchemaPackManifest[] = []; let cursor: SchemaPackManifest | null = manifest; while (cursor?.extends) { const parentName = cursor.extends; @@ -271,27 +274,52 @@ export async function resolvePack( if (chain.length > EXTENDS_DEPTH_WARN) { opts.onDepthWarn?.(chain.length, chain); } - cursor = await loadByName(parentName); + const parent = await loadByName(parentName); + ancestorsNearestFirst.push(parent); + cursor = parent; + } + const ancestorsBaseFirst = [...ancestorsNearestFirst].reverse(); + + // Resolve `borrow_from` (selective, non-transitive). Fail-closed: a + // missing borrow target throws UnknownPackError via loadByName, matching + // the extends path. Omitted `types`/`link_types` = borrow none of that + // category (selective by contract). We pull the borrowed pack's OWN + // declared types only — not its inherited/merged ones. + const borrowed: BorrowedTypes = { page_types: [], link_types: [] }; + const borrowedNames: string[] = []; + for (const entry of manifest.borrow_from) { + const src = await loadByName(entry.pack); + borrowedNames.push(entry.pack); + const wantTypes = new Set(entry.types ?? []); + const wantLinks = new Set(entry.link_types ?? []); + for (const pt of src.page_types) if (wantTypes.has(pt.name)) borrowed.page_types.push(pt); + for (const lt of src.link_types) if (wantLinks.has(lt.name)) borrowed.link_types.push(lt); } - // For v0.38 skeleton: closure is computed on the manifest itself. - // Full extends-merging (child-wins) is the v0.41+ T20 follow-up. - const alias_graph = buildAliasGraph(manifest); - const alias_closure_hash = await computeAliasClosureHash(manifest); + // Child-wins merge across the extends chain + borrowed types. Every + // downstream reader consumes `resolved.manifest`, so the merged manifest + // is what makes inheritance visible. Closure is (correctly) computed on + // the merged manifest; a merged alias cycle surfaces here as AliasCycleError. + const merged = mergeInheritedManifest(ancestorsBaseFirst, manifest, borrowed); + const alias_graph = buildAliasGraph(merged); + const alias_closure_hash = await computeAliasClosureHash(merged); const resolved: ResolvedPack = { - manifest, + manifest: merged, identity: id, manifest_sha8: sha8, alias_closure_hash, alias_graph, }; - // Capture file-stat snapshot for the stat-TTL gate. Skip names that - // the locator can't resolve (synthetic manifests in tests). + // Capture file-stat snapshot for the stat-TTL gate over EVERY file that + // fed this entry — the extends chain PLUS borrowed packs — so editing a + // borrowed pack cascade-invalidates its borrowers. Skip names the locator + // can't resolve (synthetic manifests in tests). + const trackedNames = [...new Set([...chain, ...borrowedNames])]; const files: Array<{ name: string; path: string; mtimeMs: number }> = []; if (opts.loadByPath) { - for (const n of chain) { + for (const n of trackedNames) { const path = opts.loadByPath(n); if (path === null) continue; files.push({ name: n, path, mtimeMs: safeMtimeMs(path) }); @@ -300,7 +328,7 @@ export async function resolvePack( _byName.set(manifest.name, { resolved, - chain: [...chain], + chain: trackedNames, files, lastStatMs: Date.now(), }); diff --git a/test/schema-pack-merge.test.ts b/test/schema-pack-merge.test.ts new file mode 100644 index 000000000..8c15a0202 --- /dev/null +++ b/test/schema-pack-merge.test.ts @@ -0,0 +1,331 @@ +// Schema-pack inheritance merge (T20 / issue #1749). +// +// Covers the pure merge helper (field-by-field child-wins semantics + +// page_types ordering) AND resolvePack's wiring of extends ancestors + +// borrow_from into the merged manifest. Pure unit tests; disk never touched. + +import { describe, expect, test, beforeEach } from 'bun:test'; +import { + mergeInheritedManifest, + mergeByKey, + mergeUnion, + type BorrowedTypes, +} from '../src/core/schema-pack/merge.ts'; +import { + resolvePack, + invalidatePackCache, + UnknownPackError, + _resetPackCacheForTests, +} from '../src/core/schema-pack/registry.ts'; +import { AliasCycleError, expandClosure } from '../src/core/schema-pack/closure.ts'; +import { inferTypeFromPack } from '../src/core/markdown.ts'; +import type { + SchemaPackManifest, + PackPageType, + PackLinkType, +} from '../src/core/schema-pack/manifest-v1.ts'; +import { SCHEMA_PACK_API_VERSION } from '../src/core/schema-pack/manifest-v1.ts'; + +// ── builders ──────────────────────────────────────────────────────────── +function pt(name: string, path_prefixes: string[] = [], aliases: string[] = []): PackPageType { + return { name, primitive: 'entity', path_prefixes, aliases, extractable: false, expert_routing: false }; +} +function lt(name: string): PackLinkType { + return { name }; +} +function mk(name: string, over: Partial = {}): SchemaPackManifest { + return { + api_version: SCHEMA_PACK_API_VERSION, + name, + version: '1.0.0', + description: '', + gbrain_min_version: '0.38.0', + extends: null, + borrow_from: [], + page_types: [], + link_types: [], + frontmatter_links: [], + takes_kinds: ['fact', 'take', 'bet', 'hunch'], + enrichable_types: [], + filing_rules: [], + ...over, + } as SchemaPackManifest; +} +const noBorrow: BorrowedTypes = { page_types: [], link_types: [] }; +const names = (arr: { name: string }[]) => arr.map(x => x.name); + +function loaderFor(byName: Record) { + return async (name: string): Promise => { + const m = byName[name]; + if (!m) throw new UnknownPackError(name); + return m; + }; +} + +beforeEach(() => _resetPackCacheForTests()); + +// ── 1. parent types visible; override replaces ────────────────────────── +describe('mergeInheritedManifest — page_types', () => { + test('parent page_types are visible in the child (the core bug)', () => { + const base = mk('base', { page_types: [pt('person'), pt('company')] }); + const child = mk('child', { extends: 'base', page_types: [pt('paper')] }); + const merged = mergeInheritedManifest([base], child, noBorrow); + expect(names(merged.page_types).sort()).toEqual(['company', 'paper', 'person']); + }); + + test('child override replaces a same-named parent type (no duplicate)', () => { + const base = mk('base', { page_types: [pt('person', ['people/'])] }); + const childPerson = pt('person', ['humans/']); + const child = mk('child', { extends: 'base', page_types: [childPerson] }); + const merged = mergeInheritedManifest([base], child, noBorrow); + const persons = merged.page_types.filter(p => p.name === 'person'); + expect(persons).toHaveLength(1); + expect(persons[0].path_prefixes).toEqual(['humans/']); // child value won + }); + + // ── 2. ordering: new prepended, override in place ──────────────────── + test('new child type is prepended → its path_prefix wins in inferType', () => { + // base "note" has the BROAD prefix; child adds a NARROWER "paper". + const base = mk('base', { page_types: [pt('note', ['notes/'])] }); + const child = mk('child', { extends: 'base', page_types: [pt('paper', ['notes/papers/'])] }); + const merged = mergeInheritedManifest([base], child, noBorrow); + // paper is prepended, so first-match-wins picks it for the overlapping path. + expect(merged.page_types[0].name).toBe('paper'); + expect(inferTypeFromPack('notes/papers/x.md', merged)).toBe('paper'); + expect(inferTypeFromPack('notes/other.md', merged)).toBe('note'); + }); + + test('override of a base type keeps the ancestor position (base priority intact)', () => { + // base intentionally orders [strong, person]; child overrides person only. + const base = mk('base', { page_types: [pt('strong', ['s/']), pt('person', ['p/'])] }); + const child = mk('child', { extends: 'base', page_types: [pt('person', ['p/', 'people/'])] }); + const merged = mergeInheritedManifest([base], child, noBorrow); + // strong stays first; the override does NOT hoist person to the front. + expect(names(merged.page_types)).toEqual(['strong', 'person']); + expect(inferTypeFromPack('s/x.md', merged)).toBe('strong'); + }); + + test('extends:null → full override, ancestors ignored', () => { + const child = mk('solo', { extends: null, page_types: [pt('only')] }); + const merged = mergeInheritedManifest([], child, noBorrow); + expect(names(merged.page_types)).toEqual(['only']); + }); + + test('a NEW type from a MIDDLE pack is prepended too (no 2-vs-3-level asymmetry)', () => { + // base "note" (broad prefix); a MIDDLE parent adds narrower "paper"; + // grandchild redeclares nothing. paper must still win the overlapping path + // — its priority can't depend on whether investor is active directly or as + // a middle pack under everything. ancestorsBaseFirst = [base, parent]. + const base = mk('base', { page_types: [pt('note', ['notes/'])] }); + const parent = mk('parent', { extends: 'base', page_types: [pt('paper', ['notes/papers/'])] }); + const grandchild = mk('gc', { extends: 'parent', page_types: [] }); + const merged = mergeInheritedManifest([base, parent], grandchild, noBorrow); + expect(merged.page_types[0].name).toBe('paper'); // prepended, not appended after base + expect(inferTypeFromPack('notes/papers/x.md', merged)).toBe('paper'); + expect(inferTypeFromPack('notes/other.md', merged)).toBe('note'); + }); + + test('3-level override keeps base position; new middle type still prepended', () => { + // base [strong(s/), person(p/)]; parent overrides person AND adds new mid(m/); + // child adds new kid(k/). Order: [kid, mid, strong, person] — base pair keeps + // its curated order, new types prepend nearest-first. + const base = mk('base', { page_types: [pt('strong', ['s/']), pt('person', ['p/'])] }); + const parent = mk('parent', { + extends: 'base', + page_types: [pt('person', ['p/', 'people/']), pt('mid', ['m/'])], + }); + const child = mk('child', { extends: 'parent', page_types: [pt('kid', ['k/'])] }); + const merged = mergeInheritedManifest([base, parent], child, noBorrow); + expect(names(merged.page_types)).toEqual(['kid', 'mid', 'strong', 'person']); + // parent's person override won its value while keeping base position. + expect(merged.page_types.find(p => p.name === 'person')!.path_prefixes).toEqual(['p/', 'people/']); + }); +}); + +// ── 3. the other keyed fields (the coverage #1838 lacks) ──────────────── +describe('mergeInheritedManifest — link/frontmatter/enrichable/filing', () => { + const base = mk('base', { + link_types: [lt('founded'), lt('works_at')], + frontmatter_links: [{ page_type: 'person', fields: ['company'], link_type: 'works_at' }], + enrichable_types: [{ type: 'person', rubric: 'person-default' }], + filing_rules: [{ kind: 'person', directory: 'people/', examples: [] }], + }); + const child = mk('child', { + extends: 'base', + link_types: [lt('invested_in'), lt('works_at')], // works_at overrides + frontmatter_links: [{ page_type: 'person', fields: ['org'], link_type: 'member_of' }], + enrichable_types: [{ type: 'paper', rubric: 'paper-default' }], + filing_rules: [{ kind: 'paper', directory: 'papers/', examples: [] }], + }); + const merged = mergeInheritedManifest([base], child, noBorrow); + + test('link_types merge child-wins by name', () => { + expect(names(merged.link_types).sort()).toEqual(['founded', 'invested_in', 'works_at']); + }); + test('frontmatter_links merge on composite (page_type, link_type)', () => { + // Different link_type for same page_type → both coexist. + const keys = merged.frontmatter_links.map(f => `${f.page_type}/${f.link_type}`).sort(); + expect(keys).toEqual(['person/member_of', 'person/works_at']); + }); + test('enrichable_types + filing_rules merge', () => { + expect(merged.enrichable_types.map(e => e.type).sort()).toEqual(['paper', 'person']); + expect(merged.filing_rules.map(f => f.kind).sort()).toEqual(['paper', 'person']); + }); +}); + +// ── 4. takes_kinds union ───────────────────────────────────────────────── +describe('mergeInheritedManifest — takes_kinds union', () => { + test('child omitting takes_kinds keeps the parent set', () => { + const base = mk('base', { takes_kinds: ['fact', 'take', 'bet', 'hunch', 'thesis'] }); + const child = mk('child', { extends: 'base' }); // default 4 + const merged = mergeInheritedManifest([base], child, noBorrow); + expect(merged.takes_kinds).toContain('thesis'); + expect(merged.takes_kinds).toContain('fact'); + }); + test('child additions union with the parent (dedup)', () => { + const base = mk('base', { takes_kinds: ['fact'] }); + const child = mk('child', { extends: 'base', takes_kinds: ['fact', 'wager'] }); + const merged = mergeInheritedManifest([base], child, noBorrow); + expect(merged.takes_kinds.sort()).toEqual(['fact', 'wager']); + }); +}); + +// ── 5. phases + calibration_domains are NOT inherited ──────────────────── +describe('mergeInheritedManifest — phases/calibration stay child-only', () => { + test('a child does NOT inherit the parent phases', () => { + const base = mk('base', { phases: ['extract_atoms'] }); + const child = mk('child', { extends: 'base' }); // declares no phases + const merged = mergeInheritedManifest([base], child, noBorrow); + expect(merged.phases).toBeUndefined(); + }); + test('a child does NOT inherit the parent calibration_domains', () => { + const base = mk('base', { + calibration_domains: [{ name: 'deal_success', aggregator: 'scalar_brier', page_types: ['deal'] }], + }); + const child = mk('child', { extends: 'base' }); + const merged = mergeInheritedManifest([base], child, noBorrow); + expect(merged.calibration_domains).toBeUndefined(); + }); + test('a child keeps its OWN declared phases verbatim', () => { + const base = mk('base', { phases: ['extract_atoms'] }); + const child = mk('child', { extends: 'base', phases: ['synthesize_concepts'] }); + const merged = mergeInheritedManifest([base], child, noBorrow); + expect(merged.phases).toEqual(['synthesize_concepts']); // NOT unioned with base + }); +}); + +// ── 6. borrow_from via resolvePack ─────────────────────────────────────── +describe('resolvePack — borrow_from materialization', () => { + test('borrows only the named types/link_types from a non-chain pack', async () => { + const lens = mk('lens', { + page_types: [pt('atom'), pt('unrelated')], + link_types: [lt('derived_from'), lt('noise')], + }); + const child = mk('child', { + extends: null, + page_types: [pt('own')], + borrow_from: [{ pack: 'lens', types: ['atom'], link_types: ['derived_from'] }], + }); + const resolved = await resolvePack(child, loaderFor({ lens })); + expect(names(resolved.manifest.page_types).sort()).toEqual(['atom', 'own']); + expect(names(resolved.manifest.link_types)).toEqual(['derived_from']); + }); + + test('borrow omitting a category pulls none of it', async () => { + const lens = mk('lens', { page_types: [pt('atom')], link_types: [lt('x')] }); + const child = mk('child', { + extends: null, + borrow_from: [{ pack: 'lens', types: ['atom'] }], // no link_types + }); + const resolved = await resolvePack(child, loaderFor({ lens })); + expect(names(resolved.manifest.page_types)).toEqual(['atom']); + expect(resolved.manifest.link_types).toEqual([]); + }); + + test('missing borrow target throws UnknownPackError (fail-closed)', async () => { + const child = mk('child', { extends: null, borrow_from: [{ pack: 'ghost', types: ['x'] }] }); + let caught: unknown; + try { + await resolvePack(child, loaderFor({})); + } catch (e) { + caught = e; + } + expect(caught).toBeInstanceOf(UnknownPackError); + }); +}); + +// ── 7. idempotency ────────────────────────────────────────────────────── +describe('mergeInheritedManifest — idempotency', () => { + test('redeclaring an identical parent type is a no-op', () => { + const base = mk('base', { page_types: [pt('person', ['people/'])] }); + const withRedeclare = mk('c', { extends: 'base', page_types: [pt('person', ['people/']), pt('extra')] }); + const withoutRedeclare = mk('c', { extends: 'base', page_types: [pt('extra')] }); + const a = mergeInheritedManifest([base], withRedeclare, noBorrow); + const b = mergeInheritedManifest([base], withoutRedeclare, noBorrow); + expect(names(a.page_types).sort()).toEqual(names(b.page_types).sort()); + expect(a.page_types.filter(p => p.name === 'person')).toHaveLength(1); + }); +}); + +// ── 8. merged totals (what get_active_schema_pack counts) ──────────────── +describe('resolvePack — merged manifest is what consumers read', () => { + test('resolved.manifest.page_types reflects the merged total, not child-only', async () => { + const base = mk('base', { page_types: [pt('a'), pt('b'), pt('c')] }); + const child = mk('child', { extends: 'base', page_types: [pt('d')] }); + const resolved = await resolvePack(child, loaderFor({ base })); + // get_active_schema_pack counts `pack.manifest.page_types.length`. + expect(resolved.manifest.page_types).toHaveLength(4); + }); +}); + +// ── 9. cycles, cascade, dangling aliases ───────────────────────────────── +describe('resolvePack — closure edge cases across the merged manifest', () => { + test('a cross-pack alias cycle throws AliasCycleError at resolve', async () => { + // base a→b, parent b→c, child c→a ⇒ a→b→c→a cycle only once merged. + const base = mk('base', { page_types: [pt('a', [], ['b'])] }); + const parent = mk('parent', { extends: 'base', page_types: [pt('b', [], ['c'])] }); + const child = mk('child', { extends: 'parent', page_types: [pt('c', [], ['a'])] }); + let caught: unknown; + try { + await resolvePack(child, loaderFor({ base, parent })); + } catch (e) { + caught = e; + } + expect(caught).toBeInstanceOf(AliasCycleError); + }); + + test('editing a borrowed pack cascade-invalidates the borrower', async () => { + const lens = mk('lens', { page_types: [pt('atom')] }); + const child = mk('child', { extends: null, borrow_from: [{ pack: 'lens', types: ['atom'] }] }); + await resolvePack(child, loaderFor({ lens })); + const { invalidated } = invalidatePackCache('lens'); + expect(invalidated).toContain('child'); // borrow edge is tracked in the cache chain + }); + + test('a dangling alias target is benign (no throw, closure still resolves)', () => { + // "x" aliases a type "ghost" that no page_type declares. + const base = mk('base', { page_types: [pt('x', [], ['ghost'])] }); + const child = mk('child', { extends: 'base', page_types: [pt('y')] }); + const merged = mergeInheritedManifest([base], child, noBorrow); + // resolvePack would buildAliasGraph(merged) without throwing. + const closure = expandClosureFromManifest(merged, 'x'); + expect(closure).toContain('ghost'); + }); +}); + +// small local helper to exercise closure on a merged manifest without disk. +import { buildAliasGraph } from '../src/core/schema-pack/closure.ts'; +function expandClosureFromManifest(m: SchemaPackManifest, type: string): string[] { + return expandClosure(type, buildAliasGraph(m)); +} + +// ── primitives ────────────────────────────────────────────────────────── +describe('mergeByKey / mergeUnion primitives', () => { + test('mergeByKey keeps first occurrence per key (highest precedence wins)', () => { + const out = mergeByKey([[{ k: 'a', v: 1 }], [{ k: 'a', v: 2 }, { k: 'b', v: 3 }]], x => x.k); + expect(out).toEqual([{ k: 'a', v: 1 }, { k: 'b', v: 3 }]); + }); + test('mergeUnion dedups order-preserving', () => { + expect(mergeUnion([['a', 'b'], ['b', 'c']])).toEqual(['a', 'b', 'c']); + }); +});