diff --git a/.changeset/24952-user-field-reference-refused.md b/.changeset/24952-user-field-reference-refused.md new file mode 100644 index 00000000000..80ba9f36aa3 --- /dev/null +++ b/.changeset/24952-user-field-reference-refused.md @@ -0,0 +1,24 @@ +--- +'@objectstack/spec': minor +'@objectstack/lint': minor +--- + +feat(spec)!: a `user` field whose `reference` names an object other than `sys_user` is refused at parse, and every `user` field resolves to `sys_user` + +Clause-②: no (narrowing) +Metadata-protocol: narrowed + + + +**BREAKING** (the accept set of `FieldSchema` narrows), shipped as `minor` under the repo's launch-window convention for breaking changes. + +- **What was wrong.** The spec declares a `user` field's target fixed to `sys_user`, and nothing enforced it. `{ type: 'user', reference: 'probe_account' }` parsed, and `referenceTargetOf` resolved it to `probe_account`. Readers that follow the target (`$expand`, imports, seed loading, field paths) treated the field as a link to `probe_account`, while readers that branch on the type (the person picker, a `current_user` default, a sharing rule over the users a field holds) treated its values as users. +- **FROM:** a `user` field with `reference` naming any object but `sys_user` parsed, and resolved to the named object. +- **TO:** `FieldSchema` refuses it with an issue at `reference` that names the replacement, so `ObjectSchema`, `ObjectExtensionSchema`, `defineStack`, `os validate` and the metadata save door refuse it too. `referenceTargetOf` answers `sys_user` for every `user` field, whatever `reference` says, so a stored body saved before this release resolves to `sys_user` as well. `Field.user()`'s config types `reference` as `'sys_user'` and writes `sys_user` after the config spread. +- **Unchanged:** a `user` field without `reference`, with `reference: 'sys_user'` or with a blank `reference`, and `Field.user()`'s output for every typed call. +- **Fix:** change such a field to a `lookup` on the object it names, `{ type: 'lookup', reference: 'TARGET' }` (`Field.lookup('TARGET', …)`), and choose its `deleteBehavior` on purpose: a `lookup` defaults to `'set_null'` when the target record is deleted, which a `user` field never did. A `current_user` default is not legal on a `lookup` to an object other than `sys_user`. If a person was meant, drop `reference`. +- **Why no automatic rewrite.** No ADR-0087 conversion can carry the change losslessly: a `lookup` joins the referential delete walk and `deleteBehavior` has no value that means "leave it". `objectstack migrate meta` reports the semantic entry `user-field-foreign-reference-refused` instead. + +**Supersedes the `user-field-foreign-reference` warning entry in this release.** That entry announced a `@objectstack/lint` advisory `user-field-foreign-reference`, the `USER_FIELD_FOREIGN_REFERENCE` export and its `os explain` entry, and said such a field would still parse. All three are withdrawn before release and never ship: a foreign `reference` on a `user` field is now refused by `FieldSchema` instead, so there is nothing left for the advisory to warn about. + +- **For `@objectstack/lint`:** no `user-field-foreign-reference` rule, no `USER_FIELD_FOREIGN_REFERENCE` export and no `RULE_EXPLANATIONS` entry for it. `OBJECT_REFERENCE_UNKNOWN` and every other export are unchanged. The refusal arrives as a `FieldSchema` issue at `reference`, not as a lint advisory. diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index fac2fbcb61e..248fbb33f64 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -426,7 +426,6 @@ export { validateObjectReferences, OBJECT_REFERENCE_UNKNOWN, OBJECT_REFERENCE_UNREGISTERED_PLATFORM, - USER_FIELD_FOREIGN_REFERENCE, } from './validate-object-references.js'; export type { ObjectRefFinding, ObjectRefSeverity } from './validate-object-references.js'; diff --git a/packages/lint/src/object-graph.test.ts b/packages/lint/src/object-graph.test.ts index 0f289d42cac..cd4d00a7d7d 100644 --- a/packages/lint/src/object-graph.test.ts +++ b/packages/lint/src/object-graph.test.ts @@ -37,8 +37,8 @@ const stack = { fields: { region: { type: 'text', label: 'Region' }, // [#24808] A link to a person OBJECT is a `lookup`: a `user` field's - // target is fixed to `sys_user`, and naming another object with it is - // what `user-field-foreign-reference` warns on. + // target is fixed to `sys_user`, and `FieldSchema` refuses a `user` + // field naming another object (#24952). owner: { type: 'lookup', label: 'Owner', reference: 'crm_person' }, }, }, @@ -419,21 +419,19 @@ describe('[#19289] object-graph — a `user` field takes its target from the TYP 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. - // [#24808] The shape is declared wrong and warned on - // (`user-field-foreign-reference`); this pins its protocol-18 resolution - // until the next major refuses it, so it is built here rather than kept as - // an incidental module fixture. + it('[#24952] a foreign carrier on a `user` field no longer wins: the hop lands on `sys_user`', () => { + // `reference` MATERIALIZES the constant for `user`. Naming another object + // with it is refused at parse, so it reaches the graph only unparsed, and + // the arbiter answers the type's constant rather than what was written. const foreign = indexObjectGraph({ objects: [ { name: 'crm_account', fields: { owner: { type: 'user', label: 'Owner', reference: 'crm_person' } } }, { name: 'crm_person', fields: { email: { type: 'email', label: 'Email' } } }, + { name: 'sys_user', fields: { email: { type: 'email', label: 'Email' } } }, ], }); expect(resolveFieldPath(foreign, 'crm_account', 'owner.email')) - .toMatchObject({ kind: 'ok', object: 'crm_person', field: 'email' }); + .toMatchObject({ kind: 'ok', object: 'sys_user', field: 'email' }); }); it('control: an UNREADABLE carrier still REFUSES at index time', () => { diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 9527198ce3b..d25a7356ae8 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -3566,28 +3566,6 @@ const OBJECT_REFERENCE_UNREGISTERED_PLATFORM_EXPLANATION: RuleExplanation = { ], }; -const USER_FIELD_FOREIGN_REFERENCE_EXPLANATION: RuleExplanation = { - rule: 'user-field-foreign-reference', - covers: 'why a user field cannot point at another object', - paragraphs: [ - 'A `user` field is a person picker: its target is fixed to the `sys_user` object by the type, ' + - 'which is why `Field.user()` takes no target argument. Writing `reference` with any other ' + - 'object name contradicts the type, and today nothing refuses it.', - 'Such a field is half one thing and half another. Every reader that follows the target ' + - '(`$expand`, imports, seed loading, field paths) treats it as a link to the named object, ' + - 'while every reader that branches on the type treats its values as users: the person picker ' + - 'searches people, a `current_user` default stores the acting user\'s id, and a sharing rule over ' + - 'the users a field holds shares the record with those ids.', - 'A link to another object is a `lookup`: `{ type: \'lookup\', reference: \'\' }`, or ' + - '`Field.lookup(\'\', …)`. If a person was meant, drop `reference`.', - 'It warns rather than refuses because no automatic rewrite keeps the field\'s behaviour: a `lookup` ' + - 'takes part in the delete walk with `deleteBehavior: \'set_null\'` by default, so it would start ' + - 'clearing the column when the target record is deleted, which a `user` field never does. Make ' + - 'the change by hand and check that delete behaviour is what you want. Refusing the shape at ' + - 'parse is planned for the next major.', - ], -}; - // ── Capability references (`validate-capability-references.ts`) ─────────── const CAPABILITY_REFERENCE_UNKNOWN_EXPLANATION: RuleExplanation = { @@ -4884,7 +4862,6 @@ export const RULE_EXPLANATIONS: Readonly> = Obje [OBJECT_FIELD_REF_UNKNOWN_EXPLANATION.rule]: OBJECT_FIELD_REF_UNKNOWN_EXPLANATION, [OBJECT_REFERENCE_UNKNOWN_EXPLANATION.rule]: OBJECT_REFERENCE_UNKNOWN_EXPLANATION, [OBJECT_REFERENCE_UNREGISTERED_PLATFORM_EXPLANATION.rule]: OBJECT_REFERENCE_UNREGISTERED_PLATFORM_EXPLANATION, - [USER_FIELD_FOREIGN_REFERENCE_EXPLANATION.rule]: USER_FIELD_FOREIGN_REFERENCE_EXPLANATION, [CAPABILITY_REFERENCE_UNKNOWN_EXPLANATION.rule]: CAPABILITY_REFERENCE_UNKNOWN_EXPLANATION, [LIST_VIEW_FILTERS_IN_VIEWS_MODE_EXPLANATION.rule]: LIST_VIEW_FILTERS_IN_VIEWS_MODE_EXPLANATION, [LIVENESS_LEDGER_UNREADABLE_EXPLANATION.rule]: LIVENESS_LEDGER_UNREADABLE_EXPLANATION, diff --git a/packages/lint/src/validate-object-references.test.ts b/packages/lint/src/validate-object-references.test.ts index 207d346d89b..066df1aaff4 100644 --- a/packages/lint/src/validate-object-references.test.ts +++ b/packages/lint/src/validate-object-references.test.ts @@ -5,9 +5,7 @@ import { validateObjectReferences as validateObjectReferencesUnrecorded, OBJECT_REFERENCE_UNKNOWN, OBJECT_REFERENCE_UNREGISTERED_PLATFORM, - USER_FIELD_FOREIGN_REFERENCE, } from './validate-object-references.js'; -import { Field, ObjectSchema, referenceTargetOf } from '@objectstack/spec/data'; import { runAuthoringRules, splitBySeverity } from './authoring-rules.js'; import { PLATFORM_OBJECT_PREFIXES } from '@objectstack/spec/system'; import { explainRule } from './rule-explanations.js'; @@ -706,104 +704,20 @@ describe('[#19289] validateObjectReferences — a `user` target comes from the T 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', () => { + it('control: the rule still REPORTS a real miss — a `lookup` naming an unknown object errors', () => { // Without this, "no findings" above reads equally well as a rule that - // stopped judging `user` fields altogether. - // [#24808] A `user` field naming any object but `sys_user` also draws the - // foreign-target warning, so the unknown-object error is read by rule. - const findings = validateObjectReferences(userField({ reference: 'zzz_nope' })); - expect(findings.filter((f) => f.rule === OBJECT_REFERENCE_UNKNOWN)).toHaveLength(1); - expect(findings.map((f) => f.rule).sort()).toEqual([OBJECT_REFERENCE_UNKNOWN, USER_FIELD_FOREIGN_REFERENCE].sort()); + // stopped judging fields altogether. [#24952] A `user` field can no longer + // carry the miss: its target is the type's constant whatever the carrier + // says (`FieldSchema` refuses a foreign name), so the same field leg is + // proven on a `lookup`, and a foreign carrier on `user` resolves to + // `sys_user` and draws nothing. + const lookup = validateObjectReferences(userField({ type: 'lookup', reference: 'zzz_nope' })); + expect(lookup).toHaveLength(1); + expect(lookup[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN); + expect(validateObjectReferences(userField({ reference: 'zzz_nope' }))).toEqual([]); }); }); -/** - * [#24808] A `user` field's target is FIXED to `sys_user` by the spec, and - * nothing enforced it: the reproduction below parses through `ObjectSchema` - * and resolves through `referenceTargetOf` to the authored object. Under the - * 「甲」 ruling the 18.x half is this warning (no ADR-0087 conversion can carry - * the refusal: a `lookup` joins the referential delete walk a `user` field - * sits outside), so the parse half below pins that the shape still LOADS. - */ -describe('[#24808] validateObjectReferences — a `user` field naming another object warns', () => { - const stackWith = (assignee: Record) => ({ - objects: [ - { name: 'probe_account', fields: { name: { type: 'text', label: 'Name' } } }, - { name: 'probe_task', fields: { name: { type: 'text', label: 'Name' }, owner: assignee } }, - ], - }); - const PROBE = { type: 'user', label: 'Owner', reference: 'probe_account' }; - - it('reproduction: the shape parses and the arbiter follows the authored object', () => { - const parsed = ObjectSchema.safeParse({ name: 'probe_task', fields: { owner: PROBE } }); - expect(parsed.success).toBe(true); - expect(referenceTargetOf(PROBE)).toBe('probe_account'); - }); - - it('warns — never errors — and names the field, the target and the lookup spelling', () => { - const findings = validateObjectReferences(stackWith(PROBE)); - expect(findings).toHaveLength(1); - const [f] = findings; - expect(f.rule).toBe(USER_FIELD_FOREIGN_REFERENCE); - expect(f.severity).toBe('warning'); - expect(f.path).toBe('objects[1].fields.owner.reference'); - expect(f.where).toBe('object "probe_task" · field "owner"'); - expect(f.message).toContain('"probe_account"'); - expect(f.hint).toContain("{ type: 'lookup', reference: 'probe_account' }"); - }); - - it('warns on an object extension too — the verdict needs no artifact context', () => { - const findings = validateObjectReferences({ - objectExtensions: [{ extend: 'sys_user_profile', fields: { mentor: { ...PROBE } } }], - }); - expect(findings.filter((f) => f.rule === USER_FIELD_FOREIGN_REFERENCE)).toEqual([ - expect.objectContaining({ path: 'objectExtensions[0].fields.mentor.reference', severity: 'warning' }), - ]); - }); - - it('warns on the map form of `fields` and on the array form of `objects` alike', () => { - const findings = validateObjectReferences({ - objects: { probe_task: { fields: { owner: PROBE } }, probe_account: { fields: {} } }, - }); - expect(findings.map((f) => f.rule)).toEqual([USER_FIELD_FOREIGN_REFERENCE]); - }); - - it('control: the two legal spellings — no `reference`, and `reference: "sys_user"` — stay silent', () => { - expect(validateObjectReferences(stackWith({ type: 'user', label: 'Owner' }))).toEqual([]); - expect(validateObjectReferences(stackWith({ type: 'user', label: 'Owner', reference: 'sys_user' }))).toEqual([]); - expect(validateObjectReferences(stackWith(Field.user({ label: 'Owner' })))).toEqual([]); - }); - - it('control: a `lookup` to the same object is the canonical spelling and draws nothing', () => { - expect(validateObjectReferences(stackWith(Field.lookup('probe_account', { label: 'Owner' })))).toEqual([]); - expect(validateObjectReferences(stackWith({ type: 'lookup', reference: 'probe_account' }))).toEqual([]); - }); - - it('control: an unreadable carrier still REFUSES rather than reading as a foreign target', () => { - expect(() => validateObjectReferences(stackWith({ type: 'user', reference: { object: 'sys_user' } }))) - .toThrow(/`reference` is an object/); - }); - - it('prints one line each, and `os explain` carries the reasoning the verdict leaves out', () => { - const [f] = validateObjectReferences(stackWith(PROBE)); - for (const text of [f.message, f.hint]) { - expect(text).not.toContain('\n'); - expect(text.length, text).toBeLessThanOrEqual(200); - } - const entry = explainRule(USER_FIELD_FOREIGN_REFERENCE); - expect(entry?.rule).toBe(USER_FIELD_FOREIGN_REFERENCE); - expect(entry!.paragraphs.join('\n')).toContain("deleteBehavior: 'set_null'"); - }); - - for (const command of ['validate', 'build', 'lint'] as const) { - it(`reaches \`os ${command}\` through the authoring registry, as an advisory and never a gate`, () => { - const { errors, advisories } = splitBySeverity(runAuthoringRules(command, { normalized: stackWith(PROBE) as never })); - expect(errors.filter((f) => f.rule === USER_FIELD_FOREIGN_REFERENCE)).toEqual([]); - expect(advisories.filter((f) => f.rule === USER_FIELD_FOREIGN_REFERENCE)).toHaveLength(1); - }); - } -}); - /** * [#20216] A view CONTAINER's own `object` — the key `getViewsByObject()` * indexes by. Modelled on an `os init -t app` project (namespace `my_app`), diff --git a/packages/lint/src/validate-object-references.ts b/packages/lint/src/validate-object-references.ts index bc1ff6a17b5..cc25a005868 100644 --- a/packages/lint/src/validate-object-references.ts +++ b/packages/lint/src/validate-object-references.ts @@ -83,7 +83,6 @@ import { hasPlatformObjectPrefix, isPlatformProvidedObjectName, PLATFORM_PROVIDED_OBJECT_NAMES, - SystemObjectName, } from '@objectstack/spec/system'; import { referenceCarrierOf, referenceTargetOf } from '@objectstack/spec/data'; @@ -108,31 +107,6 @@ const RELATIONSHIP_TARGET_FIELD_TYPES: ReadonlySet = new Set(['lookup', export const OBJECT_REFERENCE_UNKNOWN = 'object-reference-unknown'; export const OBJECT_REFERENCE_UNREGISTERED_PLATFORM = 'object-reference-unregistered-platform'; -/** - * [#24808 — ruled A on #24854, timed by the 「甲」 standing ruling] A `user` - * field whose `reference` names an object other than `sys_user`. - * - * The spec declares a `user` field's target FIXED to `sys_user` - * (`Field.user()`'s docblock, `IMPLICIT_REFERENCE_TARGETS` in - * `field-value.zod.ts`), and nothing enforces it: `FieldSchema` admits any - * string, and `referenceTargetOf` returns the authored carrier first, so the - * runtime's target-following readers (`$expand`, the import runner, the seed - * loader, the object graph) resolve the field against the authored object - * while every reader that branches on the TYPE treats its values as users — - * the person picker, the `current_user` default, a sharing rule's - * `fieldHoldsUsers`. The field is half a lookup and half a user. - * - * WARNING, not error, by the 「甲」 rule: a narrowing ships its refusal in an - * 18.x minor only when an ADR-0087 conversion carries it, and this one has no - * lossless rewrite — `user` sits outside the referential delete walk - * (`DELETE_SEMANTICS_REFERENCE_TYPES`, objectql) while a `lookup` materializes - * `deleteBehavior: 'set_null'` and joins it, and `deleteBehavior` has no value - * meaning "do nothing", so `{ type: 'lookup', reference }` would start nulling - * child columns on a delete that leaves them untouched today. The refusal is - * the next protocol major's; this is its loud half. - */ -export const USER_FIELD_FOREIGN_REFERENCE = 'user-field-foreign-reference'; - export type ObjectRefSeverity = 'error' | 'warning'; export interface ObjectRefFinding { @@ -208,55 +182,6 @@ function artifactProvidedObjectNames(stack: AnyRec): string[] { * Validate every object-name reference on the surfaces listed in the module * header. Returns findings (empty = clean). */ -/** - * The {@link USER_FIELD_FOREIGN_REFERENCE} findings for one stack: every - * `user` field, on an object or an object extension, whose `reference` names - * an object other than `sys_user`. - * - * Reads the CARRIER (`referenceCarrierOf`), not `referenceTargetOf`: the - * question is what the author wrote, and the arbiter would answer the same - * string for this shape anyway. An absent carrier and `reference: 'sys_user'` - * are the two legal spellings and produce nothing; an unreadable carrier - * throws through the accessor exactly as it does for the resolution leg. - * Interpolated targets are not exempted: the type fixes the target, so no - * render-time value can make another object right. - */ -function userFieldForeignReferenceFindings(stack: AnyRec): ObjectRefFinding[] { - const out: ObjectRefFinding[] = []; - const legs: Array<{ key: 'objects' | 'objectExtensions'; ownName: (rec: AnyRec) => string | undefined }> = [ - { key: 'objects', ownName: (rec) => strName(rec.name) }, - { key: 'objectExtensions', ownName: (rec) => strName(rec.extend) }, - ]; - for (const { key, ownName } of legs) { - const containers = recordsOf(stack[key]); - for (let ci = 0; ci < containers.length; ci++) { - const container = containers[ci]; - const containerName = ownName(container) ?? `#${ci}`; - const fields = recordsOf(container.fields); - for (let fi = 0; fi < fields.length; fi++) { - const field = fields[fi]; - if (field.type !== 'user') continue; - const target = referenceCarrierOf(field, 'validateObjectReferences'); - if (!target || target === SystemObjectName.USER) continue; - const fieldName = strName(field.name) ?? `#${fi}`; - out.push({ - severity: 'warning', - rule: USER_FIELD_FOREIGN_REFERENCE, - where: `${key === 'objects' ? 'object' : 'object extension'} "${containerName}" · field "${fieldName}"`, - path: `${key}[${ci}].fields.${fieldName}.reference`, - message: - `user field "${fieldName}" names reference "${target}", but a user field's target is fixed to ` + - `sys_user; it acts as a link to "${target}" whose values user-only readers take for users.`, - hint: - `Declare a lookup instead, { type: 'lookup', reference: '${target}' }, or drop \`reference\` ` + - 'if a person was meant.', - }); - } - } - } - return out; -} - export function validateObjectReferences(stack: AnyRec): ObjectRefFinding[] { const findings: ObjectRefFinding[] = []; if (!stack || typeof stack !== 'object') return findings; @@ -415,13 +340,6 @@ export function validateObjectReferences(stack: AnyRec): ObjectRefFinding[] { } } - // ── `user` fields → a target other than `sys_user` (#24808) ── - // See {@link userFieldForeignReferenceFindings}. Walks `objectExtensions` - // too: unlike the resolution leg above, this verdict needs no artifact - // context — a `user` field naming any object but `sys_user` is the same - // contradiction wherever the field is declared. - findings.push(...userFieldForeignReferenceFindings(stack)); - // ── View containers → the container's own `object` (#20216) ── // `ViewSchema.object` is `z.string()`, and nothing resolved it: `defineStack`'s // `validateCrossReferences` reads a container's `list.data` / `form.data` diff --git a/packages/spec/src/data/field-value.test.ts b/packages/spec/src/data/field-value.test.ts index 5f8b9cce1ab..fa73df1dadb 100644 --- a/packages/spec/src/data/field-value.test.ts +++ b/packages/spec/src/data/field-value.test.ts @@ -87,13 +87,12 @@ describe('semantic type classes', () => { // without it is fully specified, not under-specified (cloud#983). expect(referenceTargetOf({ type: 'user' })).toBe('sys_user'); expect(referenceTargetOf({ type: 'user', reference: 'sys_user' })).toBe('sys_user'); - // An explicit target still wins — nothing here overrides authored metadata. - // [#24808] This pins the protocol-18 behaviour on purpose, not a producer: - // the shape is declared wrong (the target is fixed) and `@objectstack/lint` - // warns on it (`user-field-foreign-reference`), but no ADR-0087 conversion - // carries its refusal, so under the 「甲」 ruling it keeps resolving as - // written until the next protocol major refuses it at parse. - expect(referenceTargetOf({ type: 'user', reference: 'my_people' })).toBe('my_people'); + // [#24952] The constant wins over any carrier: `FieldSchema` refuses a + // `user` field naming another object, so a foreign carrier reaches this + // reader only unparsed (a body stored before the refusal), and it still + // resolves to the one target the type declares. + expect(referenceTargetOf({ type: 'user', reference: 'my_people' })).toBe('sys_user'); + expect(referenceTargetOf({ type: 'user', reference: '' })).toBe('sys_user'); // Genuinely targetless: the types whose target IS author-chosen, unwritten. expect(referenceTargetOf({ type: 'lookup' })).toBeUndefined(); @@ -163,11 +162,13 @@ describe('semantic type classes', () => { // Guards the set from drifting: adding a reference type without deciding // which half it belongs to would leave `referenceTargetOf` silently // answering `undefined` for a fully-authored field. + // [#24952] A type that implies its target is not moved by a carrier; a + // type that admits one answers the carrier. for (const t of REFERENCE_VALUE_TYPES) { const implied = referenceTargetOf({ type: t }); const authored = referenceTargetOf({ type: t, reference: 'somewhere' }); - expect(authored, `authored target for ${t}`).toBe('somewhere'); expect(implied === undefined || typeof implied === 'string', `implied target for ${t}`).toBe(true); + expect(authored, `authored target for ${t}`).toBe(implied ?? 'somewhere'); } expect([...REFERENCE_VALUE_TYPES].filter((t) => referenceTargetOf({ type: t }) !== undefined)) .toEqual(['user']); diff --git a/packages/spec/src/data/field-value.zod.ts b/packages/spec/src/data/field-value.zod.ts index cd5d5ca4f1c..3702ff95fa0 100644 --- a/packages/spec/src/data/field-value.zod.ts +++ b/packages/spec/src/data/field-value.zod.ts @@ -168,16 +168,13 @@ export const REFERENCE_VALUE_TYPES: ReadonlySet = new Set([ * it. Metadata authored without it (hand-written JSON, an AI author, a Studio * form) is fully specified, not under-specified. * - * ⚠️ [#24808] Declared, not yet enforced: `FieldSchema` admits any string on a - * `user` field's `reference`, and {@link referenceTargetOf} returns an - * authored carrier FIRST, so `{ type: 'user', reference: 'probe_account' }` - * resolves to `probe_account`. Under the 「甲」 ruling that stays so for - * protocol 18 — no ADR-0087 conversion can rewrite such a field losslessly (a - * `lookup` joins the referential delete walk a `user` field sits outside), so - * re-targeting it here would change a loading field's behaviour unannounced. - * The loud half ships instead: `@objectstack/lint`'s - * `user-field-foreign-reference` warning. The refusal, and with it the - * carrier ceasing to matter here, is the next protocol major's. + * Enforced at both ends since #24952 (#24808 ruled A on #24854, shipped in an + * 18.x minor under the amended 「甲」 ruling): `FieldSchema` refuses a `user` + * field whose `reference` names any object but `sys_user`, and + * {@link referenceTargetOf} answers this constant for a member type whatever + * the carrier says, so a row that reached a reader without the parse (a stored + * body saved before the refusal, a raw registration) still resolves to the one + * target the type declares. A link to another object is a `lookup`. */ const IMPLICIT_REFERENCE_TARGETS: ReadonlyMap = new Map([ ['user', SystemObjectName.USER], @@ -220,8 +217,13 @@ export function referenceTargetOf(def: unknown): string | undefined { const { type } = def as { type?: unknown }; const reference = referenceCarrierOf(def, 'referenceTargetOf'); if (typeof type !== 'string' || !REFERENCE_VALUE_TYPES.has(type)) return undefined; - if (reference) return reference; - return IMPLICIT_REFERENCE_TARGETS.get(type); + // [#24952] A type that fixes its target answers that target, whatever the + // carrier says: the carrier is still READ above (an unreadable one throws), + // but for `user` it can only materialize the constant — `FieldSchema` + // refuses any other name. + const implicit = IMPLICIT_REFERENCE_TARGETS.get(type); + if (implicit) return implicit; + return reference || undefined; } /** diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index 9d7a024c0fb..043eb27d3ec 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -35,6 +35,13 @@ import { } from './default-value-shape'; import { AddressSchema, FILE_REFERENCE_TYPES, MULTI_CAPABLE_TYPES, MULTI_OPTION_TYPES, REFERENCE_VALUE_TYPES, SINGLE_OPTION_TYPES } from './field-value.zod'; import { ValueDomainSchema } from '../shared/value-domain.zod'; +import { SystemObjectName } from '../system/constants/system-names'; + +/** + * [#24952] The one object a `user` field links to — `IMPLICIT_REFERENCE_TARGETS`'s + * constant (`field-value.zod.ts`), read here by the refusal of any other name. + */ +const USER_FIELD_TARGET: string = SystemObjectName.USER; /** * Field Type Enum @@ -2431,6 +2438,42 @@ export const FieldSchema = lazySchema(() => { }); } + // [#24952 — #24808 ruled A on #24854; shipped in an 18.x minor under the + // amended 「甲」 ruling] A `user` field's target is FIXED to `sys_user` by the + // type (`IMPLICIT_REFERENCE_TARGETS`, `field-value.zod.ts`), so a + // `reference` naming any other object contradicts the type: every reader + // that follows the target would link the field to that object, while every + // reader that branches on the type (the person picker, a `current_user` + // default, a sharing rule over the users a field holds) takes its values for + // users. Refused here rather than on `ObjectSchema` (the + // `refuseForeignTreeReference` precedent needs the declaring object's name; + // this verdict needs only the field), so every door that parses a field map + // — objects, object extensions — refuses it alike. Absent, blank and + // `'sys_user'` are the spellings that name the constant or nothing. + // + // No ADR-0087 conversion carries it (the D3 entry + // `user-field-foreign-reference-refused` says why): rewriting the field to a + // `lookup` would add it to the referential delete walk with + // `deleteBehavior: 'set_null'`, which a `user` field never joined, and + // `deleteBehavior` has no value meaning "leave it", so the author decides. + if ( + field.type === 'user' + && field.reference !== undefined + && field.reference.trim() !== '' + && field.reference !== USER_FIELD_TARGET + ) { + ctx.addIssue({ + code: 'custom', + path: ['reference'], + message: + `A \`user\` field's target is fixed to \`${USER_FIELD_TARGET}\`, but this one names ` + + `\`reference: '${field.reference}'\`. To link records of \`${field.reference}\`, declare a ` + + `\`lookup\`: \`{ type: 'lookup', reference: '${field.reference}' }\` (\`Field.lookup('${field.reference}', …)\`), ` + + "and choose its `deleteBehavior` (a `lookup` defaults to `'set_null'` when the target record is " + + 'deleted; a `user` field never did). If a person was meant, drop `reference`.', + }); + } + // A single-choice field (`select` / `radio`) with NO option source — neither // a non-empty `options` list nor a `picklist` reference — is a choice with // nothing to choose: the form control offers nothing, and server-side value @@ -3033,19 +3076,17 @@ export type CurrencyValue = z.input; export type FieldInput = Omit, 'type'>; /** - * The `Field.user()` config: {@link FieldInput} with `reference` marked - * deprecated. A `user` field's target is a constant of the type - * (`IMPLICIT_REFERENCE_TARGETS`, `field-value.zod.ts`), so the key has nothing - * to say; it is still ACCEPTED (an identical accept set — the narrowing waits - * for the next protocol major), and a foreign value is warned on by - * `user-field-foreign-reference`. + * The `Field.user()` config: {@link FieldInput} with `reference` narrowed to + * the one value a `user` field can carry. A `user` field's target is a constant + * of the type (`IMPLICIT_REFERENCE_TARGETS`, `field-value.zod.ts`), and + * `FieldSchema` refuses any other name (#24952), so the key has nothing to say. */ type UserFieldInput = Omit & { /** * @deprecated A `user` field always targets `sys_user`; omit this key. To * link another object, use `Field.lookup(reference, …)`. */ - reference?: string; + reference?: 'sys_user'; }; /** `Field.select` with inline options — the array or `{ options }` forms. */ @@ -3222,22 +3263,18 @@ export const Field = { * @example Auto-fill the acting user on create * Field.user({ label: 'Reporter', defaultValue: 'current_user' }) * - * ⚠️ [#24808] `config.reference` is deprecated, and naming any object but - * `sys_user` with it is the contradiction `os validate` / `os build` / - * `os lint` warn on (`user-field-foreign-reference`): such a field is - * resolved against the named object by every target-following reader while - * every user-only reader takes its values for users. A link to another - * object is `Field.lookup(reference, …)`. The spread stays LAST on purpose: - * moving `reference: 'sys_user'` after it would silently re-target a field - * an untyped caller wrote, the same unannounced behaviour change the 「甲」 - * ruling keeps out of an 18.x minor — the refusal belongs to the next - * protocol major, and until then the warning is the loud half. + * [#24952] `config.reference` is deprecated and typed `'sys_user'`: a + * `user` field naming any other object is refused by `FieldSchema`, and a + * link to another object is `Field.lookup(reference, …)`. `reference` is + * written AFTER the spread, so even an untyped caller's foreign value never + * reaches the output — the parse would refuse it anyway. It keeps its second + * position (re-assigning an existing key does not move it), so the output's + * key order is what it always was. */ - user: (config: UserFieldInput = {}) => ({ - type: 'user', - reference: 'sys_user', - ...config, - } as const), + user: (config: UserFieldInput = {}) => { + const field = { type: 'user', reference: 'sys_user', ...config } as const; + return { ...field, reference: 'sys_user' } as const; + }, // Enhanced Field Type Helpers location: (config: FieldInput = {}) => ({ diff --git a/packages/spec/src/data/user-field-reference-fixed.test.ts b/packages/spec/src/data/user-field-reference-fixed.test.ts new file mode 100644 index 00000000000..cb207ff5295 --- /dev/null +++ b/packages/spec/src/data/user-field-reference-fixed.test.ts @@ -0,0 +1,108 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, expectTypeOf } from 'vitest'; +import { ObjectSchema, ObjectExtensionSchema } from './object.zod'; +import { Field, FieldSchema } from './field.zod'; +import { referenceTargetOf } from './field-value.zod'; +import { defineStack } from '../stack.zod'; + +// --------------------------------------------------------------------------- +// [#24952] A `user` field's target is FIXED to `sys_user` by the type +// (#24808, ruled A on #24854; shipped in an 18.x minor under the amended 「甲」 +// ruling). Three halves, pinned together so none can drift from the others: +// +// - the parse refuses a `user` field whose `reference` names another object, +// at every door that parses a field map, with the `lookup` spelling in the +// prescription; +// - `referenceTargetOf` answers `sys_user` for a `user` field whatever the +// carrier says (a body that reached it unparsed); +// - `Field.user()` writes `sys_user` after the config spread. +// +// Controls: the two legal spellings (no `reference`, `reference: 'sys_user'`) +// parse, and a `lookup` on the same object parses. +// --------------------------------------------------------------------------- + +const task = (owner: Record) => ({ + name: 'probe_task', + fields: { name: { type: 'text', label: 'Name' }, owner }, +}); + +const FOREIGN = { type: 'user', label: 'Owner', reference: 'probe_account' }; + +describe('a `user` field whose `reference` names another object is refused', () => { + it('FieldSchema refuses it, located at `reference`, naming the `lookup` spelling', () => { + const parsed = FieldSchema.safeParse(FOREIGN); + expect(parsed.success).toBe(false); + if (parsed.success) return; + const issues = parsed.error.issues.filter((i) => i.path.join('.') === 'reference'); + expect(issues).toHaveLength(1); + expect(issues[0].code).toBe('custom'); + expect(issues[0].message).toContain('sys_user'); + expect(issues[0].message).toContain("{ type: 'lookup', reference: 'probe_account' }"); + expect(issues[0].message).toContain('deleteBehavior'); + }); + + it('ObjectSchema refuses it, at the field path', () => { + const parsed = ObjectSchema.safeParse(task(FOREIGN)); + expect(parsed.success).toBe(false); + if (parsed.success) return; + expect(parsed.error.issues.map((i) => i.path)).toEqual([['fields', 'owner', 'reference']]); + }); + + it('ObjectExtensionSchema refuses it too — an extension cannot merge the shape the object refuses', () => { + const parsed = ObjectExtensionSchema.safeParse({ extend: 'sys_user_profile', fields: { mentor: FOREIGN } }); + expect(parsed.success).toBe(false); + if (parsed.success) return; + expect(parsed.error.issues.map((i) => i.path)).toEqual([['fields', 'mentor', 'reference']]); + }); + + it('defineStack refuses it', () => { + expect(() => + defineStack({ + manifest: { id: 'com.example.probe', namespace: 'probe', version: '0.0.1', type: 'app', name: 'Probe' }, + objects: [ + { name: 'probe_account', fields: { name: { type: 'text' } } }, + task(FOREIGN) as never, + ], + }), + ).toThrow(/target is fixed to `sys_user`/); + }); + + it('control: no `reference`, `reference: "sys_user"` and a blank carrier parse', () => { + for (const owner of [ + { type: 'user', label: 'Owner' }, + { type: 'user', label: 'Owner', reference: 'sys_user' }, + { type: 'user', label: 'Owner', reference: '' }, + Field.user({ label: 'Owner' }), + ]) { + const parsed = ObjectSchema.safeParse(task(owner)); + expect(parsed.success, parsed.success ? '' : JSON.stringify(parsed.error.issues)).toBe(true); + } + }); + + it('control: a `lookup` on the same object — the canonical spelling — parses', () => { + for (const owner of [{ type: 'lookup', label: 'Owner', reference: 'probe_account' }, Field.lookup('probe_account', { label: 'Owner' })]) { + const parsed = ObjectSchema.safeParse(task(owner)); + expect(parsed.success, parsed.success ? '' : JSON.stringify(parsed.error.issues)).toBe(true); + } + }); +}); + +describe('the target of a `user` field is `sys_user` whatever the carrier says', () => { + it('referenceTargetOf answers the constant for a foreign carrier', () => { + expect(referenceTargetOf(FOREIGN)).toBe('sys_user'); + }); + + it('Field.user() writes `sys_user` after the spread, and keeps the key in second position', () => { + // An untyped caller is the only one that can still pass a foreign value. + const field = Field.user({ label: 'Owner', reference: 'probe_account' as never }); + expect(field.reference).toBe('sys_user'); + expect(Object.keys(field)).toEqual(['type', 'reference', 'label']); + expect(Object.keys(Field.user({ label: 'Owner' }))).toEqual(['type', 'reference', 'label']); + }); + + it('Field.user() keeps its literal output types (checked by the test typecheck)', () => { + expectTypeOf(Field.user({ label: 'Owner' }).type).toEqualTypeOf<'user'>(); + expectTypeOf(Field.user({ label: 'Owner' }).reference).toEqualTypeOf<'sys_user'>(); + }); +}); diff --git a/packages/spec/src/migrations/entries/semantic/18.user-field-foreign-reference-refused.ts b/packages/spec/src/migrations/entries/semantic/18.user-field-foreign-reference-refused.ts new file mode 100644 index 00000000000..7b765200cc7 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.user-field-foreign-reference-refused.ts @@ -0,0 +1,38 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// #24808 ruled A on #24854 (enforce the declaration that a `user` field's +// target is fixed to `sys_user`), shipped in an 18.x minor under the amended +// 「甲」 ruling. A semantic entry and not a conversion: the only rewrite that +// keeps the link — to a `lookup` on the named object — changes the field's +// delete behaviour, and which behaviour the author wants is theirs to say. +export const entry: SemanticMigration = { + id: 'user-field-foreign-reference-refused', + surface: + '`object.fields[].reference` (and `objectExtension.fields[].reference`) on a field of `type: \'user\'`, ' + + 'when it names an object other than `sys_user`.', + replacement: + 'A `lookup` on the named object: `{ type: \'lookup\', reference: \'TARGET\' }` (`Field.lookup` with ' + + 'the target as its first argument), with the `deleteBehavior` the author chooses. If a person was ' + + 'meant, keep `type: \'user\'` and drop `reference`.', + reason: + 'A `user` field\'s target is a constant of the type, and the spec refuses any other name at parse. ' + + 'The field was half one thing and half another: readers that follow the target treated it as a ' + + 'link to the named object, while readers that branch on the type (the person picker, a ' + + '`current_user` default, a sharing rule over the users a field holds) treated its values as users. ' + + 'No conversion can carry the rewrite. A `lookup` joins the referential delete walk and gets ' + + '`deleteBehavior: \'set_null\'` by default, so it would start clearing the column when the target ' + + 'record is deleted, which a `user` field never did, and `deleteBehavior` has no value that means ' + + '"leave it". A converted field would also stop counting as one that holds users for sharing, and ' + + 'a `current_user` default on it would be refused. So a delete-behaviour judgement is owed by the ' + + 'author.', + acceptanceCriteria: + 'Every `user` field in the stack either declares no `reference` or declares `reference: \'sys_user\'`, ' + + 'and `os validate` passes. A field that still names another object is refused at parse with an ' + + 'issue at `fields.NAME.reference` that names the `lookup` spelling. For each field moved to a ' + + '`lookup`, its `deleteBehavior` is set on purpose and its default value, if any, is not ' + + '`current_user`. A stored object whose body carries the old shape is served as stored, but the ' + + 'runtime resolves its `user` field to `sys_user` (expand, import, seed loading), and the next save of the object is refused until ' + + 'the field is fixed.', +};