Skip to content

Commit 9d8fa97

Browse files
committed
refactor(lint): one entry point for the reference-integrity suite (#3583 D5)
Six rules answering the same question — "does this name resolve to anything?" — were wired by hand into three CLI commands. `validate`, `lint` and `compile` each carried their own import list and their own call site, so landing a rule meant remembering three places, and forgetting one meant the same stack got a different verdict depending on which command the author ran. The assessment (§2.2) named that drift as the enemy and (§5 D5) asked for this entry point once Phase 2 landed; six rules in, it has cost three hand-edits per rule. `validateReferenceIntegrity(stack)` runs `REFERENCE_INTEGRITY_RULES` in order and returns the concatenated findings. Adding a rule is now a one-line edit to that list — the commands do not change. `lint.ts` loses five near-identical for-loops; `validate.ts`/`compile.ts` lose their six-way spreads. Membership is deliberately a written-out list, mirrored by a test, so a rule joins the suite by review rather than by accident. What belongs: rules that resolve a NAME against what a stack declares. What does not: shape checks (view containers, responsive styles, seed replay/state) — a different question with its own call sites. The suite test runs one live instance per member and asserts a finding from each. A suite that silently stops invoking a member is indistinguishable from a clean stack — exactly how the dead quick-actions check in #3684 passed its own tests. `os doctor` is NOT converted: it runs only `validateWidgetBindings` and is an environment health check, not an authoring gate, so adopting the suite there is a product decision about what `doctor` is for. Named in the module doc so the remaining asymmetry stays visible. Behaviour-preserving: identical findings on all three example apps (zero) and on the HotCRM corpus (24, unchanged per rule). Refs #3583 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015r31hzuSN19iK8ATRPcCpS
1 parent 2343099 commit 9d8fa97

6 files changed

Lines changed: 270 additions & 112 deletions

File tree

packages/cli/src/commands/compile.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ import { validateVisibilityPredicates } from '@objectstack/lint';
1313
import { validateWidgetBindings } from '@objectstack/lint';
1414
import { validateDashboardActionRefs } from '@objectstack/lint';
1515
import { validateFilterTokens } from '@objectstack/lint';
16-
import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint';
17-
import { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint';
18-
import { validateNavAccess } from '@objectstack/lint';
19-
import { validateTranslationReferences } from '@objectstack/lint';
16+
import { validateReferenceIntegrity } from '@objectstack/lint';
2017
import { validateResponsiveStyles } from '@objectstack/lint';
2118
import { validateSecurityPosture, validateOrgAxisRedLines, buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
2219
import { validateReadonlyFlowWrites } from '@objectstack/lint';
@@ -342,14 +339,7 @@ export default class Compile extends Command {
342339
// option keys written as the display label) — advisory throughout,
343340
// since an orphan key is inert rather than broken.
344341
if (!flags.json) printStep('Checking object & action references (#3583)...');
345-
const refFindings = [
346-
...validateObjectReferences(result.data as Record<string, unknown>),
347-
...validateActionNameRefs(result.data as Record<string, unknown>),
348-
...validatePageFieldBindings(result.data as Record<string, unknown>),
349-
...validateChartBindings(result.data as Record<string, unknown>),
350-
...validateNavAccess(result.data as Record<string, unknown>),
351-
...validateTranslationReferences(result.data as Record<string, unknown>),
352-
];
342+
const refFindings = validateReferenceIntegrity(result.data as Record<string, unknown>);
353343
const refErrors = refFindings.filter((f) => f.severity === 'error');
354344
const refWarnings = refFindings.filter((f) => f.severity === 'warning');
355345
if (refErrors.length > 0) {

packages/cli/src/commands/lint.ts

Lines changed: 11 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,7 @@ import { computeI18nCoverage, type CoverageIssue } from '../utils/i18n-coverage.
1010
import { lintDataModel } from '../lint/data-model-rules.js';
1111
import { validateWidgetBindings } from '@objectstack/lint';
1212
import { validateRecordTitle, validateSemanticRoles, validateCapabilityReferences, validateSecurityPosture, validateOrgAxisRedLines, validateApprovalApprovers, validateSeedReplaySafety, validateSeedStateMachine } from '@objectstack/lint';
13-
import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint';
14-
import { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint';
15-
import { validateNavAccess } from '@objectstack/lint';
16-
import { validateTranslationReferences } from '@objectstack/lint';
13+
import { validateReferenceIntegrity } from '@objectstack/lint';
1714
import { collectAndLintDocs } from '../utils/collect-docs.js';
1815
import { scoreMetadata } from '../lint/score.js';
1916
import { runMetadataEval } from '../lint/metadata-eval.js';
@@ -493,90 +490,16 @@ export function lintConfig(config: any): LintIssue[] {
493490
});
494491
}
495492

496-
// ── Object-name references (issue #3583) ──
497-
// The reference sites `defineStack` does not cover: action-param
498-
// `reference`/`objectOverride`, dashboard filter `optionsFrom.object`, and
499-
// navigation `requiresObject` gates. An unprefixed miss (`user` for
500-
// `sys_user`) is a typo → error; a platform-prefixed name no known package
501-
// registers (`sys_approval_process`) is advisory, since a third-party
502-
// package may still provide it.
503-
for (const t of validateObjectReferences(config)) {
504-
issues.push({
505-
severity: t.severity,
506-
rule: t.rule,
507-
message: `${t.where}: ${t.message}`,
508-
path: t.path,
509-
fix: t.hint,
510-
});
511-
}
512-
513-
// ── Action-name references (issue #3583) ──
514-
// `bulkActions`/`rowActions`, page `quick_actions`, and nav action items bind
515-
// an action BY NAME. A name matching no defined action ships a button that
516-
// renders and does nothing — same failure `validate-dashboard-action-refs`
517-
// catches for dashboards, so the same severity.
518-
for (const t of validateActionNameRefs(config)) {
519-
issues.push({
520-
severity: t.severity,
521-
rule: t.rule,
522-
message: `${t.where}: ${t.message}`,
523-
path: t.path,
524-
fix: t.hint,
525-
});
526-
}
527-
528-
// ── Page component field bindings (issue #3583) ──
529-
// `PageComponent.properties` is an untyped bag, so a highlights strip, KPI
530-
// card, or details section can name a field the object does not have; the
531-
// component silently skips it. Advisory, matching `FORM_FIELD_UNKNOWN` —
532-
// every page consumer degrades rather than failing.
533-
for (const t of validatePageFieldBindings(config)) {
534-
issues.push({
535-
severity: t.severity,
536-
rule: t.rule,
537-
message: `${t.where}: ${t.message}`,
538-
path: t.path,
539-
fix: t.hint,
540-
});
541-
}
542-
543-
// ── Chart bindings outside dashboards (issue #3583) ──
544-
// `validate-widget-bindings` covers dashboard widgets only. Report charts,
545-
// list-view charts, and dataset-bound page chart components bind the same
546-
// semantic layer, where an axis naming a raw field instead of a dataset
547-
// measure renders an empty series (ADR-0021).
548-
for (const t of validateChartBindings(config)) {
549-
issues.push({
550-
severity: t.severity,
551-
rule: t.rule,
552-
message: `${t.where}: ${t.message}`,
553-
path: t.path,
554-
fix: t.hint,
555-
});
556-
}
557-
558-
// ── Navigation reachability vs. granted access (ADR-0090 D6, issue #3583) ──
559-
// Navigation and permissions are separate metadata, so an app can expose an
560-
// object no permission set grants read on: the entry renders and the click
561-
// fails permission-denied for every user, including admins. Advisory — the
562-
// grant may come from a set another installed package ships.
563-
for (const t of validateNavAccess(config)) {
564-
issues.push({
565-
severity: t.severity,
566-
rule: t.rule,
567-
message: `${t.where}: ${t.message}`,
568-
path: t.path,
569-
fix: t.hint,
570-
});
571-
}
572-
573-
// ── Translation-bundle references & option keys (issue #3583) ──
574-
// The i18n gate only ever asked "which expected keys are missing?". This is
575-
// the reverse the spec already names (`TranslationDiffStatus 'redundant'`):
576-
// keys pointing at metadata that no longer exists, and select-option keys
577-
// written as the display label instead of the stored value. Advisory — an
578-
// orphan key is inert, it just leaves one string untranslated.
579-
for (const t of validateTranslationReferences(config)) {
493+
// ── Reference integrity (issue #3583) ──
494+
// One suite, one call site: object-name references `defineStack` does not
495+
// cover, name-bound action surfaces, page-component field bindings, chart
496+
// axes outside dashboards, navigation vs. granted access, and translation
497+
// keys pointing at metadata that no longer exists. Every member resolves a
498+
// NAME against what the stack declares — the class the HotCRM audit found
499+
// shipping, where each instance parses, validates, and fails silently.
500+
// Adding a rule to `REFERENCE_INTEGRITY_RULES` reaches this path with no
501+
// edit here (assessment §5 D5 — the wiring drift this ends).
502+
for (const t of validateReferenceIntegrity(config)) {
580503
issues.push({
581504
severity: t.severity,
582505
rule: t.rule,

packages/cli/src/commands/validate.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,7 @@ import { validateViewContainers } from '@objectstack/lint';
1414
import { validateWidgetBindings } from '@objectstack/lint';
1515
import { validateDashboardActionRefs } from '@objectstack/lint';
1616
import { validateFilterTokens } from '@objectstack/lint';
17-
import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint';
18-
import { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint';
19-
import { validateNavAccess } from '@objectstack/lint';
20-
import { validateTranslationReferences } from '@objectstack/lint';
17+
import { validateReferenceIntegrity } from '@objectstack/lint';
2118
import { validateResponsiveStyles } from '@objectstack/lint';
2219
import { validateJsxPages, validateReactPages, validateReactPageProps, validatePageSourceStyling } from '@objectstack/lint';
2320
import { validateCapabilityReferences } from '@objectstack/lint';
@@ -310,14 +307,7 @@ export default class Validate extends Command {
310307
// to nothing and renders the source string (advisory: inert, not
311308
// broken).
312309
if (!flags.json) printStep('Checking object & action references (#3583)...');
313-
const refFindings = [
314-
...validateObjectReferences(result.data as Record<string, unknown>),
315-
...validateActionNameRefs(result.data as Record<string, unknown>),
316-
...validatePageFieldBindings(result.data as Record<string, unknown>),
317-
...validateChartBindings(result.data as Record<string, unknown>),
318-
...validateNavAccess(result.data as Record<string, unknown>),
319-
...validateTranslationReferences(result.data as Record<string, unknown>),
320-
];
310+
const refFindings = validateReferenceIntegrity(result.data as Record<string, unknown>);
321311
const refErrors = refFindings.filter((f) => f.severity === 'error');
322312
const refWarnings = refFindings.filter((f) => f.severity === 'warning');
323313
if (refErrors.length > 0) {

packages/lint/src/index.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,4 +221,17 @@ export type {
221221
TranslationRefSeverity,
222222
} from './validate-translation-references.js';
223223

224+
// One entry point for the reference-resolution rules above (#3583 §5 D5).
225+
// Adding a rule to `REFERENCE_INTEGRITY_RULES` runs it on `validate`, `lint`
226+
// and `compile` at once — the CLI call sites do not change.
227+
export {
228+
validateReferenceIntegrity,
229+
REFERENCE_INTEGRITY_RULES,
230+
} from './reference-integrity-suite.js';
231+
export type {
232+
ReferenceIntegrityFinding,
233+
ReferenceIntegrityRule,
234+
ReferenceIntegritySeverity,
235+
} from './reference-integrity-suite.js';
236+
224237
export { buildAccessMatrix, diffAccessMatrix } from './build-access-matrix.js';
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import {
5+
validateReferenceIntegrity,
6+
REFERENCE_INTEGRITY_RULES,
7+
} from './reference-integrity-suite.js';
8+
import { validateObjectReferences } from './validate-object-references.js';
9+
import { validateTranslationReferences } from './validate-translation-references.js';
10+
11+
describe('reference-integrity suite — membership', () => {
12+
// Deliberately a written-out list: adding a rule to the suite should be a
13+
// conscious edit in two places (the suite and this test), not something that
14+
// slips in unreviewed. The list is also the answer to "which rules run on
15+
// `validate` / `lint` / `compile`?".
16+
it('holds exactly the reference-resolution rules, in report order', () => {
17+
expect(REFERENCE_INTEGRITY_RULES.map((r) => r.name)).toEqual([
18+
'validateObjectReferences',
19+
'validateActionNameRefs',
20+
'validatePageFieldBindings',
21+
'validateChartBindings',
22+
'validateNavAccess',
23+
'validateTranslationReferences',
24+
]);
25+
});
26+
27+
it('wires the same function the package exports', () => {
28+
const byName = new Map(REFERENCE_INTEGRITY_RULES.map((r) => [r.name, r.run]));
29+
expect(byName.get('validateObjectReferences')).toBe(validateObjectReferences);
30+
expect(byName.get('validateTranslationReferences')).toBe(validateTranslationReferences);
31+
});
32+
});
33+
34+
describe('reference-integrity suite — every member actually runs', () => {
35+
/**
36+
* One live instance per rule, so a member that silently stops being invoked
37+
* (or is dropped from the list) fails here instead of quietly narrowing what
38+
* the CLI checks. A suite that returns nothing is indistinguishable from a
39+
* clean stack — which is exactly how the dead quick-actions check in #3684
40+
* survived its own tests.
41+
*/
42+
const stack = {
43+
objects: [
44+
{
45+
name: 'crm_lead',
46+
fields: { name: { type: 'text', label: 'Name' } },
47+
permissions: {},
48+
},
49+
],
50+
actions: [
51+
// validateObjectReferences: a param pointing at an object nothing declares.
52+
{ name: 'assign', label: 'Assign', params: [{ name: 'owner', reference: 'user' }] },
53+
],
54+
views: [
55+
{
56+
list: {
57+
type: 'grid',
58+
name: 'all_leads',
59+
data: { provider: 'object', object: 'crm_lead' },
60+
// validateActionNameRefs: no such action.
61+
bulkActions: ['mass_update'],
62+
},
63+
},
64+
],
65+
pages: [
66+
{
67+
name: 'lead_detail',
68+
object: 'crm_lead',
69+
regions: [
70+
{
71+
components: [
72+
// validatePageFieldBindings: `budget` is not a field on crm_lead.
73+
{ type: 'record:highlights', properties: { fields: [{ name: 'budget' }] } },
74+
],
75+
},
76+
],
77+
},
78+
],
79+
datasets: [
80+
{
81+
name: 'lead_metrics',
82+
object: 'crm_lead',
83+
dimensions: [{ name: 'source' }],
84+
measures: [{ name: 'count_leads', aggregate: 'count' }],
85+
},
86+
],
87+
reports: [
88+
{
89+
name: 'leads_by_source',
90+
dataset: 'lead_metrics',
91+
values: ['count_leads'],
92+
// validateChartBindings: a raw field where a declared measure is required.
93+
chart: { type: 'bar', xAxis: 'source', yAxis: 'lead_score' },
94+
},
95+
],
96+
apps: [
97+
{
98+
name: 'crm_app',
99+
navigation: [{ id: 'nav_leads', type: 'object', objectName: 'crm_lead' }],
100+
},
101+
],
102+
// validateNavAccess: a declared permission set that grants nothing on the
103+
// nav-exposed object.
104+
permissions: [{ name: 'sales_user', label: 'Sales User', objects: {} }],
105+
translations: [
106+
// validateTranslationReferences: a field the object does not declare.
107+
{ en: { objects: { crm_lead: { label: 'Lead', fields: { assigned_to: { label: 'Owner' } } } } } },
108+
],
109+
};
110+
111+
it('reports at least one finding from every member', () => {
112+
const findings = validateReferenceIntegrity(stack);
113+
const rules = new Set(findings.map((f) => f.rule));
114+
115+
expect(rules).toContain('object-reference-unknown');
116+
expect(rules).toContain('action-name-undefined');
117+
expect(rules).toContain('page-field-unknown');
118+
expect(rules).toContain('chart-measure-unknown');
119+
expect(rules).toContain('nav-object-ungranted');
120+
expect(rules).toContain('translation-target-unknown');
121+
});
122+
123+
it('concatenates in list order and carries the common finding shape', () => {
124+
const findings = validateReferenceIntegrity(stack);
125+
expect(findings.length).toBeGreaterThan(0);
126+
for (const f of findings) {
127+
expect(['error', 'warning']).toContain(f.severity);
128+
expect(typeof f.rule).toBe('string');
129+
expect(typeof f.where).toBe('string');
130+
expect(typeof f.path).toBe('string');
131+
expect(typeof f.message).toBe('string');
132+
expect(typeof f.hint).toBe('string');
133+
}
134+
// Object references run first, translations last.
135+
expect(findings[0].rule).toBe('object-reference-unknown');
136+
expect(findings[findings.length - 1].rule).toBe('translation-target-unknown');
137+
});
138+
139+
it('returns nothing for an empty stack', () => {
140+
expect(validateReferenceIntegrity({})).toEqual([]);
141+
});
142+
});

0 commit comments

Comments
 (0)