From 18cf11901aecef30258dbe4f14c2b67d5051912b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 09:26:47 +0000 Subject: [PATCH 1/2] wip: master-detail reference-spelling tolerance recorded with its measurement Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- .../src/master-detail-reference-alias.test.ts | 180 +++++++++++++++++ packages/objectql/src/master-detail.ts | 182 +++++++++++++++++- 2 files changed, 356 insertions(+), 6 deletions(-) create mode 100644 packages/objectql/src/master-detail-reference-alias.test.ts diff --git a/packages/objectql/src/master-detail-reference-alias.test.ts b/packages/objectql/src/master-detail-reference-alias.test.ts new file mode 100644 index 0000000000..0048bdb0e0 --- /dev/null +++ b/packages/objectql/src/master-detail-reference-alias.test.ts @@ -0,0 +1,180 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The reference-spelling contract of `resolveMasterDetailRelation` — three + * spellings, three different answers, pinned so none of them can drift + * silently. + * + * The module accepts the canonical `reference` and the REJECTED alias + * `referenceTo`, and reads the OTHER rejected alias `reference_to` not at all. + * That asymmetry is deliberate and is the thing most likely to be "tidied" by + * someone who notices only that a sibling reader (`resolveCbpRelation` in + * `plugin-security`) accepts all three: the ADR-0087 conversion layer already + * normalises `reference_to` on stored rehydration and on `os migrate meta`, + * and deliberately does not normalise `referenceTo`. So the one spelling that + * can arrive here unconverted is exactly the one this reader accepts. Pinning + * the asymmetry as a RECORD is the point — a test that only checked the happy + * path would let either half move without a failure. + * + * The loud half is pinned the same way `plugin-security`'s is: the report must + * name the key that ACTUALLY answered, so the diagnostic and the resolution + * can never disagree about which spelling was read. + * + * Imported relatively (`./master-detail.js`), i.e. from source through vitest's + * own resolution — no `dist/` leg, so an ablation of the loud line shows up + * here without a rebuild. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { resolveMasterDetailRelation } from './master-detail.js'; +import { SchemaRegistry } from './registry.js'; + +/** An object shape with one `master_detail` field spelled however the case needs. */ +function detailObject(name: string, key: string, master = 'crm_account') { + return { + name, + label: name, + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + account_id: { type: 'master_detail', label: 'Account', [key]: master }, + }, + } as never; +} + +describe('resolveMasterDetailRelation — the reference spelling it reads, and what it says about it', () => { + it('canonical `reference` resolves, and says NOTHING — the quiet path stays quiet', () => { + const warn = vi.fn(); + const rel = resolveMasterDetailRelation(detailObject('canon_detail', 'reference'), { warn }); + + expect(rel).toEqual({ fk: 'account_id', master: 'crm_account' }); + expect(warn).not.toHaveBeenCalled(); + }); + + it('`referenceTo` resolves TOO — the tolerance is real, not a leftover type key', () => { + const warn = vi.fn(); + const rel = resolveMasterDetailRelation(detailObject('alias_detail', 'referenceTo'), { warn }); + + expect(rel).toEqual({ fk: 'account_id', master: 'crm_account' }); + }); + + it('...and it is LOUD when it does: the report names the spelling that answered', () => { + const warn = vi.fn(); + resolveMasterDetailRelation(detailObject('loud_detail', 'referenceTo'), { warn }); + + expect(warn).toHaveBeenCalledTimes(1); + const msg = String(warn.mock.calls[0]?.[0]); + // The key that answered, the field it sat on, and the object — the + // three facts an author needs to find and rename it. + expect(msg).toContain('`referenceTo`'); + expect(msg).toContain('"loud_detail"'); + expect(msg).toContain('"account_id"'); + // ...and the half an operator needs so they do not go hunting an + // outage that did not happen. + expect(msg).toContain('UNAFFECTED'); + }); + + it('⛔ snake_case `reference_to` is NOT read here — the asymmetry with plugin-security is a record, not an oversight', () => { + const warn = vi.fn(); + const rel = resolveMasterDetailRelation(detailObject('snake_detail', 'reference_to'), { warn }); + + // No relation at all: this reader never had a `reference_to` arm, and + // the conversion layer is what serves that spelling (to `reference`) + // before a stored row ever reaches here. + expect(rel).toBeNull(); + // ...and nothing is reported, because nothing resolved from an alias. + expect(warn).not.toHaveBeenCalled(); + }); + + it('an un-injected host still hears it — the default sink is `console.warn`', () => { + const spy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + resolveMasterDetailRelation(detailObject('default_sink_detail', 'referenceTo')); + expect(spy).toHaveBeenCalledTimes(1); + expect(String(spy.mock.calls[0]?.[0])).toContain('[objectql/reference-spelling]'); + } finally { + spy.mockRestore(); + } + }); + + it('reports ONCE per object+field+spelling — the write path must not become a noise channel', () => { + const warn = vi.fn(); + const schema = detailObject('repeat_detail', 'referenceTo'); + for (let i = 0; i < 5; i++) resolveMasterDetailRelation(schema, { warn }); + + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('canonical WINS over the alias when both are present, and stays quiet', () => { + const warn = vi.fn(); + const rel = resolveMasterDetailRelation({ + name: 'both_detail', + fields: { + account_id: { + type: 'master_detail', + reference: 'crm_account', + referenceTo: 'crm_stale_legacy', + }, + }, + } as never, { warn }); + + expect(rel).toEqual({ fk: 'account_id', master: 'crm_account' }); + expect(warn).not.toHaveBeenCalled(); + }); + + it('a present-but-EMPTY `reference` does not fall through to the alias — the `??` semantics are unchanged', () => { + const warn = vi.fn(); + const rel = resolveMasterDetailRelation({ + name: 'empty_canon_detail', + fields: { + account_id: { type: 'master_detail', reference: ' ', referenceTo: 'crm_account' }, + }, + } as never, { warn }); + + // `a ?? b` falls through on null/undefined ONLY, so the empty canonical + // key still wins the read and still yields no usable name. + expect(rel).toBeNull(); + expect(warn).not.toHaveBeenCalled(); + }); + + it('two masters stay ambiguous, and report nothing — a relation that did not resolve has no spelling to name', () => { + const warn = vi.fn(); + const rel = resolveMasterDetailRelation({ + name: 'junction_detail', + fields: { + left_id: { type: 'master_detail', referenceTo: 'crm_account' }, + right_id: { type: 'master_detail', referenceTo: 'crm_contact' }, + }, + } as never, { warn }); + + expect(rel).toBeNull(); + expect(warn).not.toHaveBeenCalled(); + }); +}); + +describe('the path that makes the tolerance reachable at all', () => { + it('a raw `registerObject` carries `referenceTo` verbatim into the registry, and this reader then resolves it', () => { + // The reachability claim the module doc records, measured rather than + // asserted: `registerObject` skips Zod by design, so the rejected alias + // survives registration, and every caller of this resolver reads the + // schema back out of this same registry. + const registry = new SchemaRegistry({ multiTenant: false, searchCompanion: false } as never); + const consoleWarn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + registry.registerObject(detailObject('raw_registered_detail', 'referenceTo')); + } finally { + consoleWarn.mockRestore(); + } + + const served = registry.getObject('raw_registered_detail') as + { fields?: Record> } | undefined; + expect(served?.fields?.account_id?.referenceTo).toBe('crm_account'); + expect(served?.fields?.account_id?.reference).toBeUndefined(); + + const warn = vi.fn(); + expect(resolveMasterDetailRelation(served as never, { warn })).toEqual({ + fk: 'account_id', + master: 'crm_account', + }); + expect(warn).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/objectql/src/master-detail.ts b/packages/objectql/src/master-detail.ts index af1e7c3f8b..05d5f3b4b2 100644 --- a/packages/objectql/src/master-detail.ts +++ b/packages/objectql/src/master-detail.ts @@ -35,6 +35,59 @@ * loud, and left to the next review of ADR-0058 D5). That asymmetry is why the * build-time gate covers BOTH slots: it is the only thing standing between an * unbindable `requiredWhen` and a requirement that enforces nothing in silence. + * + * ## The tolerance, and the measurement behind it + * + * This reader accepts a second spelling of the reference target — the REJECTED + * alias `referenceTo` — and reports when that is what answered. Both halves + * are deliberate, and the reasoning is recorded here because the sentence this + * replaces was an assertion nobody had measured: it read *"`referenceTo` is the + * stored-row spelling"*, and that is not what the tree says. + * + * **What was measured** (whole tree, on `origin/main`, both spellings counted + * separately, positive controls run so no zero came from a pathspec matching + * nothing): + * + * - **Authored declarations — zero, both spellings.** Across `*.object.ts` + * (112 files), `examples/`, `packages/qa/` and the `create-objectstack` + * templates, all 8 `Field.masterDetail(...)` and 132 `Field.lookup(...)` + * declarations go through the `@objectstack/spec` builders, which emit the + * canonical key. Not one alias spelling is hand-written past them. + * - **Stored-metadata seeds, fixtures and `metadata-fs` layouts — zero, + * both spellings.** Every raw hit in JSON/YAML is prose. + * - **In-tree `referenceTo` on a field def — reader pins only.** Nine files, + * each pinning either a refusal or a tolerance. None models a deployment. + * - **Metadata AT REST in a live deployment — NOT MEASURED.** No command in + * this repository reaches it. The zeros above are zeros for the tree. + * + * **Why the tolerance stays anyway** — the population is unmeasured, but the + * PATH is not, and it is the one path nothing else covers: + * + * - A raw `registerObject` SKIPS Zod by design (`registry.ts` names those + * doors), and every caller of this resolver reads that same + * `SchemaRegistry`. So an alias-spelled object reaches here verbatim. + * - The ADR-0087 conversion layer normalises the OTHER alias — `reference_to` + * — on stored rehydration and on `os migrate meta`, and deliberately does + * NOT convert `referenceTo` (`spec/src/conversions/registry.ts`, which + * records why: camelCase is objectui's resolved action-param dialect, not + * what the objectql runtime wrote into stored rows). ⇒ `referenceTo` is the + * one spelling that is simultaneously unconverted upstream and read here. + * That asymmetry is the reason this reader is not symmetric either: it + * reads `referenceTo` and NOT `reference_to`, which is not an oversight. + * - And a miss here is not a quiet wrong answer on the readonly path. Two of + * this resolver's four callers FAIL CLOSED: an unresolved relation leaves + * `parent` unbound, and `rule-validator.ts` reads an unbound scope root as + * LOCKED. Narrowing this reader would take a raw-registered, alias-spelled + * detail object from "lock enforced against its header" to "every + * `parent`-scoped field permanently unwritable, writes silently stripped" + * — an availability defect, not a spelling correction. + * + * ⛔ So do not narrow this in place, and do not widen it either. Narrowing is + * only honest behind a migration that sweeps stored and raw-registered metadata + * first — the same precondition the sibling `controlled_by_parent` reader in + * `plugin-security` carries, and that reader's card holds the live-deployment + * census this one could not run. Widening it to `reference_to` would undo the + * conversion layer's work at the one seam that layer already covers. */ /** The child→master link: the FK field on the detail, and the master object. */ @@ -48,34 +101,151 @@ export interface MasterDetailRelation { /** The subset of a field definition this resolution reads. */ export interface RelationFieldDef { type?: string; - /** Canonical reference target. `referenceTo` is the stored-row spelling. */ + /** The one relationship spelling `@objectstack/spec` declares. */ reference?: string; + /** A REJECTED alias this reader still accepts — see "The tolerance" above. */ referenceTo?: string; } -/** The reference target a relation field names, or `undefined`. */ -function referenceOf(def: RelationFieldDef | null | undefined): string | undefined { - const raw = def?.reference ?? def?.referenceTo; +/** + * The spellings this reader accepts, in precedence order. Canonical first, so + * an object carrying both resolves from `reference` and reports nothing. + */ +const REFERENCE_SPELLINGS = ['reference', 'referenceTo'] as const; + +type ReferenceKey = (typeof REFERENCE_SPELLINGS)[number]; + +/** + * WHICH spelling answers for this field, or `undefined` when neither key is + * present. Split out so the diagnostic and the resolution cannot disagree + * about the key that was read — {@link referenceOf} derives its value from + * this answer rather than spelling a second `??` chain (the invariant + * `plugin-security`'s `refKey` records for the sibling reader). + * + * `!= null` is the exact test `a ?? b` applies, so this selects the same key + * the previous `def?.reference ?? def?.referenceTo` chain selected — a + * present-but-empty `reference` still wins the key and still yields + * `undefined` below, rather than falling through to the alias. + */ +function referenceKeyOf(def: RelationFieldDef | null | undefined): ReferenceKey | undefined { + return REFERENCE_SPELLINGS.find((k) => def?.[k] != null); +} + +/** The reference target under `key`, or `undefined` when it is not a usable name. */ +function referenceOf( + def: RelationFieldDef | null | undefined, + key: ReferenceKey | undefined, +): string | undefined { + const raw = key === undefined ? undefined : def?.[key]; return typeof raw === 'string' && raw.trim() !== '' ? raw : undefined; } +/** Options bag for {@link resolveMasterDetailRelation}. */ +export interface ResolveMasterDetailOptions { + /** + * Sink for the rejected-alias report. Defaults to `console.warn`, so a host + * that injects nothing still hears it — the same caller-supplied-callback + * shape (and the same default) as `warnFunctionalCompleteness` in + * `registry.ts`, which is a plain function in a bag rather than a method + * lifted off a receiver-sensitive logger. + */ + warn?: (message: string) => void; +} + +/** + * Relations already reported, keyed `object|field|spelling` — the report is + * once per distinct defect, not per write. + * + * Granularity matters here more than it does at the registry seam: this + * resolver runs on the WRITE path, once per write that wants a `parent` + * binding, and a per-write line is a noise defect of its own — a channel + * operators filter out, which would make the tolerance silent again by a + * longer route. The one boundary of a process-lifetime set, stated rather + * than discovered: re-introducing the SAME alias on the SAME field of the + * SAME object after it was corrected in-process reports nothing until the + * next restart. A new object, a new field or the other spelling all report. + */ +const reportedAliasRelations = new Set(); + +/** + * Report a relation that resolved only from a rejected alias. WARN, never + * throw, never a behaviour change: the relation resolved, and this is the + * record of HOW it resolved. + * + * The text names the spelling as the defect, states that behaviour is + * unaffected, and corrects the registry's line by name — all three are + * load-bearing. An operator who reads "rejected alias" and assumes something + * was denied would go looking for an outage that did not happen; and the + * registration-time diagnostic this object also trips + * (`field/relationship-without-reference`) tells them the field is + * "runtime-DEAD ... never-resolves", which is false for THIS consumer. Two + * diagnostics that disagree about the same field are worse than one, so this + * one says which is right where the resolution actually happens. + * + * No tracker ids: this string reaches operators, and `#NNNN` means nothing to + * them (`check:doc-authoring`). The anchors are the module doc above. + */ +function reportRejectedReferenceAlias( + objectName: string, + fk: string, + master: string, + alias: ReferenceKey, + options: ResolveMasterDetailOptions | undefined, +): void { + const seen = `${objectName}|${fk}|${alias}`; + if (reportedAliasRelations.has(seen)) return; + reportedAliasRelations.add(seen); + const warn = options?.warn ?? ((msg: string) => console.warn(msg)); + warn( + `[objectql/reference-spelling] object "${objectName}": its master-detail relation resolved ` + + `only from the REJECTED alias \`${alias}\` on field "${fk}" (master "${master}") — ` + + '`reference` is the one relationship spelling @objectstack/spec declares, and this object ' + + 'reached the registry without being parsed (a raw registerObject skips Zod by design). ' + + 'Behaviour is UNAFFECTED: the relation still resolves, so `parent`-scoped `readonlyWhen` ' + + 'stays bound and its lock keeps enforcing. The registry\'s functional-completeness line ' + + 'for this same field — `field/relationship-without-reference`, "runtime-DEAD ... ' + + 'never-resolves" — is wrong about THIS consumer; this line is the accurate one. Rename ' + + 'the key to `reference`: every parsed authoring path already refuses the alias by name, ' + + 'so the two answers disagree until you do.', + ); +} + /** * The object's master-detail relation, or `null` when it has none — or when it * has more than one and "the parent" is therefore not a fact the metadata * states (see the module doc). */ export function resolveMasterDetailRelation( - objectSchema: { fields?: Record } | undefined | null, + objectSchema: + | { name?: string; fields?: Record } + | undefined + | null, + options?: ResolveMasterDetailOptions, ): MasterDetailRelation | null { const fields = objectSchema?.fields; if (!fields) return null; let found: MasterDetailRelation | null = null; + let foundKey: ReferenceKey | undefined; for (const [name, def] of Object.entries(fields)) { if (def?.type !== 'master_detail') continue; - const master = referenceOf(def); + const key = referenceKeyOf(def); + const master = referenceOf(def, key); if (!master) continue; if (found) return null; // ambiguous — two masters, no single `parent` found = { fk: name, master }; + foundKey = key; + } + // Reported only for the relation this call RETURNS: a discarded candidate + // and the ambiguous case (which returns `null`) resolved nothing, so there + // is no "the alias answered" to report about them. + if (found && foundKey !== undefined && foundKey !== 'reference') { + reportRejectedReferenceAlias( + String(objectSchema?.name ?? '(unnamed)'), + found.fk, + found.master, + foundKey, + options, + ); } return found; } From c2d29a2f5fcf4aef84feb4c63f05f4a27fca7c05 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 09:32:31 +0000 Subject: [PATCH 2/2] fix(objectql): record the master-detail `referenceTo` tolerance with its measurement, and report where the alias answered Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68 --- .../master-detail-reference-alias-measured.md | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 .changeset/master-detail-reference-alias-measured.md diff --git a/.changeset/master-detail-reference-alias-measured.md b/.changeset/master-detail-reference-alias-measured.md new file mode 100644 index 0000000000..4d3532bfff --- /dev/null +++ b/.changeset/master-detail-reference-alias-measured.md @@ -0,0 +1,63 @@ +--- +"@objectstack/objectql": patch +--- + +fix(objectql): the fourth tolerant alias reader — `master-detail.ts`'s `referenceTo` tolerance recorded with its measurement, and loud where the alias answered (#13543) + +`resolveMasterDetailRelation` accepts the REJECTED alias `referenceTo` beside +the canonical `reference`, and the type beside it stated a population for that +tolerance in one line: *"`referenceTo` is the stored-row spelling."* Nothing in +the tree measured it. This is that measurement, and the tolerance's disposition +after it — the same shape #13541 gave the sibling `controlled_by_parent` reader +in `plugin-security`, arrived at by the same route. + +**The census (whole tree, both spellings counted separately, positive controls +run so no zero comes from a pathspec that matches nothing).** Authored object +declarations: **zero**, both spellings — all 8 `Field.masterDetail(...)` and 132 +`Field.lookup(...)` declarations across `*.object.ts`, `examples/`, +`packages/qa/` and the `create-objectstack` templates go through the +`@objectstack/spec` builders, which emit the canonical key. Stored-metadata +seeds, JSON/YAML fixtures and `metadata-fs` layouts: **zero**, both spellings. +The nine in-tree files that put `referenceTo` on a field def are all reader +pins. Metadata at rest in a live deployment is **NOT MEASURED** — no command in +this repository reaches it, so the zeros are zeros for the tree, not the world. + +**The assertion was wrong, and the correction is the point.** ADR-0087's +`fieldReferenceToAlias` records, in its own docblock, that camelCase +`referenceTo` is deliberately not converted because it "is not the spelling the +objectql runtime wrote into stored object rows" — the stored dialect is +`reference_to`, which this reader does not read. So the line justifying the +tolerance named the wrong spelling, and the docblock now carries the measured +account instead of the assertion. + +**The tolerance still stays, for a reason that survived the census.** A raw +`registerObject` skips Zod by design and every caller of this resolver reads +that same `SchemaRegistry`, so an alias-spelled object reaches here verbatim — +now pinned by a test that registers one and resolves it. And the conversion +layer normalises `reference_to` on stored rehydration and `os migrate meta` +while deliberately leaving `referenceTo` alone, which makes `referenceTo` the +one spelling that is simultaneously unconverted upstream and read here. Two of +this resolver's four callers fail **closed**: an unresolved relation leaves +`parent` unbound and `rule-validator.ts` reads an unbound scope root as LOCKED, +so narrowing would take a raw-registered, alias-spelled detail object from +"lock enforced against its header" to "every `parent`-scoped field permanently +unwritable, writes silently stripped". That is an availability defect, not a +spelling correction. + +**Loud where the alias is what answered.** When the relation resolves from +`referenceTo`, the resolver reports once per object+field+spelling through an +optional `warn` sink defaulting to `console.warn` — the same caller-supplied +callback shape and default as `warnFunctionalCompleteness` in the same package. +Never a throw, no behaviour change: `referenceKeyOf` selects the key with the +same `!= null` test `??` applies, so a present-but-empty `reference` still wins +the read rather than falling through to the alias. The report is once per +distinct defect rather than per write, because this resolver sits on the write +path and a per-write line is a noise defect of its own. The text also corrects +the registration-time `field/relationship-without-reference` diagnostic, which +calls the same field "runtime-DEAD ... never-resolves" — false for this +consumer, and two diagnostics disagreeing about one field is worse than one. + +⛔ Narrowing this reader is not done here and is not licensed by the zeros +above: it is only honest behind a migration that sweeps stored and +raw-registered metadata first. The live-deployment census neither this card nor +its sibling could run is still the open prerequisite.