Skip to content

Commit 31064ca

Browse files
os-billclaude
andauthored
fix(lint): a field-typed rule reads the registry's own type for an injected column, so created_at / updated_at stop escaping it (#16340) (#17430)
* fix(lint): read the registry's own type for an injected column so a field-typed rule can judge created_at / updated_at (#16340) The object graph recorded registry-injected system columns by NAME only, so a path resolving to one came back `{ kind: 'ok', injected: true }` with no `meta` and every rule asking a second question about the leaf had to stay silent. Measured before the change on one dashboard widget over one object declaring `close_date: date`: `close_date: 'last_30_days'` was refused while `created_at: 'last_30_days'`, `created_at: { $eq: … }` and `updated_at: { $in: [ … ] }` all passed lint and the runtime publish gate — and the engine then refused each of them with INVALID_FILTER / 400 on first render. `GraphObject.injected` becomes a `ReadonlyMap<string, GraphField>` carrying each column's definition, DERIVED from `injectedSystemColumnDefs` (the same spec tables `applySystemFields` spreads at registration) rather than transcribed, so lint cannot drift from the registry that provisions the column. The `filter-preset-comparand` oracle drops its `verdict.injected` bail: the marker says who wrote the column, the ruling turns on what it is. `id` keeps an empty slice — the driver provisions the primary key and no definition describes it. Pins the AUTHOR-FACING message verbatim, not only the graph shape, and pins the derivation against the spec table so a hand-copied type cannot pass. Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH Co-authored-by: Claude <noreply@anthropic.com> * fix(lint): judge an injected column on the same axis as an authored one in the dotted-filter and include rules (#16340) Two more consumers of a resolved leaf's `meta` inherit the registry type the object graph now carries, which is why the fix went to the seam rather than into `filter-preset-comparand`. `list-view-field-dotted` refuses `created_at.x` (a `datetime` scalar has nothing beneath it) and `owner_id.name` (a `lookup` stores an id, not an embedded document) — both already refused at the door by `assertFilterIsMaterializable` with the registry's field map in hand, so the linter's silence was the miss. `dataset-include-unknown` drops its `verdict.injected` bail, whose stated reason ("its type is registry-owned and invisible here") this change makes false: `include: ['owner_id']` joins, `include: ['created_at']` derives no join and is refused. `id` falls through the untyped branch of all three rules — the driver provisions the primary key and no definition table describes it. Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ba9f029 commit 31064ca

10 files changed

Lines changed: 487 additions & 55 deletions
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
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)
6+
7+
`@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.
8+
9+
Measured on `origin/main` `d57611dfd3`, one dashboard widget over one object declaring `close_date: date` and authoring no `created_at`:
10+
11+
| authored filter | before | after |
12+
|:--|:--|:--|
13+
| `close_date: 'last_30_days'` (authored `date`) | refused | refused |
14+
| `created_at: { $gte: 'last_30_days' }` (ordering — arm 1) | refused | refused |
15+
| `created_at: 'last_30_days'` | **silent** | refused |
16+
| `created_at: { $eq: 'last_30_days' }` | **silent** | refused |
17+
| `updated_at: { $in: ['last_30_days'] }` | **silent** | refused |
18+
| `stage: 'this_quarter'` (a `select` column) | silent | silent |
19+
20+
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.
21+
22+
## What changed
23+
24+
`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.
25+
26+
`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.
27+
28+
**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.
29+
30+
**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.
31+
32+
## Two more rules inherit it, in the same edit
33+
34+
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`:
35+
36+
- **`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.
37+
- **`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).
38+
39+
`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.
40+
41+
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.

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

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ import {
1717
RELATIONSHIP_FIELD_TYPES,
1818
} from './object-graph.js';
1919
import { walkFilterFieldKeys, type FilterFieldKey } from './filter-walk.js';
20+
// [#16340] Read back the registry's OWN definition table to assert the graph
21+
// derives its injected types rather than carrying a second copy of them.
22+
import { injectedSystemColumnDefs } from '@objectstack/spec/data';
2023

2124
const stack = {
2225
objects: [
@@ -94,10 +97,64 @@ describe('object-graph — resolveFieldPath verdicts', () => {
9497
expect(isUnjudgeable(verdict)).toBe(true);
9598
});
9699

97-
it('marks an injected leaf so a caller cannot mistake it for a typed field', () => {
98-
const verdict = resolveFieldPath(graph, 'crm_opportunity', 'created_at');
99-
expect(verdict).toMatchObject({ kind: 'ok', injected: true });
100-
expect((verdict as { meta?: unknown }).meta).toBeUndefined();
100+
// [#16340] An injected leaf resolves WITH the registry's own definition. The
101+
// marker still says the object does not author the column — that is the #8116
102+
// provenance question — but `meta` answers the second question a caller asks
103+
// ("is it temporal?") exactly as it does on an authored field. Before this,
104+
// the leaf carried no `meta` at all and every such caller had to stay silent;
105+
// `filter-preset-comparand` did, on the two most-filtered columns in the
106+
// platform.
107+
it("resolves an injected leaf with the registry's own type, and marks it injected", () => {
108+
expect(resolveFieldPath(graph, 'crm_opportunity', 'created_at')).toMatchObject({
109+
kind: 'ok', object: 'crm_opportunity', field: 'created_at', injected: true,
110+
meta: { type: 'datetime' },
111+
});
112+
expect(resolveFieldPath(graph, 'crm_opportunity', 'updated_at')).toMatchObject({
113+
kind: 'ok', injected: true, meta: { type: 'datetime' },
114+
});
115+
// An injected LOOKUP anchor carries its target too — read from the same
116+
// table, never re-declared here.
117+
expect(resolveFieldPath(graph, 'crm_opportunity', 'owner_id')).toMatchObject({
118+
kind: 'ok', injected: true, meta: { type: 'lookup', reference: 'sys_user' },
119+
});
120+
});
121+
122+
// The type is DERIVED, never transcribed: it must equal the definition the
123+
// registry spreads at registration, byte for byte. Reading the spec table
124+
// here is the assertion — a hand-copied 'datetime' in this package would pass
125+
// a literal pin and drift the day the registry re-types the column.
126+
it('reports the type the registry injects, not a copy of it', () => {
127+
const defs = injectedSystemColumnDefs(stack.objects[0]);
128+
for (const [name, def] of Object.entries(defs)) {
129+
const verdict = resolveFieldPath(graph, 'crm_opportunity', name);
130+
expect(verdict).toMatchObject({ kind: 'ok', injected: true });
131+
expect((verdict as { meta?: { type?: string } }).meta?.type).toBe(def.type);
132+
}
133+
expect(Object.keys(defs).length).toBeGreaterThan(0); // lit control
134+
});
135+
136+
// `id` is the one addressable column with NO definition behind it — the
137+
// DRIVER provisions the primary key. An empty slice is the truthful answer,
138+
// and it must stay distinguishable from "this column does not exist".
139+
it('resolves the primary key with an empty slice rather than a guessed type', () => {
140+
const verdict = resolveFieldPath(graph, 'crm_opportunity', 'id');
141+
expect(verdict).toMatchObject({ kind: 'ok', field: 'id', injected: true });
142+
expect((verdict as { meta?: { type?: string } }).meta?.type).toBeUndefined();
143+
expect(injectedSystemColumnDefs(stack.objects[0]).id).toBeUndefined(); // why
144+
});
145+
146+
// The opt-out rows are the registry's, not this module's: an object that
147+
// opts out has no injected column to resolve, so the reference is a real
148+
// miss and must still be reported.
149+
it('reports an injected name on an object the registry opts out of', () => {
150+
const optedOut = indexObjectGraph({
151+
objects: [{ name: 'seed_rows', systemFields: false, fields: { note: { type: 'text' } } }],
152+
});
153+
expect(resolveFieldPath(optedOut, 'seed_rows', 'created_at')).toMatchObject({
154+
kind: 'field-unknown', object: 'seed_rows', field: 'created_at',
155+
});
156+
// …and the driver's primary key survives even that row.
157+
expect(resolveFieldPath(optedOut, 'seed_rows', 'id')).toMatchObject({ kind: 'ok', injected: true });
101158
});
102159

103160
it('skips an object not in the stack, and one with no field map', () => {

packages/lint/src/object-graph.ts

Lines changed: 92 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,37 @@
3838
* compiling plugin-auth alone genuinely cannot see them.
3939
* 2. An object that declares no readable field map — ADR-0015 `external` and
4040
* datasource-introspected schemas whose columns resolve at runtime.
41-
* 3. Registry-injected system columns, which exist at runtime and never
42-
* appear in authored `fields`. Resolved per object through
41+
* 3. A relationship HOP through a registry-injected system column. The
42+
* columns themselves are not a skip — they exist at runtime, never
43+
* appear in authored `fields`, and resolve per object through
4344
* {@link injectedColumnsFor}, never the object-independent
44-
* `SYSTEM_FIELDS` union the two differ exactly where it matters (on
45+
* `SYSTEM_FIELDS` union (the two differ exactly where it matters: on
4546
* `ownership: 'none'` the platform injects no `owner_id`, so a reference
4647
* to it there is a real defect). The shipped
47-
* `showcase_task_metrics.created_at` dimension is skip 3's live case.
48+
* `showcase_task_metrics.created_at` dimension is that live case.
49+
*
50+
* ## An injected leaf carries its type (#16340)
51+
*
52+
* Skip 3 used to be wider: an injected leaf resolved by NAME alone, with no
53+
* `meta`, so every caller asking a second question about it — is it temporal?
54+
* is it a relationship? — had to treat it as unanswerable. That silence was
55+
* invisible to authors and it landed on the two most-filtered columns in the
56+
* platform: `filter-preset-comparand`'s field-typed arm refused
57+
* `close_date: 'last_30_days'` on an authored `date` column while
58+
* `created_at: 'last_30_days'` on the same widget passed the linter and the
59+
* runtime publish gate, only to be refused by the engine with a 400 on first
60+
* render.
61+
*
62+
* {@link GraphObject.injected} therefore carries each injected column's own
63+
* definition, DERIVED from `injectedColumnDefsFor` — the spec tables
64+
* `applySystemFields` spreads at registration — so lint never hand-copies
65+
* "`created_at` is a datetime" and cannot drift from the registry that
66+
* provisions it. The one column with no definition behind it is `id`: the
67+
* DRIVER provisions the primary key, so its `GraphField` is empty and a
68+
* second question about it is still unanswered — truthfully, and only there.
4869
*/
4970

50-
import { injectedColumnsFor } from './system-fields.js';
71+
import { injectedColumnDefsFor, injectedColumnsFor } from './system-fields.js';
5172

5273
/** Any plain metadata record. */
5374
type AnyRec = Record<string, unknown>;
@@ -108,8 +129,19 @@ export interface GraphObject {
108129
names: ReadonlySet<string>;
109130
/** name → the slice above. */
110131
fields: ReadonlyMap<string, GraphField>;
111-
/** Registry-injected columns addressable on THIS object (skip 3). */
112-
injected: ReadonlySet<string>;
132+
/**
133+
* Registry-injected columns addressable on THIS object, each mapped to the
134+
* registry's own definition of it (#16340).
135+
*
136+
* A MAP rather than a name set because a caller that resolves a reference
137+
* asks two questions, not one: does the column exist, and what is it? Both
138+
* halves are derived — membership from `injectedColumnsFor`, the slice from
139+
* `injectedColumnDefsFor` — so neither can drift from `applySystemFields`.
140+
* `.has(name)` answers the first question exactly as the old set did; `id`
141+
* maps to an empty slice because the driver, not the injection pass,
142+
* provisions the primary key and no definition table describes it.
143+
*/
144+
injected: ReadonlyMap<string, GraphField>;
113145
}
114146

115147
/** object name → its resolvable surface, or `null` (skip 2). */
@@ -190,6 +222,23 @@ function strName(v: unknown): string | undefined {
190222
return typeof v === 'string' && v.length > 0 ? v : undefined;
191223
}
192224

225+
/**
226+
* Read one field DEFINITION — authored or registry-injected — into the slice
227+
* this module exposes.
228+
*
229+
* One reader for both sources on purpose: an injected `created_at` and an
230+
* authored `close_date` are the same kind of answer to the same question, and
231+
* a second reader here would be free to disagree with this one about what
232+
* `type` means.
233+
*/
234+
function graphFieldOf(def: AnyRec): GraphField {
235+
return {
236+
type: typeof def.type === 'string' ? def.type : undefined,
237+
reference: strName(def.reference),
238+
multiple: def.multiple === true ? true : undefined,
239+
};
240+
}
241+
193242
/** Read one object's declared field map into the graph slice, or `null`. */
194243
function graphObjectOf(obj: AnyRec): GraphObject | null {
195244
const declared = obj.fields;
@@ -200,14 +249,20 @@ function graphObjectOf(obj: AnyRec): GraphObject | null {
200249
const n = strName(f.name);
201250
if (!n) continue;
202251
names.add(n);
203-
fields.set(n, {
204-
type: typeof f.type === 'string' ? f.type : undefined,
205-
reference: strName(f.reference),
206-
multiple: f.multiple === true ? true : undefined,
207-
});
252+
fields.set(n, graphFieldOf(f));
208253
}
209254
if (names.size === 0) return null;
210-
return { names, fields, injected: injectedColumnsFor(obj) };
255+
256+
// WHICH columns are injected and WHAT each one is are two derivations over
257+
// one plan (`resolveInjectedSystemColumns`), so they cannot disagree about
258+
// membership. `id` is in the first and not the second — the driver's primary
259+
// key has no definition table — and lands on an empty slice.
260+
const defs = injectedColumnDefsFor(obj);
261+
const injected = new Map<string, GraphField>();
262+
for (const name of injectedColumnsFor(obj)) {
263+
injected.set(name, graphFieldOf(defs.get(name) ?? {}));
264+
}
265+
return { names, fields, injected };
211266
}
212267

213268
/**
@@ -228,12 +283,21 @@ export function indexObjectGraph(stack: unknown): ObjectGraph {
228283
export type FieldPathVerdict =
229284
/**
230285
* Every hop and the leaf resolved. `object` is the object the LEAF lives on.
231-
* `injected` marks a leaf resolved through skip 3 — a registry-injected
232-
* column, real at runtime, whose TYPE and relationship target are
233-
* registry-owned and invisible here. A caller asking a second question about
234-
* the leaf (is it a relationship? is it materialised?) must treat an
235-
* `injected` leaf as unanswerable rather than assume the absence of a
236-
* declared type means the absence of the property.
286+
* `injected` marks a leaf the object does not author — a registry-injected
287+
* column, real at runtime.
288+
*
289+
* `meta` is populated for BOTH kinds (#16340): an injected leaf carries the
290+
* registry's own definition, so a caller asking a second question about it
291+
* ("is it temporal?") reads `meta.type` exactly as it does on an authored
292+
* field. The marker remains because "authored" and "injected" are still
293+
* different facts — the #8116 provenance question is asked only of injected
294+
* leaves, and an author-DECLARED column of the same name is one the author
295+
* vouches for.
296+
*
297+
* The one leaf with an EMPTY `meta` is `id`: the driver provisions the
298+
* primary key, so no definition describes it and a second question about it
299+
* genuinely has no answer here. ⛔ Do not read an absent `meta.type` as the
300+
* absence of the property — read it as "not answerable for this column".
237301
*/
238302
| { kind: 'ok'; object: string; field: string; meta?: GraphField; injected?: true }
239303
/**
@@ -296,10 +360,13 @@ export function resolveFieldPath(
296360
const meta = obj.fields.get(segment);
297361
if (!meta) {
298362
// An injected system column is REAL and some of them are relationships
299-
// (`owner_id` is a lookup at the registry), but their type and target are
300-
// registry-owned and invisible here — so `owner.name` is unanswerable,
301-
// not a miss. Reporting it would be the false positive skip 3 exists to
302-
// avoid; assuming it resolves would be the fail-open on the other side.
363+
// (`owner_id` is a `lookup` to `sys_user` at the registry). Reporting the
364+
// hop would be the false positive skip 3 exists to avoid, so it stays a
365+
// SKIP — deliberately, not for want of a target: since #16340 the slice
366+
// carries `reference`, and traversing it would newly JUDGE every path
367+
// through a platform anchor (`owner_id.name` and its siblings) wherever
368+
// `sys_user` is compiled into the stack. That is a widening with its own
369+
// findings to measure, and it is not this seam's to make silently.
303370
if (obj.injected.has(segment)) {
304371
return { kind: 'unknowable', reason: 'injected-hop', object: current };
305372
}
@@ -320,7 +387,8 @@ export function resolveFieldPath(
320387

321388
const leaf = segments[segments.length - 1];
322389
if (obj.names.has(leaf)) return { kind: 'ok', object: current, field: leaf, meta: obj.fields.get(leaf) };
323-
if (obj.injected.has(leaf)) return { kind: 'ok', object: current, field: leaf, injected: true };
390+
const injectedMeta = obj.injected.get(leaf);
391+
if (injectedMeta) return { kind: 'ok', object: current, field: leaf, meta: injectedMeta, injected: true };
324392
return { kind: 'field-unknown', object: current, field: leaf, candidates: obj.names };
325393
}
326394

0 commit comments

Comments
 (0)