diff --git a/docs/CONFORMANCE.md b/docs/CONFORMANCE.md index 7248d70ab..f13251edf 100644 --- a/docs/CONFORMANCE.md +++ b/docs/CONFORMANCE.md @@ -41,7 +41,7 @@ regenerate with `ls -d fixtures//*/ | wc -l`. | [`fixtures/template-codegen-conformance/`](../fixtures/template-codegen-conformance/) | 3 | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/template-output-render-conformance/`](../fixtures/template-output-render-conformance/) | 5 | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/generator-registry-conformance/`](../fixtures/generator-registry-conformance/) | 1 canonical manifest | ✓ | ✓ | ✓ | ✓ | ✓ | -| [`fixtures/provider-composition-conformance/`](../fixtures/provider-composition-conformance/) | 5 cases | ✓ | ✓ | — (JVM registry via Java) | ✓ | ✓ | +| [`fixtures/provider-composition-conformance/`](../fixtures/provider-composition-conformance/) | 9 (5 error-shape + 4 compose-load) | ✓ | ✓ | — (JVM registry via Java) | ✓ | ✓ | | [`fixtures/agent-context-conformance/`](../fixtures/agent-context-conformance/) | 4 | ✓ (the emitter is TS-owned) | — | — | — | — | | [`fixtures/metamodel-docs/`](../fixtures/metamodel-docs/) | 1 | ✓ (docs emit is TS-owned) | — | — | — | — | diff --git a/docs/features/extending-with-providers.md b/docs/features/extending-with-providers.md index a9bc13f80..24b8f403a 100644 --- a/docs/features/extending-with-providers.md +++ b/docs/features/extending-with-providers.md @@ -96,6 +96,15 @@ registry.extend(TYPE_SOURCE, SOURCE_SUBTYPE_RDB, { `extend` throws if the target `(type, subType)` was never registered — order matters, which is why providers declare `dependencies`. +**Extending a spec-declared core subtype survives strict-mode load.** +Adding an attr to a subtype the *library* itself registers (e.g. a +consumer `@decimals` on the core `view.currency`) composes cleanly with a +**strict** load of metadata that uses the new attr — no need to fall back +to `--lax` for the whole file just because one attr came from a consumer +provider. This is conformance-gated across all five ports (the +`compose-load/` fixtures in +[`../../fixtures/provider-composition-conformance/`](../../fixtures/provider-composition-conformance/)). + ## Wiring providers into the loader Each port has its own loader entry point. The contract is identical: @@ -125,7 +134,13 @@ loader = MetaDataLoader.from_directory("./metadata", providers=[example_toolcall ```java // Java — SPI auto-discovery is the default; META-INF/services lists every provider -// (no API call required). Programmatic compose() factory is a planned follow-up. +// (no API call required). When a consumer genuinely needs extra vocabulary +// composed with the full metamodel provider set — so its metadata still +// strict-loads against the spec contract instead of getting the weaker +// classpath-SPI registry — the sanctioned seam is: +loader.setTypeRegistry(RegistryManifest.composeMetamodelRegistry(List.of(myProvider))); +// Calling MetaDataRegistry.compose(...) directly does NOT run spec scoping — +// use composeMetamodelRegistry(extras), not a raw compose() call. ``` The semantic rule is identical across ports: diff --git a/docs/ports/java.md b/docs/ports/java.md index 36f40aff3..a113b9132 100644 --- a/docs/ports/java.md +++ b/docs/ports/java.md @@ -144,10 +144,16 @@ all providers via Kahn's algorithm and emits the same stable error codes on failure (`ERR_PROVIDER_DUPLICATE_ID`, `_MISSING_DEPENDENCY`, `_DEPENDENCY_CYCLE`). -A programmatic `MetaDataRegistry.compose(List)` -factory (matching the explicit-list approach used by TS / C# / Python) is -on the follow-up backlog for callers who want to bypass SPI auto-discovery -in tests or embedded scenarios. The conceptual reference lives in +For callers who want to bypass SPI auto-discovery — or compose extra +consumer vocabulary on top of the full metamodel provider set so it still +strict-loads against the spec contract (no `--lax` fallback) — the +sanctioned seam is +`RegistryManifest.composeMetamodelRegistry(extraProviders)`, which composes +the core metamodel providers plus `extraProviders` and runs the full +spec-description + provenance-safe attr-scoping pipeline (hand the result +to `loader.setTypeRegistry(...)`). Raw `MetaDataRegistry.compose(...)` +composes only the explicit list, skips spec scoping, and is for +internal/test partial sets. The cross-port contract lives in [`../features/extending-with-providers.md`](../features/extending-with-providers.md). ## Generate diff --git a/docs/superpowers/plans/2026-08-02-issue-265-strict-scoping-provenance.md b/docs/superpowers/plans/2026-08-02-issue-265-strict-scoping-provenance.md new file mode 100644 index 000000000..8d148ae3e --- /dev/null +++ b/docs/superpowers/plans/2026-08-02-issue-265-strict-scoping-provenance.md @@ -0,0 +1,135 @@ +# #265 provenance-scoped strict attr scoping — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. This plan is self-sufficient — every API/construction path a task needs is inlined; do NOT rely on an external "file map". + +**Goal:** Make strict attribute scoping stop pruning consumer `registry.extend()` vocabulary, so all five ports byte-agree that extending a spec-declared core subtype and strict-loading metadata that uses it is valid — gated by new cross-port conformance fixtures. + +**Architecture:** Stamp each per-type attr with the id of the provider that registered it; the FR-033 B2b prune (`applyStrictAttrScoping`) then drops only attrs contributed by the *library's own* providers, never consumer extensions. Add a Java `composeMetamodelRegistry(extraProviders)` seam so JVM consumer registries get the same (now provenance-safe) scoping instead of skipping it. Prove it with a new success-scenario shape in `provider-composition-conformance`, in a **subdirectory** so un-updated runners never see it. + +**Tech Stack:** Python 3 (`metaobjects`), C# (.NET `MetaObjects`), Java 21 (`metaobjects-metadata`) + Kotlin (inherits JVM), TypeScript (reference — no product change). bun test / pytest / xUnit / JUnit4 conformance runners. + +## Global Constraints + +- PUBLIC repo — no private/other-project names, no absolute home paths in any committed file or commit message. +- **No new error codes.** `ERR_UNKNOWN_ATTR` (typos, misplaced core attrs) and `ERR_PROVIDER_ATTR_CONFLICT` (extend-time) unchanged. +- **`registry-conformance` manifest byte-match MUST stay unchanged** — the provenance guard only *spares* consumer attrs; library-only composition (which stamps everything with library ids or leaves it unstamped→library-default) is a no-op under the guard, so prune output incl. attr order is byte-identical. +- The strict *check* stays own-attrs-only (ADR-0039) — do NOT change check semantics. +- Named conformance providers are **test-only** — never in shipped metamodel providers. +- **Unstamped / build-time-enrichment registrations default to LIBRARY-origin (prunable).** Any missed stamping path degrades to *today's* behavior; the new fixtures catch a missed consumer-side stamp. Sentinel + library-id set are **named constants** per each port's constants discipline. +- Commit author + standard `Co-Authored-By` / `Claude-Session` trailers per repo convention. +- Design doc: `docs/superpowers/specs/2026-08-02-issue-265-strict-scoping-provenance-design.md`. + +## File Structure + +- **Shared fixtures** in a NEW subdir `fixtures/provider-composition-conformance/compose-load/`: `extend-spec-subtype-registry.json`, `extend-spec-subtype-strict-load.json`, `extend-spec-subtype-typo-rejected.json`, `misplaced-core-attr-consumer-registry.json`. The flat corpus dir stays **error-shape-only** (its 5 existing manifests unchanged). README documents the subdir + why (older-runner compatibility — all four runners list the corpus dir NON-recursively and hard-require the old shape, so a new-shape manifest in the flat dir would red every un-updated runner). +- **TS runner** (reference) `server/typescript/packages/metadata/test/provider-composition-conformance.test.ts`. No product change. +- **Python** product: `provider.py`, `registry.py`, `spec_metamodel/__init__.py`, `core_types.py`; runner `tests/conformance/test_provider_composition_conformance.py`. +- **C#** product: `MetaObjects/Provider.cs`, `MetaObjects/Registry.cs`, near `Loader/MetaDataLoader.cs` `DefaultRegistry`; runner `ProviderCompositionConformanceTests.cs`. +- **Java** product: `MetaDataRegistry.java`, `RegistryManifest.java`; runner `ProviderCompositionConformanceTest.java` (covers Kotlin). + +## Manifest-shape extension (used by all tasks) + +The existing error-code manifests (`{description, providers[], expectedError, sealThenRegister?}`) are UNCHANGED. Every runner update must make `expectedError` **optional** and **dispatch on shape** (presence of the new keys). New OPTIONAL keys for the `compose-load/` subdir scenarios: + +```jsonc +{ + "description": "...", + "providers": ["extend-spec-subtype"], // named test providers, composed AFTER the library core set + "composeWithCore": true, // compose the port's LIBRARY provider set first, then `providers` + "expectAttrs": { // OPTIONAL registry-inspection: the port's declared-attr lookup its strict check uses + "type": "view", "subType": "currency", "contains": ["locale", "decimals"] + }, + "metadata": { "metadata.root": { ... } }, // OPTIONAL canonical-JSON doc to strict-load + "expectErrors": ["ERR_UNKNOWN_ATTR"] // OPTIONAL error codes the strict load must surface ([] = expect success) +} +``` + +Runner behavior: if `composeWithCore`, compose `[...libraryProviders, ...named]`; else today's named-only. If `expectAttrs`, assert the port's declared-attr set for `(type,subType)` ⊇ `contains` (flat lookup in TS/Python/C#; Java via `typeDef.getChildRequirement(name)`, direct-or-inherited). If `metadata`, strict-load it and assert the surfaced `.code`s equal `expectErrors` (order-insensitive; `[]` = zero errors). + +**Canonical named provider `extend-spec-subtype`:** id `"extend-spec-subtype"`, **no dependencies** (the provider that registers `view.currency` has a different id per port, and the corpus mandates identical id/deps across ports — so ordering is guaranteed by the `composeWithCore` contract [library set first, named appended; every port's compose is a stable topo-sort preserving input order], NOT by a declared dep). `registerTypes` extends `view.currency` with one **int** attr `decimals`. Note in the README that `composeWithCore` is the sanctioned exception to Python's "extenders MUST declare a dependency" docstring. + +**The strict-load metadata docs** (fixtures 2/3/4): a `metadata.root` with one `object.entity` carrying an `identity.primary` (so the doc is otherwise clean), and: +- fixtures 2/3: a **`field.currency`** field (that is where a `view.currency` child is structurally admitted; `@currency` may be omitted — optional, default USD) with a `view.currency` child carrying `@decimals: 2` (fixture 2) / `@decimalz: 2` (fixture 3). +- fixture 4: a **`field.boolean`** field carrying `@maxLength: 5` (a misplaced core attr). + +**Per-runner strict-load construction** (inline — do NOT reach for a `fromString`-style factory blindly; only TS/Python have strict+registry factories): +- **TS**: `MetaDataLoader.fromString(doc, "json", { registry, strict: true })` (`LoadOptions` carries both). +- **Python**: `MetaDataLoader.from_string(content, providers=[*core_providers, extend_provider], strict=True)` (the loader takes *providers*, not a registry). `expectAttrs` uses `compose_registry([*core_providers, extend_provider]).attrs_of(type, subType)`. +- **C#**: `new MetaDataLoader(registry, strict: true)` then `loader.Load(new IMetaDataSource[]{ new InMemoryStringSource(doc, format: Json) })` — `FromString`/`FromDirectory` have NO strict+inline path. +- **Java**: `new MetaDataLoader(LoaderOptions.create(false, false, true), MetaDataLoader.SUBTYPE_MANUAL, name)` → `setTypeRegistry(composedRegistry)` **before** `init()` → `init()` → `load(List.of(new InMemoryStringSource(content, "", format)))` → collect codes from `getErrors()`. + +The four fixtures: +1. `extend-spec-subtype-registry`: `composeWithCore`, `providers:["extend-spec-subtype"]`, `expectAttrs:{view,currency,contains:[locale,decimals]}`. +2. `extend-spec-subtype-strict-load`: + `metadata` (field.currency w/ `view.currency @decimals:2`), `expectErrors:[]`. +3. `extend-spec-subtype-typo-rejected`: + `metadata` (`@decimalz:2`), `expectErrors:["ERR_UNKNOWN_ATTR"]`. +4. `misplaced-core-attr-consumer-registry`: `composeWithCore`, `providers:["extend-spec-subtype"]`, `metadata` (field.boolean w/ `@maxLength:5`), `expectErrors:["ERR_UNKNOWN_ATTR"]`. + +--- + +### Task 1: Subdir fixtures + TS reference lane (all green) + +**Files:** the 4 fixtures under `compose-load/` + README (create/modify); TS runner (modify — no product change). + +- [ ] **Step 1: Write the 4 fixture JSONs** in `fixtures/provider-composition-conformance/compose-load/` exactly per the shape + metadata-doc rules above. Verify each `metadata` body is valid canonical JSON. +- [ ] **Step 2: Extend `README.md`** — document the `compose-load/` subdir (+ why: non-recursive older-runner compatibility), the new keys (`composeWithCore`/`expectAttrs`/`metadata`/`expectErrors`), the `extend-spec-subtype` named-provider entry (id/no-deps/registers `decimals` on `view.currency`), and the `composeWithCore`-vs-"extenders-must-declare-a-dep" exception note. +- [ ] **Step 3: Extend the TS runner** — glob the `compose-load/` subdir too; make `expectedError` optional + dispatch on shape; add `extend-spec-subtype` (`registry.extend("view","currency", )`); implement compose-with-core (`coreProviders`), the `expectAttrs` assertion, and strict-load via `MetaDataLoader.fromString(doc, "json", { registry, strict: true })` collecting `.code`s. +- [ ] **Step 4: Run TS** — `cd server/typescript && bun test packages/metadata/test/provider-composition-conformance.test.ts`. Expected: **all 4 green** (TS = reference: extension survives, `@decimals` accepted, `@decimalz` + misplaced `@maxLength` rejected). If any fail, the fixture/runner is wrong — fix before proceeding (TS defines correct behavior). +- [ ] **Step 5: Commit** `test(#265): compose-load conformance subdir + 4 extend-subtype fixtures (TS reference green)`. + +--- + +### Task 2: Python — RED baseline, then provenance fix + +**Files:** runner `test_provider_composition_conformance.py`; `provider.py`, `registry.py`, `spec_metamodel/__init__.py`, `core_types.py`. + +- [ ] **Step 1: Extend the Python runner** — scan the `compose-load/` subdir; `expectedError` optional + shape dispatch; add `extend-spec-subtype`; compose-with-core = `[*core_providers, extend_provider]` (NOTE: `core_providers` is a LIST at `core_types.py`, not a callable — no `()`); `expectAttrs` via `compose_registry([...]).attrs_of(...)`; strict-load via `MetaDataLoader.from_string(content, providers=[*core_providers, extend_provider], strict=True)` collecting `.code`s. +- [ ] **Step 2: Run — RED baseline.** `cd server/python && uv run pytest tests/conformance/test_provider_composition_conformance.py -k extend -q`. Expected: fixtures 1,2,3 **FAIL** (the prune deletes `decimals`); fixture 4 passes. This is the confirmed #265 repro as a gated test. +- [ ] **Step 3: Stamp provenance at registration.** `provider.py` compose loop: set `registry._current_provider_id = p.id` around each `register_types`, clear after (finally). `registry.py` `register`/`extend`: record `(type, subType, attr_name) -> current_provider_id` in a registry side-map; NO current id → the `_LIBRARY` sentinel constant. +- [ ] **Step 4: Library-id set constant.** In `core_types.py` expose a frozen `LIBRARY_PROVIDER_IDS` = the ids of the core/db/doc/prompt/ui library providers (named constant). +- [ ] **Step 5: Guard the prune.** In `_apply_strict_attr_scoping` (`spec_metamodel/__init__.py`): drop iff `prunable AND name not in allow AND (origin is _LIBRARY or origin in LIBRARY_PROVIDER_IDS)` — a consumer-origin attr is never pruned; an unstamped/build-time attr (origin `_LIBRARY`) still prunes (today's behavior). +- [ ] **Step 6: Run — GREEN.** Same `-k extend` command → all 4 pass. Then `cd server/python && uv run pytest -q` full suite, and confirm `registry-conformance` byte-match unchanged. +- [ ] **Step 7: Commit** `fix(#265): provenance-scoped strict attr prune (Python) — spare consumer extends`. + +--- + +### Task 3: C# — RED baseline, then provenance fix (mirror Python) + +**Files:** runner `ProviderCompositionConformanceTests.cs`; `Provider.cs`, `Registry.cs`, near `DefaultRegistry`. + +- [ ] **Step 1: Extend the C# runner** — scan `compose-load/`; `ExpectedError` optional + shape dispatch; `extend-spec-subtype`; compose-with-core = the 4 `DefaultRegistry` library providers + named; `expectAttrs` via `AttrsOf`; strict-load via `new MetaDataLoader(registry, strict: true)` + `loader.Load(new IMetaDataSource[]{ new InMemoryStringSource(doc, format: Json) })` collecting codes. +- [ ] **Step 2: Run — RED.** `cd server/csharp && dotnet test --filter ProviderComposition`. Expected: fixtures 1,2,3 FAIL (live confirmation C# shares the prune — was code-read-only); 4 passes. +- [ ] **Step 3: Stamp provenance** — `Provider.cs::ComposeRegistry` sets a `CurrentProviderId` around each provider; `Registry.cs::Register`/`Extend` record `(type,subType,attr) -> id`; no-current → `LibrarySentinel` constant. +- [ ] **Step 4: Library-id set constant** beside `Loader/MetaDataLoader.cs::DefaultRegistry` (the four provider ids). +- [ ] **Step 5: Guard the prune** — `Registry.cs::ApplyStrictAttrScoping`: `... AND (origin == LibrarySentinel || LibraryProviderIds.Contains(origin))`. +- [ ] **Step 6: Run — GREEN** + full C# suite + `registry-conformance` byte-match unchanged. +- [ ] **Step 7: Commit** `fix(#265): provenance-scoped strict attr prune (C#) — spare consumer extends`. + +--- + +### Task 4: Java/Kotlin — consumer-path seam (scaffolding) then provenance fix + +**Files:** runner `ProviderCompositionConformanceTest.java`; `MetaDataRegistry.java`, `RegistryManifest.java`. + +- [ ] **Step 1: Add the seam FIRST (scaffolding, not the fix).** Add `RegistryManifest.composeMetamodelRegistry(Collection extra)`: compose `metamodelProviders() + extra` → `getAllValidationConstraints()` → `applySpecDescriptions(SpecMetamodelReader.load())` → return **unsealed** (caller may seal). Document it as the `MetaDataLoader.setTypeRegistry(...)` seam. Raw `MetaDataRegistry.compose(...)` unchanged. (This only routes the consumer path THROUGH scoping — the prune is still provenance-blind until Steps 4-5.) +- [ ] **Step 2: Extend the Java runner** — scan `compose-load/`; `expectedError` optional + shape dispatch; add `extend-spec-subtype` as `registry.extendType(com.metaobjects.view.CurrencyView.class, def -> def.optionalAttribute("decimals", ))` (`view.currency` is registered by `CurrencyView.registerTypes`; `optionalAttribute(name, subType)` exists on `TypeDefinitionBuilder`; model on `CoreDBMetaDataProvider`'s attr registrations). `composeWithCore` composes via **`composeMetamodelRegistry(List.of(extendProvider))`** (the seam — the path adopters use), NOT raw `compose()`. Strict-load recipe: `new MetaDataLoader(LoaderOptions.create(false,false,true), SUBTYPE_MANUAL, name).setTypeRegistry(composed)` **before** `init()`, then `load(List.of(new InMemoryStringSource(content, "", Json)))`, collect `getErrors()` codes. +- [ ] **Step 3: Run — RED baseline.** `cd server/java && mvn -q -pl metadata test -Dtest=ProviderCompositionConformanceTest`. Expected: **fixtures 1,2,3 FAIL** (the provenance-blind prune eats `decimals` — live proof Java shares the prune bug, previously PLAUSIBLE-only); **fixture 4 GREEN** (the seam routes the misplaced `@maxLength` through scoping → correctly `ERR_UNKNOWN_ATTR`; note fixture 4 is a **regression lock for the seam**, not a red-first test). +- [ ] **Step 4: Stamp provenance.** `MetaDataRegistry.registerProviders` sets `currentProviderId` around each `provider.registerTypes(this)`. Hook **both** registration entry points: (a) `register(...)` records `(type,subType,attrName) -> id` for its attr requirements; (b) `extendType(Class, Consumer)` is class-keyed and rebuilds via `TypeDefinitionBuilder.from(existing)` writing straight into `typeDefinitions` **without** `register()` — after `build()`, **diff** the attr-typed direct-requirement names against `existing`'s and stamp ONLY the NEW names with `currentProviderId` (blanket-stamping would mis-attribute pre-existing library attrs; a same-name redefinition keeps its prior origin). Build-time / constraint-expansion copies (no current id) → `_LIBRARY` sentinel. Derive the library-id set at runtime from `RegistryManifest.metamodelProviders()` (each `getProviderId()`), as a memoized constant. +- [ ] **Step 5: Guard the prune.** `applyStrictAttrScoping`: on BOTH the direct AND inherited requirement maps, the drop clause gains `&& isLibraryOrigin(id.type(), id.subType(), req.getName())` where `isLibraryOrigin` = stamped by a `metamodelProviders()` id OR unstamped (`_LIBRARY`). Inherited copies are unstamped at the child key → library-origin → still prune (correct — they originate from library base registrations; this is the intended convergent behavior). `isPrunableAttr` unchanged. +- [ ] **Step 6: Run — GREEN** — `ProviderCompositionConformanceTest` all 4 green (Kotlin inherits via the JVM runner). Then `mvn -q -pl metadata test` full, and confirm the sealed default `composeMetamodelRegistry()` (no extras) emits the **byte-identical** `registry-conformance` manifest. +- [ ] **Step 7: Commit** `fix(#265): provenance-scoped prune + composeMetamodelRegistry(extras) seam (Java/Kotlin)`. + +--- + +### Task 5: Verify + docs + review + PR + +- [ ] **Step 1: Cross-port green.** Run all four runners; the 4 fixtures green in TS/Python/C#/Java(+Kotlin). Confirm `registry-conformance` manifest byte-match unchanged in every port. +- [ ] **Step 2: Docs.** Update `docs/features/extending-with-providers.md` with a note that extend-under-strict is conformance-gated (and, for Java adopters, the `setTypeRegistry(composeMetamodelRegistry(extras))` seam). The design doc's non-goals already record: the core-attr-name `ERR_PROVIDER_ATTR_CONFLICT` residual, the **B2a structural-children twin** (provenance-blind for consumer child-rule extends — same class, out of scope, will re-file structurally), and the convergent base-subtype behavior — verify those are present. +- [ ] **Step 3: Per-unit review** — code-reviewer + code-simplifier on the diff; fix findings in place. +- [ ] **Step 4: no-mistakes gate** — rich `--intent` (the three-way divergence; provenance mechanism incl. the unstamped→library default; the Java two-entry-point diff-stamp; the deliberate registry-conformance-unchanged invariant; the B2a residual). Ensure `.serena/` + `.worktrees/` in `.git/info/exclude`. +- [ ] **Step 5: PR** — `Closes #265`; body: the three-way divergence table, the provenance fix, the Java consumer-path fold-in, the 4 conformance fixtures (in the `compose-load/` subdir), the accepted residuals (core-attr-name conflict + B2a structural twin), and that #267 unblocks on this. **Coordinated cross-port patch** (PyPI + NuGet + Maven; npm reference-only, no product change) — flag that the release is coordinated when Doug cuts it. + +## Self-Review + +- **Spec coverage:** provenance stamp + guard (Python T2 / C# T3 / Java T4) ✓; Java consumer-path seam (T4 S1) ✓; 4 conformance fixtures + shape, in a subdir so un-updated runners stay green (T1, exercised T2–T4) ✓; TS reference lane (T1) ✓; registry-conformance-unchanged invariant asserted per port (T2/T3/T4 step 6, T5 step 1) ✓; accepted residuals documented (design + T5) ✓; batch note re #267 (T5) ✓. +- **No placeholders:** every construction/API path (strict-load per port, Java two-entry-point diff-stamp, the seam signature, the fixture metadata-doc rules, the unstamped→LIBRARY default) is inlined above — the plan is self-sufficient for fresh per-task subagents. +- **Type consistency:** `extend-spec-subtype` (no-deps, `decimals` on `view.currency`) + the manifest keys (`composeWithCore`/`expectAttrs`/`metadata`/`expectErrors`, `expectedError` now optional) are defined once (shape section) and consumed identically by all five runners; `composeMetamodelRegistry(extra)` (T4 S1) is consumed by the Java runner (T4 S2). diff --git a/docs/superpowers/specs/2026-08-02-issue-265-strict-scoping-provenance-design.md b/docs/superpowers/specs/2026-08-02-issue-265-strict-scoping-provenance-design.md new file mode 100644 index 000000000..3b3356c4a --- /dev/null +++ b/docs/superpowers/specs/2026-08-02-issue-265-strict-scoping-provenance-design.md @@ -0,0 +1,79 @@ +# #265 — strict attr scoping must not prune consumer `registry.extend()` vocabulary: design + +_Date: 2026-08-02 · Issue: [#265](https://github.com/metaobjectsdev/metaobjects/issues/265) · Scope: cross-port loader (Python + C# + Java product; TS is the reference) + a new conformance corpus shape · Status: designed (Fable cross-port investigation + live Python/TS repro)_ + +## Problem — a three-way cross-port divergence (not "Python only") + +On identical input — the library core providers **plus** a consumer provider that `registry.extend()`s a **spec-declared** core subtype with one extra attr (`@decimals` on `view.currency`), then strict-loading metadata that uses it — the five ports disagree: + +| Port | Behavior | Correct? | +|---|---|---| +| **TypeScript** | **Accepts** — no spec-scoping prune step exists | ✅ reference behavior (reproduced live) | +| **Python** | **Rejects** `ERR_UNKNOWN_ATTR` — the prune deletes the extension | ❌ the bug (reproduced live) | +| **C#** | **Rejects** — line-for-line the same compose-then-prune | ❌ same bug (code-read) | +| **Java / Kotlin** | **Accepts** — but only because the consumer-composed registry **skips spec scoping entirely** | ❌ a *second* bug: JVM consumer registries get a **weaker** strict mode (they also wrongly accept a misplaced core attr) | + +`#265`'s "Python port only" scope is wrong: **Python + C# share the prune bug; Java/Kotlin have the complementary no-scoping-on-consumer-path bug; TS is the reference.** The only workaround today (`--lax`) disables unknown-attr checking for the **whole file**, so an adopter who follows the documented "extend the registry for app vocabulary" path (`docs/features/extending-with-providers.md`) ends up with a permanently weaker gate. + +## Root cause + +FR-033 sub-step B2b's `applyStrictAttrScoping` (Python `_apply_strict_attr_scoping`, C# `ApplyStrictAttrScoping`, Java `applyStrictAttrScoping`) is a **port-alignment shim**: it trims each port's legacy over-broad attr registrations down to the spec-exact per-subtype graph so the registry-conformance manifest byte-matches `expected-registry.json` and a misplaced core attr (`@maxLength` on `field.boolean`) stays `ERR_UNKNOWN_ATTR`. It prunes any prunable attr whose **name** is not in the shipped spec allow-list — **blind to who registered it** — so it also deletes consumer `registry.extend()` extensions. TS needs no shim because its providers already register spec-exact vocabulary. + +**The strict *check* is not the bug.** Every port's strict check is deliberately own-attrs-only (ADR-0039: an inherited attr is validated once, at its declaring node), so metadata-level `extends` inheritance already works. The maintainer's "look it up the extends tree" is satisfied by **restoring the provider-registered vocabulary to the registry** — no check-semantics change (an abstract parent declaring the attr would fail identically today, because the *registry* lacks it for the subtype; tree-walking the check cannot fix a pruned registry). + +## Intended contract + +Strict load = **membership in the composed registry's declared vocabulary** (ADR-0023). An authored own `@attr` is legal iff a *registered* provider — library **or downstream** — declared it for that `(type, subType)`, or it is a `commonAttr`, or it is an `attr.properties` bag. The spec allow-list constrains **only what the library's own providers contribute**; consumer registrations always survive; all five ports byte-agree; required attrs stay enforced and genuine typos stay rejected. + +## Decision — provenance-scoped prune (+ close the Java consumer-path gap) + +Record the **contributing provider id** per per-type attr at registration; B2b prunes only attrs contributed by the **library's own** metamodel providers. + +- Compose loop sets a `currentProviderId` around each `provider.registerTypes(registry)`; `register`/`extend` stamp it into a side map keyed `(type, subType, attrName)` (no change to the frozen attr-schema types). Attrs registered outside any compose loop (build-time enrichment) default to **library-origin**. +- The prune condition gains one clause: prunable **AND** not-in-allow-list **AND** origin ∈ library-provider-id set. Each port already owns that set (Python `core_providers`; C# the four in `MetaDataLoader.DefaultRegistry`; Java `RegistryManifest.metamodelProviders()`; TS `coreProviders`). +- **Java consumer-path fix:** add a `RegistryManifest.composeMetamodelRegistry(extraProviders)` overload (compose core + extras → force constraints → `applySpecDescriptions`, now provenance-safe) and document it as the sanctioned `MetaDataLoader.setTypeRegistry(...)` seam, so JVM consumer registries get the SAME tightening every other port has. Raw `MetaDataRegistry.compose(...)` keeps today's semantics (tests rely on partial sets). Kotlin inherits both for free. + +**Rejected alternatives.** (c) *No prune; resolve allowance at check time* — the pruned registry is the single vocabulary truth read by the manifest emitters, docs-gen, YAML desugar, and allowedValues validation; special-casing only the check re-opens the misplaced-attr hole or needs the same provenance in more places. *Reorder (scope after library, before consumer providers)* — behaviorally equivalent for the standard `[*core, *mine]` list and simpler, but needs the same library-membership knowledge, mis-classifies an interleaved provider, and cannot express the Java fix (Java's scoping isn't in `compose()`). Provenance is strictly more robust. + +**No new error codes.** `ERR_UNKNOWN_ATTR` still fires for typos and misplaced core attrs; `ERR_PROVIDER_ATTR_CONFLICT` unchanged at extend time. + +## Per-port touch list + +- **Python**: `provider.py` (compose loop stamps id), `registry.py` (`register`/`extend` record origin), `spec_metamodel/__init__.py::_apply_strict_attr_scoping` (origin guard), `core_types.py` (export the frozen library-id set). +- **C#**: `MetaObjects/Provider.cs` (ComposeRegistry loop), `MetaObjects/Registry.cs` (`Register`/`Extend` record + `ApplyStrictAttrScoping` guard), library-id set beside `DefaultRegistry`. +- **Java**: `MetaDataRegistry.registerProviders` (stamp id) + `applyStrictAttrScoping` (origin guard on direct AND inherited maps); new `RegistryManifest.composeMetamodelRegistry(extraProviders)` overload; docs for the `setTypeRegistry` seam. Kotlin inherits. +- **TypeScript**: no product change — it is the semantics target and the reference lane of the new conformance scenarios. + +## Conformance — the gap that let this ship, and its fix + +**Why unseen:** no corpus composes a *consumer* provider and then strict-loads. `provider-composition-conformance` asserts error codes at compose time only, and its one extending provider deliberately registers a **fresh non-spec subtype** (dodging the prune); `registry-conformance` composes library providers only (a consumer attr would break its byte-match by design); the sole `ERR_UNKNOWN_ATTR` fixture holds only the negative case. + +**Home:** `fixtures/provider-composition-conformance/` (its five runners already have the named-provider machinery). Extend the manifest shape with success scenarios: `"composeWithCore": true` (compose the port's library provider set first) + an optional `"metadata"` + `"expect"` block (strict-load an embedded doc; assert error codes only, per corpus convention). New canonical named provider `extend-spec-subtype` extends `view.currency` with one `int` attr `decimals`. + +Four fixtures: +1. **`extend-spec-subtype-registry`** — compose core + provider; assert `attrsOf("view","currency")` contains BOTH `locale` and `decimals`. (Fails today on Python + C#; passes TS + Java → catches the bug AND the divergence.) +2. **`extend-spec-subtype-strict-load`** — strict-load a doc with `@decimals` on a `view.currency`; expect zero errors. (The end-to-end #265; fails today on Python/C#.) +3. **`extend-spec-subtype-typo-rejected`** — strict-load `@decimalz`; expect exactly `ERR_UNKNOWN_ATTR`. (Guards against over-correcting — typo-catching stays intact on an extended subtype.) +4. **`misplaced-core-attr-consumer-registry`** — strict-load `@maxLength` on a `field.boolean`; expect `ERR_UNKNOWN_ATTR` in ALL ports. (Catches the **Java** flavor — its consumer-path registry never got B2b — and forces the `composeMetamodelRegistry(extras)` routing.) + +Fixtures 1 + 4 together catch both the bug and all three divergent behaviors in the existing five-port gate. + +## Accepted residual / non-goals + +- **Extending a subtype with a core-attr *name* the port legacy-registers broadly** (e.g. `@maxLength` onto `field.boolean` via `extend`) throws `ERR_PROVIDER_ATTR_CONFLICT` on Python/C#/Java but succeeds on TS (whose registry never had it there). The deep fix — every port registers exactly-per-spec and the prune is deleted — is the eventual end state but a large per-port registration refactor; **out of scope for #265**, documented as a known residual. +- **The SAME provenance-blind hole exists in FR-033 sub-step B2a — strict *structural-children* scoping** (`_apply_strict_structural_children` / Java pass 4 / C# equivalent keep only attr-type rules + the spec's strict child graph). A consumer provider that extends a spec-declared subtype with an extra **child rule** (not an attr) would be pruned identically. #265 is attrs-only and the `extend-spec-subtype` provider adds only an attr, so the fix + fixtures do not cover B2a. It is named here so a future consumer child-rule extension re-files as the structural twin of #265, not a surprise — the same provenance mechanism would extend to it when needed. +- No change to the strict *check* semantics (own-only is correct, ADR-0039). No new error codes. No metamodel vocabulary change. +- **Convergent side-effect (intended):** a consumer extending a *base* subtype (e.g. `field.base`) ends up legal on the base only — the inherited copies on concrete subtypes are unstamped (library-origin) and still prune — which converges Java with the flat-registry semantics of TS/Python/C#. + +## Batch context + +- **#267** (Python declarative codegen config) formalizes the exact `--provider module:symbol` path #265 breaks — **fix #265 first** or #267 ships a broken sanctioned path. +- **#266** (codegen-ts enums.ts path collision) and **#258** (migrate PK-change kind) do not interact. +- The Java consumer-path weaker-strict bug is folded into this fix (not filed separately). + +## Verification + +- Reproduce the bug in a **failing test first** in each affected port (Python + C# + Java) — Fable executed only Python + TS; C#/Java behavior is code-read and must be confirmed by a red test before the fix. +- The 4 conformance fixtures green in all five port runners (TS unchanged, Python/C#/Java fixed). +- Existing `registry-conformance` manifest byte-match unchanged (library-only composition is untouched by a provenance guard that only *spares* consumer attrs). +- Full metadata/loader suites green per port. diff --git a/fixtures/provider-composition-conformance/README.md b/fixtures/provider-composition-conformance/README.md index 179a83763..8ba95d0a9 100644 --- a/fixtures/provider-composition-conformance/README.md +++ b/fixtures/provider-composition-conformance/README.md @@ -122,3 +122,99 @@ Composing `["attr-conflict-base", "attr-conflict-clash"]` → `ERR_PROVIDER_ATTR dependencies wiring is the entire contract. - The `.code` read off a caught exception is the assertion surface — message text is never compared (message wording is per-port). + +## The `compose-load/` subdir (#265) + +#265 gates a **different** invariant than the five error codes above: strict attr +scoping must not wrongly prune an attribute a *consumer* provider added (via +`registry.extend()`) to a spec-declared **core** subtype. That requires composing a +consumer provider on top of the port's real **library** provider set (not just named +test providers in isolation) and, for two of the four scenarios, strict-loading an +actual metadata document against the composed registry — a shape the five +error-code manifests above don't need and don't carry. + +These `compose-load/` fixtures live in their **own subdirectory**, not the flat +corpus dir, for backward compatibility: every existing runner (TS / Python / C# / +Java) lists the corpus directory **non-recursively** and hard-requires the old +`{description, providers[], expectedError, sealThenRegister?}` shape for every +`.json` file it finds there. Dropping a new-shape manifest into the flat dir would +red every not-yet-updated runner. `compose-load/` is invisible to a non-recursive +`readdir` of the parent, so an un-updated runner keeps passing unchanged; a runner +that has been extended for #265 globs `compose-load/` as a **second**, separate +pass. The flat dir's 5 existing manifests are unchanged. + +### Shape + +Each `compose-load/*.json` manifest carries some or all of these OPTIONAL keys +(`description` and `providers` are still present; `expectedError` / +`sealThenRegister` from the flat-dir shape do NOT appear here — a runner dispatches +on which shape a manifest carries): + +```jsonc +{ + "description": "...", + "providers": ["extend-spec-subtype"], // named test providers, composed AFTER the library core set + "composeWithCore": true, // compose the port's LIBRARY provider set first, then `providers` + "expectAttrs": { // OPTIONAL registry-inspection: the port's declared-attr lookup its strict check uses + "type": "view", "subType": "currency", "contains": ["locale", "decimals"] + }, + "metadata": { "metadata.root": { "..." : "..." } }, // OPTIONAL canonical-JSON doc to strict-load + "expectErrors": ["ERR_UNKNOWN_ATTR"] // OPTIONAL error codes the strict load must surface ([] = expect success) +} +``` + +Runner behavior: + +1. If `composeWithCore`, compose `[...libraryProviders, ...namedProviders]`; else + (today's flat-dir behavior) compose the named providers alone. +2. If `expectAttrs` is present, assert the port's declared-attr set for + `(type, subType)` is a **superset** of `contains` (flat lookup in TS / Python / + C#; Java via `typeDef.getChildRequirement(name)`, direct-or-inherited). +3. If `metadata` is present, strict-load it and assert the surfaced `.code`s equal + `expectErrors` exactly (order-insensitive; `[]` means the load must surface zero + errors). + +No new error codes: these scenarios only ever surface `ERR_UNKNOWN_ATTR` (already +gated above) or nothing. + +### Canonical named provider `extend-spec-subtype` + +- **id:** `"extend-spec-subtype"` +- **dependencies:** **none** — deliberately. The provider that registers + `view.currency` (the library's core-types provider) has a **different id per + port**, and this corpus mandates an identical id/dependency set across ports for + every named provider — so `extend-spec-subtype` cannot name that provider as a + dependency without breaking cross-port id parity. Ordering is instead guaranteed + by the `composeWithCore` contract: the library set composes first, the named set + is appended after, and every port's compose is a **stable** topological sort that + preserves input order among providers with no ordering constraint between them. + `composeWithCore` is the sanctioned exception to Python's "an extender MUST + declare a dependency on what it extends" docstring guidance — it's a corpus-level + ordering guarantee, not a per-provider one. +- **registerTypes:** `extend`s `view.currency` (a subtype the library's own core + provider registers) with one new **int** attr, `decimals`. + +Composing `[...coreProviders, "extend-spec-subtype"]` must succeed, and the +resulting registry's declared-attr set for `(view, currency)` contains BOTH +`locale` (core-declared) and `decimals` (consumer-added). + +### The four fixtures + +1. **`extend-spec-subtype-registry`** — `composeWithCore` + `extend-spec-subtype`; + asserts `(view, currency)` in the composed registry's declared-attr lookup + contains `locale` and `decimals`. +2. **`extend-spec-subtype-strict-load`** — same composition, plus a metadata + document with a `field.currency` (the structural parent a `view.currency` child + is admitted under; `@currency` itself may be omitted, it defaults to `USD`) + carrying a `view.currency @decimals: 2` child. Strict-loads with + `expectErrors: []` — the consumer-added attr is accepted, not pruned. +3. **`extend-spec-subtype-typo-rejected`** — same shape, but `@decimalz: 2` (a + typo — not the provider-registered name). Strict-loads with + `expectErrors: ["ERR_UNKNOWN_ATTR"]` — the extension widens scoping for its own + declared attr only, never for an arbitrary name. +4. **`misplaced-core-attr-consumer-registry`** — same composition (a consumer + provider IS composed in), but the metadata document puts `@maxLength` (a CORE + attr declared only on `field.string`) on a `field.boolean`. Strict-loads with + `expectErrors: ["ERR_UNKNOWN_ATTR"]` — proves the provenance guard doesn't + accidentally widen scoping for misplaced CORE attrs just because a consumer + provider is present in the composition. diff --git a/fixtures/provider-composition-conformance/compose-load/extend-spec-subtype-registry.json b/fixtures/provider-composition-conformance/compose-load/extend-spec-subtype-registry.json new file mode 100644 index 000000000..2601c0583 --- /dev/null +++ b/fixtures/provider-composition-conformance/compose-load/extend-spec-subtype-registry.json @@ -0,0 +1,10 @@ +{ + "description": "Composing the library's core provider set plus a consumer provider that extends a spec-declared core subtype (view.currency) with a new attr (decimals) — the extended attr must appear alongside the core-declared locale attr in the registry's declared-attr lookup for (view, currency). #265: strict scoping must not prune a consumer-added attr on a core subtype.", + "composeWithCore": true, + "providers": ["extend-spec-subtype"], + "expectAttrs": { + "type": "view", + "subType": "currency", + "contains": ["locale", "decimals"] + } +} diff --git a/fixtures/provider-composition-conformance/compose-load/extend-spec-subtype-strict-load.json b/fixtures/provider-composition-conformance/compose-load/extend-spec-subtype-strict-load.json new file mode 100644 index 000000000..dc177a044 --- /dev/null +++ b/fixtures/provider-composition-conformance/compose-load/extend-spec-subtype-strict-load.json @@ -0,0 +1,30 @@ +{ + "description": "A metadata document authoring the consumer-extended view.currency @decimals attr strict-loads cleanly against the composed registry — the consumer extension survives strict scoping (#265).", + "composeWithCore": true, + "providers": ["extend-spec-subtype"], + "metadata": { + "metadata.root": { + "package": "acme::compose", + "children": [ + { + "object.entity": { + "name": "Invoice", + "children": [ + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } }, + { + "field.currency": { + "name": "amount", + "children": [ + { "view.currency": { "@decimals": 2 } } + ] + } + } + ] + } + } + ] + } + }, + "expectErrors": [] +} diff --git a/fixtures/provider-composition-conformance/compose-load/extend-spec-subtype-typo-rejected.json b/fixtures/provider-composition-conformance/compose-load/extend-spec-subtype-typo-rejected.json new file mode 100644 index 000000000..9ed5ed59c --- /dev/null +++ b/fixtures/provider-composition-conformance/compose-load/extend-spec-subtype-typo-rejected.json @@ -0,0 +1,30 @@ +{ + "description": "A typo'd attr (@decimalz, not the provider-registered @decimals) on view.currency is rejected under strict load even though the base subtype was consumer-extended — the extension widens scoping for its OWN declared attr only, never for an arbitrary name (#265).", + "composeWithCore": true, + "providers": ["extend-spec-subtype"], + "metadata": { + "metadata.root": { + "package": "acme::compose", + "children": [ + { + "object.entity": { + "name": "Invoice", + "children": [ + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } }, + { + "field.currency": { + "name": "amount", + "children": [ + { "view.currency": { "@decimalz": 2 } } + ] + } + } + ] + } + } + ] + } + }, + "expectErrors": ["ERR_UNKNOWN_ATTR"] +} diff --git a/fixtures/provider-composition-conformance/compose-load/misplaced-core-attr-consumer-registry.json b/fixtures/provider-composition-conformance/compose-load/misplaced-core-attr-consumer-registry.json new file mode 100644 index 000000000..c203dd7bc --- /dev/null +++ b/fixtures/provider-composition-conformance/compose-load/misplaced-core-attr-consumer-registry.json @@ -0,0 +1,28 @@ +{ + "description": "A misplaced CORE attr (@maxLength, declared only on field.string) authored on field.boolean is rejected under strict load even with a consumer provider composed into the registry — the provenance guard must not accidentally widen scoping for core attrs either (#265).", + "composeWithCore": true, + "providers": ["extend-spec-subtype"], + "metadata": { + "metadata.root": { + "package": "acme::compose", + "children": [ + { + "object.entity": { + "name": "Flag", + "children": [ + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } }, + { + "field.boolean": { + "name": "active", + "@maxLength": 5 + } + } + ] + } + } + ] + } + }, + "expectErrors": ["ERR_UNKNOWN_ATTR"] +} diff --git a/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs b/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs index 07069d801..ccd7acb7d 100644 --- a/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs +++ b/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs @@ -13,7 +13,10 @@ using System.Text.Json; using MetaObjects; +using MetaObjects.Core.Attr; +using MetaObjects.Loader; using MetaObjects.Meta; +using MetaObjects.Presentation.View; using MetaObjects.Shared; using Xunit; @@ -85,6 +88,34 @@ public void RegisterTypes(TypeRegistry registry) } } +/// +/// #265 `compose-load/` canonical named provider. Extends `view.currency` (a +/// SPEC-DECLARED CORE subtype the library's own core-types provider registers) +/// with a new `decimals` int attr. Deliberately NO dependencies — see the corpus +/// README "Canonical named provider `extend-spec-subtype`" for why (cross-port +/// id/dep parity vs. the `composeWithCore` ordering contract). +/// +internal sealed class ExtendSpecSubtypeProvider : IMetaDataTypeProvider +{ + public string Id => "extend-spec-subtype"; + public IReadOnlyList Dependencies => System.Array.Empty(); + + public void RegisterTypes(TypeRegistry registry) + { + registry.Extend( + BaseTypes.TYPE_VIEW, + ViewConstants.VIEW_SUBTYPE_CURRENCY, + attributes: new List + { + new AttrSchema( + "decimals", + AttrConstants.ATTR_SUBTYPE_INT, + Required: false, + Description: "Test-only — #265 compose-load probe attr."), + }); + } +} + public sealed class ProviderCompositionConformanceTests { private static readonly IReadOnlyDictionary Providers = @@ -99,12 +130,25 @@ public sealed class ProviderCompositionConformanceTests ["attr-conflict-base"] = new AttrConflictBaseProvider(), ["attr-conflict-clash"] = new AttrConflictClashProvider(), ["seal-probe"] = new SealProbeProvider(), + ["extend-spec-subtype"] = new ExtendSpecSubtypeProvider(), }; + // Flat-corpus (error-code) manifest shape — unchanged. + // #265 compose-load manifest shape — see fixtures/provider-composition-conformance/README.md + // "The `compose-load/` subdir". A manifest never carries both shapes; ExpectedError is + // OPTIONAL (null) on a compose-load manifest, and ExpectAttrs/Metadata/ExpectErrors are + // OPTIONAL (null) on a flat-corpus manifest — the two runner loops below dispatch on which + // fields are present. + private sealed record ComposeLoadExpectAttrs(string Type, string SubType, string[] Contains); + private sealed record Manifest( string[] Providers, - string ExpectedError, - string? SealThenRegister); + string? ExpectedError = null, + string? SealThenRegister = null, + bool? ComposeWithCore = null, + ComposeLoadExpectAttrs? ExpectAttrs = null, + JsonElement? Metadata = null, + string[]? ExpectErrors = null); private static string CorpusRootPath() { @@ -146,6 +190,16 @@ public void ProviderComposition(string fileName) var opts = new JsonSerializerOptions { PropertyNameCaseInsensitive = true }; var manifest = JsonSerializer.Deserialize( File.ReadAllText(Path.Combine(CorpusRootPath(), fileName)), opts)!; + + // Flat-corpus manifests always carry expectedError (the old shape); guard + narrow + // rather than a non-null assertion so a malformed fixture fails loud. + string? expectedError = manifest.ExpectedError; + if (expectedError is null) + { + throw new System.InvalidOperationException( + $"flat-corpus manifest \"{fileName}\" is missing required \"expectedError\""); + } + var resolved = manifest.Providers.Select(Resolve).ToList(); if (manifest.SealThenRegister != null) @@ -155,12 +209,80 @@ public void ProviderComposition(string fileName) registry.Seal(); var probe = Resolve(manifest.SealThenRegister); var sealedEx = Assert.Throws(() => probe.RegisterTypes(registry)); - Assert.Equal(manifest.ExpectedError, sealedEx.Code.ToString()); + Assert.Equal(expectedError, sealedEx.Code.ToString()); return; } // Ordinary scenario: the compose call itself throws. var ex = Assert.Throws(() => Provider.ComposeRegistry(resolved)); - Assert.Equal(manifest.ExpectedError, ex.Code.ToString()); + Assert.Equal(expectedError, ex.Code.ToString()); + } + + // ----------------------------------------------------------------------- + // #265 `compose-load/` corpus — see fixtures/provider-composition-conformance/ + // README.md "The `compose-load/` subdir". Own directory, own loop: a manifest + // here never carries `expectedError` / `sealThenRegister` (the flat-corpus shape + // above); it carries `composeWithCore` / `expectAttrs` / `metadata` / + // `expectErrors` instead. + // ----------------------------------------------------------------------- + + private static string ComposeLoadCorpusRootPath() => + Path.Combine(CorpusRootPath(), "compose-load"); + + public static IEnumerable ComposeLoadManifestFiles() + { + foreach (var file in Directory.GetFiles(ComposeLoadCorpusRootPath(), "*.json").OrderBy(f => f, System.StringComparer.Ordinal)) + yield return new object[] { Path.GetFileName(file) }; + } + + [Fact] + public void ComposeLoadCorpusIsNonEmpty() + { + Assert.NotEmpty(ComposeLoadManifestFiles()); + } + + [Theory] + [MemberData(nameof(ComposeLoadManifestFiles))] + public void ProviderCompositionComposeLoad(string fileName) + { + var opts = new JsonSerializerOptions { PropertyNameCaseInsensitive = true }; + var manifest = JsonSerializer.Deserialize( + File.ReadAllText(Path.Combine(ComposeLoadCorpusRootPath(), fileName)), opts)!; + + var named = manifest.Providers.Select(Resolve).ToList(); + List providerSet = manifest.ComposeWithCore == true + ? CoreTypes.LibraryProviders.Concat(named).ToList() + : named; + + TypeRegistry? registry = null; + + if (manifest.ExpectAttrs is not null) + { + registry = Provider.ComposeRegistry(providerSet); + var declaredNames = registry.AttrsOf(manifest.ExpectAttrs.Type, manifest.ExpectAttrs.SubType) + .Select(a => a.Name) + .ToList(); + foreach (string name in manifest.ExpectAttrs.Contains) + { + Assert.Contains(name, declaredNames); + } + } + + if (manifest.Metadata is JsonElement metadataElement) + { + registry ??= Provider.ComposeRegistry(providerSet); + string doc = metadataElement.GetRawText(); + var loader = new MetaDataLoader(registry, strict: true); + LoadResult result = loader.Load(new IMetaDataSource[] { new InMemoryStringSource(doc, format: MetaDataFormat.Json) }); + + var actualCodes = result.Errors + .Select(e => e.Code.ToString()) + .OrderBy(c => c, System.StringComparer.Ordinal) + .ToList(); + var expectedCodes = (manifest.ExpectErrors ?? System.Array.Empty()) + .OrderBy(c => c, System.StringComparer.Ordinal) + .ToList(); + Assert.Equal(expectedCodes, actualCodes); + } } } diff --git a/server/csharp/MetaObjects/CoreTypes.cs b/server/csharp/MetaObjects/CoreTypes.cs index b6f30afb2..da1b7d4c4 100644 --- a/server/csharp/MetaObjects/CoreTypes.cs +++ b/server/csharp/MetaObjects/CoreTypes.cs @@ -40,6 +40,42 @@ public static class CoreTypes /// public static readonly IMetaDataTypeProvider CoreTypesProvider = new CoreTypesProviderImpl(); + /// + /// #265 — the library's own default provider set (core + db + ui + prompt), kept as a + /// field so 's default registry and + /// share ONE enumeration (never hand-listed twice). + /// Also what the provider-composition-conformance runner composes for its + /// `composeWithCore` scenarios (real library providers + a named consumer provider) — + /// mirrors Python's core_types.core_providers. Homed here (a peer of + /// /, not Loader/MetaDataLoader.cs) + /// so the strict-attr-scoping guard below can consult it without introducing a + /// Registry → Loader back-reference (the pre-existing dependency direction is + /// strictly Loader → Registry/Provider/CoreTypes). + /// + public static readonly IReadOnlyList LibraryProviders = + [ + CoreTypesProvider, + // DB-domain field attrs (@column / @db.indexed / @dbColumnType) — Extend over core + // field types. Mirrors Java's CoreDBMetaDataProvider and TS's dbProvider. + MetaObjects.Persistence.Db.DbMetaDataProvider.Instance, + // FR-033 concern providers — re-home the UI / prompt attrs out of the core type + // classes (read spec/metamodel/ui.json + prompt.json). The prompt provider absorbs + // the @xmlText marker the former TemplateTypesProvider registered. Mirrors the TS + // ui/prompt provider split (and Java/Python). + MetaObjects.Presentation.Ui.UiMetaDataProvider.Instance, + MetaObjects.Template.PromptMetaDataProvider.Instance, + ]; + + /// + /// #265 — the ids of the above, derived (never + /// hand-listed). Consulted by (via + /// ) to decide whether an attr's provenance is + /// "library" (still prunable against the strict per-subtype allow-list) or a + /// consumer's own provider (never pruned). + /// + public static readonly IReadOnlySet LibraryProviderIds = + LibraryProviders.Select(p => p.Id).ToHashSet(); + // ------------------------------------------------------------------------- // wildcard helper — builds a ChildRule that matches any subType and name // ------------------------------------------------------------------------- diff --git a/server/csharp/MetaObjects/Loader/MetaDataLoader.cs b/server/csharp/MetaObjects/Loader/MetaDataLoader.cs index 9a4c21edd..ef76a1d14 100644 --- a/server/csharp/MetaObjects/Loader/MetaDataLoader.cs +++ b/server/csharp/MetaObjects/Loader/MetaDataLoader.cs @@ -71,19 +71,14 @@ public MetaDataLoader(TypeRegistry registry, bool freeze = true, bool strict = f _strict = strict; } + // #265 — the library's own default provider set (core + db + ui + prompt) and its + // derived provider-id set now live on CoreTypes (CoreTypes.LibraryProviders / + // CoreTypes.LibraryProviderIds) — a peer of Registry/Provider, not here — so + // Registry.cs::ApplyStrictAttrScoping can consult the id set without a + // Registry → Loader back-reference (Loader → Registry/Provider/CoreTypes is the + // sole pre-existing dependency direction). private static TypeRegistry DefaultRegistry() => - Provider.ComposeRegistry([ - CoreTypes.CoreTypesProvider, - // DB-domain field attrs (@column / @db.indexed / @dbColumnType) — Extend over core - // field types. Mirrors Java's CoreDBMetaDataProvider and TS's dbProvider. - MetaObjects.Persistence.Db.DbMetaDataProvider.Instance, - // FR-033 concern providers — re-home the UI / prompt attrs out of the core type - // classes (read spec/metamodel/ui.json + prompt.json). The prompt provider absorbs - // the @xmlText marker the former TemplateTypesProvider registered. Mirrors the TS - // ui/prompt provider split (and Java/Python). - MetaObjects.Presentation.Ui.UiMetaDataProvider.Instance, - MetaObjects.Template.PromptMetaDataProvider.Instance, - ]); + Provider.ComposeRegistry(CoreTypes.LibraryProviders); // ------------------------------------------------------------------------- // Static factories (the 99% case, cross-language consistent) diff --git a/server/csharp/MetaObjects/Provider.cs b/server/csharp/MetaObjects/Provider.cs index fb9896a8d..cd85c4441 100644 --- a/server/csharp/MetaObjects/Provider.cs +++ b/server/csharp/MetaObjects/Provider.cs @@ -53,7 +53,21 @@ public static TypeRegistry ComposeRegistry(IReadOnlyList TypeRegistry registry = new(); foreach (IMetaDataTypeProvider provider in ordered) { - provider.RegisterTypes(registry); + // #265 — stamp the active provider id for the duration of this provider's + // RegisterTypes() call (covers both its own Register()-ed definitions AND + // any registry.Extend() it triggers, e.g. via ApplyProviderExtends), so + // Registry.cs can attribute every attr it sees to the provider that + // registered it. Cleared after so LibrarySentinel is the default outside a + // compose loop. + registry.CurrentProviderId = provider.Id; + try + { + provider.RegisterTypes(registry); + } + finally + { + registry.CurrentProviderId = null; + } } // FR-033 (sub-step B1) — after every provider has registered and BEFORE the diff --git a/server/csharp/MetaObjects/Registry.cs b/server/csharp/MetaObjects/Registry.cs index 4cc80ef66..765ddf34c 100644 --- a/server/csharp/MetaObjects/Registry.cs +++ b/server/csharp/MetaObjects/Registry.cs @@ -202,6 +202,54 @@ public sealed class TypeRegistry /// Whether this registry has been sealed (ADR-0023). public bool IsSealed => _sealed; + // ------------------------------------------------------------------ + // #265 — attr provenance (which provider registered/extended each attr) + // ------------------------------------------------------------------ + + /// + /// #265 — sentinel attr-provenance origin for an attr registered/extended with no + /// active provider id (an unstamped registration — e.g. a hand-built registry in a + /// unit test that never goes through 's + /// stamping loop, or any future registration path that forgets to set + /// ). Treated as LIBRARY origin by the strict-attr- + /// scoping prune guard () — i.e. still + /// prunable, preserving pre-#265 behavior for anything not explicitly attributed + /// to a provider. + /// + public const string LibrarySentinel = "__library__"; + + /// + /// #265 — the provider id active during the CURRENT RegisterTypes() call, + /// set/cleared by around each provider's + /// turn. Null outside a compose loop (e.g. a hand-built registry in a unit test). + /// + internal string? CurrentProviderId { get; set; } + + /// + /// #265 — (type, subType, attrName) -> the provider id that registered/extended + /// that attr ( when unstamped). Consulted by + /// so a consumer-registered attr on a + /// spec-declared subtype is never pruned by the library's strict per-subtype + /// allow-list — only LIBRARY-origin attrs are. + /// + private readonly Dictionary<(string Type, string SubType, string AttrName), string> _attrProvenance = new(); + + /// Stamp provenance for one (type, subType, attrName) — the provider active + /// via , or when + /// unstamped. Called per-attr from (captures a provider's own + /// build-time enrichment too, since it reads the attrs already on the definition at + /// registration time) and from . + private void StampAttrProvenance(string type, string subType, string attrName) + { + _attrProvenance[(type, subType, attrName)] = CurrentProviderId ?? LibrarySentinel; + } + + /// The provider id that registered/extended on + /// (, ), or + /// if unstamped. #265 provenance-scoped strict prune. + public string AttrOrigin(string type, string subType, string attrName) => + _attrProvenance.TryGetValue((type, subType, attrName), out string? origin) ? origin : LibrarySentinel; + private void CheckNotSealed(string operation) { if (_sealed) @@ -251,6 +299,14 @@ public void Register(TypeDefinition def) _defs[key] = def; + // #265 — stamp provenance for every attr present at registration time (also + // captures a provider's own build-time enrichment, e.g. a provider building its + // own AttrSchema list before calling Register()). + foreach (AttrSchema attr in def.Attributes) + { + StampAttrProvenance(def.TypeId.Type, def.TypeId.SubType, attr.Name); + } + if (_subTypes.TryGetValue(def.TypeId.Type, out List? list)) { list.Add(def.TypeId.SubType); @@ -411,6 +467,8 @@ public void Extend(string type, string subType, IReadOnlyList? attri } def.AppendAttr(attr); + // #265 — stamp the extending provider as this attr's origin. + StampAttrProvenance(type, subType, attr.Name); } foreach (ChildRule rule in childRules ?? []) @@ -656,6 +714,18 @@ private void ApplyStrictStructuralChildren(MetaObjects.Registry.Spec.SpecMetamod /// manifest anyway and the loader needs them. Only the INCLUDED logical attrs are /// pruned, which TIGHTENS the loader — a misplaced attr → its unknown-attr error. /// Types NOT in the JSON keep their attrs untouched. Mirrors Java/Python's pass 5. + /// + /// #265 — the prune is PROVENANCE-scoped: only a LIBRARY-origin logical attr + /// (registered by one of the library's own providers, or unstamped — see + /// / ) is prunable against + /// the strict per-subtype allow-list. An attr a CONSUMER provider added via + /// (e.g. a downstream app widening a spec-declared core + /// subtype) is never pruned — the strict allow-list is a library-vocabulary + /// contract, blind to consumer-registered vocabulary by design. This does not + /// relax the check itself (still own-attrs-only, ADR-0039) and does not widen + /// scoping for a misplaced LIBRARY attr just because a consumer provider happens + /// to be composed in. + /// /// private void ApplyStrictAttrScoping(MetaObjects.Registry.Spec.SpecMetamodelReader reader) { @@ -674,10 +744,17 @@ private void ApplyStrictAttrScoping(MetaObjects.Registry.Spec.SpecMetamodelReade foreach (AttrSchema attr in def.Attributes) { bool prunable = - RegistryManifest.ClassifyPerTypeAttr(attr.Name) == RegistryManifest.ExclusionReason.Included; - if (prunable && !allow.Contains(attr.Name)) + RegistryManifest.ClassifyPerTypeAttr(attr.Name) == RegistryManifest.ExclusionReason.Included + && !allow.Contains(attr.Name); + if (prunable) { - continue; // logical attr not scoped to this subtype → prune + string origin = AttrOrigin(id.Type, id.SubType, attr.Name); + bool isLibraryOrigin = + origin == LibrarySentinel || CoreTypes.LibraryProviderIds.Contains(origin); + if (isLibraryOrigin) + { + continue; // library-origin logical attr not scoped to this subtype → prune + } } attrs.Add(attr); } diff --git a/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java b/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java index 32c8909ac..25433586a 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java +++ b/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java @@ -85,6 +85,38 @@ public class MetaDataRegistry { */ private final Map commonAttributes = new ConcurrentHashMap<>(); + /** + * #265 — attr-name provenance stamped during provider composition: + * {@code (type, subType) -> (attrName -> providerId)}. Consulted by + * {@link #applyStrictAttrScoping} so the strict prune only removes + * LIBRARY-origin attrs, sparing an attr a downstream provider added via + * {@link #extendType} (or {@link #register} directly). An attr name with no + * entry here (never stamped — e.g. an inherited copy, or a rebuild pass that + * only re-describes existing requirements) defaults to LIBRARY-origin, which + * preserves today's prune behavior. Keyed the same way as + * {@link #typeDefinitions} so provenance travels with its owning type. + */ + private final Map> attrProvenance = new ConcurrentHashMap<>(); + + /** + * #265 — sentinel provenance for an attr stamped OUTSIDE a provider's + * {@code registerTypes} turn (a direct {@link #register}/{@link #extendType} + * call made while {@link #currentProviderId} is {@code null} — e.g. a + * build-time constraint-expansion rebuild, or test code calling the registry + * API directly). Treated as LIBRARY-origin (prunable) by + * {@link #applyStrictAttrScoping}, same as an unstamped name. + */ + static final String LIBRARY_PROVENANCE = "_LIBRARY"; + + /** + * #265 — the provider id currently executing its {@code registerTypes} turn + * inside {@link #registerProviders}, or {@code null} outside that scope. Set + * around each provider's turn; read by {@link #register} and + * {@link #extendType} to stamp newly-added attr requirements with the + * provider that added them. + */ + private volatile String currentProviderId; + /** * Fully-global parent-key tier in {@link #globalRequirements} — matches any * (parentType, parentSubType). Used by {@link #registerCommonAttribute} so @@ -249,6 +281,12 @@ public synchronized void registerProviders(Collection prov checkNotSealed("registerProviders"); List ordered = resolveDependenciesStrict(providers); for (MetaDataTypeProvider provider : ordered) { + // #265 — stamp attr provenance with the provider currently registering + // (read by register()/extendType()); restored (not just cleared) so a + // provider whose registerTypes() re-enters registerProviders (unusual, + // but not contractually forbidden) leaves the outer turn's id intact. + String previousProviderId = currentProviderId; + currentProviderId = provider.getProviderId(); try { provider.registerTypes(this); if (!deferredInheritanceTypes.isEmpty()) { @@ -262,6 +300,8 @@ public synchronized void registerProviders(Collection prov com.metaobjects.ErrorCode.ERR_UNKNOWN); wrap.initCause(e); throw wrap; + } finally { + currentProviderId = previousProviderId; } } } @@ -328,6 +368,15 @@ public MetaDataRegistry extendType(Class metaDataClass, Cons // Update the registered type with extended definition TypeDefinition extendedDefinition = builder.build(); typeDefinitions.put(typeIdToExtend, extendedDefinition); + // #265 — extendType() rebuilds from `existing` and writes straight into + // typeDefinitions WITHOUT going through register(), so it must stamp + // provenance itself. Unlike register()'s blanket stamp, this DIFFS: only + // attr names newly present vs. `existing` get stamped with the current + // provider. Blanket-stamping here would mis-attribute every attr `existing` + // already carried (e.g. re-registering core's own attrs as this provider's) + // — and a same-name redefinition (the attr-conflict probe) must keep its + // PRIOR origin, not be silently reassigned to whoever redefined it. + stampNewAttrProvenance(typeIdToExtend, existing, extendedDefinition); log.debug("Extended type: {} with additional attributes/children", typeIdToExtend.toQualifiedName()); @@ -469,6 +518,13 @@ public void register(TypeDefinition definition) { resolveInheritance(definition); typeDefinitions.put(typeId, definition); + // #265 — stamp this definition's direct attr requirements with whichever + // provider is currently registering (or LIBRARY_PROVENANCE outside a + // provider's turn). Blanket, not diffed: register() always REPLACES the + // (type, subType)'s complete definition (unlike extendType(), which + // rebuilds an already-registered type from `existing` — see + // stampNewAttrProvenance for why that path diffs instead). + recordAttrProvenance(typeId, definition.getDirectChildRequirements()); log.debug("Registered type: {} -> {} (parent: {})", typeId.toQualifiedName(), definition.getImplementationClass().getSimpleName(), definition.hasParent() ? definition.getParentQualifiedName() : "none"); @@ -975,6 +1031,13 @@ public synchronized void applySpecDescriptions(com.metaobjects.registry.spec.Spe * the emitter and the loader still needs them. Pruning is applied to BOTH the * direct and the inherited attr requirements (the strict set is per-subtype, so a * subtype no longer keeps a broadly-inherited attr the JSON does not scope to it). + * + *

#265 — the prune is now provenance-scoped: a disallowed attr + * is dropped only when {@link #isLibraryOrigin} says it came from the library + * (unstamped, or stamped by one of {@link RegistryManifest#libraryProviderIds()}). + * An attr a downstream consumer provider added via {@link #extendType} survives — + * without this guard, strict scoping deleted every consumer extension on a + * spec-declared core subtype, blind to who registered it.

*/ private void applyStrictAttrScoping(com.metaobjects.registry.spec.SpecMetamodelReader reader) { for (Map.Entry entry : new ArrayList<>(typeDefinitions.entrySet())) { @@ -987,11 +1050,12 @@ private void applyStrictAttrScoping(com.metaobjects.registry.spec.SpecMetamodelR Set allow = reader.strictAttrNames(id.type(), id.subType()); - // Rebuild DIRECT requirements, dropping disallowed INCLUDED attrs. + // Rebuild DIRECT requirements, dropping disallowed INCLUDED library-origin attrs. Map directReqs = new LinkedHashMap<>(); for (ChildRequirement req : def.getDirectChildRequirements()) { - if (isPrunableAttr(req) && !allow.contains(req.getName())) { - continue; // logical attr not scoped to this subtype → prune + if (isPrunableAttr(req) && !allow.contains(req.getName()) + && isLibraryOrigin(id.type(), id.subType(), req.getName())) { + continue; // library-origin logical attr not scoped to this subtype → prune } directReqs.put(directKey(req), req); } @@ -1001,12 +1065,18 @@ private void applyStrictAttrScoping(com.metaobjects.registry.spec.SpecMetamodelR directReqs, def.getParentType(), def.getParentSubType(), def.getRules(), def.getExample(), def.getWhenToUse(), def.getParents()); - // Re-populate inherited requirements, dropping disallowed INCLUDED attrs. + // Re-populate inherited requirements, dropping disallowed INCLUDED library-origin + // attrs. Inherited copies are always unstamped at the CHILD (type, subType) key + // (provenance is recorded where an attr is directly declared, not on every + // descendant it is inherited onto) — so isLibraryOrigin defaults them to + // library-origin, and they keep pruning exactly as before. This is intended: + // they genuinely originate from a library base type either way. Map inherited = new LinkedHashMap<>(); for (Map.Entry e : def.getInheritedChildRequirements().entrySet()) { ChildRequirement req = e.getValue(); - if (isPrunableAttr(req) && !allow.contains(req.getName())) { - continue; // logical attr not scoped to this subtype → prune + if (isPrunableAttr(req) && !allow.contains(req.getName()) + && isLibraryOrigin(id.type(), id.subType(), req.getName())) { + continue; // library-origin logical attr not scoped to this subtype → prune } inherited.put(e.getKey(), req); } @@ -1032,15 +1102,98 @@ private void applyStrictAttrScoping(com.metaobjects.registry.spec.SpecMetamodelR * dup) — those are left registered. */ private static boolean isPrunableAttr(ChildRequirement req) { + if (!isNamedAttrRequirement(req)) { + return false; // structural placement rule, or the any-attr wildcard + } + return RegistryManifest.classifyPerTypeAttr(req.getName()) + == RegistryManifest.ExclusionReason.INCLUDED; + } + + /** + * #265 — true for a NAMED {@code attr}-typed requirement (concrete, non-wildcard + * name): the shape {@link #recordAttrProvenance}/{@link #stampNewAttrProvenance} + * track provenance for, and the shape {@link #isPrunableAttr} narrows further by + * spec-inclusion. False for structural placement rules and the any-attr wildcard. + */ + private static boolean isNamedAttrRequirement(ChildRequirement req) { if (!MetaAttribute.TYPE_ATTR.equals(req.getExpectedType())) { return false; // structural placement rule } String name = req.getName(); - if (name == null || "*".equals(name)) { - return false; // the any-attr wildcard + return name != null && !"*".equals(name); // false for the any-attr wildcard + } + + /** + * #265 — record provenance for every named attr requirement in {@code directRequirements} + * onto {@code typeId}, stamping {@link #currentProviderId} (or {@link #LIBRARY_PROVENANCE} + * outside a provider's turn). Called from {@link #register}, which always replaces the + * (type, subType)'s COMPLETE definition — so a blanket stamp is correct here (contrast + * {@link #stampNewAttrProvenance}, which diffs because {@link #extendType} rebuilds from + * an already-registered definition). + */ + private void recordAttrProvenance(MetaDataTypeId typeId, Collection directRequirements) { + String providerId = currentProviderId != null ? currentProviderId : LIBRARY_PROVENANCE; + for (ChildRequirement req : directRequirements) { + if (!isNamedAttrRequirement(req)) { + continue; + } + attrProvenance.computeIfAbsent(typeId, k -> new ConcurrentHashMap<>()) + .put(req.getName(), providerId); + } + } + + /** + * #265 — {@link #extendType}'s provenance hook. Diffs {@code extended}'s named attr + * requirements against {@code existing}'s and stamps {@link #currentProviderId} (or + * {@link #LIBRARY_PROVENANCE}) ONLY onto names that are genuinely NEW. extendType() + * rebuilds via {@code TypeDefinitionBuilder.from(existing)} and writes the result + * straight into {@code typeDefinitions} without going through {@link #register} — a + * blanket stamp here would mis-attribute every attr {@code existing} already carried + * to whichever provider called extendType() this time. A same-name redefinition (the + * attr-conflict-clash probe) is intentionally left alone — it keeps its PRIOR origin. + */ + private void stampNewAttrProvenance(MetaDataTypeId typeId, TypeDefinition existing, TypeDefinition extended) { + // Base set must be existing's FULL (direct + inherited) requirements, not just + // direct: TypeDefinitionBuilder.from(existing) seeds its flat builder map from + // existing.getChildRequirements() (direct + inherited), and build() writes that + // flat map into `extended`'s DIRECT requirements (inherited left empty). Diffing + // against direct-only would see every previously-INHERITED attr as "new" and + // stamp it with the extending provider's id — wrongly sparing a broadly-inherited + // library attr (e.g. object.base's @discriminator inherited onto object.projection, + // scoped by B2b to object.entity only) from strict scoping's prune. See the + // InheritedAttrProvenanceRegression test. + Set existingNames = new HashSet<>(); + for (ChildRequirement req : existing.getChildRequirements()) { + if (isNamedAttrRequirement(req)) { + existingNames.add(req.getName()); + } } - return RegistryManifest.classifyPerTypeAttr(name) - == RegistryManifest.ExclusionReason.INCLUDED; + String providerId = currentProviderId != null ? currentProviderId : LIBRARY_PROVENANCE; + for (ChildRequirement req : extended.getDirectChildRequirements()) { + if (!isNamedAttrRequirement(req) || existingNames.contains(req.getName())) { + continue; + } + attrProvenance.computeIfAbsent(typeId, k -> new ConcurrentHashMap<>()) + .put(req.getName(), providerId); + } + } + + /** + * #265 — true when {@code attrName} on {@code (type, subType)} counts as + * LIBRARY-origin for {@link #applyStrictAttrScoping}'s prune: never stamped + * (the default — an inherited copy, or a rebuild pass that only re-describes + * existing requirements), stamped {@link #LIBRARY_PROVENANCE}, or stamped with + * one of {@link RegistryManifest#libraryProviderIds()}. False only for an attr + * stamped with a provider id OUTSIDE the metamodel provider set — a downstream + * consumer extension. + */ + private boolean isLibraryOrigin(String type, String subType, String attrName) { + Map byName = attrProvenance.get(new MetaDataTypeId(type, subType)); + String providerId = byName != null ? byName.get(attrName) : null; + if (providerId == null || LIBRARY_PROVENANCE.equals(providerId)) { + return true; // unstamped, or the explicit build-time sentinel + } + return RegistryManifest.libraryProviderIds().contains(providerId); } /** diff --git a/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java b/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java index 7e4117426..7168cde9e 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java +++ b/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java @@ -24,14 +24,17 @@ import com.metaobjects.view.MetaView; import java.util.ArrayList; +import java.util.Collection; import java.util.Comparator; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; +import java.util.stream.Collectors; /** * SP-G Registry Conformance — the Java registry-manifest emitter. @@ -112,7 +115,31 @@ private RegistryManifest() { * @return a new registry composed from the metamodel provider set */ public static MetaDataRegistry composeMetamodelRegistry() { - MetaDataRegistry registry = MetaDataRegistry.compose(metamodelProviders()); + return composeMetamodelRegistry(List.of()); + } + + /** + * #265 — compose the metamodel provider set PLUS the given {@code extra} + * providers (appended after the library set), returning an + * unsealed registry run through the SAME strict + * spec-description + attr-scoping pipeline {@link #composeMetamodelRegistry()} + * runs. This is the {@code MetaDataLoader.setTypeRegistry(...)} seam — the + * sanctioned path for a downstream app that genuinely needs additional + * vocabulary while its metadata still strict-loads against the full spec + * contract (see {@code docs/superpowers/specs/2026-08-02-issue-265-strict-scoping-provenance-design.md}). + * Composing zero extras ({@link #composeMetamodelRegistry()}) is + * byte-identical to today — {@code MetaDataRegistry.compose(...)} itself is + * unchanged; this only routes the caller's extra providers through the same + * post-compose pipeline. + * + * @param extra additional providers composed after the metamodel provider set + * @return a new, unsealed registry (the caller may seal it) + */ + public static MetaDataRegistry composeMetamodelRegistry(Collection extra) { + Objects.requireNonNull(extra, "extra must not be null"); + List providers = new ArrayList<>(metamodelProviders()); + providers.addAll(extra); + MetaDataRegistry registry = MetaDataRegistry.compose(providers); // Force the lazy core-constraint init NOW: it expands the named inherited // attr child-requirements (e.g. field.base's `required`/`default`/`unique` // onto each concrete field subtype). Those named requirements must exist @@ -128,6 +155,30 @@ public static MetaDataRegistry composeMetamodelRegistry() { return registry; } + /** + * #265 — memoized library provider ids: every {@link MetaDataTypeProvider#getProviderId()} + * in {@link #metamodelProviders()}. Consulted by + * {@link MetaDataRegistry#applyStrictAttrScoping} to decide whether an attr's + * stamped provenance is LIBRARY-origin (prunable by strict scoping) or + * consumer-origin (spared). Deliberately homed in this composition layer, not + * the loader layer — {@code Loader -> Registry/RegistryManifest} is the + * existing dependency direction and must not invert. + * + * @return the immutable set of library provider ids + */ + static Set libraryProviderIds() { + Set ids = libraryProviderIds; + if (ids == null) { + ids = metamodelProviders().stream() + .map(MetaDataTypeProvider::getProviderId) + .collect(Collectors.toUnmodifiableSet()); + libraryProviderIds = ids; + } + return ids; + } + + private static volatile Set libraryProviderIds; + /** * The process-wide, lazily-built, sealed default registry the * library loader uses (ADR-0023 Decision 2 — the JVM load-time pivot). diff --git a/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java b/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java index 7b22e014d..f5d261949 100644 --- a/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java +++ b/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java @@ -19,12 +19,20 @@ import com.google.gson.JsonParser; import com.metaobjects.ErrorCode; import com.metaobjects.MetaDataException; +import com.metaobjects.attr.IntAttribute; import com.metaobjects.attr.StringAttribute; +import com.metaobjects.loader.InMemoryStringSource; +import com.metaobjects.loader.LoaderOptions; +import com.metaobjects.loader.MetaDataLoader; +import com.metaobjects.loader.MetaDataSource; import com.metaobjects.registry.MetaDataRegistry; import com.metaobjects.registry.MetaDataTypeProvider; +import com.metaobjects.registry.RegistryManifest; import com.metaobjects.template.MetaTemplate; import com.metaobjects.template.TemplateConstants; +import com.metaobjects.view.CurrencyView; import org.junit.Test; +import org.junit.experimental.runners.Enclosed; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameter; @@ -37,11 +45,14 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; /** @@ -59,8 +70,21 @@ * objects, composes, and asserts the surfaced code. The registry-sealed * scenario composes, seals, then runs a probe provider's {@code registerTypes} * against the sealed registry.

+ * + *

#265 adds a SECOND, differently-shaped corpus at {@code compose-load/} + * (see the corpus README "The {@code compose-load/} subdir") that composes the + * library's real core provider set with a consumer provider and, for some + * scenarios, strict-loads an actual metadata document. JUnit4's + * {@link Parameterized} runner supports exactly one {@code @Parameters} source + * per class, so the two corpora are split into sibling {@code public static} + * classes under this class's {@link Enclosed} runner — mirroring the + * TS/Python/C# runners' two independent test loops in one file. {@link Enclosed} + * sweeps every PUBLIC nested class into its suite (see {@code getClasses()}), + * so the probe/provider helper classes below are deliberately package-private, + * not {@code public} — only {@link FlatCorpus} and {@link ComposeLoad} (both + * genuinely {@code @RunWith(Parameterized.class)}) are public.

*/ -@RunWith(Parameterized.class) +@RunWith(Enclosed.class) public class ProviderCompositionConformanceTest { private static final String CONFLICT_SUBTYPE = "compositionprobe"; @@ -70,14 +94,18 @@ public class ProviderCompositionConformanceTest { // Probe MetaData classes for the attr-conflict / seal scenarios. // attr-conflict-base + attr-conflict-clash MUST share one implementation // class — extendType() looks the registered type up by class. + // + // Package-private (NOT public): Enclosed sweeps every PUBLIC member class + // of the outer class into its suite via Class#getClasses(), which would + // try (and fail) to run these as standalone JUnit test classes. // ------------------------------------------------------------------ - public static final class CompositionProbeTemplate extends MetaTemplate { - public CompositionProbeTemplate(String name) { super(CONFLICT_SUBTYPE, name); } + static final class CompositionProbeTemplate extends MetaTemplate { + CompositionProbeTemplate(String name) { super(CONFLICT_SUBTYPE, name); } } - public static final class SealProbeTemplate extends MetaTemplate { - public SealProbeTemplate(String name) { super("sealprobe", name); } + static final class SealProbeTemplate extends MetaTemplate { + SealProbeTemplate(String name) { super("sealprobe", name); } } // ------------------------------------------------------------------ @@ -127,6 +155,24 @@ public static final class SealProbeTemplate extends MetaTemplate { @Override public String getDescription() { return "Test-only seal probe provider."; } }; + /** + * #265 {@code compose-load/} canonical named provider. Extends + * {@code view.currency} (a SPEC-DECLARED CORE subtype the library's own + * core-types provider registers) with a new {@code decimals} int attr. + * Deliberately NO dependencies — see the corpus README "Canonical named + * provider {@code extend-spec-subtype}" for why (cross-port id/dep parity + * vs. the {@code composeWithCore} ordering contract). + */ + private static final MetaDataTypeProvider EXTEND_SPEC_SUBTYPE = new MetaDataTypeProvider() { + @Override public String getProviderId() { return "extend-spec-subtype"; } + @Override public String[] getDependencies() { return new String[0]; } + @Override public void registerTypes(MetaDataRegistry registry) { + registry.extendType(CurrencyView.class, def -> + def.optionalAttribute("decimals", IntAttribute.SUBTYPE_INT)); + } + @Override public String getDescription() { return "Test-only — #265 compose-load probe: extends view.currency with @decimals."; } + }; + private static MetaDataTypeProvider resolve(String id) { MetaDataTypeProvider p = ConformanceTestProviders.TEST_PROVIDERS.get(id); if (p != null) return p; @@ -134,6 +180,7 @@ private static MetaDataTypeProvider resolve(String id) { case "attr-conflict-base": return ATTR_CONFLICT_BASE; case "attr-conflict-clash": return ATTR_CONFLICT_CLASH; case "seal-probe": return SEAL_PROBE; + case "extend-spec-subtype": return EXTEND_SPEC_SUBTYPE; default: throw new IllegalArgumentException( "Unknown named provider \"" + id + "\" in provider-composition corpus"); @@ -161,61 +208,255 @@ private static Path corpusRoot() { + Paths.get("").toAbsolutePath()); } - @Parameters(name = "{0}") - public static Collection manifests() { - try (Stream files = Files.list(corpusRoot())) { - List names = files - .filter(p -> p.getFileName().toString().endsWith(".json")) - .map(p -> p.getFileName().toString()) - .sorted() - .collect(Collectors.toList()); - if (names.isEmpty()) { - throw new AssertionError("provider-composition corpus is empty (mis-pathed root?)"); + private static String codeOf(MetaDataException ex) { + return ex.getCode().map(Enum::name).orElse(ErrorCode.ERR_UNKNOWN.name()); + } + + // ------------------------------------------------------------------ + // Flat corpus — error-code manifests ({description, providers[], + // expectedError, sealThenRegister?}). Shape UNCHANGED. + // ------------------------------------------------------------------ + + @RunWith(Parameterized.class) + public static class FlatCorpus { + + @Parameters(name = "{0}") + public static Collection manifests() { + try (Stream files = Files.list(corpusRoot())) { + List names = files + .filter(p -> p.getFileName().toString().endsWith(".json")) + .map(p -> p.getFileName().toString()) + .sorted() + .collect(Collectors.toList()); + if (names.isEmpty()) { + throw new AssertionError("provider-composition corpus is empty (mis-pathed root?)"); + } + List rows = new ArrayList<>(names.size()); + for (String n : names) rows.add(new Object[]{n}); + return rows; + } catch (IOException e) { + throw new UncheckedIOException(e); } - List rows = new ArrayList<>(names.size()); - for (String n : names) rows.add(new Object[]{n}); - return rows; - } catch (IOException e) { - throw new UncheckedIOException(e); } - } - @Parameter(0) - public String fileName; + @Parameter(0) + public String fileName; - @Test - public void providerComposition() throws IOException { - JsonObject manifest = JsonParser.parseString( - Files.readString(corpusRoot().resolve(fileName))).getAsJsonObject(); - String expected = manifest.get("expectedError").getAsString(); + @Test + public void providerComposition() throws IOException { + JsonObject manifest = JsonParser.parseString( + Files.readString(corpusRoot().resolve(fileName))).getAsJsonObject(); + String expected = manifest.get("expectedError").getAsString(); - List resolved = new ArrayList<>(); - manifest.getAsJsonArray("providers").forEach(e -> resolved.add(resolve(e.getAsString()))); + List resolved = new ArrayList<>(); + manifest.getAsJsonArray("providers").forEach(e -> resolved.add(resolve(e.getAsString()))); - if (manifest.has("sealThenRegister")) { - // Compose (must succeed), seal, then run the probe against the sealed registry. - MetaDataRegistry registry = MetaDataRegistry.compose(resolved); - registry.seal(); - MetaDataTypeProvider probe = resolve(manifest.get("sealThenRegister").getAsString()); + if (manifest.has("sealThenRegister")) { + // Compose (must succeed), seal, then run the probe against the sealed registry. + MetaDataRegistry registry = MetaDataRegistry.compose(resolved); + registry.seal(); + MetaDataTypeProvider probe = resolve(manifest.get("sealThenRegister").getAsString()); + try { + probe.registerTypes(registry); + fail("expected " + expected + " but no exception was thrown"); + } catch (MetaDataException ex) { + assertEquals(expected, codeOf(ex)); + } + return; + } + + // Ordinary scenario: compose itself throws. try { - probe.registerTypes(registry); + MetaDataRegistry.compose(resolved); fail("expected " + expected + " but no exception was thrown"); } catch (MetaDataException ex) { assertEquals(expected, codeOf(ex)); } - return; } + } + + // ------------------------------------------------------------------ + // #265 compose-load corpus — see the corpus README "The `compose-load/` + // subdir". Own directory, own nested runner: a manifest here never carries + // `expectedError` / `sealThenRegister` (the flat-corpus shape above); it + // carries `composeWithCore` / `expectAttrs` / `metadata` / `expectErrors` + // instead. + // ------------------------------------------------------------------ + + @RunWith(Parameterized.class) + public static class ComposeLoad { + + private static Path composeLoadCorpusRoot() { + return corpusRoot().resolve("compose-load"); + } + + @Parameters(name = "{0}") + public static Collection manifests() { + try (Stream files = Files.list(composeLoadCorpusRoot())) { + List names = files + .filter(p -> p.getFileName().toString().endsWith(".json")) + .map(p -> p.getFileName().toString()) + .sorted() + .collect(Collectors.toList()); + if (names.isEmpty()) { + throw new AssertionError("provider-composition compose-load corpus is empty (mis-pathed root?)"); + } + List rows = new ArrayList<>(names.size()); + for (String n : names) rows.add(new Object[]{n}); + return rows; + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @Parameter(0) + public String fileName; + + @Test + public void providerCompositionComposeLoad() throws IOException { + JsonObject manifest = JsonParser.parseString( + Files.readString(composeLoadCorpusRoot().resolve(fileName))).getAsJsonObject(); + + List named = new ArrayList<>(); + manifest.getAsJsonArray("providers").forEach(e -> named.add(resolve(e.getAsString()))); + boolean composeWithCore = manifest.has("composeWithCore") + && manifest.get("composeWithCore").getAsBoolean(); + + // composeWithCore routes through RegistryManifest.composeMetamodelRegistry — + // the MetaDataLoader.setTypeRegistry(...) seam adopters use — NOT the raw + // MetaDataRegistry.compose(...) the flat corpus above exercises. This is the + // path that must apply strict spec-description scoping to the consumer's + // extension (#265). + MetaDataRegistry registry = composeWithCore + ? RegistryManifest.composeMetamodelRegistry(named) + : MetaDataRegistry.compose(named); + + if (manifest.has("expectAttrs")) { + JsonObject expectAttrs = manifest.getAsJsonObject("expectAttrs"); + String type = expectAttrs.get("type").getAsString(); + String subType = expectAttrs.get("subType").getAsString(); + expectAttrs.getAsJsonArray("contains").forEach(nameEl -> { + String name = nameEl.getAsString(); + assertNotNull( + "expected (" + type + ", " + subType + ") to declare attr \"" + name + "\"", + registry.getChildRequirement(type, subType, name)); + }); + } + + if (manifest.has("metadata")) { + String content = manifest.get("metadata").toString(); + // sanitizeRootName normalizes hyphens but not the ".json" suffix — strip + // it ourselves so the loader name satisfies the identifier pattern. + String loaderName = "composeLoad" + fileName.replaceFirst("\\.json$", ""); + MetaDataLoader loader = new MetaDataLoader( + LoaderOptions.create(false, false, true), + MetaDataLoader.SUBTYPE_MANUAL, loaderName); + loader.setTypeRegistry(registry); + loader.init(); + + MetaDataException thrown = null; + try { + loader.load(List.of(new InMemoryStringSource( + content, "", MetaDataSource.MetaDataFormat.JSON))); + } catch (MetaDataException ex) { + thrown = ex; + } + List recordedErrors = loader.getErrors(); + List actualCodes = new ArrayList<>(); + for (MetaDataException recorded : recordedErrors) { + actualCodes.add(codeOf(recorded)); + } + // #265/#267 hardening: MetaDataException carries no equals()/hashCode() + // override, so this List#contains is a reference-identity check — it + // skips `thrown` only when it is the EXACT SAME exception instance a + // validation phase already recorded via addError() (then eager-threw), + // preventing a double count of one underlying failure. Two genuinely + // distinct exceptions sharing the same .code (a future multi-error + // fixture) are still both counted, matching the TS reference's sorted + // full error-code list comparison. + if (thrown != null && !recordedErrors.contains(thrown)) { + actualCodes.add(codeOf(thrown)); + } - // Ordinary scenario: compose itself throws. - try { - MetaDataRegistry.compose(resolved); - fail("expected " + expected + " but no exception was thrown"); - } catch (MetaDataException ex) { - assertEquals(expected, codeOf(ex)); + List expectedCodes = new ArrayList<>(); + if (manifest.has("expectErrors")) { + manifest.getAsJsonArray("expectErrors").forEach(e -> expectedCodes.add(e.getAsString())); + } + Collections.sort(actualCodes); + Collections.sort(expectedCodes); + assertEquals(expectedCodes, actualCodes); + } } } - private static String codeOf(MetaDataException ex) { - return ex.getCode().map(Enum::name).orElse(ErrorCode.ERR_UNKNOWN.name()); + // ------------------------------------------------------------------ + // #265 regression — extendType()'s diff-stamp base must be existing's FULL + // (direct + inherited) requirement set, not direct-only. Java-local (not a + // shared cross-port fixture): the direct/inherited requirement split this + // bug lives in is a Java-registry-specific concept — Python/C#/TS registries + // are flat, with no separate "inherited" tier to flatten. + // + // Not corpus-driven (a single fixed scenario), so a plain JUnit4 class with + // one @Test — Enclosed's default RunnerBuilder picks BlockJUnit4ClassRunner + // for it automatically (no @RunWith needed). + // ------------------------------------------------------------------ + + public static class InheritedAttrProvenanceRegression { + + @Test + public void extendingASubtypeDoesNotSpareItsInheritedOutOfScopeAttrs() { + // extendType() rebuilds via TypeDefinitionBuilder.from(existing), which + // seeds its flat builder map from existing.getChildRequirements() (direct + // + inherited) — build() then writes that flat map into the REBUILT + // definition's DIRECT requirements (inherited left empty). A provider + // extending object.projection for a reason entirely UNRELATED to + // @discriminator must not cause object.base's broadly-inherited + // @discriminator (B2b scopes discriminator*/discriminatorValue to + // object.entity ONLY — an FR-014 single-table-inheritance marker + // meaningless on a derived, read-only projection) to be mis-stamped as + // this provider's own and spared from strict scoping's prune. + // + // object.projection (unlike field.* / view.currency / object.value / + // identity.secondary / index.lookup) is untouched by every library + // register()/extendType()/applyProviderExtends pass — grep spec/metamodel/ + // {ui,prompt,db}.json's "extends" targets plus CoreDBMetaDataProvider's two + // explicit extendType() calls: none touch object.projection. So, unlike + // those, its inherited attrs are STILL genuinely inherited (not already + // flattened + library-stamped by an earlier pass) at the moment this + // provider's own extendType() runs — which is what actually exercises the + // diff-base bug rather than incidentally masking it. + MetaDataTypeProvider extendObjectProjection = new MetaDataTypeProvider() { + @Override public String getProviderId() { return "extend-object-projection-unrelated"; } + @Override public String[] getDependencies() { return new String[0]; } + @Override public void registerTypes(MetaDataRegistry registry) { + registry.extendType(com.metaobjects.object.ProjectionMetaObject.class, def -> + def.optionalAttribute("unrelatedProbeAttr", StringAttribute.SUBTYPE_STRING)); + } + @Override public String getDescription() { + return "Test-only — #265 regression probe: extends object.projection with an " + + "attr unrelated to @discriminator, to isolate the flatten-on-extend bug."; + } + }; + + MetaDataRegistry registry = RegistryManifest.composeMetamodelRegistry(List.of(extendObjectProjection)); + + // @discriminator is scoped to object.entity only (B2b) — object.projection + // merely INHERITS it from object.base, and that inheritance must still be + // pruned from the registry's declared-attr lookup even though an unrelated + // provider extendType()'d object.projection. Pre-fix, the flatten-on-extend + // bug mis-stamped @discriminator as the extending provider's own, so it + // survived the prune (getChildRequirement returned non-null). + assertNull( + "expected (object, projection) to NOT declare @discriminator (B2b scopes it " + + "to object.entity only; object.projection only INHERITS it from object.base " + + "and that inheritance must still be pruned after an unrelated extendType())", + registry.getChildRequirement("object", "projection", "discriminator")); + + // Sanity check: the unrelated attr this provider actually added is present — + // proves the extendType() call itself succeeded and wasn't a no-op. + assertNotNull( + "expected (object, projection) to declare the provider's own unrelatedProbeAttr", + registry.getChildRequirement("object", "projection", "unrelatedProbeAttr")); + } } } diff --git a/server/python/src/metaobjects/core_types.py b/server/python/src/metaobjects/core_types.py index cb6b25939..a45422626 100644 --- a/server/python/src/metaobjects/core_types.py +++ b/server/python/src/metaobjects/core_types.py @@ -874,3 +874,9 @@ def _register_subtypes( prompt_provider, ui_provider, ] + +# #265 — the ids of the LIBRARY providers above, as a frozen set. Consulted by +# spec_metamodel._apply_strict_attr_scoping (via TypeRegistry.attr_origin) to +# decide whether an attr's provenance is "library" (still prunable against the +# strict per-subtype allow-list) or a consumer's own provider (never pruned). +LIBRARY_PROVIDER_IDS: frozenset[str] = frozenset(p.id for p in core_providers) diff --git a/server/python/src/metaobjects/provider.py b/server/python/src/metaobjects/provider.py index 0512c0a25..a41a27311 100644 --- a/server/python/src/metaobjects/provider.py +++ b/server/python/src/metaobjects/provider.py @@ -77,7 +77,17 @@ def compose_registry(providers: list[Provider]) -> TypeRegistry: ordered = _topo_sort(providers) registry = TypeRegistry() for provider in ordered: - provider.register_types(registry) + # #265 — stamp the active provider id for the duration of this + # provider's register_types() call (covers both its own `.add()`-ed + # definitions AND any `registry.extend()` it triggers, e.g. via an + # `on_register` hook), so registry.py can attribute every attr it sees + # to the provider that registered it. Cleared after so the sentinel + # (LIBRARY_ATTR_ORIGIN) is the default outside a compose loop. + registry._current_provider_id = provider.id # noqa: SLF001 + try: + provider.register_types(registry) + finally: + registry._current_provider_id = None # noqa: SLF001 apply_spec_descriptions(registry) return registry diff --git a/server/python/src/metaobjects/registry.py b/server/python/src/metaobjects/registry.py index 37ecc0b32..542b4d1f6 100644 --- a/server/python/src/metaobjects/registry.py +++ b/server/python/src/metaobjects/registry.py @@ -8,6 +8,15 @@ from .shared.base_types import SUBTYPE_BASE from .validation_types import NodeValidator, ReferenceDescriptor +# #265 — sentinel attr-provenance origin for an attr registered/extended with no +# active provider id (an unstamped registration — e.g. a hand-built registry in a +# unit test that never goes through provider.compose_registry's stamping loop, or +# any future registration path that forgets to set `_current_provider_id`). +# Treated as LIBRARY origin by the strict-attr-scoping prune guard +# (spec_metamodel._apply_strict_attr_scoping) — i.e. still prunable, preserving +# pre-#265 behavior for anything not explicitly attributed to a provider. +LIBRARY_ATTR_ORIGIN = "__library__" + @dataclass(frozen=True) class AttrSchema: @@ -106,6 +115,16 @@ def __init__(self) -> None: # pivot off). The library seals after the metamodel bootstrap; a # downstream app composes its own (unsealed) registry. self._sealed = False + # #265 — the provider id active during the CURRENT register_types() call, + # set/cleared by provider.compose_registry around each provider's turn. + # None outside a compose loop (e.g. a hand-built registry in a unit test). + self._current_provider_id: str | None = None + # #265 — (type, subType, attrName) -> the provider id that registered + # that attr (LIBRARY_ATTR_ORIGIN when unstamped). Consulted by + # spec_metamodel._apply_strict_attr_scoping so a consumer-registered attr + # on a spec-declared subtype is never pruned by the library's strict + # per-subtype allow-list — only LIBRARY-origin attrs are. + self._attr_provenance: dict[tuple[str, str, str], str] = {} def seal(self) -> None: """Seal the registry: every subsequent mutating registration raises @@ -155,6 +174,18 @@ def register(self, definition: TypeDefinition) -> None: references=list(definition.references), validate=definition.validate, ) + # #265 — stamp provenance for every attr present at registration time + # (this also captures a provider's own build-time enrichment, e.g. + # core_types.py's post-hoc `_def.attrs.append(...)` on its own + # TypeDefinition objects before `register()` is ever called). + origin = self._current_provider_id or LIBRARY_ATTR_ORIGIN + for attr in self._defs[definition.key].attrs: + self._attr_provenance[(definition.type, definition.sub_type, attr.name)] = origin + + def attr_origin(self, type_: str, sub_type: str, attr_name: str) -> str: + """The provider id that registered/extended ``attr_name`` on ``(type_, sub_type)``, + or :data:`LIBRARY_ATTR_ORIGIN` if unstamped. #265 provenance-scoped strict prune.""" + return self._attr_provenance.get((type_, sub_type, attr_name), LIBRARY_ATTR_ORIGIN) def find(self, type_: str, sub_type: str) -> TypeDefinition | None: return self._defs.get((type_, sub_type)) @@ -244,6 +275,7 @@ def extend( ErrorCode.ERR_UNKNOWN_SUBTYPE, ) + origin = self._current_provider_id or LIBRARY_ATTR_ORIGIN for attr in attributes or []: if attr.value_type == SUBTYPE_BASE: raise ValueError( @@ -258,6 +290,8 @@ def extend( ErrorCode.ERR_PROVIDER_ATTR_CONFLICT, ) definition.attrs.append(attr) + # #265 — stamp the extending provider as this attr's origin. + self._attr_provenance[(type_, sub_type, attr.name)] = origin for rule in child_rules or []: definition.child_rules.append(rule) diff --git a/server/python/src/metaobjects/spec_metamodel/__init__.py b/server/python/src/metaobjects/spec_metamodel/__init__.py index 8c0f56769..5bbb62319 100644 --- a/server/python/src/metaobjects/spec_metamodel/__init__.py +++ b/server/python/src/metaobjects/spec_metamodel/__init__.py @@ -580,8 +580,21 @@ def _apply_strict_attr_scoping(registry, reader: SpecMetamodelReader) -> None: loader (a misplaced attr → ``ERR_UNKNOWN_ATTR``). Types NOT declared in the JSON keep their attrs untouched (no JSON-sourced strict set exists). + #265 — the prune is PROVENANCE-scoped: only a LIBRARY-origin logical attr + (registered by one of the library's own providers, or unstamped — see + ``TypeRegistry.attr_origin`` / ``LIBRARY_ATTR_ORIGIN``) is prunable against + the strict per-subtype allow-list. An attr a CONSUMER provider added via + ``registry.extend()`` (e.g. a downstream app widening a spec-declared core + subtype) is never pruned — the strict allow-list is a library-vocabulary + contract, blind to consumer-registered vocabulary by design. This does not + relax the check itself (still own-attrs-only, ADR-0039) and does not widen + scoping for a misplaced LIBRARY attr just because a consumer provider + happens to be composed in. + Deferred import avoids the provider import cycle. """ + from ..core_types import LIBRARY_PROVIDER_IDS + from ..registry import LIBRARY_ATTR_ORIGIN from ..registry_manifest import ExclusionReason, classify_per_type_attr for definition in registry._defs.values(): # noqa: SLF001 @@ -591,7 +604,11 @@ def _apply_strict_attr_scoping(registry, reader: SpecMetamodelReader) -> None: kept = [] for attr in definition.attrs: is_logical = classify_per_type_attr(attr.name) is ExclusionReason.INCLUDED - if is_logical and attr.name not in allow: - continue # logical attr not scoped to this subtype → prune + prunable = is_logical and attr.name not in allow + if prunable: + origin = registry.attr_origin(definition.type, definition.sub_type, attr.name) + is_library_origin = origin == LIBRARY_ATTR_ORIGIN or origin in LIBRARY_PROVIDER_IDS + if is_library_origin: + continue # library-origin logical attr not scoped to this subtype → prune kept.append(attr) definition.attrs[:] = kept diff --git a/server/python/tests/conformance/test_provider_composition_conformance.py b/server/python/tests/conformance/test_provider_composition_conformance.py index 51e928291..71a3d6e78 100644 --- a/server/python/tests/conformance/test_provider_composition_conformance.py +++ b/server/python/tests/conformance/test_provider_composition_conformance.py @@ -18,11 +18,15 @@ import pytest +from metaobjects.core_types import core_providers from metaobjects.errors import ErrorCode, ParseError +from metaobjects.loader.meta_data_loader import MetaDataLoader +from metaobjects.meta.core.attr.attr_constants import ATTR_SUBTYPE_INT +from metaobjects.meta.presentation.view.view_constants import VIEW_SUBTYPE_CURRENCY from metaobjects.provider import Provider, compose_registry from metaobjects.registry import AttrSchema, ChildRule, TypeDefinition, TypeRegistry from metaobjects.meta.template.meta_template import MetaTemplate -from metaobjects.shared.base_types import TYPE_ATTR +from metaobjects.shared.base_types import TYPE_ATTR, TYPE_VIEW # Fresh, otherwise-unused template subtype the attr-conflict / seal providers use. _CONFLICT_SUBTYPE = "compositionprobe" @@ -89,6 +93,32 @@ def register_types(self, registry: TypeRegistry) -> None: # noqa: D401 ) +class _ExtendSpecSubtypeProvider(Provider): + """#265 `compose-load/` canonical named provider. Extends `view.currency` (a + SPEC-DECLARED CORE subtype the library's own core-types provider registers) + with a new `decimals` int attr. Deliberately NO dependencies — see the corpus + README "Canonical named provider `extend-spec-subtype`" for why (cross-port + id/dep parity vs. the `composeWithCore` ordering contract). + """ + + def __init__(self) -> None: + super().__init__("extend-spec-subtype") + + def register_types(self, registry: TypeRegistry) -> None: # noqa: D401 + registry.extend( + TYPE_VIEW, + VIEW_SUBTYPE_CURRENCY, + attributes=[ + AttrSchema( + "decimals", + ATTR_SUBTYPE_INT, + required=False, + description="Test-only — #265 compose-load probe attr.", + ) + ], + ) + + def _providers() -> dict[str, Provider]: return { "duplicate-x": _noop_provider("duplicate-x"), @@ -100,6 +130,7 @@ def _providers() -> dict[str, Provider]: "attr-conflict-base": _attr_conflict_base_provider(), "attr-conflict-clash": _AttrConflictClashProvider(), "seal-probe": _SealProbeProvider(), + "extend-spec-subtype": _ExtendSpecSubtypeProvider(), } @@ -114,7 +145,18 @@ def _manifest_files() -> list[Path]: return sorted(_corpus_root().glob("*.json")) -def _error_code(exc: BaseException) -> str: +def _compose_load_corpus_root() -> Path: + return _corpus_root() / "compose-load" + + +def _compose_load_manifest_files() -> list[Path]: + return sorted(_compose_load_corpus_root().glob("*.json")) + + +def _error_code(exc: object) -> str: + # Accepts both a caught exception (ParseError) and a MetaError — both carry + # a `.code: ErrorCode` attribute; only the container differs (raised vs. + # collected in `LoadResult.errors`). code = getattr(exc, "code", None) if isinstance(code, ErrorCode): return code.value @@ -148,3 +190,38 @@ def test_provider_composition(manifest_path: Path) -> None: with pytest.raises(ParseError) as exc_info: compose_registry(resolved) assert _error_code(exc_info.value) == expected + + +# --------------------------------------------------------------------------- +# #265 `compose-load/` corpus — see the corpus README "The `compose-load/` +# subdir". Own directory, own loop: a manifest here never carries +# `expectedError` / `sealThenRegister` (the flat-corpus shape above); it +# carries `composeWithCore` / `expectAttrs` / `metadata` / `expectErrors` +# instead. +# --------------------------------------------------------------------------- + + +def test_compose_load_corpus_non_empty() -> None: + assert len(_compose_load_manifest_files()) > 0 + + +@pytest.mark.parametrize("manifest_path", _compose_load_manifest_files(), ids=lambda p: p.name) +def test_provider_composition_compose_load(manifest_path: Path) -> None: + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + providers = _providers() + resolved = [_resolve(providers, pid) for pid in manifest["providers"]] + provider_list = [*core_providers, *resolved] if manifest.get("composeWithCore") else resolved + + if "expectAttrs" in manifest: + registry = compose_registry(provider_list) + expect_attrs = manifest["expectAttrs"] + declared_names = [a.name for a in registry.attrs_of(expect_attrs["type"], expect_attrs["subType"])] + for name in expect_attrs["contains"]: + assert name in declared_names + + if "metadata" in manifest: + content = json.dumps(manifest["metadata"]) + result = MetaDataLoader.from_string(content, providers=provider_list, strict=True) + actual_codes = sorted(_error_code(e) for e in result.errors) + expected_codes = sorted(manifest.get("expectErrors", [])) + assert actual_codes == expected_codes diff --git a/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts b/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts index e771fd3d2..9ed103414 100644 --- a/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts +++ b/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts @@ -19,9 +19,12 @@ import type { MetaDataTypeProvider } from "../src/provider.js"; import { composeRegistry } from "../src/provider.js"; import { TypeRegistry, TypeId } from "../src/registry.js"; import { MetaTemplate } from "../src/template/meta-template.js"; -import { TYPE_TEMPLATE, TYPE_ATTR } from "../src/shared/base-types.js"; +import { TYPE_TEMPLATE, TYPE_ATTR, TYPE_VIEW } from "../src/shared/base-types.js"; import { CHILD_RULE_WILDCARD } from "../src/shared/structural.js"; -import { ATTR_SUBTYPE_STRING } from "../src/core/attr/attr-constants.js"; +import { ATTR_SUBTYPE_STRING, ATTR_SUBTYPE_INT } from "../src/core/attr/attr-constants.js"; +import { VIEW_SUBTYPE_CURRENCY } from "../src/presentation/view/view-constants.js"; +import { coreProviders } from "../src/core-types.js"; +import { MetaDataLoader } from "../src/loader/meta-data-loader.js"; // The corpus lives at the REPO ROOT — five `../` levels up from test/ // (test → metadata → packages → typescript → server → repo-root). @@ -30,6 +33,13 @@ const CORPUS = join( "../../../../../fixtures/provider-composition-conformance", ); +// #265 — a SECOND, subdir corpus that composes the library's real core provider +// set with a consumer provider and (for two scenarios) strict-loads an actual +// metadata document. Lives in its own subdir (not the flat CORPUS dir) so +// un-updated runners in other ports — which list CORPUS non-recursively and +// hard-require the old manifest shape — are unaffected. See README.md. +const COMPOSE_LOAD_CORPUS = join(CORPUS, "compose-load"); + // --------------------------------------------------------------------------- // Canonical named-provider set (see the corpus README). Test-only — these live // in conformance test code, never in shipped metamodel providers. Every port @@ -119,6 +129,24 @@ const sealProbeProvider: MetaDataTypeProvider = { }, }; +/** + * #265 — `compose-load/` canonical named provider. Extends `view.currency` (a + * SPEC-DECLARED CORE subtype the library's own core-types provider registers) + * with a new `decimals` int attr. Deliberately NO dependencies — see README.md + * "Canonical named provider `extend-spec-subtype`" for why (cross-port id/dep + * parity vs. `composeWithCore` ordering). + */ +const extendSpecSubtypeProvider: MetaDataTypeProvider = { + id: "extend-spec-subtype", + registerTypes(registry: TypeRegistry) { + registry.extend(TYPE_VIEW, VIEW_SUBTYPE_CURRENCY, { + attributes: [ + { name: "decimals", valueType: ATTR_SUBTYPE_INT, required: false, description: "Test-only — #265 compose-load probe attr." }, + ], + }); + }, +}; + const PROVIDERS: Readonly> = { "duplicate-x": duplicateXProvider, "duplicate-x-clone": duplicateXCloneProvider, @@ -128,13 +156,26 @@ const PROVIDERS: Readonly> = { "attr-conflict-base": attrConflictBaseProvider, "attr-conflict-clash": attrConflictClashProvider, "seal-probe": sealProbeProvider, + "extend-spec-subtype": extendSpecSubtypeProvider, }; +interface ExpectAttrs { + type: string; + subType: string; + contains: string[]; +} + interface Manifest { description?: string; providers: string[]; - expectedError: string; + // Flat-corpus (error-code) shape — unchanged. + expectedError?: string; sealThenRegister?: string; + // #265 compose-load shape — see README.md "The `compose-load/` subdir". + composeWithCore?: boolean; + expectAttrs?: ExpectAttrs; + metadata?: unknown; + expectErrors?: string[]; } /** Pull the stable ERR_ code off a caught error, if it carries one. */ @@ -163,6 +204,12 @@ for (const file of manifestFiles) { test(`provider-composition: ${file}`, async () => { const manifest: Manifest = await Bun.file(join(CORPUS, file)).json(); const providers = manifest.providers.map(resolve); + // Flat-corpus manifests always carry expectedError (the old shape); guard + + // narrow rather than a non-null assertion so a malformed fixture fails loud. + const expectedError = manifest.expectedError; + if (expectedError === undefined) { + throw new Error(`flat-corpus manifest "${file}" is missing required "expectedError"`); + } if (manifest.sealThenRegister !== undefined) { // Compose (must succeed), seal, then run the probe against the sealed registry. @@ -176,7 +223,7 @@ for (const file of manifestFiles) { caught = err; } expect(caught).toBeDefined(); - expect(errorCode(caught)).toBe(manifest.expectedError); + expect(errorCode(caught)).toBe(expectedError); return; } @@ -188,6 +235,48 @@ for (const file of manifestFiles) { caught = err; } expect(caught).toBeDefined(); - expect(errorCode(caught)).toBe(manifest.expectedError); + expect(errorCode(caught)).toBe(expectedError); + }); +} + +// --------------------------------------------------------------------------- +// #265 compose-load corpus — see README.md "The `compose-load/` subdir". +// Own directory, own loop: a manifest here never carries `expectedError` / +// `sealThenRegister` (the flat-corpus shape); it carries `composeWithCore` / +// `expectAttrs` / `metadata` / `expectErrors` instead. +// --------------------------------------------------------------------------- + +const composeLoadManifestFiles = readdirSync(COMPOSE_LOAD_CORPUS) + .filter((f) => f.endsWith(".json")) + .sort(); + +test("provider-composition compose-load corpus is non-empty (guards against a mis-pathed COMPOSE_LOAD_CORPUS)", () => { + expect(composeLoadManifestFiles.length).toBeGreaterThan(0); +}); + +for (const file of composeLoadManifestFiles) { + test(`provider-composition (compose-load): ${file}`, async () => { + const manifest: Manifest = await Bun.file(join(COMPOSE_LOAD_CORPUS, file)).json(); + const providers = manifest.providers.map(resolve); + + const registry = manifest.composeWithCore + ? composeRegistry([...coreProviders, ...providers]) + : composeRegistry(providers); + + if (manifest.expectAttrs !== undefined) { + const { type, subType, contains } = manifest.expectAttrs; + const declaredNames = registry.attrsOf(type, subType).map((a) => a.name); + for (const name of contains) { + expect(declaredNames).toContain(name); + } + } + + if (manifest.metadata !== undefined) { + const doc = JSON.stringify(manifest.metadata); + const result = await MetaDataLoader.fromString(doc, "json", { registry, strict: true }); + const actualCodes = result.errors.map(errorCode).sort(); + const expectedCodes = [...(manifest.expectErrors ?? [])].sort(); + expect(actualCodes).toEqual(expectedCodes); + } }); }