feat(spec)!: refuse a user field whose reference names an object other than sys_user; every user field resolves to sys_user

.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
+
+<!-- adr-0087: registered user-field-foreign-reference-refused -->
+
+**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.

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';
 

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', () => {

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: \'<object>\' }`, or ' +
-      '`Field.lookup(\'<object>\', …)`. 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<Record<string, RuleExplanation>> = 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,

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<string, unknown>) => ({
-    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`),