fix(lint,spec,objectql): resolve registry-injected system columns at author time (#5378) (#5904)

`buildFieldIndex` (validate-expressions.ts) and the `highlightFields`
existence check (validate-semantic-roles.ts) resolved field references
against the AUTHORED `fields` map only, so every registry-injected system
column was invisible: `has(record.owner_id)` was hard-rejected as an unknown
field and `highlightFields: ['owner_id']` warned that the column "is not a
field on this object". The platform's own linter denied the platform's own
contract, and apps escaped it by re-declaring system columns — hotcrm#548
declared `owner_id` on all 12 business objects for exactly this reason.

Add `resolveInjectedSystemColumns()` (@objectstack/spec/data): a pure
per-object derivation of WHICH system columns the platform provisions, and
make the registry's `applySystemFields()` consume it. Same split #3786
established for the audit family — the spec declares which columns exist,
the registry owns what each one looks like. `@objectstack/lint` (contract:
depends on spec, never on a runtime) reads the same derivation, so the
author-time verdict cannot disagree with the runtime that provisions the
column.

The union is CONDITIONAL per object, not a blanket allowance of every system
name: `ownership: 'org' | 'none'` gets no `owner_id`, so `record.owner_id`
there is still the error it should be; `tenancy.enabled: false` gets no
`organization_id`; `systemFields: { audit: false }` gets no audit family;
`systemFields: false` / `managedBy: 'better-auth'` get nothing but the
driver-provisioned `id`. Injected names also join the "did you mean?"
candidates.

The type-soundness and null-guard indexes deliberately stay declared-only
(reasons at each site): column types and nullability belong to the registry's
definitions, and the null-guard index feeds a build-breaking verdict.

Injection behaviour is unchanged — a new parity pin compares the derivation
against the live injection pass across the full condition matrix.

Fixes #5378

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/injected-system-columns-author-time.md

@@ -0,0 +1,44 @@
+---
+"@objectstack/spec": minor
+"@objectstack/objectql": patch
+"@objectstack/lint": patch
+---
+
+fix(lint,spec,objectql): 编写期表达式与 `highlightFields` 校验识别注册表注入的系统列 (#5378)
+
+平台在每个业务对象上注入 `owner_id` / `created_at` / `organization_id` 等系统列,
+文档也把 `ownership: 'user'` 写作 "injects reassignable owner_id"。但编写期的两处
+校验只读**作者声明的** `fields`,于是注入列一律当作不存在:
+
+- `validate-expressions.ts` 的 `buildFieldIndex` 让 `has(record.owner_id)` 直接
+  报错 `unknown field owner_id`;
+- `highlightFields` 存在性检查对 `['owner_id']` 发出 "is not a field on this
+  object — it is silently skipped by every consumer"。
+
+也就是平台自己的 linter 否认平台自己的契约。结果是应用被迫**重声明系统列**才能通过
+编写期校验:hotcrm#548 为此在全部 12 个业务对象上显式声明了 `owner_id`(6 个对象曾
+报 `highlightFields` 警告,`contact_welcome` 触发器的 `has(record.owner_id)` 被硬
+拒)。这正是本项目视为缺陷的形状:能力已声明(列确实注入且有文档),但执行层不认。
+
+**权威来源只有一份。** 新增纯派生 `resolveInjectedSystemColumns()`
+(`@objectstack/spec/data`)回答"这个对象带哪些系统列",并由 registry 的
+`applySystemFields()` **消费**它——沿用 #3786 为审计字段族确立的分工:spec 声明
+**有哪些**列,registry 拥有**每列长什么样**。lint 通过同一派生取答案,因此编写期
+判断与运行时注入不可能不一致(`@objectstack/lint` 的包契约是"只依赖 spec,绝不依赖
+运行时",此前它根本无法读到权威)。两个消费面共用同一判定,不各写一份。
+
+**并入是按对象有条件的**,不是无条件放行整张系统列名单:`ownership: 'org' | 'none'`
+的对象没有 `owner_id`,那里的 `record.owner_id` 仍然是真错误并继续报;
+`tenancy.enabled: false` 无 `organization_id`;`systemFields: { audit: false }` 无
+审计四列;`systemFields: false` / `managedBy: 'better-auth'` 什么都不注入(只剩驱动
+提供的主键 `id`)。真正拼错的字段照旧被拒,并且注入列现在也进入 "did you mean?" 候选
+(`record.ownerid` → 提示 `owner_id`)。
+
+被解析的注入列在诊断与补全语义上与授权字段等同;类型健全性与 null-guard 两个索引
+**刻意**仍只读声明字段,原因写在各自注释里:列的 `type` 与可空性属于 registry 的列
+定义,在 lint 侧另立一份就是本次要消灭的第二份副本,而 null-guard 喂的是会中断构建的
+判定,擅自并入会让今天能构建的 stack 变红。
+
+注入行为本身零改动:`applySystemFields` 的输出在全条件矩阵上逐列不变(新增 parity
+pin 用实跑注入代码比对)。已显式重声明系统列的应用不受影响——重声明仍然合法,
+examples 三个 app 的 `os validate` 输出改动前后完全一致。

packages/lint/src/system-fields.ts

@@ -32,14 +32,40 @@
  * genuinely does not have.
  */
 
-import { FIELD_GROUP_SYSTEM_FIELDS } from '@objectstack/spec/data';
+import { FIELD_GROUP_SYSTEM_FIELDS, resolveInjectedSystemColumns } from '@objectstack/spec/data';
 import { SystemFieldName } from '@objectstack/spec/system';
 
 /**
  * Registry-injected columns addressable at runtime without being authored in
  * `fields` — the union of the spec's two system-field declarations.
+ *
+ * OBJECT-INDEPENDENT by construction: it answers "could this name be a system
+ * column anywhere", which is the right question for a rule that only ever needs
+ * to NOT flag the name. A rule that must decide whether the column exists **on
+ * one particular object** — because it resolves a reference, rather than
+ * skipping one — wants {@link injectedColumnsFor} instead.
  */
 export const SYSTEM_FIELDS: ReadonlySet<string> = new Set<string>([
   ...FIELD_GROUP_SYSTEM_FIELDS,
   ...Object.values(SystemFieldName),
 ]);
+
+/**
+ * The system columns addressable on ONE object without being authored (#5378).
+ *
+ * Delegates to the spec's `resolveInjectedSystemColumns` — the same derivation
+ * the registry's `applySystemFields` consumes to decide what it injects — so an
+ * author-time verdict about a column's existence cannot disagree with the
+ * runtime that provisions it. ⛔ Never hand-copy the conditions here: a second
+ * copy of "does `ownership: 'none'` get `owner_id`?" is the drift this indirection
+ * exists to prevent, and it drifts silently (the wrong answer is a FALSE
+ * diagnostic on valid metadata, or silence on a genuinely missing column).
+ *
+ * Use this — not {@link SYSTEM_FIELDS} — wherever the rule RESOLVES a field
+ * reference. The two differ exactly where it matters: on `ownership: 'none'`
+ * the platform injects no `owner_id`, so `record.owner_id` there is a real
+ * defect that must still be reported.
+ */
+export function injectedColumnsFor(objectDef: unknown): ReadonlySet<string> {
+  return resolveInjectedSystemColumns(objectDef).names;
+}

packages/lint/src/validate-expressions.test.ts

@@ -1887,3 +1887,134 @@ describe('validateStackExpressions — the field-formula check now actually runs
     expect(keysReadOff('f').filter((k) => !declared.includes(k))).toEqual([]);
   });
 });
+
+// ---------------------------------------------------------------------------
+// [#5378] Registry-injected system columns resolve like any other field.
+//
+// The defect this pins: `buildFieldIndex` read the AUTHORED `fields` map only,
+// so every column the registry injects was invisible and any predicate touching
+// one was hard-rejected — the platform's linter denying the platform's own
+// contract. Apps could only escape by re-declaring system columns (hotcrm#548
+// added `owner_id` to all 12 business objects for exactly this).
+//
+// The counter-examples matter as much as the fix: injection is CONDITIONAL, so
+// the pass must keep rejecting an injected column on an object the registry does
+// NOT inject it on. The conditions come from the spec derivation the registry
+// itself consumes (`resolveInjectedSystemColumns`), never a list copied here.
+// ---------------------------------------------------------------------------
+describe('validateStackExpressions — injected system columns (#5378)', () => {
+  /** Injection-only: declares NO system column of its own. */
+  const injectionOnly = (extra: Record<string, unknown> = {}) => ({
+    name: 'crm_contact',
+    fields: { name: { type: 'text' }, email: { type: 'email' } },
+    ...extra,
+  });
+
+  const withCondition = (object: Record<string, unknown>, condition: string) =>
+    validateStackExpressions({
+      objects: [object],
+      objectValidations: undefined,
+      flows: [{
+        name: 'contact_welcome',
+        nodes: [
+          { id: 'start', type: 'start', config: { objectName: 'crm_contact', condition } },
+        ],
+        edges: [],
+      }],
+    });
+
+  // The issue's acceptance criterion #1, verbatim.
+  it('accepts has(record.owner_id) on an object that does not declare owner_id', () => {
+    expect(withCondition(injectionOnly(), 'has(record.owner_id)')).toHaveLength(0);
+  });
+
+  it.each([
+    'has(record.created_at)',
+    'has(record.created_by)',
+    'has(record.updated_at)',
+    'has(record.updated_by)',
+    'has(record.organization_id)',
+    'has(record.owning_business_unit_id)',
+    // Driver-provisioned primary key — the same class, and the commonest of all.
+    'has(record.id)',
+  ])('accepts %s on an injection-only object', (condition) => {
+    expect(withCondition(injectionOnly(), condition)).toHaveLength(0);
+  });
+
+  // ── Counter-examples: the pass must NOT become a blanket allowance ──
+
+  it("still rejects record.owner_id on ownership: 'none' (no owner column is injected)", () => {
+    const issues = withCondition(injectionOnly({ ownership: 'none' }), 'has(record.owner_id)');
+    expect(issues).toHaveLength(1);
+    expect(issues[0].message).toMatch(/unknown field `owner_id`/);
+  });
+
+  it("still rejects record.owner_id on ownership: 'org'", () => {
+    const issues = withCondition(injectionOnly({ ownership: 'org' }), 'has(record.owner_id)');
+    expect(issues).toHaveLength(1);
+    expect(issues[0].message).toMatch(/unknown field `owner_id`/);
+  });
+
+  it('still rejects record.organization_id when the object opts out of tenancy', () => {
+    const issues = withCondition(
+      injectionOnly({ tenancy: { enabled: false } }),
+      'has(record.organization_id)',
+    );
+    expect(issues).toHaveLength(1);
+    expect(issues[0].message).toMatch(/unknown field `organization_id`/);
+  });
+
+  it('still rejects an audit column when the object opts out of audit fields', () => {
+    const issues = withCondition(
+      injectionOnly({ systemFields: { audit: false } }),
+      'has(record.created_at)',
+    );
+    expect(issues).toHaveLength(1);
+    expect(issues[0].message).toMatch(/unknown field `created_at`/);
+  });
+
+  it('still rejects a genuinely unknown field on an injection-only object', () => {
+    const issues = withCondition(injectionOnly(), 'has(record.no_such_field)');
+    expect(issues).toHaveLength(1);
+    expect(issues[0].message).toMatch(/unknown field `no_such_field`/);
+  });
+
+  // The resolved names join the "did you mean?" candidates, so a typo'd system
+  // column now gets the same help an authored field always got.
+  it('suggests an injected column for a near-miss spelling', () => {
+    const issues = withCondition(injectionOnly(), 'has(record.ownerid)');
+    expect(issues).toHaveLength(1);
+    expect(issues[0].message).toMatch(/unknown field `ownerid`/);
+    expect(issues[0].message).toMatch(/did you mean `owner_id`/);
+  });
+
+  // A DECLARED system column keeps working — re-declaration stays legal, which
+  // is why an existing app that took the hotcrm#548 workaround sees no change.
+  it('accepts a declared owner_id exactly as before (re-declaration stays legal)', () => {
+    const declared = {
+      name: 'crm_contact',
+      fields: {
+        name: { type: 'text' },
+        owner_id: { type: 'lookup', reference: 'sys_user', system: true },
+      },
+    };
+    expect(withCondition(declared, 'has(record.owner_id)')).toHaveLength(0);
+  });
+
+  // Reverse verification, in the direction that is actually available here: the
+  // injected half is what carries the verdict, so removing it must turn the
+  // acceptance criterion RED while leaving the counter-examples untouched. Pinned
+  // by construction — an empty injected set is exactly pre-#5378 behaviour.
+  it('the injected half is load-bearing: an object whose plan injects nothing rejects every system column', () => {
+    // `systemFields: false` is the platform's own "inject nothing" switch, so it
+    // reproduces the old blindness on purpose — minus `id`, which is the
+    // driver's and survives the opt-out.
+    const optedOut = injectionOnly({ systemFields: false });
+    for (const condition of ['has(record.owner_id)', 'has(record.created_at)', 'has(record.organization_id)']) {
+      const issues = withCondition(optedOut, condition);
+      expect(issues, condition).toHaveLength(1);
+      expect(issues[0].message).toMatch(/unknown field/);
+    }
+    expect(withCondition(optedOut, 'has(record.id)')).toHaveLength(0);
+  });
+});

packages/lint/src/validate-expressions.ts

@@ -80,6 +80,7 @@ import { validateExpression, collectCelRootIdentifiers } from '@objectstack/form
 import { collectFlowGraphs, resolveFlowNodeExpressions } from '@objectstack/spec/automation';
 import type { FlowNodeParsed } from '@objectstack/spec/automation';
 
+import { injectedColumnsFor } from './system-fields.js';
 import { findUnguardedNullableOperands, nullGuardMessage } from './validate-null-guards.js';
 import type { NullGuardOutcome } from './validate-null-guards.js';
 
@@ -106,7 +107,24 @@ function asArray(v: unknown): AnyRec[] {
   return [];
 }
 
-/** object name → set of its field names, for schema-aware field checks. */
+/**
+ * object name → set of its field names, for schema-aware field checks.
+ *
+ * Authored fields UNION the system columns the platform injects on that object
+ * (#5378). Both halves are needed because the runtime resolves both: a predicate
+ * saying `has(record.owner_id)` on an object that never declares `owner_id`
+ * evaluates fine, because the registry provisioned the column — so rejecting it
+ * here was the platform's own linter denying the platform's own contract, and it
+ * pushed apps into re-declaring system columns purely to satisfy this pass
+ * (hotcrm#548 declared `owner_id` on all 12 business objects for that reason).
+ *
+ * The injected half is CONDITIONAL, per object, via the spec derivation the
+ * registry itself consumes — not a blanket allowance of every system name. That
+ * distinction is the whole value: on `ownership: 'none'` the platform injects no
+ * `owner_id`, so `record.owner_id` there stays the error it should be. A blanket
+ * union would have traded a false positive for a false negative and called it
+ * fixed.
+ */
 function buildFieldIndex(objects: AnyRec[]): Map<string, string[]> {
   const idx = new Map<string, string[]>();
   for (const obj of objects) {
@@ -116,7 +134,10 @@ function buildFieldIndex(objects: AnyRec[]): Map<string, string[]> {
     let names: string[] = [];
     if (Array.isArray(fields)) names = fields.map(f => (f as AnyRec).name).filter((n): n is string => typeof n === 'string');
     else if (fields && typeof fields === 'object') names = Object.keys(fields as AnyRec);
-    idx.set(name, names);
+    // Injected columns come second, de-duplicated by insertion order: a DECLARED
+    // `owner_id` is the author's field (the registry lets it win), so the
+    // authored spelling keeps its position in the "did you mean?" candidates.
+    idx.set(name, [...new Set([...names, ...injectedColumnsFor(obj)])]);
   }
   return idx;
 }
@@ -125,6 +146,14 @@ function buildFieldIndex(objects: AnyRec[]): Map<string, string[]> {
  * object name → (field name → field type), for the #1928 tier-4 type-soundness
  * check. Handles both `fields` shapes (array of `{name, type}` and name-keyed
  * map). Fields with a non-string `type` are simply omitted (treated as `dyn`).
+ *
+ * DECLARED fields only — deliberately NOT widened with the injected columns
+ * {@link buildFieldIndex} resolves (#5378). Their TYPES are the registry's
+ * (`AUDIT_FIELD_DEFS` and its siblings own what each column looks like; the spec
+ * derivation owns only which ones exist), so typing them here would be the
+ * hand-copied second table this change exists to avoid. Omitted ⇒ `dyn` ⇒ no
+ * type-soundness verdict on an injected column, which is the safe direction: the
+ * pass gains no false finding, it simply does not cover them yet.
  */
 function buildFieldTypeIndex(objects: AnyRec[]): Map<string, Record<string, string>> {
   const idx = new Map<string, Record<string, string>>();
@@ -186,7 +215,18 @@ function isNullableField(def: AnyRec): boolean {
   return true;
 }
 
-/** object name → set of field names that may hold `null` (#4763). */
+/**
+ * object name → set of field names that may hold `null` (#4763).
+ *
+ * DECLARED fields only, like {@link buildFieldTypeIndex} and for a sharper
+ * reason: this index feeds a BUILD-BREAKING verdict, so adding the injected
+ * columns {@link buildFieldIndex} now resolves would turn `record.created_at`
+ * comparisons into errors on stacks that ship today. Nullability there is also
+ * genuinely per-column and not derivable from the existence plan — the driver
+ * always populates `created_at`, while `owner_id` really can be NULL — so it
+ * needs the registry's column definitions, not this module's guesses. Leaving
+ * them out costs coverage, never a false error.
+ */
 function buildNullableFieldIndex(objects: AnyRec[]): Map<string, Set<string>> {
   const idx = new Map<string, Set<string>>();
   for (const obj of objects) {