Skip to content

Commit ade7be4

Browse files
os-zhuangclaude
andauthored
fix(lint): the seven system-field exemption lists derive from the spec's declarations (#4330) (#4339)
Five rules each carried their own hand-copy of "registry-injected columns present on almost every object but absent from authored fields", and they had already drifted — two more copies (validate-translation-references' extended list, validate-hook-body-writes' union) appeared while the issue was open. Same shape #3786 removed from the audit-provenance family. One module now owns the answer: system-fields.ts derives SYSTEM_FIELDS from FIELD_GROUP_SYSTEM_FIELDS (@objectstack/spec/data) + SystemFieldName (@objectstack/spec/system), and all seven field-resolving rules consume it. A pin test holds the boundary both ways: exactly the two declarations' union, and none of the rule-local exemptions. The two judgement calls the issue asked for, made deliberately: - The derived union is a superset of the four identical copies (adds is_deleted) and of flow-template-paths' set (adds user_id). Both are the permissive direction the rules' own comments argue for: over-inclusion costs at worst a missed warning, under-inclusion a false one. - name / owner / record_type (and the legacy physical spellings _id / space) are NOT spec system columns — name is an ordinary authored field on most objects — so they stay rule-local next to each rule's reason (IMPLICIT_FIELDS / IMPLICIT_HEADS extensions) instead of widening every field-existence rule in the package. Closes #4330 Claude-Session: https://claude.ai/code/session_01D8QhQz2V6VH1SX5SQ1nAgk Co-authored-by: Claude <noreply@anthropic.com>
1 parent fc3bd1c commit ade7be4

10 files changed

Lines changed: 173 additions & 106 deletions
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
fix(lint): the seven system-field exemption lists derive from the spec's declarations (#4330)
6+
7+
Five rules in `@objectstack/lint` each carried their own hand-copy of
8+
"registry-injected columns present on almost every object but absent from
9+
authored `fields`" — and they had already drifted from one another (two more
10+
copies had appeared by the time the fix landed). This is the shape #3786
11+
removed from the audit-provenance family, rebuilt one package over: the same
12+
list, maintained in parallel, each under a comment asking to be kept in sync
13+
with one of the others.
14+
15+
The package now has one module, `system-fields.ts`, whose `SYSTEM_FIELDS` is
16+
DERIVED from the spec's two declarations — `FIELD_GROUP_SYSTEM_FIELDS`
17+
(`@objectstack/spec/data`) and `SystemFieldName` (`@objectstack/spec/system`)
18+
— and all seven field-resolving rules consume it. A pin test holds the
19+
boundary in both directions: the set contains exactly the two declarations'
20+
union, and none of the rule-local exemptions.
21+
22+
Two deliberate behavior consequences, both in the permissive direction the
23+
rules' own comments argue for (over-inclusion costs at worst a missed
24+
warning; under-inclusion costs a false one):
25+
26+
- `widget-bindings`, `page-field-bindings` and `react-page-props` now also
27+
exempt `is_deleted`;
28+
- `flow-template-paths` now also exempts `user_id`.
29+
30+
Names that are NOT system columns in the spec's sense (`name`, `owner`,
31+
`record_type`, and the legacy physical spellings `_id` / `space`) stay
32+
rule-local next to the reason each rule exempts them, instead of widening
33+
every rule: `name` in particular is an ordinary authored field on most
34+
objects, and exempting it package-wide would stop the field-existence rules
35+
from catching a reference to a field the object genuinely does not have.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { FIELD_GROUP_SYSTEM_FIELDS } from '@objectstack/spec/data';
5+
import { SystemFieldName } from '@objectstack/spec/system';
6+
import { SYSTEM_FIELDS } from './system-fields.js';
7+
8+
describe('SYSTEM_FIELDS (#4330)', () => {
9+
it('contains every member of both spec declarations — the derivation is complete', () => {
10+
for (const f of FIELD_GROUP_SYSTEM_FIELDS) {
11+
expect(SYSTEM_FIELDS.has(f), f).toBe(true);
12+
}
13+
for (const f of Object.values(SystemFieldName)) {
14+
expect(SYSTEM_FIELDS.has(f), f).toBe(true);
15+
}
16+
});
17+
18+
it('contains nothing BEYOND the two spec declarations — additions go to the spec, not here', () => {
19+
const declared = new Set<string>([
20+
...FIELD_GROUP_SYSTEM_FIELDS,
21+
...Object.values(SystemFieldName),
22+
]);
23+
for (const f of SYSTEM_FIELDS) {
24+
expect(declared.has(f), f).toBe(true);
25+
}
26+
});
27+
28+
it('excludes the rule-local exemptions — they are decisions, not system columns', () => {
29+
// `name` is an ordinary authored field on most objects; `owner` and
30+
// `record_type` are not registry-injected; `_id` and `space` are legacy
31+
// physical spellings. Rules that exempt them do so locally, next to their
32+
// reason (see the module note). Putting any of them here would silently
33+
// stop every consumer from catching a reference to a genuinely missing
34+
// field — this test is where that accidental widening fails.
35+
for (const f of ['name', 'owner', 'record_type', '_id', 'space']) {
36+
expect(SYSTEM_FIELDS.has(f), f).toBe(false);
37+
}
38+
});
39+
});

packages/lint/src/system-fields.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The ONE answer to "which columns does the registry inject on (almost) every
5+
* object without them appearing in authored `fields`?" (#4330).
6+
*
7+
* Every field-resolving rule in this package needs that answer: a reference to
8+
* `created_at` or `owner_id` is authored against a real, addressable column
9+
* even though no object declares it, so flagging it would be the false finding
10+
* that makes authors stop trusting the linter (ADR-0072 D1). Before this
11+
* module, five rules each carried their own hand-copied list and had already
12+
* drifted from one another — the exact shape #3786 removed from the
13+
* audit-provenance family, rebuilt one package over.
14+
*
15+
* DERIVED from the spec's two declarations, so it cannot drift from them:
16+
*
17+
* - {@link FIELD_GROUP_SYSTEM_FIELDS} (`@objectstack/spec/data`) — audit
18+
* provenance plus `organization_id` / `tenant_id` / `is_deleted` /
19+
* `deleted_at`;
20+
* - {@link SystemFieldName} (`@objectstack/spec/system`) — the protocol-level
21+
* ids: `id`, `owner_id`, `user_id` and the timestamp/tenant columns.
22+
*
23+
* The union is deliberately generous, because the cost asymmetry is the same
24+
* in every consumer: over-inclusion costs at worst a missed finding on a
25+
* `systemFields: false` object (rare); under-inclusion costs a false one.
26+
*
27+
* What does NOT belong here: names that are ordinary AUTHORED fields on most
28+
* objects (`name`, `owner`, `record_type`) or legacy physical spellings
29+
* (`_id`, `space`). A rule that deliberately exempts those keeps them in a
30+
* rule-local extension next to its reason — adding them here would silently
31+
* stop every other rule from catching a reference to a field the object
32+
* genuinely does not have.
33+
*/
34+
35+
import { FIELD_GROUP_SYSTEM_FIELDS } from '@objectstack/spec/data';
36+
import { SystemFieldName } from '@objectstack/spec/system';
37+
38+
/**
39+
* Registry-injected columns addressable at runtime without being authored in
40+
* `fields` — the union of the spec's two system-field declarations.
41+
*/
42+
export const SYSTEM_FIELDS: ReadonlySet<string> = new Set<string>([
43+
...FIELD_GROUP_SYSTEM_FIELDS,
44+
...Object.values(SystemFieldName),
45+
]);

packages/lint/src/validate-flow-template-paths.ts

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@
5555
// - Structured scalar heads (`json` / `composite` / `repeater` / `record`) may
5656
// carry legitimate sub-paths — their `.<sub>` access is left alone.
5757

58+
import { SYSTEM_FIELDS } from './system-fields.js';
59+
5860
export type FlowTemplatePathSeverity = 'error' | 'warning';
5961

6062
export interface FlowTemplatePathFinding {
@@ -86,24 +88,16 @@ function asArray(v: unknown): AnyRec[] {
8688
return [];
8789
}
8890

89-
// System/audit columns the platform injects on every object — always
90-
// addressable in a `{record.<col>}` template even though they are not authored
91-
// fields. Mirrors `validateFormLayout`'s system-field set plus the audit/tenant
92-
// columns from `FIELD_GROUP_SYSTEM_FIELDS`.
93-
const SYSTEM_FIELDS: ReadonlySet<string> = new Set([
94-
'id',
95-
'name',
96-
'owner',
97-
'owner_id',
98-
'created_at',
99-
'created_by',
100-
'updated_at',
101-
'updated_by',
102-
'organization_id',
103-
'tenant_id',
104-
'is_deleted',
105-
'deleted_at',
106-
'record_type',
91+
// Path heads addressable in a `{record.<col>}` template without being authored
92+
// fields: the package-shared registry-injected columns (`system-fields.ts`,
93+
// #4330) plus three heads this rule has always exempted. `name`, `owner` and
94+
// `record_type` are NOT registry-injected system columns (`name` in particular
95+
// is an ordinary authored field on most objects), so they stay rule-local —
96+
// see the shared module's note — instead of widening every field-existence
97+
// rule in the package.
98+
const IMPLICIT_HEADS: ReadonlySet<string> = new Set([
99+
...SYSTEM_FIELDS,
100+
'name', 'owner', 'record_type',
107101
]);
108102

109103
// Field types that address ANOTHER object — a `.<subfield>` hop through one is
@@ -332,7 +326,7 @@ export function validateFlowTemplatePaths(stack: AnyRec): FlowTemplatePathFindin
332326
// A trailing numeric segment is an array index (#1872), not a hop.
333327
const nextIsIdentifier = hasSubPath && !/^\d+$/.test(rest[1]);
334328

335-
const isKnown = fieldTypes.has(head) || SYSTEM_FIELDS.has(head);
329+
const isKnown = fieldTypes.has(head) || IMPLICIT_HEADS.has(head);
336330

337331
if (!isKnown) {
338332
if (seenUnknown.has(head)) continue;

packages/lint/src/validate-hook-body-writes.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ import { createRequire } from 'node:module';
4343
import type ts from 'typescript';
4444
import { findClosestMatches, formatSuggestion } from '@objectstack/spec/shared';
4545

46+
import { SYSTEM_FIELDS } from './system-fields.js';
47+
4648
// The TypeScript compiler must NOT be imported at module top level: it is
4749
// ~9 MB of CJS, and @objectstack/lint sits on the kernel boot path — while
4850
// this gate only parses when a hook actually carries an L2 JS body. Same
@@ -175,18 +177,17 @@ const API_WRITE_METHODS: ReadonlyMap<string, number> = new Map([
175177
const INPUT_ENVELOPE_KEYS: ReadonlySet<string> = new Set(['id', 'options', 'ast', 'data']);
176178

177179
/**
178-
* Registry-injected columns present on (almost) every object but absent from
179-
* `object.fields` — always legitimately writable by automation. The UNION of
180-
* the sets the other field-resolving rules carry, because the cost asymmetry
181-
* is the same everywhere: over-inclusion is at worst a missed finding,
182-
* under-inclusion is a false one.
180+
* Columns always legitimately writable by automation without appearing in
181+
* `object.fields`: the package-shared registry-injected columns
182+
* (`system-fields.ts`, #4330) plus the UNION of its sibling rules' local
183+
* exemptions (`_id`/`name`/`space` from validate-translation-references,
184+
* `name`/`owner`/`record_type` from validate-flow-template-paths) — because
185+
* the cost asymmetry is the same everywhere: over-inclusion is at worst a
186+
* missed finding, under-inclusion is a false one.
183187
*/
184-
const SYSTEM_FIELDS: ReadonlySet<string> = new Set([
185-
'id', '_id', 'name', 'space',
186-
'owner', 'owner_id', 'user_id',
187-
'created_at', 'created_by', 'updated_at', 'updated_by',
188-
'organization_id', 'tenant_id',
189-
'is_deleted', 'deleted_at', 'record_type',
188+
const IMPLICIT_FIELDS: ReadonlySet<string> = new Set([
189+
...SYSTEM_FIELDS,
190+
'_id', 'name', 'space', 'owner', 'record_type',
190191
]);
191192

192193
type AnyRec = Record<string, unknown>;
@@ -414,7 +415,7 @@ export function validateHookBodyWrites(stack: AnyRec): HookBodyWriteFinding[] {
414415
// missing on EVERY named target (a multi-target body may branch per
415416
// object, so a partial miss is not statically wrong).
416417
if (!inputJudgeable) continue;
417-
if (SYSTEM_FIELDS.has(w.field)) continue;
418+
if (IMPLICIT_FIELDS.has(w.field)) continue;
418419
if (targetSets.some((s) => s!.has(w.field))) continue;
419420

420421
reported.add(dedupeKey);
@@ -438,7 +439,7 @@ export function validateHookBodyWrites(stack: AnyRec): HookBodyWriteFinding[] {
438439
// ctx.api write → the named object.
439440
const known = objectFields!.get(w.object);
440441
if (!known) continue; // object declared by another package — cannot judge
441-
if (SYSTEM_FIELDS.has(w.field) || known.has(w.field)) continue;
442+
if (IMPLICIT_FIELDS.has(w.field) || known.has(w.field)) continue;
442443

443444
reported.add(dedupeKey);
444445
findings.push({
@@ -468,7 +469,7 @@ function unionCandidates(targetSets: ReadonlyArray<Set<string> | undefined>): st
468469

469470
/** Did-you-mean (declared + system columns as candidates) plus the fix. */
470471
function fixHint(field: string, declared: string[]): string {
471-
const suggestion = formatSuggestion(findClosestMatches(field, [...declared, ...SYSTEM_FIELDS]));
472+
const suggestion = formatSuggestion(findClosestMatches(field, [...declared, ...IMPLICIT_FIELDS]));
472473
return (
473474
(suggestion ? `${suggestion} ` : '') +
474475
`Fix the field name, or declare '${field}' on the object. Only the literal write patterns in ` +

packages/lint/src/validate-page-field-bindings.ts

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -59,22 +59,10 @@ export interface PageFieldFinding {
5959
}
6060

6161
import { walkPageComponents, type AnyRec } from './page-walk.js';
62-
63-
/**
64-
* Registry-injected fields present on (almost) every object but NOT declared in
65-
* `object.fields`. Copied verbatim from `validate-widget-bindings` so the two
66-
* rules agree on what counts as a field. Deliberately generous: over-inclusion
67-
* costs at worst a missed warning on a `systemFields: false` object; under-
68-
* inclusion costs a false one, and a false finding is what makes authors stop
69-
* trusting the linter (ADR-0072 D1). Real pages DO reference these — e.g.
70-
* `sys_user.page.ts` lists `created_at` in a related-list's columns.
71-
*/
72-
const SYSTEM_FIELDS = new Set<string>([
73-
'id',
74-
'created_at', 'created_by', 'updated_at', 'updated_by',
75-
'owner_id', 'organization_id', 'tenant_id', 'user_id',
76-
'deleted_at',
77-
]);
62+
// Real pages DO reference registry-injected columns — e.g. `sys_user.page.ts`
63+
// lists `created_at` in a related-list's columns — so the shared set is load-
64+
// bearing here, not merely defensive.
65+
import { SYSTEM_FIELDS } from './system-fields.js';
7866

7967
function asArray(v: unknown): AnyRec[] {
8068
if (Array.isArray(v)) return v as AnyRec[];

packages/lint/src/validate-react-page-props.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ import {
3535
indexObjectSearchTargets,
3636
} from './validate-searchable-fields.js';
3737

38+
import { SYSTEM_FIELDS } from './system-fields.js';
39+
3840
// The TypeScript compiler must NOT be imported at module top level: it is
3941
// ~9 MB of CJS (~70 ms+ to parse, worse on container cold starts), and
4042
// @objectstack/lint sits on the kernel boot path — while this gate only runs
@@ -209,19 +211,6 @@ export const REACT_CHART_AXIS_UNKNOWN = 'react-chart-axis-unknown';
209211

210212
const CHART_FUNCTIONS = ['count', 'sum', 'avg', 'min', 'max'] as const;
211213

212-
/**
213-
* Registry-injected fields present on (almost) every object but absent from
214-
* `object.fields`. Same set as `validate-page-field-bindings`, for the same
215-
* reason: over-inclusion costs at worst a missed finding, under-inclusion
216-
* costs a false one.
217-
*/
218-
const SYSTEM_FIELDS = new Set<string>([
219-
'id',
220-
'created_at', 'created_by', 'updated_at', 'updated_by',
221-
'owner_id', 'organization_id', 'tenant_id', 'user_id',
222-
'deleted_at',
223-
]);
224-
225214
/**
226215
* Both `objects` and an object's `fields` are authored either as an array of
227216
* `{ name }` records or as a name-keyed map — normalize to the array form, the

packages/lint/src/validate-searchable-fields.ts

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@
5757
* 2. An object that declares no field map at all — external objects and
5858
* datasource-introspected schemas whose columns are resolved at runtime.
5959
* 3. Registry-injected system columns, which are searchable at runtime but
60-
* never appear in authored `fields`. Derived from the spec's own
61-
* declarations rather than hand-copied: this package already carries five
62-
* slightly-different copies of that list (#3786's lesson, unlearned).
60+
* never appear in authored `fields`the package-shared `SYSTEM_FIELDS`
61+
* (`system-fields.ts`), derived from the spec's own declarations rather
62+
* than hand-copied (#4330).
6363
*
6464
* Dotted paths are NOT skipped here, unlike every sibling rule. Elsewhere
6565
* `owner_id.name` is left alone because the query engine resolves the traversal;
@@ -68,8 +68,7 @@
6868
* one wrong spelling most likely to be borrowed from `select`/`sort`.
6969
*/
7070

71-
import { FIELD_GROUP_SYSTEM_FIELDS } from '@objectstack/spec/data';
72-
import { SystemFieldName } from '@objectstack/spec/system';
71+
import { SYSTEM_FIELDS } from './system-fields.js';
7372

7473
export const SEARCHABLE_FIELD_UNKNOWN = 'searchable-field-unknown';
7574

@@ -92,18 +91,6 @@ export interface SearchableFieldFinding {
9291

9392
type AnyRec = Record<string, unknown>;
9493

95-
/**
96-
* Registry-injected columns that are addressable at runtime without being
97-
* authored in `fields`. DERIVED from the spec's two declarations —
98-
* `FIELD_GROUP_SYSTEM_FIELDS` (audit provenance + tenant + soft-delete) and
99-
* `SystemFieldName` (the protocol-level ids) — so it cannot drift from them the
100-
* way five hand-copied variants in this package already have.
101-
*/
102-
const SYSTEM_FIELDS: ReadonlySet<string> = new Set<string>([
103-
...FIELD_GROUP_SYSTEM_FIELDS,
104-
...Object.values(SystemFieldName),
105-
]);
106-
10794
/** Coerce a collection (array or name-keyed map) to an array of records. */
10895
function asArray(v: unknown): AnyRec[] {
10996
if (Array.isArray(v)) return v as AnyRec[];

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767

6868
import { hasPlatformObjectPrefix, isPlatformProvidedObjectName } from '@objectstack/spec/system';
6969
import { walkPageComponents } from './page-walk.js';
70+
import { SYSTEM_FIELDS } from './system-fields.js';
7071

7172
export const TRANSLATION_TARGET_UNKNOWN = 'translation-target-unknown';
7273
export const TRANSLATION_OPTION_KEY_UNKNOWN = 'translation-option-key-unknown';
@@ -163,15 +164,17 @@ function listNames(names: Iterable<string>, max = 12): string {
163164
}
164165

165166
/**
166-
* Audit/system fields every object carries implicitly. A bundle translating
167-
* them is authoring against a real field, so they must not read as orphans.
168-
* Mirrors `validate-page-field-bindings`' list.
167+
* Fields a bundle may translate without reading as orphans: the package-shared
168+
* registry-injected columns (`system-fields.ts`, #4330) plus three exemptions
169+
* this rule has carried since it landed — `_id` and `space` are legacy
170+
* physical spellings older bundles still key, and `name` is an ordinary
171+
* authored field on most objects. None of the three is a system column in the
172+
* spec's sense, so they stay rule-local (see the shared module's note) instead
173+
* of widening every field-existence rule in the package.
169174
*/
170-
const SYSTEM_FIELDS: ReadonlySet<string> = new Set([
171-
'id', '_id', 'name',
172-
'created_at', 'created_by', 'updated_at', 'updated_by',
173-
'owner_id', 'organization_id', 'tenant_id', 'user_id',
174-
'is_deleted', 'deleted_at', 'space',
175+
const IMPLICIT_FIELDS: ReadonlySet<string> = new Set([
176+
...SYSTEM_FIELDS,
177+
'_id', 'name', 'space',
175178
]);
176179

177180
/** Everything a bundle may legally name under one object. */
@@ -502,7 +505,7 @@ export function validateTranslationReferences(stack: AnyRec): TranslationRefFind
502505
const fieldPath = `${objPath}.fields.${fieldName}`;
503506
const field = facts.fields.get(fieldName);
504507
if (!field) {
505-
if (SYSTEM_FIELDS.has(fieldName)) continue;
508+
if (IMPLICIT_FIELDS.has(fieldName)) continue;
506509
orphan(
507510
`${inLocale} · object "${objectName}" · field "${fieldName}"`,
508511
fieldPath,

0 commit comments

Comments
 (0)