From b4fbc8cc299e7fc6ad59ac8e05a96c40ad173386 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 03:59:58 +0000 Subject: [PATCH 1/2] fix(meta): /meta object reads report the audit governance the write path enforces (#4513) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The read surface resolved object documents through the sys_metadata overlay and MetadataService before the SchemaRegistry, and only the registry has been through applySystemFields — so a materialized created_at carrying FieldSchema defaults reported readonly: false while ObjectQL.update was refusing writes to that same field (#4447 closed the write half). The audit-family governance table moves to @objectstack/metadata-core, the one package both objectql and metadata-protocol depend on, by the same criterion as the #5619 dispatch predicates; every /meta object read exit now applies it. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KDU3qAuJyajAQm3GkUXdfA --- .../meta-read-audit-field-governance.md | 63 ++++ .../src/audit-field-governance.ts | 148 ++++++++ packages/metadata-core/src/index.ts | 9 + .../protocol.audit-field-governance.test.ts | 336 ++++++++++++++++++ packages/metadata-protocol/src/protocol.ts | 60 +++- .../src/engine-audit-anchor-write.test.ts | 100 ++++++ packages/objectql/src/registry.ts | 25 +- 7 files changed, 726 insertions(+), 15 deletions(-) create mode 100644 .changeset/meta-read-audit-field-governance.md create mode 100644 packages/metadata-core/src/audit-field-governance.ts create mode 100644 packages/metadata-protocol/src/protocol.audit-field-governance.test.ts diff --git a/.changeset/meta-read-audit-field-governance.md b/.changeset/meta-read-audit-field-governance.md new file mode 100644 index 0000000000..e1df029a14 --- /dev/null +++ b/.changeset/meta-read-audit-field-governance.md @@ -0,0 +1,63 @@ +--- +"@objectstack/metadata-core": patch +"@objectstack/metadata-protocol": patch +"@objectstack/objectql": patch +--- + +fix(meta): `/meta` object reads stop reporting `readonly: false` on fields the write path refuses (#4513) + +`#4447` made the audit-provenance family (`created_at`, `created_by`, +`updated_at`, `updated_by`) engine-owned on the **write** path: the registry's +`applySystemFields` forces `{ readonly: true, system: true }` over a *declared* +audit field, and `ObjectQL.update` strips a non-system caller's write to it. + +The **read** path never learned it. A `/meta` object read resolves through +`sys_metadata` overlay → MetadataService → SchemaRegistry, and only the last of +those three has been through `applySystemFields` — so an object whose built +artifact ships a materialized `created_at` carrying FieldSchema defaults +(`readonly: false`) reported that value to every client while writes to that +same field were being refused. Measured before the fix, all of the read exits +agreed with each other and disagreed with the engine: + +``` +single read: {"type":"datetime","label":"Created At","readonly":false} +list read: {"type":"datetime","label":"Created At","readonly":false} +cached read: {"type":"datetime","label":"Created At","readonly":false} +layered read: {"type":"datetime","label":"Created At","readonly":false} +``` + +One field, two answers — and the machine-readable one, the only face a client +or an AI author writing code off `/meta` can see, was the wrong one. + +**What changes.** Every `/meta` object read exit now reports the audit family +the way the engine enforces it. That covers the single-item read (both the +singular and plural type spelling), the list read, the cached/ETag branch, the +`?preview=draft` and `?state=draft` reads, and the layered read's `effective` +layer. `GET` bodies for objects that declare an audit field will show +`readonly: true, system: true` where they previously showed `readonly: false` +or omitted the keys; nothing else about the document changes, and the ETag for +such an object changes once. + +**What deliberately does not change.** + +- The layered read's `code` and `overlay` layers stay raw — showing the + package's declaration beside the governed `effective` value is the + diagnostic's whole point. +- `sys_metadata` still stores exactly what the author saved; the correction is + applied on the way out, so no phantom customization appears in the diff. +- An object that opts out of the audit family (`systemFields: false`, + `systemFields.audit: false`, `managedBy: 'better-auth'`) is untouched — the + engine enforces nothing there, so a read that claimed otherwise would be the + same lie pointing the other way. +- Only `readonly` and `system` are forced. Every other key an author writes — + `label`, `description`, `hidden`, `group`, and `type` for an external object + mapping a differently-typed remote column — stays theirs. + +The governance table moved from `packages/objectql/src/registry.ts` to +`@objectstack/metadata-core` (`AUDIT_FIELD_GOVERNANCE`, plus the +`applyAuditFieldGovernance` normalizer the read path applies), by the same +criterion and for the same cycle as the `#5619` engine-dispatch predicates: +`@objectstack/objectql` depends on `@objectstack/metadata-protocol`, so the +read path cannot import the table from the registry that enforces it, and a +second copy would agree only until someone edited one side. `objectql` +re-exports the symbol from its original path, so its public API is unchanged. diff --git a/packages/metadata-core/src/audit-field-governance.ts b/packages/metadata-core/src/audit-field-governance.ts new file mode 100644 index 0000000000..6b58641d13 --- /dev/null +++ b/packages/metadata-core/src/audit-field-governance.ts @@ -0,0 +1,148 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The **one** answer to "which keys on an object's audit-provenance columns are + * platform-owned rather than authorable?" — and the normalizer that applies + * that answer to a metadata document (objectstack#4513, from objectstack#4447). + * + * ## What #4447 established, and the half it left open + * + * `applySystemFields` (`@objectstack/objectql`) injects the + * {@link AUDIT_PROVENANCE_FIELDS} family and, since #4447, **forces** + * `readonly: true` / `system: true` over a *declared* audit field as well: + * `fields: { ...additions, ...schema.fields, ...overrides }`. That is what the + * write path enforces — `ObjectQL.update` strips a non-system caller's write to + * a statically-`readonly` field off the registry's post-injection schema, so a + * forged `created_at` is refused whatever the author declared. + * + * The **read** path never learned it. `GET /api/v1/meta/objects/:name` answers + * from `sys_metadata` (the stored overlay / build-artifact body) first and + * consults the registry only as a fallback, so an object whose artifact ships a + * materialized `created_at` carrying FieldSchema DEFAULTS (`readonly: false`) + * reported `readonly: false` to every client while writes to that same field + * were being refused. Measured on `origin/main` before this module existed, all + * four protocol read exits agreed with each other and disagreed with the + * engine: + * + * ``` + * single read: {"type":"datetime","label":"Created At","readonly":false} + * list read: {"type":"datetime","label":"Created At","readonly":false} + * cached read: {"type":"datetime","label":"Created At","readonly":false} + * layered read: {"type":"datetime","label":"Created At","readonly":false} + * ``` + * + * One field, two answers, and the machine-readable one was the wrong one — the + * face that clients, and AI authors writing code against `/meta`, are the only + * ones able to see. + * + * ## Why this module lives in `@objectstack/metadata-core` + * + * The same criterion `engine-delete-dispatch.ts` records, for the same cycle: + * `@objectstack/objectql` **depends on** `@objectstack/metadata-protocol`, so + * the read path cannot import the governance table from the registry that owns + * it. When a reverse import is impossible, the only honest way out is to sink + * the contract into a package **both sides already depend on** — and this + * package's own dependencies are `{ @objectstack/spec, zod }`, so there is no + * new edge and no new cycle. + * + * The alternative — a second governance table inside the read path — is exactly + * the drift this repo keeps paying for: the read would agree with the write + * only until someone edited one side, which is the state #4513 records. + * + * ## What it deliberately does NOT do + * + * - **It governs only DECLARED audit fields.** `applySystemFields` *injects* an + * absent one; this normalizer does not, because the served document is the + * authored metadata document and injecting columns into it would rewrite what + * a `GET` → `PUT` round-trip persists (the #4326 invariant) and what the + * layered read reports as "customised". An absent field does not claim + * `readonly: false`, so it is not the lie #4513 names. + * - **It governs only the audit family.** A declared `organization_id` / + * `owner_id` / `owning_business_unit_id` is the author's field and the + * registry lets it win (those are `additions`, not `overrides`), so reporting + * the author's value for them already agrees with what the write path + * enforces. Forcing them here would create the mismatch in the other + * direction. + */ + +import { + AUDIT_PROVENANCE_FIELDS, + resolveInjectedSystemColumns, + type AuditProvenanceField, +} from '@objectstack/spec/data'; + +/** + * The subset of an audit column's definition that is NOT authorable — the keys + * that decide **who may write** the column. + * + * Only `readonly` / `system` travel: everything else an author writes — + * `label`, `description`, `hidden`, `group`, and even `type` for an external + * object mapping a differently-typed remote column — stays theirs. Narrower is + * the point: this overrides an author, so it takes only what the defect + * requires (#4447). + * + * Keyed by the spec's {@link AUDIT_PROVENANCE_FIELDS} tuple, so a name added + * there without an entry here — or an entry for a name the spec dropped — is a + * compile error rather than a silently diverging copy. + */ +export const AUDIT_FIELD_GOVERNANCE: Record> = + Object.fromEntries( + AUDIT_PROVENANCE_FIELDS.map((name) => [name, { readonly: true, system: true }]), + ) as unknown as Record>; + +/** Does this field definition already carry every governance key at its governed value? */ +function isGoverned(declared: unknown, governance: Record): boolean { + if (!declared || typeof declared !== 'object' || Array.isArray(declared)) return false; + const rec = declared as Record; + for (const [key, value] of Object.entries(governance)) { + if (rec[key] !== value) return false; + } + return true; +} + +/** + * Force {@link AUDIT_FIELD_GOVERNANCE} onto every audit-provenance field the + * document declares, so what a reader is told about who may write the column + * matches what the engine enforces. + * + * Pure and total, with the same tolerance contract as + * {@link resolveInjectedSystemColumns}: any input may be handed to it, + * including a bare record that has never been through Zod. Objects that opt out + * of the audit family (`systemFields: false`, `systemFields.audit: false`, + * `managedBy: 'better-auth'`) carry no platform governance and are returned + * untouched — the same rows `applySystemFields` skips. + * + * Returns the **same reference** when nothing needed forcing, so a read path + * that already agrees with the engine (a registry-sourced document, which went + * through `applySystemFields` at registration) pays one comparison and no copy. + * + * @param doc An object metadata document, or any bare record shaped like one. + */ +export function applyAuditFieldGovernance(doc: T): T { + if (!doc || typeof doc !== 'object' || Array.isArray(doc)) return doc; + const rec = doc as unknown as Record; + const fields = rec.fields; + if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return doc; + + // WHICH columns this object carries is the spec's derivation — the same one + // `applySystemFields` consumes. Re-deriving the opt-out conditions here is + // precisely the drift this module exists to prevent. + if (!resolveInjectedSystemColumns(rec).audit) return doc; + + const declaredFields = fields as Record; + let governed: Record | undefined; + for (const name of AUDIT_PROVENANCE_FIELDS) { + const declared = declaredFields[name]; + // Absent is not a lie — see the module header. Only a DECLARED audit field + // can claim a writability the engine refuses. + if (declared === undefined || declared === null) continue; + if (isGoverned(declared, AUDIT_FIELD_GOVERNANCE[name])) continue; + governed ??= { ...declaredFields }; + governed[name] = typeof declared === 'object' && !Array.isArray(declared) + ? { ...(declared as Record), ...AUDIT_FIELD_GOVERNANCE[name] } + : { ...AUDIT_FIELD_GOVERNANCE[name] }; + } + + if (governed === undefined) return doc; + return { ...rec, fields: governed } as unknown as T; +} diff --git a/packages/metadata-core/src/index.ts b/packages/metadata-core/src/index.ts index 1c09f5ed3a..aba553c99a 100644 --- a/packages/metadata-core/src/index.ts +++ b/packages/metadata-core/src/index.ts @@ -26,3 +26,12 @@ export * from './objects/index.js'; // See `scripts/check-engine-double-contract.mjs` — the gate over the doubles. export * from './engine-delete-dispatch.js'; export * from './engine-update-dispatch.js'; + +// [#4513] The audit-family GOVERNANCE table (#4447) and its normalizer, sunk +// here for the same reason and by the same criterion as the two dispatch +// predicates above: the `/meta` READ path lives in +// `@objectstack/metadata-protocol`, which `@objectstack/objectql` depends on, +// so it cannot import the table from the registry that enforces it. The read +// surface and the write path now derive one answer from one table instead of +// reporting two. +export * from './audit-field-governance.js'; diff --git a/packages/metadata-protocol/src/protocol.audit-field-governance.test.ts b/packages/metadata-protocol/src/protocol.audit-field-governance.test.ts new file mode 100644 index 0000000000..8f27658548 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.audit-field-governance.test.ts @@ -0,0 +1,336 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#4513] Every `/meta` object read exit reports the audit family the way the + * ENGINE enforces it — `readonly: true, system: true` — and never the + * `readonly: false` a stored artifact/overlay body happens to declare. + * + * ## The defect, measured on `origin/main` before the fix + * + * `#4447` made the audit family engine-owned on the WRITE path: the registry's + * `applySystemFields` forces `{ readonly: true, system: true }` over a declared + * `created_at`, and `ObjectQL.update` strips a non-system caller's write to it. + * The READ path never learned it — a `/meta` object read resolves through + * `sys_metadata` overlay → MetadataService → SchemaRegistry, and only the last + * of those three has ever been through `applySystemFields`. With the showcase's + * materialized `created_at` (FieldSchema defaults, so `readonly: false`) stored + * as the overlay body, all four exits answered: + * + * ``` + * single read: {"type":"datetime","label":"Created At","readonly":false} + * list read: {"type":"datetime","label":"Created At","readonly":false} + * cached read: {"type":"datetime","label":"Created At","readonly":false} + * layered read: {"type":"datetime","label":"Created At","readonly":false} + * ``` + * + * while a PATCH to that same field was being refused. One field, two answers, + * and the machine-readable one — the only face a client, or an AI author + * writing code off `/meta`, can see — was the wrong one. + * + * ## Why one case table and not one file per exit + * + * The exits are several faces of ONE contract, and the defect is that they can + * answer differently. A per-exit file lets a future divergence hide in the file + * nobody added a case to; driving every exit from one table means a new exit + * that disagrees fails here, and the table is where you add it. + * + * `_diagnostics.valid` is asserted alongside every row on purpose: the rule + * judges a VALUE (who may write the column), so its fixture has to be a + * document the spec fully accepts — otherwise the row would be pinning the + * governance of something no author could have written in the first place. + */ + +import { describe, expect, it } from 'vitest'; +// [#5619] The producer's OWN write-verb dispatch decisions, so the fake engine +// below cannot accept a call ObjectQL refuses. From `@objectstack/metadata-core` +// and not `@objectstack/objectql`: objectql DEPENDS ON this package, so that +// import would close a cycle turbo rejects outright. +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { AUDIT_PROVENANCE_FIELDS } from '@objectstack/spec/data'; +import { ObjectStackProtocolImplementation } from './index.js'; + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + package_id: string | null; + state: string; + metadata: string; + checksum?: string; + version?: number; +} + +function matches(r: Row, where: Record): boolean { + for (const [k, v] of Object.entries(where)) { + if (v === undefined) continue; + if ((r as any)[k] !== v) return false; + } + return true; +} + +function keyOf(w: Record) { + return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; +} + +function makeStubEngine() { + const rows = new Map(); + let nextId = 0; + const findRow = (w: Record) => { + for (const [k, r] of rows) if (matches(r, w)) return { key: k, row: r }; + return null; + }; + const engine: any = { + async findOne(_t: string, opts: { where: Record }) { + return findRow(opts.where)?.row ?? null; + }, + async find(_t: string, opts: { where: Record }) { + return Array.from(rows.values()).filter((r) => matches(r, opts.where)); + }, + async insert(table: string, data: Record) { + if (table !== 'sys_metadata') return { id: 'side_table' }; + const row = { id: `r_${++nextId}`, ...(data as any) } as Row; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(table: string, data: Record, opts: { where: Record }) { + assertEngineUpdateDispatch(data, opts); + if (table !== 'sys_metadata') return { id: null }; + const found = findRow(opts.where); + if (!found) return { id: null }; + const merged = { ...found.row, ...(data as any) }; + rows.delete(found.key); + rows.set(keyOf(merged), merged); + return { id: found.row.id }; + }, + async delete(_t: string, opts?: Record) { + assertEngineDeleteDispatch(opts); + return { deleted: 0 }; + }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + async syncObjectSchema() { /* no DDL in this stub */ }, + registry: { + listItems: () => [], + isPackageDisabled: () => false, + getItem: () => undefined, + registerItem: () => {}, + registerObject: () => {}, + getPackage: () => undefined, + }, + }; + return { engine, rows }; +} + +/** + * The showcase's built object body, verbatim in the shape that caused #4447 — + * a MATERIALIZED audit field carrying only FieldSchema defaults. `readonly` is + * spelled `false` on two of the four and left ABSENT on the other two, because + * absent defaults to false as well and the read must not be right only for the + * spelling that happens to be explicit. + */ +const artifactObject = (name: string) => ({ + name, + label: 'Invoice', + fields: { + amount: { type: 'currency', label: 'Amount' }, + created_at: { + label: 'Created At', type: 'datetime', required: false, + searchable: false, multiple: false, unique: false, + deleteBehavior: 'set_null', hidden: false, + readonly: false, sortable: true, externalId: false, + }, + created_by: { type: 'lookup', reference: 'sys_user', label: 'Created By' }, + updated_at: { type: 'datetime', label: 'Updated At', readonly: false }, + updated_by: { type: 'lookup', reference: 'sys_user', label: 'Updated By' }, + }, +}); + +/** One `/meta` read exit: how a client gets an object document out of the protocol. */ +interface Exit { + /** The route this exit serves, so a failure names the surface a client would hit. */ + readonly label: string; + /** Seeded before the read (draft exits need a draft row rather than an active one). */ + readonly mode?: 'draft'; + read( + protocol: ObjectStackProtocolImplementation, + name: string, + ): Promise | undefined>; +} + +const EXITS: readonly Exit[] = [ + { + label: 'GET /meta/object/:name — single item', + async read(p, name) { + return (await p.getMetaItem({ type: 'object', name })).item as any; + }, + }, + { + label: 'GET /meta/objects/:name — the PLURAL spelling of the same route (#4432)', + async read(p, name) { + return (await (p as any).getMetaItem({ type: 'objects', name })).item; + }, + }, + { + label: 'GET /meta/objects — list', + async read(p, name) { + const list: any = await p.getMetaItems({ type: 'object' }); + return (list.items as any[]).find((i) => i?.name === name); + }, + }, + { + label: 'GET /meta/object/:name — cached / ETag branch', + async read(p, name) { + const cached: any = await p.getMetaItemCached({ type: 'object', name }); + return cached?.data; + }, + }, + { + label: 'GET /meta/object/:name?layers=1 — the `effective` layer', + async read(p, name) { + const layered: any = await (p as any).getMetaItemLayered({ type: 'object', name }); + return layered?.effective; + }, + }, + { + label: 'GET /meta/object/:name?preview=draft — draft overlaid on active', + mode: 'draft', + async read(p, name) { + return (await p.getMetaItem({ type: 'object', name, previewDrafts: true })).item as any; + }, + }, + { + label: 'GET /meta/object/:name?state=draft — the strict draft read', + mode: 'draft', + async read(p, name) { + return (await p.getMetaItem({ type: 'object', name, state: 'draft' })).item as any; + }, + }, +]; + +async function seed(mode: 'active' | 'draft', name: string) { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await protocol.saveMetaItem({ + type: 'object', name, item: artifactObject(name), + ...(mode === 'draft' ? { mode: 'draft' as const } : {}), + }); + return { protocol, rows }; +} + +describe('[#4513] the /meta read surface agrees with the enforced write behaviour', () => { + for (const exit of EXITS) { + describe(exit.label, () => { + it('reports every audit-provenance field as engine-owned', async () => { + const { protocol } = await seed(exit.mode ?? 'active', 'crm_invoice'); + const served = await exit.read(protocol, 'crm_invoice'); + expect(served, 'the exit must actually resolve the item').toBeDefined(); + + for (const field of AUDIT_PROVENANCE_FIELDS) { + expect( + served!.fields[field], + `${field} is stripped from a non-system caller's write; the read must say so`, + ).toMatchObject({ readonly: true, system: true }); + } + }); + + it('keeps every non-governance key the author declared', async () => { + const { protocol } = await seed(exit.mode ?? 'active', 'crm_invoice'); + const served = await exit.read(protocol, 'crm_invoice'); + // Governance is `readonly` + `system` and nothing else — the + // author's presentation and storage shape survive untouched. + expect(served!.fields.created_at).toMatchObject({ + label: 'Created At', type: 'datetime', sortable: true, required: false, + }); + expect(served!.fields.created_by).toMatchObject({ + type: 'lookup', reference: 'sys_user', label: 'Created By', + }); + // …and an ordinary business field is not touched at all. + expect(served!.fields.amount).toEqual({ type: 'currency', label: 'Amount' }); + }); + }); + } + + it('the governed document is one the spec fully accepts', async () => { + // The rule judges a VALUE, so the fixture has to parse green — otherwise + // every row above would be pinning the writability of a document no + // author could have written. `_diagnostics` is the product's own + // `safeParse` over exactly the body it just served. + const { protocol } = await seed('active', 'crm_invoice'); + const served: any = (await protocol.getMetaItem({ type: 'object', name: 'crm_invoice' })).item; + expect(served._diagnostics).toEqual({ valid: true }); + }); + + it('an object that opts OUT of the audit family is left exactly as declared', async () => { + // The mirror row. `systemFields: { audit: false }` makes + // `applySystemFields` skip the family entirely, so the engine enforces + // NOTHING on this object's `created_at` — and a read that forced + // `readonly: true` here would be the same lie pointing the other way. + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + const optedOut = { + ...artifactObject('legacy_ledger'), + systemFields: { audit: false }, + }; + await protocol.saveMetaItem({ type: 'object', name: 'legacy_ledger', item: optedOut }); + + const served: any = (await protocol.getMetaItem({ type: 'object', name: 'legacy_ledger' })).item; + expect(served.fields.created_at.readonly).toBe(false); + expect(served.fields.created_at.system).toBeUndefined(); + expect(served.fields.created_by.readonly).toBeUndefined(); + }); + + it('a non-object metadata type is never touched', async () => { + // The governance is an OBJECT-schema contract, so the normalizer is + // gated on the metadata TYPE and not on "this document happens to have + // a `fields` key" — nothing stops another type from carrying one, and + // rewriting it there would be inventing a rule the engine does not have. + // + // Seeded straight into `sys_metadata` rather than through + // `saveMetaItem`: the subject here is the READ path, and a `page` body + // shaped like this is (correctly) refused by the save path's schema, so + // going through it would only measure that refusal. + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + const stored = { + name: 'invoice_page', + fields: { created_at: { type: 'datetime', label: 'Created At', readonly: false } }, + }; + await engine.insert('sys_metadata', { + type: 'page', name: 'invoice_page', organization_id: null, package_id: null, + state: 'active', metadata: JSON.stringify(stored), + }); + + const served: any = (await protocol.getMetaItem({ type: 'page', name: 'invoice_page' })).item; + expect(served.fields.created_at).toEqual({ + type: 'datetime', label: 'Created At', readonly: false, + }); + }); +}); + +describe('[#4513] the served correction does not become a phantom customization', () => { + it('the overlay row keeps what the author actually stored', async () => { + // The read reports the EFFECTIVE value; `sys_metadata` keeps the + // author's declaration. Collapsing the two would make the layered + // read's `code` vs `overlay` diff report a customization nobody made. + const { protocol, rows } = await seed('active', 'crm_invoice'); + await protocol.getMetaItem({ type: 'object', name: 'crm_invoice' }); + + const stored = JSON.parse(Array.from(rows.values()).find((r) => r.name === 'crm_invoice')!.metadata); + expect(stored.fields.created_at.readonly).toBe(false); + expect(stored.fields.created_at.system).toBeUndefined(); + }); + + it('the layered read still shows the raw `code`/`overlay` layers beside the governed `effective`', async () => { + const { protocol } = await seed('active', 'crm_invoice'); + const layered: any = await (protocol as any).getMetaItemLayered({ + type: 'object', name: 'crm_invoice', + }); + // The diagnostic's whole point is seeing the layers as they are… + expect(layered.overlay.fields.created_at.readonly).toBe(false); + // …while `effective` keeps its documented promise of being what + // `getMetaItem` would return. + expect(layered.effective.fields.created_at).toMatchObject({ readonly: true, system: true }); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 8031298444..b9f7d217c5 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -8,7 +8,7 @@ import { readEnvWithDeprecation } from '@objectstack/types'; import type { MetadataHostEngine } from './host-engine.js'; import { evaluateRuntimeAuthoringGate } from './runtime-authoring-gate.js'; import { SysMetadataRepository, type SysMetadataEngine } from './sys-metadata-repository.js'; -import { ConflictError, assertProtocolCompat, type MetadataItem } from '@objectstack/metadata-core'; +import { ConflictError, assertProtocolCompat, applyAuditFieldGovernance, type MetadataItem } from '@objectstack/metadata-core'; // [#5532] One vocabulary of "which driver read errors are benign", shared with // `sys-metadata-repository.ts` in this package and with `DatabaseLoader` in // `@objectstack/metadata` (#5108). See `rethrowUnlessMetadataStoreUnprovisioned`. @@ -130,6 +130,33 @@ function canonicalizeMetaRequestType(request: T): T return type === request.type ? request : { ...request, type }; } +/** + * [#4513] The last thing every `/meta` read does to an OBJECT document before + * it leaves this service: make the field metadata it reports agree with what + * the engine enforces on the write path. + * + * The mismatch this closes is structural, not incidental. A `/meta` object read + * resolves through `sys_metadata` overlay → MetadataService → SchemaRegistry, + * and only the last of those three has been through `applySystemFields` — so + * the two stored layers answered with whatever the artifact/overlay body + * happened to declare, while `ObjectQL.update` was stripping caller writes to + * the audit family off the registry's post-injection schema. `created_at` read + * `readonly: false` and wrote as read-only, on the same field, at the same + * moment, from the one face a client can actually see (#4447 fixed the write + * half; this is the read half). + * + * Applied per EXIT rather than inside `decorateMetadataItem`: decoration is a + * diagnostics concern whose output `stripReadDecorations` deliberately removes + * again on write, and governance is neither — it is what the document means. + * + * `applyAuditFieldGovernance` returns its input by reference when nothing needed + * forcing, so the registry-sourced path (already governed at registration) and + * every non-object type pay a comparison and no copy. + */ +function governServedItem(type: string, item: T): T { + return canonicalMetaType(type) === 'object' ? applyAuditFieldGovernance(item) : item; +} + /** * [#5206 step 2] Where an author of THIS path sets the namespace the ADR-0121 * D2 gate demands. @@ -3544,7 +3571,11 @@ export class ObjectStackProtocolImplementation implements (it as any)?.name, packageId ?? ((it as any)?._packageId as string | undefined), ); - return mergeArtifactProtection(it, a) as any; + // [#4513] Same governance as the single-item read — the list + // is the other exit a client reads field metadata from, and + // an overlay row wins over the (already-governed) registry + // entry in the merge above, so it carries the same lie. + return governServedItem(request.type, mergeArtifactProtection(it, a)) as any; }), ), }; @@ -3605,7 +3636,11 @@ export class ObjectStackProtocolImplementation implements if (recPkg && (draftItem as any)._packageId === undefined) (draftItem as any)._packageId = recPkg; (draftItem as any)._draft = true; } - return { type: request.type, name: request.name, item: decorateMetadataItem(request.type, draftItem) }; + return { + type: request.type, + name: request.name, + item: decorateMetadataItem(request.type, governServedItem(request.type, draftItem)), + }; } } catch (error) { // [#5532] Falling through to the active read here would answer @@ -3695,7 +3730,11 @@ export class ObjectStackProtocolImplementation implements err.status = 404; throw err; } - return { type: request.type, name: request.name, item: decorateMetadataItem(request.type, item) }; + return { + type: request.type, + name: request.name, + item: decorateMetadataItem(request.type, governServedItem(request.type, item)), + }; } // 2. MetadataService (runtime-registered items: HMR-updated view/page/ @@ -3790,7 +3829,7 @@ export class ObjectStackProtocolImplementation implements const artifactItem = this.lookupArtifactItem(request.type, request.name, request.packageId); let decorated = decorateMetadataItem( request.type, - mergeArtifactProtection(item, artifactItem), + governServedItem(request.type, mergeArtifactProtection(item, artifactItem)), ); // ADR-0047 — list views additionally get reference-integrity // diagnostics (userFilters/tabs fields must exist on the source @@ -4051,7 +4090,16 @@ export class ObjectStackProtocolImplementation implements this.rethrowUnlessMetadataStoreUnprovisioned(error); } - const effective: unknown | null = overlay ?? code; + // [#4513] `effective` is documented above as "what `getMetaItem` would + // return", and the response's `_diagnostics` is computed from it — so it + // carries the same audit-family governance that read now applies, or the + // sentence stops being true the moment the overlay declares a writable + // `created_at`. `code` and `overlay` are deliberately left RAW: they are + // the diagnostic's whole point (what the package shipped vs what was + // customised), and a Studio diff showing `code`'s declaration next to + // `effective`'s governed value is the platform override made visible, + // not a defect. + const effective: unknown | null = governServedItem(request.type, overlay ?? code); const _diagnostics = effective !== null && effective !== undefined diff --git a/packages/objectql/src/engine-audit-anchor-write.test.ts b/packages/objectql/src/engine-audit-anchor-write.test.ts index 778b2b2b1d..5dab85217d 100644 --- a/packages/objectql/src/engine-audit-anchor-write.test.ts +++ b/packages/objectql/src/engine-audit-anchor-write.test.ts @@ -11,6 +11,11 @@ */ import { describe, it, expect, beforeEach } from 'vitest'; +// [#4513] The `/meta` read path's share of the governance table this file's +// write-path assertions measure. Imported from `@objectstack/metadata-core` +// because that is where it lives for BOTH consumers — see the last describe. +import { applyAuditFieldGovernance } from '@objectstack/metadata-core'; +import { AUDIT_PROVENANCE_FIELDS } from '@objectstack/spec/data'; import { ObjectQL } from './engine.js'; const taskObject = { @@ -304,3 +309,98 @@ describe('[#4447] a declared audit field cannot loosen the platform posture', () }); }); }); + +// --------------------------------------------------------------------------- +// [#4513] The other half of the same contract: what the `/meta` READ surface +// tells a client about these fields. +// +// #4447 closed the write path off the registry's post-injection schema — a +// place `@objectstack/metadata-protocol` structurally cannot reach (`objectql` +// depends on IT, so the import would close a turbo-rejected cycle). The read +// path therefore answered from the stored artifact/overlay body and reported +// `readonly: false` on the very field every assertion above proves is refused. +// +// `applyAuditFieldGovernance` is the read path's share of one table, and this +// block is where the two are held against each other with BOTH sides live: the +// engine's refusal measured on a real write, and the normalizer's answer +// measured on the same document. A future key added to the governance the +// registry forces, but not to the normalizer, fails here rather than in a +// production `/meta` body. +// --------------------------------------------------------------------------- +describe('[#4513] the read-path normalizer answers what this engine enforces', () => { + const shadowed = { + name: 'audit_readface', + label: 'Shadowed', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' as const }, + // The same materialized artifact field as the block above. + created_at: { + label: 'Created At', type: 'datetime' as const, required: false, + readonly: false, sortable: true, + }, + }, + }; + + let engine: ObjectQL; + beforeEach(async () => { + engine = new ObjectQL(); + const { driver } = makeStubDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(shadowed as any); + }); + + it('the normalizer reproduces the registry\'s governance, field for field', () => { + // Ground truth: what the engine will enforce, read off the live registry. + const enforced: any = engine.registry.getObject('audit_readface').fields; + // What a `/meta` read is allowed to say, computed from the AUTHORED + // document — the body the read path actually has in hand. + const reported: any = (applyAuditFieldGovernance(shadowed) as any).fields; + + for (const name of AUDIT_PROVENANCE_FIELDS) { + if (!reported[name]) continue; // the read path governs declared fields only + expect(reported[name].readonly, `${name}.readonly`).toBe(enforced[name].readonly); + expect(reported[name].system, `${name}.system`).toBe(enforced[name].system); + } + expect(reported.created_at).toMatchObject({ readonly: true, system: true }); + }); + + it('the refusal and the reported value are measured on the same field', async () => { + const row: any = await engine.insert( + 'audit_readface', { title: 'T' }, { context: { userId: 'u1' } } as any, + ); + const real = row.created_at; + await engine.update( + 'audit_readface', + { title: 'T2', created_at: FORGED }, + { where: { id: row.id }, context: { userId: 'u1' } } as any, + ); + const after: any = await engine.findOne('audit_readface', { where: { id: row.id } } as any); + + // The write was refused … + expect(after.created_at).toBe(real); + expect(after.created_at).not.toBe(FORGED); + // … so the read is not permitted to advertise it as writable. + expect((applyAuditFieldGovernance(shadowed) as any).fields.created_at.readonly).toBe(true); + }); + + it('leaves an audit-opted-out object alone, because the engine does too', () => { + const optedOut = { + name: 'audit_optout', + label: 'Opted out', + systemFields: { audit: false }, + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + created_at: { label: 'Created At', type: 'datetime' as const, readonly: false }, + }, + }; + engine.registry.registerObject(optedOut as any); + + const enforced: any = engine.registry.getObject('audit_optout').fields.created_at; + const reported: any = (applyAuditFieldGovernance(optedOut) as any).fields.created_at; + expect(enforced.readonly).toBe(false); + expect(reported.readonly).toBe(false); + expect(reported.system).toBeUndefined(); + }); +}); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index a72fb1dcc4..5b3ce386ff 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -1,6 +1,9 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ServiceObject, ObjectSchema, ObjectOwnership, provisionPrimary, resolveCrudAffordances, resolveInjectedSystemColumns, LEGACY_API_METHODS, AUDIT_PROVENANCE_FIELDS, type AuditProvenanceField } from '@objectstack/spec/data'; +// [#4513] The audit-family governance table — see the re-export below for why +// it lives in a package both `objectql` and `metadata-protocol` depend on. +import { AUDIT_FIELD_GOVERNANCE } from '@objectstack/metadata-core'; import { SystemFieldName } from '@objectstack/spec/system'; import { resolveTenancyPosture, resolveSearchPinyinEnabled } from '@objectstack/types'; import { postureEnforcesWall } from '@objectstack/spec/security'; @@ -300,16 +303,20 @@ const AUDIT_FIELD_DEFS = { * Only `readonly` / `system` travel: everything else an author writes — * `label`, `description`, `hidden`, `group`, and even `type` for an * external object mapping a differently-typed remote column — stays theirs. + * `type` and `reference` are deliberately NOT forced: an external/federated + * object legitimately maps its audit column to a differently-typed remote + * column, and #4447 is about writability, not storage shape. Narrower is the + * point — this overrides an author, so it takes only what the defect requires. + * + * [#4513] The table itself now lives in `@objectstack/metadata-core`, because + * the `/meta` READ surface has to report the same answer this injection + * enforces and could not reach it here: `@objectstack/objectql` depends on + * `@objectstack/metadata-protocol`, so the import a read-side pin needs would + * close a turbo-rejected cycle. Same criterion, same package, and for the same + * reason as the #5619 dispatch predicates. Re-exported here so the symbol + * still resolves from `@objectstack/objectql`. */ -const AUDIT_FIELD_GOVERNANCE: Record> = - Object.fromEntries( - // ONLY the keys that decide WHO MAY WRITE the column. `type` and - // `reference` are deliberately NOT forced: an external/federated object - // legitimately maps its audit column to a differently-typed remote column, - // and #4447 is about writability, not storage shape. Narrower is the point - // — this overrides an author, so it takes only what the defect requires. - AUDIT_PROVENANCE_FIELDS.map((name) => [name, { readonly: true, system: true }]), - ) as unknown as Record>; +export { AUDIT_FIELD_GOVERNANCE }; /** * [ADR-0117 D1] The injected BU-ownership column name, spelled out so it greps From 5574b6a7113f954967ebc2bbd0800faeea1ba456 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 04:41:44 +0000 Subject: [PATCH 2/2] test(objectql): type the new #4513 call sites so the TEST_DEBT ratchet stays at its floor check:query-options-erasure counted a new `findOne(..., {...} as any)` against the test-surface ceiling, and check:type-check-debt measured objectql's hidden test layer at +2. Both are shrink-only ratchets, so the new lines pay their own way: the findOne options bag drops its erasure (the call is on contract and the signature infers it), registerObject passes its required packageId, and the getObject reads use optional chaining. Measured 353 vs the ledger's 355. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01KDU3qAuJyajAQm3GkUXdfA --- .../src/engine-audit-anchor-write.test.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/objectql/src/engine-audit-anchor-write.test.ts b/packages/objectql/src/engine-audit-anchor-write.test.ts index 5dab85217d..cd9451d02f 100644 --- a/packages/objectql/src/engine-audit-anchor-write.test.ts +++ b/packages/objectql/src/engine-audit-anchor-write.test.ts @@ -348,12 +348,15 @@ describe('[#4513] the read-path normalizer answers what this engine enforces', ( const { driver } = makeStubDriver(); engine.registerDriver(driver, true); await engine.init(); - engine.registry.registerObject(shadowed as any); + // [#4311] Typed call site: `packageId` is required, and the ledger over + // this package's hidden test layer is a shrink-only ratchet — a new test + // pays its own way rather than raising the frozen number. + engine.registry.registerObject(shadowed as any, 'test'); }); it('the normalizer reproduces the registry\'s governance, field for field', () => { // Ground truth: what the engine will enforce, read off the live registry. - const enforced: any = engine.registry.getObject('audit_readface').fields; + const enforced: any = engine.registry.getObject('audit_readface')?.fields; // What a `/meta` read is allowed to say, computed from the AUTHORED // document — the body the read path actually has in hand. const reported: any = (applyAuditFieldGovernance(shadowed) as any).fields; @@ -376,7 +379,9 @@ describe('[#4513] the read-path normalizer answers what this engine enforces', ( { title: 'T2', created_at: FORGED }, { where: { id: row.id }, context: { userId: 'u1' } } as any, ); - const after: any = await engine.findOne('audit_readface', { where: { id: row.id } } as any); + // [#4918] No `as any` on the options bag — this call is ON contract, and + // the signature infers it. + const after: any = await engine.findOne('audit_readface', { where: { id: row.id } }); // The write was refused … expect(after.created_at).toBe(real); @@ -395,9 +400,9 @@ describe('[#4513] the read-path normalizer answers what this engine enforces', ( created_at: { label: 'Created At', type: 'datetime' as const, readonly: false }, }, }; - engine.registry.registerObject(optedOut as any); + engine.registry.registerObject(optedOut as any, 'test'); - const enforced: any = engine.registry.getObject('audit_optout').fields.created_at; + const enforced: any = engine.registry.getObject('audit_optout')?.fields.created_at; const reported: any = (applyAuditFieldGovernance(optedOut) as any).fields.created_at; expect(enforced.readonly).toBe(false); expect(reported.readonly).toBe(false);