Skip to content

Commit 3c1bbd2

Browse files
os-muskclaude
andauthored
fix(objectql,metadata): derive a view container's object through the shared helper, so the row's own name is LAST at every SOURCE registrar (#14399) (#14665)
* fix(objectql,metadata): derive a view container's object through the shared helper, so the row's own `name` is LAST at every SOURCE registrar (#14399) The ObjectQL boot-loop registrar read a container's own `name` FIRST, while `deriveViewContainerObject` (@objectstack/metadata) and `expandRuntimeViewContainer` (@objectstack/metadata-protocol) read it LAST. A container written as `{ name: 'lead_views', object: 'crm_lead', list: { … } }` was therefore keyed `lead_views` by one SOURCE registrar and `crm_lead` by the others, expansion included — and `getViewsByObject()` / `GET /meta/view?object=` filter on the expanded items' `object`, so which registrar loaded the document decided whether the views were addressable under the object at all. The boot loop's container branch now calls `deriveViewContainerObject` by import; a fourth hand-copy of the chain was the defect, not the repair. `deriveViewContainerObject` moves onto `@objectstack/metadata`'s root entry to make that import legal without reaching into another package's `src/`. Only the CONTAINER branch moves, gated on `isAggregatedViewContainer`; the assembled `viewItems:` channel still keys by its own `name` first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 * docs(changeset): state the measured migration surface (0 of 54 non-test view-container sources are divergent) (#14399) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 * chore(docs): re-anchor system-context census after the engine.ts line shift (#14399) Generated by `node scripts/check-system-context-census.mjs --fix` on the merged tree; 15 anchors rewritten, never hand-edited. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent dc7c226 commit 3c1bbd2

5 files changed

Lines changed: 414 additions & 27 deletions

File tree

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
---
2+
"@objectstack/metadata": minor
3+
"@objectstack/objectql": patch
4+
---
5+
6+
fix(objectql,metadata): the ObjectQL boot loop derives a view container's object through the shared `deriveViewContainerObject`, so the row's own `name` is the LAST term at every SOURCE registrar (#14399)
7+
8+
Three sites derive "which object does an aggregated `defineView` container bind
9+
to". After #13407 / #13913 / #13912 all three read the container's own top-level
10+
`object` before the `list.data.object` chain, but they still disagreed about the
11+
row's own `name`:
12+
13+
- `packages/objectql/src/engine.ts` `resolveMetadataItemName('views', item)`
14+
the boot-loop SOURCE registrar — read `name` FIRST, before `object`;
15+
- `deriveViewContainerObject` (`@objectstack/metadata`, used by the artifact/HMR
16+
SOURCE registrar and by `getViewsByObject()`) and `expandRuntimeViewContainer`
17+
(`@objectstack/metadata-protocol`, the runtime door) both read `name` LAST.
18+
19+
A container written as `{ name: 'lead_views', object: 'crm_lead', list: { … } }`
20+
therefore registered under `lead_views` through the boot loop and under
21+
`crm_lead` everywhere else, with the whole expansion (`<object>.<key>`) carried
22+
along — and since `getViewsByObject()` / `GET /meta/view?object=` filter the
23+
expanded items by their `object`, which registrar loaded the document decided
24+
whether the views were addressable under the object at all. No error, no
25+
diagnostic.
26+
27+
The boot loop's container branch now calls `deriveViewContainerObject` — by
28+
import, not by re-spelling: a fourth hand-copy of the chain was the defect, not
29+
the repair. The direction is the 2026-08-07 meta-rule rather than taste (one
30+
operation, two inconsistent implementations, the side bound by a DECLARATION
31+
wins): `ViewSchema.object`'s own `.describe()` names its readers, while the boot
32+
loop's order argued from item identity, which declares nothing about the
33+
binding. The two sites that already held the winning order are untouched.
34+
35+
**`@objectstack/metadata` — new public export (`minor`).**
36+
`deriveViewContainerObject` was module-local; it is now on the package's root
37+
entry, because `packages/objectql` is a SOURCE registrar for the same containers
38+
and has to mint the same key. `packages/objectql` already declares
39+
`@objectstack/metadata` as a dependency and nothing in `packages/metadata`
40+
depends on `objectql`, so the import adds no cycle.
41+
42+
**Scope of the behaviour change.** Only the `views` CONTAINER branch moves, gated
43+
on `isAggregatedViewContainer`: the assembled `viewItems:` channel (standalone
44+
ViewItems and flattened overlays, every member of `AssembledViewArtifactSchema`
45+
requiring `viewKind`) still keys by its own `name` first, which is its identity
46+
and not a binding. `item.id` is untouched and cannot fire for a container —
47+
`ViewSchema` is a `strictObject` declaring `name` and `object` and no `id`.
48+
49+
**No migration surface.** Measured on this tree: of the 54 non-test sources that
50+
author or carry view containers, ZERO declare a `name` that differs from the
51+
object they bind to, so every in-tree container derived identically at all three
52+
sites before this change and does after it. What moves is the latent shape only.
53+
54+
⚠️ One card premise was measured false and is recorded in the new pin rather
55+
than quietly dropped: the artifact/HMR registrar does not silently mint a second
56+
key for a divergent container. It derives `crm_lead` correctly and then refuses
57+
the whole artifact load — `assertMetadataRegisterContract` (#7378 row 1),
58+
`VALIDATION_ERROR` / 400 — because the document's own `data.name` still reads
59+
`lead_views`. The boot loop reconciles that field and the artifact door does
60+
not; that residual asymmetry is a separate defect at a separate site and is
61+
filed as its own card.

content/docs/permissions/system-context.mdx

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -109,18 +109,18 @@ that silently does not happen.
109109

110110
| # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor |
111111
|:--|:---|:---|:---|:---|
112-
| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11160` |
113-
| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11343` |
114-
| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9895` |
112+
| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11204` |
113+
| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11387` |
114+
| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:9939` |
115115
| 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1746` |
116-
| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9943`, `readonly-strict-errors.ts:66` |
117-
| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5762` |
118-
| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3606`, `:3616`, `:3643` |
116+
| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:9987`, `readonly-strict-errors.ts:66` |
117+
| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5806` |
118+
| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3650`, `:3660`, `:3687` |
119119
| 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` |
120120
| 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:98` |
121-
| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6460` |
122-
| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11955` |
123-
| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11884` |
121+
| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6504` |
122+
| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:11999` |
123+
| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:11928` |
124124

125125
### 3. Sharing (`plugin-sharing`)
126126

@@ -179,8 +179,8 @@ a reader tracing where elevation travels needs them.
179179

180180
| # | Site | Package | What it does |
181181
|:--|:---|:---|:---|
182-
| 62 | `objectql/src/engine.ts:3413` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
183-
| 63 | `objectql/src/engine.ts:14304` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
182+
| 62 | `objectql/src/engine.ts:3457` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes |
183+
| 63 | `objectql/src/engine.ts:14348` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag |
184184
| 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report |
185185
| 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across |
186186

@@ -195,7 +195,7 @@ assuming `isSystem` covers it is a documented source of bugs.
195195
|:---|:---|:---|
196196
| "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1971` (rationale at `:1881``1883`, #3760), `flow.zod.ts:685` |
197197
| "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) |
198-
| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9878``9895` |
198+
| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:9922``9939` |
199199
| "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) |
200200
| "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280``281` |
201201
| "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:286` |

packages/metadata/src/index.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,22 @@ export { YAMLSerializer } from './serializers/yaml-serializer.js';
3939
export * as Migration from './migration/index.js';
4040
export { TypeScriptSerializer } from './serializers/typescript-serializer.js';
4141

42+
// View container binding
43+
//
44+
// [#14399] `deriveViewContainerObject` is this package's ONE spelling of "which
45+
// object does an aggregated `defineView` container bind to" — the container's
46+
// own top-level `object` first, then `list.data.object`, `form.data.object`,
47+
// and the row's own `name` last (its own docblock carries the ruling). It is
48+
// published here because the ObjectQL boot-loop registrar
49+
// (`packages/objectql/src/engine.ts`, `resolveMetadataItemName`) is a SOURCE
50+
// registrar for the same containers and has to mint the same registry key: it
51+
// used to consult the row's `name` FIRST, so a container whose `name` differs
52+
// from its `object` registered under two different keys depending on which
53+
// registrar loaded it. `packages/objectql` already declares this package as a
54+
// dependency and nothing here depends on it, so the import is the repair — a
55+
// fifth hand-copy of the chain is the defect, not the fix.
56+
export { deriveViewContainerObject } from './view-container-expansion.js';
57+
4258
// Re-export types from spec
4359
export type {
4460
MetadataFormat,

packages/objectql/src/engine.ts

Lines changed: 59 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,11 @@ import {
185185
AssembledViewArtifactSchema,
186186
isViewContainerShaped,
187187
} from '@objectstack/spec';
188+
// [#14399] The ONE spelling of "which object does an aggregated `defineView`
189+
// container bind to", imported rather than re-spelled. See
190+
// `resolveMetadataItemName` below for why this registrar had a fourth copy and
191+
// why it lost it.
192+
import { deriveViewContainerObject } from '@objectstack/metadata';
188193
import { bindHooksToEngine } from './hook-binder.js';
189194
import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, ValidationError, buildFieldError, resolveFieldLabel, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField, valueShapeStrictEffective, mediaStrictEffective } from './validation/record-validator.js';
190195
import type { AdmittedValueShapeViolation, AdmittedValueShapeViolationSink } from './validation/record-validator.js';
@@ -2014,28 +2019,67 @@ const METADATA_ARRAY_KEYS = [
20142019
*
20152020
* Most metadata items expose a top-level `name` (or `id`). The `View`
20162021
* container defined by `@objectstack/spec/ui` is special: it aggregates
2017-
* `list / form / listViews / formViews` for a single object and is
2018-
* keyed implicitly by its target object name (see `data.object`).
2022+
* `list / form / listViews / formViews` for a single object and is keyed by
2023+
* the OBJECT it binds to, not by its own row identity — which is what
2024+
* `/api/v1/meta/views/:object`, `getViewsByObject()` and
2025+
* `GET /meta/view?object=` all address it by.
20192026
*
2020-
* Per spec, `ViewSchema` does NOT have a top-level `name` field
2021-
* (view.zod.ts), so we resolve it from the inner data source. This
2022-
* matches the server-side metadata API contract (`/api/v1/meta/views/:object`).
2027+
* ⚠️ [#14399] The sentence that used to stand here — "per spec, `ViewSchema`
2028+
* does NOT have a top-level `name` field" — is measurably false and was the
2029+
* premise for consulting `item.name` first. `ViewSchema` declares an optional
2030+
* `name` (`view.zod.ts`), described there as "supplied by the metadata door;
2031+
* for an object-scoped container it is the object name". "Is the object name"
2032+
* is a CONVENTION the door does not enforce, so the two never actually had to
2033+
* agree — and where they disagreed, this registrar and the other two picked
2034+
* different keys for the same document.
20232035
*/
20242036
function resolveMetadataItemName(key: string, item: any): string | undefined {
20252037
if (!item) return undefined;
2038+
// [#14399] The aggregated `views` CONTAINER branch, taken FIRST and answered
2039+
// by the shared derivation. Everything below is unchanged.
2040+
//
2041+
// This registrar used to consult `item.name` before anything else, for every
2042+
// key including this one — so a container written as
2043+
// `{ name: 'lead_views', object: 'crm_lead', list: {…} }` registered under
2044+
// `lead_views` here while the artifact/HMR SOURCE registrar
2045+
// (`MetadataPlugin._parseAndRegisterArtifact`) and the runtime door
2046+
// (`expandRuntimeViewContainer`) both registered it under `crm_lead`. Same
2047+
// document, two source registrars, two registry keys and two sets of expanded
2048+
// item names, with `getViewsByObject()` / `GET /meta/view?object=` answering
2049+
// for the object only when the right registrar happened to load it.
2050+
//
2051+
// The 2026-08-07 meta-rule settles the direction rather than taste: one
2052+
// operation with two inconsistent implementations, the side bound by a
2053+
// DECLARATION wins. `ViewSchema.object`'s own `.describe()` names its readers
2054+
// (`getViewsByObject()` / `GET /meta/view?object=`); this loop's old order
2055+
// argued from item identity, which declares nothing about the binding. So the
2056+
// container branch adopts `deriveViewContainerObject` — by import, because a
2057+
// fourth hand-copy of a chain that already exists three times is the defect
2058+
// this repair exists to close, not the repair.
2059+
//
2060+
// The gate is `isAggregatedViewContainer`, which is what makes this the
2061+
// CONTAINER branch and nothing wider: it is false for every artifact carrying
2062+
// a `viewKind`, so the assembled `viewItems:` channel below (standalone
2063+
// ViewItems and flattened list/form overlays — every member of
2064+
// `AssembledViewArtifactSchema` requires `viewKind`) still resolves by its own
2065+
// `name` first, which is its identity and not a binding.
2066+
//
2067+
// `item.id` is untouched and stays reachable for every other key. It cannot
2068+
// fire for a spec-valid container: `ViewSchema` is a `strictObject` that
2069+
// declares `name` and `object` and no `id`, so an `id` on a container is
2070+
// refused at the authoring and metadata doors before this seam sees it.
2071+
if (key === 'views' && isAggregatedViewContainer(item)) {
2072+
return deriveViewContainerObject(item);
2073+
}
20262074
if (item.name) return item.name;
20272075
if (item.id) return item.id;
20282076
if (key === 'views') {
2029-
// Independent ViewItems ("Object has-many View") carry a top-level `name`
2030-
// (handled above) and bind to their object via `object`. The aggregated
2031-
// container has no top-level name/object, so fall back to its inner data
2032-
// source — matching the loader's expansion key.
2033-
return (
2034-
item?.object ||
2035-
item?.list?.data?.object ||
2036-
item?.form?.data?.object ||
2037-
undefined
2038-
);
2077+
// A `views` entry that is NOT an aggregated container and carries neither
2078+
// `name` nor `id` — e.g. a flattened overlay whose optional `name` was
2079+
// omitted. Same derivation, and identical to the chain that used to be
2080+
// written out here: with `item.name` already known falsy, the helper's
2081+
// trailing `name` term contributes nothing.
2082+
return deriveViewContainerObject(item);
20392083
}
20402084
return undefined;
20412085
}

0 commit comments

Comments
 (0)