diff --git a/.changeset/19289-implicit-reference-target-census.md b/.changeset/19289-implicit-reference-target-census.md new file mode 100644 index 00000000000..c2a45ba85d6 --- /dev/null +++ b/.changeset/19289-implicit-reference-target-census.md @@ -0,0 +1,20 @@ +--- +"@objectstack/metadata-protocol": patch +"@objectstack/lint": patch +"@objectstack/rest": patch +--- + +Four consumers of the implicit-reference-target contract resolve a reference field's target through `referenceTargetOf` instead of the materialized `reference` carrier, so a `{ type: 'user' }` field authored without one seeds, serves, and lints as the fully specified metadata the spec says it is (#19289). + +`IMPLICIT_REFERENCE_TARGETS` (`@objectstack/spec/data`) says a `user` field's target is "a CONSTANT OF THE TYPE, so `reference` on a `user` field materializes that constant; it does not supply it. Metadata authored without it (hand-written JSON, an AI author, a Studio form) is **fully specified, not under-specified**." Two arbiters answer two different questions — `referenceCarrierOf` what the carrier says, `referenceTargetOf` what the field points at — and for `user` only the second matches that text. #18550 standardized a population of readers on the first, which is correct wherever a site's own type gate excludes `user` and wrong wherever it does not. This is the census of that population: 17 carrier call sites judged one by one, four repaired. + +Clause-②: no + +Not a widening. It deletes a mistaken refusal of metadata the published contract already declares complete, which the charter files as `no` — 「删已发布契约文本本就否定的误拒本身是 `no`」. No key, alias or spelling is newly accepted anywhere: the target comes from the spec's own constant, never from a second way of writing it. + +- **`@objectstack/rest` — the loud one.** A `publicPicker` on a spec-complete `{ type: 'user' }` field answered `500 LOOKUP_TARGET_MISSING`, so opening a reference picker on a "responsible person" column returned an error page. It now answers `200` over `sys_user`. ⛔ This is not a re-widening of #12920's narrowing: a stored def spelling the target `referenceTo` / `target` / `options.objectName` still resolves nothing and still answers `500`, pinned in both directions. +- **`@objectstack/metadata-protocol` — the silent one, and the one that stored a wrong value.** A seed row's `{ type: 'user' }` field contributed no `dependsOn` edge and never reached `references`, so its natural key was written **verbatim** into a column that holds a record id — the dangling reference `buildDependencyGraph`'s own docblock names as the cause of broken parent joins. ⚠️ Upgrading seed authors: such a field now takes the same path the explicit `reference: 'sys_user'` spelling always took, which includes the failure path — a natural key that resolves to no `sys_user` row now DROPS the whole record, counted, reported and logged at `error`, where it was previously written verbatim. Seed `sys_user` before the referencing object, enable `multiPass`, or fix the key. +- **`@objectstack/lint` — the widest.** `object-graph`'s field slice fed `resolveFieldPath`, whose `RELATIONSHIP_FIELD_TYPES` admits `user`; a carrier-less one answered `hop-untargeted`, which `isUnjudgeable` treats as "the graph could not answer". Every rule in the package that resolves a field path therefore stopped judging any path through such a field, reporting nothing. `validate-field-consumers` separately dropped the `displayField` consumer edge onto `sys_user`, so a field that column displays was reported consumed by nobody. +- **Nothing else widens.** `user` is the only member of `IMPLICIT_REFERENCE_TARGETS`, so a `lookup` / `master_detail` / `tree` whose author-chosen target is absent still names nothing, exactly as before — pinned at every repaired site. +- **The unreadable-carrier behaviour is unchanged.** `referenceTargetOf` reads the carrier through `referenceCarrierOf` **before** it judges the type, so #13053/#18550's `TypeError` on an object- or array-valued `reference` still fires everywhere it fired before. The implicit target is not a fallback that swallows it. +- **No authoring change.** Metadata that already spells `reference: 'sys_user'` resolves to the same target it always did; nobody has to restate the constant, and nobody has to stop restating it. diff --git a/content/docs/ui/forms.mdx b/content/docs/ui/forms.mdx index ca4a78ea958..9977e25ffcf 100644 --- a/content/docs/ui/forms.mdx +++ b/content/docs/ui/forms.mdx @@ -257,7 +257,7 @@ sections: [{ | `displayFields` | Fields projected into each result row (plus `id`); the visitor's `q` is `contains`-matched against the **first** entry. At most 5; omitted → `['name']`. | | `maxResults` | Rows per request, integer 1–50 (default 20). 50 is a hard server ceiling; there is **no pagination** on this surface (`offset` is pinned to 0), so a leaked endpoint cannot enumerate the table. | | `filter` | Static pre-filter rows (same `{ field, operator, value }` dialect as list-view filters), ANDed ahead of the visitor's search. | -| `object` | The object to search. Optional — omit it and the server resolves the target from the field's own definition on the parent object: its `reference` key, and only that key. A stored row spelling the target `referenceTo` / `target` / `options.objectName` is **not** resolved — the route answers `500 LOOKUP_TARGET_MISSING` — because `FieldSchema` accepts no spelling but `reference`. That key is also **read through the one carrier accessor**, so a stored row whose `reference` holds something other than a string (an object, an array) is refused rather than searched — see the error table below. Declare it only to search something other than what the field points at. | +| `object` | The object to search. Optional — omit it and the server resolves the target from the field's own definition on the parent object: its `reference` key, or — for a field type whose target is fixed by the TYPE rather than chosen by the author (`user`, whose constant is `sys_user`) — that constant. Those two are the only sources, and a `user` field authored without `reference` is fully specified, not under-specified (#19289). A stored row spelling the target `referenceTo` / `target` / `options.objectName` is **not** resolved, because `FieldSchema` accepts no spelling but `reference`: on a `lookup` / `master_detail` the route then answers `500 LOOKUP_TARGET_MISSING`, and on a `user` field the type's own constant answers instead — the alias contributes nothing either way. That key is also **read through the one carrier accessor**, so a stored row whose `reference` holds something other than a string (an object, an array) is refused rather than searched — see the error table below. Declare it only to search something other than what the field points at. | Those four keys are the whole block. It admits exactly what the route enforces — an unknown subkey, a 6th display field, or `maxResults: 51` is a **parse diff --git a/packages/lint/src/object-graph.test.ts b/packages/lint/src/object-graph.test.ts index cecbd87e64b..894bde68bb7 100644 --- a/packages/lint/src/object-graph.test.ts +++ b/packages/lint/src/object-graph.test.ts @@ -291,3 +291,95 @@ describe('object-graph — a non-record entry in `stack.objects` (#15494)', () = expect(resolveFieldPath(g, 'b', 'n')).toMatchObject({ kind: 'ok' }); }); }); + +/** + * [#19289] A `{ type: 'user' }` field with no `reference` is TRAVERSABLE — the + * third defect found by the implicit-target census, and the widest-reaching of + * the four. + * + * `RELATIONSHIP_FIELD_TYPES` admits `user`, so `resolveFieldPath` hops through + * one. The slice's `reference` came from `referenceCarrierOf`, which answers + * what the CARRIER says — and `IMPLICIT_REFERENCE_TARGETS` + * (`packages/spec/src/data/field-value.zod.ts`) declares a `user` field's + * target "a CONSTANT OF THE TYPE", with metadata authored without `reference` + * "fully specified, not under-specified". So a spec-complete field answered + * `hop-untargeted`. + * + * ## Why that is the SILENT class, not a false positive + * + * `hop-untargeted` is `isUnjudgeable`, and `describeFieldPathVerdict` returns + * `undefined` for it — "the graph could not answer". Every rule in this package + * that resolves a field PATH therefore STOPPED JUDGING any path through an + * author's "responsible person" column, reporting nothing at all. That is the + * failure mode `isUnjudgeable`'s own docblock says this family exists to end: + * "a missed report is silence". + * + * The repair is not an arbiter swap at the call — `graphFieldOf` synthesized + * `{ reference: def.reference }` and threw `type` away before the arbiter could + * see it. The field is now passed through whole. + */ +describe('[#19289] object-graph — a `user` field takes its target from the TYPE', () => { + const implicitStack = { + objects: [ + { + name: 'crm_task', + fields: { + subject: { type: 'text', label: 'Subject' }, + // Spec-complete: no `reference`, because the type supplies it. + assignee: { type: 'user', label: 'Assignee' }, + // The same field, with the constant materialized by hand. + assignee_explicit: { type: 'user', label: 'Assignee', reference: 'sys_user' }, + // ⛔ The boundary: `lookup` has no constant, so it stays untargeted. + orphan: { type: 'lookup', label: 'Orphan' }, + }, + }, + { name: 'sys_user', fields: { name: { type: 'text' }, email: { type: 'email' } } }, + ], + }; + const implicitGraph = indexObjectGraph(implicitStack); + + it('THE DEFECT: `assignee.email` RESOLVES — it is no longer `hop-untargeted`', () => { + const verdict = resolveFieldPath(implicitGraph, 'crm_task', 'assignee.email'); + expect(verdict).toMatchObject({ kind: 'ok', object: 'sys_user', field: 'email' }); + // The load-bearing half: an unjudgeable verdict is what made every + // consuming rule fall silent, so this is what actually ended. + expect(isUnjudgeable(verdict)).toBe(false); + }); + + it('the two legal spellings of one fully-specified field resolve identically', () => { + expect(resolveFieldPath(implicitGraph, 'crm_task', 'assignee.email')) + .toEqual(resolveFieldPath(implicitGraph, 'crm_task', 'assignee_explicit.email')); + }); + + it('a MISS through the implicit hop is now REPORTED, where it used to be swallowed', () => { + // The other direction, and the one that proves judgement resumed rather + // than merely changing shape: a typo'd leaf beyond the hop produces a real + // finding instead of silence. + const verdict = resolveFieldPath(implicitGraph, 'crm_task', 'assignee.emial'); + expect(verdict).toMatchObject({ kind: 'field-unknown', object: 'sys_user', field: 'emial' }); + expect(isUnjudgeable(verdict)).toBe(false); + }); + + it('control: `lookup` with no carrier is STILL `hop-untargeted` — only `user` has a constant', () => { + const verdict = resolveFieldPath(implicitGraph, 'crm_task', 'orphan.x'); + expect(verdict).toMatchObject({ kind: 'hop-untargeted' }); + expect(isUnjudgeable(verdict)).toBe(true); + }); + + it('control: an EXPLICIT carrier still wins over the constant', () => { + // `reference` MATERIALIZES the constant for `user`; where an author named a + // different object the arbiter returns what they wrote, unchanged. The + // module fixture above relies on this (`owner` → `crm_person`). + expect(resolveFieldPath(graph, 'crm_opportunity', 'account.owner.email')) + .toMatchObject({ kind: 'ok', object: 'crm_person', field: 'email' }); + }); + + it('control: an UNREADABLE carrier still REFUSES at index time', () => { + // `referenceTargetOf` reads the carrier through `referenceCarrierOf` before + // it judges the type, so #13053's refusal is untouched — the implicit + // target is not a fallback that swallows a broken carrier. + const broken = { objects: [{ name: 'crm_task', fields: { assignee: { type: 'user', reference: { object: 'sys_user' } } } }] }; + expect(() => indexObjectGraph(broken)).toThrow(TypeError); + expect(() => indexObjectGraph(broken)).toThrow(/`reference` is an object/); + }); +}); diff --git a/packages/lint/src/object-graph.ts b/packages/lint/src/object-graph.ts index 70975d983c7..bac2db6f0d9 100644 --- a/packages/lint/src/object-graph.ts +++ b/packages/lint/src/object-graph.ts @@ -68,7 +68,7 @@ * second question about it is still unanswered — truthfully, and only there. */ -import { referenceCarrierOf } from '@objectstack/spec/data'; +import { referenceTargetOf } from '@objectstack/spec/data'; import { injectedColumnDefsFor, injectedColumnsFor } from './system-fields.js'; @@ -239,7 +239,26 @@ function graphFieldOf(def: AnyRec): GraphField { // ⛔ NOT `strName` here. A carrier in a shape no reader can read is refused // rather than narrowed to `undefined` (#13053): every rule downstream reads // this slice, so a silent narrowing here is that blindness wholesaled. - reference: referenceCarrierOf({ reference: def.reference }, 'object-graph graphFieldOf'), + // `referenceTargetOf` reads the carrier through `referenceCarrierOf` before + // it judges anything, so that refusal is unchanged. + // + // [#19289] The whole DEFINITION is passed through, and the arbiter is + // `referenceTargetOf` — the question this slice answers is "what does this + // field point at", ⛔ not "what does its carrier say", and for `user` the + // two differ. `RELATIONSHIP_FIELD_TYPES` above admits `user`, so + // {@link resolveFieldPath} traverses one — and a spec-complete + // `{ type: 'user' }` field (`IMPLICIT_REFERENCE_TARGETS` declares its target + // a CONSTANT OF THE TYPE, such metadata "fully specified, not + // under-specified") read as `hop-untargeted`, which + // {@link isUnjudgeable} treats as "the graph could not answer". Every rule + // that resolves a path through an author's "responsible person" column + // therefore STOPPED JUDGING IT, silently, across this package — the failure + // mode the verdict union's own docblock says this family exists to end. + // + // Reading `def` whole rather than `{ reference: def.reference }` is what + // makes the target question askable at all: the synthesized literal threw + // `type` away before the arbiter could see it. + reference: referenceTargetOf(def), multiple: def.multiple === true ? true : undefined, }; } diff --git a/packages/lint/src/validate-field-consumers.test.ts b/packages/lint/src/validate-field-consumers.test.ts index e9a4199e751..53482b8672a 100644 --- a/packages/lint/src/validate-field-consumers.test.ts +++ b/packages/lint/src/validate-field-consumers.test.ts @@ -513,7 +513,9 @@ describe('validateFieldConsumers — an unreadable `reference` carrier is refuse it('an OBJECT-valued carrier REFUSES — ⛔ not a silent missing edge', () => { const run = () => validateFieldConsumers(stackWith({ reference: { object: 'crm_account' } })); expect(run).toThrow(TypeError); - expect(run).toThrow(/validate-field-consumers walkObject/); + // [#19289] `walkObject` asks `referenceTargetOf`, which reads the carrier + // through `referenceCarrierOf` and so names itself in the refusal. + expect(run).toThrow(/referenceTargetOf/); expect(run).toThrow(/`reference` is an object/); expect(run).toThrow(/FieldSchema declares it as an optional STRING/); }); @@ -530,3 +532,60 @@ describe('validateFieldConsumers — an unreadable `reference` carrier is refuse expect(findings.map((f) => f.path)).toContain('objects[0].fields.legal_name'); }); }); + +/** + * [#19289] The `displayField` consumer edge of a `{ type: 'user' }` field lands + * on `sys_user` even when no `reference` is written — the fourth defect of the + * implicit-target census. + * + * This walk has NO type gate, so a `user` field reaches it, and the read went + * through `referenceCarrierOf` — what the CARRIER says. + * `IMPLICIT_REFERENCE_TARGETS` (`packages/spec/src/data/field-value.zod.ts`) + * declares a `user` field's target "a CONSTANT OF THE TYPE", with metadata + * authored without `reference` "fully specified, not under-specified", so the + * carrier answered `undefined` and the edge onto `sys_user.` was + * never recorded. The field that column DOES display was then reported + * consumed by nobody — the same silent under-record #19198 and #19264 repaired + * at their own consumers. + * + * ⛔ Materiality, stated so the pin is not read wider than it is: the edge is + * only recordable where `sys_user` is compiled INTO the linted stack, which is + * what this fixture arranges. Where it is not, the ledger never declared the + * target and the outcome is unchanged. + * + * The repair is not an arbiter swap at the call — the synthesized + * `{ reference: field.reference }` literal threw `type` away before the arbiter + * could see it. The field is now passed through whole. + */ +describe('[#19289] validateFieldConsumers — a `user` field displays a field on `sys_user`', () => { + const stackWithUser = (assignee: AnyRec): AnyRec => ({ + objects: [ + { name: 'sys_user', fields: { name: { type: 'text' }, full_name: { type: 'text' } } }, + { + name: 'crm_task', + fields: { + name: { type: 'text' }, + assignee: { type: 'user', displayField: 'full_name', ...assignee }, + }, + }, + ], + views: [{ name: 'task_list', object: 'crm_task', viewKind: 'list', columns: ['name'] }], + }); + + /** Paths this rule reports — the displayed field appearing here IS the defect. */ + const pathsFor = (assignee: AnyRec) => validateFieldConsumers(stackWithUser(assignee)).map((f) => f.path); + + it('THE DEFECT: with no `reference`, `sys_user.full_name` is no longer reported as consumed by nobody', () => { + expect(pathsFor({})).not.toContain('objects[0].fields.full_name'); + }); + + it('the two legal spellings of one fully-specified field record the same edge', () => { + expect(pathsFor({})).toEqual(pathsFor({ reference: 'sys_user' })); + }); + + it('control: an UNREADABLE carrier still REFUSES — the implicit target does not swallow it', () => { + const run = () => validateFieldConsumers(stackWithUser({ reference: { object: 'sys_user' } })); + expect(run).toThrow(TypeError); + expect(run).toThrow(/`reference` is an object/); + }); +}); diff --git a/packages/lint/src/validate-field-consumers.ts b/packages/lint/src/validate-field-consumers.ts index 09f2a6dde64..31af2efbd3e 100644 --- a/packages/lint/src/validate-field-consumers.ts +++ b/packages/lint/src/validate-field-consumers.ts @@ -121,7 +121,7 @@ import { deriveFieldGroupLayout, resolveDisplayField } from '@objectstack/spec/data'; import type { DisplayNameObjectMeta } from '@objectstack/spec/data'; -import { referenceCarrierOf } from '@objectstack/spec/data'; +import { referenceTargetOf } from '@objectstack/spec/data'; import { collectionEntries } from './collection-entries.js'; import { recordsOf } from './object-graph.js'; import { injectedColumnsFor } from './system-fields.js'; @@ -550,14 +550,26 @@ function walkObject(ledger: ConsumerLedger, obj: AnyRec, objectName: string, obj walk(ledger, value, objectName, 'objects', `${objPath}.${key}`, [key], key); } for (const { rec: field, path: fieldPath } of collectionEntries(obj.fields, fieldsPath)) { - // [#18550] The carrier through the ONE arbiter: `strName` answered + // [#18550] The target through the ONE arbiter: `strName` answered // `undefined` for an unreadable one exactly as it does for an absent one, // so the `displayField` consumer edge below was never recorded and the // ledger under-reported — a field a lookup DOES display read as unused. - // Absence still answers `undefined` and records nothing. - // Same form as the sibling lint readers: the literal `.reference` read - // stays at the site, only the shape judgment moves to the arbiter. - const reference = referenceCarrierOf({ reference: field.reference }, 'validate-field-consumers walkObject'); + // Absence still answers `undefined` and records nothing, and an unreadable + // carrier still REFUSES (`referenceTargetOf` reads it through + // `referenceCarrierOf` before it judges anything). + // + // [#19289] The whole FIELD is passed through and the arbiter is + // `referenceTargetOf`, ⛔ not `referenceCarrierOf`. There is no type gate + // here, so a `{ type: 'user', displayField: … }` field reaches this line — + // and for `user` the carrier is not the target + // (`IMPLICIT_REFERENCE_TARGETS`: a CONSTANT OF THE TYPE, such metadata + // "fully specified, not under-specified"). Reading the carrier dropped the + // edge to `sys_user.` wherever `sys_user` is compiled into + // the linted stack, so a field that column DOES display was reported unused + // — the same silent under-record #19198 and #19264 repaired elsewhere. + // The synthesized `{ reference: field.reference }` literal is what made the + // target question unaskable: it threw `type` away before the arbiter saw it. + const reference = referenceTargetOf(field); const displayField = strName(field.displayField); if (reference && displayField && ledger.declares(reference, displayField)) { ledger.record(reference, displayField, { root: 'objects', path: `${fieldPath}.displayField`, kind: 'display' }); diff --git a/packages/lint/src/validate-object-references.test.ts b/packages/lint/src/validate-object-references.test.ts index 7bc41bd80a5..831c430ea0d 100644 --- a/packages/lint/src/validate-object-references.test.ts +++ b/packages/lint/src/validate-object-references.test.ts @@ -561,7 +561,9 @@ describe('validateObjectReferences — an unreadable `reference` carrier is refu it('an OBJECT-valued FIELD carrier REFUSES — ⛔ not silence, and ⛔ not "unknown object"', () => { const run = () => validateObjectReferences(fieldCarrier({ reference: { object: 'crm_account' } })); expect(run).toThrow(TypeError); - expect(run).toThrow(/validate-object-references field target/); + // [#19289] The reader is now `referenceTargetOf` — see the field-target + // site for why this rule asks the TARGET question, not the carrier one. + expect(run).toThrow(/referenceTargetOf/); expect(run).toThrow(/`reference` is an object/); expect(run).toThrow(/FieldSchema declares it as an optional STRING/); }); @@ -594,3 +596,77 @@ describe('validateObjectReferences — an unreadable `reference` carrier is refu expect(validateObjectReferences(paramCarrier(carrier))).toHaveLength(0); }); }); + +/** + * [#19289] The FIELD target comes from the type; the ACTION-PARAM target + * cannot, and the two halves of this file's census verdict are deliberately + * different. + * + * `RELATIONSHIP_TARGET_FIELD_TYPES` admits `user`, so the field site DOES ask + * the target question about a `user` field — and answered it from the carrier, + * which for that type is not the target (`IMPLICIT_REFERENCE_TARGETS`: a + * CONSTANT OF THE TYPE, `packages/spec/src/data/field-value.zod.ts`). The + * OUTPUT was already right, for the wrong reason: absence answered `undefined` + * and `check` returns early, which is the same silence the resolved target + * produces, since `sys_user` is admitted at rung ③ + * (`isPlatformProvidedObjectName`). The field site now reaches that silence by + * the same route for both legal spellings. + * + * ⛔ The PARAM site stays on the carrier, and the last case here is why. + * `ActionParamSchema.type` is OPTIONAL — a field-backed param inherits its + * type at runtime — so the target question is unanswerable from a param alone, + * and asking it discards every carrier whose param declares no type. Nothing + * is owed there in the other direction either: a carrier-less `user` param + * already produces no finding. + * + * ⛔ No new finding is introduced for any spelling — an assertion that one + * appeared would be a regression, not a fix. + */ +describe('[#19289] validateObjectReferences — a `user` target comes from the TYPE', () => { + const userField = (extra: Record) => ({ + ...baseStack(), + objects: [ + ...baseStack().objects, + { name: 'crm_task', fields: { name: { type: 'text' }, assignee: { type: 'user', ...extra } } }, + ], + }); + const userParam = (extra: Record) => ({ + ...baseStack(), + actions: [{ name: 'mass_reassign', params: [{ name: 'owner', type: 'user', ...extra }] }], + }); + + it('a `user` FIELD with no `reference` produces no finding — the target is a constant of the type', () => { + expect(validateObjectReferences(userField({}))).toHaveLength(0); + }); + + it('a `user` field and the same field with `reference: "sys_user"` agree exactly', () => { + expect(validateObjectReferences(userField({}))).toEqual(validateObjectReferences(userField({ reference: 'sys_user' }))); + }); + + it('a `user` PARAM with no `reference` produces no finding either — `check` returns early on absence', () => { + expect(validateObjectReferences(userParam({}))).toHaveLength(0); + }); + + it('⛔ REGRESSION GUARD: a param with NO `type` still has its carrier checked', () => { + // The case that refused the param-site swap, kept here beside the verdict + // it produced. `reference-integrity-suite.test.ts` carries the same shape + // in its corpus (`{ name: 'owner', reference: 'user' }` — 'user' being the + // classic miss for 'sys_user'); asking `referenceTargetOf` here answered + // `undefined` for it and `object-reference-unknown` vanished from that + // suite's findings altogether. + const findings = validateObjectReferences({ + ...baseStack(), + actions: [{ name: 'mass_reassign', params: [{ name: 'owner', reference: 'zzz_nope' }] }], + }); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN); + }); + + it('control: the rule still REPORTS a real miss — an explicit carrier naming an unknown object errors', () => { + // Without this, "no findings" above reads equally well as a rule that + // stopped judging `user` fields altogether. + const findings = validateObjectReferences(userField({ reference: 'zzz_nope' })); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN); + }); +}); diff --git a/packages/lint/src/validate-object-references.ts b/packages/lint/src/validate-object-references.ts index 7bc785e4874..befd1c7517b 100644 --- a/packages/lint/src/validate-object-references.ts +++ b/packages/lint/src/validate-object-references.ts @@ -79,7 +79,7 @@ import { isPlatformProvidedObjectName, PLATFORM_PROVIDED_OBJECT_NAMES, } from '@objectstack/spec/system'; -import { referenceCarrierOf } from '@objectstack/spec/data'; +import { referenceCarrierOf, referenceTargetOf } from '@objectstack/spec/data'; import { recordsOf, suggestName } from './object-graph.js'; @@ -301,9 +301,24 @@ export function validateObjectReferences(stack: AnyRec): ObjectRefFinding[] { // not this rule's unknown-object error either. Absence still answers // `undefined` and this rule still, deliberately, says nothing about it — // an absent target is `field/relationship-without-reference`'s subject, - // not this rule's. + // not this rule's. An unreadable carrier still REFUSES: + // `referenceTargetOf` reads it through `referenceCarrierOf` first. + // + // [#19289] The whole FIELD is passed through and the arbiter is + // `referenceTargetOf`. `RELATIONSHIP_TARGET_FIELD_TYPES` admits `user`, + // so this rule DOES ask the target question about a `user` field — and + // answered it from the carrier, which for that type is not the target + // (`IMPLICIT_REFERENCE_TARGETS`: a CONSTANT OF THE TYPE). The OUTPUT was + // right for the wrong reason: a spec-complete `{ type: 'user' }` field + // answered `undefined` and `check` returned early, so the two legal + // spellings of one fully-specified field — with `reference: 'sys_user'` + // and without — took different paths to the same silence. They now take + // the same path: the target resolves to `sys_user`, which + // `isPlatformProvidedObjectName` admits at rung ③. ⛔ No new finding is + // produced for either spelling; what ends is the latent misread waiting + // for the day absence stops returning early. check( - referenceCarrierOf({ reference: field.reference }, 'validate-object-references field target'), + referenceTargetOf(field), `object "${objName}" · field "${fieldName}"`, `objects[${oi}].fields.${fieldName}.reference`, `${type} target`, @@ -325,6 +340,22 @@ export function validateObjectReferences(stack: AnyRec): ObjectRefFinding[] { // `ActionParamSchema.reference`, whose own docblock says the key name // "deliberately mirrors `FieldSchema.reference` so the same spelling" // carries the target object's name — one contract, so one reader. + // + // ⛔ [#19289] This site STAYS on the carrier, and the census judged it + // rather than missing it. A param is NOT a field definition: + // `ActionParamSchema.type` is OPTIONAL, because a field-backed param + // inherits its type from the referenced field at runtime — "not visible + // at parse time", as that schema's own refinement comment says. So the + // target question is UNANSWERABLE from a param alone, and asking it + // discards every carrier whose param declares no type. + // + // Measured, ⛔ not reasoned: swapping this one arbiter deleted a live + // check. `reference-integrity-suite.test.ts` holds a corpus param + // `{ name: 'owner', reference: 'user' }` — no `type`, and `user` is the + // classic miss for `sys_user` — and `object-reference-unknown` vanished + // from the suite's findings entirely. Nothing is owed here either way: a + // `user` param that omits the carrier already produces NO finding, + // because `check` returns early on absence. check( referenceCarrierOf({ reference: param.reference }, 'validate-object-references action param target'), where, diff --git a/packages/metadata-protocol/src/seed-loader-implicit-reference-target.test.ts b/packages/metadata-protocol/src/seed-loader-implicit-reference-target.test.ts new file mode 100644 index 00000000000..bbb3cfc7ce1 --- /dev/null +++ b/packages/metadata-protocol/src/seed-loader-implicit-reference-target.test.ts @@ -0,0 +1,122 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#19289] `buildDependencyGraph` must resolve a `user` field's target from the + * TYPE, not from the carrier — the first of the two CONFIRMED defects of the + * implicit-target census. + * + * `IMPLICIT_REFERENCE_TARGETS` (`packages/spec/src/data/field-value.zod.ts`) + * declares a `user` field's target "a CONSTANT OF THE TYPE" and metadata + * authored without `reference` "fully specified, not under-specified". This + * function's type gate admits `user`, so it DOES ask the target question about + * such a field — and #18550 standardized the read on `referenceCarrierOf`, + * which answers what the CARRIER says. For `lookup` / `master_detail` the two + * arbiters agree; for `user` only `referenceTargetOf` matches the contract. + * + * ## The failure was SILENT, and its cost is a wrong stored value + * + * A spec-complete `{ type: 'user' }` field answered `undefined`, hit the + * `if (!targetObject) continue`, and so contributed NO `dependsOn` edge and was + * NEVER pushed onto `references`. Nothing threw and nothing logged. Resolution + * only maps the natural keys that reached `references`, so the seed value was + * written VERBATIM — `'Ada Lovelace'` stored in a column that holds a record id + * — which this function's own docblock names as the cause of dangling + * references and broken parent joins. + * + * ⛔ These assertions are on the `references` ROW, not on a throw: the defect + * never threw, so a `toThrow` pin could not have caught it and cannot guard it. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { SeedLoaderService } from './seed-loader.js'; +import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; + +function createLogger() { + return { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; +} + +/** The graph builder reads metadata only — `getSchema` is the whole surface. */ +function engineWith(schemas: Record): IDataEngine { + return { getSchema: vi.fn((name: string) => schemas[name]) } as unknown as IDataEngine; +} + +function emptyMetadata(): IMetadataService { + return { getObject: vi.fn(async () => undefined) } as unknown as IMetadataService; +} + +const account = { name: 'crm_account', fields: { name: { type: 'text', required: true } } }; + +/** One `crm_task` whose `owner` field is whatever the case supplies. */ +const taskWith = (owner: Record) => ({ + name: 'crm_task', + fields: { + name: { type: 'text', required: true }, + owner, + }, +}); + +const graphOver = (schemas: Record, names: string[] = ['crm_account', 'crm_task']) => + new SeedLoaderService(engineWith(schemas), emptyMetadata(), createLogger()).buildDependencyGraph(names); + +const referencesOf = async (schemas: Record, names?: string[]) => + (await graphOver(schemas, names)).nodes.find((n) => n.object === 'crm_task')?.references ?? []; + +describe('[#19289] seed dependency graph — a `user` field takes its target from the TYPE', () => { + it('a `{ type: "user" }` field with NO `reference` still produces a reference row targeting `sys_user`', async () => { + // THE DEFECT, stated as an assertion. Before the repair this array was + // empty and the seed value was written verbatim. + const references = await referencesOf({ crm_account: account, crm_task: taskWith({ type: 'user' }) }); + expect(references).toHaveLength(1); + expect(references[0]).toMatchObject({ + field: 'owner', + targetObject: 'sys_user', + targetField: 'name', + fieldType: 'user', + }); + }); + + it('the two legal spellings of one fully-specified field agree', async () => { + // The contract's point: `reference: 'sys_user'` MATERIALIZES the constant, + // it does not supply it — so writing it and omitting it are the same + // metadata and must resolve the same way. + const implicit = await referencesOf({ crm_account: account, crm_task: taskWith({ type: 'user' }) }); + const explicit = await referencesOf({ + crm_account: account, + crm_task: taskWith({ type: 'user', reference: 'sys_user' }), + }); + expect(implicit).toEqual(explicit); + }); + + it('the `dependsOn` edge appears when `sys_user` is itself in the graph', async () => { + // The other half the carrier read dropped: ordering. `dependsOn` is only + // recorded for objects inside the seeded set, so this case puts it there. + const graph = await graphOver( + { + sys_user: { name: 'sys_user', fields: { name: { type: 'text' } } }, + crm_task: taskWith({ type: 'user' }), + }, + ['sys_user', 'crm_task'], + ); + expect(graph.nodes.find((n) => n.object === 'crm_task')?.dependsOn).toEqual(['sys_user']); + expect(graph.insertOrder.indexOf('sys_user')).toBeLessThan(graph.insertOrder.indexOf('crm_task')); + }); + + // ── The boundary. `user` is the ONLY member of `IMPLICIT_REFERENCE_TARGETS`, + // so no other type may start inventing a target. ⛔ These are the cases a + // mechanical "swap every arbiter" sweep would break. + it.each([ + ['lookup', { type: 'lookup' }], + ['master_detail', { type: 'master_detail' }], + ])('%s with no carrier still names NO target — nothing supplies one for it', async (_label, owner) => { + expect(await referencesOf({ crm_account: account, crm_task: taskWith(owner) })).toEqual([]); + }); + + it('a `user` field whose carrier is unreadable still REFUSES — absence and unreadability stay different', async () => { + // `referenceTargetOf` reads the carrier through `referenceCarrierOf` before + // it judges anything, so #18550's refusal is untouched: the implicit target + // is NOT a fallback that swallows a broken carrier. + const attempt = () => graphOver({ crm_account: account, crm_task: taskWith({ type: 'user', reference: { object: 'sys_user' } }) }); + await expect(attempt()).rejects.toThrow(TypeError); + await expect(attempt()).rejects.toThrow(/`reference` is an object/); + }); +}); diff --git a/packages/metadata-protocol/src/seed-loader-reference-carrier.test.ts b/packages/metadata-protocol/src/seed-loader-reference-carrier.test.ts index 126e4ef412d..4cc9e8bd6ff 100644 --- a/packages/metadata-protocol/src/seed-loader-reference-carrier.test.ts +++ b/packages/metadata-protocol/src/seed-loader-reference-carrier.test.ts @@ -77,7 +77,13 @@ describe('[#18550] seed dependency graph — an unreadable `reference` carrier i // other input would satisfy that. const attempt = () => graphOver({ author, book: bookWith({ reference: { object: 'author' } }) }); await expect(attempt()).rejects.toThrow(TypeError); - await expect(attempt()).rejects.toThrow(/SeedLoader\.buildDependencyGraph/); + // [#19289] The reader named in the refusal is now `referenceTargetOf`, the + // arbiter this site asks (the carrier read happens INSIDE it, so the throw + // and its prescription are unchanged). The assertions that carry this + // case's weight are the three below: the error CLASS, the offending SHAPE + // and the prescription. What is deliberately NOT weakened is the input — + // this call reaches the arbiter only through `buildDependencyGraph`. + await expect(attempt()).rejects.toThrow(/referenceTargetOf/); await expect(attempt()).rejects.toThrow(/`reference` is an object/); await expect(attempt()).rejects.toThrow(/FieldSchema declares it as an optional STRING/); }); diff --git a/packages/metadata-protocol/src/seed-loader.ts b/packages/metadata-protocol/src/seed-loader.ts index ecedc1f0a4c..07c291dd54b 100644 --- a/packages/metadata-protocol/src/seed-loader.ts +++ b/packages/metadata-protocol/src/seed-loader.ts @@ -13,7 +13,7 @@ import type { SeedLoadResultParsed, Seed, } from '@objectstack/spec/data'; -import { SeedLoaderConfigSchema, isMultiValueField, referenceCarrierOf } from '@objectstack/spec/data'; +import { SeedLoaderConfigSchema, isMultiValueField, referenceTargetOf } from '@objectstack/spec/data'; import { SEED_WRITE_EXECUTION_CONTEXT } from '@objectstack/spec/kernel'; import { resolveSeedRecord } from '@objectstack/formula'; import { bulkWrite, withTransientRetry, defaultIsTransientError, type BulkWriteRowResult, runWithAdvisoryAggregation, type AdvisoryGroup } from '@objectstack/core'; @@ -697,19 +697,33 @@ export class SeedLoaderService implements ISeedLoaderService { const fields = objDef.fields as Record; for (const [fieldName, fieldDef] of Object.entries(fields)) { if (fieldDef.type === 'lookup' || fieldDef.type === 'master_detail' || fieldDef.type === 'user') { - // [#18550] The carrier goes through the ONE arbiter, which also + // [#18550] The read goes through the ONE arbiter, which also // retires the `as string` cast this read used to carry — the cast // asserted exactly what the truthiness test had not checked, so an // object-valued carrier became a `targetObject` that matched no // name in `objectSet`, contributed no `dependsOn` edge, and was // then pushed onto `references` for resolution to make of what it - // could. ABSENCE is unchanged: `undefined` / `null` / `''` answer - // `undefined` and the field is skipped, which is what a relational - // field naming no target means. The type gate stays FIRST so the - // set of fields whose carrier is read here is byte-identical to - // before — a `text` field carrying a stray `reference` is still - // never read, and so still never refused. - const targetObject = referenceCarrierOf(fieldDef, 'SeedLoader.buildDependencyGraph'); + // could. An unreadable carrier still REFUSES here: `referenceTargetOf` + // reads the carrier through `referenceCarrierOf` before it judges + // anything, so that throw is unchanged. + // + // [#19289] The arbiter is `referenceTargetOf`, ⛔ not + // `referenceCarrierOf`. This gate admits `user`, and for a `user` + // field the carrier is NOT the target: `IMPLICIT_REFERENCE_TARGETS` + // declares the target a CONSTANT OF THE TYPE (`sys_user`) and such + // metadata "fully specified, not under-specified". Reading the + // carrier made a spec-complete `{ type: 'user' }` field contribute no + // `dependsOn` edge and never reach `references`, so its natural key + // was written VERBATIM instead of resolved to a record id — the + // dangling reference this function's own docblock below names as the + // cause of broken parent joins, arrived at silently. + // + // ABSENCE keeps its meaning for the types that have no constant: + // a `lookup` / `master_detail` with no carrier still answers + // `undefined` and is still skipped, which is what a relational field + // naming no target means. The type gate stays FIRST, so a `text` + // field carrying a stray `reference` is still never read. + const targetObject = referenceTargetOf(fieldDef); if (!targetObject) continue; // Track dependency ordering only for objects within the graph diff --git a/packages/rest/src/public-form-lookup-picker.test.ts b/packages/rest/src/public-form-lookup-picker.test.ts index 79211ec2624..e9af9dfd688 100644 --- a/packages/rest/src/public-form-lookup-picker.test.ts +++ b/packages/rest/src/public-form-lookup-picker.test.ts @@ -634,3 +634,103 @@ describe('#18550 an UNREADABLE `reference` carrier is refused, not reported as a expect(findData.mock.calls[0][0].object).toBe('sys_user'); }); }); + +/** + * [#19289] A `publicPicker` on a spec-complete `{ type: 'user' }` field must be + * ANSWERED — the second of the two CONFIRMED defects of the implicit-target + * census, and the LOUD one that graded this card `p1`. + * + * `IMPLICIT_REFERENCE_TARGETS` (`packages/spec/src/data/field-value.zod.ts`) + * declares a `user` field's target "a CONSTANT OF THE TYPE" (`sys_user`) and + * metadata authored without `reference` "fully specified, not + * under-specified". This read has no type gate at all — it resolves whatever + * field the picker names — so a `user` field reaches it, and #18550 pointed it + * at `referenceCarrierOf`, which answers what the CARRIER says. The carrier is + * absent on such a field, so the route answered + * `500 LOOKUP_TARGET_MISSING`: opening a reference picker on a "responsible + * person" column returned an error page for metadata the published contract + * already calls complete. + * + * ⛔ This is NOT a re-widening of #12920's narrowing, and the controls below + * are what say so rather than the prose: the rejected aliases still resolve + * NOTHING, and the alias suite above still passes unchanged. What is deleted + * is a mistaken REFUSAL of metadata the contract declares complete — the + * target comes from the spec's own constant, never from a second spelling. + */ +describe('#19289 a `user` field needs no carrier — the picker answers instead of 500ing', () => { + const NO_OBJECT_PICKER = { displayFields: ['name', 'email'], maxResults: 10 }; + const savedWithoutObject = () => persistedBody(studioForm([{ field: 'owner', publicPicker: NO_OBJECT_PICKER }])); + const ownerDefIs = (ownerDef: unknown) => ({ ...leadObject, fields: { ...leadObject.fields, owner: ownerDef } }); + + const answerFor = async (ownerDef: unknown) => { + const stored = await savedWithoutObject(); + const { findData, lookup } = routesOver( + stored, + [{ id: 'usr_1', name: 'Ada', email: 'ada@example.com' }], + ownerDefIs(ownerDef), + ); + const res = mockRes(); + await lookup.handler({ params: { slug: 'contact', field: 'owner' }, query: {} } as any, res); + return { res, findData }; + }; + + it('THE DEFECT: `{ type: "user" }` with no `reference` answers 200 over `sys_user`, not 500', async () => { + const { res, findData } = await answerFor({ type: 'user', label: 'Owner' }); + expect(res.body.code).not.toBe('LOOKUP_TARGET_MISSING'); + expect(res.statusCode).toBe(200); + // ⛔ Not just "no longer 500": the route must query the object the TYPE + // names. A 200 over the wrong object is the same defect wearing a + // success code. + expect(findData).toHaveBeenCalledTimes(1); + expect(findData.mock.calls[0][0].object).toBe('sys_user'); + }); + + it('the two legal spellings of one fully-specified field answer identically', async () => { + // `reference: 'sys_user'` MATERIALIZES the constant, it does not supply + // it, so writing it and omitting it are the same metadata. + const implicit = await answerFor({ type: 'user', label: 'Owner' }); + const explicit = await answerFor({ type: 'user', reference: 'sys_user', label: 'Owner' }); + expect(implicit.res.statusCode).toBe(explicit.res.statusCode); + expect(implicit.findData.mock.calls[0][0].object).toBe(explicit.findData.mock.calls[0][0].object); + }); + + // ── The boundary: `user` is the ONLY member of `IMPLICIT_REFERENCE_TARGETS`. + it.each([ + ['lookup', 'lookup'], + ['master_detail', 'master_detail'], + ])('control: a carrier-less `%s` is STILL LOOKUP_TARGET_MISSING — nothing supplies a target for it', async (_l, type) => { + const { res, findData } = await answerFor({ type, label: 'Owner' }); + expect(res.statusCode).toBe(500); + expect(res.body.code).toBe('LOOKUP_TARGET_MISSING'); + expect(findData).not.toHaveBeenCalled(); + }); + + it('control: the #12920 narrowing HOLDS — a `user` field spelling the target `referenceTo` resolves it from the TYPE, never from the alias', async () => { + // The discriminating case. `referenceTo: 'zzz_aliased_object'` is a + // rejected alias: if it were being folded, `findData` would be asked + // for `zzz_aliased_object`. It is asked for `sys_user` — the type's own + // constant — so the alias contributed NOTHING. + const { res, findData } = await answerFor({ type: 'user', referenceTo: 'zzz_aliased_object', label: 'Owner' }); + expect(res.statusCode).toBe(200); + expect(findData.mock.calls[0][0].object).toBe('sys_user'); + expect(findData.mock.calls[0][0].object).not.toBe('zzz_aliased_object'); + }); + + it('control: a `lookup` spelling the target `referenceTo` STILL resolves nothing — the alias is still refused', async () => { + const { res, findData } = await answerFor({ type: 'lookup', referenceTo: 'zzz_aliased_object', label: 'Owner' }); + expect(res.statusCode).toBe(500); + expect(res.body.code).toBe('LOOKUP_TARGET_MISSING'); + expect(findData).not.toHaveBeenCalled(); + }); + + it('control: an UNREADABLE carrier on a `user` field still REFUSES — the implicit target is not a fallback that swallows it', async () => { + // `referenceTargetOf` reads the carrier through `referenceCarrierOf` + // BEFORE it judges the type, so #18550's refusal is untouched: a broken + // carrier does not quietly become `sys_user`. + const { res, findData } = await answerFor({ type: 'user', reference: { object: 'sys_user' }, label: 'Owner' }); + expect(res.statusCode).toBe(500); + expect(res.body.code).toBe('INTERNAL_ERROR'); + expect(res.body.code).not.toBe('LOOKUP_TARGET_MISSING'); + expect(findData).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index f998125e7f6..a5a7ed3dbc1 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -157,7 +157,7 @@ import { isApiOperationAllowed, API_PRIMITIVES, DATA_ACTION_TO_API_OPERATION, - referenceCarrierOf, + referenceTargetOf, } from '@objectstack/spec/data'; // [#8013] The SHARED envelope writer (#3973), aliased. [#9098] The alias no // longer exists to dodge a NAME collision — the local responder this used to @@ -10902,16 +10902,37 @@ export class RestServer { // `catch`, through the one arbiter — see there for why it moved. fieldDef = obj?.fields?.[fieldName]; } catch {/* ignore */} - // ABSENCE stays silent and unchanged: `undefined` / - // `null` / `''` all answer `undefined`, so the route - // falls to the `LOOKUP_TARGET_MISSING` refusal below - // exactly as before. UNREADABILITY throws past this - // handler's outer `catch`, which classifies and LOGS it - // (`mapDataError` + `logError`) rather than reporting a - // missing target — and it also stops an object-valued - // carrier from being forwarded as `query.object` into - // `findData`, which is what it did before this change. - referenceObject = referenceCarrierOf(fieldDef, 'REST public-form lookup picker'); + // [#19289] The arbiter is `referenceTargetOf`, ⛔ not + // `referenceCarrierOf`. This read has NO type gate — it + // resolves whatever field the picker names — so a + // `{ type: 'user' }` field reaches it, and for that type + // the carrier is not the target: + // `IMPLICIT_REFERENCE_TARGETS` declares it a CONSTANT OF + // THE TYPE (`sys_user`) and metadata authored without + // `reference` "fully specified, not under-specified". + // Reading the carrier answered a spec-complete field + // `500 LOOKUP_TARGET_MISSING`, so opening the picker on + // a "responsible person" column returned an error page. + // ⛔ This is NOT a re-widening of the #12920 narrowing + // below: no alias is re-admitted and no `??` chain + // returns. `referenceTargetOf` reads the canonical key + // through `referenceCarrierOf` and supplies the type's + // own constant only where the spec declares one — a + // stored def spelling the target `referenceTo` / + // `target` / `options.objectName` still resolves NOTHING + // here and still answers `500`. + // + // ABSENCE stays silent and unchanged for the types that + // have no constant: a `lookup` / `master_detail` with + // `undefined` / `null` / `''` still answers `undefined`, + // so the route falls to the `LOOKUP_TARGET_MISSING` + // refusal below exactly as before. UNREADABILITY throws + // past this handler's outer `catch`, which classifies and + // LOGS it (`mapDataError` + `logError`) rather than + // reporting a missing target — and it also stops an + // object-valued carrier from being forwarded as + // `query.object` into `findData`. + referenceObject = referenceTargetOf(fieldDef); } if (!referenceObject) { res.status(500).json({