Skip to content

Commit f163028

Browse files
os-zhuangclaude
andauthored
feat(lint): reference-integrity validation for object and action names (#3583) (#3657)
* feat(lint): reference-integrity validation for object and action names (#3583) The HotCRM audit found ~20 shipped instances of one bug class — metadata naming something that does not exist — all passing `objectstack validate` and `objectstack lint` cleanly and failing silently at runtime. This closes the object-name and action-name half of that class. Root cause of the false negatives: every cross-reference check answered "might this come from another package?" with a PREFIX GUESS (`startsWith('sys_')`), which cannot distinguish `sys_user` (real) from `sys_approval_process` (fictional — removed by ADR-0019, registered by nothing). So spec now ships a curated PLATFORM_PROVIDED_OBJECT_NAMES registry, kept honest by a conformance test that scans each package's *.object.ts declarations, following the PLATFORM_CAPABILITY_NAMES precedent (lint keeps its one-way lint -> spec dependency). New rules, wired into BOTH `os validate` and `os lint` in the same change so the paths cannot drift: - validate-object-references: action-param reference/objectOverride, dashboard globalFilters optionsFrom.object, nav requiresObject gates. Unresolved + unprefixed => error (the pure typo class); unresolved + platform-prefixed but unregistered => warning (a third-party package may still provide it). - validate-action-name-refs: bulkActions/rowActions, page quick-actions actionNames, nav action items. Unresolved => error, matching the existing dashboard-action-target posture; the runtime ships no built-in action names, so a miss is dead, full stop. Fixes found while mapping the surface: - defineStack skipped app.areas[].navigation entirely, so an areas-based app got NO nav cross-reference checking; it also only recursed into `group` children, missing children on `object` nav items. - i18n coverage only walked the record-map shape of field `options`, but the canonical shape is a {value,label}[] ARRAY — option-label coverage never fired for canonically-shaped select fields. - Hook conditions lost all field-awareness when `object` was an array of targets; now checked per target with de-duplicated diagnostics. - A widget binding no `dataset` silently bypassed every binding and chart check on the raw-config paths; `dataset` is schema-required, so it is now reported. Verified zero false positives against all three example apps (app-crm, app-showcase, app-todo). Suites: lint 390, spec 6669, cli 639 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBks5G7AkwrvgF2kL5rfkT * fix: address CI findings on the reference-integrity rules (#3583) - Raw NUL byte in the hook-dedup key (validate-expressions.ts) tripped `check:nul-bytes`. A raw NUL makes grep/ripgrep treat the whole file as binary and silently return zero matches, so the file drops out of code search and every grep-based lint. Written as the \u0000 escape per the repo convention — byte-identical at runtime. - CodeQL (high): the `{…}` interpolation test used /\{[^}]+\}/, which backtracks quadratically on a long run of `{` with no closing brace. Replaced with a linear two-index scan; same semantics, including the "at least one character between the braces" rule. - spec public API gained 6 exports (the platform-object registry), so the api-surface snapshot needed regenerating. 0 breaking, 6 added. Also wire the two rules into `os compile` alongside `validate`/`lint`. Every sibling reference rule (widget bindings, dashboard action refs, filter tokens) already runs there, so leaving these out would recreate the exact validate/lint/compile wiring drift the assessment flagged. Document the new checks in docs/deployment/validating-metadata.mdx — a "Dangling object and action names" section with the resolvability severity table, plus the entry-points row. Suites: lint 390, spec 6669, cli 636 passing; eslint, nul-bytes and api-surface checks clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBks5G7AkwrvgF2kL5rfkT * chore: register sys_migration after merging main (#3583) Merging main brought in the ADR-0104 deployment-migration gate, whose `sys_migration` object the platform-object registry did not list — and the conformance ratchet failed exactly as intended, which is the point of having it rather than a prefix heuristic. Also regenerates the api-surface snapshot for the union of main's new exports and this branch's. Re-verified on the merged base: lint 390, spec 6681, cli 637 passing; eslint, nul-bytes, api-surface clean; and the three example apps (app-crm, app-showcase, app-todo) still produce zero findings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBks5G7AkwrvgF2kL5rfkT --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 984396b commit f163028

22 files changed

Lines changed: 1881 additions & 20 deletions
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/lint': minor
4+
'@objectstack/cli': minor
5+
---
6+
7+
Reference-integrity validation for object and action names (issue #3583)
8+
9+
A HotCRM audit found ~20 shipped instances of one bug class — metadata naming
10+
something that does not exist — all passing `objectstack validate` / `lint`
11+
cleanly and failing silently at runtime. This closes the object-name and
12+
action-name half of that class.
13+
14+
**New — `@objectstack/spec`:** `PLATFORM_PROVIDED_OBJECT_NAMES`, a curated
15+
registry of every object name contributed by a platform package, official
16+
plugin, or the cloud runtime, plus `isPlatformProvidedObjectName()` and
17+
`hasPlatformObjectPrefix()`. This replaces the `startsWith('sys_')` prefix guess
18+
that could not tell `sys_user` (real) from `sys_approval_process` (fictional —
19+
removed by ADR-0019, registered by nothing), which is why every fictional
20+
platform-prefixed reference shipped. A conformance test scans each package's
21+
`*.object.ts` declarations and fails if the registry drifts.
22+
23+
**New lint rules** (wired into both `os validate` and `os lint`):
24+
25+
- `validate-object-references` — action-param `reference` / `objectOverride`,
26+
dashboard `globalFilters[].optionsFrom.object`, and navigation
27+
`requiresObject` gates. Severity follows resolvability: an unresolved
28+
*unprefixed* name is a typo (**error**`object: 'user'` where the platform
29+
object is `sys_user`); an unresolved *platform-prefixed* name is **advisory**,
30+
since a third-party package may still provide it.
31+
- `validate-action-name-refs` — the surfaces that bind an action BY NAME:
32+
list-view `bulkActions` / `rowActions`, page `record:quick_actions`
33+
`actionNames`, and nav action items. A name matching no defined action is an
34+
**error** (the button renders and does nothing), matching the existing
35+
dashboard-action-target rule.
36+
37+
**Fixes:**
38+
39+
- `defineStack` cross-reference validation now walks `app.areas[].navigation`
40+
an areas-based app previously got no navigation checking at all — and recurses
41+
into `children` on `object` nav items, not only `group` ones.
42+
- `os lint` i18n coverage now reads field `options` in the canonical
43+
`{value,label}[]` array shape; it only handled the record map, so option-label
44+
coverage silently never fired for canonically-shaped select fields.
45+
- Hook `condition` expressions are now field-checked when `object` is an ARRAY
46+
of targets (previously only a single string target was checked, so a
47+
multi-target hook filtering on a nonexistent field passed clean). Per-target
48+
diagnostics are de-duplicated.
49+
- A dashboard widget binding no `dataset` at all is now reported instead of
50+
silently bypassing every binding and chart check on the raw-config
51+
(`lint`/`doctor`) paths. `dataset` is schema-required, so this matches what
52+
the parsed paths already enforce.

content/docs/deployment/validating-metadata.mdx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,39 @@ header: {
8383
whose `objects/reports/dashboards/pages/views` route is unregistered is **warned**
8484
(external, interpolated, and opaque routes are skipped).
8585

86+
### 4. Dangling object and action names
87+
88+
The same gate covers the reference sites that are plain strings in the schema:
89+
an action param's record-picker target, a dashboard filter's options source, a
90+
navigation capability gate, and every surface that binds an action **by name**
91+
(`bulkActions` / `rowActions`, a page's `record:quick_actions`, a nav action item).
92+
93+
```ts
94+
// ✗ the platform user object is `sys_user` — `user` resolves to nothing → error
95+
{ name: 'owner', type: 'lookup', reference: 'user' }
96+
97+
// ✗ no action named `mass_update` is defined anywhere → error
98+
defineView({ /**/ bulkActions: ['mass_update', 'mass_delete'] })
99+
100+
// ⚠ platform-shaped, but no known package registers it → warning
101+
{ id: 'nav_approvals', type: 'object', objectName: 'sys_approval_process',
102+
requiresObject: 'sys_approval_process' }
103+
```
104+
105+
Severity follows **resolvability**, because "this might be provided by another
106+
installed package" is a real possibility that must not be guessed at:
107+
108+
| The name… | Result |
109+
|---|---|
110+
| resolves to one of your own objects ||
111+
| is unresolved and carries no platform prefix | **error** — your objects are namespace-prefixed and present in the stack, so there is no legitimate elsewhere. This is the typo class (`user` for `sys_user`). |
112+
| resolves to a known platform / plugin / cloud object ||
113+
| carries a platform prefix but no known package registers it | **warning** — a third-party package may still provide it, so this stays advisory |
114+
115+
Interpolated targets (`${…}`, `{…}`) are skipped — they resolve at render time.
116+
Action names get no third-party softening: the runtime ships no built-in action
117+
names, so a name resolving nowhere is always an **error**.
118+
86119
## The one gate, two entry points
87120

88121
`os validate` and `os build` (alias of `os compile`) run the **same** validator:
@@ -93,6 +126,7 @@ whose `objects/reports/dashboards/pages/views` route is unregistered is **warned
93126
| CEL / predicate validation |||
94127
| Widget-binding integrity |||
95128
| Dashboard action/route references (ADR-0049) |||
129+
| Object & action name references (#3583) |||
96130
| Security posture (ADR-0090 — e.g. every custom object declares `sharingModel`) |||
97131
| Emits `dist/objectstack.json` |||
98132

packages/cli/src/commands/compile.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +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';
1617
import { validateResponsiveStyles } from '@objectstack/lint';
1718
import { validateSecurityPosture, validateOrgAxisRedLines, buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
1819
import { validateReadonlyFlowWrites } from '@objectstack/lint';
@@ -319,6 +320,43 @@ export default class Compile extends Command {
319320
this.exit(1);
320321
}
321322

323+
// 3b-bis. Object & action name references (#3583) — the reference sites
324+
// `defineStack` does not cover: action-param `reference` /
325+
// `objectOverride`, dashboard filter `optionsFrom.object`, nav
326+
// `requiresObject` gates, and the name-bound action surfaces
327+
// (`bulkActions`/`rowActions`, page quick-actions, nav action items).
328+
// All plain strings in the schema, so a name resolving to nothing
329+
// ships and fails silently. Errors fail the build; the
330+
// platform-prefixed-but-unregistered case is advisory (a third-party
331+
// package may still provide it).
332+
if (!flags.json) printStep('Checking object & action references (#3583)...');
333+
const refFindings = [
334+
...validateObjectReferences(result.data as Record<string, unknown>),
335+
...validateActionNameRefs(result.data as Record<string, unknown>),
336+
];
337+
const refErrors = refFindings.filter((f) => f.severity === 'error');
338+
const refWarnings = refFindings.filter((f) => f.severity === 'warning');
339+
if (refErrors.length > 0) {
340+
if (flags.json) {
341+
console.log(JSON.stringify({ success: false, error: 'reference integrity validation failed', issues: refErrors }));
342+
this.exit(1);
343+
}
344+
console.log('');
345+
printError(`Reference integrity check failed (${refErrors.length} issue${refErrors.length > 1 ? 's' : ''})`);
346+
for (const f of refErrors.slice(0, 50)) {
347+
console.log(` • ${f.where}: ${f.message}`);
348+
console.log(chalk.dim(` ${f.hint}`));
349+
console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`));
350+
}
351+
this.exit(1);
352+
}
353+
if (!flags.json) {
354+
for (const w of refWarnings.slice(0, 50)) {
355+
console.log(chalk.yellow(` ⚠ ${w.where}: ${w.message}`));
356+
console.log(chalk.dim(` ${w.hint}`));
357+
}
358+
}
359+
322360
// 3c. SDUI scoped-styling correctness (ADR-0065) — a styled node without
323361
// an `id` drops its CSS silently; Tailwind-in-className does nothing
324362
// from metadata. Same bar for hand-authored and AI-generated pages

packages/cli/src/commands/lint.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +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';
1314
import { collectAndLintDocs } from '../utils/collect-docs.js';
1415
import { scoreMetadata } from '../lint/score.js';
1516
import { runMetadataEval } from '../lint/metadata-eval.js';
@@ -487,6 +488,38 @@ export function lintConfig(config: any): LintIssue[] {
487488
});
488489
}
489490

491+
// ── Object-name references (issue #3583) ──
492+
// The reference sites `defineStack` does not cover: action-param
493+
// `reference`/`objectOverride`, dashboard filter `optionsFrom.object`, and
494+
// navigation `requiresObject` gates. An unprefixed miss (`user` for
495+
// `sys_user`) is a typo → error; a platform-prefixed name no known package
496+
// registers (`sys_approval_process`) is advisory, since a third-party
497+
// package may still provide it.
498+
for (const t of validateObjectReferences(config)) {
499+
issues.push({
500+
severity: t.severity,
501+
rule: t.rule,
502+
message: `${t.where}: ${t.message}`,
503+
path: t.path,
504+
fix: t.hint,
505+
});
506+
}
507+
508+
// ── Action-name references (issue #3583) ──
509+
// `bulkActions`/`rowActions`, page `quick_actions`, and nav action items bind
510+
// an action BY NAME. A name matching no defined action ships a button that
511+
// renders and does nothing — same failure `validate-dashboard-action-refs`
512+
// catches for dashboards, so the same severity.
513+
for (const t of validateActionNameRefs(config)) {
514+
issues.push({
515+
severity: t.severity,
516+
rule: t.rule,
517+
message: `${t.where}: ${t.message}`,
518+
path: t.path,
519+
fix: t.hint,
520+
});
521+
}
522+
490523
return issues;
491524
}
492525

packages/cli/src/commands/validate.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +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';
1718
import { validateResponsiveStyles } from '@objectstack/lint';
1819
import { validateJsxPages, validateReactPages, validateReactPageProps, validatePageSourceStyling } from '@objectstack/lint';
1920
import { validateCapabilityReferences } from '@objectstack/lint';
@@ -281,6 +282,48 @@ export default class Validate extends Command {
281282
this.exit(1);
282283
}
283284

285+
// 3a-quater. Object-name + action-name reference integrity (#3583). The
286+
// reference sites `defineStack` does not cover: action-param
287+
// `reference`/`objectOverride`, dashboard filter `optionsFrom.object`,
288+
// nav `requiresObject` gates, and the name-bound action surfaces
289+
// (`bulkActions`/`rowActions`, page quick-actions, nav action items).
290+
// All are plain strings in the schema, so a name resolving to nothing
291+
// parses, ships, and fails silently at runtime. An unprefixed miss is
292+
// a typo (error); a platform-prefixed name no known package registers
293+
// is advisory (a third-party package may still provide it).
294+
if (!flags.json) printStep('Checking object & action references (#3583)...');
295+
const refFindings = [
296+
...validateObjectReferences(result.data as Record<string, unknown>),
297+
...validateActionNameRefs(result.data as Record<string, unknown>),
298+
];
299+
const refErrors = refFindings.filter((f) => f.severity === 'error');
300+
const refWarnings = refFindings.filter((f) => f.severity === 'warning');
301+
if (refErrors.length > 0) {
302+
if (flags.json) {
303+
console.log(JSON.stringify({
304+
valid: false,
305+
errors: refErrors,
306+
warnings: [...widgetWarnings, ...actionRefWarnings, ...refWarnings],
307+
duration: timer.elapsed(),
308+
}, null, 2));
309+
this.exit(1);
310+
}
311+
console.log('');
312+
printError(`Reference integrity check failed (${refErrors.length} issue${refErrors.length > 1 ? 's' : ''})`);
313+
for (const f of refErrors.slice(0, 50)) {
314+
console.log(` • ${f.where}: ${f.message}`);
315+
console.log(chalk.dim(` ${f.hint}`));
316+
console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`));
317+
}
318+
this.exit(1);
319+
}
320+
if (!flags.json) {
321+
for (const w of refWarnings.slice(0, 50)) {
322+
console.log(chalk.yellow(` ⚠ ${w.where}: ${w.message}`));
323+
console.log(chalk.dim(` ${w.hint}`));
324+
}
325+
}
326+
284327
// 3b. SDUI scoped-styling correctness (ADR-0065) — a styled node's
285328
// responsiveStyles must be scopable (needs an `id`), reference real
286329
// CSS properties + design tokens, and carry a `large` base;

packages/cli/src/utils/i18n-coverage.ts

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -209,19 +209,32 @@ function collectExpectedKeys(config: any): ExpectedKey[] {
209209
`Field ${objectName}.${fieldName} label`,
210210
inlineText(field?.label),
211211
);
212+
// Options — accept BOTH shapes, exactly as `i18n-extract.ts` does.
213+
// `FieldSchema.options` is canonically a `{value, label}[]` ARRAY, but
214+
// this only ever handled the record map, so option coverage silently
215+
// never fired for a canonically-shaped select field (issue #3583).
212216
const opts = field?.options;
213-
if (opts && typeof opts === 'object' && !Array.isArray(opts)) {
214-
for (const [optionKey, optionLabel] of Object.entries<any>(opts)) {
215-
// Mirrors the extractor: an option's source text is its label, or
216-
// its own value when the map holds no label string.
217-
pushKey(
218-
keys,
219-
['objects', objectName, 'fields', fieldName, 'options', optionKey],
220-
'option',
221-
`Option ${objectName}.${fieldName}.${optionKey}`,
222-
inlineText(optionLabel) ?? optionKey,
223-
);
224-
}
217+
const optionEntries: Array<[string, unknown]> = Array.isArray(opts)
218+
? opts.flatMap((opt: any) =>
219+
opt && typeof opt === 'object' && 'value' in opt
220+
? [[String(opt.value), opt.label ?? opt.value] as [string, unknown]]
221+
: typeof opt === 'string'
222+
? [[opt, opt] as [string, unknown]]
223+
: [],
224+
)
225+
: opts && typeof opts === 'object'
226+
? Object.entries<any>(opts)
227+
: [];
228+
for (const [optionKey, optionLabel] of optionEntries) {
229+
// Mirrors the extractor: an option's source text is its label, or
230+
// its own value when no label string is present.
231+
pushKey(
232+
keys,
233+
['objects', objectName, 'fields', fieldName, 'options', optionKey],
234+
'option',
235+
`Option ${objectName}.${fieldName}.${optionKey}`,
236+
inlineText(optionLabel) ?? optionKey,
237+
);
225238
}
226239
}
227240
}

packages/cli/test/i18n-coverage.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,51 @@ describe('computeI18nCoverage', () => {
129129
expect(zhKeys.has('globalActions.export_csv.successMessage')).toBe(true);
130130
});
131131

132+
// Issue #3583 — `FieldSchema.options` is canonically a `{value,label}[]`
133+
// ARRAY, but coverage only walked the record-map shape, so option-label
134+
// coverage silently never fired for a canonically-shaped select field.
135+
it('covers options declared in the canonical array shape', () => {
136+
const arrayOptionConfig: any = {
137+
objects: [
138+
{
139+
name: 'account',
140+
label: 'Account',
141+
fields: {
142+
stage: {
143+
label: 'Stage',
144+
options: [
145+
{ value: 'planning', label: 'Planning' },
146+
{ value: 'direct_mail', label: 'Direct Mail' },
147+
],
148+
},
149+
},
150+
},
151+
],
152+
translations: [{ 'zh-CN': {} }],
153+
};
154+
const report = computeI18nCoverage(arrayOptionConfig, { defaultLocale: 'en' });
155+
const zhKeys = new Set(report.issues.filter((i) => i.locale === 'zh-CN').map((i) => i.key));
156+
expect(zhKeys.has('objects.account.fields.stage.options.planning')).toBe(true);
157+
expect(zhKeys.has('objects.account.fields.stage.options.direct_mail')).toBe(true);
158+
});
159+
160+
it('covers options declared as a bare string array', () => {
161+
const stringOptionConfig: any = {
162+
objects: [
163+
{
164+
name: 'account',
165+
label: 'Account',
166+
fields: { stage: { label: 'Stage', options: ['planning', 'closed'] } },
167+
},
168+
],
169+
translations: [{ 'zh-CN': {} }],
170+
};
171+
const report = computeI18nCoverage(stringOptionConfig, { defaultLocale: 'en' });
172+
const zhKeys = new Set(report.issues.filter((i) => i.locale === 'zh-CN').map((i) => i.key));
173+
expect(zhKeys.has('objects.account.fields.stage.options.planning')).toBe(true);
174+
expect(zhKeys.has('objects.account.fields.stage.options.closed')).toBe(true);
175+
});
176+
132177
it('promotes warnings to errors under --strict', () => {
133178
const report = computeI18nCoverage(baseConfig, { defaultLocale: 'en', strict: true });
134179
const zhIssues = report.issues.filter((i) => i.locale === 'zh-CN');

packages/lint/src/index.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,4 +181,14 @@ export type {
181181
export { validateFilterTokens, FILTER_TOKEN_UNKNOWN } from './validate-filter-tokens.js';
182182
export type { FilterTokenFinding, FilterTokenSeverity } from './validate-filter-tokens.js';
183183

184+
export {
185+
validateObjectReferences,
186+
OBJECT_REFERENCE_UNKNOWN,
187+
OBJECT_REFERENCE_UNREGISTERED_PLATFORM,
188+
} from './validate-object-references.js';
189+
export type { ObjectRefFinding, ObjectRefSeverity } from './validate-object-references.js';
190+
191+
export { validateActionNameRefs, ACTION_NAME_UNDEFINED } from './validate-action-name-refs.js';
192+
export type { ActionNameRefFinding, ActionNameRefSeverity } from './validate-action-name-refs.js';
193+
184194
export { buildAccessMatrix, diffAccessMatrix } from './build-access-matrix.js';

0 commit comments

Comments
 (0)