From 9d8fa9794fda41128730479f73304b5cb63a1611 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 07:14:07 +0000 Subject: [PATCH 1/3] refactor(lint): one entry point for the reference-integrity suite (#3583 D5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six rules answering the same question — "does this name resolve to anything?" — were wired by hand into three CLI commands. `validate`, `lint` and `compile` each carried their own import list and their own call site, so landing a rule meant remembering three places, and forgetting one meant the same stack got a different verdict depending on which command the author ran. The assessment (§2.2) named that drift as the enemy and (§5 D5) asked for this entry point once Phase 2 landed; six rules in, it has cost three hand-edits per rule. `validateReferenceIntegrity(stack)` runs `REFERENCE_INTEGRITY_RULES` in order and returns the concatenated findings. Adding a rule is now a one-line edit to that list — the commands do not change. `lint.ts` loses five near-identical for-loops; `validate.ts`/`compile.ts` lose their six-way spreads. Membership is deliberately a written-out list, mirrored by a test, so a rule joins the suite by review rather than by accident. What belongs: rules that resolve a NAME against what a stack declares. What does not: shape checks (view containers, responsive styles, seed replay/state) — a different question with its own call sites. The suite test runs one live instance per member and asserts a finding from each. A suite that silently stops invoking a member is indistinguishable from a clean stack — exactly how the dead quick-actions check in #3684 passed its own tests. `os doctor` is NOT converted: it runs only `validateWidgetBindings` and is an environment health check, not an authoring gate, so adopting the suite there is a product decision about what `doctor` is for. Named in the module doc so the remaining asymmetry stays visible. Behaviour-preserving: identical findings on all three example apps (zero) and on the HotCRM corpus (24, unchanged per rule). Refs #3583 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015r31hzuSN19iK8ATRPcCpS --- packages/cli/src/commands/compile.ts | 14 +- packages/cli/src/commands/lint.ts | 99 ++---------- packages/cli/src/commands/validate.ts | 14 +- packages/lint/src/index.ts | 13 ++ .../src/reference-integrity-suite.test.ts | 142 ++++++++++++++++++ .../lint/src/reference-integrity-suite.ts | 100 ++++++++++++ 6 files changed, 270 insertions(+), 112 deletions(-) create mode 100644 packages/lint/src/reference-integrity-suite.test.ts create mode 100644 packages/lint/src/reference-integrity-suite.ts diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index f6373a702f..9539ce51be 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -13,10 +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 { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint'; -import { validateNavAccess } from '@objectstack/lint'; -import { validateTranslationReferences } from '@objectstack/lint'; +import { validateReferenceIntegrity } from '@objectstack/lint'; import { validateResponsiveStyles } from '@objectstack/lint'; import { validateSecurityPosture, validateOrgAxisRedLines, buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint'; import { validateReadonlyFlowWrites } from '@objectstack/lint'; @@ -342,14 +339,7 @@ export default class Compile extends Command { // option keys written as the display label) — advisory throughout, // since an orphan key is inert rather than broken. if (!flags.json) printStep('Checking object & action references (#3583)...'); - const refFindings = [ - ...validateObjectReferences(result.data as Record), - ...validateActionNameRefs(result.data as Record), - ...validatePageFieldBindings(result.data as Record), - ...validateChartBindings(result.data as Record), - ...validateNavAccess(result.data as Record), - ...validateTranslationReferences(result.data as Record), - ]; + const refFindings = validateReferenceIntegrity(result.data as Record); const refErrors = refFindings.filter((f) => f.severity === 'error'); const refWarnings = refFindings.filter((f) => f.severity === 'warning'); if (refErrors.length > 0) { diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index bb7d4014b8..c0f8418a69 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -10,10 +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 { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint'; -import { validateNavAccess } from '@objectstack/lint'; -import { validateTranslationReferences } from '@objectstack/lint'; +import { validateReferenceIntegrity } from '@objectstack/lint'; import { collectAndLintDocs } from '../utils/collect-docs.js'; import { scoreMetadata } from '../lint/score.js'; import { runMetadataEval } from '../lint/metadata-eval.js'; @@ -493,90 +490,16 @@ 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, - }); - } - - // ── Page component field bindings (issue #3583) ── - // `PageComponent.properties` is an untyped bag, so a highlights strip, KPI - // card, or details section can name a field the object does not have; the - // component silently skips it. Advisory, matching `FORM_FIELD_UNKNOWN` — - // every page consumer degrades rather than failing. - for (const t of validatePageFieldBindings(config)) { - issues.push({ - severity: t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, - }); - } - - // ── Chart bindings outside dashboards (issue #3583) ── - // `validate-widget-bindings` covers dashboard widgets only. Report charts, - // list-view charts, and dataset-bound page chart components bind the same - // semantic layer, where an axis naming a raw field instead of a dataset - // measure renders an empty series (ADR-0021). - for (const t of validateChartBindings(config)) { - issues.push({ - severity: t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, - }); - } - - // ── Navigation reachability vs. granted access (ADR-0090 D6, issue #3583) ── - // Navigation and permissions are separate metadata, so an app can expose an - // object no permission set grants read on: the entry renders and the click - // fails permission-denied for every user, including admins. Advisory — the - // grant may come from a set another installed package ships. - for (const t of validateNavAccess(config)) { - issues.push({ - severity: t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, - }); - } - - // ── Translation-bundle references & option keys (issue #3583) ── - // The i18n gate only ever asked "which expected keys are missing?". This is - // the reverse the spec already names (`TranslationDiffStatus 'redundant'`): - // keys pointing at metadata that no longer exists, and select-option keys - // written as the display label instead of the stored value. Advisory — an - // orphan key is inert, it just leaves one string untranslated. - for (const t of validateTranslationReferences(config)) { + // ── Reference integrity (issue #3583) ── + // One suite, one call site: object-name references `defineStack` does not + // cover, name-bound action surfaces, page-component field bindings, chart + // axes outside dashboards, navigation vs. granted access, and translation + // keys pointing at metadata that no longer exists. Every member resolves a + // NAME against what the stack declares — the class the HotCRM audit found + // shipping, where each instance parses, validates, and fails silently. + // Adding a rule to `REFERENCE_INTEGRITY_RULES` reaches this path with no + // edit here (assessment §5 D5 — the wiring drift this ends). + for (const t of validateReferenceIntegrity(config)) { issues.push({ severity: t.severity, rule: t.rule, diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index b5e40a38bd..b3dbcc7ce8 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -14,10 +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 { validatePageFieldBindings, validateChartBindings } from '@objectstack/lint'; -import { validateNavAccess } from '@objectstack/lint'; -import { validateTranslationReferences } from '@objectstack/lint'; +import { validateReferenceIntegrity } from '@objectstack/lint'; import { validateResponsiveStyles } from '@objectstack/lint'; import { validateJsxPages, validateReactPages, validateReactPageProps, validatePageSourceStyling } from '@objectstack/lint'; import { validateCapabilityReferences } from '@objectstack/lint'; @@ -310,14 +307,7 @@ export default class Validate extends Command { // to nothing and renders the source string (advisory: inert, not // broken). if (!flags.json) printStep('Checking object & action references (#3583)...'); - const refFindings = [ - ...validateObjectReferences(result.data as Record), - ...validateActionNameRefs(result.data as Record), - ...validatePageFieldBindings(result.data as Record), - ...validateChartBindings(result.data as Record), - ...validateNavAccess(result.data as Record), - ...validateTranslationReferences(result.data as Record), - ]; + const refFindings = validateReferenceIntegrity(result.data as Record); const refErrors = refFindings.filter((f) => f.severity === 'error'); const refWarnings = refFindings.filter((f) => f.severity === 'warning'); if (refErrors.length > 0) { diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index ceb547e9b8..4a16970fce 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -221,4 +221,17 @@ export type { TranslationRefSeverity, } from './validate-translation-references.js'; +// One entry point for the reference-resolution rules above (#3583 §5 D5). +// Adding a rule to `REFERENCE_INTEGRITY_RULES` runs it on `validate`, `lint` +// and `compile` at once — the CLI call sites do not change. +export { + validateReferenceIntegrity, + REFERENCE_INTEGRITY_RULES, +} from './reference-integrity-suite.js'; +export type { + ReferenceIntegrityFinding, + ReferenceIntegrityRule, + ReferenceIntegritySeverity, +} from './reference-integrity-suite.js'; + export { buildAccessMatrix, diffAccessMatrix } from './build-access-matrix.js'; diff --git a/packages/lint/src/reference-integrity-suite.test.ts b/packages/lint/src/reference-integrity-suite.test.ts new file mode 100644 index 0000000000..c9d42a78ac --- /dev/null +++ b/packages/lint/src/reference-integrity-suite.test.ts @@ -0,0 +1,142 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { + validateReferenceIntegrity, + REFERENCE_INTEGRITY_RULES, +} from './reference-integrity-suite.js'; +import { validateObjectReferences } from './validate-object-references.js'; +import { validateTranslationReferences } from './validate-translation-references.js'; + +describe('reference-integrity suite — membership', () => { + // Deliberately a written-out list: adding a rule to the suite should be a + // conscious edit in two places (the suite and this test), not something that + // slips in unreviewed. The list is also the answer to "which rules run on + // `validate` / `lint` / `compile`?". + it('holds exactly the reference-resolution rules, in report order', () => { + expect(REFERENCE_INTEGRITY_RULES.map((r) => r.name)).toEqual([ + 'validateObjectReferences', + 'validateActionNameRefs', + 'validatePageFieldBindings', + 'validateChartBindings', + 'validateNavAccess', + 'validateTranslationReferences', + ]); + }); + + it('wires the same function the package exports', () => { + const byName = new Map(REFERENCE_INTEGRITY_RULES.map((r) => [r.name, r.run])); + expect(byName.get('validateObjectReferences')).toBe(validateObjectReferences); + expect(byName.get('validateTranslationReferences')).toBe(validateTranslationReferences); + }); +}); + +describe('reference-integrity suite — every member actually runs', () => { + /** + * One live instance per rule, so a member that silently stops being invoked + * (or is dropped from the list) fails here instead of quietly narrowing what + * the CLI checks. A suite that returns nothing is indistinguishable from a + * clean stack — which is exactly how the dead quick-actions check in #3684 + * survived its own tests. + */ + const stack = { + objects: [ + { + name: 'crm_lead', + fields: { name: { type: 'text', label: 'Name' } }, + permissions: {}, + }, + ], + actions: [ + // validateObjectReferences: a param pointing at an object nothing declares. + { name: 'assign', label: 'Assign', params: [{ name: 'owner', reference: 'user' }] }, + ], + views: [ + { + list: { + type: 'grid', + name: 'all_leads', + data: { provider: 'object', object: 'crm_lead' }, + // validateActionNameRefs: no such action. + bulkActions: ['mass_update'], + }, + }, + ], + pages: [ + { + name: 'lead_detail', + object: 'crm_lead', + regions: [ + { + components: [ + // validatePageFieldBindings: `budget` is not a field on crm_lead. + { type: 'record:highlights', properties: { fields: [{ name: 'budget' }] } }, + ], + }, + ], + }, + ], + datasets: [ + { + name: 'lead_metrics', + object: 'crm_lead', + dimensions: [{ name: 'source' }], + measures: [{ name: 'count_leads', aggregate: 'count' }], + }, + ], + reports: [ + { + name: 'leads_by_source', + dataset: 'lead_metrics', + values: ['count_leads'], + // validateChartBindings: a raw field where a declared measure is required. + chart: { type: 'bar', xAxis: 'source', yAxis: 'lead_score' }, + }, + ], + apps: [ + { + name: 'crm_app', + navigation: [{ id: 'nav_leads', type: 'object', objectName: 'crm_lead' }], + }, + ], + // validateNavAccess: a declared permission set that grants nothing on the + // nav-exposed object. + permissions: [{ name: 'sales_user', label: 'Sales User', objects: {} }], + translations: [ + // validateTranslationReferences: a field the object does not declare. + { en: { objects: { crm_lead: { label: 'Lead', fields: { assigned_to: { label: 'Owner' } } } } } }, + ], + }; + + it('reports at least one finding from every member', () => { + const findings = validateReferenceIntegrity(stack); + const rules = new Set(findings.map((f) => f.rule)); + + expect(rules).toContain('object-reference-unknown'); + expect(rules).toContain('action-name-undefined'); + expect(rules).toContain('page-field-unknown'); + expect(rules).toContain('chart-measure-unknown'); + expect(rules).toContain('nav-object-ungranted'); + expect(rules).toContain('translation-target-unknown'); + }); + + it('concatenates in list order and carries the common finding shape', () => { + const findings = validateReferenceIntegrity(stack); + expect(findings.length).toBeGreaterThan(0); + for (const f of findings) { + expect(['error', 'warning']).toContain(f.severity); + expect(typeof f.rule).toBe('string'); + expect(typeof f.where).toBe('string'); + expect(typeof f.path).toBe('string'); + expect(typeof f.message).toBe('string'); + expect(typeof f.hint).toBe('string'); + } + // Object references run first, translations last. + expect(findings[0].rule).toBe('object-reference-unknown'); + expect(findings[findings.length - 1].rule).toBe('translation-target-unknown'); + }); + + it('returns nothing for an empty stack', () => { + expect(validateReferenceIntegrity({})).toEqual([]); + }); +}); diff --git a/packages/lint/src/reference-integrity-suite.ts b/packages/lint/src/reference-integrity-suite.ts new file mode 100644 index 0000000000..d934fb884c --- /dev/null +++ b/packages/lint/src/reference-integrity-suite.ts @@ -0,0 +1,100 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The reference-integrity suite — one entry point for the rules that answer + * "does this name resolve to anything?" (issue #3583, assessment §5 D5). + * + * ## Why this exists + * + * These rules were wired **by hand** into each CLI entry point that runs them. + * `os validate`, `os lint` and `os compile` each grew their own import list and + * their own call site, so landing a rule meant remembering three places — and + * the assessment's §2.2 already named the resulting drift as the enemy: the + * same stack, checked by a different rule subset depending on which command + * the author happened to run. + * + * The suite makes the next rule's wiring a ONE-LINE edit here, and makes the + * question "which rules run on this path?" answerable by reading one list. + * + * ## What belongs in it + * + * A rule belongs here when it resolves a NAME written in metadata against the + * things a stack actually declares — objects, actions, fields, measures, + * permissions, translation keys. That is the family the HotCRM audit found + * shipping broken: every instance parsed, validated, and failed silently at + * runtime because nothing checked that the name pointed at anything. + * + * Rules that check SHAPE rather than reference (view containers, responsive + * styles, seed replay safety, seed state machines, seed/security posture) stay + * out — they answer a different question and have their own call sites. + * + * ## Known remaining asymmetry + * + * `os doctor` runs only `validateWidgetBindings` and is NOT converted here: it + * is an environment health check (node version, config presence, circular + * lookups), not an authoring gate, so adopting the suite there is a product + * decision about what `doctor` is for — not a wiring cleanup. It is named here + * so the gap stays visible instead of being rediscovered. + */ + +import { validateObjectReferences } from './validate-object-references.js'; +import { validateActionNameRefs } from './validate-action-name-refs.js'; +import { validatePageFieldBindings } from './validate-page-field-bindings.js'; +import { validateChartBindings } from './validate-chart-bindings.js'; +import { validateNavAccess } from './validate-nav-access.js'; +import { validateTranslationReferences } from './validate-translation-references.js'; + +export type ReferenceIntegritySeverity = 'error' | 'warning'; + +/** + * The shape every rule in the suite already returns. Declared here so callers + * can hold one type instead of a six-way union. + */ +export interface ReferenceIntegrityFinding { + /** `error` = the reference is dead; `warning` = it may resolve elsewhere, or the miss is inert. */ + severity: ReferenceIntegritySeverity; + /** Diagnostic rule id (stable; used by allowlists and docs). */ + rule: string; + /** Human-readable location. */ + where: string; + /** Config path. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +/** One member of the suite. `name` is the exported function's name — the id a wiring test can assert on. */ +export interface ReferenceIntegrityRule { + name: string; + run: (stack: Record) => ReferenceIntegrityFinding[]; +} + +/** + * Every reference-integrity rule, in the order their findings are reported. + * + * ADDING A RULE: append it here and it runs on `validate`, `lint` and + * `compile` at once. Do not re-wire the commands. + */ +export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [ + { name: 'validateObjectReferences', run: validateObjectReferences }, + { name: 'validateActionNameRefs', run: validateActionNameRefs }, + { name: 'validatePageFieldBindings', run: validatePageFieldBindings }, + { name: 'validateChartBindings', run: validateChartBindings }, + { name: 'validateNavAccess', run: validateNavAccess }, + { name: 'validateTranslationReferences', run: validateTranslationReferences }, +]; + +/** + * Run every reference-integrity rule over a stack. Returns the concatenated + * findings (empty = clean). Pure: no I/O, safe on both the schema-parsed stack + * and the raw/normalized config the `lint` path carries. + */ +export function validateReferenceIntegrity(stack: Record): ReferenceIntegrityFinding[] { + const findings: ReferenceIntegrityFinding[] = []; + for (const rule of REFERENCE_INTEGRITY_RULES) { + findings.push(...rule.run(stack)); + } + return findings; +} From 0053774758ca690100a487c799784c40f4c9e580 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 07:16:11 +0000 Subject: [PATCH 2/3] =?UTF-8?q?docs(audit):=20record=20the=20HotCRM=20resi?= =?UTF-8?q?dual=20and=20close=20out=20#3583's=20ratchet=20(=C2=A76.4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §6.4 asked for a re-run of the audit after the rules landed, so the issue could close with an explicit accepted-gap statement instead of silence. This is that re-run, against the same corpus the original audit read (hotcrm @ 8b28fa2, v2.2.2, ~18k lines). Results: 23 findings, no false positives, mapping category-for-category onto the issue's list — `optionsFrom.object: 'user'` on four dashboards, the three undefined bulk actions plus two row actions, four report axes naming raw fields, the `sys_approval_process` navigation, two nav-exposed objects granted by none of the six permission sets, and six orphan translation keys. Three things worth recording beyond the counts: - Categories 4 and 5a report zero, and the zero is real: injecting a bogus field into HotCRM's `record:highlights` components moves the page rule from 0 to 3, so the walk reaches the pages and the corpus is simply clean there. - The corpus caught a false-positive class before release (~40 `_views` keys), because a view record is a container whose object binding lives inside `list.data.object`. The examples missed it since none of their bundles carry `_views` keys at all — so §6.1's false-positive floor has to be read per branch, not per rule. - HotCRM no longer parses against the dev spec at all (24 Zod errors: `enable.trash`/`enable.mru`, `body` on non-script actions). Pre-existing drift, unrelated to this work, but it is why the lint path taking raw config matters: an app pinned to a published spec can be un-`validate`-able against main. Accepted gaps are named with their evidence: hook body write sets (D4, documented at the authoring surfaces), knowledge indexes (D1 — no definition site, 2 refs unresolvable by construction), skill hand-off (free-form prompt text, §7 non-goal), and R8's option-value literals (Tier-B, needs a verification note first). The one buildable gap left is R7: HotCRM declares 8 agent→skill and 16 skill→tool references against a stack with zero tools — every one dead, as the original audit reported. It needs D1 and D2 decided first, which are spec questions rather than lint ones, so it belongs in its own issue. §5 D5 is also marked done, pointing at the suite entry point. Refs #3583 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015r31hzuSN19iK8ATRPcCpS --- ...metadata-reference-integrity-assessment.md | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/docs/audits/2026-07-app-metadata-reference-integrity-assessment.md b/docs/audits/2026-07-app-metadata-reference-integrity-assessment.md index 74706724c3..1d101e98af 100644 --- a/docs/audits/2026-07-app-metadata-reference-integrity-assessment.md +++ b/docs/audits/2026-07-app-metadata-reference-integrity-assessment.md @@ -116,7 +116,7 @@ Field-level checks inside a cross-package object stay S3 (skip) — we cannot ju - **D2 — runtime-registered skills/tools vs. static lint.** `ask`/`build` kernel agents register skills/tools via service at boot (same invisibility as plugin objects). Extend the Phase-1 registry with official skill/tool names, or run R7 at S2 advisory severity. Small decision; take it when R7 starts. - **D3 — where does nav-access belong?** R5-as-lint-warning (proposed) vs. folding into `compile`'s access-matrix snapshot gate. Lint wins on path symmetry (ADR-0078); the snapshot gate stays the drift detector. - **D4 — hook body writes are not statically checkable, say so.** The write set lives in opaque JS (`hook-body.zod.ts`). Options: (a) accept the gap and document it (hook writes are the one HotCRM category with no static answer); (b) a structured `writes: [field]` declaration on `HookSchema` that the runtime enforces (contract-first, but new spec surface + runtime work); (c) registration-time dev diagnostics (ADR-0078 §4, deferred/evidence-gated there). Recommendation: (a) now, file (b) as its own issue — do not let this category stall the other eight. **Decided 2026-07-27 (#3700): (a); revised 2026-07-28 — (b) dropped, #3700 closed as not planned.** The gap stays documented at the authoring surfaces (`content/docs/automation/hook-bodies.mdx` "Not statically checked: the write set"; `ScriptBodySchema` TSDoc); the documentation is the whole disposition, so ADR-0107, which only recorded it, was withdrawn. (b) will not be built — the declaration duplicates the write set in a second place the author must keep in sync, adding authoring friction and a new error surface (worst for AI authors); in practice, exercising hooks against a SQL driver (in-memory SQLite) covers the failure mode that schemaless `memory://` green lights hide. (c) remains deferred under ADR-0078 §4, unchanged. -- **D5 — rule-suite wiring drift.** `validate`/`lint`/`compile`/`doctor` each hand-pick rule subsets. A shared "run the reference-integrity suite" entry point in `@objectstack/lint` would make the next rule's wiring a one-liner and end the drift. Worth doing with Phase 2, not before. +- **D5 — rule-suite wiring drift. Done 2026-07-28.** `validateReferenceIntegrity` + `REFERENCE_INTEGRITY_RULES` (`packages/lint/src/reference-integrity-suite.ts`) is the single entry point; `validate`/`lint`/`compile` each call it once, so a new rule is a one-line edit to the list. Membership is a written-out list mirrored by a test (a rule joins by review, not by accident), and the suite test runs one live instance per member so a member that stops being invoked fails loudly instead of reading as a clean stack. `doctor` is deliberately NOT converted — it runs only `validateWidgetBindings` and is an environment health check, not an authoring gate, so adopting the suite there is a product decision rather than a wiring cleanup; the asymmetry is named in the module doc so it stays visible. --- @@ -127,7 +127,38 @@ Per ADR-0078 §5 and the audit discipline (`2026-06-metadata-functional-complete 1. **HotCRM is the regression corpus.** Each of the ~20 shipped instances becomes a fixture in the owning rule's `.test.ts` (reduced to minimal stacks — HotCRM itself isn't vendored). A rule ships only when it catches its instances *and* runs clean over `examples/`, the showcase app, and the platform seed apps (zero false positives — the ADR-0072 D1 trust contract: one dead finding and authors stop trusting the linter). 2. **Each Phase-2/3 rule lands separately** (one PR each), behind its verification pass. No omnibus "reference integrity" mega-PR. 3. **Phase 1's registry gets conformance tests in the owning packages** the same day the list lands — an unhonest registry is worse than the prefix heuristic it replaces. -4. Re-run the HotCRM audit (or its fixture corpus) after Phase 2 and record the residual: what still passes cleanly and *why* (D1/D2/D4 categories), so the issue can be closed with an explicit accepted-gap statement instead of silence. +4. Re-run the HotCRM audit (or its fixture corpus) after Phase 2 and record the residual: what still passes cleanly and *why* (D1/D2/D4 categories), so the issue can be closed with an explicit accepted-gap statement instead of silence. **Done 2026-07-28 — see §6a.** + +## 6a. Residual after Phases 0–3 (the §6.4 re-run, 2026-07-28) + +**Method.** `objectstack-ai/hotcrm` @ `8b28fa2` (v2.2.2 — the same repo the original audit read, ~18k lines of shipped metadata), checked with the workspace build of the suite via `os lint` (the raw-config path). `os validate` cannot be the vehicle: HotCRM trips **24 Zod errors** against the current dev spec — `enable.trash` / `enable.mru` (removed in the 16.x line, #2377/ADR-0049) and `body` on non-`script` actions — so the parse aborts before any rule runs. That drift is pre-existing and unrelated to this work, but it is worth naming: **an app pinned to a published spec can be un-`validate`-able against `main`**, which is exactly why the lint path takes raw config. + +**What the suite now catches on the real corpus — 23 findings, no false positives:** + +| Rule | Findings | Issue category | The shipped instance | +|---|---|---|---| +| `object-reference-unknown` | 4 | 1b | `optionsFrom.object: 'user'` on four dashboards (the platform object is `sys_user`) | +| `action-name-undefined` | 5 | 3 | `bulkActions: ['mass_update','mass_delete','assign_owner']` + `rowActions: ['edit','delete']` | +| `chart-measure-unknown` | 4 | 7 | report `yAxis` naming raw fields (`amount`, `case_number`) instead of dataset measures | +| `translation-target-unknown` | 6 | 9 | `apps.crm_enterprise.navigation.group_products` / `.group_analytics` × 3 locales; the app declares `group_sales` / `group_work` / `group_marketing` / `group_service` / `group_insights` | +| `object-reference-unregistered-platform` | 2 | 2 | navigation `objectName` + `requiresObject` on `sys_approval_process`, which nothing registers | +| `nav-object-ungranted` | 2 | 8 | `crm_knowledge_article`, `crm_forecast` exposed in navigation, granted by none of the 6 declared permission sets | + +Categories **4** (page field bindings) and **5a** (hook conditions) report **zero** — and that zero is real, not an empty walk: injecting one bogus field into HotCRM's `record:highlights` components moves `validate-page-field-bindings` from 0 to 3 findings, so the walk reaches the pages and the corpus is simply clean there today (HotCRM has moved on since the audit). + +**A false positive the corpus caught before release.** The first cut of R6 reported ~40 `_views` keys as orphans — all correct. A view record is a *container* (`list`, `listViews.`, `formViews.`) whose object binding lives at `list.data.object`, not at the record root, so the record resolved to nothing and was skipped whole. The examples missed it because none of their bundles carry `_views` keys at all: the walk ran, but that branch of the universe never did. **The §6.1 false-positive floor has to be read per branch, not per rule** — "zero findings on examples/" is only evidence for the branches those examples actually exercise. + +**What still passes silently — the accepted gaps:** + +| Gap | Status | Evidence on HotCRM | +|---|---|---| +| **5b — hook body write set** | Accepted, documented (§5 D4). Not statically checkable: the write set lives in an opaque JS body. Marked at the authoring surfaces (`content/docs/automation/hook-bodies.mdx`, `ScriptBodySchema` TSDoc). | 24 hooks, write sets unchecked by construction | +| **6 — AI references** (R7 unbuilt) | **The one category with resolvable references and no rule.** | 2 agents / 6 skills declaring **8** agent→skill refs and **16** skill→tool refs — against a stack that declares **zero tools**, so every one of those 16 is dead today, exactly as the original audit reported | +| **6 — knowledge indexes** | Blocked on **D1**: `agent.knowledge.indexes` has no definition site in a stack (`KnowledgeSourceSchema` is never referenced by `stack.zod.ts`), so a rule would institutionalise the gap. | 2 knowledge refs, unresolvable by construction | +| **6 — skill hand-off** | Non-goal (§7): the hand-off lives in free-form prompt text, which has no structure to check. | — | +| **R8 — option-value literals in other metadata** | Tier-B candidate, unbuilt: kanban group values, `ChartConfig.colors` keys, filter literals. Needs a verification note before it becomes a rule (high false-positive surface — filters legitimately hold dynamic values). | not measured | + +**Disposition for #3583.** Eight of the nine categories are now gated on `validate` / `lint` / `compile`; the ninth (5b) is an accepted, documented gap. The remaining buildable work is **R7** — and it is worth its own issue rather than keeping this one open, because it needs decision **D1** (a spec change or an `[EXPERIMENTAL]` marker) and **D2** (runtime-registered kernel skills/tools vs. static lint) first, neither of which is a lint question. ## 7. Non-goals From f0094d294e569cc210192df364166891454cc205 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 07:19:47 +0000 Subject: [PATCH 3/3] chore(changeset): reference-integrity suite entry point (#3583 D5) The suite adds public API to @objectstack/lint (validateReferenceIntegrity, REFERENCE_INTEGRITY_RULES and its types), so it releases as a minor there and a patch on the CLI, whose behaviour is unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015r31hzuSN19iK8ATRPcCpS --- .changeset/lint-reference-integrity-suite.md | 30 ++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .changeset/lint-reference-integrity-suite.md diff --git a/.changeset/lint-reference-integrity-suite.md b/.changeset/lint-reference-integrity-suite.md new file mode 100644 index 0000000000..958e205c03 --- /dev/null +++ b/.changeset/lint-reference-integrity-suite.md @@ -0,0 +1,30 @@ +--- +"@objectstack/lint": minor +"@objectstack/cli": patch +--- + +refactor(lint): one entry point for the reference-integrity suite (#3583 D5) + +Six rules that answer the same question — "does this name resolve to anything?" +— were wired by hand into three CLI commands, so landing a rule meant editing +`validate`, `lint` and `compile`, and forgetting one meant the same stack got a +different verdict depending on which command the author ran. + +New public API on `@objectstack/lint`: + +- `validateReferenceIntegrity(stack)` — runs every reference-integrity rule and + returns the concatenated findings. +- `REFERENCE_INTEGRITY_RULES` — the ordered list behind it (`validateObjectReferences`, + `validateActionNameRefs`, `validatePageFieldBindings`, `validateChartBindings`, + `validateNavAccess`, `validateTranslationReferences`). +- `ReferenceIntegrityFinding` / `ReferenceIntegrityRule` / `ReferenceIntegritySeverity` + — one finding type instead of a six-way union. + +Adding a rule to that list reaches `validate`, `lint` and `compile` with no +further wiring. The individual rule exports are unchanged, so nothing that +imports them directly needs to move. + +Behaviour-preserving: identical findings on the three example apps (zero) and +on the HotCRM corpus (24, unchanged per rule). `os doctor` is deliberately not +converted — it runs only `validateWidgetBindings` and is an environment health +check rather than an authoring gate.