From 8ffaec15b504b9326fd2ea63bb158a3e708ad68c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 17:36:45 +0000 Subject: [PATCH] feat(lint): resolve an ADR-0021 dataset's own references at validate/build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dataset could name a base object that does not exist, join a relationship that does not exist, and bind every dimension and measure to fields that do not exist, and `objectstack validate` exited 0 with "Validation passed"; `build` wrote the dangling dataset into dist/objectstack.json. The author-time rule pass already walked those exact nodes — `filter-token-unknown` stands at `datasets[i].measures[j].filter..$lt` and reasons about the VALUE — so the traversal and the machinery were both present and only the resolution was missing. Adds `validateDatasetReferences` to the reference-integrity suite with four gating rule ids (dataset-include-unknown, dataset-field-unknown, dataset-field-not-included, dataset-filter-field-unknown), and adds the base object as a `datasets[].object` reference site on `validateObjectReferences`, where the curated cross-package severity ladder already lives. Two reusable seams ship with it, exported and pinned by their own tests: `object-graph.ts` (indexObjectGraph / resolveFieldPath, a discriminated verdict union rather than a boolean) and `walkFilterFieldKeys` in `filter-walk.ts` (the field-key half of a filter subtree, across all three authored shapes). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV --- .changeset/dataset-reference-resolution.md | 95 ++++ packages/lint/src/filter-walk.ts | 132 +++++ packages/lint/src/index.ts | 31 ++ packages/lint/src/object-graph.test.ts | 196 +++++++ packages/lint/src/object-graph.ts | 310 +++++++++++ .../src/reference-integrity-suite.test.ts | 13 +- .../lint/src/reference-integrity-suite.ts | 11 + .../src/validate-dataset-references.test.ts | 526 ++++++++++++++++++ .../lint/src/validate-dataset-references.ts | 449 +++++++++++++++ .../src/validate-object-references.test.ts | 67 +++ .../lint/src/validate-object-references.ts | 32 ++ 11 files changed, 1861 insertions(+), 1 deletion(-) create mode 100644 .changeset/dataset-reference-resolution.md create mode 100644 packages/lint/src/object-graph.test.ts create mode 100644 packages/lint/src/object-graph.ts create mode 100644 packages/lint/src/validate-dataset-references.test.ts create mode 100644 packages/lint/src/validate-dataset-references.ts diff --git a/.changeset/dataset-reference-resolution.md b/.changeset/dataset-reference-resolution.md new file mode 100644 index 0000000000..687a8f3ffe --- /dev/null +++ b/.changeset/dataset-reference-resolution.md @@ -0,0 +1,95 @@ +--- +'@objectstack/lint': minor +--- + +Resolve an ADR-0021 dataset's own references — base object, `include[]`, +`dimensions[].field` / `measures[].field`, and filter KEYS — at +`validate`/`build` (#14105) + +A dataset could name a **base object that does not exist**, join a +**relationship that does not exist**, and bind every dimension and measure to +**fields that do not exist**, and `objectstack validate` exited **0** with +`✓ Validation passed`. `objectstack build` also exited 0 and wrote the dangling +dataset into `dist/objectstack.json`. + +The sting was that the author-time rule pass **already walked those exact +nodes**. Measured on published 17.2.0, each mutation applied on its own and +confirmed on disk before running: + +| mutation | before | after | +|:----------------------------------------------------|:-------|:------| +| dimension `field` → a base field that does not exist | passed | `dataset-field-unknown` | +| dimension `field` → a joined field that does not exist | passed | `dataset-field-unknown` | +| measure `field` → a field that does not exist | passed | `dataset-field-unknown` | +| measure filter KEY → a field that does not exist | passed | `dataset-filter-field-unknown` | +| `include[]` → a relationship that does not exist | passed | `dataset-include-unknown` | +| `object` → an object that does not exist | passed | `object-reference-unknown` | + +The two controls in that measurement — a duplicate measure name +(`DatasetSchema.superRefine`) and a bad date macro in a **measure filter** +(`filter-token-unknown`) — both failed the build, so datasets were +demonstrably in the validation path the whole time. `filter-token-unknown` +already stood at `datasets[1].measures[1].filter.last_update_at.$lt` and +reasoned about the **value**; nothing standing in that same position resolved +the **key**, or the sibling `field` one level up. + +This matters more for a dataset than for most metadata because a dataset is the +semantic layer: dashboards and reports bind its dimensions and measures by name +(ADR-0021), and the consumer end of that binding is already guarded +(`widget-dataset-unknown` / `widget-dimension-unknown` / `widget-measure-unknown`, +#7529/#8902). So the surviving hole was the quiet one — every binding resolves, +the board renders, and the charts are empty or subtly wrong because the dataset +underneath addresses columns that do not exist. + +**Five verdicts, all `error`.** Four are new rule ids on a new suite member, +`validateDatasetReferences`: + +- `dataset-include-unknown` — an `include[]` entry that resolves to nothing, or + to a field that is not a relationship, so no join can be derived from it. +- `dataset-field-unknown` — a dimension or measure `field` path that resolves to + no column, on the base object or on any joined object along the path. +- `dataset-field-not-included` — the second real check: a dotted path that + RESOLVES, but whose relationship prefix was never declared in `include`. + ADR-0021 D-C joins only declared paths, so the column is out of the query's + reach however real it is. +- `dataset-filter-field-unknown` — a filter key on `Dataset.filter` or + `measures[].filter`, in any of the three authored filter shapes. + +The fifth, the base object itself, lands on `validateObjectReferences` as a new +`datasets[].object` reference site rather than as a sixth id here. That rule's +charter IS object-name references that are plain `z.string()`, and putting it +there buys the curated cross-package severity ladder: the platform's own +`system.datasets.ts` declares five datasets over `sys_*` objects, three of which +live in packages a stack compiling plugin-auth alone cannot see. All five +resolve through `PLATFORM_PROVIDED_OBJECT_NAMES`; a local "not in this stack ⇒ +error" check would have reported every one of them. When the base object does +not resolve, `validateDatasetReferences` skips the dataset entirely, so one typo +yields one finding rather than one per dimension, measure and filter key. + +**Skips, so a finding is never a guess** (ADR-0072 D1): an object this stack +does not define, an object with no readable field map (ADR-0015 `external` and +introspected schemas), a registry-injected system column, and any hop *through* +one — an injected `owner_id` is a lookup at the registry whose target is +invisible here, so `owner_id.name` is unanswerable rather than a miss. The +shipped `showcase_task_metrics` dimension `{ field: 'created_at' }` is that skip's +live case, and every shipped dataset in the repo is silent under the new rule. + +**Two reusable seams ship with it**, newly exported, because the same two +questions are asked at a dashboard widget's filter keys and `sortBy` and at a +list view's field positions, and three independent copies of a hop-walker drift: + +- `object-graph.ts` — `indexObjectGraph` / `resolveFieldPath` / `isUnjudgeable`, + answering "what does this `relationship[.relationship].field` path resolve to?" + as a discriminated **verdict** union rather than a boolean, so a caller can + tell "this hop is not a relationship" from "this leaf does not exist" and write + the right prescription. Plus `nearestName` / `suggestName` / `listNames`. +- `walkFilterFieldKeys` (`filter-walk.ts`) — the FIELD-KEY half of a filter + subtree, beside the subtree-finding half that module already owns. It handles + all three authored shapes (Mongo condition object, `{ field, operator, value }` + rules, `[field, op, value]` triples), because a reader that handles only one + shape is the exact bug #3574 was filed against, and it composes a nested + condition object into one relationship path so `{ account: { region: … } }` + reports `account.region` rather than a bare `region` resolved against the + wrong object. + +Both hold mechanism only — no rule ids, no severities, no findings. diff --git a/packages/lint/src/filter-walk.ts b/packages/lint/src/filter-walk.ts index a17e7aa188..50a3c758f3 100644 --- a/packages/lint/src/filter-walk.ts +++ b/packages/lint/src/filter-walk.ts @@ -35,8 +35,21 @@ * they resolve an additional vocabulary (`AppContextSelector` ids such as * `{active_package}`) that is meaningless in a filter, and restricting the walk * is what holds false positives at zero. + * + * ## The two halves of a filter, and where each one is answered + * + * `walkAuthoredFilters` finds the SUBTREES. Inside one, a rule wants either the + * VALUES (`validate-filter-tokens.ts` classifies placeholders in them; + * `validate-preset-comparands.ts` judges ordering comparands) or the FIELD KEYS + * — the names the query is filtered BY. {@link walkFilterFieldKeys} is the + * second half (#14105), and it is here rather than in its first caller because + * the shape dispatch is identical to the one `validate-preset-comparands.ts` + * already performs on values: the platform authors filters three ways, and a + * reader that handles only one shape is the exact bug #3574 was filed against. */ +import { VALID_AST_OPERATORS } from '@objectstack/spec/data'; + /** Any plain metadata record. */ type AnyRec = Record; @@ -162,3 +175,122 @@ export function walkAuthoredFilters( }); } } + +// ── The FIELD-KEY half of a filter subtree (#14105) ────────────────────────── + +/** One field position inside an authored filter. */ +export interface FilterFieldKey { + /** + * The field the condition filters BY, exactly as authored — a bare name + * (`status`), or a dotted relationship path (`account.region`, whether + * spelled that way or reached by descending a nested condition object). + */ + field: string; + /** Config path of the position, e.g. `datasets[1].measures[1].filter.last_update_at`. */ + path: string; +} + +/** Recursion guard — an authored filter is a bounded document, not a graph. */ +const MAX_KEY_DEPTH = 32; + +function isPlainObject(v: unknown): v is AnyRec { + return !!v && typeof v === 'object' && !Array.isArray(v) && !(v instanceof Date); +} + +/** + * Emit the field positions of one Mongo-style condition NODE. + * + * `prefix` carries the relationship path accumulated by descending nested + * condition objects, so `{ account: { region: { $eq: 'emea' } } }` reports the + * single position `account.region` rather than a bare `region` that would + * resolve against the wrong object. A node whose value carries `$` operators is + * a leaf condition and ends the descent; a `$`-prefixed key that is not a + * recognised combinator is skipped and NOT descended, matching + * `validate-preset-comparands.ts` — an unrecognised operator's operand shape is + * not ours to guess. + */ +function conditionFieldKeys( + node: AnyRec, + path: string, + prefix: string, + visit: (key: FilterFieldKey) => void, + depth: number, +): void { + if (depth > MAX_KEY_DEPTH) return; + for (const [key, value] of Object.entries(node)) { + const here = `${path}.${key}`; + if (key === '$and' || key === '$or') { + if (Array.isArray(value)) { + value.forEach((arm, i) => { + if (isPlainObject(arm)) conditionFieldKeys(arm, `${here}[${i}]`, prefix, visit, depth + 1); + }); + } + continue; + } + if (key === '$not') { + if (isPlainObject(value)) conditionFieldKeys(value, here, prefix, visit, depth + 1); + continue; + } + if (key.startsWith('$')) continue; // unrecognised combinator + const field = prefix ? `${prefix}.${key}` : key; + if (isPlainObject(value) && !Object.keys(value).some((k) => k.startsWith('$'))) { + // Nested relation / deep equality — the field position is one level + // deeper, so descend rather than reporting the intermediate hop twice. + // An EMPTY nested object addresses nothing further; report the hop itself. + if (Object.keys(value).length > 0) { + conditionFieldKeys(value, here, field, visit, depth + 1); + continue; + } + } + visit({ field, path: here }); + } +} + +/** + * Emit every FIELD KEY inside one authored filter subtree, whatever shape it + * was authored in — the key half of what `validate-preset-comparands.ts` does + * for values, and the traversal `filter-token-unknown` already performs while + * reasoning only about the strings it finds. + * + * Holds no judgement: it does not know which object the filter is bound to and + * emits no findings. Resolution is {@link resolveFieldPath}'s job + * (`object-graph.ts`) and the verdict is the caller's. + */ +export function walkFilterFieldKeys( + node: unknown, + path: string, + visit: (key: FilterFieldKey) => void, + depth = 0, +): void { + if (depth > MAX_KEY_DEPTH) return; + + if (Array.isArray(node)) { + // Triple: ['field', op, value] — the field position is a non-keyword + // string and the operator position is in the AST vocabulary (the + // `isFilterAST` test `validate-preset-comparands.ts` uses). + if ( + typeof node[0] === 'string' && typeof node[1] === 'string' + && !['and', 'or'].includes(node[0].toLowerCase()) + && VALID_AST_OPERATORS.has(node[1].toLowerCase()) + ) { + visit({ field: node[0], path: `${path}[0]` }); + return; + } + // Group ['and'|'or', ...members] or a bare list — recurse the members. + node.forEach((member, i) => { + if (typeof member === 'string') return; // the leading keyword + walkFilterFieldKeys(member, `${path}[${i}]`, visit, depth + 1); + }); + return; + } + + if (!isPlainObject(node)) return; + + // View filter rule: { field, operator[, value] }. + if (typeof node.field === 'string' && typeof node.operator === 'string') { + visit({ field: node.field, path: `${path}.field` }); + return; + } + + conditionFieldKeys(node, path, '', visit, depth); +} diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 28316b5544..7239244f7c 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -467,6 +467,37 @@ export { } from './validate-chart-bindings.js'; export type { ChartBindingFinding, ChartBindingSeverity } from './validate-chart-bindings.js'; +// [#14105] The layer BELOW the chart/widget binding rules above: a dataset's +// own `include[]`, `dimensions[].field`, `measures[].field` and filter KEYS, +// resolved against the object graph. Its base-object half is +// `validateObjectReferences`' `datasets[].object` site. +export { + validateDatasetReferences, + DATASET_INCLUDE_UNKNOWN, + DATASET_FIELD_UNKNOWN, + DATASET_FIELD_NOT_INCLUDED, + DATASET_FILTER_FIELD_UNKNOWN, +} from './validate-dataset-references.js'; +export type { DatasetRefFinding, DatasetRefSeverity } from './validate-dataset-references.js'; + +// The two reusable seams the rule above is written on, exported because the +// queued siblings (#14148 widget filter keys + sortBy, #14107 list-view field +// positions) must reuse ONE mechanism rather than growing a second hop-walker +// and a second filter-key reader. Both hold mechanism only — no rule ids, no +// severities, no findings; the judgement stays with the rule that asks. +export { + indexObjectGraph, + resolveFieldPath, + isUnjudgeable, + nearestName, + suggestName, + listNames, + RELATIONSHIP_FIELD_TYPES, +} from './object-graph.js'; +export type { ObjectGraph, GraphObject, GraphField, FieldPathVerdict } from './object-graph.js'; +export { walkFilterFieldKeys } from './filter-walk.js'; +export type { FilterFieldKey } from './filter-walk.js'; + // #4762 — the two STATIC artifacts an object validation rule carries (a // `format` rule's `regex`, a `json_schema` rule's `schema`) are fail-OPEN at // runtime: one that does not compile is logged and skipped, so the rule is diff --git a/packages/lint/src/object-graph.test.ts b/packages/lint/src/object-graph.test.ts new file mode 100644 index 0000000000..b8c435b527 --- /dev/null +++ b/packages/lint/src/object-graph.test.ts @@ -0,0 +1,196 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The seam's own pins (#14105). `validate-dataset-references.test.ts` exercises +// this module through one caller; these pin the VERDICTS directly, because the +// queued siblings (#14148 widget filter keys + sortBy, #14107 list-view field +// positions) consume the verdict union rather than that caller's findings. A +// verdict that silently changes kind would break them with a green suite here +// otherwise. + +import { describe, it, expect } from 'vitest'; +import { + indexObjectGraph, + resolveFieldPath, + isUnjudgeable, + nearestName, + listNames, + RELATIONSHIP_FIELD_TYPES, +} from './object-graph.js'; +import { walkFilterFieldKeys, type FilterFieldKey } from './filter-walk.js'; + +const stack = { + objects: [ + { + name: 'crm_opportunity', + fields: { + amount: { type: 'currency', label: 'Amount' }, + account: { type: 'lookup', label: 'Account', reference: 'crm_account' }, + untargeted: { type: 'lookup', label: 'Dangling' }, + }, + }, + { + name: 'crm_account', + fields: { + region: { type: 'text', label: 'Region' }, + owner: { type: 'user', label: 'Owner', reference: 'crm_person' }, + }, + }, + { name: 'crm_person', fields: { email: { type: 'email', label: 'Email' } } }, + // No readable field map — ADR-0015 external / introspected. + { name: 'ext_thing', datasource: 'remote' }, + ], +}; + +const graph = indexObjectGraph(stack); + +describe('object-graph — resolveFieldPath verdicts', () => { + it('resolves a base field', () => { + expect(resolveFieldPath(graph, 'crm_opportunity', 'amount')).toMatchObject({ + kind: 'ok', + object: 'crm_opportunity', + field: 'amount', + }); + }); + + it('resolves a two-hop path onto the object the LEAF lives on', () => { + expect(resolveFieldPath(graph, 'crm_opportunity', 'account.owner.email')).toMatchObject({ + kind: 'ok', + object: 'crm_person', + field: 'email', + }); + }); + + it('reports the leaf miss against the object it landed on', () => { + expect(resolveFieldPath(graph, 'crm_opportunity', 'account.regionn')).toMatchObject({ + kind: 'field-unknown', + object: 'crm_account', + field: 'regionn', + }); + }); + + it('reports an unknown hop with its 0-based position', () => { + expect(resolveFieldPath(graph, 'crm_opportunity', 'accunt.region')).toMatchObject({ + kind: 'hop-unknown', + at: 0, + segment: 'accunt', + object: 'crm_opportunity', + }); + }); + + it('distinguishes a hop through a NON-relationship from an unknown one', () => { + // The distinction is the whole reason the verdict is a union: "amount is + // not a relationship" and "accunt is not a field" need different fixes. + expect(resolveFieldPath(graph, 'crm_opportunity', 'amount.x')).toMatchObject({ + kind: 'hop-not-relationship', + at: 0, + segment: 'amount', + type: 'currency', + }); + }); + + it('treats a relationship with no reference target as unanswerable', () => { + const verdict = resolveFieldPath(graph, 'crm_opportunity', 'untargeted.x'); + expect(verdict).toMatchObject({ kind: 'hop-untargeted' }); + 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(); + }); + + it('skips an object not in the stack, and one with no field map', () => { + expect(isUnjudgeable(resolveFieldPath(graph, 'not_here', 'x'))).toBe(true); + expect(resolveFieldPath(graph, 'not_here', 'x')).toMatchObject({ reason: 'object-not-in-stack' }); + expect(resolveFieldPath(graph, 'ext_thing', 'x')).toMatchObject({ reason: 'no-field-map' }); + }); + + it('skips a hop through an injected column rather than guessing its target', () => { + expect(resolveFieldPath(graph, 'crm_opportunity', 'owner_id.name')).toMatchObject({ + kind: 'unknowable', + reason: 'injected-hop', + }); + }); + + it('answers undefined when there is nothing to resolve', () => { + expect(resolveFieldPath(graph, undefined, 'amount')).toBeUndefined(); + expect(resolveFieldPath(graph, 'crm_opportunity', '')).toBeUndefined(); + expect(isUnjudgeable(undefined)).toBe(true); + }); + + it('reads the name-keyed collection shape', () => { + const mapGraph = indexObjectGraph({ objects: { a: { fields: { n: { type: 'text' } } } } }); + expect(resolveFieldPath(mapGraph, 'a', 'n')).toMatchObject({ kind: 'ok' }); + }); + + it('carries the relationship type set the platform traverses', () => { + expect([...RELATIONSHIP_FIELD_TYPES].sort()).toEqual(['lookup', 'master_detail', 'tree', 'user']); + }); +}); + +describe('object-graph — suggestion helpers', () => { + it('suggests within the budget and stays silent outside it', () => { + expect(nearestName('regionn', ['region', 'amount'])).toBe('region'); + expect(nearestName('zzzzzzzz', ['region', 'amount'])).toBeUndefined(); + }); + + it('lists names sorted, or `(none)`', () => { + expect(listNames(['b', 'a'])).toBe('a, b'); + expect(listNames([])).toBe('(none)'); + }); +}); + +describe('filter-walk — walkFilterFieldKeys across the three authored shapes', () => { + const keys = (node: unknown): FilterFieldKey[] => { + const out: FilterFieldKey[] = []; + walkFilterFieldKeys(node, 'f', (k) => out.push(k)); + return out; + }; + + it('reads a Mongo condition object, operators and all', () => { + expect(keys({ status: 'open', amount: { $gt: 10 } })).toEqual([ + { field: 'status', path: 'f.status' }, + { field: 'amount', path: 'f.amount' }, + ]); + }); + + it('descends combinators without reporting them as fields', () => { + expect(keys({ $or: [{ a: 1 }, { $not: { b: 2 } }] })).toEqual([ + { field: 'a', path: 'f.$or[0].a' }, + { field: 'b', path: 'f.$or[1].$not.b' }, + ]); + }); + + it('composes a nested condition object into one relationship path', () => { + expect(keys({ account: { region: 'emea' } })).toEqual([ + { field: 'account.region', path: 'f.account.region' }, + ]); + }); + + it('reports the hop itself when the nested object is empty', () => { + expect(keys({ account: {} })).toEqual([{ field: 'account', path: 'f.account' }]); + }); + + it('reads the { field, operator, value } rule shape', () => { + expect(keys([{ field: 'status', operator: 'equals', value: 'open' }])).toEqual([ + { field: 'status', path: 'f[0].field' }, + ]); + }); + + it('reads triples and their and/or groups', () => { + expect(keys(['and', ['status', '=', 'open'], ['amount', '>', 1]])).toEqual([ + { field: 'status', path: 'f[1][0]' }, + { field: 'amount', path: 'f[2][0]' }, + ]); + }); + + it('does not descend an unrecognised $ operator', () => { + expect(keys({ $weird: { a: 1 } })).toEqual([]); + }); + + it('tolerates non-filter input', () => { + expect(keys(undefined)).toEqual([]); + expect(keys('a string')).toEqual([]); + }); +}); diff --git a/packages/lint/src/object-graph.ts b/packages/lint/src/object-graph.ts new file mode 100644 index 0000000000..ce7d38c8b9 --- /dev/null +++ b/packages/lint/src/object-graph.ts @@ -0,0 +1,310 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Shared resolution: what does a `relationship[.relationship].field` path + * address in this stack's object graph? (issue #14105) + * + * Three rules in this package need the same answer to the same question — "does + * this dotted field path resolve to a real column, through real joins?" — and + * they ask it at different positions: a dataset's `dimensions[].field` / + * `measures[].field` / filter keys (#14105), a widget's filter keys and + * `sortBy` (#14148), and a list view's field positions (#14107). The + * subtree-finding half of the same family already lives in `filter-walk.ts` + * (#3574/#5330); this file is the RESOLUTION half, written from that file's + * argument: with N copies of a hop-walker the next author fixes one of N and + * the survivors keep the old verdict. + * + * ## What is shared, and what deliberately is NOT + * + * The MECHANISM is shared: index the objects once, walk a dotted path hop by + * hop through the declared relationship fields, and return a VERDICT. The + * JUDGEMENT is the caller's — this module emits no findings, holds no rule ids + * and decides no severities, because the same verdict means different things at + * different positions (a dataset dimension's dangling path renders an empty + * chart; a list view's renders a blank column). {@link FieldPathVerdict} is + * deliberately a discriminated union rather than a boolean for exactly that + * reason: a caller that cannot distinguish "the head hop is not a + * relationship" from "the leaf field does not exist" cannot write the + * prescription an author needs. + * + * ## The three skips, matching every field-existence rule in this package + * + * Verdicts are `unknowable` — never a miss — when the graph cannot answer + * (ADR-0072 D1: one dead finding and authors stop trusting the linter): + * + * 1. An object this stack does not define. It may come from another package + * — the shipped `system.datasets.ts` is five datasets over `sys_*` + * objects that live in `plugin-audit` and the cloud runtime, so a stack + * 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 + * {@link injectedColumnsFor}, never the object-independent + * `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. + */ + +import { injectedColumnsFor } from './system-fields.js'; + +/** Any plain metadata record. */ +type AnyRec = Record; + +/** + * Field types that address ANOTHER object, so a `.` through one is a join + * the query compiler derives rather than a column read. + * + * `user` and `tree` are members alongside the two obvious ones for the reason + * `validate-flow-template-paths.ts` lists them: both store a foreign key and + * both are traversed the same way. A dataset `include` naming one is joinable + * exactly as a `lookup` is. + */ +export const RELATIONSHIP_FIELD_TYPES: ReadonlySet = new Set([ + 'lookup', + 'master_detail', + 'user', + 'tree', +]); + +/** The slice of one field this module reads. */ +export interface GraphField { + /** Declared `type`, when the author wrote one. */ + type?: string; + /** + * The object a relationship field addresses. + * + * `reference` is the ONLY spelling `FieldSchema` declares — `referenceTo` / + * `relatedTo` / `target` / `targetObject` / `lookupObject` are REJECTED + * aliases the strict error map maps back to it (#5017), so a field spelling + * one of them does not parse at all. Re-admitting an alias here would be the + * tolerant consumer Prime Directive #12 refuses, so only `reference` is read. + */ + reference?: string; +} + +/** + * One object's resolvable surface. `null` in the index marks an object that + * declares no readable field map, keeping "declared nothing" distinguishable + * from "not in this stack" — the same distinction + * {@link indexObjectSearchTargets} draws for the search axis. + */ +export interface GraphObject { + /** Authored field names. */ + names: ReadonlySet; + /** name → the slice above. */ + fields: ReadonlyMap; + /** Registry-injected columns addressable on THIS object (skip 3). */ + injected: ReadonlySet; +} + +/** object name → its resolvable surface, or `null` (skip 2). */ +export type ObjectGraph = ReadonlyMap; + +/** Coerce a collection (array or name-keyed map) to an array of records. */ +function asArray(v: unknown): AnyRec[] { + if (Array.isArray(v)) return v as AnyRec[]; + if (v && typeof v === 'object') { + return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) })); + } + return []; +} + +function strName(v: unknown): string | undefined { + return typeof v === 'string' && v.length > 0 ? v : undefined; +} + +/** Read one object's declared field map into the graph slice, or `null`. */ +function graphObjectOf(obj: AnyRec): GraphObject | null { + const declared = obj.fields; + if (!declared || typeof declared !== 'object') return null; + const names = new Set(); + const fields = new Map(); + for (const f of asArray(declared)) { + 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), + }); + } + if (names.size === 0) return null; + return { names, fields, injected: injectedColumnsFor(obj) }; +} + +/** + * Index a stack's objects once. Callers that resolve many paths build this a + * single time and hand it to every {@link resolveFieldPath} call. + */ +export function indexObjectGraph(stack: unknown): ObjectGraph { + const graph = new Map(); + if (!stack || typeof stack !== 'object') return graph; + for (const obj of asArray((stack as AnyRec).objects)) { + const name = strName(obj.name); + if (name) graph.set(name, graphObjectOf(obj)); + } + return graph; +} + +/** What a path resolved to, or precisely how it failed. */ +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. + */ + | { kind: 'ok'; object: string; field: string; meta?: GraphField; injected?: true } + /** + * The graph cannot answer — one of the three skips. Never report this: it is + * the absence of a judgement, not a passing one. + */ + | { kind: 'unknowable'; reason: 'object-not-in-stack' | 'no-field-map' | 'injected-hop'; object: string } + /** + * A hop names nothing on the object it was written against. `at` is the + * 0-based segment index, `object` the object the hop was resolved against. + */ + | { kind: 'hop-unknown'; at: number; segment: string; object: string; candidates: ReadonlySet } + /** + * A hop resolves to a real field that is NOT a relationship, so there is + * nothing to traverse through — the author wrote `amount.total` where + * `amount` is a number. + */ + | { kind: 'hop-not-relationship'; at: number; segment: string; object: string; type?: string } + /** + * A relationship hop resolves but declares no `reference`, so the target + * object is unknown and the rest of the path cannot be judged. Treated as a + * SKIP by callers for the same reason as `unknowable`. + */ + | { kind: 'hop-untargeted'; at: number; segment: string; object: string } + /** Every hop resolved; the LEAF names nothing on the object it landed on. */ + | { kind: 'field-unknown'; object: string; field: string; candidates: ReadonlySet }; + +/** + * Resolve `path` against `objectName` in `graph`. + * + * A bare `field` is the one-segment case and resolves against the root object + * directly. A dotted `a.b.field` walks `a`, then `b`, as to-one relationship + * hops, and resolves the leaf on whatever object the last hop landed on. + * + * ⛔ This function answers EXISTENCE only. Whether a hop was DECLARED joinable + * — an ADR-0021 dataset joins only what `Dataset.include` names — is a second + * question the caller owns, because only the caller knows which declaration + * governs the position. Conflating them here would make the seam unusable at + * the positions that have no `include` (a list view's field path, #14107). + */ +export function resolveFieldPath( + graph: ObjectGraph, + objectName: string | undefined, + path: string, +): FieldPathVerdict | undefined { + const root = strName(objectName); + const raw = strName(path); + if (!root || !raw) return undefined; // nothing to resolve + + const segments = raw.split('.'); + let current = root; + + // Every segment but the last is a relationship hop. + for (let i = 0; i < segments.length - 1; i++) { + if (!graph.has(current)) return { kind: 'unknowable', reason: 'object-not-in-stack', object: current }; + const obj = graph.get(current); + if (!obj) return { kind: 'unknowable', reason: 'no-field-map', object: current }; + + const segment = segments[i]; + 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. + if (obj.injected.has(segment)) { + return { kind: 'unknowable', reason: 'injected-hop', object: current }; + } + return { kind: 'hop-unknown', at: i, segment, object: current, candidates: obj.names }; + } + if (!meta.type || !RELATIONSHIP_FIELD_TYPES.has(meta.type)) { + return { kind: 'hop-not-relationship', at: i, segment, object: current, type: meta.type }; + } + if (!meta.reference) { + return { kind: 'hop-untargeted', at: i, segment, object: current }; + } + current = meta.reference; + } + + if (!graph.has(current)) return { kind: 'unknowable', reason: 'object-not-in-stack', object: current }; + const obj = graph.get(current); + if (!obj) return { kind: 'unknowable', reason: 'no-field-map', object: current }; + + 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 }; + return { kind: 'field-unknown', object: current, field: leaf, candidates: obj.names }; +} + +/** + * True when the verdict is one no rule may report — the graph could not answer. + * Callers spell the skip through this predicate rather than re-listing the + * kinds, so a future verdict added to the union defaults to being reported + * loudly (a missed skip is a visible false positive; a missed report is + * silence, which is the failure mode this whole family exists to end). + */ +export function isUnjudgeable(verdict: FieldPathVerdict | undefined): boolean { + return !verdict || verdict.kind === 'unknowable' || verdict.kind === 'hop-untargeted'; +} + +/** + * Nearest declared name for a typo'd reference, or `undefined` when nothing is + * close enough. The budget — `max(2, floor(len/3))` — is the one + * `validate-sortable-fields.ts` and `validate-object-references.ts` already + * use, restated here rather than imported from either because neither exports + * it; consolidating the three copies is recorded as a follow-up rather than + * done under this card's scope. + */ +export function nearestName(target: string, known: Iterable): string | undefined { + let best: string | undefined; + let bestScore = Infinity; + for (const candidate of known) { + const d = distance(target, candidate); + if (d < bestScore) { + bestScore = d; + best = candidate; + } + } + return best && bestScore <= Math.max(2, Math.floor(target.length / 3)) ? best : undefined; +} + +/** ` Did you mean "x"?`, or the empty string — the platform's message shape. */ +export function suggestName(target: string, known: Iterable): string { + const best = nearestName(target, known); + return best ? ` Did you mean "${best}"?` : ''; +} + +function distance(a: string, b: string): number { + const m = a.length; + const n = b.length; + if (m === 0) return n; + if (n === 0) return m; + let prev = Array.from({ length: n + 1 }, (_, j) => j); + for (let i = 1; i <= m; i++) { + const curr = [i, ...new Array(n).fill(0)]; + for (let j = 1; j <= n; j++) { + const cost = a[i - 1] === b[j - 1] ? 0 : 1; + curr[j] = Math.min(curr[j - 1] + 1, prev[j] + 1, prev[j - 1] + cost); + } + prev = curr; + } + return prev[n]; +} + +/** A readable field list for a hint, or `(none)`. */ +export function listNames(names: Iterable): string { + const arr = [...names].sort(); + return arr.length > 0 ? arr.join(', ') : '(none)'; +} diff --git a/packages/lint/src/reference-integrity-suite.test.ts b/packages/lint/src/reference-integrity-suite.test.ts index 04da0f0949..b50d0f7451 100644 --- a/packages/lint/src/reference-integrity-suite.test.ts +++ b/packages/lint/src/reference-integrity-suite.test.ts @@ -21,6 +21,10 @@ describe('reference-integrity suite — membership', () => { 'validateActionNameRefs', 'validatePageFieldBindings', 'validateChartBindings', + // [#14105] The dataset's OWN references, one level below the two binding + // rules above it — the conscious edit this written-out list exists to + // force, placed so the family reports top-down. + 'validateDatasetReferences', 'validateNavAccess', 'validateNavTargetRefs', // [#13216] The conscious edit this written-out list exists to force: the @@ -194,7 +198,13 @@ describe('reference-integrity suite — every member actually runs', () => { { name: 'lead_metrics', object: 'crm_lead', - dimensions: [{ name: 'source' }], + // validateDatasetReferences (#14105): `budget` is not a field on + // crm_lead, so the dimension groups by a column that does not exist — + // the chart renders successfully and empty. The dimension NAME stays + // `source` because validateChartBindings resolves the report's + // `xAxis: 'source'` against it; the two members must be able to go + // silent independently. + dimensions: [{ name: 'source', field: 'budget' }], measures: [{ name: 'count_leads', aggregate: 'count' }], }, ], @@ -318,6 +328,7 @@ describe('reference-integrity suite — every member actually runs', () => { expect(rules).toContain('action-name-undefined'); expect(rules).toContain('page-field-unknown'); expect(rules).toContain('chart-measure-unknown'); + expect(rules).toContain('dataset-field-unknown'); expect(rules).toContain('nav-object-ungranted'); expect(rules).toContain('nav-object-unservable'); expect(rules).toContain('translation-target-unknown'); diff --git a/packages/lint/src/reference-integrity-suite.ts b/packages/lint/src/reference-integrity-suite.ts index e3772a3ce2..8351d2a509 100644 --- a/packages/lint/src/reference-integrity-suite.ts +++ b/packages/lint/src/reference-integrity-suite.ts @@ -82,6 +82,7 @@ import { validateSortableFields } from './validate-sortable-fields.js'; import { validateActionNameRefs } from './validate-action-name-refs.js'; import { validatePageFieldBindings } from './validate-page-field-bindings.js'; import { validateChartBindings } from './validate-chart-bindings.js'; +import { validateDatasetReferences } from './validate-dataset-references.js'; import { validateNavAccess } from './validate-nav-access.js'; import { validateNavTargetRefs } from './validate-nav-target-refs.js'; import { validateViewPageRefs } from './validate-view-page-refs.js'; @@ -189,6 +190,16 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [ { name: 'validateActionNameRefs', run: validateActionNameRefs }, { name: 'validatePageFieldBindings', run: validatePageFieldBindings }, { name: 'validateChartBindings', run: validateChartBindings }, + // [#14105] One level BELOW the two members above it. `validateChartBindings` + // and `validateWidgetBindings` resolve a presentation's binding against the + // dataset (#7529/#8902); this one resolves the DATASET's own references — + // `include[]`, `dimensions[].field`, `measures[].field` and filter KEYS — + // against the object graph. Placed here so the family reports top-down: a + // board proven to point at a real dataset, then that dataset proven to point + // at real columns. Its base-object sibling is `validateObjectReferences` + // (rung ①, and the reason this member skips a dataset whose object does not + // resolve rather than repeating the typo once per position). + { name: 'validateDatasetReferences', run: validateDatasetReferences }, { name: 'validateNavAccess', run: validateNavAccess }, // Nav targets that are NOT object names — page/report/dashboard. Restores the // coverage `defineStack`'s own cross-reference block switches off whenever the diff --git a/packages/lint/src/validate-dataset-references.test.ts b/packages/lint/src/validate-dataset-references.test.ts new file mode 100644 index 0000000000..fbd5ad7257 --- /dev/null +++ b/packages/lint/src/validate-dataset-references.test.ts @@ -0,0 +1,526 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { + validateDatasetReferences, + DATASET_INCLUDE_UNKNOWN, + DATASET_FIELD_UNKNOWN, + DATASET_FIELD_NOT_INCLUDED, + DATASET_FILTER_FIELD_UNKNOWN, +} from './validate-dataset-references.js'; + +/** + * The object graph every case below resolves against. Deliberately a real + * two-hop chain — `duly_task › duty (lookup) › duly_duty › owner (lookup) › + * sys_user` — because the ADR-0071 multi-hop path is where a resolver that + * only ever looks at the base object passes everything it should catch. + */ +const objects = [ + { + name: 'duly_task', + fields: { + period_key: { type: 'text', label: 'Period' }, + last_update_at: { type: 'datetime', label: 'Last Update' }, + amount: { type: 'currency', label: 'Amount' }, + status: { type: 'select', label: 'Status' }, + duty: { type: 'lookup', label: 'Duty', reference: 'duly_duty' }, + }, + }, + { + name: 'duly_duty', + fields: { + frequency: { type: 'select', label: 'Frequency' }, + owner: { type: 'lookup', label: 'Owner', reference: 'duly_person' }, + }, + }, + { + name: 'duly_person', + fields: { region: { type: 'text', label: 'Region' } }, + }, +]; + +/** One dataset over `duly_task`, with whatever the case under test declares. */ +const stackWith = (dataset: Record) => ({ + objects, + datasets: [{ name: 'duly_health', label: 'Duty Health', object: 'duly_task', ...dataset }], +}); + +const rules = (stack: Record) => + validateDatasetReferences(stack).map((f) => f.rule); + +// ── The measured matrix (#14105) ───────────────────────────────────────────── +// +// Each row is one mutation the card measured on published 17.2.0, where +// `objectstack validate` exited 0 with "Validation passed". The clean spelling +// is asserted beside every one of them, because a rule that reports the typo +// AND the correct spelling has not narrowed the accept set, it has broken it. + +describe('validateDatasetReferences — the six shapes that validated clean before', () => { + it('refuses a dimension bound to a base field that does not exist', () => { + const findings = validateDatasetReferences( + stackWith({ dimensions: [{ name: 'period', field: 'period_kee' }], measures: [] }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(DATASET_FIELD_UNKNOWN); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('datasets[0].dimensions[0].field'); + expect(findings[0].where).toBe('dataset "duly_health" › dimension "period"'); + // The platform's message shape — the resolver knows the near miss. + expect(findings[0].message).toContain('Did you mean "period_key"?'); + }); + + it('accepts the same dimension spelled correctly', () => { + expect( + rules(stackWith({ dimensions: [{ name: 'period', field: 'period_key' }], measures: [] })), + ).toEqual([]); + }); + + it('refuses a dimension bound to a JOINED field that does not exist', () => { + const findings = validateDatasetReferences( + stackWith({ + include: ['duty'], + dimensions: [{ name: 'freq', field: 'duty.frequenci' }], + measures: [], + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(DATASET_FIELD_UNKNOWN); + // Resolved on the JOINED object, not the base one — the whole point of the + // hop walk. A resolver that only knew `duly_task` would have reported this + // path against the wrong object, or not at all. + expect(findings[0].message).toContain('object "duly_duty"'); + expect(findings[0].message).toContain('Did you mean "frequency"?'); + }); + + it('accepts a two-hop path when every hop and the leaf resolve', () => { + expect( + rules( + stackWith({ + include: ['duty.owner'], + dimensions: [{ name: 'region', field: 'duty.owner.region' }], + measures: [], + }), + ), + ).toEqual([]); + }); + + it('refuses a measure bound to a field that does not exist', () => { + const findings = validateDatasetReferences( + stackWith({ + dimensions: [], + measures: [{ name: 'freshness', aggregate: 'max', field: 'last_update_att' }], + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(DATASET_FIELD_UNKNOWN); + expect(findings[0].path).toBe('datasets[0].measures[0].field'); + expect(findings[0].where).toBe('dataset "duly_health" › measure "freshness"'); + }); + + it('refuses a measure FILTER KEY that does not exist — the value half already gated', () => { + const findings = validateDatasetReferences( + stackWith({ + dimensions: [], + measures: [ + { + name: 'stale', + aggregate: 'count', + filter: { last_update_attt: { $lt: '{7_days_ago}' } }, + }, + ], + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(DATASET_FILTER_FIELD_UNKNOWN); + // The exact position `filter-token-unknown` already reaches for the VALUE. + expect(findings[0].path).toBe('datasets[0].measures[0].filter.last_update_attt'); + expect(findings[0].message).toContain('Did you mean "last_update_at"?'); + }); + + it('accepts the same measure filter spelled correctly', () => { + expect( + rules( + stackWith({ + dimensions: [], + measures: [ + { name: 'stale', aggregate: 'count', filter: { last_update_at: { $lt: '{7_days_ago}' } } }, + ], + }), + ), + ).toEqual([]); + }); + + it('refuses an include[] entry naming a relationship that does not exist', () => { + const findings = validateDatasetReferences( + stackWith({ include: ['dutee'], dimensions: [], measures: [] }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(DATASET_INCLUDE_UNKNOWN); + expect(findings[0].path).toBe('datasets[0].include[0]'); + expect(findings[0].message).toContain('Did you mean "duty"?'); + }); + + it('leaves the base object to validate-object-references and reports nothing itself', () => { + // Row 6 of the measured matrix. `object: 'duly_tsk'` is judged by the + // object-name ladder (`object-reference-unknown`), and THIS rule must go + // silent so one typo does not yield one finding per dimension, measure and + // filter key. + expect( + validateDatasetReferences({ + objects, + datasets: [ + { + name: 'duly_health', + object: 'duly_tsk', + include: ['duty'], + dimensions: [{ name: 'period', field: 'period_key' }], + measures: [{ name: 'n', aggregate: 'count', field: 'amount' }], + }, + ], + }), + ).toEqual([]); + }); +}); + +// ── The second real check the card names: declared joinability ─────────────── + +describe('validateDatasetReferences — ADR-0021 joins only DECLARED paths', () => { + it('refuses a resolvable dotted path whose prefix is not in include', () => { + const findings = validateDatasetReferences( + stackWith({ dimensions: [{ name: 'freq', field: 'duty.frequency' }], measures: [] }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(DATASET_FIELD_NOT_INCLUDED); + expect(findings[0].severity).toBe('error'); + expect(findings[0].message).toContain('"duty" is not declared'); + expect(findings[0].hint).toContain('Add "duty" to include'); + }); + + it('accepts an intermediate hop implied by a longer declared path', () => { + // ADR-0021: declaring `a.b` implicitly includes `a`. So `duty.frequency` is + // joinable on the strength of `include: ['duty.owner']` alone. + expect( + rules( + stackWith({ + include: ['duty.owner'], + dimensions: [{ name: 'freq', field: 'duty.frequency' }], + measures: [], + }), + ), + ).toEqual([]); + }); + + it('never asks the include question of a BASE field', () => { + expect( + rules(stackWith({ dimensions: [{ name: 'amt', field: 'amount' }], measures: [] })), + ).toEqual([]); + }); + + it('reports existence, not joinability, when the path also fails to resolve', () => { + // Both defects are present (`nope` is neither declared in `include` nor a + // real relationship). Exactly ONE finding, and it is the one carrying the + // "did you mean" — a position that reports twice teaches the wrong fix. + const findings = validateDatasetReferences( + stackWith({ dimensions: [{ name: 'x', field: 'nope.frequency' }], measures: [] }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(DATASET_FIELD_UNKNOWN); + }); +}); + +// ── include[] that resolves but cannot be joined through ───────────────────── + +describe('validateDatasetReferences — include[] must name a RELATIONSHIP', () => { + it('refuses an include entry naming an ordinary field', () => { + const findings = validateDatasetReferences( + stackWith({ include: ['status'], dimensions: [], measures: [] }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(DATASET_INCLUDE_UNKNOWN); + expect(findings[0].message).toContain('`select` field'); + expect(findings[0].message).toContain('not a relationship'); + }); + + it('accepts an include entry naming a lookup', () => { + expect(rules(stackWith({ include: ['duty'], dimensions: [], measures: [] }))).toEqual([]); + }); + + it('refuses a multi-hop include whose intermediate hop is not traversable', () => { + const findings = validateDatasetReferences( + stackWith({ include: ['status.owner'], dimensions: [], measures: [] }), + ); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(DATASET_INCLUDE_UNKNOWN); + expect(findings[0].message).toContain('nothing to join through'); + }); +}); + +// ── All three authored filter shapes ───────────────────────────────────────── +// +// #3574's own failure mode was a resolver that handled ONE filter shape. The +// key half is walked by the shared `walkFilterFieldKeys`, and these pin that a +// filter authored any of the three ways is judged. + +describe('validateDatasetReferences — filter keys in every authored shape', () => { + const withFilter = (filter: unknown) => + validateDatasetReferences(stackWith({ dimensions: [], measures: [], filter })); + + it('reads the Mongo condition object', () => { + expect(withFilter({ nope: 'x' }).map((f) => f.rule)).toEqual([DATASET_FILTER_FIELD_UNKNOWN]); + expect(withFilter({ status: 'open' })).toEqual([]); + }); + + it('descends $and / $or / $not arms', () => { + const findings = withFilter({ $and: [{ status: 'open' }, { $not: { nope: 1 } }] }); + expect(findings.map((f) => f.rule)).toEqual([DATASET_FILTER_FIELD_UNKNOWN]); + expect(findings[0].path).toBe('datasets[0].filter.$and[1].$not.nope'); + }); + + it('reads the { field, operator, value } rule shape', () => { + const findings = withFilter([{ field: 'nope', operator: 'equals', value: 1 }]); + expect(findings.map((f) => f.rule)).toEqual([DATASET_FILTER_FIELD_UNKNOWN]); + expect(findings[0].path).toBe('datasets[0].filter[0].field'); + expect(withFilter([{ field: 'status', operator: 'equals', value: 'open' }])).toEqual([]); + }); + + it('reads the [field, op, value] triple shape', () => { + const findings = withFilter([['nope', '=', 1]]); + expect(findings.map((f) => f.rule)).toEqual([DATASET_FILTER_FIELD_UNKNOWN]); + expect(findings[0].path).toBe('datasets[0].filter[0][0]'); + expect(withFilter([['status', '=', 'open']])).toEqual([]); + }); + + it('composes a nested condition object into one relationship path', () => { + // `{ duty: { frequency: … } }` is deep equality through a relationship, so + // the field position is `duty.frequency` — NOT a bare `frequency`, which + // would resolve against the wrong object and report a phantom miss. + expect( + validateDatasetReferences( + stackWith({ + include: ['duty'], + dimensions: [], + measures: [], + filter: { duty: { frequency: 'daily' } }, + }), + ), + ).toEqual([]); + const findings = validateDatasetReferences( + stackWith({ + include: ['duty'], + dimensions: [], + measures: [], + filter: { duty: { frequenci: 'daily' } }, + }), + ); + expect(findings.map((f) => f.rule)).toEqual([DATASET_FILTER_FIELD_UNKNOWN]); + expect(findings[0].path).toBe('datasets[0].filter.duty.frequenci'); + }); + + it('does not judge the operand shape of an unrecognised $ operator', () => { + expect(withFilter({ $weird: { nope: 1 } })).toEqual([]); + }); +}); + +// ── The skips: never report what the graph cannot answer (ADR-0072 D1) ─────── + +describe('validateDatasetReferences — the three skips', () => { + it('skips a dataset over an object this stack does not define', () => { + // The live case: the platform ships five datasets over `sys_*` objects that + // live in plugin-audit and the cloud runtime. + expect( + validateDatasetReferences({ + objects, + datasets: [ + { + name: 'sys_audit_log_metrics', + object: 'sys_audit_log', + dimensions: [{ name: 'action', field: 'action' }], + measures: [{ name: 'event_count', aggregate: 'count' }], + }, + ], + }), + ).toEqual([]); + }); + + it('skips an object that declares no readable field map', () => { + expect( + validateDatasetReferences({ + objects: [{ name: 'ext_customer', datasource: 'remote' }], + datasets: [ + { + name: 'ext_metrics', + object: 'ext_customer', + dimensions: [{ name: 'region', field: 'region' }], + measures: [], + }, + ], + }), + ).toEqual([]); + }); + + it('skips a registry-injected system column', () => { + // The live case: `showcase_task_metrics` groups by `created_at`, a real + // runtime column that appears in no authored `fields`. + expect( + rules( + stackWith({ + dimensions: [{ name: 'created', field: 'created_at', dateGranularity: 'month' }], + measures: [], + }), + ), + ).toEqual([]); + }); + + it('skips a hop THROUGH an injected column, whose target is registry-owned', () => { + // `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. + expect(rules(stackWith({ dimensions: [{ name: 'o', field: 'owner_id.name' }], measures: [] }))).toEqual([]); + }); + + it('skips a relationship field that declares no reference target', () => { + expect( + validateDatasetReferences({ + objects: [ + { name: 'a', fields: { rel: { type: 'lookup', label: 'Rel' }, n: { type: 'text', label: 'N' } } }, + ], + datasets: [ + { name: 'm', object: 'a', include: ['rel'], dimensions: [{ name: 'x', field: 'rel.anything' }], measures: [] }, + ], + }), + ).toEqual([]); + }); +}); + +// ── Positions that carry no reference at all ───────────────────────────────── + +describe('validateDatasetReferences — shapes with nothing to resolve', () => { + it('accepts a count(*) measure with no field', () => { + expect(rules(stackWith({ dimensions: [], measures: [{ name: 'n', aggregate: 'count' }] }))).toEqual([]); + }); + + it('accepts a derived measure, whose refs are measure names the schema owns', () => { + expect( + rules( + stackWith({ + dimensions: [], + measures: [ + { name: 'a', aggregate: 'count' }, + { name: 'b', aggregate: 'count' }, + { name: 'r', derived: { op: 'ratio', of: ['a', 'b'] } }, + ], + }), + ), + ).toEqual([]); + }); + + it('returns nothing for a stack with no datasets, and for an empty stack', () => { + expect(validateDatasetReferences({ objects })).toEqual([]); + expect(validateDatasetReferences({})).toEqual([]); + }); + + it('reads the name-keyed collection shape as well as the array shape', () => { + const findings = validateDatasetReferences({ + objects: { duly_task: { fields: { amount: { type: 'currency', label: 'Amount' } } } }, + datasets: { duly_health: { object: 'duly_task', dimensions: [{ name: 'x', field: 'amoun' }], measures: [] } }, + }); + expect(findings.map((f) => f.rule)).toEqual([DATASET_FIELD_UNKNOWN]); + expect(findings[0].where).toBe('dataset "duly_health" › dimension "x"'); + }); + + it('carries the common finding shape on every finding', () => { + const findings = validateDatasetReferences( + stackWith({ + include: ['dutee'], + dimensions: [{ name: 'd', field: 'nope' }], + measures: [{ name: 'm', aggregate: 'sum', field: 'nope2', filter: { nope3: 1 } }], + }), + ); + expect(findings.length).toBe(4); + for (const f of findings) { + expect(f.severity).toBe('error'); + expect(typeof f.rule).toBe('string'); + expect(f.where.startsWith('dataset "duly_health"')).toBe(true); + expect(f.path.startsWith('datasets[0]')).toBe(true); + expect(f.message.length).toBeGreaterThan(0); + expect(f.hint.length).toBeGreaterThan(0); + } + }); +}); + +// ── The shipped corpus stays clean ─────────────────────────────────────────── + +describe('validateDatasetReferences — the shipped dataset shapes', () => { + it('is silent on the app-crm opportunity dataset, verbatim', () => { + expect( + validateDatasetReferences({ + objects: [ + { + name: 'crm_opportunity', + fields: { + stage: { type: 'select', label: 'Stage' }, + account: { type: 'lookup', label: 'Account', reference: 'crm_account' }, + close_date: { type: 'date', label: 'Close Date' }, + amount: { type: 'currency', label: 'Amount' }, + }, + }, + ], + datasets: [ + { + name: 'opportunity_metrics', + object: 'crm_opportunity', + dimensions: [ + { name: 'stage', field: 'stage', type: 'string' }, + { name: 'account', field: 'account', type: 'lookup' }, + { name: 'close_date', field: 'close_date', type: 'date', dateGranularity: 'month' }, + ], + measures: [ + { name: 'opp_count', aggregate: 'count' }, + { name: 'total_amount', aggregate: 'sum', field: 'amount' }, + { name: 'avg_amount', aggregate: 'avg', field: 'amount' }, + ], + }, + ], + }), + ).toEqual([]); + }); + + it('is silent on the showcase invoice dataset, whose measure filter is a real key', () => { + expect( + validateDatasetReferences({ + objects: [ + { + name: 'showcase_invoice', + fields: { + status: { type: 'select', label: 'Status' }, + region: { type: 'text', label: 'Region' }, + issued_on: { type: 'date', label: 'Issued' }, + account: { type: 'lookup', label: 'Account', reference: 'showcase_account' }, + total: { type: 'currency', label: 'Total' }, + }, + }, + ], + datasets: [ + { + name: 'showcase_invoice_metrics', + object: 'showcase_invoice', + dimensions: [ + { name: 'status', field: 'status' }, + { name: 'region', field: 'region' }, + { name: 'issued_on', field: 'issued_on', dateGranularity: 'month' }, + { name: 'account', field: 'account' }, + ], + measures: [ + { name: 'invoice_count', aggregate: 'count' }, + { name: 'subtotal_sum', aggregate: 'sum', field: 'total' }, + { name: 'paid_count', aggregate: 'count', filter: { status: 'paid' } }, + { name: 'paid_rate', derived: { op: 'ratio', of: ['paid_count', 'invoice_count'] } }, + ], + }, + ], + }), + ).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-dataset-references.ts b/packages/lint/src/validate-dataset-references.ts new file mode 100644 index 0000000000..02024959d5 --- /dev/null +++ b/packages/lint/src/validate-dataset-references.ts @@ -0,0 +1,449 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14105 — ADR-0021 dataset reference integrity] A dataset's `include` paths, + * `dimensions[].field` / `measures[].field` paths and filter KEYS must name + * something the object graph actually has. + * + * ## The state this rule ends + * + * Measured on `@objectstack/spec` 17.2.0 against a real app (three datasets + * over one object): every one of these mutations, applied on its own and + * confirmed on disk, left `objectstack validate` at **exit 0, "Validation + * passed"**, and `objectstack build` wrote the dangling dataset into + * `dist/objectstack.json`: + * + * | mutation | before | after | + * |:--------------------------------------------------|:-------|:------| + * | dimension `field` → a base field that does not exist | passed | `dataset-field-unknown` | + * | dimension `field` → a joined field that does not exist | passed | `dataset-field-unknown` | + * | measure `field` → a field that does not exist | passed | `dataset-field-unknown` | + * | measure filter KEY → a field that does not exist | passed | `dataset-filter-field-unknown` | + * | `include[]` → a relationship that does not exist | passed | `dataset-include-unknown` | + * | `object` → an object that does not exist | passed | `object-reference-unknown` (see below) | + * + * The controls in that measurement — a duplicate measure name (`superRefine`) + * and a bad date macro in a measure filter (`filter-token-unknown`) — BOTH + * failed the build, so the datasets were demonstrably in the validation path + * the whole time. That is what makes this a hole rather than an unread file, + * and it is the sharpest form of the argument: `filter-token-unknown` already + * stands at `datasets[1].measures[1].filter.last_update_at.$lt` and reasons + * about the VALUE. Nothing standing in that same position resolved the KEY, or + * the sibling `field` one level up. + * + * ## Why a dataset's dangling reference is worse than most + * + * `filter-token-unknown`'s own wording states the failure mode it exists to + * prevent: the value is *"sent to the data engine as a literal string, matches + * no record, and the surface renders empty."* A dangling field path produces + * the same outcome from the same node. + * + * A dataset is the semantic layer — dashboards and reports bind its dimensions + * and measures BY NAME (ADR-0021) — and the CONSUMER end of that binding is + * already guarded: `widget-dataset-unknown` / `widget-dimension-unknown` / + * `widget-measure-unknown` (#7529, shipped #8902) refuse a widget that names a + * dataset, dimension or measure that does not exist. So the surviving hole was + * the quiet one, one level down: every binding resolves, the board renders, and + * the charts are empty or subtly wrong because the dataset underneath addresses + * columns that do not exist. + * + * ## Severity: `error`, on every verdict here + * + * The bar this package states is "gate when no reading of the metadata behaves + * as written", and each verdict clears it. A dimension bound to a column that + * does not exist cannot group by anything; a measure bound to one cannot + * aggregate anything; a filter key that names nothing either widens the scope + * (the condition is dropped) or empties it (the engine compares a missing + * column) — and every one of those outcomes reaches a human as a chart that + * rendered successfully. This is the same call `dashboard-filter-field-unknown` + * (#3365) makes one layer up on the identical question, and the reason + * `validate-searchable-fields` gives for gating rather than advising: a + * consumer that SKIPS an unknown name and renders the rest may be warned about; + * a declaration whose whole purpose is to name a column must not ship naming + * nothing. + * + * ## What this rule deliberately does NOT own + * + * **`Dataset.object` — the base object itself.** Checked, and it lands in + * `validate-object-references.ts` instead, because that rule's charter IS + * object-name reference sites that are plain `z.string()` and therefore ship + * whatever the author typed. Putting it there buys the curated cross-package + * severity ladder (`PLATFORM_PROVIDED_OBJECT_NAMES`) rather than a second, + * naive "not in this stack ⇒ error" — which matters immediately: the platform's + * own `system.datasets.ts` declares five datasets over `sys_user`, + * `sys_organization`, `sys_session`, `sys_package_installation` and + * `sys_audit_log`, three of which live in packages a stack compiling + * plugin-auth alone cannot see. A local ladder would have reported all five. + * When the base object does not resolve, THIS rule skips the dataset entirely + * (skip 1 below) so one typo yields one finding, not five. + * + * **Dimension/measure NAMES and the derived-measure graph.** `DatasetSchema`'s + * own `superRefine` already refuses duplicates and a `derived.of` naming a + * measure the dataset does not declare, and `validateWidgetBindings` / + * `validateChartBindings` own the presentation end. Restating either here would + * double-report. + * + * **Aggregate coherence.** `measure-aggregate-incoherent` + * (`validate-widget-bindings.ts`) already asks whether the aggregate SUITS the + * field's type. This rule asks only whether the field EXISTS — and the two now + * compose, because a measure whose field resolves is exactly the input that + * check was already written to want. + * + * ## Skips — the same three every field-existence rule in this package takes + * + * Resolution is {@link resolveFieldPath}'s (`object-graph.ts`), and its + * `unknowable` verdicts are never reported (ADR-0072 D1: one dead finding and + * authors stop trusting the linter). They are: + * + * 1. an object this stack does not define — the `sys_*` datasets above; + * 2. an object that declares no readable field map — ADR-0015 `external` and + * datasource-introspected schemas; + * 3. a registry-injected system column, and any hop THROUGH one. The shipped + * `showcase_task_metrics` dimension `{ field: 'created_at' }` is skip 3's + * live case: a real runtime column that appears in no authored `fields`. + * + * ## The reusable seam + * + * Nothing here is dataset-specific except the positions walked. The two halves + * — "where are the field keys in this filter?" ({@link walkFilterFieldKeys}, + * `filter-walk.ts`) and "what does this dotted path resolve to?" + * ({@link resolveFieldPath}, `object-graph.ts`) — are written as shared + * mechanism with the judgement left to the caller, because the same two + * questions are asked at a dashboard widget's filter keys and `sortBy` (#14148) + * and at a list view's field positions (#14107). Those surfaces are NOT + * implemented here; the seam is what keeps them one mechanism reused rather + * than three independent rules drifting apart. + */ + +import { walkFilterFieldKeys } from './filter-walk.js'; +import { + RELATIONSHIP_FIELD_TYPES, + indexObjectGraph, + isUnjudgeable, + listNames, + resolveFieldPath, + suggestName, + type FieldPathVerdict, + type ObjectGraph, +} from './object-graph.js'; + +/** An `include[]` entry that does not name a joinable relationship path. */ +export const DATASET_INCLUDE_UNKNOWN = 'dataset-include-unknown'; +/** A `dimensions[].field` / `measures[].field` path that resolves to no column. */ +export const DATASET_FIELD_UNKNOWN = 'dataset-field-unknown'; +/** A field path whose relationship prefix was never declared in `include`. */ +export const DATASET_FIELD_NOT_INCLUDED = 'dataset-field-not-included'; +/** A filter KEY — on the dataset or a measure — that resolves to no column. */ +export const DATASET_FILTER_FIELD_UNKNOWN = 'dataset-filter-field-unknown'; + +export type DatasetRefSeverity = 'error' | 'warning'; + +export interface DatasetRefFinding { + /** Always `error` today — see the severity note on this module. */ + severity: DatasetRefSeverity; + /** Diagnostic rule id. */ + rule: string; + /** Human-readable location, e.g. `dataset "sales" › dimension "region"`. */ + where: string; + /** Config path, e.g. `datasets[0].dimensions[2].field`. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +type AnyRec = Record; + +function isRec(v: unknown): v is AnyRec { + return !!v && typeof v === 'object' && !Array.isArray(v); +} + +function strName(v: unknown): string | undefined { + return typeof v === 'string' && v.length > 0 ? v : undefined; +} + +/** Coerce a collection (array or name-keyed map) to an array of records. */ +function asArray(v: unknown): AnyRec[] { + if (Array.isArray(v)) return v as AnyRec[]; + if (v && typeof v === 'object') { + return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) })); + } + return []; +} + +/** + * The relationship prefixes a dataset declared as joinable. + * + * ADR-0021: *"Declaring `a.b` implicitly includes the intermediate `a`."* So + * every PREFIX of every declared path is joinable, not only the paths as + * written — which is why this expands rather than reading `include` verbatim. + */ +function joinablePrefixes(include: unknown): ReadonlySet { + const prefixes = new Set(); + if (!Array.isArray(include)) return prefixes; + for (const entry of include) { + if (typeof entry !== 'string' || !entry) continue; + const segments = entry.split('.'); + for (let i = 1; i <= segments.length; i++) { + prefixes.add(segments.slice(0, i).join('.')); + } + } + return prefixes; +} + +/** + * Turn a resolution verdict into the message half of an existence finding, or + * `undefined` when the verdict is one no rule may report. + * + * Shared by the three positions that resolve a field PATH (dimension, measure, + * filter key) so they cannot drift into three different accounts of the same + * miss. The caller supplies `subject` — how the position is named in prose — + * and owns the rule id, the path and the hint's prescription. + */ +function existenceMessage( + verdict: FieldPathVerdict, + path: string, + subject: string, +): { message: string; detail: string } | undefined { + switch (verdict.kind) { + case 'ok': + case 'unknowable': + case 'hop-untargeted': + return undefined; + case 'hop-unknown': + return { + message: + `${subject} "${path}" traverses "${verdict.segment}", which is not a field on object ` + + `"${verdict.object}".${suggestName(verdict.segment, verdict.candidates)}`, + detail: `Fields on "${verdict.object}": ${listNames(verdict.candidates)}.`, + }; + case 'hop-not-relationship': + return { + message: + `${subject} "${path}" traverses "${verdict.segment}", which is a` + + `${verdict.type ? ` \`${verdict.type}\`` : 'n ordinary'} field on object ` + + `"${verdict.object}" and not a relationship — there is nothing to join through.`, + detail: + `Only ${[...RELATIONSHIP_FIELD_TYPES].sort().join(' / ')} fields are traversable ` + + `(ADR-0021 derives every join from the object graph; you never write an ON clause).`, + }; + case 'field-unknown': + return { + message: + `${subject} "${path}" is not a field on object "${verdict.object}".` + + `${suggestName(verdict.field, verdict.candidates)}`, + detail: `Fields on "${verdict.object}": ${listNames(verdict.candidates)}.`, + }; + } +} + +/** The shared consequence sentence — why an unresolved path is not merely inert. */ +const SILENT_EMPTY = + 'The path is compiled into the analytics query as written, so it addresses a column ' + + 'that does not exist: the surface renders successfully with empty or wrong numbers, ' + + 'and nothing reports the miss.'; + +/** + * Validate every ADR-0021 dataset's references against the object graph. + * Returns findings (empty = clean). Pure `(stack) => Finding[]`; no I/O, and + * safe on both the schema-parsed stack and the raw config the `lint` path + * carries. + */ +export function validateDatasetReferences(stack: AnyRec): DatasetRefFinding[] { + const findings: DatasetRefFinding[] = []; + if (!isRec(stack)) return findings; + + const datasets = asArray(stack.datasets); + if (datasets.length === 0) return findings; + + const graph: ObjectGraph = indexObjectGraph(stack); + + datasets.forEach((ds, di) => { + const dsName = strName(ds.name) ?? `#${di}`; + const where = `dataset "${dsName}"`; + const dsPath = `datasets[${di}]`; + const object = strName(ds.object); + if (!object) return; // no base binding — `DatasetSchema.object` is required; the schema owns it + + // ── Skip 1/2, once for the whole dataset ── + // An unresolvable base object is `validate-object-references.ts`' finding + // (see the module note). Resolving anything against it here would turn one + // typo into a finding per dimension, per measure and per filter key. + if (!graph.has(object) || !graph.get(object)) return; + + const included = joinablePrefixes(ds.include); + + // ── (1) `include[]` names a real, traversable relationship ── + const include = Array.isArray(ds.include) ? ds.include : []; + include.forEach((entry, ii) => { + if (typeof entry !== 'string' || !entry) return; + const path = `${dsPath}.include[${ii}]`; + const verdict = resolveFieldPath(graph, object, entry); + if (isUnjudgeable(verdict) || !verdict) return; + + const prescription = + `Declare a relationship (lookup / master_detail) that exists on the join chain from ` + + `"${object}", or drop the entry. ADR-0071 allows up to 3 hops and declaring "a.b" ` + + `implicitly includes "a".`; + + 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; + const type = verdict.meta?.type; + if (type && RELATIONSHIP_FIELD_TYPES.has(type)) return; + findings.push({ + severity: 'error', + rule: DATASET_INCLUDE_UNKNOWN, + where, + path, + message: + `include[${ii}] "${entry}" names a` + + `${type ? ` \`${type}\`` : 'n ordinary'} field on object "${verdict.object}", ` + + `not a relationship — no join can be derived from it, so every dimension or ` + + `measure written against that prefix addresses nothing.`, + hint: prescription, + }); + return; + } + + const account = existenceMessage(verdict, entry, `include[${ii}]`); + if (!account) return; + findings.push({ + severity: 'error', + rule: DATASET_INCLUDE_UNKNOWN, + where, + path, + message: + `${account.message} Joins are COMPILED from \`include\` (ADR-0021), so an entry ` + + `that resolves to nothing produces no join at all.`, + hint: `${prescription} ${account.detail}`, + }); + }); + + /** + * Judge one authored field PATH — the shared core behind a dimension's + * `field`, a measure's `field` and every filter key. Existence first (it + * carries the "did you mean"), then the ADR-0021 joinability clause, and at + * most ONE finding per position either way. + */ + const checkFieldPath = ( + raw: unknown, + positionWhere: string, + path: string, + subject: string, + rule: string, + prescription: string, + ): void => { + const written = strName(raw); + if (!written) return; + const verdict = resolveFieldPath(graph, object, written); + if (isUnjudgeable(verdict) || !verdict) return; + + const account = existenceMessage(verdict, written, subject); + if (account) { + findings.push({ + severity: 'error', + rule, + where: positionWhere, + path, + message: `${account.message} ${SILENT_EMPTY}`, + hint: `${prescription} ${account.detail}`, + }); + return; + } + + // The path RESOLVES. The second real check: a dotted path is only + // joinable when its relationship prefix was declared in `include` — + // ADR-0021 D-C, "only declared paths are joinable". A path whose prefix + // was never declared compiles to no join, so the column is not in the + // query's reach however real it is. + const cut = written.lastIndexOf('.'); + if (cut < 0) return; // a base field needs no join + const prefix = written.slice(0, cut); + if (included.has(prefix)) return; + findings.push({ + severity: 'error', + rule: DATASET_FIELD_NOT_INCLUDED, + where: positionWhere, + path, + message: + `${subject} "${written}" resolves on the object graph, but its relationship ` + + `prefix "${prefix}" is not declared in this dataset's \`include\` — and ADR-0021 ` + + `joins ONLY declared paths, so no join is compiled and the column is out of the ` + + `query's reach. ${SILENT_EMPTY}`, + hint: + `Add "${prefix}" to include (declaring "a.b" implicitly includes "a"), or bind ` + + `this position to a field on "${object}" itself. Declared include paths: ` + + `${included.size > 0 ? [...included].sort().join(', ') : '(none)'}.`, + }); + }; + + // ── (2) `dimensions[].field` ── + asArray(ds.dimensions).forEach((dim, i) => { + const name = strName(dim.name) ?? `#${i}`; + checkFieldPath( + dim.field, + `${where} › dimension "${name}"`, + `${dsPath}.dimensions[${i}].field`, + 'dimension field', + DATASET_FIELD_UNKNOWN, + `Bind the dimension to a field on "${object}", or to a ` + + `\`relationship[.relationship].field\` path whose prefix is declared in \`include\`.`, + ); + }); + + // ── (3) `measures[].field` ── + // A derived measure combines OTHER measures by name and declares no field; + // `DatasetSchema.superRefine` owns that graph, and `field` is legitimately + // absent on a plain `count`. Both fall out of `checkFieldPath`'s own + // "nothing written, nothing to resolve" guard. + asArray(ds.measures).forEach((measure, i) => { + const name = strName(measure.name) ?? `#${i}`; + checkFieldPath( + measure.field, + `${where} › measure "${name}"`, + `${dsPath}.measures[${i}].field`, + 'measure field', + DATASET_FIELD_UNKNOWN, + `Bind the measure to a field on "${object}", or to a ` + + `\`relationship[.relationship].field\` path whose prefix is declared in \`include\`.`, + ); + }); + + // ── (4) filter KEYS, on the dataset scope filter and on every measure ── + // The traversal `filter-token-unknown` already performs on the VALUES, + // asked of the KEYS — the same node, the same walk, the question that was + // missing. `walkFilterFieldKeys` handles all three authored filter shapes + // (Mongo condition object, `{ field, operator, value }` rules, and + // `[field, op, value]` triples) so a filter authored one way is not judged + // while another is silently skipped (#3574's own failure mode). + const checkFilter = (filter: unknown, positionWhere: string, path: string): void => { + if (filter === undefined || filter === null) return; + walkFilterFieldKeys(filter, path, ({ field, path: at }) => { + checkFieldPath( + field, + positionWhere, + at, + 'filter key', + DATASET_FILTER_FIELD_UNKNOWN, + `Filter on a field that exists on "${object}" (or on a declared \`include\` path).`, + ); + }); + }; + + checkFilter(ds.filter, `${where} › filter`, `${dsPath}.filter`); + asArray(ds.measures).forEach((measure, i) => { + const name = strName(measure.name) ?? `#${i}`; + checkFilter( + measure.filter, + `${where} › measure "${name}" › filter`, + `${dsPath}.measures[${i}].filter`, + ); + }); + }); + + return findings; +} diff --git a/packages/lint/src/validate-object-references.test.ts b/packages/lint/src/validate-object-references.test.ts index f71994ef2c..c453a68189 100644 --- a/packages/lint/src/validate-object-references.test.ts +++ b/packages/lint/src/validate-object-references.test.ts @@ -110,6 +110,73 @@ describe('validateObjectReferences — dashboard global filters', () => { }); }); +describe('validateObjectReferences — ADR-0021 dataset base object (#14105)', () => { + // The measured row 6: `object: 'duly_tsk'` where the object is `duly_task`. + // On published 17.2.0 this exited 0 with "Validation passed", and `build` + // wrote the dangling dataset into `dist/objectstack.json`. + it('errors on a dataset over an object that does not exist', () => { + const findings = validateObjectReferences({ + ...baseStack(), + datasets: [ + { + name: 'lead_metrics', + object: 'crm_led', + dimensions: [{ name: 'status', field: 'status' }], + measures: [{ name: 'n', aggregate: 'count' }], + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN); + expect(findings[0].path).toBe('datasets[0].object'); + expect(findings[0].where).toBe('dataset "lead_metrics"'); + expect(findings[0].message).toContain('Did you mean "crm_lead"?'); + }); + + it('accepts a dataset over an own object', () => { + expect( + validateObjectReferences({ + ...baseStack(), + datasets: [{ name: 'lead_metrics', object: 'crm_lead', dimensions: [], measures: [] }], + }), + ).toEqual([]); + }); + + it('accepts the platform datasets this stack cannot see (rung ③)', () => { + // `system.datasets.ts` ships five of these. A local "not in this stack ⇒ + // error" check would have reported every one; the curated registry is the + // whole reason this site lives on this rule. + expect( + validateObjectReferences({ + ...baseStack(), + datasets: [ + { name: 'sys_user_metrics', object: 'sys_user', dimensions: [], measures: [] }, + { name: 'sys_organization_metrics', object: 'sys_organization', dimensions: [], measures: [] }, + { name: 'sys_session_metrics', object: 'sys_session', dimensions: [], measures: [] }, + { name: 'sys_audit_log_metrics', object: 'sys_audit_log', dimensions: [], measures: [] }, + { + name: 'sys_package_installation_metrics', + object: 'sys_package_installation', + dimensions: [], + measures: [], + }, + ], + }), + ).toEqual([]); + }); + + it('warns rather than errors on a platform-shaped base object nothing registers', () => { + const findings = validateObjectReferences({ + ...baseStack(), + datasets: [{ name: 'm', object: 'sys_approval_process', dimensions: [], measures: [] }], + }); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNREGISTERED_PLATFORM); + }); +}); + describe('validateObjectReferences — the severity ladder', () => { // The `sys_approval_process` case: platform-shaped but registered by nothing. // ADR-0019 removed the process object when approval became a flow node. diff --git a/packages/lint/src/validate-object-references.ts b/packages/lint/src/validate-object-references.ts index 18cad9236e..b377729e7e 100644 --- a/packages/lint/src/validate-object-references.ts +++ b/packages/lint/src/validate-object-references.ts @@ -23,6 +23,13 @@ * id text input. * - dashboard `globalFilters[].optionsFrom.object` — the object a filter * dropdown fetches its options from. Dead → an always-empty dropdown. + * - ADR-0021 `datasets[].object` (#14105) — the FROM of the semantic layer. + * Dead → every report and dashboard widget bound to that dataset queries an + * object that does not exist, and `validate`/`build` both exited 0 before + * this site existed. Its field-level siblings (`include`, + * `dimensions[].field`, `measures[].field`, filter keys) live in + * `validate-dataset-references.ts`, which skips a dataset whose base object + * lands here so one typo yields one finding. * - navigation `requiresObject` / `requiresService` capability gates. This is * the escape hatch `stack.zod.ts` honours to SKIP nav validation, so a typo * here is doubly silent: the entry is hidden forever (the runtime never @@ -294,6 +301,31 @@ export function validateObjectReferences(stack: AnyRec): ObjectRefFinding[] { } } + // ── ADR-0021 datasets → the base object (#14105) ── + // `DatasetSchema.object` is `z.string()`, so nothing resolved it: a dataset + // over an object that does not exist passed `objectstack validate` at exit 0 + // and `build` wrote it into `dist/objectstack.json` (measured on 17.2.0). It + // belongs on THIS rule rather than beside the dataset field checks + // (`validate-dataset-references.ts`) because the reference is an object NAME, + // and the severity ladder above is the whole reason: the platform's own + // `system.datasets.ts` declares five datasets over `sys_*` objects, three of + // which live in packages a stack compiling plugin-auth alone cannot see. All + // five resolve through `PLATFORM_PROVIDED_OBJECT_NAMES` (rung ③); a local + // "not in this stack ⇒ error" check would have reported every one of them. + const datasets = asArray(stack.datasets); + for (let dsi = 0; dsi < datasets.length; dsi++) { + const ds = datasets[dsi]; + if (!ds || typeof ds !== 'object') continue; + check( + strName(ds.object), + `dataset "${strName(ds.name) ?? `#${dsi}`}"`, + `datasets[${dsi}].object`, + 'dataset base object', + 'A dataset is the FROM of every report and dashboard widget bound to it (ADR-0021), so ' + + 'an unknown base object leaves every one of those surfaces querying nothing.', + ); + } + // ── App navigation → requiresObject gates (and gated objectName) ── const apps = asArray(stack.apps); for (let ai = 0; ai < apps.length; ai++) {