diff --git a/.changeset/lint-injected-temporal-column-types.md b/.changeset/lint-injected-temporal-column-types.md new file mode 100644 index 0000000000..3176ab8d5b --- /dev/null +++ b/.changeset/lint-injected-temporal-column-types.md @@ -0,0 +1,41 @@ +--- +"@objectstack/lint": minor +--- + +fix(lint): a field-typed rule reads the registry's own type for an injected column, so `created_at` / `updated_at` stop escaping the preset-comparand refusal (#16340) + +`@objectstack/lint`'s object graph recorded the registry-injected system columns by NAME only. A path resolving to one came back `{ kind: 'ok', injected: true }` with no `meta`, so every rule asking a SECOND question about the leaf — "is it temporal?" — had to treat it as unanswerable and stay silent. That silence landed on the two most-filtered columns in the platform. + +Measured on `origin/main` `d57611dfd3`, one dashboard widget over one object declaring `close_date: date` and authoring no `created_at`: + +| authored filter | before | after | +|:--|:--|:--| +| `close_date: 'last_30_days'` (authored `date`) | refused | refused | +| `created_at: { $gte: 'last_30_days' }` (ordering — arm 1) | refused | refused | +| `created_at: 'last_30_days'` | **silent** | refused | +| `created_at: { $eq: 'last_30_days' }` | **silent** | refused | +| `updated_at: { $in: ['last_30_days'] }` | **silent** | refused | +| `stage: 'this_quarter'` (a `select` column) | silent | silent | + +The engine already refused all three of those at query time (`INVALID_FILTER` / 400, the registry's field map in hand), so the gap was purely author-time: `objectstack lint` and the runtime publish gate passed a filter the runtime then refused with a 400 on first render — and an AI author's correction loop only sees what fails the build. + +## What changed + +`GraphObject.injected` is now a `ReadonlyMap` rather than a `ReadonlySet`: each injected column carries the registry's own definition. Both halves are DERIVED from one plan — membership from `resolveInjectedSystemColumns`, the slice from `injectedSystemColumnDefs` (`@objectstack/spec/data`, the same tables `applySystemFields` spreads at registration) — so lint never hand-copies "`created_at` is a datetime" and cannot drift from the runtime that provisions it. `resolveFieldPath` populates `meta` for an injected leaf accordingly, and `filter-preset-comparand`'s field-type oracle lost its `verdict.injected` bail: the marker says WHO wrote the column, and the ruling turns on what the column IS. + +`id` is the one addressable column with no definition behind it — the DRIVER provisions the primary key — so its slice is empty and a second question about it is still unanswered, truthfully and only there. The `select`-column reading arm 2 exists to protect is untouched: no injected column is a picklist. + +**Behaviour change for authors**: a stack that filtered an injected `date` / `datetime` column against one of the thirteen dashboard date-range preset names in an equality or membership position now fails `objectstack lint` and the runtime publish gate where it previously passed. Every such filter was already refused by the engine at query time; the error simply moves to where the filter is written. Write the `{date-macro}` window the message names, or an ISO date. + +**Type change for direct consumers of the seam**: `GraphObject.injected` changed from `ReadonlySet` to `ReadonlyMap`. `.has(name)` answers exactly as before; code that iterated the set or spread it into one needs `.keys()`. Shipped as `minor` under the repo's launch-window convention. + +## Two more rules inherit it, in the same edit + +The type reaches every rule that asks a second question about a resolved leaf, which is the whole reason it was fixed at the seam rather than inside `filter-preset-comparand`: + +- **`list-view-field-dotted`** now refuses a dotted list-view filter key whose head is an injected column, on the same axis as an authored one. `created_at.x` reads as the `datetime` scalar it is (nothing beneath it for a path to reach) and `owner_id.name` as the `lookup` it is (it stores an id, not an embedded document). `assertFilterIsMaterializable` and the REST ingress have always answered `400 INVALID_FIELD` for both — the linter was silent only because the type was missing here. +- **`dataset-include-unknown`** now judges an `include[]` entry naming an injected column instead of bailing on the marker: `include: ['owner_id']` joins (it is the registry's `lookup`), `include: ['created_at']` is refused (a `datetime` derives no join, so every dimension written against that prefix addresses nothing). + +`id` falls through the untyped branch of all three rules — the DRIVER provisions the primary key and no definition table describes it, so an unreadable head is what the door sees too, and none of them invents a refusal there. + +A relationship HOP through an injected column stays a skip (`unknowable` / `injected-hop`), deliberately: the slice now carries `reference`, and traversing it would newly judge every path through a platform anchor wherever `sys_user` is compiled into the stack — a widening with its own findings to measure. diff --git a/packages/lint/src/object-graph.test.ts b/packages/lint/src/object-graph.test.ts index c36877d727..cecbd87e64 100644 --- a/packages/lint/src/object-graph.test.ts +++ b/packages/lint/src/object-graph.test.ts @@ -17,6 +17,9 @@ import { RELATIONSHIP_FIELD_TYPES, } from './object-graph.js'; import { walkFilterFieldKeys, type FilterFieldKey } from './filter-walk.js'; +// [#16340] Read back the registry's OWN definition table to assert the graph +// derives its injected types rather than carrying a second copy of them. +import { injectedSystemColumnDefs } from '@objectstack/spec/data'; const stack = { objects: [ @@ -94,10 +97,64 @@ describe('object-graph — resolveFieldPath verdicts', () => { expect(isUnjudgeable(verdict)).toBe(true); }); - it('marks an injected leaf so a caller cannot mistake it for a typed field', () => { - const verdict = resolveFieldPath(graph, 'crm_opportunity', 'created_at'); - expect(verdict).toMatchObject({ kind: 'ok', injected: true }); - expect((verdict as { meta?: unknown }).meta).toBeUndefined(); + // [#16340] An injected leaf resolves WITH the registry's own definition. The + // marker still says the object does not author the column — that is the #8116 + // provenance question — but `meta` answers the second question a caller asks + // ("is it temporal?") exactly as it does on an authored field. Before this, + // the leaf carried no `meta` at all and every such caller had to stay silent; + // `filter-preset-comparand` did, on the two most-filtered columns in the + // platform. + it("resolves an injected leaf with the registry's own type, and marks it injected", () => { + expect(resolveFieldPath(graph, 'crm_opportunity', 'created_at')).toMatchObject({ + kind: 'ok', object: 'crm_opportunity', field: 'created_at', injected: true, + meta: { type: 'datetime' }, + }); + expect(resolveFieldPath(graph, 'crm_opportunity', 'updated_at')).toMatchObject({ + kind: 'ok', injected: true, meta: { type: 'datetime' }, + }); + // An injected LOOKUP anchor carries its target too — read from the same + // table, never re-declared here. + expect(resolveFieldPath(graph, 'crm_opportunity', 'owner_id')).toMatchObject({ + kind: 'ok', injected: true, meta: { type: 'lookup', reference: 'sys_user' }, + }); + }); + + // The type is DERIVED, never transcribed: it must equal the definition the + // registry spreads at registration, byte for byte. Reading the spec table + // here is the assertion — a hand-copied 'datetime' in this package would pass + // a literal pin and drift the day the registry re-types the column. + it('reports the type the registry injects, not a copy of it', () => { + const defs = injectedSystemColumnDefs(stack.objects[0]); + for (const [name, def] of Object.entries(defs)) { + const verdict = resolveFieldPath(graph, 'crm_opportunity', name); + expect(verdict).toMatchObject({ kind: 'ok', injected: true }); + expect((verdict as { meta?: { type?: string } }).meta?.type).toBe(def.type); + } + expect(Object.keys(defs).length).toBeGreaterThan(0); // lit control + }); + + // `id` is the one addressable column with NO definition behind it — the + // DRIVER provisions the primary key. An empty slice is the truthful answer, + // and it must stay distinguishable from "this column does not exist". + it('resolves the primary key with an empty slice rather than a guessed type', () => { + const verdict = resolveFieldPath(graph, 'crm_opportunity', 'id'); + expect(verdict).toMatchObject({ kind: 'ok', field: 'id', injected: true }); + expect((verdict as { meta?: { type?: string } }).meta?.type).toBeUndefined(); + expect(injectedSystemColumnDefs(stack.objects[0]).id).toBeUndefined(); // why + }); + + // The opt-out rows are the registry's, not this module's: an object that + // opts out has no injected column to resolve, so the reference is a real + // miss and must still be reported. + it('reports an injected name on an object the registry opts out of', () => { + const optedOut = indexObjectGraph({ + objects: [{ name: 'seed_rows', systemFields: false, fields: { note: { type: 'text' } } }], + }); + expect(resolveFieldPath(optedOut, 'seed_rows', 'created_at')).toMatchObject({ + kind: 'field-unknown', object: 'seed_rows', field: 'created_at', + }); + // …and the driver's primary key survives even that row. + expect(resolveFieldPath(optedOut, 'seed_rows', 'id')).toMatchObject({ kind: 'ok', injected: true }); }); it('skips an object not in the stack, and one with no field map', () => { diff --git a/packages/lint/src/object-graph.ts b/packages/lint/src/object-graph.ts index 10f1b138e3..9bc1b05baf 100644 --- a/packages/lint/src/object-graph.ts +++ b/packages/lint/src/object-graph.ts @@ -38,16 +38,37 @@ * compiling plugin-auth alone genuinely cannot see them. * 2. An object that declares no readable field map — ADR-0015 `external` and * datasource-introspected schemas whose columns resolve at runtime. - * 3. Registry-injected system columns, which exist at runtime and never - * appear in authored `fields`. Resolved per object through + * 3. A relationship HOP through a registry-injected system column. The + * columns themselves are not a skip — they exist at runtime, never + * appear in authored `fields`, and resolve per object through * {@link injectedColumnsFor}, never the object-independent - * `SYSTEM_FIELDS` union — the two differ exactly where it matters (on + * `SYSTEM_FIELDS` union (the two differ exactly where it matters: on * `ownership: 'none'` the platform injects no `owner_id`, so a reference * to it there is a real defect). The shipped - * `showcase_task_metrics.created_at` dimension is skip 3's live case. + * `showcase_task_metrics.created_at` dimension is that live case. + * + * ## An injected leaf carries its type (#16340) + * + * Skip 3 used to be wider: an injected leaf resolved by NAME alone, with no + * `meta`, so every caller asking a second question about it — is it temporal? + * is it a relationship? — had to treat it as unanswerable. That silence was + * invisible to authors and it landed on the two most-filtered columns in the + * platform: `filter-preset-comparand`'s field-typed arm refused + * `close_date: 'last_30_days'` on an authored `date` column while + * `created_at: 'last_30_days'` on the same widget passed the linter and the + * runtime publish gate, only to be refused by the engine with a 400 on first + * render. + * + * {@link GraphObject.injected} therefore carries each injected column's own + * definition, DERIVED from `injectedColumnDefsFor` — the spec tables + * `applySystemFields` spreads at registration — so lint never hand-copies + * "`created_at` is a datetime" and cannot drift from the registry that + * provisions it. The one column with no definition behind it is `id`: the + * DRIVER provisions the primary key, so its `GraphField` is empty and a + * second question about it is still unanswered — truthfully, and only there. */ -import { injectedColumnsFor } from './system-fields.js'; +import { injectedColumnDefsFor, injectedColumnsFor } from './system-fields.js'; /** Any plain metadata record. */ type AnyRec = Record; @@ -108,8 +129,19 @@ export interface GraphObject { names: ReadonlySet; /** name → the slice above. */ fields: ReadonlyMap; - /** Registry-injected columns addressable on THIS object (skip 3). */ - injected: ReadonlySet; + /** + * Registry-injected columns addressable on THIS object, each mapped to the + * registry's own definition of it (#16340). + * + * A MAP rather than a name set because a caller that resolves a reference + * asks two questions, not one: does the column exist, and what is it? Both + * halves are derived — membership from `injectedColumnsFor`, the slice from + * `injectedColumnDefsFor` — so neither can drift from `applySystemFields`. + * `.has(name)` answers the first question exactly as the old set did; `id` + * maps to an empty slice because the driver, not the injection pass, + * provisions the primary key and no definition table describes it. + */ + injected: ReadonlyMap; } /** object name → its resolvable surface, or `null` (skip 2). */ @@ -190,6 +222,23 @@ function strName(v: unknown): string | undefined { return typeof v === 'string' && v.length > 0 ? v : undefined; } +/** + * Read one field DEFINITION — authored or registry-injected — into the slice + * this module exposes. + * + * One reader for both sources on purpose: an injected `created_at` and an + * authored `close_date` are the same kind of answer to the same question, and + * a second reader here would be free to disagree with this one about what + * `type` means. + */ +function graphFieldOf(def: AnyRec): GraphField { + return { + type: typeof def.type === 'string' ? def.type : undefined, + reference: strName(def.reference), + multiple: def.multiple === true ? true : undefined, + }; +} + /** Read one object's declared field map into the graph slice, or `null`. */ function graphObjectOf(obj: AnyRec): GraphObject | null { const declared = obj.fields; @@ -200,14 +249,20 @@ function graphObjectOf(obj: AnyRec): GraphObject | null { const n = strName(f.name); if (!n) continue; names.add(n); - fields.set(n, { - type: typeof f.type === 'string' ? f.type : undefined, - reference: strName(f.reference), - multiple: f.multiple === true ? true : undefined, - }); + fields.set(n, graphFieldOf(f)); } if (names.size === 0) return null; - return { names, fields, injected: injectedColumnsFor(obj) }; + + // WHICH columns are injected and WHAT each one is are two derivations over + // one plan (`resolveInjectedSystemColumns`), so they cannot disagree about + // membership. `id` is in the first and not the second — the driver's primary + // key has no definition table — and lands on an empty slice. + const defs = injectedColumnDefsFor(obj); + const injected = new Map(); + for (const name of injectedColumnsFor(obj)) { + injected.set(name, graphFieldOf(defs.get(name) ?? {})); + } + return { names, fields, injected }; } /** @@ -228,12 +283,21 @@ export function indexObjectGraph(stack: unknown): ObjectGraph { export type FieldPathVerdict = /** * Every hop and the leaf resolved. `object` is the object the LEAF lives on. - * `injected` marks a leaf resolved through skip 3 — a registry-injected - * column, real at runtime, whose TYPE and relationship target are - * registry-owned and invisible here. A caller asking a second question about - * the leaf (is it a relationship? is it materialised?) must treat an - * `injected` leaf as unanswerable rather than assume the absence of a - * declared type means the absence of the property. + * `injected` marks a leaf the object does not author — a registry-injected + * column, real at runtime. + * + * `meta` is populated for BOTH kinds (#16340): an injected leaf carries the + * registry's own definition, so a caller asking a second question about it + * ("is it temporal?") reads `meta.type` exactly as it does on an authored + * field. The marker remains because "authored" and "injected" are still + * different facts — the #8116 provenance question is asked only of injected + * leaves, and an author-DECLARED column of the same name is one the author + * vouches for. + * + * The one leaf with an EMPTY `meta` is `id`: the driver provisions the + * primary key, so no definition describes it and a second question about it + * genuinely has no answer here. ⛔ Do not read an absent `meta.type` as the + * absence of the property — read it as "not answerable for this column". */ | { kind: 'ok'; object: string; field: string; meta?: GraphField; injected?: true } /** @@ -296,10 +360,13 @@ export function resolveFieldPath( const meta = obj.fields.get(segment); if (!meta) { // An injected system column is REAL and some of them are relationships - // (`owner_id` is a lookup at the registry), but their type and target are - // registry-owned and invisible here — so `owner.name` is unanswerable, - // not a miss. Reporting it would be the false positive skip 3 exists to - // avoid; assuming it resolves would be the fail-open on the other side. + // (`owner_id` is a `lookup` to `sys_user` at the registry). Reporting the + // hop would be the false positive skip 3 exists to avoid, so it stays a + // SKIP — deliberately, not for want of a target: since #16340 the slice + // carries `reference`, and traversing it would newly JUDGE every path + // through a platform anchor (`owner_id.name` and its siblings) wherever + // `sys_user` is compiled into the stack. That is a widening with its own + // findings to measure, and it is not this seam's to make silently. if (obj.injected.has(segment)) { return { kind: 'unknowable', reason: 'injected-hop', object: current }; } @@ -320,7 +387,8 @@ export function resolveFieldPath( const leaf = segments[segments.length - 1]; if (obj.names.has(leaf)) return { kind: 'ok', object: current, field: leaf, meta: obj.fields.get(leaf) }; - if (obj.injected.has(leaf)) return { kind: 'ok', object: current, field: leaf, injected: true }; + const injectedMeta = obj.injected.get(leaf); + if (injectedMeta) return { kind: 'ok', object: current, field: leaf, meta: injectedMeta, injected: true }; return { kind: 'field-unknown', object: current, field: leaf, candidates: obj.names }; } diff --git a/packages/lint/src/system-fields.ts b/packages/lint/src/system-fields.ts index 692f041eb7..3a07a37ef3 100644 --- a/packages/lint/src/system-fields.ts +++ b/packages/lint/src/system-fields.ts @@ -34,6 +34,7 @@ import { FIELD_GROUP_SYSTEM_FIELDS, + injectedSystemColumnDefs, resolveInjectedSystemColumns, unprovisionedInjectedColumns, } from '@objectstack/spec/data'; @@ -74,6 +75,39 @@ export function injectedColumnsFor(objectDef: unknown): ReadonlySet { return resolveInjectedSystemColumns(objectDef).names; } +/** + * WHAT each injected column on THIS object looks like — the registry's own + * definition, keyed by column name (#16340). + * + * The companion to {@link injectedColumnsFor}: that one answers WHICH columns + * are addressable, this one answers what each of them IS. A rule that only + * needs to not-flag a name wants the first; a rule that asks a SECOND question + * about a resolved leaf — is it temporal? is it a relationship? — needs this, + * and before it existed every such rule had to treat an injected leaf as + * unanswerable and stay silent (the `filter-preset-comparand` field-typed arm + * did exactly that on `created_at` / `updated_at`, the two temporal columns an + * author reaches for most). + * + * Delegates to the spec's `injectedSystemColumnDefs` — the same tables + * `applySystemFields` spreads at registration, gated by the same + * `resolveInjectedSystemColumns` plan — so the type an author-time rule reads + * is byte-for-byte the type the registry injects. ⛔ Never hand-copy a column's + * type here: a local `created_at is a datetime` table is the second copy this + * module's header forbids, and it drifts silently in the direction that hurts + * (a rule judging a column the registry has since re-typed). + * + * `id` is deliberately ABSENT while {@link injectedColumnsFor} reports it: the + * primary key is provisioned by the DRIVER, not by the injection pass, so no + * definition table describes it. A caller that resolves a name present in the + * name set but missing here has an addressable column of unknown type — which + * is the truthful answer, not a gap. + */ +export function injectedColumnDefsFor( + objectDef: unknown, +): ReadonlyMap>> { + return new Map(Object.entries(injectedSystemColumnDefs(objectDef))); +} + /** * The injected columns THIS object registers with NO storage behind them * (#8116) — the #7865 provenance marker, in the per-object set shape lint diff --git a/packages/lint/src/validate-dataset-references.test.ts b/packages/lint/src/validate-dataset-references.test.ts index fbd5ad7257..4d2c864a4e 100644 --- a/packages/lint/src/validate-dataset-references.test.ts +++ b/packages/lint/src/validate-dataset-references.test.ts @@ -245,6 +245,36 @@ describe('validateDatasetReferences — include[] must name a RELATIONSHIP', () expect(rules(stackWith({ include: ['duty'], dimensions: [], measures: [] }))).toEqual([]); }); + // [#16340] An injected column is judged on the same axis as an authored one: + // the graph carries the registry's own definition for it. `owner_id` IS the + // `lookup` the registry declares, so an include naming it joins; `created_at` + // is a `datetime`, so an include naming it derives no join and every + // dimension written against that prefix addresses nothing — which is the + // finding, not a guess. Before this the rule bailed on the marker and said + // neither thing. + it('accepts an include naming an INJECTED relationship anchor', () => { + expect(rules(stackWith({ include: ['owner_id'], dimensions: [], measures: [] }))).toEqual([]); + }); + + it('refuses an include naming an injected column that is not a relationship', () => { + const findings = validateDatasetReferences( + stackWith({ include: ['created_at'], dimensions: [], measures: [] }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(DATASET_INCLUDE_UNKNOWN); + expect(findings[0].message).toContain('`datetime` field'); + expect(findings[0].message).toContain('not a relationship'); + }); + + it('⛔ leaves the primary key untyped — the driver provisions it, no table describes it', () => { + const findings = validateDatasetReferences( + stackWith({ include: ['id'], dimensions: [], measures: [] }), + ); + expect(findings).toHaveLength(1); + // The untyped branch: "an ordinary field", not a guessed type. + expect(findings[0].message).toContain('an ordinary field'); + }); + it('refuses a multi-hop include whose intermediate hop is not traversable', () => { const findings = validateDatasetReferences( stackWith({ include: ['status.owner'], dimensions: [], measures: [] }), @@ -371,12 +401,15 @@ describe('validateDatasetReferences — the three skips', () => { ).toEqual([]); }); - it('skips a hop THROUGH an injected column, whose target is registry-owned', () => { + it('skips a hop THROUGH an injected column, whose target the seam does not traverse', () => { // `owner_id` IS injected on this object (`ownership` omitted ⇒ both anchors) - // and IS a lookup at the registry — but its type and target are invisible - // here, so `owner_id.name` is unanswerable rather than a miss. Reporting it - // would be the false positive skip 3 exists to avoid; assuming it resolves - // would be the fail-open on the other side. + // and IS a lookup at the registry. [#16340] The slice now carries that + // target, but `resolveFieldPath` still answers `injected-hop` rather than + // walking it: traversing would newly judge every path through a platform + // anchor wherever `sys_user` is compiled in, which is a widening with its + // own findings to measure. Reporting the hop would be the false positive + // skip 3 exists to avoid; assuming it resolves would be the fail-open on + // the other side. expect(rules(stackWith({ dimensions: [{ name: 'o', field: 'owner_id.name' }], measures: [] }))).toEqual([]); }); diff --git a/packages/lint/src/validate-dataset-references.ts b/packages/lint/src/validate-dataset-references.ts index 31a1a0179a..a895af0373 100644 --- a/packages/lint/src/validate-dataset-references.ts +++ b/packages/lint/src/validate-dataset-references.ts @@ -214,9 +214,14 @@ export function validateDatasetReferences(stack: AnyRec): DatasetRefFinding[] { if (verdict.kind === 'ok') { // The entry resolves to a real field — but `include` joins, so the - // field must BE a relationship. An injected column's type is - // registry-owned and invisible here, so it is unanswerable, not a miss. - if (verdict.injected) return; + // field must BE a relationship. [#16340] An injected column is judged + // on the same axis as an authored one: the graph carries the + // registry's own definition, so `owner_id` reads as the `lookup` it is + // and `created_at` as the `datetime` it is. The bail that used to sit + // here ("its type is registry-owned and invisible") is gone with the + // limitation that justified it. The primary key still falls through + // the untyped branch below — the DRIVER provisions it and no + // definition table describes it. const type = verdict.meta?.type; if (type && RELATIONSHIP_FIELD_TYPES.has(type)) return; findings.push({ diff --git a/packages/lint/src/validate-list-view-field-refs.test.ts b/packages/lint/src/validate-list-view-field-refs.test.ts index c073df1849..1614fd794e 100644 --- a/packages/lint/src/validate-list-view-field-refs.test.ts +++ b/packages/lint/src/validate-list-view-field-refs.test.ts @@ -427,10 +427,32 @@ describe('#14282 — a dotted key the FILTER door refuses, and the ones it serve expect(validateListViewFieldRefs(stackWith(mutate(filterOn('tags.0'))))).toEqual([]); }); - it('a registry-injected head is NOT refused at a filter — its type is invisible here', () => { - // `created_at` resolves through skip 3 with no readable type, and - // `classifyDottedFilterHead` answers `null` for an unreadable head. - expect(validateListViewFieldRefs(stackWith(mutate(filterOn('created_at.x'))))).toEqual([]); + // [#16340] A registry-injected head IS judged now: the graph carries the + // registry's own definition for it, so the classifier reads the same + // `datetime` the DOOR reads. `assertFilterIsMaterializable` has always + // refused `created_at.x` with `400 INVALID_FIELD` — the linter was silent + // only because the type was missing here, which is the miss #16340 closed. + it('a registry-injected scalar head is refused at a filter, as the door refuses it', () => { + const findings = validateListViewFieldRefs(stackWith(mutate(filterOn('created_at.x')))); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(LIST_VIEW_FIELD_DOTTED); + expect(findings[0].severity).toBe('error'); + expect(findings[0].message).toContain('`datetime` field'); + expect(findings[0].message).toContain('single scalar value'); + }); + + it('an injected RELATION head is refused on the same axis as an authored one', () => { + const findings = validateListViewFieldRefs(stackWith(mutate(filterOn('owner_id.name')))); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('`lookup` field'); + expect(findings[0].message).toContain("stores the related record's id"); + }); + + it('⛔ the primary key is NOT refused — the driver provisions it and no table types it', () => { + // The one injected column with no definition behind it. An unreadable head + // is what `classifyDottedFilterHead` answers `null` for, and the door + // serves it, so the linter must not invent a refusal here. + expect(validateListViewFieldRefs(stackWith(mutate(filterOn('id.x'))))).toEqual([]); }); it('the tab and user-filter tab presets are judged on the same axis', () => { diff --git a/packages/lint/src/validate-list-view-field-refs.ts b/packages/lint/src/validate-list-view-field-refs.ts index 70560db7dc..763de8c704 100644 --- a/packages/lint/src/validate-list-view-field-refs.ts +++ b/packages/lint/src/validate-list-view-field-refs.ts @@ -204,9 +204,18 @@ * never reported (ADR-0072 D1: one dead finding and authors stop trusting the * linter): an object this stack does not define, an object that declares no * readable field map (ADR-0015 `external`, datasource-introspected schemas), - * and a registry-injected system column. A fourth skip is this surface's own: - * a list view whose `data.provider` is not `object` binds to no object graph - * at all, so none of its field names is resolvable here. + * and a hop THROUGH a registry-injected system column. A fourth skip is this + * surface's own: a list view whose `data.provider` is not `object` binds to no + * object graph at all, so none of its field names is resolvable here. + * + * [#16340] An injected column at the HEAD of a dotted filter key is not a skip + * and never was — it resolves. What used to be missing was its TYPE, so + * `classifyDottedFilterHead` read an unreadable head and this rule stayed + * silent on `created_at.x` while `assertFilterIsMaterializable` refused it at + * the door with the registry's own field map in hand. The graph now carries + * the registry's definition for each injected column, so the two answer alike. + * `id` remains unreadable — the DRIVER provisions the primary key and no + * definition table describes it — and the door serves it, so this rule does too. */ import { classifyDottedFilterHead } from '@objectstack/spec/data'; diff --git a/packages/lint/src/validate-preset-comparands.test.ts b/packages/lint/src/validate-preset-comparands.test.ts index e42b2ceca3..7e797dc336 100644 --- a/packages/lint/src/validate-preset-comparands.test.ts +++ b/packages/lint/src/validate-preset-comparands.test.ts @@ -394,8 +394,11 @@ describe('validatePresetComparands — arm 2, the FIELD-TYPED equality / members period: { $in: ['last_30_days'] }, // A `time` column: the ruling names date / datetime only. opens_at: 'today', - // A registry-injected column: its type is invisible to the graph. - created_at: 'last_30_days', + // [#16340] A registry-injected column is NOT here any more — its + // type is the registry's own and the arm reads it. `id` is: the + // driver provisions the primary key, so no definition table + // describes it and the arm has nothing to judge. + id: 'last_30_days', // A field the object does not declare: another rule's finding. close_dat: 'last_30_days', // The platform's own correct spellings in the judged positions. @@ -424,6 +427,139 @@ describe('validatePresetComparands — arm 2, the FIELD-TYPED equality / members })).toEqual([]); }); + // ── [#16340] Registry-injected temporal columns ─────────────────────────── + // + // `created_at` / `updated_at` are `datetime` columns the registry injects on + // almost every object; no object AUTHORS them. While the object graph held + // injected columns by NAME only, this arm could not read their type and said + // NOTHING — measured on `origin/main` d57611dfd3 over the stack below: + // `close_date: 'last_30_days'` (authored `date`) was refused while + // `created_at: 'last_30_days'` on the same dashboard passed lint and the + // runtime publish gate, and the author first learned of it from the engine's + // 400 on render. + // + // ⚠️ These assertions pin the SENTENCE THE AUTHOR READS, not the graph shape + // behind it. A pin on `GraphObject.injected` alone can go green while the + // author still receives the wrong message — or none. + const injectedBoard = (filter: unknown) => ({ + objects: crmObjects, + datasets: crmDatasets, + dashboards: [{ name: 'sales', widgets: [widget('w', filter)] }], + }); + + it('refuses a preset against an injected `created_at`, with the whole author-facing message', () => { + const findings = validatePresetComparands(injectedBoard({ created_at: 'last_30_days' })); + expect(findings).toHaveLength(1); + const f = findings[0]; + expect(f.severity).toBe('error'); + expect(f.rule).toBe(FILTER_PRESET_COMPARAND); + expect(f.where).toBe('dashboard "sales" · widget "w"'); + expect(f.path).toBe('dashboards[0].widgets[0].filter.created_at'); + // Verbatim. The author acts on this text; a re-wording is a decision, not a + // refactor, and it should turn this red. + expect(f.message).toBe( + '"last_30_days" is a dashboard date-range PRESET name, not a filter value. ' + + 'It is only understood by the dashboard date-filter positions ' + + "(dateRange.defaultRange, a date global filter's defaultValue), where the " + + 'console lowers it to {date-macro} bounds before querying. As a bare "$eq" ' + + 'comparand nothing resolves it: a declared datetime/date field refuses the ' + + 'query at the engine (INVALID_FILTER / 400), and any other column compares ' + + "the literal string. Write the date-macro window instead — e.g. { $gte: " + + "'{30_days_ago}' } — or an ISO date such as \"2026-01-15\". Refused at " + + 'authoring time so the error surfaces where the filter is written.', + ); + expect(f.hint).toBe( + 'Presets belong to the dashboard date-filter bar (dateRange.defaultRange, a ' + + "date global filter's defaultValue). In a filter comparand, write the " + + '{date-macro} window the message names, or an ISO date.', + ); + }); + + it('refuses every residue position on an injected column, as it already does on a declared one', () => { + const findings = validatePresetComparands({ + objects: crmObjects, + datasets: crmDatasets, + dashboards: [{ + name: 'sales', + widgets: [ + widget('bare', { created_at: 'last_30_days' }), + widget('eq', { created_at: { $eq: 'last_30_days' } }), + widget('ne', { updated_at: { $ne: 'this_week' } }), + widget('in', { updated_at: { $in: ['last_30_days'] } }), + widget('nin', { created_at: { $nin: ['today'] } }), + // Arm 1 caught this one before #16340 — position alone decides it. + widget('gte', { created_at: { $gte: 'last_30_days' } }), + ], + }], + views: [{ + name: 'recent', + data: { provider: 'object', object: 'crm_opportunity' }, + filter: [{ field: 'updated_at', operator: 'equals', value: 'last_7_days' }], + }], + pages: [{ + name: 'p', + components: [{ + type: 'list', + dataSource: { object: 'crm_opportunity' }, + filter: ['created_at', '=', 'yesterday'], + }], + }], + }); + expect(findings.map((f) => f.path)).toEqual([ + 'dashboards[0].widgets[0].filter.created_at', + 'dashboards[0].widgets[1].filter.created_at.$eq', + 'dashboards[0].widgets[2].filter.updated_at.$ne', + 'dashboards[0].widgets[3].filter.updated_at.$in[0]', + 'dashboards[0].widgets[4].filter.created_at.$nin[0]', + 'dashboards[0].widgets[5].filter.created_at.$gte', + 'views[0].filter[0].value', + 'pages[0].components[0].filter[2]', + ]); + for (const f of findings) expect(f.message).toContain('is a dashboard date-range PRESET name'); + }); + + it('resolves an injected leaf through a relationship hop, like any other leaf', () => { + const findings = validatePresetComparands({ + objects: crmObjects, + datasets: crmDatasets, + dashboards: [{ + name: 'sales', + widgets: [widget('hop', { account: { created_at: 'last_30_days' } })], + }], + }); + expect(findings.map((f) => f.path)).toEqual([ + 'dashboards[0].widgets[0].filter.account.created_at', + ]); + }); + + it('stays silent where the injection stops — the discriminating controls', () => { + // `ownership: 'none'` / `systemFields: false` are the rows where the + // registry injects nothing, so the columns do not exist and this rule has + // nothing to say (the *-field-unknown rules own that finding). `id` exists + // on every row but is the DRIVER's primary key, with no definition behind + // it — an addressable column of unknown type, judged by nobody. + expect(validatePresetComparands({ + objects: [ + { name: 'seed_rows', systemFields: false, fields: { note: { type: 'text' } } }, + { name: 'crm_opportunity', fields: { close_date: { type: 'date' } } }, + ], + datasets: [ + { name: 'seeds', object: 'seed_rows', measures: [] }, + { name: 'deals', object: 'crm_opportunity', measures: [] }, + ], + dashboards: [{ + name: 'sales', + widgets: [ + widget('optedOut', { created_at: 'last_30_days' }, { dataset: 'seeds' }), + widget('pk', { id: 'last_30_days' }), + // An injected LOOKUP anchor is not temporal — the type is read, not + // the fact that the platform put the column there. + widget('anchor', { owner_id: 'this_quarter' }), + ], + }], + })).toEqual([]); + }); + // [#16106 review finding B1] A form field's `publicPicker.filter` is a static // pre-filter the public-lookup route runs on the REFERENCED object // (`picker.object`, else the field's `reference`). Binding it to the view's diff --git a/packages/lint/src/validate-preset-comparands.ts b/packages/lint/src/validate-preset-comparands.ts index 23a8afdb33..21a3857227 100644 --- a/packages/lint/src/validate-preset-comparands.ts +++ b/packages/lint/src/validate-preset-comparands.ts @@ -127,8 +127,32 @@ import { indexObjectGraph, recordsOf, resolveFieldPath, type ObjectGraph } from * * The field key (a bare name, or a dotted relationship path) is then resolved * through `resolveFieldPath` (`object-graph.ts`), and the comparand is judged - * only when the LEAF resolves to an author-declared field of type `date` or - * `datetime` — the two types the ruling names. + * only when the LEAF resolves to a field of type `date` or `datetime` — the + * two types the ruling names. + * + * ### Registry-injected columns are judged too (#16340) + * + * `created_at` and `updated_at` are `datetime` columns the registry injects on + * almost every object, and they are the temporal columns an author reaches for + * most — but no object AUTHORS them, so for as long as the object graph + * recorded injected columns by name alone this arm could not see their type + * and stayed silent on exactly them. Measured on `origin/main` before the fix, + * on one dashboard widget over one object declaring `close_date: date`: + * + * ``` + * close_date: 'last_30_days' REFUSED (authored date column) + * created_at: { $gte: 'last_30_days' } REFUSED (arm 1 — ordering, field-agnostic) + * created_at: 'last_30_days' SILENT <- the gap + * created_at: { $eq: 'last_30_days' } SILENT <- the gap + * updated_at: { $in: ['last_30_days'] } SILENT <- the gap + * stage: 'this_quarter' (select column) SILENT <- correct: the picklist case + * ``` + * + * The oracle below no longer asks whether the leaf was AUTHORED, only what it + * IS, because `GraphObject.injected` now carries each injected column's own + * registry definition. The `select`-column reading the arm exists to protect + * is untouched — an injected column's type comes from the platform's own + * tables, and not one of them is a picklist. * * ### What this arm deliberately does NOT judge * @@ -138,9 +162,6 @@ import { indexObjectGraph, recordsOf, resolveFieldPath, type ObjectGraph } from * - a position no ancestor binds (an app-level filter, a dashboard-level * filter outside a widget), a dataset or object the stack does not declare, * an object with no readable field map, a view on a non-`object` provider; - * - a leaf that resolves only as a registry-INJECTED column (`created_at`, - * `updated_at`, …): the object graph carries no type for those — their type - * is registry-owned and invisible here (`FieldPathVerdict`'s own contract); * - a `time` field: the ruling names `date` / `datetime`, and a wall-clock * column has no preset-shaped authoring slip worth a rule of its own; * - a field the object does not declare at all (a typo) — that is the @@ -275,10 +296,16 @@ function finding( // ── Arm 2's field-type oracle ───────────────────────────────────────────────── /** - * "Is this field key, resolved against the filter's bound object, an - * author-declared `date` / `datetime` field?" — the one question arm 2 asks. - * A key it cannot answer (no bound object, an unresolvable path, an injected - * leaf, any other type) answers `false`, so the position stays unjudged. + * "Is this field key, resolved against the filter's bound object, a `date` / + * `datetime` field?" — the one question arm 2 asks. A key it cannot answer (no + * bound object, an unresolvable path, a column whose type nothing declares, any + * other type) answers `false`, so the position stays unjudged. + * + * [#16340] It reads `meta.type` on an INJECTED leaf exactly as on an authored + * one. There is no `verdict.injected` bail: the marker says WHO wrote the + * column, and the ruling turns on what the column IS. The only injected column + * with no type behind it is `id` — the driver's primary key — and it falls out + * through the same `undefined` type check as any untyped authored field. */ type TemporalFieldOracle = (field: string) => boolean; @@ -288,7 +315,7 @@ function temporalFieldOracle(graph: ObjectGraph, object: string | undefined): Te if (!object) return UNBOUND; return (field) => { const verdict = resolveFieldPath(graph, object, field); - if (!verdict || verdict.kind !== 'ok' || verdict.injected) return false; + if (!verdict || verdict.kind !== 'ok') return false; const type = verdict.meta?.type; return typeof type === 'string' && FIELD_TYPED_TEMPORAL_TYPES.has(type); };