diff --git a/.changeset/reference-integrity-object-and-action-names.md b/.changeset/reference-integrity-object-and-action-names.md new file mode 100644 index 0000000000..4125c9cf4f --- /dev/null +++ b/.changeset/reference-integrity-object-and-action-names.md @@ -0,0 +1,52 @@ +--- +'@objectstack/spec': minor +'@objectstack/lint': minor +'@objectstack/cli': minor +--- + +Reference-integrity validation for object and action names (issue #3583) + +A HotCRM audit found ~20 shipped instances of one bug class — metadata naming +something that does not exist — all passing `objectstack validate` / `lint` +cleanly and failing silently at runtime. This closes the object-name and +action-name half of that class. + +**New — `@objectstack/spec`:** `PLATFORM_PROVIDED_OBJECT_NAMES`, a curated +registry of every object name contributed by a platform package, official +plugin, or the cloud runtime, plus `isPlatformProvidedObjectName()` and +`hasPlatformObjectPrefix()`. This replaces the `startsWith('sys_')` prefix guess +that could not tell `sys_user` (real) from `sys_approval_process` (fictional — +removed by ADR-0019, registered by nothing), which is why every fictional +platform-prefixed reference shipped. A conformance test scans each package's +`*.object.ts` declarations and fails if the registry drifts. + +**New lint rules** (wired into both `os validate` and `os lint`): + +- `validate-object-references` — action-param `reference` / `objectOverride`, + dashboard `globalFilters[].optionsFrom.object`, and navigation + `requiresObject` gates. Severity follows resolvability: an unresolved + *unprefixed* name is a typo (**error** — `object: 'user'` where the platform + object is `sys_user`); an unresolved *platform-prefixed* name is **advisory**, + since a third-party package may still provide it. +- `validate-action-name-refs` — the surfaces that bind an action BY NAME: + list-view `bulkActions` / `rowActions`, page `record:quick_actions` + `actionNames`, and nav action items. A name matching no defined action is an + **error** (the button renders and does nothing), matching the existing + dashboard-action-target rule. + +**Fixes:** + +- `defineStack` cross-reference validation now walks `app.areas[].navigation` — + an areas-based app previously got no navigation checking at all — and recurses + into `children` on `object` nav items, not only `group` ones. +- `os lint` i18n coverage now reads field `options` in the canonical + `{value,label}[]` array shape; it only handled the record map, so option-label + coverage silently never fired for canonically-shaped select fields. +- Hook `condition` expressions are now field-checked when `object` is an ARRAY + of targets (previously only a single string target was checked, so a + multi-target hook filtering on a nonexistent field passed clean). Per-target + diagnostics are de-duplicated. +- A dashboard widget binding no `dataset` at all is now reported instead of + silently bypassing every binding and chart check on the raw-config + (`lint`/`doctor`) paths. `dataset` is schema-required, so this matches what + the parsed paths already enforce. diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index 2d1fbcff84..6839f2ca12 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -83,6 +83,39 @@ header: { whose `objects/reports/dashboards/pages/views` route is unregistered is **warned** (external, interpolated, and opaque routes are skipped). +### 4. Dangling object and action names + +The same gate covers the reference sites that are plain strings in the schema: +an action param's record-picker target, a dashboard filter's options source, a +navigation capability gate, and every surface that binds an action **by name** +(`bulkActions` / `rowActions`, a page's `record:quick_actions`, a nav action item). + +```ts +// ✗ the platform user object is `sys_user` — `user` resolves to nothing → error +{ name: 'owner', type: 'lookup', reference: 'user' } + +// ✗ no action named `mass_update` is defined anywhere → error +defineView({ /* … */ bulkActions: ['mass_update', 'mass_delete'] }) + +// ⚠ platform-shaped, but no known package registers it → warning +{ id: 'nav_approvals', type: 'object', objectName: 'sys_approval_process', + requiresObject: 'sys_approval_process' } +``` + +Severity follows **resolvability**, because "this might be provided by another +installed package" is a real possibility that must not be guessed at: + +| The name… | Result | +|---|---| +| resolves to one of your own objects | ✓ | +| 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`). | +| resolves to a known platform / plugin / cloud object | ✓ | +| carries a platform prefix but no known package registers it | **warning** — a third-party package may still provide it, so this stays advisory | + +Interpolated targets (`${…}`, `{…}`) are skipped — they resolve at render time. +Action names get no third-party softening: the runtime ships no built-in action +names, so a name resolving nowhere is always an **error**. + ## The one gate, two entry points `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 | CEL / predicate validation | ✓ | ✓ | | Widget-binding integrity | ✓ | ✓ | | Dashboard action/route references (ADR-0049) | ✓ | ✓ | +| Object & action name references (#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 46d1eb207b..d49a1641b4 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -13,6 +13,7 @@ import { validateVisibilityPredicates } from '@objectstack/lint'; import { validateWidgetBindings } from '@objectstack/lint'; import { validateDashboardActionRefs } from '@objectstack/lint'; import { validateFilterTokens } from '@objectstack/lint'; +import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint'; import { validateResponsiveStyles } from '@objectstack/lint'; import { validateSecurityPosture, validateOrgAxisRedLines, buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint'; import { validateReadonlyFlowWrites } from '@objectstack/lint'; @@ -319,6 +320,43 @@ export default class Compile extends Command { this.exit(1); } + // 3b-bis. Object & action name references (#3583) — the reference sites + // `defineStack` does not cover: action-param `reference` / + // `objectOverride`, dashboard filter `optionsFrom.object`, nav + // `requiresObject` gates, and the name-bound action surfaces + // (`bulkActions`/`rowActions`, page quick-actions, nav action items). + // 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 + // package may still provide it). + if (!flags.json) printStep('Checking object & action references (#3583)...'); + const refFindings = [ + ...validateObjectReferences(result.data as Record), + ...validateActionNameRefs(result.data as Record), + ]; + const refErrors = refFindings.filter((f) => f.severity === 'error'); + const refWarnings = refFindings.filter((f) => f.severity === 'warning'); + if (refErrors.length > 0) { + if (flags.json) { + console.log(JSON.stringify({ success: false, error: 'reference integrity validation failed', issues: refErrors })); + this.exit(1); + } + console.log(''); + printError(`Reference integrity check failed (${refErrors.length} issue${refErrors.length > 1 ? 's' : ''})`); + for (const f of refErrors.slice(0, 50)) { + console.log(` • ${f.where}: ${f.message}`); + console.log(chalk.dim(` ${f.hint}`)); + console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`)); + } + this.exit(1); + } + if (!flags.json) { + for (const w of refWarnings.slice(0, 50)) { + console.log(chalk.yellow(` ⚠ ${w.where}: ${w.message}`)); + console.log(chalk.dim(` ${w.hint}`)); + } + } + // 3c. SDUI scoped-styling correctness (ADR-0065) — a styled node without // an `id` drops its CSS silently; Tailwind-in-className does nothing // from metadata. Same bar for hand-authored and AI-generated pages diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index 3917482e7a..2dcc965d56 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -10,6 +10,7 @@ import { computeI18nCoverage, type CoverageIssue } from '../utils/i18n-coverage. 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 { collectAndLintDocs } from '../utils/collect-docs.js'; import { scoreMetadata } from '../lint/score.js'; import { runMetadataEval } from '../lint/metadata-eval.js'; @@ -487,6 +488,38 @@ export function lintConfig(config: any): LintIssue[] { }); } + // ── Object-name references (issue #3583) ── + // The reference sites `defineStack` does not cover: action-param + // `reference`/`objectOverride`, dashboard filter `optionsFrom.object`, and + // navigation `requiresObject` gates. An unprefixed miss (`user` for + // `sys_user`) is a typo → error; a platform-prefixed name no known package + // registers (`sys_approval_process`) is advisory, since a third-party + // package may still provide it. + for (const t of validateObjectReferences(config)) { + issues.push({ + severity: t.severity, + rule: t.rule, + message: `${t.where}: ${t.message}`, + path: t.path, + fix: t.hint, + }); + } + + // ── Action-name references (issue #3583) ── + // `bulkActions`/`rowActions`, page `quick_actions`, and nav action items bind + // an action BY NAME. A name matching no defined action ships a button that + // renders and does nothing — same failure `validate-dashboard-action-refs` + // catches for dashboards, so the same severity. + for (const t of validateActionNameRefs(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 7df47faa25..5381717603 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -14,6 +14,7 @@ import { validateViewContainers } from '@objectstack/lint'; import { validateWidgetBindings } from '@objectstack/lint'; import { validateDashboardActionRefs } from '@objectstack/lint'; import { validateFilterTokens } from '@objectstack/lint'; +import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint'; import { validateResponsiveStyles } from '@objectstack/lint'; import { validateJsxPages, validateReactPages, validateReactPageProps, validatePageSourceStyling } from '@objectstack/lint'; import { validateCapabilityReferences } from '@objectstack/lint'; @@ -281,6 +282,48 @@ export default class Validate extends Command { this.exit(1); } + // 3a-quater. Object-name + action-name reference integrity (#3583). The + // reference sites `defineStack` does not cover: action-param + // `reference`/`objectOverride`, dashboard filter `optionsFrom.object`, + // nav `requiresObject` gates, and the name-bound action surfaces + // (`bulkActions`/`rowActions`, page quick-actions, nav action items). + // 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 + // is advisory (a third-party package may still provide it). + if (!flags.json) printStep('Checking object & action references (#3583)...'); + const refFindings = [ + ...validateObjectReferences(result.data as Record), + ...validateActionNameRefs(result.data as Record), + ]; + const refErrors = refFindings.filter((f) => f.severity === 'error'); + const refWarnings = refFindings.filter((f) => f.severity === 'warning'); + if (refErrors.length > 0) { + if (flags.json) { + console.log(JSON.stringify({ + valid: false, + errors: refErrors, + warnings: [...widgetWarnings, ...actionRefWarnings, ...refWarnings], + duration: timer.elapsed(), + }, null, 2)); + this.exit(1); + } + console.log(''); + printError(`Reference integrity check failed (${refErrors.length} issue${refErrors.length > 1 ? 's' : ''})`); + for (const f of refErrors.slice(0, 50)) { + console.log(` • ${f.where}: ${f.message}`); + console.log(chalk.dim(` ${f.hint}`)); + console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`)); + } + this.exit(1); + } + if (!flags.json) { + for (const w of refWarnings.slice(0, 50)) { + console.log(chalk.yellow(` ⚠ ${w.where}: ${w.message}`)); + console.log(chalk.dim(` ${w.hint}`)); + } + } + // 3b. SDUI scoped-styling correctness (ADR-0065) — a styled node's // responsiveStyles must be scopable (needs an `id`), reference real // CSS properties + design tokens, and carry a `large` base; diff --git a/packages/cli/src/utils/i18n-coverage.ts b/packages/cli/src/utils/i18n-coverage.ts index c655a67905..854d3bf023 100644 --- a/packages/cli/src/utils/i18n-coverage.ts +++ b/packages/cli/src/utils/i18n-coverage.ts @@ -209,19 +209,32 @@ function collectExpectedKeys(config: any): ExpectedKey[] { `Field ${objectName}.${fieldName} label`, inlineText(field?.label), ); + // Options — accept BOTH shapes, exactly as `i18n-extract.ts` does. + // `FieldSchema.options` is canonically a `{value, label}[]` ARRAY, but + // this only ever handled the record map, so option coverage silently + // never fired for a canonically-shaped select field (issue #3583). const opts = field?.options; - if (opts && typeof opts === 'object' && !Array.isArray(opts)) { - for (const [optionKey, optionLabel] of Object.entries(opts)) { - // Mirrors the extractor: an option's source text is its label, or - // its own value when the map holds no label string. - pushKey( - keys, - ['objects', objectName, 'fields', fieldName, 'options', optionKey], - 'option', - `Option ${objectName}.${fieldName}.${optionKey}`, - inlineText(optionLabel) ?? optionKey, - ); - } + const optionEntries: Array<[string, unknown]> = Array.isArray(opts) + ? opts.flatMap((opt: any) => + opt && typeof opt === 'object' && 'value' in opt + ? [[String(opt.value), opt.label ?? opt.value] as [string, unknown]] + : typeof opt === 'string' + ? [[opt, opt] as [string, unknown]] + : [], + ) + : opts && typeof opts === 'object' + ? Object.entries(opts) + : []; + for (const [optionKey, optionLabel] of optionEntries) { + // Mirrors the extractor: an option's source text is its label, or + // its own value when no label string is present. + pushKey( + keys, + ['objects', objectName, 'fields', fieldName, 'options', optionKey], + 'option', + `Option ${objectName}.${fieldName}.${optionKey}`, + inlineText(optionLabel) ?? optionKey, + ); } } } diff --git a/packages/cli/test/i18n-coverage.test.ts b/packages/cli/test/i18n-coverage.test.ts index 7cb564cd90..4291bc53b7 100644 --- a/packages/cli/test/i18n-coverage.test.ts +++ b/packages/cli/test/i18n-coverage.test.ts @@ -129,6 +129,51 @@ describe('computeI18nCoverage', () => { expect(zhKeys.has('globalActions.export_csv.successMessage')).toBe(true); }); + // Issue #3583 — `FieldSchema.options` is canonically a `{value,label}[]` + // ARRAY, but coverage only walked the record-map shape, so option-label + // coverage silently never fired for a canonically-shaped select field. + it('covers options declared in the canonical array shape', () => { + const arrayOptionConfig: any = { + objects: [ + { + name: 'account', + label: 'Account', + fields: { + stage: { + label: 'Stage', + options: [ + { value: 'planning', label: 'Planning' }, + { value: 'direct_mail', label: 'Direct Mail' }, + ], + }, + }, + }, + ], + translations: [{ 'zh-CN': {} }], + }; + const report = computeI18nCoverage(arrayOptionConfig, { defaultLocale: 'en' }); + const zhKeys = new Set(report.issues.filter((i) => i.locale === 'zh-CN').map((i) => i.key)); + expect(zhKeys.has('objects.account.fields.stage.options.planning')).toBe(true); + expect(zhKeys.has('objects.account.fields.stage.options.direct_mail')).toBe(true); + }); + + it('covers options declared as a bare string array', () => { + const stringOptionConfig: any = { + objects: [ + { + name: 'account', + label: 'Account', + fields: { stage: { label: 'Stage', options: ['planning', 'closed'] } }, + }, + ], + translations: [{ 'zh-CN': {} }], + }; + const report = computeI18nCoverage(stringOptionConfig, { defaultLocale: 'en' }); + const zhKeys = new Set(report.issues.filter((i) => i.locale === 'zh-CN').map((i) => i.key)); + expect(zhKeys.has('objects.account.fields.stage.options.planning')).toBe(true); + expect(zhKeys.has('objects.account.fields.stage.options.closed')).toBe(true); + }); + it('promotes warnings to errors under --strict', () => { const report = computeI18nCoverage(baseConfig, { defaultLocale: 'en', strict: true }); const zhIssues = report.issues.filter((i) => i.locale === 'zh-CN'); diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index db3b7a45d5..8f7c025eef 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -181,4 +181,14 @@ export type { export { validateFilterTokens, FILTER_TOKEN_UNKNOWN } from './validate-filter-tokens.js'; export type { FilterTokenFinding, FilterTokenSeverity } from './validate-filter-tokens.js'; +export { + validateObjectReferences, + OBJECT_REFERENCE_UNKNOWN, + OBJECT_REFERENCE_UNREGISTERED_PLATFORM, +} from './validate-object-references.js'; +export type { ObjectRefFinding, ObjectRefSeverity } from './validate-object-references.js'; + +export { validateActionNameRefs, ACTION_NAME_UNDEFINED } from './validate-action-name-refs.js'; +export type { ActionNameRefFinding, ActionNameRefSeverity } from './validate-action-name-refs.js'; + export { buildAccessMatrix, diffAccessMatrix } from './build-access-matrix.js'; diff --git a/packages/lint/src/validate-action-name-refs.test.ts b/packages/lint/src/validate-action-name-refs.test.ts new file mode 100644 index 0000000000..f957ea71c0 --- /dev/null +++ b/packages/lint/src/validate-action-name-refs.test.ts @@ -0,0 +1,187 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { validateActionNameRefs, ACTION_NAME_UNDEFINED } from './validate-action-name-refs.js'; + +const withActions = () => ({ + objects: [{ name: 'crm_lead', fields: { name: { type: 'text' } } }], + actions: [{ name: 'crm_convert_lead', label: 'Convert', type: 'script' }], +}); + +describe('validateActionNameRefs — list view bulk/row actions', () => { + // The literal HotCRM instance: three bulk actions, none of them defined. + it('errors on every undefined bulkAction', () => { + const findings = validateActionNameRefs({ + ...withActions(), + views: [ + { + name: 'crm_lead', + list: { bulkActions: ['mass_update', 'mass_delete', 'assign_owner'] }, + }, + ], + }); + expect(findings).toHaveLength(3); + expect(findings.every((f) => f.severity === 'error')).toBe(true); + expect(findings.every((f) => f.rule === ACTION_NAME_UNDEFINED)).toBe(true); + expect(findings.map((f) => f.path)).toEqual([ + 'views[0].list.bulkActions[0]', + 'views[0].list.bulkActions[1]', + 'views[0].list.bulkActions[2]', + ]); + expect(findings[0].message).toContain('does nothing when clicked'); + }); + + it('errors on an undefined rowAction', () => { + const findings = validateActionNameRefs({ + ...withActions(), + views: [{ name: 'crm_lead', list: { rowActions: ['complete_task'] } }], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('views[0].list.rowActions[0]'); + }); + + it('accepts a defined action, global or object-embedded', () => { + const stack = { + objects: [ + { + name: 'crm_lead', + fields: {}, + actions: [{ name: 'crm_mark_hot', label: 'Mark hot', type: 'script' }], + }, + ], + actions: [{ name: 'crm_convert_lead', label: 'Convert', type: 'script' }], + views: [ + { name: 'crm_lead', list: { rowActions: ['crm_convert_lead'], bulkActions: ['crm_mark_hot'] } }, + ], + }; + expect(validateActionNameRefs(stack)).toEqual([]); + }); + + it('checks each named listViews entry', () => { + const findings = validateActionNameRefs({ + ...withActions(), + views: [ + { + name: 'crm_lead', + listViews: { + all: { bulkActions: ['crm_convert_lead'] }, + hot: { bulkActions: ['ghost_action'] }, + }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('views[0].listViews.hot.bulkActions[0]'); + }); + + it('offers a did-you-mean for a near-miss', () => { + const findings = validateActionNameRefs({ + ...withActions(), + views: [{ name: 'crm_lead', list: { bulkActions: ['crm_convert_leads'] } }], + }); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('Did you mean "crm_convert_lead"?'); + }); +}); + +describe('validateActionNameRefs — page quick actions', () => { + it('errors on an undefined actionNames entry', () => { + const findings = validateActionNameRefs({ + ...withActions(), + pages: [ + { + name: 'lead_record', + 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]'); + }); + + it('walks nested component children', () => { + const findings = validateActionNameRefs({ + ...withActions(), + pages: [ + { + name: 'p', + components: [ + { + type: 'layout:columns', + 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]'); + }); +}); + +describe('validateActionNameRefs — navigation action items', () => { + it('errors on an undefined nav actionName', () => { + const findings = validateActionNameRefs({ + ...withActions(), + apps: [ + { + name: 'crm', + navigation: [ + { id: 'nav_new', type: 'action', label: 'New', actionDef: { actionName: 'ghost_action' } }, + ], + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('apps[0].navigation[0].actionDef.actionName'); + }); + + it('accepts a defined nav actionName and walks areas', () => { + const findings = validateActionNameRefs({ + ...withActions(), + apps: [ + { + name: 'crm', + areas: [ + { + id: 'a', + label: 'A', + navigation: [ + { id: 'nav_c', type: 'action', label: 'Convert', actionDef: { actionName: 'crm_convert_lead' } }, + ], + }, + ], + }, + ], + }); + expect(findings).toEqual([]); + }); +}); + +describe('validateActionNameRefs — floor', () => { + it('is silent on a clean stack and tolerates empty input', () => { + expect(validateActionNameRefs(withActions())).toEqual([]); + expect(validateActionNameRefs({})).toEqual([]); + expect(validateActionNameRefs(null as unknown as Record)).toEqual([]); + }); + + it('ignores a non-action nav item that happens to carry an actionDef', () => { + const findings = validateActionNameRefs({ + ...withActions(), + apps: [ + { + name: 'crm', + navigation: [ + { id: 'nav_o', type: 'object', label: 'Leads', objectName: 'crm_lead', actionDef: { actionName: 'ghost' } }, + ], + }, + ], + }); + expect(findings).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-action-name-refs.ts b/packages/lint/src/validate-action-name-refs.ts new file mode 100644 index 0000000000..1c8c1a44ed --- /dev/null +++ b/packages/lint/src/validate-action-name-refs.ts @@ -0,0 +1,254 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0049 — references] Action-NAME reference integrity for the surfaces that + * bind an action by name (issue #3583). + * + * `locations` is an action's primary binding, but several surfaces reference + * actions **by name** instead (see `content/docs/ui/actions.mdx` — "Surfaces can + * also reference actions by name"). Every one of those fields is a plain + * `z.array(z.string())` / `z.string()`, so a name that matches no defined action + * parses and ships: + * + * - list views — `rowActions[]` / `bulkActions[]` (both the default `list` + * container and each `listViews.` entry) + * - page components — `record:quick_actions` → `properties.actionNames[]` + * - app navigation — `{ type: 'action', actionDef: { actionName } }` + * + * The HotCRM audit shipped `bulkActions: ['mass_update', 'mass_delete', + * 'assign_owner']` with none of the three defined anywhere: the toolbar renders + * the buttons, selecting rows enables them, and clicking does nothing. + * + * This is the same failure `validate-dashboard-action-refs` catches for + * dashboard header/widget buttons (`DASHBOARD_ACTION_TARGET_UNDEFINED`), so it + * carries the same severity: **error**. It is a genuine dead reference, and — + * unlike an object name — there is no cross-package escape hatch to soften it + * with. The runtime ships NO built-in action names (there is no + * `BUILTIN_ACTIONS` registry; `list_toolbar`/`list_item` are *locations*, not + * actions), so a name resolving nowhere is dead, full stop. + * + * Scope note: this rule asks only "is this action defined ANYWHERE in the + * stack?". It deliberately does NOT check that a view's action belongs to the + * view's own object, nor that the action declares the matching `location` — + * both are real but distinct classes, and folding them in here would trade the + * zero-false-positive posture (ADR-0072 D1) for coverage this issue did not ask + * for. An action defined by another installed package is the one legitimate + * miss; it is called out in the hint rather than guessed at. + */ + +export const ACTION_NAME_UNDEFINED = 'action-name-undefined'; + +export type ActionNameRefSeverity = 'error' | 'warning'; + +export interface ActionNameRefFinding { + /** Always `error` — a name-bound action that resolves nowhere is a dead button. */ + severity: ActionNameRefSeverity; + /** Diagnostic rule id. */ + rule: string; + /** Human-readable location, e.g. `view "crm_lead" · list "all" · bulkActions`. */ + where: string; + /** Config path, e.g. `views[0].list.bulkActions[1]`. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +type AnyRec = Record; + +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 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 candidate of known) { + const d = distance(target, candidate); + if (d < bestScore) { + bestScore = d; + best = candidate; + } + } + const limit = Math.max(2, Math.floor(target.length / 3)); + return best && bestScore <= limit ? ` Did you mean "${best}"?` : ''; +} + +/** Every action name defined in the stack (global + object-embedded). */ +function collectActionNames(stack: AnyRec): Set { + const names = new Set(); + for (const action of asArray(stack.actions)) { + const n = strName(action?.name); + if (n) names.add(n); + } + for (const obj of asArray(stack.objects)) { + if (!obj || typeof obj !== 'object') continue; + for (const action of asArray(obj.actions)) { + const n = strName(action?.name); + if (n) names.add(n); + } + } + return names; +} + +/** + * Validate every name-bound action reference in a stack. Returns findings + * (empty = clean). + */ +export function validateActionNameRefs(stack: AnyRec): ActionNameRefFinding[] { + const findings: ActionNameRefFinding[] = []; + if (!stack || typeof stack !== 'object') return findings; + + const known = collectActionNames(stack); + + const check = (name: string, where: string, path: string, surface: string) => { + if (known.has(name)) return; + findings.push({ + severity: 'error', + rule: ACTION_NAME_UNDEFINED, + where, + path, + message: + `${surface} names action "${name}", which is defined by no action in this stack ` + + `(neither \`stack.actions\` nor any object's \`actions\`). The button renders and ` + + `does nothing when clicked — a dead affordance the runtime cannot dispatch.` + + suggest(name, known), + hint: + `Define an action named "${name}" (in \`stack.actions\` or the object's \`actions\`) ` + + `with the location this surface needs, remove the reference, or ignore this if the ` + + `action is contributed by another installed package.` + + (known.size > 0 ? ` Defined actions: ${[...known].sort().join(', ')}.` : ''), + }); + }; + + // ── List views: rowActions / bulkActions on `list` + each `listViews.` ── + const views = asArray(stack.views); + for (let vi = 0; vi < views.length; vi++) { + const view = views[vi]; + if (!view || typeof view !== 'object') continue; + const viewName = strName(view.name) ?? strName(view.object) ?? `#${vi}`; + + const checkListContainer = (container: unknown, label: string, path: string) => { + if (!container || typeof container !== 'object') return; + const list = container as AnyRec; + for (const key of ['rowActions', 'bulkActions'] as const) { + const names = strList(list[key]); + for (let ai = 0; ai < names.length; ai++) { + check( + names[ai], + `view "${viewName}" · ${label} · ${key}`, + `${path}.${key}[${ai}]`, + key === 'bulkActions' ? 'Bulk-action menu' : 'Row-action menu', + ); + } + } + }; + + checkListContainer(view.list, 'list', `views[${vi}].list`); + const listViews = view.listViews; + if (listViews && typeof listViews === 'object' && !Array.isArray(listViews)) { + for (const [key, lv] of Object.entries(listViews as AnyRec)) { + checkListContainer(lv, `listViews.${key}`, `views[${vi}].listViews.${key}`); + } + } + } + + // ── Page components: record:quick_actions → properties.actionNames ── + 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}`; + + 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`); + } + }; + + walkComponents(page.components, `pages[${pi}].components`); + } + + // ── App navigation: { type: 'action', actionDef: { actionName } } ── + const apps = asArray(stack.apps); + for (let ai = 0; ai < apps.length; ai++) { + const app = apps[ai]; + if (!app || typeof app !== 'object') continue; + const appName = strName(app.name) ?? `#${ai}`; + + const walkNav = (items: unknown, basePath: string) => { + const navItems = asArray(items); + for (let ni = 0; ni < navItems.length; ni++) { + const nav = navItems[ni]; + if (!nav || typeof nav !== 'object') continue; + const navPath = `${basePath}[${ni}]`; + const actionDef = nav.actionDef as AnyRec | undefined; + const actionName = strName(actionDef?.actionName); + if (nav.type === 'action' && actionName) { + check( + actionName, + `app "${appName}" · nav "${strName(nav.id) ?? `#${ni}`}"`, + `${navPath}.actionDef.actionName`, + 'Navigation action item', + ); + } + if (Array.isArray(nav.children)) walkNav(nav.children, `${navPath}.children`); + } + }; + + walkNav(app.navigation, `apps[${ai}].navigation`); + const areas = asArray(app.areas); + for (let ri = 0; ri < areas.length; ri++) { + walkNav(areas[ri]?.navigation, `apps[${ai}].areas[${ri}].navigation`); + } + } + + return findings; +} diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 57ea136ff8..d727b5e265 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -451,6 +451,62 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { expect(issues.some(i => i.where.includes("hook 'on_close'") && /bare reference `status`/.test(i.message))).toBe(true); }); + // Issue #3583 — an array-valued `object` previously dropped to `undefined`, + // so a multi-target hook got NO field-awareness at all: a condition + // filtering on a field that exists on neither target passed clean. + it('flags an unknown field in a multi-target hook condition', () => { + const issues = validateStackExpressions({ + objects: [ + { name: 'crm_lead', fields: { status: { type: 'select' } } }, + { name: 'crm_account', fields: { status: { type: 'select' } } }, + ], + hooks: [ + { name: 'on_campaign', object: ['crm_lead', 'crm_account'], condition: 'record.campaign == "spring"' }, + ], + }); + const hookIssues = issues.filter(i => i.where.includes("hook 'on_campaign'")); + expect(hookIssues.length).toBeGreaterThan(0); + expect(hookIssues.some(i => /campaign/.test(i.message))).toBe(true); + // Both targets are named, so the author knows where it breaks. + expect(hookIssues.some(i => i.where.includes('crm_lead'))).toBe(true); + expect(hookIssues.some(i => i.where.includes('crm_account'))).toBe(true); + }); + + it('accepts a multi-target hook condition on a field both targets share', () => { + const issues = validateStackExpressions({ + objects: [ + { name: 'crm_lead', fields: { status: { type: 'select' } } }, + { name: 'crm_account', fields: { status: { type: 'select' } } }, + ], + hooks: [ + { name: 'ok', object: ['crm_lead', 'crm_account'], condition: 'record.status == "closed"' }, + ], + }); + expect(issues.filter(i => i.where.includes("hook 'ok'"))).toEqual([]); + }); + + it('does not repeat one object-independent error per hook target', () => { + const issues = validateStackExpressions({ + objects: [ + { name: 'crm_lead', fields: { status: { type: 'select' } } }, + { name: 'crm_account', fields: { status: { type: 'select' } } }, + ], + hooks: [ + { name: 'broken', object: ['crm_lead', 'crm_account'], condition: '{record.status} == "x"' }, + ], + }); + const messages = issues.filter(i => i.where.includes("hook 'broken'")).map(i => i.message); + expect(messages.length).toBe(new Set(messages).size); + }); + + it('still checks a wildcard hook target for syntax', () => { + const issues = validateStackExpressions({ + objects: [{ name: 'crm_lead', fields: { status: { type: 'select' } } }], + hooks: [{ name: 'star', object: '*', condition: '{record.status} == "x"' }], + }); + expect(issues.some(i => i.where.includes("hook 'star'"))).toBe(true); + }); + it('flags a bare-field nested `when` on a conditional validation rule', () => { const issues = validateStackExpressions({ objects: [{ diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 255ff68fc4..20dfc3f95b 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -256,8 +256,49 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { // evaluated against the record, so a bare ref silently makes the hook // run on every record (or never) instead of the intended subset. for (const hook of asArray(stack.hooks)) { - const hookObj = typeof hook.object === 'string' ? hook.object : undefined; // array targets → no single field set - check(`hook '${(hook.name as string) ?? '?'}'${hookObj ? ` (${hookObj})` : ''} condition`, hook.condition, hookObj, 'record'); + const hookName = (hook.name as string) ?? '?'; + if (typeof hook.object === 'string') { + check(`hook '${hookName}' (${hook.object}) condition`, hook.condition, hook.object, 'record'); + continue; + } + + // A hook may target MANY objects (`object: ['a','b']`). Previously any + // non-string target dropped to `undefined`, so the condition got NO + // field-awareness at all — a hook filtering on a field that exists on none + // of its targets passed clean (issue #3583). The hook body runs against + // each target in turn, so a ref missing from ANY of them silently + // misbehaves there; validate per target and de-duplicate the + // object-independent diagnostics (syntax/shape) that every pass repeats. + const targets = Array.isArray(hook.object) + ? (hook.object as unknown[]).filter((o): o is string => typeof o === 'string' && o !== '*') + : []; + if (targets.length === 0) { + // `'*'` (or an unusable shape) — no single field set to judge against; + // syntax/shape is still validated. + check(`hook '${hookName}' condition`, hook.condition, undefined, 'record'); + continue; + } + + const before = issues.length; + const seen = new Set(); + const kept: ExprIssue[] = []; + for (const target of targets) { + const mark = issues.length; + check(`hook '${hookName}' (${target}) condition`, hook.condition, target, 'record'); + for (let i = mark; i < issues.length; i++) { + const issue = issues[i]; + const key = `${issue.message}\u0000${issue.source ?? ''}`; + // Keep the first occurrence of each distinct diagnostic. A field-unknown + // finding differs per target (it names the object), so each survives; + // a syntax error is identical across targets and collapses to one. + if (!seen.has(key)) { + seen.add(key); + kept.push(issue); + } + } + } + issues.length = before; + issues.push(...kept); } return issues; diff --git a/packages/lint/src/validate-object-references.test.ts b/packages/lint/src/validate-object-references.test.ts new file mode 100644 index 0000000000..f71994ef2c --- /dev/null +++ b/packages/lint/src/validate-object-references.test.ts @@ -0,0 +1,227 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { + validateObjectReferences, + OBJECT_REFERENCE_UNKNOWN, + OBJECT_REFERENCE_UNREGISTERED_PLATFORM, +} from './validate-object-references.js'; + +/** Minimal stack with one own object, mirroring the HotCRM shape. */ +const baseStack = () => ({ + objects: [ + { name: 'crm_lead', fields: { name: { type: 'text' }, status: { type: 'text' } } }, + { name: 'crm_account', fields: { name: { type: 'text' } } }, + ], +}); + +describe('validateObjectReferences — action params', () => { + // The literal HotCRM instance: a bulk-action lookup pointing at `user` + // instead of the platform object `sys_user`. + it('errors on a bulk-action lookup param referencing `user`', () => { + const findings = validateObjectReferences({ + ...baseStack(), + actions: [ + { + name: 'mass_reassign', + params: [{ name: 'owner', type: 'lookup', reference: 'user' }], + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN); + expect(findings[0].path).toBe('actions[0].params[0].reference'); + expect(findings[0].message).toContain('"user"'); + // The fix-it must name the real platform object, since that is the actual mistake. + expect(findings[0].hint).toContain('sys_user'); + }); + + it('accepts a lookup param referencing a real platform object', () => { + const findings = validateObjectReferences({ + ...baseStack(), + actions: [ + { name: 'mass_reassign', params: [{ name: 'owner', type: 'lookup', reference: 'sys_user' }] }, + ], + }); + expect(findings).toEqual([]); + }); + + it('accepts a lookup param referencing an own object', () => { + const findings = validateObjectReferences({ + ...baseStack(), + actions: [ + { name: 'link', params: [{ name: 'account', type: 'lookup', reference: 'crm_account' }] }, + ], + }); + expect(findings).toEqual([]); + }); + + it('errors on an unknown objectOverride', () => { + const findings = validateObjectReferences({ + ...baseStack(), + actions: [{ name: 'a', params: [{ field: 'role', objectOverride: 'member' }] }], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('actions[0].params[0].objectOverride'); + }); + + it('checks object-embedded actions too', () => { + const stack = baseStack(); + (stack.objects[0] as Record).actions = [ + { name: 'convert', params: [{ name: 'target', type: 'lookup', reference: 'accounts' }] }, + ]; + const findings = validateObjectReferences(stack); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('objects[0].actions[0].params[0].reference'); + // `accounts` is one edit away from `crm_account`? No — but the suggester + // should stay quiet rather than offer a bad guess. + expect(findings[0].severity).toBe('error'); + }); +}); + +describe('validateObjectReferences — dashboard global filters', () => { + // The other HotCRM `object: 'user'` instance. + it('errors on optionsFrom.object naming a nonexistent object', () => { + const findings = validateObjectReferences({ + ...baseStack(), + dashboards: [ + { + name: 'sales_overview', + globalFilters: [ + { name: 'owner', optionsFrom: { object: 'user', valueField: 'id', labelField: 'name' } }, + ], + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('dashboards[0].globalFilters[0].optionsFrom.object'); + }); + + it('accepts optionsFrom.object naming an own object', () => { + const findings = validateObjectReferences({ + ...baseStack(), + dashboards: [ + { name: 'd', globalFilters: [{ name: 'acct', optionsFrom: { object: 'crm_account' } }] }, + ], + }); + expect(findings).toEqual([]); + }); +}); + +describe('validateObjectReferences — the severity ladder', () => { + // The `sys_approval_process` case: platform-shaped but registered by nothing. + // ADR-0019 removed the process object when approval became a flow node. + it('warns (not errors) on a platform-prefixed name no package registers', () => { + const findings = validateObjectReferences({ + ...baseStack(), + apps: [ + { + name: 'crm', + navigation: [ + { id: 'nav_approvals', type: 'object', label: 'Approvals', objectName: 'sys_approval_process', requiresObject: 'sys_approval_process' }, + ], + }, + ], + }); + expect(findings.length).toBeGreaterThan(0); + for (const f of findings) { + expect(f.severity).toBe('warning'); + expect(f.rule).toBe(OBJECT_REFERENCE_UNREGISTERED_PLATFORM); + } + // Both the gate and the (gate-exempted) target are reported. + expect(findings.map((f) => f.path)).toEqual([ + 'apps[0].navigation[0].requiresObject', + 'apps[0].navigation[0].objectName', + ]); + expect(findings[0].hint).toContain('sys_approval_request'); + }); + + it('accepts a requiresObject naming a real plugin object', () => { + const findings = validateObjectReferences({ + ...baseStack(), + apps: [ + { + name: 'crm', + navigation: [ + { id: 'nav_approvals', type: 'object', label: 'Approvals', objectName: 'sys_approval_request', requiresObject: 'sys_approval_request' }, + ], + }, + ], + }); + expect(findings).toEqual([]); + }); + + it('accepts a cloud-only object name', () => { + const findings = validateObjectReferences({ + ...baseStack(), + apps: [ + { + name: 'admin', + navigation: [ + { id: 'nav_apps', type: 'object', label: 'Apps', objectName: 'sys_app', requiresObject: 'sys_app' }, + ], + }, + ], + }); + expect(findings).toEqual([]); + }); + + it('walks area navigation and nested children', () => { + const findings = validateObjectReferences({ + ...baseStack(), + apps: [ + { + name: 'crm', + areas: [ + { + id: 'area_sales', + label: 'Sales', + navigation: [ + { + id: 'nav_group', + type: 'group', + label: 'Group', + children: [{ id: 'nav_x', type: 'object', label: 'X', requiresObject: 'nope_object' }], + }, + ], + }, + ], + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('apps[0].areas[0].navigation[0].children[0].requiresObject'); + }); +}); + +describe('validateObjectReferences — exemptions (false-positive floor)', () => { + it('skips interpolated targets', () => { + const findings = validateObjectReferences({ + ...baseStack(), + actions: [ + { name: 'a', params: [{ name: 'p', reference: '${objectName}' }] }, + { name: 'b', params: [{ name: 'p', reference: '{current_object}' }] }, + ], + }); + expect(findings).toEqual([]); + }); + + it('is silent on a clean stack', () => { + expect(validateObjectReferences(baseStack())).toEqual([]); + }); + + it('tolerates a non-object / empty input', () => { + expect(validateObjectReferences({} as Record)).toEqual([]); + expect(validateObjectReferences(null as unknown as Record)).toEqual([]); + }); + + it('accepts the map-shaped (normalized) stack form', () => { + const findings = validateObjectReferences({ + objects: { crm_lead: { fields: {} } }, + actions: { link: { params: [{ name: 'p', reference: 'crm_lead' }] } }, + }); + expect(findings).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-object-references.ts b/packages/lint/src/validate-object-references.ts new file mode 100644 index 0000000000..18cad9236e --- /dev/null +++ b/packages/lint/src/validate-object-references.ts @@ -0,0 +1,346 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0072 — reference resolvability] Object-name reference integrity for the + * surfaces no other rule owns (issue #3583). + * + * A HotCRM audit (~18k lines of shipped metadata) found ~20 instances of ONE + * bug class: metadata naming an object that does not exist. Every instance + * passed `objectstack validate` and `objectstack lint` cleanly and failed + * SILENTLY at runtime — `object: 'user'` where the platform object is + * `sys_user`, navigation targeting `sys_approval_process` (which + * `@objectstack/plugin-approvals` never registers; ADR-0019 removed it). + * + * `defineStack`'s `validateCrossReferences` already hard-fails on the object + * references it knows about (hooks, view data, seeds, mappings, permission + * grants, nav `objectName`, action targets). This rule covers the REST — the + * reference sites that are plain `z.string()` in the schema and therefore ship + * whatever the author typed: + * + * - action params — `reference` (inline lookup/master_detail target) and + * `objectOverride` (the object owning a field-backed param). Both are the + * record picker's search target; a dead one degrades the dialog to a raw + * id text input. + * - dashboard `globalFilters[].optionsFrom.object` — the object a filter + * dropdown fetches its options from. Dead → an always-empty dropdown. + * - navigation `requiresObject` / `requiresService` capability gates. This is + * the escape hatch `stack.zod.ts` honours to SKIP nav validation, so a typo + * here is doubly silent: the entry is hidden forever (the runtime never + * finds the object in its SchemaRegistry) AND the skip suppresses the + * cross-reference error that would have caught the nav target. + * - the nav `objectName` of an item that carries `requiresObject` — exempted + * from the `defineStack` throw for good reason (it may come from another + * package), but still worth an advisory when NO known package provides it. + * + * ── Severity ladder (the point of the rule) ────────────────────────────── + * + * Prior rules answered "might this object come from another package?" with a + * PREFIX GUESS (`name.startsWith('sys_')` → skip). That guess cannot tell + * `sys_user` (real) from `sys_approval_process` (fictional), so every fictional + * platform-prefixed reference shipped. This rule resolves against the curated + * `PLATFORM_PROVIDED_OBJECT_NAMES` registry instead: + * + * 1. resolves in the stack's own objects → OK + * 2. unresolved, NOT platform-prefixed → ERROR + * (`user`, `total_revenue` — no cross-package story exists for an + * unprefixed name, since a stack's objects are namespace-prefixed; + * this is the pure typo class and the bulk of the HotCRM findings) + * 3. unresolved, prefixed, IN the registry → OK + * 4. unresolved, prefixed, NOT in the registry → WARNING + * (`sys_approval_process` — no package we know of registers it, but a + * third-party package still might, so advisory is the honest ceiling) + * + * Interpolated targets (`${…}`, `{…}`) are skipped — they resolve at render + * time, the same conservative exemption `validate-dashboard-action-refs` uses + * to keep false positives near zero (ADR-0072 D1: one dead finding and authors + * stop trusting the linter). + */ + +import { + hasPlatformObjectPrefix, + isPlatformProvidedObjectName, + PLATFORM_PROVIDED_OBJECT_NAMES, +} from '@objectstack/spec/system'; + +/** Materialized once for the repeated edit-distance scans in `suggest`. */ +const PLATFORM_NAMES: readonly string[] = [...PLATFORM_PROVIDED_OBJECT_NAMES]; + +export const OBJECT_REFERENCE_UNKNOWN = 'object-reference-unknown'; +export const OBJECT_REFERENCE_UNREGISTERED_PLATFORM = 'object-reference-unregistered-platform'; + +export type ObjectRefSeverity = 'error' | 'warning'; + +export interface ObjectRefFinding { + /** `error` for an unresolvable own-stack name; `warning` for an unknown platform name. */ + severity: ObjectRefSeverity; + /** Diagnostic rule id. */ + rule: string; + /** Human-readable location, e.g. `action "mass_update" · param "owner"`. */ + where: string; + /** Config path, e.g. `actions[2].params[0].reference`. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +type AnyRec = Record; + +/** Coerce a collection (array or name-keyed map) to an array of records, + * injecting `name` from the map key — mirrors the sibling authoring lints so + * the rule works on both the parsed (array) and normalized (map) stack shapes. */ +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; +} + +/** + * A target the author cannot have meant literally — `${…}` or `{…}` resolved at + * render time. + * + * Scanned rather than matched with `/\{[^}]+\}/`: that pattern backtracks + * quadratically on a long run of `{` with no closing brace (CodeQL + * polynomial-ReDoS), and the question here is simply "is there a `{` with a + * later `}` and something in between", which one pass answers. + */ +function isInterpolated(target: string): boolean { + if (target.includes('${')) return true; + const open = target.indexOf('{'); + // `+ 2` keeps the original "at least one character between the braces" + // semantics, so a literal `{}` is not treated as a placeholder. + return open !== -1 && target.indexOf('}', open + 2) !== -1; +} + +/** Levenshtein-bounded "did you mean?" over the known names. */ +function suggest(target: string, known: Iterable): string { + let best: string | undefined; + let bestScore = Infinity; + for (const candidate of known) { + const d = distance(target, candidate); + if (d < bestScore) { + bestScore = d; + best = candidate; + } + } + // Only offer a suggestion that is plausibly the same identifier mistyped. + const limit = Math.max(2, Math.floor(target.length / 3)); + return best && bestScore <= limit ? ` Did you mean "${best}"?` : ''; +} + +function distance(a: string, b: string): number { + const m = a.length; + const n = b.length; + if (m === 0) return n; + if (n === 0) return m; + let prev = Array.from({ length: n + 1 }, (_, j) => j); + for (let i = 1; i <= m; i++) { + const curr = [i, ...new Array(n).fill(0)]; + for (let j = 1; j <= n; j++) { + const cost = a[i - 1] === b[j - 1] ? 0 : 1; + curr[j] = Math.min(curr[j - 1] + 1, prev[j] + 1, prev[j - 1] + cost); + } + prev = curr; + } + return prev[n]; +} + +/** + * Validate every object-name reference on the surfaces listed in the module + * header. Returns findings (empty = clean). + */ +export function validateObjectReferences(stack: AnyRec): ObjectRefFinding[] { + const findings: ObjectRefFinding[] = []; + if (!stack || typeof stack !== 'object') return findings; + + const objects = asArray(stack.objects); + const ownObjects = new Set(); + for (const obj of objects) { + const n = strName(obj.name); + if (n) ownObjects.add(n); + } + + /** + * Resolve one reference through the ladder and record a finding if it fails. + * `subject` describes the reference for the message ("record-picker target"). + */ + const check = ( + target: string | undefined, + where: string, + path: string, + subject: string, + fix: string, + ) => { + const name = strName(target); + if (!name) return; + if (isInterpolated(name)) return; // resolved at render time + if (ownObjects.has(name)) return; // ① own object + if (isPlatformProvidedObjectName(name)) return; // ③ known platform object + + if (hasPlatformObjectPrefix(name)) { + // ④ Platform-shaped, but no package we know of registers it. + findings.push({ + severity: 'warning', + rule: OBJECT_REFERENCE_UNREGISTERED_PLATFORM, + where, + path, + message: + `${subject} "${name}" carries a platform namespace prefix, but no platform ` + + `package, official plugin, or cloud runtime object registers that name — ` + + `and this stack does not define it either. If nothing provides it at runtime ` + + `the reference resolves to nothing and fails silently.` + + suggest(name, PLATFORM_NAMES), + hint: + `Check the spelling against the object the providing package actually registers ` + + `(e.g. "sys_approval_request", not "sys_approval_process" — the process object ` + + `was removed when approval became a flow node, ADR-0019). If a third-party ` + + `package genuinely provides it, this warning is expected. ${fix}`, + }); + return; + } + + // ② Unprefixed and unresolved — a stack's own objects are namespace- + // prefixed and present here, so there is no legitimate elsewhere. + findings.push({ + severity: 'error', + rule: OBJECT_REFERENCE_UNKNOWN, + where, + path, + message: + `${subject} "${name}" resolves to no object defined in this stack. ` + + `The reference is inert at runtime — nothing reports the miss.` + + suggest(name, ownObjects), + hint: + `Point it at one of this stack's objects, or at a platform object by its full ` + + `name (the platform user object is "sys_user", not "user"). ${fix}` + + (ownObjects.size > 0 ? ` Defined objects: ${[...ownObjects].sort().join(', ')}.` : ''), + }); + }; + + // ── Actions (global + object-embedded) → param object targets ── + const checkActionParams = (action: AnyRec, actionPath: string, actionLabel: string) => { + const params = asArray(action.params); + for (let pi = 0; pi < params.length; pi++) { + const param = params[pi]; + if (!param || typeof param !== 'object') continue; + const paramLabel = strName(param.name) ?? strName(param.field) ?? `#${pi}`; + const where = `${actionLabel} · param "${paramLabel}"`; + check( + strName(param.reference), + where, + `${actionPath}.params[${pi}].reference`, + 'record-picker target', + 'Without a resolvable target the picker degrades to a raw record-id text input.', + ); + check( + strName(param.objectOverride), + where, + `${actionPath}.params[${pi}].objectOverride`, + 'field-backed param object', + 'The param inherits type/options from a field on this object, so an unknown object leaves it untyped.', + ); + } + }; + + const globalActions = asArray(stack.actions); + for (let ai = 0; ai < globalActions.length; ai++) { + const action = globalActions[ai]; + if (!action || typeof action !== 'object') continue; + checkActionParams(action, `actions[${ai}]`, `action "${strName(action.name) ?? `#${ai}`}"`); + } + + for (let oi = 0; oi < objects.length; oi++) { + const obj = objects[oi]; + if (!obj || typeof obj !== 'object') continue; + const objName = strName(obj.name) ?? `#${oi}`; + const objActions = asArray(obj.actions); + for (let ai = 0; ai < objActions.length; ai++) { + const action = objActions[ai]; + if (!action || typeof action !== 'object') continue; + checkActionParams( + action, + `objects[${oi}].actions[${ai}]`, + `object "${objName}" · action "${strName(action.name) ?? `#${ai}`}"`, + ); + } + } + + // ── Dashboard global filters → optionsFrom.object ── + const dashboards = asArray(stack.dashboards); + for (let di = 0; di < dashboards.length; di++) { + const dash = dashboards[di]; + if (!dash || typeof dash !== 'object') continue; + const dashName = strName(dash.name) ?? `#${di}`; + const filters = asArray(dash.globalFilters); + for (let fi = 0; fi < filters.length; fi++) { + const filter = filters[fi]; + if (!filter || typeof filter !== 'object') continue; + const optionsFrom = filter.optionsFrom as AnyRec | undefined; + if (!optionsFrom || typeof optionsFrom !== 'object') continue; + check( + strName(optionsFrom.object), + `dashboard "${dashName}" · filter "${strName(filter.name) ?? `#${fi}`}"`, + `dashboards[${di}].globalFilters[${fi}].optionsFrom.object`, + 'filter options source', + 'The dropdown fetches its options from this object; an unknown one renders an always-empty filter.', + ); + } + } + + // ── App navigation → requiresObject gates (and gated objectName) ── + const apps = asArray(stack.apps); + for (let ai = 0; ai < apps.length; ai++) { + const app = apps[ai]; + if (!app || typeof app !== 'object') continue; + const appName = strName(app.name) ?? `#${ai}`; + + const walkNav = (items: unknown, basePath: string) => { + const navItems = asArray(items); + for (let ni = 0; ni < navItems.length; ni++) { + const nav = navItems[ni]; + if (!nav || typeof nav !== 'object') continue; + const navId = strName(nav.id) ?? `#${ni}`; + const where = `app "${appName}" · nav "${navId}"`; + const navPath = `${basePath}[${ni}]`; + + check( + strName(nav.requiresObject), + where, + `${navPath}.requiresObject`, + 'capability gate object', + 'The entry is hidden unless this object is registered, so a typo hides it permanently — ' + + 'and it suppresses the nav cross-reference check that would have caught the target.', + ); + + // A nav target exempted from the `defineStack` throw by `requiresObject` + // still deserves an advisory when nothing known provides it. + if (nav.requiresObject && strName(nav.objectName)) { + check( + strName(nav.objectName), + where, + `${navPath}.objectName`, + 'navigation target', + 'Declaring `requiresObject` exempts this target from the build-time check, so it is only verified here.', + ); + } + + if (Array.isArray(nav.children)) walkNav(nav.children, `${navPath}.children`); + } + }; + + walkNav(app.navigation, `apps[${ai}].navigation`); + const areas = asArray(app.areas); + for (let ri = 0; ri < areas.length; ri++) { + walkNav(areas[ri]?.navigation, `apps[${ai}].areas[${ri}].navigation`); + } + } + + return findings; +} diff --git a/packages/lint/src/validate-widget-bindings.test.ts b/packages/lint/src/validate-widget-bindings.test.ts index b179be51d8..ea53238909 100644 --- a/packages/lint/src/validate-widget-bindings.test.ts +++ b/packages/lint/src/validate-widget-bindings.test.ts @@ -92,6 +92,20 @@ describe('validateWidgetBindings (reference integrity, issue #1721)', () => { expect(findings[0].hint).toContain('Did you mean "expense_line_metrics"?'); }); + // Issue #3583 — on the raw-config paths (`lint`/`doctor`) a widget with no + // `dataset` key at all fell through a bare `continue` and silently bypassed + // every binding and chart check. `dataset` is schema-REQUIRED, so reporting + // it here matches what the parsed paths already enforce. + it('(a2) errors on a widget that binds no dataset at all', () => { + const stack = chartStack(); + delete (stack as any).dashboards[0].widgets[0].dataset; + const findings = validateWidgetBindings(stack); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].rule).toBe(WIDGET_DATASET_UNKNOWN); + expect(findings[0].message).toContain('binds no `dataset`'); + }); + it('(b) errors on a dimension name the dataset does not declare', () => { const findings = validateWidgetBindings(chartStack({ dimensions: ['categry'] })); expect(findings).toHaveLength(1); diff --git a/packages/lint/src/validate-widget-bindings.ts b/packages/lint/src/validate-widget-bindings.ts index 3cabad6942..cd3c82282d 100644 --- a/packages/lint/src/validate-widget-bindings.ts +++ b/packages/lint/src/validate-widget-bindings.ts @@ -433,7 +433,27 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { `Define the dataset with defineDataset() or fix the reference (ADR-0021).`, }); } - // Without a resolved dataset there is nothing to check names against. + // A widget with NO `dataset` key at all. `DashboardWidgetSchema.dataset` + // is REQUIRED, so the schema-parsed paths (`compile`, `validate`) reject + // this before we run — but `lint`/`doctor` hand us raw, un-parsed config, + // where it previously fell into the `continue` below and silently + // bypassed EVERY binding and chart check (issue #3583). Report it rather + // than skip: an unbound widget resolves no data and renders empty. + if (!dsName) { + push({ + severity: 'error', + rule: WIDGET_DATASET_UNKNOWN, + message: + `binds no \`dataset\` — the ADR-0021 widget shape requires one, so this ` + + `widget resolves no data and renders empty.`, + hint: + `Set \`dataset: ''\` (plus \`values\`, and \`dimensions\` where the chart ` + + `family needs them). Declared datasets: ${list(datasets.keys())}.`, + }); + continue; + } + // A named-but-unresolvable dataset was already reported above; either way + // there is nothing left to check names against. if (!dataset) continue; // ── (a1) dashboard filter fields exist on the widget's object (#3365) ── diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index ca655a3509..73b24d61fd 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -650,6 +650,7 @@ "BookSchema (const)", "BucketConfig (type)", "BucketConfigSchema (const)", + "CLOUD_PROVIDED_OBJECT_NAMES (const)", "CRDTMergeResult (type)", "CRDTMergeResultSchema (const)", "CRDTState (type)", @@ -1011,6 +1012,9 @@ "OpenTelemetryCompatibilitySchema (const)", "OtelExporterType (type)", "PKG_CONVENTIONS (const)", + "PLATFORM_OBJECTS_BY_PACKAGE (const)", + "PLATFORM_OBJECT_PREFIXES (const)", + "PLATFORM_PROVIDED_OBJECT_NAMES (const)", "PNCounter (type)", "PNCounterSchema (const)", "PackageDirectory (type)", @@ -1268,7 +1272,9 @@ "docAudienceAllows (function)", "emailTemplateForm (const)", "gcsStorageExample (const)", + "hasPlatformObjectPrefix (function)", "isDataMigrationFlagVerified (function)", + "isPlatformProvidedObjectName (function)", "isPublicAudience (function)", "minioStorageExample (const)", "resolveActionConfirm (function)", diff --git a/packages/spec/src/stack.test.ts b/packages/spec/src/stack.test.ts index 18303ee1e5..3be7d5b883 100644 --- a/packages/spec/src/stack.test.ts +++ b/packages/spec/src/stack.test.ts @@ -683,6 +683,80 @@ describe('defineStack - Navigation Cross-Reference Validation', () => { }; expect(() => defineStack(config)).not.toThrow(); }); + + // Issue #3583 — an areas-based app was skipped entirely by the old + // `if (!app.navigation) continue`, so none of its nav references were checked. + it('should detect an undefined object in an area navigation', () => { + const config = { + manifest: baseManifest, + objects: [{ name: 'task', fields: { title: { type: 'text' } } }], + apps: [ + { + name: 'my_app', + label: 'My App', + areas: [ + { + id: 'area_work', + label: 'Work', + navigation: [ + { id: 'nav_missing', type: 'object' as const, label: 'Missing', objectName: 'nonexistent_object' }, + ], + }, + ], + }, + ], + }; + expect(() => defineStack(config)).toThrow('nonexistent_object'); + }); + + it('should pass when area navigation references resolve', () => { + const config = { + manifest: baseManifest, + objects: [{ name: 'task', fields: { title: { type: 'text' } } }], + apps: [ + { + name: 'my_app', + label: 'My App', + areas: [ + { + id: 'area_work', + label: 'Work', + navigation: [ + { id: 'nav_tasks', type: 'object' as const, label: 'Tasks', objectName: 'task' }, + ], + }, + ], + }, + ], + }; + expect(() => defineStack(config)).not.toThrow(); + }); + + // Children hang off `object` nav items too, not just `group` ones. + it('should detect an undefined object nested under an object nav item', () => { + const config = { + manifest: baseManifest, + objects: [{ name: 'task', fields: { title: { type: 'text' } } }], + apps: [ + { + name: 'my_app', + label: 'My App', + navigation: [ + { + id: 'nav_tasks', + type: 'object' as const, + label: 'Tasks', + objectName: 'task', + children: [ + { id: 'nav_child', type: 'object' as const, label: 'Child', objectName: 'nonexistent_object' }, + ], + }, + ], + }, + ], + }; + expect(() => defineStack(config)).toThrow('nonexistent_object'); + }); }); // ============================================================================ diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index a99e1230b5..400eb0fcba 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -7,6 +7,7 @@ import { validateObjectNamespacePrefix } from './kernel/namespace-prefix'; import { PLATFORM_CAPABILITY_TOKENS } from './kernel/platform-capabilities'; import { DatasourceSchema } from './data/datasource.zod'; import { TranslationBundleSchema, TranslationConfigSchema } from './system/translation.zod'; +import { hasPlatformObjectPrefix } from './system/constants/platform-object-names'; import { objectStackErrorMap, formatZodError } from './shared/error-map.zod'; import { normalizeStackInput, type MetadataCollectionInput, type MapSupportedField } from './shared/metadata-collection.zod'; @@ -602,9 +603,17 @@ function validateSingleApp(config: ObjectStackDefinition): string[] { * ADR-0090 business-unit tree (`sys_business_unit`) or grants a delegated * administrator CRUD on the RBAC link tables (`sys_user_position`, ADR-0090 * D12). The typo net stays intact for the stack's OWN objects. + * + * Kept as a PREFIX test at this layer on purpose: these are hard `defineStack` + * throws, and a third-party package may legitimately contribute a prefixed + * object this repo's registry cannot know about — failing the build on that + * would be worse than the typo it catches. The narrower "prefixed but no known + * package registers it" signal (`sys_approval_process`, issue #3583) is an + * ADVISORY finding, so it lives in `@objectstack/lint`'s reference rules where + * it can warn instead of throw — see `isPlatformProvidedObjectName`. */ function isPlatformObjectName(name: string): boolean { - return /^(sys_|cloud_|ai_)/.test(name); + return hasPlatformObjectPrefix(name); } /** @@ -756,7 +765,6 @@ function validateCrossReferences(config: ObjectStackDefinition): string[] { } for (const app of config.apps) { - if (!app.navigation) continue; const checkNavItems = (items: unknown[], appName: string) => { for (const item of items) { if (!item || typeof item !== 'object') continue; @@ -787,13 +795,25 @@ function validateCrossReferences(config: ObjectStackDefinition): string[] { `App '${appName}' navigation references report '${nav.reportName}' which is not defined in reports.`, ); } - // Recurse into group children - if (nav.type === 'group' && Array.isArray(nav.children)) { + // Recurse into children. NOT gated on `type === 'group'`: an `object` + // nav item carries `children` too (NavigationItemSchema extends it + // with a child array for per-view entries), and a targeted child + // nested under one was previously skipped. + if (Array.isArray(nav.children)) { checkNavItems(nav.children, appName); } } }; - checkNavItems(app.navigation, app.name); + // Both nav containers. `areas[]` was previously skipped entirely by an + // `if (!app.navigation) continue`, so an areas-based app got NO nav + // cross-reference checking at all (issue #3583). + if (Array.isArray(app.navigation)) checkNavItems(app.navigation, app.name); + if (Array.isArray(app.areas)) { + for (const area of app.areas) { + const nav = (area as { navigation?: unknown })?.navigation; + if (Array.isArray(nav)) checkNavItems(nav, app.name); + } + } } } diff --git a/packages/spec/src/system/constants/index.ts b/packages/spec/src/system/constants/index.ts index 52812455c9..d48a2cbc83 100644 --- a/packages/spec/src/system/constants/index.ts +++ b/packages/spec/src/system/constants/index.ts @@ -12,3 +12,4 @@ export * from './paths'; export * from './system-names'; +export * from './platform-object-names'; diff --git a/packages/spec/src/system/constants/platform-object-names.test.ts b/packages/spec/src/system/constants/platform-object-names.test.ts new file mode 100644 index 0000000000..aa69771d57 --- /dev/null +++ b/packages/spec/src/system/constants/platform-object-names.test.ts @@ -0,0 +1,156 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Conformance ratchet for PLATFORM_PROVIDED_OBJECT_NAMES (issue #3583). +// +// The registry replaces a prefix heuristic (`startsWith('sys_')`) with a +// curated list, which is only an improvement while the list is TRUE: consumers +// now treat "platform-prefixed but absent" as a probable typo, so a stale entry +// turns a real object into a false warning, and a missing entry lets a +// fictional reference (the `sys_approval_process` case) ship silently again. +// +// This test is the thing that keeps it true. It scans every `*.object.ts` in +// the monorepo for its `ObjectSchema.create({ name })` declaration and asserts +// the registry matches, per owning package. Scanning source (rather than +// importing the packages) keeps the check here, next to the data, without +// inverting the spec → * dependency direction. +// +// If this fails: you added/removed/moved a platform object. Update +// PLATFORM_OBJECTS_BY_PACKAGE in ./platform-object-names.ts to match. + +import { describe, it, expect } from 'vitest'; +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + PLATFORM_OBJECTS_BY_PACKAGE, + PLATFORM_PROVIDED_OBJECT_NAMES, + CLOUD_PROVIDED_OBJECT_NAMES, + hasPlatformObjectPrefix, + isPlatformProvidedObjectName, +} from './platform-object-names'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +/** …/packages/spec/src/system/constants → repo root */ +const REPO_ROOT = resolve(HERE, '../../../../..'); +const PACKAGES_DIR = join(REPO_ROOT, 'packages'); + +/** + * Packages whose `*.object.ts` files are FIXTURES, not platform contributions: + * a downstream-contract QA harness and the `create-objectstack` scaffolding + * templates (the `blank` starter's `note.object.ts`). Neither ships objects + * into a running platform, so neither belongs in the registry. + */ +const FIXTURE_PACKAGES = new Set(['qa', 'create-objectstack']); + +function walk(dir: string, out: string[] = []): string[] { + let entries: string[]; + try { + entries = readdirSync(dir); + } catch { + return out; + } + for (const entry of entries) { + if (entry === 'node_modules' || entry === 'dist' || entry.startsWith('.')) continue; + const full = join(dir, entry); + let isDir: boolean; + try { + isDir = statSync(full).isDirectory(); + } catch { + continue; + } + if (isDir) walk(full, out); + else if (entry.endsWith('.object.ts')) out.push(full); + } + return out; +} + +/** The object name declared by an `ObjectSchema.create({ name: '…' })` file. */ +function declaredObjectName(source: string): string | undefined { + const idx = source.indexOf('ObjectSchema.create({'); + if (idx === -1) return undefined; + const m = /name:\s*'([a-z0-9_]+)'/.exec(source.slice(idx)); + return m?.[1]; +} + +/** `packages/plugins/plugin-audit/src/x.object.ts` → `plugin-audit`. */ +function owningPackage(file: string): string | undefined { + const rel = file.slice(PACKAGES_DIR.length + 1); + const parts = rel.split(/[\\/]/); + const srcAt = parts.indexOf('src'); + if (srcAt < 1) return undefined; + return parts[srcAt - 1]; +} + +function scanDeclaredObjects(): Map> { + const byPackage = new Map>(); + for (const file of walk(PACKAGES_DIR)) { + const pkg = owningPackage(file); + if (!pkg) continue; + const rel = file.slice(PACKAGES_DIR.length + 1); + const topLevel = rel.split(/[\\/]/)[0]; + if (FIXTURE_PACKAGES.has(topLevel) || FIXTURE_PACKAGES.has(pkg)) continue; + const name = declaredObjectName(readFileSync(file, 'utf8')); + if (!name) continue; + if (!byPackage.has(pkg)) byPackage.set(pkg, new Set()); + byPackage.get(pkg)!.add(name); + } + return byPackage; +} + +const sorted = (v: Iterable) => [...v].sort(); + +describe('PLATFORM_PROVIDED_OBJECT_NAMES — conformance with what packages register', () => { + const declared = scanDeclaredObjects(); + + it('finds the platform object sources (guards against a broken scan)', () => { + // A silently-empty scan would make every assertion below vacuously pass. + expect(declared.size).toBeGreaterThan(5); + expect(declared.get('platform-objects')?.has('sys_user')).toBe(true); + }); + + it('registers exactly the objects each package declares', () => { + for (const [pkg, names] of declared) { + const registered = PLATFORM_OBJECTS_BY_PACKAGE[pkg]; + expect(registered, `package "${pkg}" declares objects but has no registry group`).toBeDefined(); + expect(sorted(registered ?? []), `registry group "${pkg}" is out of date`).toEqual(sorted(names)); + } + }); + + it('has no registry group for a package that declares no objects', () => { + for (const pkg of Object.keys(PLATFORM_OBJECTS_BY_PACKAGE)) { + expect(declared.has(pkg), `registry group "${pkg}" no longer matches any package`).toBe(true); + } + }); + + it('every registered name carries a reserved platform prefix', () => { + for (const name of PLATFORM_PROVIDED_OBJECT_NAMES) { + expect(hasPlatformObjectPrefix(name), `"${name}" is not platform-prefixed`).toBe(true); + } + }); + + it('cloud-only names are registered but not declared in this repo', () => { + // They live in @objectstack/service-tenant (cloud repo, ADR-0003). If one + // ever moves here, the group assertions above would flag it as unregistered. + const allDeclared = new Set([...declared.values()].flatMap((s) => [...s])); + for (const name of CLOUD_PROVIDED_OBJECT_NAMES) { + expect(isPlatformProvidedObjectName(name)).toBe(true); + expect(allDeclared.has(name), `"${name}" is now declared here — move it out of CLOUD_PROVIDED_OBJECT_NAMES`).toBe(false); + } + }); +}); + +describe('platform-object predicates', () => { + it('separates a real platform object from a fictional one', () => { + // The exact bug the registry exists to catch: both are `sys_`-prefixed, so + // the old heuristic exempted both. Only one is real. + expect(isPlatformProvidedObjectName('sys_user')).toBe(true); + expect(isPlatformProvidedObjectName('sys_approval_request')).toBe(true); + expect(isPlatformProvidedObjectName('sys_approval_process')).toBe(false); + expect(hasPlatformObjectPrefix('sys_approval_process')).toBe(true); + }); + + it('treats an unprefixed name as neither', () => { + expect(hasPlatformObjectPrefix('crm_lead')).toBe(false); + expect(isPlatformProvidedObjectName('user')).toBe(false); + }); +}); diff --git a/packages/spec/src/system/constants/platform-object-names.ts b/packages/spec/src/system/constants/platform-object-names.ts new file mode 100644 index 0000000000..c08b3d6519 --- /dev/null +++ b/packages/spec/src/system/constants/platform-object-names.ts @@ -0,0 +1,191 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Canonical registry of PLATFORM-PROVIDED object names (issue #3583). + * + * A stack legitimately references objects it does not itself define — the + * platform contributes `sys_user`, an installed plugin contributes + * `sys_approval_request`, the cloud runtime contributes `sys_app`. Every + * cross-reference check therefore needs an answer to "is this name real, or a + * typo?", and until this module existed each one guessed with a **prefix + * heuristic** (`name.startsWith('sys_')` in `validate-flow-trigger-readiness` + * and `validate-security-posture`; `/^(sys_|cloud_|ai_)/` in `stack.zod.ts`). + * + * A prefix guess cannot distinguish `sys_user` (real) from + * `sys_approval_process` (fictional — removed by ADR-0019, registered by + * nothing), so every fictional platform-prefixed reference shipped silently. + * That is exactly the bug class the HotCRM audit found, and the reason this + * list is a **curated set of real names** rather than a pattern. + * + * Structure mirrors the {@link PLATFORM_CAPABILITIES} precedent: the contract + * package owns the list (everything depends on spec, so no dependency + * inversion), and the OWNING packages carry conformance tests asserting the + * list matches what they actually register. Lint consumes it through the + * documented `@objectstack/spec` dependency — it never imports + * `platform-objects` or a plugin (see `@objectstack/lint`'s one-way rule). + * + * Maintenance contract: adding an object to a platform package means adding + * its name here. The owning package's conformance test fails otherwise — an + * out-of-date registry is worse than the heuristic it replaced, because + * consumers now trust it. + */ + +/** + * Objects contributed by packages IN THIS REPOSITORY, grouped by owning + * package. Each group is the exact set that package registers. + */ +export const PLATFORM_OBJECTS_BY_PACKAGE: Readonly> = { + /** `@objectstack/metadata-core` — metadata storage & commit history. */ + 'metadata-core': [ + 'sys_metadata', + 'sys_metadata_audit', + 'sys_metadata_commit', + 'sys_metadata_history', + 'sys_view_definition', + ], + /** `@objectstack/platform-objects` — identity, org, auth, settings, jobs. */ + 'platform-objects': [ + 'sys_account', + 'sys_api_key', + 'sys_attachment', + 'sys_business_unit', + 'sys_business_unit_member', + 'sys_device_code', + 'sys_email', + 'sys_email_template', + 'sys_import_job', + 'sys_invitation', + 'sys_job', + 'sys_job_queue', + 'sys_job_run', + 'sys_jwks', + 'sys_member', + 'sys_migration', + 'sys_notification', + 'sys_oauth_access_token', + 'sys_oauth_application', + 'sys_oauth_client_assertion', + 'sys_oauth_client_resource', + 'sys_oauth_consent', + 'sys_oauth_refresh_token', + 'sys_oauth_resource', + 'sys_organization', + 'sys_report_schedule', + 'sys_saved_report', + 'sys_scim_provider', + 'sys_secret', + 'sys_session', + 'sys_setting', + 'sys_setting_audit', + 'sys_sso_provider', + 'sys_team', + 'sys_team_member', + 'sys_two_factor', + 'sys_user', + 'sys_user_preference', + 'sys_verification', + ], + /** `@objectstack/plugin-approvals` — ADR-0019 approval-as-flow-node runtime. */ + 'plugin-approvals': [ + 'sys_approval_action', + 'sys_approval_approver', + 'sys_approval_delegation', + 'sys_approval_request', + 'sys_approval_token', + ], + /** `@objectstack/plugin-audit` — audit trail & activity feed (ADR-0052). */ + 'plugin-audit': ['sys_activity', 'sys_audit_log', 'sys_comment'], + /** `@objectstack/plugin-security` — ADR-0090 permission model v2. */ + 'plugin-security': [ + 'sys_audience_binding_suggestion', + 'sys_capability', + 'sys_permission_set', + 'sys_position', + 'sys_position_permission_set', + 'sys_user_permission_set', + 'sys_user_position', + ], + /** `@objectstack/plugin-sharing` — record shares & share links. */ + 'plugin-sharing': ['sys_record_share', 'sys_share_link', 'sys_sharing_rule'], + /** `@objectstack/plugin-webhooks` — outbound webhook registrations. */ + 'plugin-webhooks': ['sys_webhook'], + /** `@objectstack/service-automation` — flow run history. */ + 'service-automation': ['sys_automation_run'], + /** `@objectstack/service-messaging` — notification & delivery pipeline. */ + 'service-messaging': [ + 'sys_http_delivery', + 'sys_inbox_message', + 'sys_notification_delivery', + 'sys_notification_preference', + 'sys_notification_receipt', + 'sys_notification_subscription', + 'sys_notification_template', + ], + /** `@objectstack/service-realtime` — presence state. */ + 'service-realtime': ['sys_presence'], + /** `@objectstack/service-storage` — file storage & upload sessions. */ + 'service-storage': ['sys_file', 'sys_upload_session'], +} as const; + +/** + * Objects contributed by the CLOUD runtime (`@objectstack/service-tenant`, + * defined in the separate `cloud` repository). They do not exist in a + * single-environment OSS runtime, which is precisely why `NavigationItem` + * documents `requiresObject: 'sys_app'` as the way to reference one safely. + * + * Listed here so a cloud-targeted stack is not told its references are + * fictional. They cannot be conformance-tested from this repo — the cloud repo + * owns that half of the contract. + */ +export const CLOUD_PROVIDED_OBJECT_NAMES: readonly string[] = [ + 'sys_app', + 'sys_environment', + 'sys_environment_member', + 'sys_package', + 'sys_package_installation', +]; + +/** + * Every platform-provided object name (this repo + cloud), for fast membership + * checks in lint rules and cross-reference validation. + */ +export const PLATFORM_PROVIDED_OBJECT_NAMES: ReadonlySet = new Set([ + ...Object.values(PLATFORM_OBJECTS_BY_PACKAGE).flat(), + ...CLOUD_PROVIDED_OBJECT_NAMES, +]); + +/** + * Namespace prefixes reserved for platform-provided objects. A stack's own + * objects may not use them (`defineStack` namespace rules), so an unresolved + * name carrying one of these is "expected from elsewhere" rather than "surely a + * typo" — the distinction that drives severity (see + * {@link isPlatformProvidedObjectName}). + */ +export const PLATFORM_OBJECT_PREFIXES: readonly string[] = ['sys_', 'cloud_', 'ai_']; + +/** + * Does `name` carry a reserved platform namespace prefix? + * + * This is the OLD heuristic, kept as an explicit, named predicate because it + * still answers a real question — "could this name legitimately come from + * outside this stack?" — for third-party packages the registry cannot know + * about. Callers must NOT use it as proof the object exists; pair it with + * {@link isPlatformProvidedObjectName} to tell a real platform object from a + * fictional one. + */ +export function hasPlatformObjectPrefix(name: string): boolean { + return PLATFORM_OBJECT_PREFIXES.some((p) => name.startsWith(p)); +} + +/** + * Is `name` a KNOWN platform-provided object (registered by a package in this + * repo, or by the cloud runtime)? + * + * `true` means the reference resolves against a real object contributed + * outside the stack. `false` for a platform-prefixed name means "no known + * package registers this" — the `sys_approval_process` case: advisory, not + * fatal, because a third-party package may legitimately provide it. + */ +export function isPlatformProvidedObjectName(name: string): boolean { + return PLATFORM_PROVIDED_OBJECT_NAMES.has(name); +}