feat(lint,spec): warn on a user field whose reference names an object other than sys_user (18.x loud half; refusal is the next major's)

.changeset/24808-user-field-foreign-reference-warning.md

@@ -0,0 +1,15 @@
+---
+'@objectstack/lint': minor
+'@objectstack/spec': patch
+---
+
+feat(lint,spec): `os validate`, `os build` and `os lint` warn on a `user` field whose `reference` names an object other than `sys_user`, and `Field.user()` marks `reference` deprecated
+
+Clause-②: yes (widening)
+
+- **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, `os validate` passed it, and `referenceTargetOf` resolved it to `probe_account`. So every reader that follows the target (`$expand`, imports, seed loading, the object graph) treated the field as a link to `probe_account`, while every reader that branches on the type (the person picker, the `current_user` default, a sharing rule over the users a field holds) treated its values as users.
+- **What changed.** A new advisory, `user-field-foreign-reference`, fires on such a field on an object or an object extension. It names the field and the target, and its hint is the canonical spelling: `{ type: 'lookup', reference: '<target>' }` (`Field.lookup('<target>', …)`), or drop `reference` if a person was meant. It is a warning, so no command's exit code changes. `Field.user()`'s config now types `reference` as `@deprecated`. The accepted values are unchanged.
+- **Why only a warning in 18.x.** A narrowing ships its refusal in an 18.x minor only when an ADR-0087 conversion can rewrite the old shape. This one cannot, because the rewrite is not lossless. A `user` field is outside the referential delete walk, while a `lookup` gets `deleteBehavior: 'set_null'` and joins it. `deleteBehavior` has no value that means "do nothing", so a converted field would start clearing child columns when its target record is deleted, which a `user` field never does. The refusal is planned for the next major.
+- **Published.** `USER_FIELD_FOREIGN_REFERENCE` (`user-field-foreign-reference`) is exported from `@objectstack/lint` beside `OBJECT_REFERENCE_UNKNOWN`, and `RULE_EXPLANATIONS` gains its entry, so `os explain user-field-foreign-reference` prints why the field is half a link and half a user, and why the rewrite to a `lookup` is left to the author.
+- **Unchanged.** Parsing, the runtime's resolution of such a field (it still follows the authored `reference`), `Field.user()`'s output, and `user` fields written without `reference` or with `reference: 'sys_user'`.
+- **What to do.** Change a `user` field that points at another object to a `lookup` on that object. No in-repo app or package wrote this shape. Two test fixtures did: one is now a `lookup`, and the other deliberately pins the 18.x resolution.

packages/lint/src/index.ts

@@ -426,6 +426,7 @@ 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

@@ -36,7 +36,10 @@ const stack = {
       name: 'crm_account',
       fields: {
         region: { type: 'text', label: 'Region' },
-        owner: { type: 'user', label: 'Owner', reference: 'crm_person' },
+        // [#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.
+        owner: { type: 'lookup', label: 'Owner', reference: 'crm_person' },
       },
     },
     { name: 'crm_person', fields: { email: { type: 'email', label: 'Email' } } },
@@ -418,9 +421,18 @@ describe('[#19289] object-graph — a `user` field takes its target from the TYP
 
   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'))
+    // 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.
+    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' } } },
+      ],
+    });
+    expect(resolveFieldPath(foreign, 'crm_account', 'owner.email'))
       .toMatchObject({ kind: 'ok', object: 'crm_person', field: 'email' });
   });
 

packages/lint/src/rule-explanations.ts

@@ -3562,6 +3562,28 @@ 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 = {
@@ -4858,6 +4880,7 @@ 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,7 +5,9 @@ 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';
@@ -707,10 +709,99 @@ describe('[#19289] validateObjectReferences — a `user` target comes from the T
   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.
+    // [#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());
+  });
+});
+
+/**
+ * [#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);
-    expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN);
+    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);
+    });
+  }
 });
 
 /**

packages/lint/src/validate-object-references.ts

@@ -83,6 +83,7 @@ import {
   hasPlatformObjectPrefix,
   isPlatformProvidedObjectName,
   PLATFORM_PROVIDED_OBJECT_NAMES,
+  SystemObjectName,
 } from '@objectstack/spec/system';
 import { referenceCarrierOf, referenceTargetOf } from '@objectstack/spec/data';
 
@@ -107,6 +108,31 @@ const RELATIONSHIP_TARGET_FIELD_TYPES: ReadonlySet<string> = 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 {
@@ -182,6 +208,55 @@ 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;
@@ -340,6 +415,13 @@ 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`

packages/spec/src/data/field-value.test.ts

@@ -88,6 +88,11 @@ describe('semantic type classes', () => {
     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');
 
     // Genuinely targetless: the types whose target IS author-chosen, unwritten.