diff --git a/.changeset/page-field-and-chart-binding-lint.md b/.changeset/page-field-and-chart-binding-lint.md new file mode 100644 index 0000000000..8d5281da06 --- /dev/null +++ b/.changeset/page-field-and-chart-binding-lint.md @@ -0,0 +1,40 @@ +--- +'@objectstack/lint': minor +'@objectstack/cli': minor +--- + +Page-component field bindings and non-dashboard chart bindings (issue #3583, Phase 2) + +Two more reference-integrity rules from the #3583 assessment, both wired into +`os validate`, `os lint`, and `os compile`. + +**`validate-page-field-bindings`** — `PageComponent.properties` is an untyped +bag, so a highlights strip, KPI card, or details section can name a field the +bound object does not have; the component silently skips it. Which object a +component binds follows `dataSource.object` → `properties.object` → the page's +`object`, so multi-object pages are checked per element. `record:related_list` +resolves its columns/sort/filter against the **related** object and its +add-picker against that picker's own object. Advisory (matching +`FORM_FIELD_UNKNOWN`). Relationship paths, system fields, cross-package objects, +and unregistered component types are skipped. + +**`validate-chart-bindings`** — extends ADR-0021 axis checking past dashboards to +report charts (`report.chart` and `report.blocks[].chart`), list-view charts +(`views[].list`, `views[].listViews.*`, `objects[].listViews.*`), and +dataset-bound page chart components. An axis naming a raw field instead of a +declared measure is an **error** (the series comes back empty); an axis naming a +declared-but-unselected measure is a **warning**. The report shape needed its own +handling: `ReportChartSchema` narrows `xAxis`/`yAxis` to bare strings, which the +dashboard rule's array guard skips silently. The react `` block is +object-bound, not dataset-bound, and is deliberately left out — nothing defines +what its aggregate names the result column. + +**Fixes:** the page walk used by `validate-action-name-refs` read a top-level +`page.components` array, which `PageSchema` does not have — components live under +`regions[].components[]` and `slots`, and sub-trees nest inside the untyped +`properties` bag (`children`, `items[].children`, `body`, `footer`) rather than a +`children` key on the component. The rule was therefore visiting nothing on a +schema-parsed stack. Traversal now lives in one shared, tested module; on the +showcase app it reaches 194 components where the previous shape found 46. +Source-authored pages (`kind: 'html' | 'react' | 'jsx'`) are skipped — their +`regions` hold a derived cache the `source` wins over. diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index 6839f2ca12..a352d32343 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -116,6 +116,49 @@ Interpolated targets (`${…}`, `{…}`) are skipped — they resolve at render Action names get no third-party softening: the runtime ships no built-in action names, so a name resolving nowhere is always an **error**. +### 5. Page components bound to fields that don't exist + +A page component's `properties` is an untyped bag, so a highlights strip, a KPI +card, or a details section can name a field the bound object does not have. The +component silently skips it and the page renders one item short. + +```ts +// page.object = 'crm_lead' +{ type: 'record:highlights', properties: { fields: ['status', 'total_revenue'] } } +// ↑ not a crm_lead field → warning +``` + +Which object a component binds follows `dataSource.object` → `properties.object` +→ the page's `object`, so a multi-object page is checked per element rather than +against one page-wide guess. A `record:related_list`'s `columns`/`sort`/`filter` +resolve against its **related** object (`objectName`), and its add-picker against +its own. Advisory, like form-layout field references — every consumer degrades +rather than failing. + +Skipped, to keep false positives at zero: relationship paths (`account.name`, +resolved by the query engine), registry-injected system fields (`created_at`, +`owner_id`, …), components bound to an object another package defines, and +unregistered component types. + +### 6. Chart axes naming raw fields instead of dataset measures + +Post-[ADR-0021](/docs/data-modeling/analytics) a chart's result rows are keyed by +the **dataset measure name**, not the underlying column — so an axis pointing at +the raw field renders with an empty series. Dashboard widgets were already +checked; report charts, list-view charts, and dataset-bound page chart components +are checked the same way. + +```ts +// dataset declares measure `est_hours` (sum of `estimate_hours`) +chart: { type: 'bar', xAxis: 'status', yAxis: 'estimate_hours' } +// ↑ the base column, not the measure → error +``` + +An axis naming a measure the dataset declares but this chart does not *select* +(not in `values`) is a **warning**: the query never returns it, so it plots +nothing. The react `` block is object-bound rather than +dataset-bound and is not checked here. + ## The one gate, two entry points `os validate` and `os build` (alias of `os compile`) run the **same** validator: @@ -127,6 +170,8 @@ names, so a name resolving nowhere is always an **error**. | Widget-binding integrity | ✓ | ✓ | | Dashboard action/route references (ADR-0049) | ✓ | ✓ | | Object & action name references (#3583) | ✓ | ✓ | +| Page-component field bindings (#3583) | ✓ | ✓ | +| Chart bindings outside dashboards (#3583) | ✓ | ✓ | | Security posture (ADR-0090 — e.g. every custom object declares `sharingModel`) | ✓ | ✓ | | Emits `dist/objectstack.json` | — | ✓ | diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index d49a1641b4..063c4de125 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -14,6 +14,7 @@ import { validateWidgetBindings } from '@objectstack/lint'; import { validateDashboardActionRefs } from '@objectstack/lint'; import { validateFilterTokens } from '@objectstack/lint'; import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint'; +import { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint'; import { validateResponsiveStyles } from '@objectstack/lint'; import { validateSecurityPosture, validateOrgAxisRedLines, buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint'; import { validateReadonlyFlowWrites } from '@objectstack/lint'; @@ -325,6 +326,10 @@ export default class Compile extends Command { // `objectOverride`, dashboard filter `optionsFrom.object`, nav // `requiresObject` gates, and the name-bound action surfaces // (`bulkActions`/`rowActions`, page quick-actions, nav action items). + // Plus page-component field bindings and the chart surfaces outside + // dashboards (report charts, list-view charts, dataset-bound page + // chart components) — same ADR-0021 semantic layer, where an axis + // naming a raw field instead of a measure renders an empty series. // All plain strings in the schema, so a name resolving to nothing // ships and fails silently. Errors fail the build; the // platform-prefixed-but-unregistered case is advisory (a third-party @@ -333,6 +338,8 @@ export default class Compile extends Command { const refFindings = [ ...validateObjectReferences(result.data as Record), ...validateActionNameRefs(result.data as Record), + ...validatePageFieldBindings(result.data as Record), + ...validateChartBindings(result.data as Record), ]; const refErrors = refFindings.filter((f) => f.severity === 'error'); const refWarnings = refFindings.filter((f) => f.severity === 'warning'); diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index 2dcc965d56..8e8d6c3edf 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -11,6 +11,7 @@ import { lintDataModel } from '../lint/data-model-rules.js'; import { validateWidgetBindings } from '@objectstack/lint'; import { validateRecordTitle, validateSemanticRoles, validateCapabilityReferences, validateSecurityPosture, validateOrgAxisRedLines, validateApprovalApprovers, validateSeedReplaySafety, validateSeedStateMachine } from '@objectstack/lint'; import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint'; +import { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint'; import { collectAndLintDocs } from '../utils/collect-docs.js'; import { scoreMetadata } from '../lint/score.js'; import { runMetadataEval } from '../lint/metadata-eval.js'; @@ -520,6 +521,36 @@ export function lintConfig(config: any): LintIssue[] { }); } + // ── Page component field bindings (issue #3583) ── + // `PageComponent.properties` is an untyped bag, so a highlights strip, KPI + // card, or details section can name a field the object does not have; the + // component silently skips it. Advisory, matching `FORM_FIELD_UNKNOWN` — + // every page consumer degrades rather than failing. + for (const t of validatePageFieldBindings(config)) { + issues.push({ + severity: t.severity, + rule: t.rule, + message: `${t.where}: ${t.message}`, + path: t.path, + fix: t.hint, + }); + } + + // ── Chart bindings outside dashboards (issue #3583) ── + // `validate-widget-bindings` covers dashboard widgets only. Report charts, + // list-view charts, and dataset-bound page chart components bind the same + // semantic layer, where an axis naming a raw field instead of a dataset + // measure renders an empty series (ADR-0021). + for (const t of validateChartBindings(config)) { + issues.push({ + severity: t.severity, + rule: t.rule, + message: `${t.where}: ${t.message}`, + path: t.path, + fix: t.hint, + }); + } + return issues; } diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 5381717603..bf96ff7d8c 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -15,6 +15,7 @@ import { validateWidgetBindings } from '@objectstack/lint'; import { validateDashboardActionRefs } from '@objectstack/lint'; import { validateFilterTokens } from '@objectstack/lint'; import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint'; +import { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint'; import { validateResponsiveStyles } from '@objectstack/lint'; import { validateJsxPages, validateReactPages, validateReactPageProps, validatePageSourceStyling } from '@objectstack/lint'; import { validateCapabilityReferences } from '@objectstack/lint'; @@ -287,6 +288,10 @@ export default class Validate extends Command { // `reference`/`objectOverride`, dashboard filter `optionsFrom.object`, // nav `requiresObject` gates, and the name-bound action surfaces // (`bulkActions`/`rowActions`, page quick-actions, nav action items). + // Plus page-component field bindings and the chart surfaces outside + // dashboards (report charts, list-view charts, dataset-bound page + // chart components) — same ADR-0021 semantic layer, where an axis + // naming a raw field instead of a measure renders an empty series. // All are plain strings in the schema, so a name resolving to nothing // parses, ships, and fails silently at runtime. An unprefixed miss is // a typo (error); a platform-prefixed name no known package registers @@ -295,6 +300,8 @@ export default class Validate extends Command { const refFindings = [ ...validateObjectReferences(result.data as Record), ...validateActionNameRefs(result.data as Record), + ...validatePageFieldBindings(result.data as Record), + ...validateChartBindings(result.data as Record), ]; const refErrors = refFindings.filter((f) => f.severity === 'error'); const refWarnings = refFindings.filter((f) => f.severity === 'warning'); diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 8f7c025eef..ef4fc71a60 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -191,4 +191,16 @@ export type { ObjectRefFinding, ObjectRefSeverity } from './validate-object-refe export { validateActionNameRefs, ACTION_NAME_UNDEFINED } from './validate-action-name-refs.js'; export type { ActionNameRefFinding, ActionNameRefSeverity } from './validate-action-name-refs.js'; +export { validatePageFieldBindings, PAGE_FIELD_UNKNOWN } from './validate-page-field-bindings.js'; +export type { PageFieldFinding, PageFieldSeverity } from './validate-page-field-bindings.js'; + +export { + validateChartBindings, + CHART_DIMENSION_UNKNOWN, + CHART_MEASURE_UNKNOWN, + CHART_DATASET_UNKNOWN, + CHART_AXIS_NOT_SELECTED, +} from './validate-chart-bindings.js'; +export type { ChartBindingFinding, ChartBindingSeverity } from './validate-chart-bindings.js'; + export { buildAccessMatrix, diffAccessMatrix } from './build-access-matrix.js'; diff --git a/packages/lint/src/page-walk.test.ts b/packages/lint/src/page-walk.test.ts new file mode 100644 index 0000000000..3bccaad8c3 --- /dev/null +++ b/packages/lint/src/page-walk.test.ts @@ -0,0 +1,158 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { walkPageComponents, isSourceAuthoredPage } from './page-walk.js'; + +const paths = (page: Record) => + walkPageComponents(page, 'pages[0]').map((w) => w.path); + +describe('walkPageComponents — where components actually live', () => { + it('walks regions[].components[]', () => { + expect( + paths({ + regions: [ + { name: 'main', components: [{ type: 'a' }, { type: 'b' }] }, + { name: 'side', components: [{ type: 'c' }] }, + ], + }), + ).toEqual([ + 'pages[0].regions[0].components[0]', + 'pages[0].regions[0].components[1]', + 'pages[0].regions[1].components[0]', + ]); + }); + + it('walks slots, normalizing a single component to a list', () => { + expect( + paths({ + kind: 'slotted', + slots: { + highlights: { type: 'a' }, + tabs: [{ type: 'b' }, { type: 'c' }], + }, + }), + ).toEqual([ + 'pages[0].slots.highlights', + 'pages[0].slots.tabs[0]', + 'pages[0].slots.tabs[1]', + ]); + }); + + it('finds nothing in a top-level `components` array — PageSchema has none', () => { + // Guards the bug this module exists to prevent: a walker reading + // `page.components` visits nothing on a real stack while looking correct. + expect(paths({ components: [{ type: 'a' }] })).toEqual([]); + }); + + it('recurses through properties.children — the layout-container shape', () => { + expect( + paths({ + regions: [ + { + name: 'main', + components: [ + { + type: 'flex', + properties: { + children: [ + { type: 'flex', properties: { children: [{ type: 'leaf' }] } }, + ], + }, + }, + ], + }, + ], + }), + ).toEqual([ + 'pages[0].regions[0].components[0]', + 'pages[0].regions[0].components[0].properties.children[0]', + 'pages[0].regions[0].components[0].properties.children[0].properties.children[0]', + ]); + }); + + it('recurses through properties.items[].children, body and footer', () => { + expect( + paths({ + regions: [ + { + name: 'main', + components: [ + { type: 'page:tabs', properties: { items: [{ children: [{ type: 'x' }] }] } }, + { type: 'page:card', properties: { body: [{ type: 'y' }], footer: [{ type: 'z' }] } }, + ], + }, + ], + }), + ).toEqual([ + 'pages[0].regions[0].components[0]', + 'pages[0].regions[0].components[0].properties.items[0].children[0]', + 'pages[0].regions[0].components[1]', + 'pages[0].regions[0].components[1].properties.body[0]', + 'pages[0].regions[0].components[1].properties.footer[0]', + ]); + }); +}); + +describe('walkPageComponents — object binding precedence', () => { + const bindings = (page: Record) => + walkPageComponents(page, 'pages[0]').map((w) => w.objectName); + + it('inherits the page object, and lets dataSource then properties override', () => { + expect( + bindings({ + object: 'page_obj', + regions: [ + { + name: 'main', + components: [ + { type: 'a' }, + { type: 'b', dataSource: { object: 'ds_obj' } }, + { type: 'c', properties: { object: 'prop_obj' } }, + // dataSource wins over properties. + { type: 'd', dataSource: { object: 'ds_obj' }, properties: { object: 'prop_obj' } }, + ], + }, + ], + }), + ).toEqual(['page_obj', 'ds_obj', 'prop_obj', 'ds_obj']); + }); + + it('propagates an overridden binding down to nested children', () => { + const walked = walkPageComponents( + { + object: 'page_obj', + regions: [ + { + name: 'main', + components: [ + { + type: 'flex', + dataSource: { object: 'ds_obj' }, + properties: { children: [{ type: 'leaf' }] }, + }, + ], + }, + ], + }, + 'pages[0]', + ); + expect(walked.map((w) => w.objectName)).toEqual(['ds_obj', 'ds_obj']); + }); +}); + +describe('isSourceAuthoredPage', () => { + it('treats html/react/jsx as source-authored and skips their regions', () => { + for (const kind of ['html', 'react', 'jsx']) { + expect(isSourceAuthoredPage({ kind })).toBe(true); + expect( + paths({ kind, regions: [{ name: 'main', components: [{ type: 'a' }] }] }), + ).toEqual([]); + } + }); + + it('treats full/slotted (and an absent kind) as authored metadata', () => { + expect(isSourceAuthoredPage({ kind: 'full' })).toBe(false); + expect(isSourceAuthoredPage({ kind: 'slotted' })).toBe(false); + expect(isSourceAuthoredPage({})).toBe(false); + }); +}); diff --git a/packages/lint/src/page-walk.ts b/packages/lint/src/page-walk.ts new file mode 100644 index 0000000000..58dd0ab5f8 --- /dev/null +++ b/packages/lint/src/page-walk.ts @@ -0,0 +1,139 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Shared page-component traversal for the lint rules that inspect + * `PageComponent.properties` (issue #3583). + * + * Getting this walk right is subtle enough that duplicating it has already + * produced one dead rule, so it lives here once: + * + * - Components hang off `page.regions[].components[]` and `page.slots.` + * — there is NO top-level `page.components`. A walker that reads + * `page.components` silently visits nothing on a schema-parsed stack. + * - `slots.` is `PageComponent | PageComponent[]` — a single component + * is legal and must be normalized. + * - `PageComponentSchema` is `.strict()`, so a component carries no `children` + * key of its own. Sub-trees live INSIDE the untyped `properties` bag: + * `page:tabs` → `properties.items[].children`, `page:accordion` → + * `properties.items[].children`, `page:card` → `properties.body` / + * `properties.footer`. All are `z.array(z.unknown())`, so the recursion is + * untyped and has to be done by hand. + * - `kind: 'html' | 'react' | 'jsx'` pages are authored as `source`, which is + * authoritative; their `regions` hold at most a DERIVED cache that the + * source wins over. Linting that cache reports findings about metadata the + * author never wrote, so those pages are skipped here and covered by + * `validate-jsx-pages` / `validate-react-page-props` instead. + */ + +export type AnyRec = Record; + +/** A visited component plus everything needed to locate and bind it. */ +export interface WalkedComponent { + /** The component record itself. */ + component: AnyRec; + /** Config path, e.g. `pages[0].regions[1].components[2]`. */ + path: string; + /** + * The object this component binds against, by precedence: + * `dataSource.object` → `properties.object` → the page's `object`. + * `undefined` when nothing in the chain names one. + */ + objectName?: string; +} + +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; +} + +/** Page kinds whose component tree is a derived cache, not authored metadata. */ +const SOURCE_AUTHORED_KINDS = new Set(['html', 'react', 'jsx']); + +/** Is this page authored as `source` (so its `regions` must not be linted)? */ +export function isSourceAuthoredPage(page: AnyRec): boolean { + const kind = strName(page.kind); + return kind !== undefined && SOURCE_AUTHORED_KINDS.has(kind); +} + +/** + * Walk every component on a page, depth-first, yielding each with its config + * path and resolved object binding. Source-authored pages yield nothing. + * + * `pagePath` is the caller's path prefix for the page (e.g. `pages[3]`). + */ +export function walkPageComponents(page: AnyRec, pagePath: string): WalkedComponent[] { + const out: WalkedComponent[] = []; + if (!isRec(page) || isSourceAuthoredPage(page)) return out; + + const pageObject = strName(page.object); + + const visit = (node: unknown, path: string, inheritedObject?: string) => { + if (!isRec(node)) return; + + // Per-element `dataSource` overrides the page object so one page can bind + // several objects; an inline `properties.object` does the same for the + // element-family components that declare one. + const props = isRec(node.properties) ? node.properties : undefined; + const dataSource = isRec(node.dataSource) ? node.dataSource : undefined; + const objectName = + strName(dataSource?.object) ?? strName(props?.object) ?? inheritedObject; + + out.push({ component: node, path, objectName }); + + if (!props) return; + + // `page:tabs` / `page:accordion` — items[].children[] + if (Array.isArray(props.items)) { + for (let i = 0; i < props.items.length; i++) { + const item = props.items[i]; + if (!isRec(item) || !Array.isArray(item.children)) continue; + for (let c = 0; c < item.children.length; c++) { + visit(item.children[c], `${path}.properties.items[${i}].children[${c}]`, objectName); + } + } + } + // Generic layout nesting — `properties.children[]`. Not in any props + // schema, but it is how real pages compose layout containers (`type: + // 'flex'` grids in the showcase command-center wrap every chart this way). + // Omitting it hides whole sub-trees from every rule built on this walk. + if (Array.isArray(props.children)) { + for (let i = 0; i < props.children.length; i++) { + visit(props.children[i], `${path}.properties.children[${i}]`, objectName); + } + } + // `page:card` — body[] / footer[] + for (const key of ['body', 'footer'] as const) { + const slotList = props[key]; + if (!Array.isArray(slotList)) continue; + for (let i = 0; i < slotList.length; i++) { + visit(slotList[i], `${path}.properties.${key}[${i}]`, objectName); + } + } + }; + + const regions = Array.isArray(page.regions) ? page.regions : []; + for (let r = 0; r < regions.length; r++) { + const region = regions[r]; + if (!isRec(region) || !Array.isArray(region.components)) continue; + for (let c = 0; c < region.components.length; c++) { + visit(region.components[c], `${pagePath}.regions[${r}].components[${c}]`, pageObject); + } + } + + const slots = isRec(page.slots) ? page.slots : undefined; + if (slots) { + for (const [slot, value] of Object.entries(slots)) { + // A slot holds a single component or an array of them. + const list = Array.isArray(value) ? value : [value]; + const indexed = Array.isArray(value); + for (let i = 0; i < list.length; i++) { + visit(list[i], `${pagePath}.slots.${slot}${indexed ? `[${i}]` : ''}`, pageObject); + } + } + } + + return out; +} diff --git a/packages/lint/src/validate-action-name-refs.test.ts b/packages/lint/src/validate-action-name-refs.test.ts index f957ea71c0..ed54fdb001 100644 --- a/packages/lint/src/validate-action-name-refs.test.ts +++ b/packages/lint/src/validate-action-name-refs.test.ts @@ -84,43 +84,107 @@ describe('validateActionNameRefs — list view bulk/row actions', () => { }); }); +// These fixtures use the REAL page shape. An earlier version of this suite +// invented a top-level `page.components` array with `children` nesting — a +// shape `PageSchema` does not have (components live under +// `regions[].components[]` / `slots`, and `PageComponentSchema` is `.strict()` +// so it carries no `children`). The rule passed those tests while visiting +// nothing at all on a real stack. describe('validateActionNameRefs — page quick actions', () => { - it('errors on an undefined actionNames entry', () => { + it('errors on an undefined actionNames entry in a region', () => { const findings = validateActionNameRefs({ ...withActions(), pages: [ { name: 'lead_record', - components: [ + regions: [ { - type: 'record:quick_actions', - properties: { location: 'record_section', actionNames: ['showcase_mark_done'] }, + name: 'main', + components: [ + { + type: 'record:quick_actions', + properties: { location: 'record_section', actionNames: ['showcase_mark_done'] }, + }, + ], }, ], }, ], }); expect(findings).toHaveLength(1); - expect(findings[0].path).toBe('pages[0].components[0].properties.actionNames[0]'); + expect(findings[0].path).toBe('pages[0].regions[0].components[0].properties.actionNames[0]'); + }); + + it('walks a slotted page', () => { + const findings = validateActionNameRefs({ + ...withActions(), + pages: [ + { + name: 'p', + kind: 'slotted', + slots: { + actions: { type: 'record:quick_actions', properties: { actionNames: ['nope'] } }, + }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('pages[0].slots.actions.properties.actionNames[0]'); }); - it('walks nested component children', () => { + it('recurses into tab children nested in the properties bag', () => { const findings = validateActionNameRefs({ ...withActions(), pages: [ { name: 'p', - components: [ + regions: [ { - type: 'layout:columns', - children: [{ type: 'record:quick_actions', properties: { actionNames: ['nope'] } }], + name: 'main', + components: [ + { + type: 'page:tabs', + properties: { + items: [ + { + label: 'Overview', + children: [ + { type: 'record:quick_actions', properties: { actionNames: ['nope'] } }, + ], + }, + ], + }, + }, + ], }, ], }, ], }); expect(findings).toHaveLength(1); - expect(findings[0].path).toBe('pages[0].components[0].children[0].properties.actionNames[0]'); + expect(findings[0].path).toBe( + 'pages[0].regions[0].components[0].properties.items[0].children[0].properties.actionNames[0]', + ); + }); + + it('skips a source-authored page (its regions are a derived cache)', () => { + const findings = validateActionNameRefs({ + ...withActions(), + pages: [ + { + name: 'p', + kind: 'jsx', + source: '
', + regions: [ + { + name: 'main', + components: [{ type: 'record:quick_actions', properties: { actionNames: ['nope'] } }], + }, + ], + }, + ], + }); + expect(findings).toEqual([]); }); }); diff --git a/packages/lint/src/validate-action-name-refs.ts b/packages/lint/src/validate-action-name-refs.ts index 1c8c1a44ed..69ecf69a07 100644 --- a/packages/lint/src/validate-action-name-refs.ts +++ b/packages/lint/src/validate-action-name-refs.ts @@ -36,6 +36,8 @@ * miss; it is called out in the hint rather than guessed at. */ +import { walkPageComponents } from './page-walk.js'; + export const ACTION_NAME_UNDEFINED = 'action-name-undefined'; export type ActionNameRefSeverity = 'error' | 'warning'; @@ -190,30 +192,23 @@ export function validateActionNameRefs(stack: AnyRec): ActionNameRefFinding[] { if (!page || typeof page !== 'object') continue; const pageName = strName(page.name) ?? `#${pi}`; - const walkComponents = (components: unknown, basePath: string) => { - const list = Array.isArray(components) ? (components as AnyRec[]) : []; - for (let ci = 0; ci < list.length; ci++) { - const comp = list[ci]; - if (!comp || typeof comp !== 'object') continue; - const compPath = `${basePath}[${ci}]`; - const props = comp.properties as AnyRec | undefined; - if (props && typeof props === 'object') { - const names = strList(props.actionNames); - for (let ai = 0; ai < names.length; ai++) { - check( - names[ai], - `page "${pageName}" · component "${strName(comp.type) ?? `#${ci}`}"`, - `${compPath}.properties.actionNames[${ai}]`, - 'Quick-actions bar', - ); - } - } - if (Array.isArray(comp.children)) walkComponents(comp.children, `${compPath}.children`); - if (Array.isArray(comp.components)) walkComponents(comp.components, `${compPath}.components`); + // Traversal is shared (`page-walk.ts`): the component tree is NOT where a + // first reading suggests. Components hang off `regions[].components[]` and + // `slots`, never a top-level `page.components`, and sub-trees nest inside + // the untyped `properties` bag rather than under a `children` key. + for (const { component, path } of walkPageComponents(page, `pages[${pi}]`)) { + const props = component.properties as AnyRec | undefined; + if (!props || typeof props !== 'object') continue; + const names = strList(props.actionNames); + for (let ai = 0; ai < names.length; ai++) { + check( + names[ai], + `page "${pageName}" · component "${strName(component.type) ?? '?'}"`, + `${path}.properties.actionNames[${ai}]`, + 'Quick-actions bar', + ); } - }; - - walkComponents(page.components, `pages[${pi}].components`); + } } // ── App navigation: { type: 'action', actionDef: { actionName } } ── diff --git a/packages/lint/src/validate-chart-bindings.test.ts b/packages/lint/src/validate-chart-bindings.test.ts new file mode 100644 index 0000000000..8b5d88bb45 --- /dev/null +++ b/packages/lint/src/validate-chart-bindings.test.ts @@ -0,0 +1,333 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { + validateChartBindings, + CHART_DIMENSION_UNKNOWN, + CHART_MEASURE_UNKNOWN, + CHART_DATASET_UNKNOWN, + CHART_AXIS_NOT_SELECTED, +} from './validate-chart-bindings.js'; + +/** A dataset whose measure names deliberately differ from the base fields. */ +const baseStack = () => ({ + datasets: [ + { + name: 'task_metrics', + object: 'showcase_task', + dimensions: [{ name: 'status', field: 'status' }, { name: 'priority', field: 'priority' }], + measures: [ + { name: 'task_count', aggregate: 'count' }, + { name: 'est_hours', aggregate: 'sum', field: 'estimate_hours' }, + ], + }, + ], +}); + +describe('validateChartBindings — report charts', () => { + // The HotCRM instance: an axis naming the RAW FIELD instead of the measure. + it('errors on a yAxis naming a raw field rather than a dataset measure', () => { + const findings = validateChartBindings({ + ...baseStack(), + reports: [ + { + name: 'hours_by_status', + dataset: 'task_metrics', + rows: ['status'], + values: ['est_hours'], + chart: { type: 'bar', xAxis: 'status', yAxis: 'estimate_hours' }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].path).toBe('reports[0].chart.yAxis'); + expect(findings[0].message).toContain('estimate_hours'); + expect(findings[0].hint).toContain('est_hours'); + }); + + // The dashboard rule's `Array.isArray(yAxis)` guard would skip this shape. + it('handles the report string yAxis, not just the array form', () => { + const clean = validateChartBindings({ + ...baseStack(), + reports: [ + { + name: 'ok', + dataset: 'task_metrics', + values: ['est_hours'], + chart: { type: 'bar', xAxis: 'status', yAxis: 'est_hours' }, + }, + ], + }); + expect(clean).toEqual([]); + }); + + it('errors on an xAxis naming an undeclared dimension', () => { + const findings = validateChartBindings({ + ...baseStack(), + reports: [ + { + name: 'r', + dataset: 'task_metrics', + values: ['task_count'], + chart: { type: 'bar', xAxis: 'assignee', yAxis: 'task_count' }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_DIMENSION_UNKNOWN); + expect(findings[0].path).toBe('reports[0].chart.xAxis'); + }); + + it('warns when the yAxis measure is declared but not selected', () => { + const findings = validateChartBindings({ + ...baseStack(), + reports: [ + { + name: 'r', + dataset: 'task_metrics', + values: ['task_count'], + chart: { type: 'bar', xAxis: 'status', yAxis: 'est_hours' }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].rule).toBe(CHART_AXIS_NOT_SELECTED); + }); + + it('errors on an unresolvable dataset', () => { + const findings = validateChartBindings({ + ...baseStack(), + reports: [ + { name: 'r', dataset: 'task_metric', values: [], chart: { type: 'bar', xAxis: 'status', yAxis: 'x' } }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_DATASET_UNKNOWN); + expect(findings[0].hint).toContain('Did you mean "task_metrics"?'); + }); + + it('checks a joined report block chart against the block dataset', () => { + const findings = validateChartBindings({ + ...baseStack(), + reports: [ + { + name: 'joined', + type: 'joined', + blocks: [ + { + name: 'b1', + dataset: 'task_metrics', + values: ['task_count'], + chart: { type: 'pie', xAxis: 'ghost_dim', yAxis: 'task_count' }, + }, + ], + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('reports[0].blocks[0].chart.xAxis'); + }); + + it('checks report series names as measures', () => { + const findings = validateChartBindings({ + ...baseStack(), + reports: [ + { + name: 'r', + dataset: 'task_metrics', + values: ['task_count'], + chart: { type: 'bar', xAxis: 'status', yAxis: 'task_count', series: [{ name: 'ghost_measure' }] }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('reports[0].chart.series[0].name'); + }); +}); + +describe('validateChartBindings — list-view charts', () => { + it('errors on an unknown measure in views[].listViews', () => { + const findings = validateChartBindings({ + ...baseStack(), + views: [ + { + name: 'showcase_task', + listViews: { + chart: { + type: 'chart', + chart: { chartType: 'bar', dataset: 'task_metrics', dimensions: ['status'], values: ['estimate_hours'] }, + }, + }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].path).toBe('views[0].listViews.chart.chart.values[0]'); + }); + + it('errors on an unknown dimension in views[].list', () => { + const findings = validateChartBindings({ + ...baseStack(), + views: [ + { + name: 'v', + list: { chart: { chartType: 'bar', dataset: 'task_metrics', dimensions: ['ghost'], values: ['task_count'] } }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_DIMENSION_UNKNOWN); + expect(findings[0].path).toBe('views[0].list.chart.dimensions[0]'); + }); + + it('checks object-level listViews too', () => { + const findings = validateChartBindings({ + ...baseStack(), + objects: [ + { + name: 'showcase_task', + fields: {}, + listViews: { + by_status: { + chart: { chartType: 'pie', dataset: 'task_metrics', dimensions: ['status'], values: ['ghost'] }, + }, + }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('objects[0].listViews.by_status.chart.values[0]'); + }); + + it('accepts a fully resolved list chart', () => { + const findings = validateChartBindings({ + ...baseStack(), + views: [ + { + name: 'v', + listViews: { + chart: { chart: { chartType: 'bar', dataset: 'task_metrics', dimensions: ['status'], values: ['est_hours'] } }, + }, + }, + ], + }); + expect(findings).toEqual([]); + }); +}); + +describe('validateChartBindings — dataset-bound page chart components', () => { + it('checks dimensions/values and a ChartConfig-style yAxis', () => { + const findings = validateChartBindings({ + ...baseStack(), + pages: [ + { + name: 'command_center', + regions: [ + { + name: 'main', + components: [ + { + type: 'object-chart', + properties: { + dataset: 'task_metrics', + chartType: 'bar', + dimensions: ['status'], + values: ['task_count'], + yAxis: [{ field: 'estimate_hours', stepSize: 1 }], + }, + }, + ], + }, + ], + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].path).toBe( + 'pages[0].regions[0].components[0].properties.yAxis[0].field', + ); + }); + + it('accepts a resolved page chart', () => { + const findings = validateChartBindings({ + ...baseStack(), + pages: [ + { + name: 'p', + regions: [ + { + name: 'main', + components: [ + { + type: 'object-chart', + properties: { + dataset: 'task_metrics', + dimensions: ['status'], + values: ['task_count'], + yAxis: [{ field: 'task_count' }], + }, + }, + ], + }, + ], + }, + ], + }); + expect(findings).toEqual([]); + }); + + it('leaves an object-bound chart component alone (no dataset key)', () => { + const findings = validateChartBindings({ + ...baseStack(), + pages: [ + { + name: 'p', + regions: [ + { + name: 'main', + components: [ + { + type: 'object-chart', + properties: { + objectName: 'showcase_invoice', + aggregate: { field: 'total', function: 'sum', groupBy: 'status' }, + }, + }, + ], + }, + ], + }, + ], + }); + expect(findings).toEqual([]); + }); +}); + +describe('validateChartBindings — floor', () => { + it('is silent with no charts anywhere and tolerates empty input', () => { + expect(validateChartBindings(baseStack())).toEqual([]); + expect(validateChartBindings({})).toEqual([]); + expect(validateChartBindings(null as unknown as Record)).toEqual([]); + }); + + it('ignores a report with no chart', () => { + const findings = validateChartBindings({ + ...baseStack(), + reports: [{ name: 'plain', dataset: 'task_metrics', rows: ['status'], values: ['task_count'] }], + }); + expect(findings).toEqual([]); + }); + + it('does not mistake a tree view named "org_chart" for a chart', () => { + const findings = validateChartBindings({ + ...baseStack(), + views: [{ name: 'business_unit', listViews: { org_chart: { type: 'tree' } } }], + }); + expect(findings).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-chart-bindings.ts b/packages/lint/src/validate-chart-bindings.ts new file mode 100644 index 0000000000..cf593e0fb7 --- /dev/null +++ b/packages/lint/src/validate-chart-bindings.ts @@ -0,0 +1,392 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0021 — semantic layer] Chart-binding integrity for the surfaces the + * dashboard rule does not reach (issue #3583, assessment R4). + * + * `validate-widget-bindings` already resolves a dashboard widget's + * `chartConfig` axes against its dataset's declared dimensions and measures + * (`chart-field-unknown`). It is scoped to `stack.dashboards`, so three other + * chart surfaces ship unchecked — and the HotCRM audit found exactly the bug + * that scoping allows: an axis naming a RAW FIELD instead of a dataset measure. + * Post-ADR-0021 the result rows are keyed by measure NAME (`sum_amount`), not + * the base column (`amount`), so the axis renders and the series is empty. + * + * Surfaces covered here: + * + * 1. **Report charts** — `report.chart` and `report.blocks[].chart`. + * `ReportChartSchema` narrows `xAxis`/`yAxis` from ChartConfig's + * object/array shapes to bare STRINGS, which is why simply pointing the + * dashboard rule at reports would find nothing: its `Array.isArray(yAxis)` + * guard skips a string silently. `series[].name` keeps the array shape. + * 2. **List-view charts** — `ListChartConfigSchema` (`dataset` + + * `dimensions` + `values`), reachable through `views[].list`, + * `views[].listViews.`, and `objects[].listViews.`. + * 3. **Dataset-bound page chart components** — a `PageComponent` whose + * `properties` carry a `dataset` (the `object-chart` component). Same + * binding shape as a list chart, but it arrives through the untyped + * `properties` bag. + * + * Deliberately NOT covered: the react `` block. It is + * OBJECT-bound (`objectName` + an inline `aggregate`), not dataset-bound, and + * nothing in the repo pins down what the runtime names the aggregated result + * column — so there is no defensible answer for what its `yAxis[].field` + * should resolve against. Guessing one would manufacture false positives on + * the single authored usage. Its `aggregate.field` / `groupBy` ARE checkable + * against the object's fields, but only by reading JSX attribute values, which + * `validate-react-page-props` does not do today. Left for a follow-up rather + * than half-built (ADR-0078 §5: verify, then enforce). + */ + +export const CHART_DIMENSION_UNKNOWN = 'chart-dimension-unknown'; +export const CHART_MEASURE_UNKNOWN = 'chart-measure-unknown'; +export const CHART_DATASET_UNKNOWN = 'chart-dataset-unknown'; +export const CHART_AXIS_NOT_SELECTED = 'chart-axis-not-selected'; + +export type ChartBindingSeverity = 'error' | 'warning'; + +export interface ChartBindingFinding { + severity: ChartBindingSeverity; + /** Diagnostic rule id. */ + rule: string; + /** Human-readable location, e.g. `report "hours_by_status" · chart`. */ + where: string; + /** Config path, e.g. `reports[2].chart.yAxis`. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +import { walkPageComponents, type AnyRec } from './page-walk.js'; + +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; +} + +function strList(v: unknown): string[] { + return Array.isArray(v) ? v.filter((x): x is string => typeof x === 'string' && x.length > 0) : []; +} + +function isRec(v: unknown): v is AnyRec { + return !!v && typeof v === 'object' && !Array.isArray(v); +} + +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]; +} + +function suggest(target: string, known: Iterable): string { + let best: string | undefined; + let bestScore = Infinity; + for (const c of known) { + const d = distance(target, c); + if (d < bestScore) { + bestScore = d; + best = c; + } + } + const limit = Math.max(2, Math.floor(target.length / 3)); + return best && bestScore <= limit ? ` Did you mean "${best}"?` : ''; +} + +function list(names: Iterable): string { + const all = [...names].sort(); + return all.length ? all.join(', ') : '(none)'; +} + +/** A dataset's declared dimension and measure names. */ +interface DatasetNames { + dimensions: Set; + measures: Set; +} + +function indexDatasets(stack: AnyRec): Map { + const out = new Map(); + for (const ds of asArray(stack.datasets)) { + const name = strName(ds.name); + if (!name) continue; + const dimensions = new Set(); + for (const d of asArray(ds.dimensions)) { + const n = strName(d.name); + if (n) dimensions.add(n); + } + const measures = new Set(); + for (const m of asArray(ds.measures)) { + const n = strName(m.name); + if (n) measures.add(n); + } + out.set(name, { dimensions, measures }); + } + return out; +} + +/** + * One dataset-bound chart to check: the binding, the selection, and where it + * came from. `xAxis`/`yAxis`/`series` are the ChartConfig-style axis refs; + * `dimensions`/`values` are the list-chart-style selection. + */ +interface ChartBinding { + dataset?: string; + /** Selected dimension names (list-chart shape). */ + dimensions?: { names: string[]; path: string }; + /** Selected measure names (list-chart / report shape). */ + values?: { names: string[]; path: string }; + /** Single dimension ref (report `xAxis`). */ + xAxis?: { name: string; path: string }; + /** Single measure ref (report `yAxis`). */ + yAxis?: { name: string; path: string }; + /** Series names — measure refs (ChartConfig shape). */ + series?: Array<{ name: string; path: string }>; + where: string; + /** Path of the chart container, for the dataset-level finding. */ + path: string; +} + +export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { + const findings: ChartBindingFinding[] = []; + if (!stack || typeof stack !== 'object') return findings; + + const datasets = indexDatasets(stack); + if (datasets.size === 0 && !stack.reports && !stack.views && !stack.pages) return findings; + + const check = (binding: ChartBinding) => { + const dsName = binding.dataset; + if (!dsName) return; // nothing bound — the shape rules own that case + const ds = datasets.get(dsName); + if (!ds) { + findings.push({ + severity: 'error', + rule: CHART_DATASET_UNKNOWN, + where: binding.where, + path: `${binding.path}.dataset`, + message: + `binds dataset "${dsName}", which resolves to no declared dataset — ` + + `the chart has no data to render.`, + hint: + `Declared datasets: ${list(datasets.keys())}.${suggest(dsName, datasets.keys())} ` + + `Define it with defineDataset() or fix the reference (ADR-0021).`, + }); + return; + } + + const dimensionRef = (name: string, path: string) => { + if (ds.dimensions.has(name)) return; + findings.push({ + severity: 'error', + rule: CHART_DIMENSION_UNKNOWN, + where: binding.where, + path, + message: + `"${name}" is not a dimension declared by dataset "${dsName}". ` + + `Post-ADR-0021 result rows are keyed by DIMENSION NAME, not the base ` + + `field, so this axis renders with no categories.`, + hint: + `Dataset dimensions: ${list(ds.dimensions)}.${suggest(name, ds.dimensions)} ` + + `Declare the dimension on the dataset, or bind an existing one.`, + }); + }; + + const measureRef = (name: string, path: string, selected?: Set) => { + if (!ds.measures.has(name)) { + findings.push({ + severity: 'error', + rule: CHART_MEASURE_UNKNOWN, + where: binding.where, + path, + message: + `"${name}" is not a measure declared by dataset "${dsName}". ` + + `Post-ADR-0021 result rows are keyed by MEASURE NAME (e.g. "sum_amount"), ` + + `not the base field (e.g. "amount"), so this series comes back empty.`, + hint: + `Dataset measures: ${list(ds.measures)}.${suggest(name, ds.measures)} ` + + `Declare the measure on the dataset, or bind an existing one.`, + }); + return; + } + // Declared but not part of this chart's selection: the query never asks + // for it, so the axis still plots nothing. Advisory — the selection may + // legitimately be widened at runtime. + if (selected && selected.size > 0 && !selected.has(name)) { + findings.push({ + severity: 'warning', + rule: CHART_AXIS_NOT_SELECTED, + where: binding.where, + path, + message: + `"${name}" is a declared measure of "${dsName}" but is not in this chart's ` + + `selected values (${list(selected)}) — the query does not return it, ` + + `so the series plots nothing.`, + hint: `Add "${name}" to \`values\`, or point the axis at a selected measure.`, + }); + } + }; + + const dimSel = binding.dimensions; + if (dimSel) { + for (let i = 0; i < dimSel.names.length; i++) { + dimensionRef(dimSel.names[i], `${dimSel.path}[${i}]`); + } + } + const valSel = binding.values; + const selected = new Set(valSel?.names ?? []); + if (valSel) { + for (let i = 0; i < valSel.names.length; i++) { + measureRef(valSel.names[i], `${valSel.path}[${i}]`); + } + } + if (binding.xAxis) dimensionRef(binding.xAxis.name, binding.xAxis.path); + if (binding.yAxis) measureRef(binding.yAxis.name, binding.yAxis.path, selected); + for (const s of binding.series ?? []) measureRef(s.name, s.path, selected); + }; + + // ── 1. Report charts (report.chart + report.blocks[].chart) ── + const reports = asArray(stack.reports); + for (let ri = 0; ri < reports.length; ri++) { + const report = reports[ri]; + if (!isRec(report)) continue; + const reportName = strName(report.name) ?? `#${ri}`; + + const checkReportChart = ( + chart: unknown, + dataset: string | undefined, + values: string[], + where: string, + path: string, + ) => { + if (!isRec(chart)) return; + check({ + dataset, + // `values` is the report's measure SELECTION, not a chart ref; feeding + // it in lets the yAxis "declared but not selected" check work without + // reporting the selection itself twice. + values: { names: values, path: `${path}.values` }, + xAxis: strName(chart.xAxis) ? { name: strName(chart.xAxis)!, path: `${path}.chart.xAxis` } : undefined, + yAxis: strName(chart.yAxis) ? { name: strName(chart.yAxis)!, path: `${path}.chart.yAxis` } : undefined, + series: asArray(chart.series) + .map((s, si) => ({ name: strName(s.name), path: `${path}.chart.series[${si}].name` })) + .filter((s): s is { name: string; path: string } => !!s.name), + where, + path: `${path}.chart`, + }); + }; + + checkReportChart( + report.chart, + strName(report.dataset), + strList(report.values), + `report "${reportName}" · chart`, + `reports[${ri}]`, + ); + + const blocks = Array.isArray(report.blocks) ? report.blocks : []; + for (let bi = 0; bi < blocks.length; bi++) { + const block = blocks[bi]; + if (!isRec(block)) continue; + checkReportChart( + block.chart, + strName(block.dataset), + strList(block.values), + `report "${reportName}" · block "${strName(block.name) ?? `#${bi}`}" chart`, + `reports[${ri}].blocks[${bi}]`, + ); + } + } + + // ── 2. List-view charts ── + const checkListChart = (container: unknown, where: string, path: string) => { + if (!isRec(container)) return; + const chart = container.chart; + if (!isRec(chart)) return; + check({ + dataset: strName(chart.dataset), + dimensions: { names: strList(chart.dimensions), path: `${path}.chart.dimensions` }, + values: { names: strList(chart.values), path: `${path}.chart.values` }, + where, + path: `${path}.chart`, + }); + }; + + const views = asArray(stack.views); + for (let vi = 0; vi < views.length; vi++) { + const view = views[vi]; + if (!isRec(view)) continue; + const viewName = strName(view.name) ?? strName(view.objectName) ?? `#${vi}`; + checkListChart(view.list, `view "${viewName}" · list chart`, `views[${vi}].list`); + if (isRec(view.listViews)) { + for (const [key, lv] of Object.entries(view.listViews)) { + checkListChart(lv, `view "${viewName}" · listViews.${key} chart`, `views[${vi}].listViews.${key}`); + } + } + } + + const objects = asArray(stack.objects); + for (let oi = 0; oi < objects.length; oi++) { + const obj = objects[oi]; + if (!isRec(obj) || !isRec(obj.listViews)) continue; + const objName = strName(obj.name) ?? `#${oi}`; + for (const [key, lv] of Object.entries(obj.listViews)) { + checkListChart( + lv, + `object "${objName}" · listViews.${key} chart`, + `objects[${oi}].listViews.${key}`, + ); + } + } + + // ── 3. Dataset-bound page chart components ── + // A chart component arrives through the untyped `properties` bag. The + // presence of a `dataset` key is what marks it dataset-bound (and so + // checkable); an object-bound chart has none and is left alone. + const pages = asArray(stack.pages); + for (let pi = 0; pi < pages.length; pi++) { + const page = pages[pi]; + if (!isRec(page)) continue; + const pageName = strName(page.name) ?? `#${pi}`; + for (const { component, path } of walkPageComponents(page, `pages[${pi}]`)) { + const props = isRec(component.properties) ? component.properties : undefined; + if (!props || !strName(props.dataset)) continue; + // A page chart mixes the list-chart selection (`dataset`/`dimensions`/ + // `values`) with ChartConfig-style axes (`yAxis: [{ field }]`), so both + // shapes are read here. + const axisRefs = asArray(props.yAxis) + .map((a, ai) => ({ name: strName(a.field), path: `${path}.properties.yAxis[${ai}].field` })) + .filter((a): a is { name: string; path: string } => !!a.name); + const seriesRefs = asArray(props.series) + .map((s, si) => ({ name: strName(s.name), path: `${path}.properties.series[${si}].name` })) + .filter((s): s is { name: string; path: string } => !!s.name); + check({ + dataset: strName(props.dataset), + dimensions: { names: strList(props.dimensions), path: `${path}.properties.dimensions` }, + values: { names: strList(props.values), path: `${path}.properties.values` }, + series: [...axisRefs, ...seriesRefs], + where: `page "${pageName}" · ${strName(component.type) ?? 'chart'}`, + path: `${path}.properties`, + }); + } + } + + return findings; +} diff --git a/packages/lint/src/validate-page-field-bindings.test.ts b/packages/lint/src/validate-page-field-bindings.test.ts new file mode 100644 index 0000000000..09c3c473eb --- /dev/null +++ b/packages/lint/src/validate-page-field-bindings.test.ts @@ -0,0 +1,319 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { validatePageFieldBindings, PAGE_FIELD_UNKNOWN } from './validate-page-field-bindings.js'; + +const baseStack = () => ({ + objects: [ + { + name: 'crm_lead', + fields: { + name: { type: 'text' }, + status: { type: 'select' }, + account: { type: 'lookup', reference: 'crm_account' }, + amount: { type: 'currency' }, + }, + }, + { + name: 'crm_account', + fields: { name: { type: 'text' }, region: { type: 'text' } }, + }, + ], +}); + +/** A `kind: 'full'` record page with one region holding `components`. */ +const pageWith = (components: unknown[], extra: Record = {}) => ({ + name: 'lead_detail', + object: 'crm_lead', + regions: [{ name: 'main', components }], + ...extra, +}); + +describe('validatePageFieldBindings — highlights / KPI cards', () => { + // The HotCRM instance: a highlights strip naming a field the object lacks. + it('warns on an unknown highlights field', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { type: 'record:highlights', properties: { fields: ['status', 'total_revenue'] } }, + ])], + }); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].rule).toBe(PAGE_FIELD_UNKNOWN); + expect(findings[0].path).toBe('pages[0].regions[0].components[0].properties.fields[1]'); + expect(findings[0].message).toContain('total_revenue'); + expect(findings[0].message).toContain('crm_lead'); + }); + + it('accepts the object form of a highlights field', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { type: 'record:highlights', properties: { fields: [{ name: 'status', label: 'Status' }] } }, + ])], + }); + expect(findings).toEqual([]); + }); + + it('flags the object form when the name is unknown', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { type: 'record:highlights', properties: { fields: [{ name: 'ghost' }] } }, + ])], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('pages[0].regions[0].components[0].properties.fields[0].name'); + }); + + it('warns on an unknown element:number aggregate field, against its own object', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { + type: 'element:number', + properties: { object: 'crm_account', field: 'amount', aggregate: 'sum' }, + }, + ])], + }); + // `amount` exists on crm_lead but NOT on crm_account, which this KPI binds. + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('crm_account'); + }); +}); + +describe('validatePageFieldBindings — record:details real authored shape', () => { + // Real pages author `sections: [{ label, fields }]`, which RecordDetailsProps + // does not describe (it survives because `properties` is unvalidated). + it('walks sections[].fields[]', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { + type: 'record:details', + properties: { + sections: [ + { label: 'Overview', fields: ['name', 'status'] }, + { label: 'Money', fields: ['amount', 'nonexistent_field'] }, + ], + }, + }, + ])], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe( + 'pages[0].regions[0].components[0].properties.sections[1].fields[1]', + ); + }); + + it('walks hideFields', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { type: 'record:details', properties: { hideFields: ['status', 'ghost'] } }, + ])], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('pages[0].regions[0].components[0].properties.hideFields[1]'); + }); +}); + +describe('validatePageFieldBindings — related lists bind the related object', () => { + it('checks columns against objectName, not the page object', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { + type: 'record:related_list', + properties: { + objectName: 'crm_account', + relationshipField: 'region', + // `status` is a crm_lead field; the related object is crm_account. + columns: ['name', 'status'], + }, + }, + ])], + }); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('crm_account'); + expect(findings[0].path).toBe('pages[0].regions[0].components[0].properties.columns[1]'); + }); + + it('checks the add-picker against its own object', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { + type: 'record:related_list', + properties: { + objectName: 'crm_account', + add: { picker: { object: 'crm_lead', valueField: 'name', labelField: 'ghost' } }, + }, + }, + ])], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe( + 'pages[0].regions[0].components[0].properties.add.picker.labelField', + ); + }); +}); + +describe('validatePageFieldBindings — binding precedence and traversal', () => { + it('dataSource.object overrides the page object', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { + type: 'record:highlights', + dataSource: { object: 'crm_account' }, + properties: { fields: ['region', 'status'] }, + }, + ])], + }); + // `region` is on crm_account (ok); `status` is not. + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('crm_account'); + }); + + it('walks slots and nested tab children', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [ + { + name: 'p', + object: 'crm_lead', + kind: 'slotted', + slots: { + highlights: { type: 'record:highlights', properties: { fields: ['ghost_a'] } }, + tabs: { + type: 'page:tabs', + properties: { + items: [ + { + label: 'Detail', + children: [ + { type: 'record:details', properties: { fields: ['ghost_b'] } }, + ], + }, + ], + }, + }, + }, + }, + ], + }); + expect(findings).toHaveLength(2); + expect(findings.map((f) => f.path).sort()).toEqual([ + 'pages[0].slots.highlights.properties.fields[0]', + 'pages[0].slots.tabs.properties.items[0].children[0].properties.fields[0]', + ]); + }); + + it('checks interfaceConfig against `source`', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [ + { + name: 'list_page', + type: 'list', + object: 'crm_lead', + interfaceConfig: { + source: 'crm_lead', + columns: ['name', { field: 'ghost_col' }], + sort: [{ field: 'amount', order: 'desc' }], + filterBy: [{ field: 'ghost_filter', operator: 'equals', value: 'x' }], + userFilters: { element: 'dropdown', fields: [{ field: 'status' }] }, + }, + }, + ], + }); + expect(findings.map((f) => f.path).sort()).toEqual([ + 'pages[0].interfaceConfig.columns[1].field', + 'pages[0].interfaceConfig.filterBy[0].field', + ]); + }); +}); + +describe('validatePageFieldBindings — false-positive floor', () => { + it('skips an unregistered component type', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { + type: 'record:line_items', + properties: { childObject: 'crm_account', amountField: 'whatever', columns: [{ field: 'nope' }] }, + }, + ])], + }); + expect(findings).toEqual([]); + }); + + it('skips a component bound to a cross-package object', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { + type: 'record:highlights', + dataSource: { object: 'sys_user' }, + properties: { fields: ['anything_at_all'] }, + }, + ])], + }); + expect(findings).toEqual([]); + }); + + it('skips relationship dot-paths and system fields', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { + type: 'record:details', + properties: { fields: ['account.name', 'created_at', 'id', 'owner_id'] }, + }, + ])], + }); + expect(findings).toEqual([]); + }); + + it('skips a source-authored page', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([{ type: 'record:highlights', properties: { fields: ['ghost'] } }], { + kind: 'react', + source: 'export default () => null;', + })], + }); + expect(findings).toEqual([]); + }); + + it('is silent on a clean page and tolerates empty input', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [pageWith([ + { type: 'record:highlights', properties: { fields: ['name', 'status'] } }, + { type: 'record:path', properties: { statusField: 'status' } }, + ])], + }); + expect(findings).toEqual([]); + expect(validatePageFieldBindings({})).toEqual([]); + expect(validatePageFieldBindings(null as unknown as Record)).toEqual([]); + }); + + it('does not check a component with no resolvable object', () => { + const findings = validatePageFieldBindings({ + ...baseStack(), + pages: [ + { + name: 'app_page', + type: 'app', + regions: [{ name: 'main', components: [ + { type: 'record:highlights', properties: { fields: ['ghost'] } }, + ] }], + }, + ], + }); + expect(findings).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-page-field-bindings.ts b/packages/lint/src/validate-page-field-bindings.ts new file mode 100644 index 0000000000..2153346aa6 --- /dev/null +++ b/packages/lint/src/validate-page-field-bindings.ts @@ -0,0 +1,302 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0078 — completeness] Field-reference integrity for page components + * (issue #3583, assessment R3). + * + * `PageComponent.properties` is `z.record(z.string(), z.unknown())` — an + * untyped bag. The typed prop schemas exist (`ComponentPropsMap` in + * `@objectstack/spec/ui`) but nothing validates `properties` against them, so + * every field name a component references ships exactly as typed. The HotCRM + * audit found KPI cards and page headers bound to fields the object does not + * have; each renders blank or falls back, and nothing reports the miss. + * + * Field-existence lint already exists for forms (`FORM_FIELD_UNKNOWN`), + * semantic roles, and flow templates. This is the same check for pages, at the + * same advisory severity: every consumer degrades gracefully (a missing field + * is skipped, not crashed on), so a warning is the honest level. + * + * ── Which object a component binds ────────────────────────────────────── + * + * `dataSource.object` → `properties.object` → the page's `object`. A per-element + * `dataSource` exists precisely so one page can bind several objects, and the + * `element:*` family declares its own `object`; both must win over the page's. + * + * ── Why a hand-written descriptor table ───────────────────────────────── + * + * `ComponentPropsMap` cannot drive this rule: a Zod schema does not say which + * of its `z.string()` props is a FIELD NAME (`RecordPathProps.statusField` and + * `AIChatWindowProps.agentId` are both plain strings), and the type universe is + * open anyway — `PageComponent.type` is `z.union([PageComponentType, + * z.string()])`, so unregistered types like `record:line_items` parse and are + * authored in the wild. The table below names the field-bearing props + * explicitly; an unknown component type is SKIPPED silently, never flagged. + * + * The table also covers shapes the props schemas do not yet describe but real + * pages authored anyway (they pass only because `properties` is unvalidated): + * `record:details` `sections[].fields[]` and `hideFields[]`, and the record + * picker's `labelField`. Linting the schema's shape alone would find nothing on + * the actual corpus. + */ + +export const PAGE_FIELD_UNKNOWN = 'page-field-unknown'; + +export type PageFieldSeverity = 'error' | 'warning'; + +export interface PageFieldFinding { + /** Always `warning` — page renderers skip an unknown field rather than fail. */ + severity: PageFieldSeverity; + /** Diagnostic rule id. */ + rule: string; + /** Human-readable location, e.g. `page "task_detail" · record:highlights`. */ + where: string; + /** Config path, e.g. `pages[0].regions[1].components[0].properties.fields[2]`. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +import { walkPageComponents, type AnyRec } from './page-walk.js'; + +/** + * Registry-injected fields present on (almost) every object but NOT declared in + * `object.fields`. Copied verbatim from `validate-widget-bindings` so the two + * rules agree on what counts as a field. Deliberately generous: over-inclusion + * costs at worst a missed warning on a `systemFields: false` object; under- + * inclusion costs a false one, and a false finding is what makes authors stop + * trusting the linter (ADR-0072 D1). Real pages DO reference these — e.g. + * `sys_user.page.ts` lists `created_at` in a related-list's columns. + */ +const SYSTEM_FIELDS = new Set([ + 'id', + 'created_at', 'created_by', 'updated_at', 'updated_by', + 'owner_id', 'organization_id', 'tenant_id', 'user_id', + 'deleted_at', +]); + +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; +} + +function isRec(v: unknown): v is AnyRec { + return !!v && typeof v === 'object' && !Array.isArray(v); +} + +/** A field reference found in a props bag, with the path that located it. */ +interface FieldRef { + name: string; + path: string; +} + +/** + * Pull field names out of a value that may be a bare string, a `{field}` or + * `{name}` record, or an array of either — the three shapes the component props + * use interchangeably (`record:highlights` keys its object form `name`, while + * columns/sort/filter key theirs `field`). + */ +function fieldRefsFrom(value: unknown, basePath: string): FieldRef[] { + const out: FieldRef[] = []; + const one = (v: unknown, path: string) => { + const bare = strName(v); + if (bare) { + out.push({ name: bare, path }); + return; + } + if (!isRec(v)) return; + const named = strName(v.field) ?? strName(v.name); + if (named) out.push({ name: named, path: `${path}.${strName(v.field) ? 'field' : 'name'}` }); + }; + if (Array.isArray(value)) { + for (let i = 0; i < value.length; i++) one(value[i], `${basePath}[${i}]`); + } else { + one(value, basePath); + } + return out; +} + +/** + * Per-component-type descriptor: which `properties` paths hold field names, and + * whether they resolve against this component's own object or another one. + * + * `props` entries are read from `component.properties`. Entries under + * `nestedSections` walk `properties.[].fields[]` — the section shape real + * pages author for `record:details`. + */ +interface ComponentFieldSpec { + /** Props holding field names bound to the component's resolved object. */ + props?: readonly string[]; + /** Props holding `{...}[]` section objects whose `fields[]` are field names. */ + nestedSections?: readonly string[]; +} + +const COMPONENT_FIELD_SPECS: Readonly> = { + 'record:highlights': { props: ['fields'] }, + // `sections`/`hideFields` are not in RecordDetailsProps, but every real page + // authors them (they survive because `properties` is unvalidated). + 'record:details': { props: ['fields', 'hideFields'], nestedSections: ['sections'] }, + 'record:path': { props: ['statusField'] }, + 'element:number': { props: ['field'] }, + 'element:filter': { props: ['fields'] }, + 'element:form': { props: ['fields'] }, + // The schema says `displayField`; real pages author `labelField`. Accept both. + 'element:record_picker': { props: ['displayField', 'labelField', 'searchFields'] }, +}; + +/** + * `record:related_list` is special: its `columns`/`sort`/`filter` resolve + * against the RELATED object (`properties.objectName`), not the page's object, + * so it cannot ride the generic table. + */ +const RELATED_LIST_TYPE = 'record:related_list'; + +export function validatePageFieldBindings(stack: AnyRec): PageFieldFinding[] { + const findings: PageFieldFinding[] = []; + if (!stack || typeof stack !== 'object') return findings; + + // object name → its declared field names. Built with `asArray` so BOTH + // `fields` shapes (array of `{name}` and name-keyed map) resolve. + const objectFields = new Map>(); + for (const obj of asArray(stack.objects)) { + const name = strName(obj.name); + if (!name) continue; + const names = new Set(); + for (const f of asArray(obj.fields)) { + const fn = strName(f.name); + if (fn) names.add(fn); + } + objectFields.set(name, names); + } + + const pages = asArray(stack.pages); + for (let pi = 0; pi < pages.length; pi++) { + const page = pages[pi]; + if (!page || typeof page !== 'object') continue; + const pageName = strName(page.name) ?? `#${pi}`; + + /** + * Check one batch of refs against `objectName`. Bails out entirely when the + * object is not defined in this stack — it may come from another installed + * package, and we cannot judge fields on a schema we cannot see (the same + * skip the flow/widget rules use). + */ + const checkRefs = (refs: FieldRef[], objectName: string | undefined, where: string) => { + if (!objectName) return; // nothing to resolve against + const known = objectFields.get(objectName); + if (!known) return; // cross-package object — unknowable here + for (const ref of refs) { + // A relationship path (`account.name`) is resolved by the query engine, + // not a base column, so it cannot be judged here. + if (ref.name.includes('.')) continue; + if (known.has(ref.name) || SYSTEM_FIELDS.has(ref.name)) continue; + findings.push({ + severity: 'warning', + rule: PAGE_FIELD_UNKNOWN, + where, + path: ref.path, + message: + `field "${ref.name}" is not a field on object "${objectName}" — ` + + `the component silently skips it, so it never renders.`, + hint: + `Fix the field name, or add "${ref.name}" to ${objectName}. ` + + `References must match the object's field names exactly.` + + (known.size > 0 ? ` Object fields: ${[...known].sort().join(', ')}.` : ''), + }); + } + }; + + for (const { component, path, objectName } of walkPageComponents(page, `pages[${pi}]`)) { + const type = strName(component.type); + const props = isRec(component.properties) ? component.properties : undefined; + if (!type || !props) continue; + const where = `page "${pageName}" · ${type}`; + + if (type === RELATED_LIST_TYPE) { + // Columns / sort / filter address the RELATED object. + const relatedObject = strName(props.objectName); + const relatedRefs: FieldRef[] = [ + ...fieldRefsFrom(props.columns, `${path}.properties.columns`), + ...fieldRefsFrom(props.sort, `${path}.properties.sort`), + ...fieldRefsFrom(props.filter, `${path}.properties.filter`), + ...fieldRefsFrom(props.relationshipField, `${path}.properties.relationshipField`), + ]; + checkRefs(relatedRefs, relatedObject, where); + // `relationshipValueField` names a field on the PARENT (page) object. + checkRefs( + fieldRefsFrom(props.relationshipValueField, `${path}.properties.relationshipValueField`), + objectName, + where, + ); + // The add-picker resolves against its own object. + const add = isRec(props.add) ? props.add : undefined; + const picker = add && isRec(add.picker) ? add.picker : undefined; + if (picker) { + checkRefs( + [ + ...fieldRefsFrom(picker.valueField, `${path}.properties.add.picker.valueField`), + ...fieldRefsFrom(picker.labelField, `${path}.properties.add.picker.labelField`), + ], + strName(picker.object), + where, + ); + } + if (add) { + checkRefs( + fieldRefsFrom(add.linkField, `${path}.properties.add.linkField`), + relatedObject, + where, + ); + } + continue; + } + + const spec = COMPONENT_FIELD_SPECS[type]; + if (!spec) continue; // unregistered / non-field component — skip silently + + const refs: FieldRef[] = []; + for (const key of spec.props ?? []) { + refs.push(...fieldRefsFrom(props[key], `${path}.properties.${key}`)); + } + for (const key of spec.nestedSections ?? []) { + const sections = Array.isArray(props[key]) ? (props[key] as unknown[]) : []; + for (let si = 0; si < sections.length; si++) { + const section = sections[si]; + if (!isRec(section)) continue; + refs.push( + ...fieldRefsFrom(section.fields, `${path}.properties.${key}[${si}].fields`), + ); + } + } + checkRefs(refs, objectName, where); + } + + // ── interfaceConfig (list pages) ── + // Bound by `interfaceConfig.source`, falling back to the page's object. + const cfg = isRec(page.interfaceConfig) ? page.interfaceConfig : undefined; + if (cfg) { + const cfgObject = strName(cfg.source) ?? strName(page.object); + const base = `pages[${pi}].interfaceConfig`; + const refs: FieldRef[] = [ + ...fieldRefsFrom(cfg.columns, `${base}.columns`), + ...fieldRefsFrom(cfg.sort, `${base}.sort`), + ...fieldRefsFrom(cfg.filterBy, `${base}.filterBy`), + ]; + const userFilters = isRec(cfg.userFilters) ? cfg.userFilters : undefined; + if (userFilters) { + refs.push(...fieldRefsFrom(userFilters.fields, `${base}.userFilters.fields`)); + } + checkRefs(refs, cfgObject, `page "${pageName}" · interfaceConfig`); + } + } + + return findings; +}