Skip to content

Commit 4e99b57

Browse files
committed
feat(lint,spec): warn on a user field whose reference names an object other than sys_user
WIP: the 18.x loud half under the standing ruling; no ADR-0087 conversion carries the refusal. Claude-Session: https://claude.ai/code/session_01D6F9axfoPzR7t8G1J7Ytzz Co-authored-by: Claude <noreply@anthropic.com>
1 parent b42c621 commit 4e99b57

6 files changed

Lines changed: 223 additions & 6 deletions

File tree

‎packages/lint/src/object-graph.test.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,10 @@ const stack = {
3636
name: 'crm_account',
3737
fields: {
3838
region: { type: 'text', label: 'Region' },
39-
owner: { type: 'user', label: 'Owner', reference: 'crm_person' },
39+
// [#24808] A link to a person OBJECT is a `lookup`: a `user` field's
40+
// target is fixed to `sys_user`, and naming another object with it is
41+
// what `user-field-foreign-reference` warns on.
42+
owner: { type: 'lookup', label: 'Owner', reference: 'crm_person' },
4043
},
4144
},
4245
{ 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
418421

419422
it('control: an EXPLICIT carrier still wins over the constant', () => {
420423
// `reference` MATERIALIZES the constant for `user`; where an author named a
421-
// different object the arbiter returns what they wrote, unchanged. The
422-
// module fixture above relies on this (`owner` → `crm_person`).
423-
expect(resolveFieldPath(graph, 'crm_opportunity', 'account.owner.email'))
424+
// different object the arbiter returns what they wrote, unchanged.
425+
// [#24808] The shape is declared wrong and warned on
426+
// (`user-field-foreign-reference`); this pins its protocol-18 resolution
427+
// until the next major refuses it, so it is built here rather than kept as
428+
// an incidental module fixture.
429+
const foreign = indexObjectGraph({
430+
objects: [
431+
{ name: 'crm_account', fields: { owner: { type: 'user', label: 'Owner', reference: 'crm_person' } } },
432+
{ name: 'crm_person', fields: { email: { type: 'email', label: 'Email' } } },
433+
],
434+
});
435+
expect(resolveFieldPath(foreign, 'crm_account', 'owner.email'))
424436
.toMatchObject({ kind: 'ok', object: 'crm_person', field: 'email' });
425437
});
426438

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

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import {
55
validateObjectReferences as validateObjectReferencesUnrecorded,
66
OBJECT_REFERENCE_UNKNOWN,
77
OBJECT_REFERENCE_UNREGISTERED_PLATFORM,
8+
USER_FIELD_FOREIGN_REFERENCE,
89
} from './validate-object-references.js';
10+
import { Field, ObjectSchema, referenceTargetOf } from '@objectstack/spec/data';
911
import { runAuthoringRules, splitBySeverity } from './authoring-rules.js';
1012
import { PLATFORM_OBJECT_PREFIXES } from '@objectstack/spec/system';
1113
import { explainRule } from './rule-explanations.js';
@@ -707,10 +709,88 @@ describe('[#19289] validateObjectReferences — a `user` target comes from the T
707709
it('control: the rule still REPORTS a real miss — an explicit carrier naming an unknown object errors', () => {
708710
// Without this, "no findings" above reads equally well as a rule that
709711
// stopped judging `user` fields altogether.
712+
// [#24808] A `user` field naming any object but `sys_user` also draws the
713+
// foreign-target warning, so the unknown-object error is read by rule.
710714
const findings = validateObjectReferences(userField({ reference: 'zzz_nope' }));
715+
expect(findings.filter((f) => f.rule === OBJECT_REFERENCE_UNKNOWN)).toHaveLength(1);
716+
expect(findings.map((f) => f.rule).sort()).toEqual([OBJECT_REFERENCE_UNKNOWN, USER_FIELD_FOREIGN_REFERENCE].sort());
717+
});
718+
});
719+
720+
/**
721+
* [#24808] A `user` field's target is FIXED to `sys_user` by the spec, and
722+
* nothing enforced it: the reproduction below parses through `ObjectSchema`
723+
* and resolves through `referenceTargetOf` to the authored object. Under the
724+
* 「甲」 ruling the 18.x half is this warning (no ADR-0087 conversion can carry
725+
* the refusal: a `lookup` joins the referential delete walk a `user` field
726+
* sits outside), so the parse half below pins that the shape still LOADS.
727+
*/
728+
describe('[#24808] validateObjectReferences — a `user` field naming another object warns', () => {
729+
const stackWith = (assignee: Record<string, unknown>) => ({
730+
objects: [
731+
{ name: 'probe_account', fields: { name: { type: 'text', label: 'Name' } } },
732+
{ name: 'probe_task', fields: { name: { type: 'text', label: 'Name' }, owner: assignee } },
733+
],
734+
});
735+
const PROBE = { type: 'user', label: 'Owner', reference: 'probe_account' };
736+
737+
it('reproduction: the shape parses and the arbiter follows the authored object', () => {
738+
const parsed = ObjectSchema.safeParse({ name: 'probe_task', fields: { owner: PROBE } });
739+
expect(parsed.success).toBe(true);
740+
expect(referenceTargetOf(PROBE)).toBe('probe_account');
741+
});
742+
743+
it('warns — never errors — and names the field, the target and the lookup spelling', () => {
744+
const findings = validateObjectReferences(stackWith(PROBE));
711745
expect(findings).toHaveLength(1);
712-
expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN);
746+
const [f] = findings;
747+
expect(f.rule).toBe(USER_FIELD_FOREIGN_REFERENCE);
748+
expect(f.severity).toBe('warning');
749+
expect(f.path).toBe('objects[1].fields.owner.reference');
750+
expect(f.where).toBe('object "probe_task" · field "owner"');
751+
expect(f.message).toContain('"probe_account"');
752+
expect(f.hint).toContain("{ type: 'lookup', reference: 'probe_account' }");
753+
});
754+
755+
it('warns on an object extension too — the verdict needs no artifact context', () => {
756+
const findings = validateObjectReferences({
757+
objectExtensions: [{ extend: 'sys_user_profile', fields: { mentor: { ...PROBE } } }],
758+
});
759+
expect(findings.filter((f) => f.rule === USER_FIELD_FOREIGN_REFERENCE)).toEqual([
760+
expect.objectContaining({ path: 'objectExtensions[0].fields.mentor.reference', severity: 'warning' }),
761+
]);
762+
});
763+
764+
it('warns on the map form of `fields` and on the array form of `objects` alike', () => {
765+
const findings = validateObjectReferences({
766+
objects: { probe_task: { fields: { owner: PROBE } }, probe_account: { fields: {} } },
767+
});
768+
expect(findings.map((f) => f.rule)).toEqual([USER_FIELD_FOREIGN_REFERENCE]);
769+
});
770+
771+
it('control: the two legal spellings — no `reference`, and `reference: "sys_user"` — stay silent', () => {
772+
expect(validateObjectReferences(stackWith({ type: 'user', label: 'Owner' }))).toEqual([]);
773+
expect(validateObjectReferences(stackWith({ type: 'user', label: 'Owner', reference: 'sys_user' }))).toEqual([]);
774+
expect(validateObjectReferences(stackWith(Field.user({ label: 'Owner' })))).toEqual([]);
713775
});
776+
777+
it('control: a `lookup` to the same object is the canonical spelling and draws nothing', () => {
778+
expect(validateObjectReferences(stackWith(Field.lookup('probe_account', { label: 'Owner' })))).toEqual([]);
779+
expect(validateObjectReferences(stackWith({ type: 'lookup', reference: 'probe_account' }))).toEqual([]);
780+
});
781+
782+
it('control: an unreadable carrier still REFUSES rather than reading as a foreign target', () => {
783+
expect(() => validateObjectReferences(stackWith({ type: 'user', reference: { object: 'sys_user' } })))
784+
.toThrow(/`reference` is an object/);
785+
});
786+
787+
for (const command of ['validate', 'build', 'lint'] as const) {
788+
it(`reaches \`os ${command}\` through the authoring registry, as an advisory and never a gate`, () => {
789+
const { errors, advisories } = splitBySeverity(runAuthoringRules(command, { normalized: stackWith(PROBE) as never }));
790+
expect(errors.filter((f) => f.rule === USER_FIELD_FOREIGN_REFERENCE)).toEqual([]);
791+
expect(advisories.filter((f) => f.rule === USER_FIELD_FOREIGN_REFERENCE)).toHaveLength(1);
792+
});
793+
}
714794
});
715795

716796
/**

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

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ import {
8383
hasPlatformObjectPrefix,
8484
isPlatformProvidedObjectName,
8585
PLATFORM_PROVIDED_OBJECT_NAMES,
86+
SystemObjectName,
8687
} from '@objectstack/spec/system';
8788
import { referenceCarrierOf, referenceTargetOf } from '@objectstack/spec/data';
8889

@@ -107,6 +108,31 @@ const RELATIONSHIP_TARGET_FIELD_TYPES: ReadonlySet<string> = new Set(['lookup',
107108
export const OBJECT_REFERENCE_UNKNOWN = 'object-reference-unknown';
108109
export const OBJECT_REFERENCE_UNREGISTERED_PLATFORM = 'object-reference-unregistered-platform';
109110

111+
/**
112+
* [#24808 — ruled A on #24854, timed by the 「甲」 standing ruling] A `user`
113+
* field whose `reference` names an object other than `sys_user`.
114+
*
115+
* The spec declares a `user` field's target FIXED to `sys_user`
116+
* (`Field.user()`'s docblock, `IMPLICIT_REFERENCE_TARGETS` in
117+
* `field-value.zod.ts`), and nothing enforces it: `FieldSchema` admits any
118+
* string, and `referenceTargetOf` returns the authored carrier first, so the
119+
* runtime's target-following readers (`$expand`, the import runner, the seed
120+
* loader, the object graph) resolve the field against the authored object
121+
* while every reader that branches on the TYPE treats its values as users —
122+
* the person picker, the `current_user` default, a sharing rule's
123+
* `fieldHoldsUsers`. The field is half a lookup and half a user.
124+
*
125+
* WARNING, not error, by the 「甲」 rule: a narrowing ships its refusal in an
126+
* 18.x minor only when an ADR-0087 conversion carries it, and this one has no
127+
* lossless rewrite — `user` sits outside the referential delete walk
128+
* (`DELETE_SEMANTICS_REFERENCE_TYPES`, objectql) while a `lookup` materializes
129+
* `deleteBehavior: 'set_null'` and joins it, and `deleteBehavior` has no value
130+
* meaning "do nothing", so `{ type: 'lookup', reference }` would start nulling
131+
* child columns on a delete that leaves them untouched today. The refusal is
132+
* the next protocol major's; this is its loud half.
133+
*/
134+
export const USER_FIELD_FOREIGN_REFERENCE = 'user-field-foreign-reference';
135+
110136
export type ObjectRefSeverity = 'error' | 'warning';
111137

112138
export interface ObjectRefFinding {
@@ -182,6 +208,55 @@ function artifactProvidedObjectNames(stack: AnyRec): string[] {
182208
* Validate every object-name reference on the surfaces listed in the module
183209
* header. Returns findings (empty = clean).
184210
*/
211+
/**
212+
* The {@link USER_FIELD_FOREIGN_REFERENCE} findings for one stack: every
213+
* `user` field, on an object or an object extension, whose `reference` names
214+
* an object other than `sys_user`.
215+
*
216+
* Reads the CARRIER (`referenceCarrierOf`), not `referenceTargetOf`: the
217+
* question is what the author wrote, and the arbiter would answer the same
218+
* string for this shape anyway. An absent carrier and `reference: 'sys_user'`
219+
* are the two legal spellings and produce nothing; an unreadable carrier
220+
* throws through the accessor exactly as it does for the resolution leg.
221+
* Interpolated targets are not exempted: the type fixes the target, so no
222+
* render-time value can make another object right.
223+
*/
224+
function userFieldForeignReferenceFindings(stack: AnyRec): ObjectRefFinding[] {
225+
const out: ObjectRefFinding[] = [];
226+
const legs: Array<{ key: 'objects' | 'objectExtensions'; ownName: (rec: AnyRec) => string | undefined }> = [
227+
{ key: 'objects', ownName: (rec) => strName(rec.name) },
228+
{ key: 'objectExtensions', ownName: (rec) => strName(rec.extend) },
229+
];
230+
for (const { key, ownName } of legs) {
231+
const containers = recordsOf(stack[key]);
232+
for (let ci = 0; ci < containers.length; ci++) {
233+
const container = containers[ci];
234+
const containerName = ownName(container) ?? `#${ci}`;
235+
const fields = recordsOf(container.fields);
236+
for (let fi = 0; fi < fields.length; fi++) {
237+
const field = fields[fi];
238+
if (field.type !== 'user') continue;
239+
const target = referenceCarrierOf(field, 'validateObjectReferences');
240+
if (!target || target === SystemObjectName.USER) continue;
241+
const fieldName = strName(field.name) ?? `#${fi}`;
242+
out.push({
243+
severity: 'warning',
244+
rule: USER_FIELD_FOREIGN_REFERENCE,
245+
where: `${key === 'objects' ? 'object' : 'object extension'} "${containerName}" · field "${fieldName}"`,
246+
path: `${key}[${ci}].fields.${fieldName}.reference`,
247+
message:
248+
`user field "${fieldName}" names reference "${target}", but a user field's target is fixed to ` +
249+
`sys_user; it acts as a link to "${target}" whose values user-only readers take for users.`,
250+
hint:
251+
`Declare a lookup instead, { type: 'lookup', reference: '${target}' }, or drop \`reference\` ` +
252+
'if a person was meant.',
253+
});
254+
}
255+
}
256+
}
257+
return out;
258+
}
259+
185260
export function validateObjectReferences(stack: AnyRec): ObjectRefFinding[] {
186261
const findings: ObjectRefFinding[] = [];
187262
if (!stack || typeof stack !== 'object') return findings;
@@ -340,6 +415,13 @@ export function validateObjectReferences(stack: AnyRec): ObjectRefFinding[] {
340415
}
341416
}
342417

418+
// ── `user` fields → a target other than `sys_user` (#24808) ──
419+
// See {@link userFieldForeignReferenceFindings}. Walks `objectExtensions`
420+
// too: unlike the resolution leg above, this verdict needs no artifact
421+
// context — a `user` field naming any object but `sys_user` is the same
422+
// contradiction wherever the field is declared.
423+
findings.push(...userFieldForeignReferenceFindings(stack));
424+
343425
// ── View containers → the container's own `object` (#20216) ──
344426
// `ViewSchema.object` is `z.string()`, and nothing resolved it: `defineStack`'s
345427
// `validateCrossReferences` reads a container's `list.data` / `form.data`

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ describe('semantic type classes', () => {
8888
expect(referenceTargetOf({ type: 'user' })).toBe('sys_user');
8989
expect(referenceTargetOf({ type: 'user', reference: 'sys_user' })).toBe('sys_user');
9090
// An explicit target still wins — nothing here overrides authored metadata.
91+
// [#24808] This pins the protocol-18 behaviour on purpose, not a producer:
92+
// the shape is declared wrong (the target is fixed) and `@objectstack/lint`
93+
// warns on it (`user-field-foreign-reference`), but no ADR-0087 conversion
94+
// carries its refusal, so under the 「甲」 ruling it keeps resolving as
95+
// written until the next protocol major refuses it at parse.
9196
expect(referenceTargetOf({ type: 'user', reference: 'my_people' })).toBe('my_people');
9297

9398
// Genuinely targetless: the types whose target IS author-chosen, unwritten.

‎packages/spec/src/data/field-value.zod.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,17 @@ export const REFERENCE_VALUE_TYPES: ReadonlySet<string> = new Set([
167167
* `reference` on a `user` field materializes that constant; it does not supply
168168
* it. Metadata authored without it (hand-written JSON, an AI author, a Studio
169169
* form) is fully specified, not under-specified.
170+
*
171+
* ⚠️ [#24808] Declared, not yet enforced: `FieldSchema` admits any string on a
172+
* `user` field's `reference`, and {@link referenceTargetOf} returns an
173+
* authored carrier FIRST, so `{ type: 'user', reference: 'probe_account' }`
174+
* resolves to `probe_account`. Under the 「甲」 ruling that stays so for
175+
* protocol 18 — no ADR-0087 conversion can rewrite such a field losslessly (a
176+
* `lookup` joins the referential delete walk a `user` field sits outside), so
177+
* re-targeting it here would change a loading field's behaviour unannounced.
178+
* The loud half ships instead: `@objectstack/lint`'s
179+
* `user-field-foreign-reference` warning. The refusal, and with it the
180+
* carrier ceasing to matter here, is the next protocol major's.
170181
*/
171182
const IMPLICIT_REFERENCE_TARGETS: ReadonlyMap<string, string> = new Map([
172183
['user', SystemObjectName.USER],

‎packages/spec/src/data/field.zod.ts‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3032,6 +3032,22 @@ export type CurrencyValue = z.input<typeof CurrencyValueSchema>;
30323032
*/
30333033
export type FieldInput = Omit<Partial<Field>, 'type'>;
30343034

3035+
/**
3036+
* The `Field.user()` config: {@link FieldInput} with `reference` marked
3037+
* deprecated. A `user` field's target is a constant of the type
3038+
* (`IMPLICIT_REFERENCE_TARGETS`, `field-value.zod.ts`), so the key has nothing
3039+
* to say; it is still ACCEPTED (an identical accept set — the narrowing waits
3040+
* for the next protocol major), and a foreign value is warned on by
3041+
* `user-field-foreign-reference`.
3042+
*/
3043+
type UserFieldInput = Omit<FieldInput, 'reference'> & {
3044+
/**
3045+
* @deprecated A `user` field always targets `sys_user`; omit this key. To
3046+
* link another object, use `Field.lookup(reference, …)`.
3047+
*/
3048+
reference?: string;
3049+
};
3050+
30353051
/** `Field.select` with inline options — the array or `{ options }` forms. */
30363052
function selectWithOptions(optionsOrConfig: Array<SelectOption | string> | Omit<FieldInput, 'options'> & { options: Array<SelectOption | string> }, config?: FieldInput) {
30373053
// Helper function to convert string to lowercase snake_case
@@ -3200,8 +3216,19 @@ export const Field = {
32003216
* Field.user({ label: 'Watchers', multiple: true })
32013217
* @example Auto-fill the acting user on create
32023218
* Field.user({ label: 'Reporter', defaultValue: 'current_user' })
3219+
*
3220+
* ⚠️ [#24808] `config.reference` is deprecated, and naming any object but
3221+
* `sys_user` with it is the contradiction `os validate` / `os build` /
3222+
* `os lint` warn on (`user-field-foreign-reference`): such a field is
3223+
* resolved against the named object by every target-following reader while
3224+
* every user-only reader takes its values for users. A link to another
3225+
* object is `Field.lookup(reference, …)`. The spread stays LAST on purpose:
3226+
* moving `reference: 'sys_user'` after it would silently re-target a field
3227+
* an untyped caller wrote, the same unannounced behaviour change the 「甲」
3228+
* ruling keeps out of an 18.x minor — the refusal belongs to the next
3229+
* protocol major, and until then the warning is the loud half.
32033230
*/
3204-
user: (config: FieldInput = {}) => ({
3231+
user: (config: UserFieldInput = {}) => ({
32053232
type: 'user',
32063233
reference: 'sys_user',
32073234
...config,

0 commit comments

Comments
 (0)