Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions .changeset/lint-injected-temporal-column-types.md
Original file line number Diff line number Diff line change
@@ -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<string, GraphField>` rather than a `ReadonlySet<string>`: 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<string>` to `ReadonlyMap<string, GraphField>`. `.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.
65 changes: 61 additions & 4 deletions packages/lint/src/object-graph.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down Expand Up @@ -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', () => {
Expand Down
116 changes: 92 additions & 24 deletions packages/lint/src/object-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>;
Expand Down Expand Up @@ -108,8 +129,19 @@ export interface GraphObject {
names: ReadonlySet<string>;
/** name → the slice above. */
fields: ReadonlyMap<string, GraphField>;
/** Registry-injected columns addressable on THIS object (skip 3). */
injected: ReadonlySet<string>;
/**
* 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<string, GraphField>;
}

/** object name → its resolvable surface, or `null` (skip 2). */
Expand Down Expand Up @@ -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;
Expand All @@ -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<string, GraphField>();
for (const name of injectedColumnsFor(obj)) {
injected.set(name, graphFieldOf(defs.get(name) ?? {}));
}
return { names, fields, injected };
}

/**
Expand All @@ -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 }
/**
Expand Down Expand Up @@ -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 };
}
Expand All @@ -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 };
}

Expand Down
Loading
Loading