From 878e02ffe80a1001b055d60645e0ce941bef543f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 07:46:31 +0000 Subject: [PATCH 1/3] fix(cli): every author-time rule that can gate runs on all three commands (#4409) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `os validate`, `os build` and `os lint` each hand-wired their own subset of the author-time rules. Nothing connected the three lists, so "which rules run here?" was answerable only by diffing three 800-line files by eye — and the answer drifted every time a rule landed. The audit found 23 of 26 rules running on some strict subset, nine of them able to emit `error`. The worst direction was the least obvious: `os build` — the command that PUBLISHES — was the weakest gate of the three. A flow whose expression approver does not parse (`approval-expression-invalid`) built and published green; only `os lint` stopped it, and CI usually runs the other two. This is the same failure mode's fifth appearance (#3583, #3782, #4384/#4394, #4402). Each earlier repair removed an instance and left the MODE: a rule's command coverage was whatever its author remembered to type, and forgetting was silent. #4402's guard could not catch the rest — it filtered on the current member names of one suite, so a rule hand-wired into two commands from outside that suite passed it without a word. Replace remembering with a table: - `packages/cli/src/lint/authoring-rules.ts` declares all 26 rules as data — tier (gating/advisory), which stack tier they read (pre-parse vs parsed), which commands run them, and a written reason for the one narrowing. All three commands consume it through `runAuthoringRules()`; the three command files shrink by ~1000 lines between them. - `authoring-rule-wiring.test.ts` upgrades the guard from a name list to a ratchet: a gating rule on fewer than three commands fails, a narrowed rule with no reason fails, a command that calls a rule directly fails, and an `advisory` claim is checked against the rule's own source so a gate cannot wear an advisory label to buy partial coverage. Remaining direct calls are listed with reasons in `DIRECT_CALL_RATCHET` / `LINT_IMPORT_RATCHET`. - `authoring-rule-command-parity.test.ts` proves the verdict, not just the wiring: one case per previously-blind gating rule, plus the issue's own repro driven through the real CLI — exit 1 on all three commands where it was 1/0/0 before. Two things fall out of one report per run rather than exiting at the first failing gate: an author with three unrelated problems sees all three, and `--strict` now covers every advisory instead of the roughly half that happened to be printed inline. Also closes the same hole one gate over: `collectAndLintDocs` failed `os build` and never ran on `os validate`, invisible because the parity guard keyed on the `lint*`/`validate*` naming convention and that gate is called `collect*`. The guard now names each shared non-registry gate explicitly. Cost is not what argued against this: the heavy deps (typescript, sucrase) are already lazy, and the heaviest rule of the set has run on all three commands as a suite member since #4340 without anyone noticing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Sz61GE1CWCSnGW4qShSEXs --- content/docs/deployment/cli.mdx | 39 +- .../docs/deployment/validating-metadata.mdx | 130 ++-- .../commands/authoring-rule-wiring.test.ts | 330 +++++++++ packages/cli/src/commands/compile.ts | 480 ++----------- packages/cli/src/commands/lint.ts | 205 ++---- .../reference-integrity-wiring.test.ts | 82 --- packages/cli/src/commands/validate.ts | 639 +++--------------- packages/cli/src/lint/authoring-rules.ts | 594 ++++++++++++++++ packages/cli/src/utils/sdui-manifest.ts | 43 ++ .../authoring-rule-command-parity.test.ts | 195 ++++++ .../test/validate-build-gate-parity.test.ts | 136 ++-- 11 files changed, 1554 insertions(+), 1319 deletions(-) create mode 100644 packages/cli/src/commands/authoring-rule-wiring.test.ts delete mode 100644 packages/cli/src/commands/reference-integrity-wiring.test.ts create mode 100644 packages/cli/src/lint/authoring-rules.ts create mode 100644 packages/cli/src/utils/sdui-manifest.ts create mode 100644 packages/cli/test/authoring-rule-command-parity.test.ts diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 9f9be95817..54c90ba576 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -435,6 +435,14 @@ os validate path/to/config # Validate specific file `dataset` / `dimensions` / `values` resolves to a declared dataset/field, so a dangling binding fails here instead of rendering an empty chart. +…and every other author-time rule the three commands share — view shape, +name/action/filter references, page sources, approval approvers, security +posture, the autonumber and view-reference lints. All of them come from one +registry, so the list is the same on `os build` and `os lint`; see +[The one gate, three entry points](/docs/deployment/validating-metadata#the-one-gate-three-entry-points) +for the full matrix. Every failing rule is reported in a single run rather than +stopping at the first, so one pass shows the whole hole. + **Options:** - `--strict` — Treat warnings as errors (exit code 1) - `--json` — Output results as JSON @@ -444,13 +452,17 @@ os validate path/to/config # Validate specific file - Missing `manifest.namespace` (required for multi-app hosting) - No objects defined - No apps or plugins defined +- Every advisory the rule registry raised (dangling semantic-role pointers, + replay-unsafe seeds, ambiguous flow status, deprecated visibility aliases, …) -`os validate` and `os build` share one validator, so a config that passes -`os validate` will not fail the build on schema/predicate/binding grounds. In a -scaffolded project these are wired as `npm run validate` and `npm run build`; -your `AGENTS.md` tells coding agents to run `npm run validate` after editing -metadata. See [Validating metadata](/docs/deployment/validating-metadata). +`os validate`, `os build` and `os lint` share one rule registry, so a config that +passes any of them will not fail another on schema/predicate/binding grounds — a +CLI test fails the build if a rule that can gate runs on fewer than all three +(#4409). In a scaffolded project these are wired as `npm run validate` and +`npm run build`; your `AGENTS.md` tells coding agents to run `npm run validate` +after editing metadata. See +[Validating metadata](/docs/deployment/validating-metadata). #### `os info` @@ -819,21 +831,32 @@ os create example my-app # Create examples/my-app | Command | Description | |---------|-------------| -| `os lint [config]` | Check metadata for style and convention issues (beyond `validate`'s hard gates) | +| `os lint [config]` | Every author-time gate `validate`/`build` run, plus style and convention checks | | `os test [files]` | Run Quality Protocol test scenarios against a running server | | `os doctor` | Check development environment health | #### `os lint` -Style and convention checks on top of `os validate` — naming, labels, translation coverage — with a 0-100 quality score: +The cheapest of the three author-time commands. It runs the same rule registry +`os validate` and `os build` run — so anything that can fail a build fails here +too — and adds its own style rubric: naming, labels, namespace prefixes, +data-model conventions, translation coverage, with a 0-100 quality score. ```bash -os lint # Style / convention checks +os lint # Author-time rules + style / convention checks os lint --score # Append a 0-100 metadata quality score (letter-graded) os lint --fix # Show what would be fixed (dry-run) os lint --json # JSON output for CI ``` +It does not replace `os validate`: `os lint` never parses the stack against the +Zod schema (a schema error is `os validate`'s verdict to give), and it emits no +artifact. What it does guarantee is the direction that matters for a pre-flight +— a green `os lint` is not followed by a red `os build`. That was not true +before #4409: `os lint` ran one gating rule neither other command ran and missed +six that both of them ran, so it disagreed with the build in **both** +directions. + #### `os test` Runs Quality Protocol test scenarios (JSON-based BDD) against a running ObjectStack server. diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index 5cadf05f09..50db7b96c3 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -318,51 +318,67 @@ Skipped, to keep false positives at zero: the same set as §8 — non-static values, `{...spread}` usages, relationship paths, system fields, and objects another package defines. -## The one gate, two entry points - -`os validate` and `os build` (alias of `os compile`) run the **same** validator: - -| | `os validate` | `os build` | -|---|---|---| -| Protocol schema (Zod) | ✓ | ✓ | -| CEL / predicate validation | ✓ | ✓ | -| Widget-binding integrity | ✓ | ✓ | -| Dashboard action/route references (ADR-0049) | ✓ | ✓ | -| Object & action name references (#3583) | ✓ | ✓ | -| Page-component field bindings (#3583) | ✓ | ✓ | -| React page block field bindings — §10 (#4340) | ✓ | ✓ | -| Chart bindings outside dashboards (#3583) | ✓ | ✓ | -| Navigation vs. granted access (ADR-0090 D6) | ✓ | ✓ | -| Security posture (ADR-0090 — e.g. every custom object declares `sharingModel`) | ✓ | ✓ | -| Autonumber `{field}` interpolation | ✓ | ✓ | -| View references — form targets, view-key collisions (#2554) | ✓ | ✓ | -| Flow authoring anti-patterns (#1874) | ✓ | ✓ | -| Liveness author-warnings | ✓ | ✓ | -| Undeclared authoring keys — every metadata collection (#3786) and the stack's own top-level keys (#4167) | ✓ | ✓ | -| Emits `dist/objectstack.json` | — | ✓ | - -So `os validate` is the fast inner-loop check (no artifact); `os build` is what -you run when you need the deployable artifact. A config that passes `os validate` -will not fail `os build` on schema/predicate/binding grounds — a test in the CLI -asserts that every gate `os build` runs is also run by `os validate`, so the two -cannot drift apart again (#3782). Both entry points -also check SDUI styling (ADR-0065), and `os validate` additionally runs a set of -view- and page-SHAPE checks — list-view navigation modes (ADR-0053), view -container shape, and whether a JSX/React page source parses at all -(ADR-0080/0081) — that catch UI metadata which would otherwise be silently -dropped. - -The field bindings INSIDE a react page source are a different matter: they are -reference-integrity, so they run wherever the suite runs. `os lint` gets them -too — it shares the same `REFERENCE_INTEGRITY_RULES` list, which is why the -table's reference rows are the ones a cheap pre-flight can rely on. That was not -always true: the react-page prop gate was hand-wired into `os validate` alone -until #4340's follow-up, so `os lint` and `os build` accepted a page whose every -field binding was stale — the same divergence #4394 closed for readonly flow -writes. A CLI test now asserts no command reaches for a suite member directly -(#4384). - -A clean run walks each gate and reports timing: +## The one gate, three entry points + +`os validate`, `os build` (alias of `os compile`) and `os lint` run the **same** +author-time rules, from one table — `AUTHORING_RULES` in +`packages/cli/src/lint/authoring-rules.ts`: + +| | `os validate` | `os build` | `os lint` | +|---|---|---|---| +| Protocol schema (Zod) | ✓ | ✓ | — | +| CEL / predicate validation (ADR-0032) | ✓ | ✓ | ✓ | +| List-view navigation modes (ADR-0053) | ✓ | ✓ | ✓ | +| View container shape | ✓ | ✓ | ✓ | +| Widget-binding integrity (ADR-0021) | ✓ | ✓ | ✓ | +| Dashboard action/route references (ADR-0049) | ✓ | ✓ | ✓ | +| Filter placeholder resolvability (#3574) | ✓ | ✓ | ✓ | +| Object & action name references (#3583) | ✓ | ✓ | ✓ | +| Page-component field bindings (#3583) | ✓ | ✓ | ✓ | +| React page block field bindings — §10 (#4340) | ✓ | ✓ | ✓ | +| Chart bindings outside dashboards (#3583) | ✓ | ✓ | ✓ | +| Navigation vs. granted access (ADR-0090 D6) | ✓ | ✓ | ✓ | +| SDUI scoped styling (ADR-0065) | ✓ | ✓ | ✓ | +| JSX / React page source parses (ADR-0080/0081) | ✓ | ✓ | ✓ | +| Approval-node approvers (ADR-0090 D3) | ✓ | ✓ | ✓ | +| Security posture (ADR-0090 — e.g. every custom object declares `sharingModel`) | ✓ | ✓ | ✓ | +| Organization-axis red lines (ADR-0105 D6) | ✓ | ✓ | ✓ | +| Autonumber `{field}` interpolation | ✓ | ✓ | ✓ | +| View references — form targets, view-key collisions (#2554) | ✓ | ✓ | ✓ | +| Flow authoring anti-patterns (#1874) | ✓ | ✓ | ✓ | +| Advisory: flow trigger wiring, record titles, semantic roles, seed replay/state safety, capability references, liveness, visibility aliases | ✓ | ✓ | ✓ | +| Package docs — flatness, prefixes, links (ADR-0046) | ✓ | ✓ | ✓ | +| Undeclared authoring keys — every metadata collection (#3786) and the stack's own top-level keys (#4167) | ✓ | ✓ | — | +| Naming, labels, data-model conventions, i18n coverage | — | — | ✓ | +| Emits `dist/objectstack.json` | — | ✓ | — | + +So `os validate` is the fast inner-loop check (no artifact), `os build` is what +you run when you need the deployable artifact, and `os lint` adds its own style +rubric on top. **Any rule that can fail a build runs on all three**, so a green +`os lint` means the build's gates are green too, and a stack cannot be published +through the one command that happens to skip a check. + +Two rows are deliberately not universal, and both are one-directional (neither +lets a stack through a gate another command enforces): the Zod parse and the +undeclared-key diff need the pre-parse tier and the schema, which only the two +commands that parse actually have; and `os lint`'s own rubric — snake_case +names, missing labels, data-model conventions — is a lint verdict, not a publish +gate. `os build` has never rejected a camelCase object name. + +That invariant is enforced, not merely documented. Each rule declares its command +coverage as data, and a CLI test fails if a rule that can emit `error` runs on +fewer than all three, if a narrowed rule carries no written reason, or if any +command reaches for a rule directly instead of going through the registry. + +The enforcement exists because the contract drifted four separate times, and the +last audit (#4409) found 23 of 26 rules running on some strict subset of the +three — nine of them able to fail a build. The worst direction was the least +obvious: **`os build` was the weakest of the three gates**, so it emitted an +artifact for stacks the other two refuse. A flow whose expression approver did +not parse built and published green; only `os lint` stopped it, and CI usually +runs the other two. + +A clean run walks the registry and reports timing: ``` ◆ Validate @@ -371,19 +387,9 @@ A clean run walks each gate and reports timing: Config: /path/to/support-desk/objectstack.config.ts Load time: 21ms → Validating against ObjectStack Protocol... - → Validating expressions (ADR-0032)... - → Checking list-view navigation modes (ADR-0053)... - → Checking view container shape... - → Checking dashboard widget bindings (ADR-0021)... - → Checking dashboard action references (ADR-0049)... - → Checking SDUI styling (ADR-0065)... - → Checking JSX-source pages (ADR-0080)... - → Checking React-source pages (ADR-0081)... - → Checking source-page styling (ADR-0065)... - → Checking capability references (ADR-0066)... - → Checking flow trigger wiring... - → Running authoring lints (#3782)... - → Checking security posture (ADR-0090 D7)... + → Running author-time rules (26)... + → Checking capability providers (#3366)... + → Checking package docs (ADR-0046)... ✓ Validation passed (64ms) @@ -397,9 +403,11 @@ see [the gate in action](/docs/getting-started/build-with-claude-code#4-the-gate for the bare-reference example verbatim. -`os lint` is a **separate** pass — style and convention checks (snake_case -naming, required labels, namespace prefixes, data-model patterns). Run it too, -but it does not replace `os validate`, and `os validate` does not replace it. +`os lint` runs every gate above **plus** its own style rubric (snake_case +naming, required labels, namespace prefixes, data-model patterns, translation +coverage). It does not replace `os validate` — it never parses against the Zod +schema, so a schema error is `os validate`'s verdict to give — but a rule that +can fail the build fails `os lint` too. ## The workflow diff --git a/packages/cli/src/commands/authoring-rule-wiring.test.ts b/packages/cli/src/commands/authoring-rule-wiring.test.ts new file mode 100644 index 0000000000..4e87a70a5c --- /dev/null +++ b/packages/cli/src/commands/authoring-rule-wiring.test.ts @@ -0,0 +1,330 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The ratchet behind `lint/authoring-rules.ts` (#4409). Supersedes +// `reference-integrity-wiring.test.ts`, which guarded the same seam for one +// rule family (#3583 §5 D5, #4384) — the suite is now one entry in the registry +// this file guards, so its invariants are carried below rather than duplicated. +// +// ## Why a test and not a comment +// +// The defect is not a wrong result — it is a MISSING CALL, and a missing call +// produces no failing assertion anywhere. Every command's own tests pass, every +// rule's unit tests pass, and the only symptom is that `os validate`, `os build` +// and `os lint` quietly disagree about the same stack. +// +// That failure mode was repaired four times before anyone guarded the mode +// itself: the reference-integrity suite (#3583), the four CLI-local authoring +// lints wired into `build` alone (#3782), `validateReadonlyFlowWrites` missing +// from `lint` (#4384/#4394), and the name-list guard that followed (#4402). +// Each repair removed an instance. #4409 measured what was left: 23 of 26 rules +// on a strict subset of the three commands, nine of them able to emit `error`, +// and `os build` — the command that PUBLISHES — the weakest gate of the three. +// +// #4402's guard could not have caught any of it. It filtered on the CURRENT +// member names of one suite, so a rule wired by hand into two commands from +// outside that suite passed it in silence. A name list only guards the names on +// it; a ratchet guards the shape. +// +// ## The invariants +// +// 1. A rule that can emit `error` runs on all three commands. A gate is only as +// strong as the weakest command CI happens to run. +// 2. A rule that runs on fewer than three carries a written reason. +// 3. An `advisory` claim is TRUE — checked against the rule's own source, so +// the tier cannot be used to launder a gate into partial coverage. #3760 +// promoted a `lintFlowPatterns` rule from advisory to gating and nothing +// anywhere asked whether its coverage should follow. +// 4. No command hand-wires a rule. Every remaining direct call is on the +// ratchet below with a reason, and adding one is an explicit edit. +// +// ## Why it scans source +// +// vitest inlines imports through its transform, so a spy on `@objectstack/lint` +// cannot prove which symbols a command file actually reaches for — the same +// reason `@objectstack/lint`'s `lazy-deps.test.ts` scans `src/` rather than +// probing a module cache. Behavioural coverage of what each rule FINDS lives in +// that rule's own tests; this file guards only the seam between the registry and +// the three call sites. + +import { existsSync, readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, it, expect } from 'vitest'; +import { REFERENCE_INTEGRITY_RULES } from '@objectstack/lint'; +import { + AUTHORING_COMMANDS, + AUTHORING_RULES, + authoringRulesFor, + type AuthoringCommand, +} from '../lint/authoring-rules.js'; + +const commandsDir = dirname(fileURLToPath(import.meta.url)); +const repoRoot = join(commandsDir, '..', '..', '..', '..'); + +/** The command source file each authoring command lives in. */ +const COMMAND_FILES: Readonly> = { + validate: 'validate.ts', + build: 'compile.ts', + lint: 'lint.ts', +}; + +const sourceOf = (file: string) => readFileSync(join(commandsDir, file), 'utf8'); + +/** + * Rules a command file may still call directly, each with the reason it is not + * a registry entry. + * + * This is the ratchet — the `FLOW_WRITE_NODE_TYPES_DEFERRED` / `TEST_DEBT` + * discipline applied to rule wiring. Adding a key here is a deliberate claim + * that the rule is NOT one of the three commands' shared author-time checks; + * anything else belongs in `AUTHORING_RULES`, where all three commands get it + * at once. A rule that merely reads the stack does not qualify, no matter how + * convenient the local call site is. + */ +const DIRECT_CALL_RATCHET: Readonly> = { + lintConfig: + "`os lint`'s own entry point, defined in lint.ts itself — the rubric (naming, labels, structure), " + + 'not a shared authoring rule. Its `error` severity is a lint verdict, not a publish gate.', + lintDataModel: + "`os lint`'s data-model best-practice sweep (ADR-0035 conventions + the eval rubric in score.ts). " + + 'Deliberately lint-only: `os build` has never rejected a lookup that should have been a ' + + 'master_detail, and making it do so is a product decision, not a wiring fix.', + lintUnknownStackKeys: + 'Needs `ObjectStackDefinitionSchema` to diff the authored keys against what the schema declares, ' + + 'and must read the PRE-parse stack, which only the two commands that parse actually have. ' + + '`os lint` never parses, so it has nothing to diff against.', + lintUnknownAuthoringKeys: + 'The object/field half of the same pre-parse key diff (#3786) — wired with, and for the same ' + + 'reason as, `lintUnknownStackKeys`.', +}; + +/** + * Symbols a command file may import from `@objectstack/lint` without being a + * registry entry. Same ratchet discipline, on the import rather than the call — + * `buildAccessMatrix`/`diffAccessMatrix` do not match the `lint*`/`validate*` + * naming convention the call-site scan keys on, so the import scan is what + * covers them. + */ +const LINT_IMPORT_RATCHET: Readonly> = { + buildAccessMatrix: + '[ADR-0090 D6] Derives the (permission set × object) capability matrix. Not a rule — it produces ' + + 'the snapshot `os build` diffs against a committed file, so it is an artifact step, not a check.', + diffAccessMatrix: + 'The other half of the D6 snapshot gate: it compares a committed `access-matrix.json` against the ' + + 'matrix above. Reads a file next to the config, so it cannot run where that file may not exist.', +}; + +/** Every registry rule name, plus the member names of the suite it embeds. */ +const REGISTRY_NAMES = new Set([ + ...AUTHORING_RULES.map((r) => r.name), + ...REFERENCE_INTEGRITY_RULES.map((r) => r.name), +]); + +/** Every `lintFoo(`/`validateFoo(` call site in a source file. */ +function ruleCallsIn(source: string): string[] { + return [...new Set(source.match(/\b(?:lint|validate)[A-Z]\w*(?=\s*\()/g) ?? [])]; +} + +/** Every symbol imported from `@objectstack/lint` by a source file. */ +function lintImportsIn(source: string): string[] { + const names: string[] = []; + const importRe = /import\s+(?:type\s+)?\{([^}]*)\}\s*from\s*['"]@objectstack\/lint['"]/g; + for (const m of source.matchAll(importRe)) { + for (const raw of m[1].split(',')) { + const name = raw.trim().replace(/^type\s+/, '').split(/\s+as\s+/)[0].trim(); + if (name) names.push(name); + } + } + return [...new Set(names)]; +} + +/** + * The body of `export function `, up to the next top-level `export` (or + * end of file) — so a module hosting several rules is read one rule at a time. + * + * Known limitation, stated rather than hidden: a finding emitted from a helper + * declared ABOVE the export is outside the slice. The check is a ratchet on the + * common shape (a rule emits its own findings), not a proof. + */ +function ruleBody(source: string, name: string): string | null { + const start = source.indexOf(`export function ${name}`); + if (start < 0) return null; + const rest = source.slice(start + 1); + const end = rest.indexOf('\nexport '); + return end < 0 ? rest : rest.slice(0, end); +} + +/** Does this rule body emit `severity: 'error'`? (Union type declarations do not count.) */ +function emitsError(body: string): boolean { + const withoutTypeDecls = body + .split('\n') + .filter((line) => !/severity\??:\s*(?:'[a-z]+'\s*\|\s*)+'[a-z]+'/.test(line)) + .join('\n'); + return /severity:[^;\n]*'error'/.test(withoutTypeDecls); +} + +describe('authoring-rule registry wiring (#4409)', () => { + it.each([...AUTHORING_COMMANDS])('os %s runs the registry and nothing by hand', (command) => { + const source = sourceOf(COMMAND_FILES[command]); + expect(source, `${COMMAND_FILES[command]} must run the registry`).toMatch(/\brunAuthoringRules\s*\(/); + + const handWired = ruleCallsIn(source).filter((name) => REGISTRY_NAMES.has(name)); + expect( + handWired, + `${COMMAND_FILES[command]} calls registry rule(s) directly: ${handWired.join(', ')}.\n` + + `Run them through runAuthoringRules() instead — a per-rule call site is exactly how a rule ` + + `ends up on two commands out of three (#3782, #4394, #4409).`, + ).toEqual([]); + }); + + it.each([...AUTHORING_COMMANDS])('os %s has no unratcheted direct rule call', (command) => { + const source = sourceOf(COMMAND_FILES[command]); + const unratcheted = ruleCallsIn(source) + .filter((name) => !REGISTRY_NAMES.has(name)) + .filter((name) => !(name in DIRECT_CALL_RATCHET)) + .sort(); + + expect( + unratcheted, + `${COMMAND_FILES[command]} hand-wires ${unratcheted.length} rule(s) the registry does not know ` + + `about: ${unratcheted.join(', ')}.\n` + + `Add each to AUTHORING_RULES in packages/cli/src/lint/authoring-rules.ts so all three commands ` + + `run it — or, if it genuinely is not a shared author-time rule (it needs the filesystem, the ` + + `emitted artifact, or it belongs to os lint's own style rubric), add it to DIRECT_CALL_RATCHET ` + + `in this file WITH the reason. Silence is the one option that is not available.`, + ).toEqual([]); + }); + + it.each([...AUTHORING_COMMANDS])('os %s imports no unratcheted symbol from @objectstack/lint', (command) => { + const source = sourceOf(COMMAND_FILES[command]); + const unratcheted = lintImportsIn(source) + .filter((name) => !(name in LINT_IMPORT_RATCHET)) + .sort(); + + expect( + unratcheted, + `${COMMAND_FILES[command]} imports ${unratcheted.join(', ')} from @objectstack/lint directly. ` + + `Register the rule in AUTHORING_RULES, or add the symbol to LINT_IMPORT_RATCHET with a reason.`, + ).toEqual([]); + }); + + // ── The invariant the issue exists for ─────────────────────────────── + + /** + * Gating rules that do NOT yet run on all three commands, each with the reason + * and the plan to close it. + * + * Empty, and that is the healthy state — a stack must not be publishable + * through a command that skips a gate another command enforces. An entry here + * is a KNOWN hole: `os build` may ship what `os lint` refuses, or the reverse. + */ + const GATING_COVERAGE_DEBT: Readonly> = {}; + + it('every gating rule runs on all three commands', () => { + const holes = AUTHORING_RULES.filter((r) => r.tier === 'gating') + .filter((r) => r.commands.length !== AUTHORING_COMMANDS.length) + .filter((r) => !(r.name in GATING_COVERAGE_DEBT)) + .map((r) => `${r.name} (runs on: ${r.commands.join(', ')})`); + + expect( + holes, + `${holes.length} rule(s) can emit \`error\` but do not run on all three authoring commands: ` + + `${holes.join('; ')}.\n` + + `A gate is only as strong as the weakest command an author or CI happens to run, so partial ` + + `coverage is not a stricter check — it is a coin flip. Widen \`commands\` to all three.`, + ).toEqual([]); + }); + + it('the three commands run the identical gating set', () => { + const gatingFor = (command: AuthoringCommand) => + authoringRulesFor(command) + .filter((r) => r.tier === 'gating') + .map((r) => r.name) + .sort(); + + const [validate, build, lint] = AUTHORING_COMMANDS.map(gatingFor); + expect(build, 'os build must gate on exactly what os validate gates on').toEqual(validate); + expect(lint, 'os lint must gate on exactly what os validate gates on').toEqual(validate); + }); + + it('every narrowed rule carries a reason', () => { + const unexplained = AUTHORING_RULES.filter((r) => r.commands.length !== AUTHORING_COMMANDS.length) + .filter((r) => (r.scopeReason ?? '').trim().length < 40) + .map((r) => r.name); + + expect( + unexplained, + `${unexplained.join(', ')} run(s) on fewer than three commands with no substantive scopeReason. ` + + `A narrowing must be a written decision, not an omission — that distinction IS the fix (#4409).`, + ).toEqual([]); + }); + + it('every rule declares a source file that exists', () => { + const missing = AUTHORING_RULES.filter((r) => !existsSync(join(repoRoot, r.source))).map( + (r) => `${r.name} → ${r.source}`, + ); + expect(missing, `stale source path(s): ${missing.join(', ')}`).toEqual([]); + }); + + it('every advisory rule really is advisory', () => { + // The check that keeps the tier honest: without it, mislabelling a gate as + // advisory would silently buy it the right to partial coverage. + const liars = AUTHORING_RULES.filter((r) => r.tier === 'advisory') + .map((r) => { + const body = ruleBody(readFileSync(join(repoRoot, r.source), 'utf8'), r.name); + if (body === null) return `${r.name} (no \`export function ${r.name}\` in ${r.source})`; + return emitsError(body) ? `${r.name} (${r.source} emits severity: 'error')` : null; + }) + .filter((x): x is string => x !== null); + + expect( + liars, + `${liars.join('; ')}.\n` + + `A rule that can emit \`error\` is \`gating\` and must run on all three commands. Change its ` + + `tier and widen \`commands\` — do not leave a gate wearing an advisory label.`, + ).toEqual([]); + }); + + // ── Guards the guard ───────────────────────────────────────────────── + + it('the registry is non-empty and still holds the rules that motivated it', () => { + expect(AUTHORING_RULES.length).toBeGreaterThan(20); + const names = AUTHORING_RULES.map((r) => r.name); + // The three that `os build` was blind to — it published what the other + // commands refuse. `validateApprovalApprovers` is #4409's worked example. + expect(names).toContain('validateApprovalApprovers'); + expect(names).toContain('validateListViewMode'); + expect(names).toContain('validateViewContainers'); + // The gating pair `os lint` was blind to, so the cheap pre-flight passed + // stacks the build rejects. + expect(names).toContain('lintAutonumberFormats'); + expect(names).toContain('lintViewRefs'); + // The suite #3583/#4402 built, now one entry among the rest. + expect(names).toContain('validateReferenceIntegrity'); + expect(REFERENCE_INTEGRITY_RULES.length).toBeGreaterThan(0); + // The rule whose absence from `os lint` motivated the suite's own guard. + expect(REFERENCE_INTEGRITY_RULES.map((r) => r.name)).toContain('validateReadonlyFlowWrites'); + }); + + it('the source scans still match something (non-vacuous)', () => { + // If the extraction regexes silently stop matching, every set-difference + // above passes while checking nothing. + expect(ruleCallsIn(sourceOf('lint.ts')).length).toBeGreaterThan(0); + expect(lintImportsIn(sourceOf('compile.ts')).length).toBeGreaterThan(0); + expect(emitsError("severity: 'error',")).toBe(true); + expect(emitsError("severity: 'error' | 'warning';")).toBe(false); + expect(emitsError("if (f.severity === 'error') return;")).toBe(false); + }); + + it('every ratchet entry is still load-bearing', () => { + // A ratchet nobody prunes rots into a permission slip. Each entry must + // correspond to a call/import that actually exists somewhere. + const allCalls = new Set(Object.values(COMMAND_FILES).flatMap((f) => ruleCallsIn(sourceOf(f)))); + const staleCalls = Object.keys(DIRECT_CALL_RATCHET).filter((n) => !allCalls.has(n)); + expect(staleCalls, `DIRECT_CALL_RATCHET entries with no call site left: ${staleCalls.join(', ')}`).toEqual([]); + + const allImports = new Set(Object.values(COMMAND_FILES).flatMap((f) => lintImportsIn(sourceOf(f)))); + const staleImports = Object.keys(LINT_IMPORT_RATCHET).filter((n) => !allImports.has(n)); + expect(staleImports, `LINT_IMPORT_RATCHET entries with no import left: ${staleImports.join(', ')}`).toEqual([]); + }); +}); diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index e6a7680dd2..db6686b774 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -15,19 +15,9 @@ import { } from '@objectstack/spec'; import { loadConfig } from '../utils/config.js'; import { lowerCallables } from '../utils/lower-callables.js'; -import { validateStackExpressions } from '@objectstack/lint'; -import { validateVisibilityPredicates } from '@objectstack/lint'; -import { validateWidgetBindings } from '@objectstack/lint'; -import { validateDashboardActionRefs } from '@objectstack/lint'; -import { validateFilterTokens } from '@objectstack/lint'; -import { validateReferenceIntegrity } from '@objectstack/lint'; -import { validateResponsiveStyles } from '@objectstack/lint'; -import { validateSecurityPosture, validateOrgAxisRedLines, buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint'; -import { lintFlowPatterns } from '../utils/lint-flow-patterns.js'; -import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js'; -import { lintUniqueDeclarations } from '../lint/data-model-rules.js'; -import { lintLivenessProperties } from '../utils/lint-liveness-properties.js'; -import { lintViewRefs } from '../utils/lint-view-refs.js'; +import { buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint'; +import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '../lint/authoring-rules.js'; +import { resolveSduiManifest } from '../utils/sdui-manifest.js'; import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js'; import { collectAndLintDocs } from '../utils/collect-docs.js'; import { buildRuntimeBundle, cleanupOldRuntimeBundles } from '../utils/build-runtime.js'; @@ -173,46 +163,62 @@ export default class Compile extends Command { this.exit(1); } - // 3b. Validate expressions against the resolved schema (ADR-0032 §1a/1b). - // The whole normalized stack is in hand here, so flow/validation - // predicates are checked for CEL syntax AND that `record.` - // references exist on the target object — failing the build with a - // located, corrective message instead of a silent runtime `false`. - if (!flags.json) printStep('Validating expressions (ADR-0032)...'); - const exprIssues = validateStackExpressions(result.data as Record); - const exprErrors = exprIssues.filter((i) => i.severity !== 'warning'); - const exprWarnings = exprIssues.filter((i) => i.severity === 'warning'); - if (exprErrors.length > 0) { + // 3b. The author-time rule registry (#4409) — one table, three commands. + // `os build` was the WEAKEST of the three authoring gates before it: + // it published stacks `os validate` or `os lint` refuse, because the + // rules each command ran were whatever its author remembered to wire. + // `validateApprovalApprovers` was the worked example — a flow whose + // expression approver does not parse built and published green while + // `os lint` rejected it. The build is the command that SHIPS, so + // "weakest gate" here means broken metadata reaching an environment. + // + // Which rules run, on which stack tier, and why any of them is scoped + // is declared in `lint/authoring-rules.ts`. Do not add a call site here. + const registered = authoringRulesFor('build'); + if (!flags.json) printStep(`Running author-time rules (${registered.length})...`); + const findings = runAuthoringRules('build', { + normalized: normalized as Record, + parsed: result.data as Record, + sduiManifest: resolveSduiManifest(), + }); + const { errors: ruleErrors, advisories: ruleAdvisories } = splitBySeverity(findings); + + if (ruleAdvisories.length > 0 && !flags.json) { + console.log(''); + for (const f of ruleAdvisories.slice(0, 50)) { + printWarning(`${f.where}: ${f.message}`); + if (f.hint) console.log(chalk.dim(` ${f.hint}`)); + console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`)); + } + } + if (ruleErrors.length > 0) { + // Every failing rule reports at once — see the note in `validate.ts`. if (flags.json) { - await emitJson({ success: false, error: 'expression validation failed', issues: exprErrors, warnings: exprWarnings }, 0, { compact: true }); + await emitJson( + { success: false, error: 'author-time rules failed', issues: ruleErrors, warnings: ruleAdvisories }, + 0, + { compact: true }, + ); this.exit(1); } console.log(''); - printError(`Expression validation failed (${exprErrors.length} issue${exprErrors.length > 1 ? 's' : ''})`); - for (const i of exprErrors.slice(0, 50)) { - console.log(` • ${i.where}: ${i.message}`); - console.log(` source: \`${i.source}\``); + printError(`Author-time rules failed (${ruleErrors.length} issue${ruleErrors.length > 1 ? 's' : ''})`); + for (const f of ruleErrors.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); } - // Advisory expression warnings (#1928 tier 3) — surfaced, never fatal. - if (exprWarnings.length > 0 && !flags.json) { - printWarning(`Expression warnings (${exprWarnings.length})`); - for (const i of exprWarnings.slice(0, 50)) { - console.log(` • ${i.where}: ${i.message}`); - console.log(` source: \`${i.source}\``); - } - } - // 3b-ter. [#3366] Installable-provider preflight. Every capability the app + // 3c. [#3366] Installable-provider preflight. Every capability the app // DECLARES in `requires: [...]` must have a provider resolvable in the - // active edition. `os validate` only checks the token vocabulary and - // `os build` never resolved providers, so a `requires` entry whose - // provider has NO installable version in this edition (e.g. `ai` → - // @objectstack/service-ai, cloud-only since ADR-0025) slipped through - // to a generic `os start` crash. Fail the build with the edition-aware - // message instead; an absent-but-installable provider is a `pnpm add` - // hint (advisory), and a satisfied list passes silently. + // active edition. A `requires` entry whose provider has NO installable + // version in this edition (e.g. `ai` → @objectstack/service-ai, + // cloud-only since ADR-0025) otherwise slips through to a generic + // `os start` crash. Absent-but-installable is a `pnpm add` hint. + // + // Not a registry rule: it reads `node_modules`, not the stack. if (!flags.json) printStep('Checking capability providers (#3366)...'); const capPreflight = preflightRequiredCapabilities({ requires: Array.isArray((config as { requires?: unknown[] }).requires) @@ -243,25 +249,11 @@ export default class Compile extends Command { } } - // 3b-bis. ADR-0089 D3b — deprecated visibility aliases + mis-layered - // binding root. Checked on `normalized` (PRE-parse): the schema folds - // `visibleOn`/`visibility` into `visibleWhen` at parse, so `result.data` - // no longer carries the alias the author wrote. Advisory, never fatal. - const visibilityFindings = validateVisibilityPredicates(normalized as Record); - if (visibilityFindings.length > 0 && !flags.json) { - printWarning(`Visibility warnings (${visibilityFindings.length}) — ADR-0089`); - for (const f of visibilityFindings.slice(0, 50)) { - console.log(` • ${f.where}: ${f.message}`); - console.log(` ${f.hint}`); - console.log(` rule: ${f.rule} at ${f.path}`); - } - } - - // 3b-ter. [#3786] Keys `ObjectSchema` / `FieldSchema` do not declare, and - // so drop silently on the way to storage. PRE-parse for the same - // reason as the rule above. `defineStack` already warns for configs - // authored through it; this covers the ones that skip it (a plain - // object default-export, `strict: false`) and would otherwise emit an + // 3d. [#3786] Keys `ObjectSchema` / `FieldSchema` do not declare, and so + // drop silently on the way to storage. PRE-parse, since the parse is + // what strips them. `defineStack` already warns for configs authored + // through it; this covers the ones that skip it (a plain object + // default-export, `strict: false`) and would otherwise emit an // artifact with the key quietly gone. Advisory, never fatal. const unknownKeyFindings = [ ...lintUnknownStackKeys(normalized as Record, ObjectStackDefinitionSchema), @@ -274,356 +266,16 @@ export default class Compile extends Command { } } - // 3c. Widget-binding diagnostics (issues #1719/#1721) — semantic checks - // that need the widget's `dataset` reference resolved to its dataset - // and `dimensions`/`values` resolved to declared names. Errors are - // unresolvable bindings (dangling dataset/dimension/measure or a - // chartConfig field the query result won't contain) and fail the - // build; warnings are advisory and suppressible per widget via - // `suppressWarnings: ['']`. - if (!flags.json) printStep('Checking dashboard widget bindings (ADR-0021)...'); - const widgetFindings = validateWidgetBindings(result.data as Record); - const widgetErrors = widgetFindings.filter((f) => f.severity === 'error'); - const widgetWarnings = widgetFindings.filter((f) => f.severity === 'warning'); - if (widgetErrors.length > 0) { - if (flags.json) { - await emitJson({ success: false, error: 'widget binding validation failed', issues: widgetErrors }, 0, { compact: true }); - this.exit(1); - } - console.log(''); - printError(`Dashboard widget integrity failed (${widgetErrors.length} issue${widgetErrors.length > 1 ? 's' : ''})`); - for (const f of widgetErrors.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 (widgetWarnings.length > 0 && !flags.json) { - console.log(''); - for (const w of widgetWarnings) { - printWarning(`${w.where}: ${w.message}`); - console.log(chalk.dim(` ${w.hint}`)); - console.log(chalk.dim(` rule: ${w.rule} at ${w.path}`)); - } - } - - // 3c-bis. Dashboard action/route reference integrity (ADR-0049 for - // references, #3367). A header/widget action naming a `script`/`modal` - // target that resolves to no defined action, or a `url` target that - // matches no in-app route, ships a button that renders and silently - // does nothing on click. Dead script/modal targets fail the build - // (they fail open at runtime); unresolved url routes are advisory. - if (!flags.json) printStep('Checking dashboard action references (ADR-0049)...'); - const actionRefFindings = validateDashboardActionRefs(result.data as Record); - const actionRefErrors = actionRefFindings.filter((f) => f.severity === 'error'); - const actionRefWarnings = actionRefFindings.filter((f) => f.severity === 'warning'); - if (actionRefErrors.length > 0) { - if (flags.json) { - await emitJson({ success: false, error: 'dashboard action reference validation failed', issues: actionRefErrors }, 0, { compact: true }); - this.exit(1); - } - console.log(''); - printError(`Dashboard action reference check failed (${actionRefErrors.length} issue${actionRefErrors.length > 1 ? 's' : ''})`); - for (const f of actionRefErrors.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 (actionRefWarnings.length > 0 && !flags.json) { - console.log(''); - for (const w of actionRefWarnings) { - printWarning(`${w.where}: ${w.message}`); - console.log(chalk.dim(` ${w.hint}`)); - console.log(chalk.dim(` rule: ${w.rule} at ${w.path}`)); - } - } - - // 3a-ter. Filter placeholder resolvability (#3574). A filter value that - // resolves in neither vocabulary — `{current_user}` instead of - // `{current_user_id}` — reaches the data engine as a literal and - // matches nothing, so the surface renders empty with no error. That - // silent zero is indistinguishable from a genuine zero at review - // time, and an AI author reads it as a successful query. Fails the - // build because authoring time is the last point the author sees it. - if (!flags.json) printStep('Checking filter placeholders (#3574)...'); - const filterTokenFindings = validateFilterTokens(result.data as Record); - const filterTokenErrors = filterTokenFindings.filter((f) => f.severity === 'error'); - if (filterTokenErrors.length > 0) { - if (flags.json) { - await emitJson({ success: false, error: 'filter placeholder validation failed', issues: filterTokenErrors }, 0, { compact: true }); - this.exit(1); - } - console.log(''); - printError(`Filter placeholder check failed (${filterTokenErrors.length} issue${filterTokenErrors.length > 1 ? 's' : ''})`); - for (const f of filterTokenErrors.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); - } - - // 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). - // Plus page-component field bindings and the chart surfaces outside - // dashboards (report charts, list-view charts, dataset-bound page - // chart components) — same ADR-0021 semantic layer, where an axis - // naming a raw field instead of a measure renders an empty series. - // All plain strings in the schema, so a name resolving to nothing - // ships and fails silently. Errors fail the build; the - // platform-prefixed-but-unregistered case is advisory (a third-party - // package may still provide it). Translation bundles are checked in - // the reverse direction (keys naming metadata that does not exist, - // 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 = 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) { - if (flags.json) { - await emitJson({ success: false, error: 'reference integrity validation failed', issues: refErrors }, 0, { compact: true }); - 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 - // (ADR-0019). Errors fail the build; warnings are advisory. - if (!flags.json) printStep('Checking SDUI styling (ADR-0065)...'); - const styleFindings = validateResponsiveStyles(result.data as Record); - const styleErrors = styleFindings.filter((f) => f.severity === 'error'); - const styleWarnings = styleFindings.filter((f) => f.severity === 'warning'); - if (styleErrors.length > 0) { - if (flags.json) { - await emitJson({ success: false, error: 'SDUI styling validation failed', issues: styleErrors }, 0, { compact: true }); - this.exit(1); - } - console.log(''); - printError(`SDUI styling check failed (${styleErrors.length} issue${styleErrors.length > 1 ? 's' : ''})`); - for (const f of styleErrors.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 (styleWarnings.length > 0 && !flags.json) { - console.log(''); - for (const w of styleWarnings) { - printWarning(`${w.where}: ${w.message}`); - console.log(chalk.dim(` ${w.hint}`)); - console.log(chalk.dim(` rule: ${w.rule} at ${w.path}`)); - } - } - - // 3d. Flow authoring anti-pattern lint (#1874) — for valid-but-fragile flow - // metadata (e.g. a record-change trigger using a date-EQUALITY time - // condition that only fires on the exact day). Guides the author — very - // often an AI generating templates — toward the robust pattern. - // - // Findings are advisory by DEFAULT, but a finding marked - // `severity: 'error'` FAILS the build (#3760). Before that, this gate - // read as a gate and behaved as a comment: `flow-runas-unscoped` flags - // metadata the runtime now REFUSES to execute, and for the audience the - // rule exists to protect — very often an AI generating flows in bulk — - // an advisory line is close to no net at all. - const flowLint = lintFlowPatterns(result.data as Record); - const flowLintErrors = flowLint.filter((f) => f.severity === 'error'); - const flowLintWarnings = flowLint.filter((f) => f.severity !== 'error'); - if (flowLintWarnings.length > 0 && !flags.json) { - console.log(''); - for (const fnd of flowLintWarnings) { - printWarning(`${fnd.where}: ${fnd.message}`); - console.log(chalk.dim(` ${fnd.hint}`)); - console.log(chalk.dim(` rule: ${fnd.rule}`)); - } - } - if (flowLintErrors.length > 0) { - if (flags.json) { - this.log(JSON.stringify({ success: false, flowLintErrors }, null, 2)); - this.exit(1); - } - console.log(''); - printError(`Flow authoring check failed (${flowLintErrors.length} error${flowLintErrors.length > 1 ? 's' : ''})`); - for (const fnd of flowLintErrors) { - console.log(` • ${fnd.where}: ${fnd.message}`); - console.log(chalk.dim(` ${fnd.hint}`)); - console.log(chalk.dim(` rule: ${fnd.rule}`)); - } - this.exit(1); - } - - // 3d-bis. Liveness author-warning lint — close the spec-liveness loop on - // the author side: an authored property the ledger marks dead-and- - // misleading (e.g. `object.enable.files`, `field.columnName`) or - // experimental is set hopefully but does nothing / isn't enforced at - // runtime. Advisory only; ledger-driven (entries opt in via - // `authorWarn`), so it's high-signal and NEVER fails the build. - const livenessLint = lintLivenessProperties(result.data as Record); - if (livenessLint.length > 0 && !flags.json) { - console.log(''); - for (const fnd of livenessLint) { - printWarning(`${fnd.where}: ${fnd.message}`); - console.log(chalk.dim(` ${fnd.hint}`)); - console.log(chalk.dim(` rule: ${fnd.rule}`)); - } - } - - // 3d-ter. Autonumber `{field}` interpolation lint. A format like - // `{plan_no}{000}` makes the referenced field part of the counter - // scope, so it must exist and be set at create time — otherwise the - // runtime throws (or, unlinted, silently mis-numbers). An unknown - // field is broken → fails the build; an optional field is fragile → - // advisory warning. Mirrors the broken/fragile two-level guardrail. - const autonumberLint = lintAutonumberFormats(result.data as Record); - const autonumberErrors = autonumberLint.filter((f) => f.severity === 'error'); - const autonumberWarnings = autonumberLint.filter((f) => f.severity === 'warning'); - if (autonumberErrors.length > 0) { - if (flags.json) { - await emitJson({ success: false, error: 'autonumber format validation failed', issues: autonumberErrors }, 0, { compact: true }); - this.exit(1); - } - console.log(''); - printError(`Autonumber format validation failed (${autonumberErrors.length} issue${autonumberErrors.length > 1 ? 's' : ''})`); - for (const f of autonumberErrors) { - console.log(` • ${f.where}: ${f.message}`); - console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule}`)); - } - this.exit(1); - } - if (autonumberWarnings.length > 0 && !flags.json) { - console.log(''); - for (const f of autonumberWarnings) { - printWarning(`${f.where}: ${f.message}`); - console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule}`)); - } - } - - // 3d-quinquies. Contradictory uniqueness declarations (#3991). A column - // carrying BOTH a field-level `unique: true` and a single-column - // declared unique index has two intents, of which exactly one takes - // effect: since #3696 the field-level form is per-tenant while a - // declared index is platform-wide, so the global index wins and the - // tenant composite becomes unreachable. Advisory — the artifact is - // well-defined; the cost is a declaration that does nothing. Shares - // `lintUniqueDeclarations` with `os lint` so both agree. - const uniqueLint = lintUniqueDeclarations( - Array.isArray((result.data as Record).objects) - ? ((result.data as Record).objects as any[]) - : [], - ); - if (uniqueLint.length > 0 && !flags.json) { - console.log(''); - for (const f of uniqueLint) { - printWarning(`${f.path}: ${f.message}`); - if (f.fix) console.log(chalk.dim(` ${f.fix}`)); - console.log(chalk.dim(` rule: ${f.rule}`)); - } - } - - // 3d-quater. View-reference lint (#2554) — resolves form action targets - // and view-key collisions at build time. A `type:'form'` target that - // names a missing view or a LIST view opens a broken/blank form at - // runtime; a list/form key collision silently renames one view so - // references resolve to the OTHER. Both are broken → fail the build. - // This shifts objectui's runtime `viewKind` guard left to compile. - const viewRefLint = lintViewRefs(result.data as Record); - const viewRefErrors = viewRefLint.filter((f) => f.severity === 'error'); - const viewRefWarnings = viewRefLint.filter((f) => f.severity === 'warning'); - if (viewRefErrors.length > 0) { - if (flags.json) { - await emitJson({ success: false, error: 'view reference validation failed', issues: viewRefErrors }, 0, { compact: true }); - this.exit(1); - } - console.log(''); - printError(`View reference validation failed (${viewRefErrors.length} issue${viewRefErrors.length > 1 ? 's' : ''})`); - for (const f of viewRefErrors) { - console.log(` • ${f.where}: ${f.message}`); - console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule}`)); - } - this.exit(1); - } - if (viewRefWarnings.length > 0 && !flags.json) { - console.log(''); - for (const f of viewRefWarnings) { - printWarning(`${f.where}: ${f.message}`); - console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule}`)); - } - } - - // 3e. [ADR-0090 D7] Security-domain publish linter. Every error rule - // mirrors a runtime enforcement point (fail-closed OWD default, - // canonical enum, anchor binding gate, vocabulary freeze) — the lint - // moves the failure from a runtime deny to an author-time fix-it. - // Errors GATE the build (per ADR-0049 this is not advisory - // security); `info` findings are printed dimmed and never fatal. - if (!flags.json) printStep('Checking security posture (ADR-0090 D7)...'); - const securityFindings = [ - ...validateSecurityPosture(result.data as Record), - // [ADR-0105 D6] Organization-axis red lines: no permission inheritance - // along the org tree, and business-unit trees stay org-internal. Same - // finding shape, same gate — an `error` here blocks exactly as a - // security-posture error does. - ...validateOrgAxisRedLines(result.data as Record), - ]; - const securityErrors = securityFindings.filter((f) => f.severity === 'error'); - const securityAdvisories = securityFindings.filter((f) => f.severity !== 'error'); - if (securityErrors.length > 0) { - if (flags.json) { - await emitJson({ success: false, error: 'security posture validation failed', issues: securityErrors }, 0, { compact: true }); - this.exit(1); - } - console.log(''); - printError(`Security posture check failed (${securityErrors.length} issue${securityErrors.length > 1 ? 's' : ''})`); - for (const f of securityErrors.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 (securityAdvisories.length > 0 && !flags.json) { - console.log(''); - for (const f of securityAdvisories) { - printWarning(`${f.where}: ${f.message}`); - console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule}`)); - } - } - - // 3f. [ADR-0090 D6] Access-matrix snapshot gate. Opt-in per app: when + // 3e. [ADR-0090 D6] Access-matrix snapshot gate. Opt-in per app: when // `access-matrix.json` sits next to the config, the (permission set // × object) capability matrix derived from THIS build must match it // — a drift fails the build with a SEMANTIC diff ("'crm_admin' // gains delete on 'crm_lead'") until the snapshot is updated via // --update-access-matrix. An unchanged matrix auto-passes, so the // gate costs nothing until someone changes who-can-do-what. + // + // Not a registry rule: it reads (and with the flag, writes) a file + // next to the config rather than answering a question about the stack. { const matrixPath = path.join(path.dirname(absolutePath), 'access-matrix.json'); const currentMatrix = buildAccessMatrix(result.data as Record); @@ -657,11 +309,14 @@ export default class Compile extends Command { } } - // 3d. Package docs (ADR-0046): compile flat `src/docs/*.md` into + // 3f. Package docs (ADR-0046): compile flat `src/docs/*.md` into // `docs: DocSchema[]` and lint the combined set (flatness, // namespace-prefixed names, MDX/image ban, same-package link // resolution). Errors fail the build — the artifact is the // publish unit, so this IS the publish lint for docs. + // + // Not a registry rule: it reads `src/docs/` off disk, and the docs it + // collects there are an INPUT to the artifact, not just a check. if (!flags.json) printStep('Collecting package docs (ADR-0046)...'); const docsResult = collectAndLintDocs(absolutePath, result.data as Record); const docErrors = docsResult.issues.filter((i) => i.severity === 'error'); @@ -776,7 +431,10 @@ export default class Compile extends Command { handlersBundled: lowering.count, runtimeModule: runtimeBundle?.outputFileName ?? null, runtimeModuleSize: runtimeBundle?.size ?? 0, - warnings: widgetWarnings, + // The whole registry's advisory set, in the shape `os validate --json` + // reports. This key used to carry the widget rule's warnings alone — + // one gate out of the twenty-odd that raise them. + warnings: ruleAdvisories, // Same key `os validate --json` uses, so a CI consumer reads one shape // from either command rather than learning two. conversions: conversionNotices, @@ -790,8 +448,8 @@ export default class Compile extends Command { // 5. Summary console.log(''); printSuccess(`Build complete ${chalk.dim(`(${timer.display()})`)}`); - if (widgetWarnings.length > 0) { - printWarning(`${widgetWarnings.length} widget-binding warning(s) — see above`); + if (ruleAdvisories.length > 0) { + printWarning(`${ruleAdvisories.length} author-time warning(s) — see above`); } console.log(''); printMetadataStats(stats); diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index c0f8418a69..6d71845874 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -8,9 +8,8 @@ import { PROTOCOL_MAJOR } from '@objectstack/spec/kernel'; import { loadConfig, BUNDLE_REQUIRE_EXTERNALS } from '../utils/config.js'; import { computeI18nCoverage, type CoverageIssue } from '../utils/i18n-coverage.js'; 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 { validateReferenceIntegrity } from '@objectstack/lint'; +import { runAuthoringRules } from '../lint/authoring-rules.js'; +import { resolveSduiManifest } from '../utils/sdui-manifest.js'; import { collectAndLintDocs } from '../utils/collect-docs.js'; import { scoreMetadata } from '../lint/score.js'; import { runMetadataEval } from '../lint/metadata-eval.js'; @@ -135,7 +134,17 @@ function getViewLabel(view: any, viewPath: string): { label?: string; path: stri // ─── Lint Engine ──────────────────────────────────────────────────── -export function lintConfig(config: any): LintIssue[] { +export interface LintConfigOptions { + /** + * ADR-0080 SDUI component manifest, when the project ships one. Present, the + * JSX gate does full component/prop validation; absent, it stays parse-level. + * The `os lint` command resolves it; `scoreMetadata` deliberately does not — + * the scorer is a pure function of a stack and must not read the filesystem. + */ + sduiManifest?: unknown; +} + +export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue[] { const issues: LintIssue[] = []; const push = (issue: LintIssue | null) => { @@ -346,166 +355,34 @@ export function lintConfig(config: any): LintIssue[] { // objectstack-data/-ui skills. These double as the eval rubric (see score.ts). issues.push(...lintDataModel(objects)); - // ── Dashboard widget bindings (ADR-0021, issues #1719/#1721) ── - // Reference integrity (errors): widget `dataset`/`dimensions`/`values` and - // chartConfig axis/series fields must resolve against the declared - // datasets. Advisory shapes (warnings): e.g. a table/pivot widget whose - // binding resolves to count-only measures with no dimensions — almost - // always a record listing that belongs in an object-bound ListView - // (ADR-0017), not an analytics dataset. - for (const w of validateWidgetBindings(config)) { - issues.push({ - severity: w.severity, - rule: w.rule, - message: `${w.where}: ${w.message}`, - path: w.path, - fix: w.hint, - }); - } - - // ── Record-title contract (ADR-0079) ── - // titleFormat is retired (render-only template the server can't return or - // query) in favour of nameField; and an object with no resolvable title - // (no nameField/displayNameField and nothing derivable) ships records with - // no meaningful name. Both are advisory warnings — the auto-provision - // transform and the `Record #` floor keep a green build from ever - // shipping a fully title-less object (the ADR-0078 "not cloud-only" parity - // with cloud graph-lint). - for (const t of validateRecordTitle(config)) { - issues.push({ - severity: t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, - }); - } - - // ── Semantic-role pointers (ADR-0085) ── - // stageField / highlightFields / Field.group are pointers into the object's - // field map; a dangling pointer is Zod-valid but silently inert at render - // time (the ADR-0078 completeness gate). All advisory — every consumer - // degrades gracefully. - for (const t of validateSemanticRoles(config)) { - issues.push({ - severity: t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, - }); - } - - // ── Capability references (ADR-0066 ⑨) ── - // requiredPermissions naming a capability that is registered nowhere - // (no built-in, no permission set grants it, no sys_capability seed) is - // almost certainly a typo. Advisory — the reference fails closed at runtime, - // and the capability may legitimately be provided by another installed package. - for (const t of validateCapabilityReferences(config)) { - issues.push({ - severity: t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, - }); - } - - // ── Security posture (ADR-0090 D7) ── - // The security-domain publish linter: unset/alias OWD, external dial wider - // than internal, wildcard VAMA, high-privilege everyone-suggested sets, the - // reserved word "role", and private-object read grants with no depth. Runs - // on the NORMALIZED (pre-zod) input here, so alias values that the schema - // gate would reject in `os compile` get a located fix-it instead of a Zod - // enum error. `error` findings gate `os compile`; `info` maps to suggestion. - for (const t of validateSecurityPosture(config)) { - issues.push({ - severity: t.severity === 'info' ? 'suggestion' : t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, - }); - } - - // ── Organization-axis red lines (ADR-0105 D6) ── - // The org tree (`parent_organization_id`) is a REPORTING dimension. An RLS - // policy or sharing rule that walks it builds a second permission hierarchy — - // the dual-hierarchy mistake ADR-0057 D5 retired — and cannot widen Layer 0 - // anyway, so it grants nothing it appears to. Business-unit grants on - // platform-global objects are the other half: no org column to scope against - // means the grant spans every organization. - for (const t of validateOrgAxisRedLines(config)) { - issues.push({ - severity: t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, - }); - } - - // ── Approval-node approvers (ADR-0090 D3 fallout) ── - // `{ type: 'role' }` resolves against the better-auth org-membership tier - // (owner/admin/member), NOT positions — a position name authored there - // silently routes the approval to nobody. Advisory: the fix-it points at - // `{ type: 'position' }` (sys_user_position). - for (const t of validateApprovalApprovers(config)) { - issues.push({ - severity: t.severity === 'info' ? 'suggestion' : t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, - }); - } - - // ── Seed replay safety (framework#3434) ── - // Seeds are replayed on every boot / re-publish, so a `mode: 'insert'` dataset - // duplicates its table on every restart (the loader's insert path has no - // existing-row check). Advisory: the fix-it points at `ignore`/`upsert` + an - // `externalId` (single field, or a composite list for a join table). - for (const t of validateSeedReplaySafety(config)) { - issues.push({ - severity: t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, - }); - } - - // ── Seed value vs state machine (framework#3433 follow-up) ── - // #3433 exempts seed writes from the `state_machine` rule, so a seeded status - // the FSM does not declare is no longer rejected at write time. Re-add that - // safety net at author time: a value outside the machine's declared states is - // almost certainly a typo. Advisory — the exemption itself is legitimate. - for (const t of validateSeedStateMachine(config)) { - issues.push({ - severity: t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, - }); - } - - // ── 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)) { + // ── The author-time rule registry (#4409) ── + // Everything above this line is `os lint`'s OWN rubric: naming, labels, + // structure, data-model conventions. Its `error` severity is a lint verdict, + // not a publish gate — `os build` has never rejected a camelCase object name. + // + // Everything below comes from the table the three authoring commands share. + // `os lint` used to hand-wire its own subset of it, and the subsets disagreed: + // it ran `validateApprovalApprovers` (which gates) that neither other command + // ran, and missed six gating rules that both of them ran — so it returned + // clean for stacks `os build` rejects AND rejected stacks `os build` ships. + // A pre-flight that disagrees with the gate in both directions is worse than + // no pre-flight: the only rational responses are to re-verify everything or + // to stop trusting it. + // + // The registry is `os lint`'s single call site into that set. Adding a rule + // there reaches this command with no edit here. Do NOT import a rule directly. + // + // `os lint` does not Zod-parse (a schema error is `os validate`'s verdict to + // give), so the registry runs both stack tiers against the normalized input — + // which is what this command already did for the reference-integrity suite + // and the security linter. + for (const f of runAuthoringRules('lint', { normalized: config, sduiManifest: opts.sduiManifest })) { issues.push({ - severity: t.severity, - rule: t.rule, - message: `${t.where}: ${t.message}`, - path: t.path, - fix: t.hint, + severity: f.severity === 'info' ? 'suggestion' : f.severity, + rule: f.rule, + message: `${f.where}: ${f.message}`, + path: f.path, + fix: f.hint, }); } @@ -577,7 +454,7 @@ export default class Lint extends Command { } const normalized = normalizeStackInput(config as Record); - const issues = lintConfig(normalized); + const issues = lintConfig(normalized, { sduiManifest: resolveSduiManifest() }); // ── Package docs (ADR-0046) ── collected src/docs/*.md + inline docs: // flatness, namespace-prefixed names, MDX/image ban, link resolution. diff --git a/packages/cli/src/commands/reference-integrity-wiring.test.ts b/packages/cli/src/commands/reference-integrity-wiring.test.ts deleted file mode 100644 index 0037dc0898..0000000000 --- a/packages/cli/src/commands/reference-integrity-wiring.test.ts +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. -// -// Structural wiring guard for the suite's whole reason to exist (#3583 §5 D5). -// -// ## Why a test and not a comment -// -// The defect this catches is not a wrong result — it is a MISSING CALL, and a -// missing call produces no failing assertion anywhere. Every command's own -// tests pass, the rule's unit tests pass, and the only symptom is that -// `os validate`, `os lint` and `os compile` disagree about the same stack. -// -// That is not hypothetical. `validateReadonlyFlowWrites` was hand-wired into -// `validate` and `compile` and never into `lint` from #3425 until #4394 — and -// because it GATES (`flow-update-readonly-field` is an `error`), `os lint` -// spent that whole time returning clean for stacks `os compile` refuses. The -// suite's own header had been naming it as the standing proof of the drift the -// suite exists to end, which is a comment doing a test's job. #4394 removed -// that instance; this file removes the failure MODE, so the next rule cannot -// repeat it silently (#4384). -// -// ## The invariant -// -// `os lint` ⊇ `os compile`'s gate set. `lint` is the cheap pre-flight and -// `compile` is the gate; a green `lint` followed by a red `compile` makes the -// pre-flight worthless — and for an agent it is worse than worthless, because -// the remaining options are re-verifying everything (slow) or learning to -// distrust the signal (dangerous). -// -// ## Why it scans source -// -// vitest inlines imports through its transform, so a spy on `@objectstack/lint` -// cannot prove which symbols a command file actually reaches for — the same -// reason `lazy-deps.test.ts` scans `src/` rather than probing a module cache. -// Behavioural coverage of what the suite CONTAINS lives in -// `@objectstack/lint`'s `reference-integrity-suite.test.ts`; this file guards -// only the seam between that suite and the three call sites. - -import { readFileSync } from 'node:fs'; -import { dirname, join } from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { describe, it, expect } from 'vitest'; -import { REFERENCE_INTEGRITY_RULES } from '@objectstack/lint'; - -const commandsDir = dirname(fileURLToPath(import.meta.url)); - -/** The three commands that must hold a stack to the same author-time bar. */ -const AUTHORING_COMMANDS = ['validate.ts', 'lint.ts', 'compile.ts'] as const; - -const sourceOf = (file: string) => readFileSync(join(commandsDir, file), 'utf8'); - -describe('reference-integrity wiring (#3583 §5 D5, #4384)', () => { - it.each(AUTHORING_COMMANDS)('%s runs the suite', (file) => { - expect(sourceOf(file)).toMatch(/\bvalidateReferenceIntegrity\s*\(/); - }); - - // The regression itself. A second, per-rule call site is how a rule ends up - // on two commands out of three, and it always starts with importing that rule - // by name — so the import is what this asserts on. Catching the call instead - // would miss the window between adding the import and adding the second call. - it.each(AUTHORING_COMMANDS)('%s does not import a suite member directly', (file) => { - const source = sourceOf(file); - const reached = REFERENCE_INTEGRITY_RULES.map((r) => r.name).filter((name) => - new RegExp( - String.raw`^\s*import\s[^;]*\b${name}\b[^;]*from\s*['"]@objectstack/lint['"]`, - 'm', - ).test(source), - ); - expect( - reached, - `${file} imports suite member(s) directly — run them via validateReferenceIntegrity instead, ` + - `or all three commands will drift the way validateReadonlyFlowWrites did (#4394)`, - ).toEqual([]); - }); - - // Guards the guard: were the suite ever emptied or the export renamed, the - // assertions above would pass vacuously while checking nothing. - it('the suite is non-empty, so the assertions above are not vacuous', () => { - expect(REFERENCE_INTEGRITY_RULES.length).toBeGreaterThan(0); - // The rule whose absence from `os lint` is the reason this file exists. - expect(REFERENCE_INTEGRITY_RULES.map((r) => r.name)).toContain('validateReadonlyFlowWrites'); - }); -}); diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 1c0d0007cf..60091e9a4d 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -1,9 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Args, Command, Flags } from '@oclif/core'; -import { existsSync, readFileSync } from 'node:fs'; -import { createRequire } from 'node:module'; -import { join, dirname } from 'node:path'; +import { dirname } from 'node:path'; import chalk from 'chalk'; import { ZodError } from 'zod'; import { @@ -15,25 +13,10 @@ import { type ConversionNotice, } from '@objectstack/spec'; import { loadConfig } from '../utils/config.js'; -import { validateStackExpressions } from '@objectstack/lint'; -import { validateListViewMode } from '@objectstack/lint'; -import { validateViewContainers } from '@objectstack/lint'; -import { validateWidgetBindings } from '@objectstack/lint'; -import { validateDashboardActionRefs } from '@objectstack/lint'; -import { validateFilterTokens } from '@objectstack/lint'; -import { validateReferenceIntegrity } from '@objectstack/lint'; -import { validateResponsiveStyles } from '@objectstack/lint'; -import { validateJsxPages, validateReactPages, validatePageSourceStyling } from '@objectstack/lint'; -import { validateCapabilityReferences } from '@objectstack/lint'; -import { validateVisibilityPredicates } from '@objectstack/lint'; -import { validateSecurityPosture, validateOrgAxisRedLines } from '@objectstack/lint'; -import { validateFlowTriggerReadiness } from '@objectstack/lint'; -import { lintFlowPatterns } from '../utils/lint-flow-patterns.js'; -import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js'; -import { lintUniqueDeclarations } from '../lint/data-model-rules.js'; -import { lintLivenessProperties } from '../utils/lint-liveness-properties.js'; -import { lintViewRefs } from '../utils/lint-view-refs.js'; +import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '../lint/authoring-rules.js'; +import { resolveSduiManifest } from '../utils/sdui-manifest.js'; import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js'; +import { collectAndLintDocs } from '../utils/collect-docs.js'; import { printHeader, printKV, @@ -66,7 +49,7 @@ export default class Validate extends Command { const { args, flags } = await this.parse(Validate); const timer = createTimer(); - + if (!flags.json) { printHeader('Validate'); } @@ -75,7 +58,7 @@ export default class Validate extends Command { // 1. Load configuration if (!flags.json) printStep('Loading configuration...'); const { config, absolutePath, duration } = await loadConfig(args.config); - + if (!flags.json) { printKV('Config', absolutePath); printKV('Load time', `${duration}ms`); @@ -92,10 +75,10 @@ export default class Validate extends Command { onConversionNotice: (n) => conversionNotices.push(n), }); // [#3786] Keys `ObjectSchema` / `FieldSchema` do not declare, and so drop - // silently. PRE-parse for the same reason the visibility rule below is: - // the parse is what strips them, so `result.data` no longer carries the - // key the author actually wrote. Computed here rather than down in the - // warnings section so the `--json` path reports it too — the + // silently. PRE-parse for the same reason the registry's `normalized`-tier + // rules are: the parse is what strips them, so `result.data` no longer + // carries the key the author actually wrote. Computed here rather than + // down in the warnings section so the `--json` path reports it too — the // "computed, then discarded" shape this file already had to fix once. const unknownKeyWarnings = [ ...lintUnknownStackKeys(normalized as Record, ObjectStackDefinitionSchema), @@ -119,528 +102,58 @@ export default class Validate extends Command { this.exit(1); } - // 2b. Expression validation (ADR-0032 §1a/1b) — the same gate `os build` - // runs, brought to the read-only check so authors catch it without - // emitting an artifact. CEL predicates in actions/validations/flows/ - // sharing/hooks are checked for syntax AND that `record.` - // references resolve on the target object. This is what catches a - // BARE field ref (`done` instead of `record.done`) that would - // otherwise silently hide an action on every record (#2183/#2185). - if (!flags.json) printStep('Validating expressions (ADR-0032)...'); - const exprIssues = validateStackExpressions(result.data as Record); - const exprErrors = exprIssues.filter((i) => i.severity !== 'warning'); - const exprWarnings = exprIssues.filter((i) => i.severity === 'warning'); - - if (exprErrors.length > 0) { - if (flags.json) { - await emitJson({ - valid: false, - errors: exprErrors, - warnings: exprWarnings, - duration: timer.elapsed(), - }); - this.exit(1); - } - console.log(''); - printError(`Expression validation failed (${exprErrors.length} issue${exprErrors.length > 1 ? 's' : ''})`); - for (const i of exprErrors.slice(0, 50)) { - console.log(` • ${i.where}: ${i.message}`); - console.log(chalk.dim(` source: \`${i.source}\``)); - } - this.exit(1); - } - - // 2c. ADR-0053 list-view navigation modes — `userFilters`/`quickFilters` - // on an object list view ("views" mode) are silently dropped: the - // object-list schema (ObjectListViewSchema) OMITS them, so this is - // checked on `normalized` (PRE-parse) — `result.data` has already had - // the field stripped. They belong to a page list ("filters" mode). - // See objectui #2219 and ADR-0053 phase 4. - if (!flags.json) printStep('Checking list-view navigation modes (ADR-0053)...'); - const listViewFindings = validateListViewMode(normalized as Record); - const listViewErrors = listViewFindings.filter((f) => f.severity === 'error'); - - if (listViewErrors.length > 0) { - if (flags.json) { - await emitJson({ - valid: false, - errors: listViewErrors, - duration: timer.elapsed(), - }); - this.exit(1); - } - console.log(''); - printError(`List-view mode check failed (${listViewErrors.length} issue${listViewErrors.length > 1 ? 's' : ''})`); - for (const f of listViewErrors.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); - } - - // 2d. View container shape — a flat list-view object in `views: []` - // parses to an EMPTY container (ViewSchema strips unknown keys), so - // the schema step passes while zero views register and the Console - // silently renders nothing. Checked on `normalized` (PRE-parse) — - // `result.data` has already had the flat keys stripped. - if (!flags.json) printStep('Checking view container shape...'); - const viewContainerFindings = validateViewContainers(normalized as Record); - const viewContainerErrors = viewContainerFindings.filter((f) => f.severity === 'error'); - - if (viewContainerErrors.length > 0) { - if (flags.json) { - await emitJson({ - valid: false, - errors: viewContainerErrors, - duration: timer.elapsed(), - }); - this.exit(1); - } - console.log(''); - printError(`View container check failed (${viewContainerErrors.length} issue${viewContainerErrors.length > 1 ? 's' : ''})`); - for (const f of viewContainerErrors.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); - } - - // 3. Dashboard widget reference integrity (issue #1721) — a semantic - // cross-reference pass the protocol schema cannot express: every - // widget's `dataset`/`dimensions`/`values` and chartConfig - // axis/series fields must resolve against the declared datasets - // (ADR-0021). Errors fail validation; warnings are advisory. - if (!flags.json) printStep('Checking dashboard widget bindings (ADR-0021)...'); - const widgetFindings = validateWidgetBindings(result.data as Record); - const widgetErrors = widgetFindings.filter((f) => f.severity === 'error'); - const widgetWarnings = widgetFindings.filter((f) => f.severity === 'warning'); - - if (widgetErrors.length > 0) { - if (flags.json) { - await emitJson({ - valid: false, - errors: widgetErrors, - warnings: widgetWarnings, - duration: timer.elapsed(), - }); - this.exit(1); - } - console.log(''); - printError(`Dashboard widget integrity failed (${widgetErrors.length} issue${widgetErrors.length > 1 ? 's' : ''})`); - for (const f of widgetErrors.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); - } - - // 3a-bis. Dashboard action/route reference integrity (ADR-0049 for - // references, #3367) — a header/widget action names a `script`/`modal` - // target that resolves to no defined action, or a `url` target that - // matches no in-app route. Nothing else flags it, so it ships as a - // button that renders and silently does nothing on click (a false - // affordance). Dead script/modal targets are errors (fail open at - // runtime); unresolved url routes are advisory warnings. - if (!flags.json) printStep('Checking dashboard action references (ADR-0049)...'); - const actionRefFindings = validateDashboardActionRefs(result.data as Record); - const actionRefErrors = actionRefFindings.filter((f) => f.severity === 'error'); - const actionRefWarnings = actionRefFindings.filter((f) => f.severity === 'warning'); - if (actionRefErrors.length > 0) { - if (flags.json) { - await emitJson({ - valid: false, - errors: actionRefErrors, - warnings: [...widgetWarnings, ...actionRefWarnings], - duration: timer.elapsed(), - }); - this.exit(1); - } - console.log(''); - printError(`Dashboard action reference check failed (${actionRefErrors.length} issue${actionRefErrors.length > 1 ? 's' : ''})`); - for (const f of actionRefErrors.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 actionRefWarnings.slice(0, 50)) { - console.log(chalk.yellow(` ⚠ ${w.where}: ${w.message}`)); - console.log(chalk.dim(` ${w.hint}`)); - } - } - - // 3a-ter. Filter placeholder resolvability (#3574) — a filter value like - // `{current_user}` resolves in no vocabulary, reaches the data engine - // as a literal, matches nothing, and the surface renders empty with - // no error anywhere. Silent-zero is indistinguishable from a genuine - // zero, so it survives human review; and an AI author reads the 0 as - // a successful query. Caught here because authoring time is the only - // place the diagnostic can still reach the author. - if (!flags.json) printStep('Checking filter placeholders (#3574)...'); - const filterTokenFindings = validateFilterTokens(result.data as Record); - const filterTokenErrors = filterTokenFindings.filter((f) => f.severity === 'error'); - if (filterTokenErrors.length > 0) { - if (flags.json) { - await emitJson({ - valid: false, - errors: filterTokenErrors, - warnings: [...widgetWarnings, ...actionRefWarnings], - duration: timer.elapsed(), - }); - this.exit(1); - } - console.log(''); - printError(`Filter placeholder check failed (${filterTokenErrors.length} issue${filterTokenErrors.length > 1 ? 's' : ''})`); - for (const f of filterTokenErrors.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); - } - - // 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). - // Plus page-component field bindings and the chart surfaces outside - // dashboards (report charts, list-view charts, dataset-bound page - // chart components) — same ADR-0021 semantic layer, where an axis - // naming a raw field instead of a measure renders an empty series. - // All are plain strings in the schema, so a name resolving to nothing - // parses, ships, and fails silently at runtime. An unprefixed miss is - // a typo (error); a platform-prefixed name no known package registers - // is advisory (a third-party package may still provide it). - // Translation bundles get the same treatment in reverse: a key naming - // a field/view/action/section that no longer exists — or an option - // keyed by its display label instead of its stored value — resolves - // to nothing and renders the source string (advisory: inert, not - // broken). - if (!flags.json) printStep('Checking object & action references (#3583)...'); - 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) { - if (flags.json) { - await emitJson({ - valid: false, - errors: refErrors, - warnings: [...widgetWarnings, ...actionRefWarnings, ...refWarnings], - duration: timer.elapsed(), - }); - 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; - // Tailwind-in-className silently does nothing. Same bar for - // hand-authored and AI-generated pages (ADR-0019). - if (!flags.json) printStep('Checking SDUI styling (ADR-0065)...'); - const styleFindings = validateResponsiveStyles(result.data as Record); - const styleErrors = styleFindings.filter((f) => f.severity === 'error'); - const styleWarnings = styleFindings.filter((f) => f.severity === 'warning'); - - if (styleErrors.length > 0) { - if (flags.json) { - await emitJson({ - valid: false, - errors: styleErrors, - warnings: [...widgetWarnings, ...styleWarnings], - duration: timer.elapsed(), - }); - this.exit(1); - } - console.log(''); - printError(`SDUI styling check failed (${styleErrors.length} issue${styleErrors.length > 1 ? 's' : ''})`); - for (const f of styleErrors.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); - } - - // 3b. JSX-source pages (ADR-0080) — a kind:'jsx' page's `source` is - // parsed (never executed) and compiled to the SDUI tree at save - // time. Parse it now so malformed source fails loudly (ADR-0078) - // instead of being stored and breaking only at render. - if (!flags.json) printStep('Checking JSX-source pages (ADR-0080)...'); - // Optional component manifest (ADR-0080): if the project ships a - // `sdui.manifest.json` (generated from the registry's public tier), the - // gate does full component/prop validation; otherwise parse-level. - let sduiManifest: unknown; - try { - const mp = join(process.cwd(), 'sdui.manifest.json'); - if (existsSync(mp)) sduiManifest = JSON.parse(readFileSync(mp, 'utf8')); - if (!sduiManifest) { - // Fall back to the manifest shipped inside @objectstack/console - // (built from objectui's public-tier registry; cli already deps it). - const cp = createRequire(import.meta.url).resolve('@objectstack/console/dist/sdui.manifest.json'); - if (existsSync(cp)) sduiManifest = JSON.parse(readFileSync(cp, 'utf8')); - } - } catch { /* fall back to parse-level */ } - const jsxFindings = validateJsxPages( - result.data as Record, - sduiManifest ? { manifest: sduiManifest as never } : {}, - ); - const jsxErrors = jsxFindings.filter((f) => f.severity === 'error'); - const jsxWarnings = jsxFindings.filter((f) => f.severity === 'warning'); - - if (jsxErrors.length > 0) { - if (flags.json) { - await emitJson({ - valid: false, - errors: jsxErrors, - warnings: [...widgetWarnings, ...styleWarnings, ...jsxWarnings], - duration: timer.elapsed(), - }); - this.exit(1); - } - console.log(''); - printError(`JSX-source page check failed (${jsxErrors.length} issue${jsxErrors.length > 1 ? 's' : ''})`); - for (const f of jsxErrors.slice(0, 50)) { - console.log(` \u2022 ${f.where}: ${f.message}`); - console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`)); - } - this.exit(1); - } - - // 3c. React-source pages (ADR-0081) — a kind:'react' page's `source` is - // real React executed at render. Transpile it now (Sucrase, never - // executed) so syntax errors fail loudly at build, not at render. - if (!flags.json) printStep('Checking React-source pages (ADR-0081)...'); - const reactFindings = validateReactPages(result.data as Record); - const reactErrors = reactFindings.filter((f) => f.severity === 'error'); - if (reactErrors.length > 0) { - if (flags.json) { - await emitJson({ - valid: false, - errors: reactErrors, - warnings: [...widgetWarnings, ...styleWarnings, ...jsxWarnings], - duration: timer.elapsed(), - }); - this.exit(1); - } - console.log(''); - printError(`React-source page check failed (${reactErrors.length} issue${reactErrors.length > 1 ? 's' : ''})`); - for (const f of reactErrors.slice(0, 50)) { - console.log(` \u2022 ${f.where}: ${f.message}`); - console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`)); - } - this.exit(1); - } - - // 3d. React-source page PROPS are checked by `REFERENCE_INTEGRITY_RULES` - // (step 3a above), not from here (#4340 follow-up). They ran from - // this call site ALONE, so `os lint` and `os compile` accepted a - // react page whose every field binding was stale — including the - // gating ones. That is `validateReadonlyFlowWrites`' divergence - // (#4394) one surface over. The input is unchanged: the suite is - // handed the same `result.data` this block passed. - - // 3e. Source-tier page styling (ADR-0065): Tailwind className in a - // kind:'html'/'react' page source silently no-ops (the build never - // scans authored metadata) — warn with the inline-style fix. - if (!flags.json) printStep('Checking source-page styling (ADR-0065)...'); - const sourceStyleFindings = validatePageSourceStyling(result.data as Record); - const sourceStyleWarnings = sourceStyleFindings.filter((f) => f.severity === 'warning'); - if (!flags.json) { - for (const w of sourceStyleWarnings.slice(0, 50)) { - console.log(chalk.yellow(` \u26a0 ${w.where}: ${w.message}`)); - console.log(chalk.dim(` ${w.hint}`)); - } - } - - // 3f. Capability references (ADR-0066 ⑨): a requiredPermissions entry - // naming a capability registered nowhere (no built-in, no permission - // set grants it, no sys_capability seed) is almost certainly a typo — - // it fails closed at runtime. Advisory: the capability may legitimately - // be provided by another installed package. - if (!flags.json) printStep('Checking capability references (ADR-0066)...'); - const capFindings = validateCapabilityReferences(result.data as Record); - const capWarnings = capFindings.filter((f) => f.severity === 'warning'); - if (!flags.json) { - for (const w of capWarnings.slice(0, 50)) { - console.log(chalk.yellow(` ⚠ ${w.where}: ${w.message}`)); - console.log(chalk.dim(` ${w.hint}`)); - } - } - - // 3g. Auto-launched flow trigger wiring (2026-07-17 third-party eval): - // a record-change flow whose start-node objectName matches nothing - // never fires — silently. Also nudges auto-triggered flows to declare - // an explicit deployment status (the schema default is 'draft', and - // draft flows DO still fire — ambiguous intent). Advisory: objects - // may come from other installed packages. - if (!flags.json) printStep('Checking flow trigger wiring...'); - const flowReadinessFindings = validateFlowTriggerReadiness(normalized as Record); - const flowReadinessWarnings = flowReadinessFindings.filter((f) => f.severity === 'warning'); - if (!flags.json) { - for (const w of flowReadinessWarnings.slice(0, 50)) { - console.log(chalk.yellow(` ⚠ ${w.where}: ${w.message}`)); - console.log(chalk.dim(` ${w.hint}`)); - } - } - - // 3g-bis. Flow template path references (#3426) used to be checked here by - // hand. It is a reference rule — a `{record.}` token resolved - // against the bound object's declared fields — so it now runs as a - // member of REFERENCE_INTEGRITY_RULES in step 3 above, which reaches - // `os lint` and `os compile` at the same time. Those two accepted a - // flow whose filter token the runtime refuses (#3810) for as long as - // this call site was the only one. - - // 3e3. [#3782] The four authoring lints that live in the CLI itself rather - // than in `@objectstack/lint`. Every other gate on this command is a - // `@objectstack/lint` import, so these four were only ever reachable - // from `compile.ts` — `os build` ran them, `os validate` did not, and - // the drift went unnoticed while all of their findings were advisory. - // Two of them already GATE the build (`lintAutonumberFormats`, - // `lintViewRefs` emit `severity: 'error'`), which made this command - // report a clean stack that `os build` then rejected — exactly the - // contract this command exists to uphold. Severity handling mirrors - // `compile.ts` per lint, so the two surfaces agree by construction. - if (!flags.json) printStep('Running authoring lints (#3782)...'); - - // Flow authoring anti-patterns (#1874). Advisory today; `severity: 'error'` - // is honoured so a blocking rule (#3760's `flow-runas-unscoped`) gates here - // the moment it gates the build, with no further wiring. - const flowLint = lintFlowPatterns(result.data as Record); - const flowLintErrors = flowLint.filter((f) => f.severity === 'error'); - const flowLintWarnings = flowLint.filter((f) => f.severity !== 'error'); - - // Liveness author-warnings — an authored property the ledger marks - // dead-and-misleading or experimental. Advisory only, never fatal. - const livenessLint = lintLivenessProperties(result.data as Record); - - // Autonumber `{field}` interpolation — an unknown field is broken (error); - // an optional one is fragile (warning). - const autonumberLint = lintAutonumberFormats(result.data as Record); - const autonumberErrors = autonumberLint.filter((f) => f.severity === 'error'); - const autonumberWarnings = autonumberLint.filter((f) => f.severity !== 'error'); - - // View references (#2554) — a form action target naming a missing or LIST - // view, and list/form view-key collisions. Both are broken → error. - const viewRefLint = lintViewRefs(result.data as Record); - const viewRefErrors = viewRefLint.filter((f) => f.severity === 'error'); - const viewRefWarnings = viewRefLint.filter((f) => f.severity !== 'error'); - - // Contradictory uniqueness declarations (#3991) — a column carrying both a - // field-level `unique: true` and a single-column declared unique index has - // two intents, of which exactly one takes effect. Advisory. Mapped into the - // `{ where, hint }` shape the shared renderer below expects; the rule lives - // in `lint/data-model-rules.ts` so `os lint` reports the same finding. - const uniqueLintWarnings = lintUniqueDeclarations( - Array.isArray((result.data as Record).objects) - ? ((result.data as Record).objects as any[]) - : [], - ).map((f) => ({ where: f.path, message: f.message, hint: f.fix ?? '', rule: f.rule, severity: 'warning' as const })); - - const authoringLintErrors = [...flowLintErrors, ...autonumberErrors, ...viewRefErrors]; - const authoringLintWarnings = [ - ...flowLintWarnings, - ...livenessLint, - ...autonumberWarnings, - ...viewRefWarnings, - ...uniqueLintWarnings, - ]; - if (authoringLintErrors.length > 0) { - if (flags.json) { - await emitJson({ - valid: false, - errors: authoringLintErrors, - duration: timer.elapsed(), - }); - this.exit(1); - } - console.log(''); - printError(`Authoring lint failed (${authoringLintErrors.length} issue${authoringLintErrors.length > 1 ? 's' : ''})`); - for (const f of authoringLintErrors.slice(0, 50)) { - console.log(` • ${f.where}: ${f.message}`); - console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule}`)); - } - this.exit(1); - } - if (!flags.json) { - for (const f of authoringLintWarnings.slice(0, 50)) { - console.log(chalk.yellow(` ⚠ ${f.where}: ${f.message}`)); - console.log(chalk.dim(` ${f.hint}`)); - } - } + // 3. The author-time rule registry (#4409). Every rule the three authoring + // commands share — expressions, view shape, widget/action/filter/name + // references, SDUI styling, page sources, security posture, the CLI's + // own authoring lints — runs from ONE table, so `os validate`, + // `os build` and `os lint` hold a stack to the same bar by construction. + // Before it, each command hand-wired its own subset: 23 of 26 rules ran + // on some strict subset of the three, and `os build` — the command that + // PUBLISHES — was the weakest gate of the three. + // + // Which rules run, on which stack tier, and why any of them is scoped + // is declared in `lint/authoring-rules.ts`. Do not add a call site here. + const registered = authoringRulesFor('validate'); + if (!flags.json) printStep(`Running author-time rules (${registered.length})...`); + const findings = runAuthoringRules('validate', { + normalized: normalized as Record, + parsed: result.data as Record, + sduiManifest: resolveSduiManifest(), + }); + const { errors: ruleErrors, advisories: ruleAdvisories } = splitBySeverity(findings); - // 3f. [ADR-0090 D7] Security posture — the same gate `os compile`/`os build` - // run. Without it here, `os validate` passed a stack (e.g. a custom - // object with no explicit sharingModel) that the build then rejected, - // breaking this command's contract of being the artifact-free run of - // the same gates. Errors gate; advisories print dimmed. - if (!flags.json) printStep('Checking security posture (ADR-0090 D7)...'); - const securityFindings = [ - ...validateSecurityPosture(result.data as Record), - // [ADR-0105 D6] Organization-axis red lines: no permission inheritance - // along the org tree, and business-unit trees stay org-internal. Same - // finding shape, same gate — an `error` here blocks exactly as a - // security-posture error does. - ...validateOrgAxisRedLines(result.data as Record), - ]; - const securityErrors = securityFindings.filter((f) => f.severity === 'error'); - const securityAdvisories = securityFindings.filter((f) => f.severity !== 'error'); - if (securityErrors.length > 0) { + if (ruleErrors.length > 0) { + // Every failing rule reports at once. The command used to exit at the + // first failing gate, so an author with three unrelated problems fixed + // them in three round trips and could not see how deep the hole went. if (flags.json) { await emitJson({ valid: false, - errors: securityErrors, + errors: ruleErrors, + warnings: ruleAdvisories, duration: timer.elapsed(), }); this.exit(1); } console.log(''); - printError(`Security posture check failed (${securityErrors.length} issue${securityErrors.length > 1 ? 's' : ''})`); - for (const f of securityErrors.slice(0, 50)) { + printError(`Author-time rules failed (${ruleErrors.length} issue${ruleErrors.length > 1 ? 's' : ''})`); + for (const f of ruleErrors.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 f of securityAdvisories.slice(0, 50)) { - console.log(chalk.yellow(` ⚠ ${f.where}: ${f.message}`)); - console.log(chalk.dim(` ${f.hint}`)); - } - } - // 3h. [#3366] Installable-provider preflight — the shift-left of the + // 3b. [#3366] Installable-provider preflight — the shift-left of the // `serve`-time capability check. `os validate` previously only checked // the `requires` tokens against the vocabulary (ADR-0066), never // whether each token's provider is resolvable in the active edition. A // token whose provider has NO installable version here (e.g. `ai` → // @objectstack/service-ai, cloud-only) fails; absent-but-installable is // an advisory `pnpm add` hint. Mirrors the `os build` gate exactly. + // + // Not a registry rule: it reads `node_modules`, not the stack. if (!flags.json) printStep('Checking capability providers (#3366)...'); const capProviderPreflight = preflightRequiredCapabilities({ requires: Array.isArray((config as { requires?: unknown[] }).requires) @@ -670,6 +183,39 @@ export default class Validate extends Command { this.exit(1); } + // 3c. Package docs (ADR-0046) — flatness, namespace-prefixed names, the + // MDX/image ban, same-package link resolution. `os build` has always + // FAILED on a doc error (the artifact is the publish unit, so that is + // the publish lint for docs) while this command never ran it: the same + // "build rejects what validate accepts" hole #4409 found among the + // metadata rules, one gate over. It went unnoticed because the parity + // guard keyed on the `lint*`/`validate*` naming convention and this + // one is called `collectAndLintDocs`. + // + // Not a registry rule: it reads `src/docs/*.md` off disk. + if (!flags.json) printStep('Checking package docs (ADR-0046)...'); + const docsResult = collectAndLintDocs(absolutePath, result.data as Record); + const docErrors = docsResult.issues.filter((i) => i.severity === 'error'); + const docWarnings = docsResult.issues.filter((i) => i.severity !== 'error'); + if (docErrors.length > 0) { + if (flags.json) { + await emitJson({ + valid: false, + errors: docErrors, + warnings: ruleAdvisories, + duration: timer.elapsed(), + }); + this.exit(1); + } + console.log(''); + printError(`Package docs validation failed (${docErrors.length} issue${docErrors.length > 1 ? 's' : ''})`); + for (const i of docErrors.slice(0, 50)) { + console.log(` • ${i.path}: ${i.message}`); + console.log(chalk.dim(` rule: ${i.rule}`)); + } + this.exit(1); + } + // 4. Collect and display stats const stats = collectMetadataStats(config); @@ -682,13 +228,11 @@ export default class Validate extends Command { valid: true, manifest: config.manifest, stats, - // `refWarnings` carries the whole reference-integrity suite, which now - // includes the flow-template-path rule this list used to name directly. - // It was absent here before: on a CLEAN run `--json` reported none of - // the suite's warnings, though the failure path (above) and the console - // both did. Same shape of bug as the dropped errors — computed, then - // discarded — so it is fixed rather than reproduced under a new name. - warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...refWarnings, ...authoringLintWarnings, ...unknownKeyWarnings, ...securityAdvisories, ...capProviderWarnings], + // One advisory list for the whole registry. This used to be a + // hand-maintained concatenation of per-gate arrays, and it leaked + // twice: warnings computed and then dropped from `--json` while the + // console printed them. A single list cannot drift from itself. + warnings: [...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings], conversions: conversionNotices, specVersionGap: specGap, duration: timer.elapsed(), @@ -705,15 +249,6 @@ export default class Validate extends Command { warnings.push(w.message); } - // ADR-0089 D3b — deprecated visibility aliases + mis-layered binding root. - // Checked on `normalized` (PRE-parse): the schema folds `visibleOn`/ - // `visibility` into `visibleWhen` during parse, so `result.data` no longer - // carries the alias the author actually wrote. - const visibilityFindings = validateVisibilityPredicates(normalized as Record); - for (const f of visibilityFindings) { - warnings.push(`${f.where}: ${f.message} — ${f.hint}`); - } - // [#3786] Undeclared object/field keys — computed pre-parse above, // alongside `normalized`, for the same reason. warnings.push(...unknownKeyWarnings); @@ -724,18 +259,18 @@ export default class Validate extends Command { for (const n of conversionNotices) { warnings.push(`${n.path}: '${n.from}' → '${n.to}' (converted at load; conversion '${n.conversionId}', retires in protocol ${n.retiresIn})`); } - for (const i of exprWarnings) { - warnings.push(`${i.where}: ${i.message}`); - } - for (const f of widgetWarnings) { - warnings.push(`${f.where}: ${f.message}`); - } - for (const f of styleWarnings) { + + // Every advisory the registry raised. All of them feed `--strict` now: + // before, roughly half were printed inline and invisible to it, so + // `--strict` failed or passed depending on which gate happened to raise + // the finding — a second, quieter version of the same coverage drift. + for (const f of ruleAdvisories) { warnings.push(`${f.where}: ${f.message}`); } - for (const f of jsxWarnings) { - warnings.push(`${f.where}: ${f.message}`); + for (const w of docWarnings) { + warnings.push(`${w.path}: ${w.message}`); } + if (stats.objects === 0) { warnings.push('No objects defined — this stack has no data model'); } diff --git a/packages/cli/src/lint/authoring-rules.ts b/packages/cli/src/lint/authoring-rules.ts new file mode 100644 index 0000000000..38df529a05 --- /dev/null +++ b/packages/cli/src/lint/authoring-rules.ts @@ -0,0 +1,594 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The author-time rule registry — WHICH rules `os validate`, `os build` and + * `os lint` run, declared as data, with a written reason for every narrowing + * (#4409). + * + * ## Why this exists + * + * Each of the three authoring commands grew its own import list and its own + * call site per rule. Nothing connected them, so "which rules run here?" was + * answerable only by reading three 800-line files and diffing them by eye — and + * the answer drifted every time a rule landed. At the point this registry was + * written, 23 of the 26 hand-wired rules ran on some strict subset of the three, + * and nine of those could emit `severity: 'error'`. The worst direction was not + * the obvious one: `os build` was the WEAKEST of the three gates, so it emitted + * an artifact for stacks `os validate` or `os lint` refuses. A flow whose + * expression approver does not parse (`approval-expression-invalid`, an `error`) + * built and published green — only `os lint` stopped it, and CI usually runs the + * other two. + * + * That failure mode had already been fixed four times, one instance at a time: + * the reference-integrity suite (#3583 §5 D5), the four CLI-local authoring + * lints that ran on `build` alone (#3782), `validateReadonlyFlowWrites` missing + * from `lint` (#4384/#4394), and the wiring guard that followed (#4402). Each + * repair removed an instance and left the MODE — a rule's command coverage was + * whatever its author remembered to type, and forgetting was silent. This file + * replaces "remembering" with a table, and the guard in + * `commands/authoring-rule-wiring.test.ts` makes a narrowing an explicit, + * reasoned edit instead of an omission. + * + * ## The invariant + * + * **Any rule that can emit `error` runs on all three commands.** A gate is only + * as strong as the weakest command an author or CI happens to run, so a gating + * rule with partial coverage is not a stricter check — it is a coin flip. + * + * An `advisory` rule (never emits `error`) MAY be scoped to fewer commands, but + * only with a `scopeReason` recorded here. The distinction that matters is not + * cost, it is consequence: a missing advisory costs the author a hint, a missing + * gate ships broken metadata. + * + * Cost, as it turns out, argues for almost nothing. The heavy dependencies + * (`typescript` ~9 MB, `sucrase` ~1.5 MB) are already lazy and load only when a + * stack actually carries the metadata that needs them — a contract pinned by + * `@objectstack/lint`'s `lazy-deps.test.ts`. `validateReactPageProps`, the + * heaviest rule of the set, has run on all three commands as a suite member + * since #4340 without anyone noticing a cost. So `os lint` stays light on the + * stacks that do not use those surfaces, whether or not the rules are wired. + * + * ## Adding a rule + * + * Append one entry here. It reaches all three commands at once and nothing else + * needs editing. Do NOT import the rule into a command file — the wiring guard + * fails on a direct import, because that is precisely how a rule ends up running + * on two commands out of three. + * + * ## What is NOT in here + * + * This registry covers the rules the three commands SHARE. Two neighbouring + * families are deliberately outside it, and the guard's ratchet lists them by + * name so the boundary stays a decision rather than an oversight: + * + * - **`os lint`'s own style rubric** (snake_case names, missing labels, the + * data-model best-practice sweep, docs, i18n coverage). Its `error` severity + * is a LINT verdict, not a publish gate — `os build` has never rejected a + * camelCase object name and making it do so is a product decision, not a + * wiring fix. + * - **Gates that need more than the stack** — the capability-provider preflight + * (reads `node_modules`), package docs (reads `src/docs/`), the access-matrix + * snapshot (reads/writes a file next to the config). They are I/O, not pure + * metadata rules, and each is wired where its input exists. + */ + +import { + validateStackExpressions, + validateListViewMode, + validateViewContainers, + validateWidgetBindings, + validateDashboardActionRefs, + validateFilterTokens, + validateReferenceIntegrity, + validateResponsiveStyles, + validateJsxPages, + validateReactPages, + validatePageSourceStyling, + validateCapabilityReferences, + validateFlowTriggerReadiness, + validateApprovalApprovers, + validateRecordTitle, + validateSemanticRoles, + validateSeedReplaySafety, + validateSeedStateMachine, + validateVisibilityPredicates, + validateSecurityPosture, + validateOrgAxisRedLines, +} from '@objectstack/lint'; +import { lintFlowPatterns } from '../utils/lint-flow-patterns.js'; +import { lintLivenessProperties } from '../utils/lint-liveness-properties.js'; +import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js'; +import { lintViewRefs } from '../utils/lint-view-refs.js'; +import { lintUniqueDeclarations } from './data-model-rules.js'; + +type AnyRec = Record; + +// ─── Types ────────────────────────────────────────────────────────── + +/** The three commands that hold a stack to the same author-time bar. */ +export const AUTHORING_COMMANDS = ['validate', 'build', 'lint'] as const; +export type AuthoringCommand = (typeof AUTHORING_COMMANDS)[number]; + +/** `error` gates. `warning` advises. `info` is a suggestion (`os lint` grades it as one). */ +export type AuthoringSeverity = 'error' | 'warning' | 'info'; + +/** + * The one finding shape all three commands render. Rules whose own return type + * predates it are adapted at their registry entry, so the commands hold one + * type instead of a twenty-way union. + */ +export interface AuthoringFinding { + severity: AuthoringSeverity; + /** Stable diagnostic rule id (used by docs, allowlists and `--json` consumers). */ + rule: string; + /** Human-readable location, e.g. `object "leave_request"`. */ + where: string; + /** Config path, e.g. `objects[3].sharingModel`. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +/** + * `gating` = the rule can emit `severity: 'error'`, so it MUST run on all three + * commands. `advisory` = it never does, and may be scoped with a reason. + * + * The claim is not taken on trust: the wiring guard reads each `advisory` rule's + * own source and fails if it emits an `error`. That check is the reason the tier + * is worth declaring — #3760 promoted a `lintFlowPatterns` rule from advisory to + * gating, and nothing anywhere asked whether its command coverage should follow. + */ +export type AuthoringRuleTier = 'gating' | 'advisory'; + +/** + * Which tier of the stack a rule reads. + * + * - `normalized` — the `normalizeStackInput` output, BEFORE the Zod parse. The + * rules that need it check keys the parse strips (a flat list view in + * `views: []`, `userFilters` on an object list view, a `visibleOn` alias): by + * the time `result.data` exists the evidence is gone. + * - `parsed` — the post-parse stack, where defaults are filled and shapes are + * settled. + * + * `os lint` never parses (it is the cheap pre-flight; a schema error is + * `os validate`'s verdict to give), so it runs BOTH tiers on the normalized + * stack. Every rule here is written to tolerate that — it is what `os lint` + * already did for the reference-integrity suite and the security linter. + */ +export type AuthoringRuleInputTier = 'normalized' | 'parsed'; + +/** Per-run inputs a rule may need beyond the stack itself. */ +export interface AuthoringRuleContext { + /** ADR-0080 SDUI component manifest, when the project ships one. */ + sduiManifest?: unknown; +} + +export interface AuthoringRule { + /** The exported function's name — the id the wiring guard asserts on. */ + name: string; + tier: AuthoringRuleTier; + input: AuthoringRuleInputTier; + /** Which commands run it. Must be all three when `tier` is `gating`. */ + commands: readonly AuthoringCommand[]; + /** Repo-relative path to the rule's implementation (the guard verifies the tier claim against it). */ + source: string; + /** REQUIRED when `commands` is not all three: why this rule is scoped. */ + scopeReason?: string; + run: (stack: AnyRec, ctx: AuthoringRuleContext) => readonly AuthoringFinding[]; +} + +/** Every command runs every rule unless an entry says otherwise. */ +const ALL: readonly AuthoringCommand[] = AUTHORING_COMMANDS; + +/** + * `ExprIssue` is the one rule finding that carries no rule id of its own — it + * predates the `{ rule, path, hint }` shape every other rule settled on. Given + * one here so `os lint --json` and the docs can name it like any other. + */ +export const EXPRESSION_INVALID = 'expression-invalid'; + +// ─── The registry ─────────────────────────────────────────────────── + +/** + * Every author-time rule the three commands share, in the order their findings + * are reported. + */ +export const AUTHORING_RULES: readonly AuthoringRule[] = [ + // ADR-0032 §1a/1b — CEL predicates in actions/validations/flows/sharing/hooks + // are parsed for syntax AND checked that each `record.` resolves. This + // is what catches a BARE field ref (`done` instead of `record.done`) that + // would otherwise silently hide an action on every record (#2183/#2185). + { + name: 'validateStackExpressions', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-expressions.ts', + run: (stack) => + validateStackExpressions(stack).map((i) => ({ + severity: i.severity ?? 'error', + rule: EXPRESSION_INVALID, + where: i.where, + path: i.where, + message: i.message, + hint: `source: \`${i.source}\``, + })), + }, + // ADR-0053 — `userFilters`/`quickFilters` on an object list view ("views" + // mode) are silently dropped: `ObjectListViewSchema` omits them, so this must + // read the pre-parse tier or the evidence is already gone. + { + name: 'validateListViewMode', + tier: 'gating', + input: 'normalized', + commands: ALL, + source: 'packages/lint/src/validate-list-view-mode.ts', + run: (stack) => validateListViewMode(stack), + }, + // A flat list-view object in `views: []` parses to an EMPTY container + // (ViewSchema strips unknown keys): the schema step passes, zero views + // register, and the Console renders nothing. Pre-parse for the same reason. + { + name: 'validateViewContainers', + tier: 'gating', + input: 'normalized', + commands: ALL, + source: 'packages/lint/src/validate-view-containers.ts', + run: (stack) => validateViewContainers(stack), + }, + // ADR-0021 (#1719/#1721) — a widget's `dataset`/`dimensions`/`values` and its + // chartConfig axis/series must resolve against the declared datasets. + { + name: 'validateWidgetBindings', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-widget-bindings.ts', + run: (stack) => validateWidgetBindings(stack), + }, + // ADR-0049 / #3367 — a header or widget action naming a `script`/`modal` + // target that resolves to no defined action ships a button that renders and + // silently does nothing on click. Unresolved `url` routes stay advisory. + { + name: 'validateDashboardActionRefs', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-dashboard-action-refs.ts', + run: (stack) => validateDashboardActionRefs(stack), + }, + // #3574 — a filter value like `{current_user}` resolves in no vocabulary, + // reaches the data engine as a literal and matches nothing. The surface + // renders empty with no error, and a silent zero is indistinguishable from a + // genuine one at review time. + { + name: 'validateFilterTokens', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-filter-tokens.ts', + run: (stack) => validateFilterTokens(stack), + }, + // The reference-integrity suite (#3583 §5 D5) — itself a registry, of the + // rules that answer "does this name resolve to anything?". It reached all + // three commands before this file existed; it is an entry here so the two + // registries compose instead of competing, and so its members are covered by + // the same guard as everything else. + { + name: 'validateReferenceIntegrity', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/reference-integrity-suite.ts', + run: (stack) => validateReferenceIntegrity(stack), + }, + // ADR-0065 — a styled node's responsiveStyles must be scopable (needs an + // `id`), name real CSS properties + design tokens, and carry a `large` base. + { + name: 'validateResponsiveStyles', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-responsive-styles.ts', + run: (stack) => validateResponsiveStyles(stack), + }, + // ADR-0080 — a `kind:'jsx'` page's `source` is parsed (never executed) and + // compiled to the SDUI tree at save time, so malformed source must fail loudly + // here (ADR-0078) instead of being stored and breaking only at render. + { + name: 'validateJsxPages', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-jsx-pages.ts', + run: (stack, ctx) => + validateJsxPages(stack, ctx.sduiManifest ? { manifest: ctx.sduiManifest as never } : {}), + }, + // ADR-0081 — a `kind:'react'` page's `source` is real React executed at + // render. Transpiled here (Sucrase, never executed) so a syntax error fails at + // author time, not at render. Lazy: only a stack with such a page pays. + { + name: 'validateReactPages', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-react-pages.ts', + run: (stack) => validateReactPages(stack), + }, + // ADR-0065, source tier — Tailwind `className` in a `kind:'html'`/`'react'` + // page silently no-ops (the build never scans authored metadata). + { + name: 'validatePageSourceStyling', + tier: 'advisory', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-page-source-styling.ts', + run: (stack) => validatePageSourceStyling(stack), + }, + // ADR-0066 ⑨ — a `requiredPermissions` entry naming a capability registered + // nowhere fails closed at runtime. Advisory: another installed package may + // legitimately provide it. + { + name: 'validateCapabilityReferences', + tier: 'advisory', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-capability-references.ts', + run: (stack) => validateCapabilityReferences(stack), + }, + // A record-change flow whose start-node objectName matches nothing never + // fires — silently. Reads the pre-parse tier so an author sees what they + // wrote. Advisory: the object may come from another installed package. + { + name: 'validateFlowTriggerReadiness', + tier: 'advisory', + input: 'normalized', + commands: ALL, + source: 'packages/lint/src/validate-flow-trigger-readiness.ts', + run: (stack) => validateFlowTriggerReadiness(stack), + }, + // ADR-0090 D3 fallout — an approval `{ type: 'role' }` resolves against the + // better-auth org-membership tier, not positions, so a position name authored + // there routes the approval to nobody; and an expression approver that does + // not parse can never resolve. The rule whose absence from `os build` and + // `os validate` was #4409's worked example: it gates, and it ran on `os lint` + // alone, so a broken approval flow built and published green. + { + name: 'validateApprovalApprovers', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-approval-approvers.ts', + run: (stack) => validateApprovalApprovers(stack), + }, + // ADR-0079 — `titleFormat` is retired in favour of `nameField`, and an object + // with no resolvable title ships records with no meaningful name. Advisory: + // auto-provision and the `Record #` floor keep it from ever being fatal. + { + name: 'validateRecordTitle', + tier: 'advisory', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-record-title.ts', + run: (stack) => validateRecordTitle(stack), + }, + // ADR-0085 — `stageField` / `highlightFields` / `Field.group` are pointers + // into the object's field map; a dangling one is Zod-valid and silently inert + // at render. Advisory: every consumer degrades gracefully. + { + name: 'validateSemanticRoles', + tier: 'advisory', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-semantic-roles.ts', + run: (stack) => validateSemanticRoles(stack), + }, + // framework#3434 — seeds replay on every boot, so a `mode: 'insert'` dataset + // duplicates its table on every restart. + { + name: 'validateSeedReplaySafety', + tier: 'advisory', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-seed-replay-safety.ts', + run: (stack) => validateSeedReplaySafety(stack), + }, + // framework#3433 follow-up — #3433 exempts seed writes from the + // `state_machine` rule, so a seeded status the FSM does not declare is no + // longer rejected at write time. Re-added at author time; advisory, because + // the exemption itself is legitimate. + { + name: 'validateSeedStateMachine', + tier: 'advisory', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-seed-state-machine.ts', + run: (stack) => validateSeedStateMachine(stack), + }, + // ADR-0089 D3b — deprecated visibility aliases and a mis-layered binding root. + // Pre-parse: the schema folds `visibleOn`/`visibility` into `visibleWhen` + // during parse, so the alias the author wrote is gone from `result.data`. + { + name: 'validateVisibilityPredicates', + tier: 'advisory', + input: 'normalized', + commands: ALL, + source: 'packages/lint/src/validate-visibility-predicates.ts', + run: (stack) => validateVisibilityPredicates(stack), + }, + // #1874 — flow authoring anti-patterns. Advisory by default; a finding marked + // `error` gates (#3760 promoted `flow-runas-unscoped`, which flags metadata + // the runtime now REFUSES to execute). + { + name: 'lintFlowPatterns', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/cli/src/utils/lint-flow-patterns.ts', + run: (stack) => + lintFlowPatterns(stack).map((f) => ({ + severity: f.severity ?? 'warning', + rule: f.rule, + where: f.where, + path: f.where, + message: f.message, + hint: f.hint, + })), + }, + // The spec-liveness loop on the author side: a property the ledger marks + // dead-and-misleading or experimental is set hopefully and does nothing. + // Ledger-driven (entries opt in via `authorWarn`), so it is high-signal and + // never fatal. + { + name: 'lintLivenessProperties', + tier: 'advisory', + input: 'parsed', + commands: ALL, + source: 'packages/cli/src/utils/lint-liveness-properties.ts', + run: (stack) => + lintLivenessProperties(stack).map((f) => ({ + severity: 'warning' as const, + rule: f.rule, + where: f.where, + path: f.where, + message: f.message, + hint: f.hint, + })), + }, + // A format like `{plan_no}{000}` makes the referenced field part of the + // counter scope, so it must exist and be set at create time. Unknown field → + // broken (error); optional field → fragile (warning). + { + name: 'lintAutonumberFormats', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/cli/src/utils/lint-autonumber-formats.ts', + run: (stack) => + lintAutonumberFormats(stack).map((f) => ({ + severity: f.severity, + rule: f.rule, + where: f.where, + path: f.where, + message: f.message, + hint: f.hint, + })), + }, + // #2554 — a `type:'form'` action target naming a missing or LIST view opens a + // broken form at runtime; a list/form view-key collision silently renames one + // view so references resolve to the OTHER. Both are broken. + { + name: 'lintViewRefs', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/cli/src/utils/lint-view-refs.ts', + run: (stack) => + lintViewRefs(stack).map((f) => ({ + severity: f.severity, + rule: f.rule, + where: f.where, + path: f.where, + message: f.message, + hint: f.hint, + })), + }, + // #3991 — a column carrying BOTH a field-level `unique: true` and a + // single-column declared unique index has two intents, of which exactly one + // takes effect (the global index wins; the tenant composite is unreachable). + { + name: 'lintUniqueDeclarations', + tier: 'advisory', + input: 'parsed', + commands: ['validate', 'build'], + source: 'packages/cli/src/lint/data-model-rules.ts', + scopeReason: + "`os lint` already reports this rule through `lintDataModel`, which calls it directly as R10 of " + + 'its best-practice sweep — registering it for `lint` as well would report every finding twice. ' + + 'This is coverage recorded, not coverage missing: all three commands report the rule.', + run: (stack) => + lintUniqueDeclarations(Array.isArray(stack.objects) ? (stack.objects as unknown[]) : []).map((f) => ({ + severity: f.severity === 'suggestion' ? ('info' as const) : f.severity, + rule: f.rule, + where: f.path, + path: f.path, + message: f.message, + hint: f.fix ?? '', + })), + }, + // ADR-0090 D7 — the security-domain publish linter. Every `error` rule mirrors + // a runtime enforcement point (fail-closed OWD default, canonical enum, anchor + // binding gate, vocabulary freeze), moving the failure from a runtime deny to + // an author-time fix-it. Per ADR-0049 this is not advisory security. + { + name: 'validateSecurityPosture', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-security-posture.ts', + run: (stack) => validateSecurityPosture(stack), + }, + // ADR-0105 D6 — the org tree is a REPORTING dimension. An RLS policy or + // sharing rule that walks it builds a second permission hierarchy (the + // dual-hierarchy mistake ADR-0057 D5 retired) and cannot widen Layer 0 anyway, + // so it grants nothing it appears to. + { + name: 'validateOrgAxisRedLines', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-org-axis-red-lines.ts', + run: (stack) => validateOrgAxisRedLines(stack), + }, +]; + +// ─── Runner ───────────────────────────────────────────────────────── + +/** The stack tiers a command has in hand when it runs the registry. */ +export interface AuthoringRuleRun extends AuthoringRuleContext { + /** `normalizeStackInput` output — pre-Zod-parse. Always required. */ + normalized: AnyRec; + /** + * Post-Zod-parse stack. Omitted by `os lint`, which does not parse; `parsed` + * rules then read `normalized` (see `AuthoringRuleInputTier`). + */ + parsed?: AnyRec; +} + +/** The rules `command` runs, in registry order. */ +export function authoringRulesFor(command: AuthoringCommand): readonly AuthoringRule[] { + return AUTHORING_RULES.filter((r) => r.commands.includes(command)); +} + +/** + * Run every rule registered for `command` and return the concatenated findings + * (empty = clean). + * + * Findings are collected across ALL rules rather than short-circuiting at the + * first failing one. The commands used to exit at the first failing gate, which + * meant an author with three unrelated problems fixed them in three round trips + * and could not tell how deep the hole went. One report per run is also what + * makes the three commands comparable: same rules, same order, same output. + */ +export function runAuthoringRules(command: AuthoringCommand, run: AuthoringRuleRun): AuthoringFinding[] { + const findings: AuthoringFinding[] = []; + const ctx: AuthoringRuleContext = { sduiManifest: run.sduiManifest }; + for (const rule of authoringRulesFor(command)) { + const stack = rule.input === 'normalized' ? run.normalized : (run.parsed ?? run.normalized); + findings.push(...rule.run(stack, ctx)); + } + return findings; +} + +/** Split findings into the gating set and the advisory set (`warning` + `info`). */ +export function splitBySeverity(findings: readonly AuthoringFinding[]): { + errors: AuthoringFinding[]; + advisories: AuthoringFinding[]; +} { + return { + errors: findings.filter((f) => f.severity === 'error'), + advisories: findings.filter((f) => f.severity !== 'error'), + }; +} diff --git a/packages/cli/src/utils/sdui-manifest.ts b/packages/cli/src/utils/sdui-manifest.ts new file mode 100644 index 0000000000..865d79ffd6 --- /dev/null +++ b/packages/cli/src/utils/sdui-manifest.ts @@ -0,0 +1,43 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Resolve the optional ADR-0080 SDUI component manifest. + * + * `validateJsxPages` does full component/prop validation when a manifest is in + * hand and falls back to parse-level checking when it is not. The resolution + * order (project file, then the copy shipped inside `@objectstack/console`) used + * to live inline in `validate.ts` — the only command that ran the JSX gate. Once + * `os build` and `os lint` run it too (#4409), a rule whose STRENGTH depends on + * how its caller resolves an input is a second drift axis waiting to open: the + * same page could pass on one command and fail on another purely because one + * call site forgot the console fallback. One resolver, three callers. + */ + +import { existsSync, readFileSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { join } from 'node:path'; + +/** + * The manifest for the current project, or `undefined` when neither source is + * present. Never throws: an unreadable or malformed manifest degrades to + * parse-level JSX validation, which is what the gate did before manifests + * existed. + */ +export function resolveSduiManifest(): unknown { + try { + const projectManifest = join(process.cwd(), 'sdui.manifest.json'); + if (existsSync(projectManifest)) { + const parsed = JSON.parse(readFileSync(projectManifest, 'utf8')); + if (parsed) return parsed; + } + // Fall back to the manifest shipped inside @objectstack/console (built from + // objectui's public-tier registry; the CLI already depends on it). + const consoleManifest = createRequire(import.meta.url).resolve( + '@objectstack/console/dist/sdui.manifest.json', + ); + if (existsSync(consoleManifest)) return JSON.parse(readFileSync(consoleManifest, 'utf8')); + } catch { + /* fall back to parse-level */ + } + return undefined; +} diff --git a/packages/cli/test/authoring-rule-command-parity.test.ts b/packages/cli/test/authoring-rule-command-parity.test.ts new file mode 100644 index 0000000000..e3c5d83e41 --- /dev/null +++ b/packages/cli/test/authoring-rule-command-parity.test.ts @@ -0,0 +1,195 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The BEHAVIOURAL half of #4409, next to the structural ratchet in + * `src/commands/authoring-rule-wiring.test.ts`. + * + * The ratchet proves the three authoring commands are WIRED to the same rule + * table. This file proves the wiring produces the same VERDICT: one stack, one + * planted defect, three commands, three rejections. Wiring and verdict are + * genuinely different claims — a command can run a rule and still not gate on + * what it says, which is how #3782's four lints were "wired" into `os build` + * while `os validate` ran and then ignored the same class of finding. + * + * Every case below is a rule the issue MEASURED as running on a strict subset + * of the three, with the command it was blind to named. Each one could emit + * `error`, so each was a gate whose verdict depended on which command CI + * happened to run — nine coin flips. The end-to-end case at the bottom is the + * issue's own repro, run through the real CLI rather than the registry: + * + * os lint EXIT=1 ✗ approval-expression-invalid + * os validate EXIT=0 + * os build EXIT=0 ← the artifact shipped anyway + */ + +import { describe, expect, it } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { AUTHORING_COMMANDS, runAuthoringRules, type AuthoringCommand } from '../src/lint/authoring-rules.js'; + +const cliBin = join(fileURLToPath(new URL('.', import.meta.url)), '..', 'bin', 'run-dev.js'); + +/** A stack that satisfies the security linter, so only the planted defect gates. */ +const withBaseline = (stack: Record) => ({ + manifest: { id: 'parity', namespace: 'parity', version: '1.0.0', name: 'Parity', type: 'app', engines: { protocol: '^17' } }, + ...stack, +}); + +const listView = (object: string) => ({ type: 'grid', label: 'All', columns: ['title'], data: { provider: 'object', object } }); +const formView = (object: string) => ({ type: 'simple', data: { provider: 'object', object }, sections: [] }); + +/** + * One case per rule #4409 measured as partially covered, each planting the + * defect that rule exists to catch. `blindTo` records which command let it + * through before the registry — it is documentation, not an assertion, so the + * table still reads as the issue's matrix once every hole is closed. + */ +const CASES: ReadonlyArray<{ rule: string; blindTo: readonly AuthoringCommand[]; stack: Record }> = [ + // ── P1: gates `os build` did not run, so it PUBLISHED what the others refuse ── + { + rule: 'approval-expression-invalid', + blindTo: ['validate', 'build'], + stack: withBaseline({ + objects: [{ name: 'parity_task', label: 'Task', sharingModel: 'private', fields: { name: { type: 'text', label: 'Name' } } }], + flows: [{ + name: 'parity_flow', label: 'Parity', type: 'record_change', runAs: 'system', status: 'active', + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: { objectName: 'parity_task', triggerType: 'onCreate' } }, + { id: 'appr', type: 'approval', label: 'Approve', config: { approvers: [{ type: 'expression', value: 'record.owner ==' }] } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'appr', type: 'default' }, + { id: 'e2', source: 'appr', target: 'end', type: 'default' }, + ], + }], + }), + }, + { + rule: 'list-view-filters-in-views-mode', + blindTo: ['build', 'lint'], + stack: withBaseline({ + objects: [{ name: 'parity_task', label: 'Task', sharingModel: 'private', listViews: { tabular: { label: 'Tabular', userFilters: { element: 'tabs' } } }, fields: { name: { type: 'text', label: 'Name' } } }], + }), + }, + { + rule: 'view-container-shape', + blindTo: ['build', 'lint'], + stack: withBaseline({ + views: [{ name: 'all_tasks', label: 'All Tasks', type: 'grid', data: { provider: 'object', object: 'parity_task' }, columns: ['title'] }], + }), + }, + // ── P2: gates `os lint` did not run, so the cheap pre-flight lied ── + { + rule: 'expression-invalid', + blindTo: ['lint'], + stack: withBaseline({ + objects: [{ name: 'parity_lead', label: 'Lead', sharingModel: 'private', fields: { lead_score: { type: 'number', label: 'Score' } }, validations: [{ name: 'r', expression: 'lead_score > 100' }] }], + }), + }, + { + rule: 'filter-token-unknown', + blindTo: ['lint'], + stack: withBaseline({ + dashboards: [{ name: 'exec', label: 'Exec', widgets: [{ id: 'mine', type: 'metric', dataset: 'case_metrics', filter: { owner: '{current_user}', status: 'open' } }] }], + }), + }, + { + rule: 'dashboard-action-target-undefined', + blindTo: ['lint'], + stack: withBaseline({ + dashboards: [{ name: 'exec', label: 'Exec', header: { actions: [{ label: 'Export PDF', actionType: 'script', actionUrl: 'export_dashboard_pdf' }] }, widgets: [] }], + }), + }, + { + rule: 'style-node-missing-id', + blindTo: ['lint'], + stack: withBaseline({ + pages: [{ name: 'pricing', regions: [{ name: 'main', components: [{ type: 'flex', responsiveStyles: { large: { padding: 'var(--space-4)' } } }] }] }], + }), + }, + { + rule: 'autonumber-references-unknown-field', + blindTo: ['lint'], + stack: withBaseline({ + objects: [{ name: 'parity_task', label: 'Task', sharingModel: 'private', fields: { task_no: { type: 'autonumber', label: 'No', autonumberFormat: '{plan_no}{000}' } } }], + }), + }, + { + rule: 'view-ref-form-target-kind', + blindTo: ['lint'], + stack: withBaseline({ + views: [{ name: 'parity_task', list: listView('parity_task'), formViews: { default: formView('parity_task') } }], + actions: [{ name: 'log_time', type: 'form', target: 'parity_task.default' }], + }), + }, +]; + +describe('every authoring command reaches the same verdict (#4409)', () => { + it.each(CASES.map((c) => [c.rule, c] as const))('%s gates on all three commands', (_rule, testCase) => { + for (const command of AUTHORING_COMMANDS) { + // `os lint` never Zod-parses, so it hands the normalized stack to both + // tiers — modelled here exactly as the command does it. + const findings = runAuthoringRules(command, { + normalized: testCase.stack, + ...(command === 'lint' ? {} : { parsed: testCase.stack }), + }); + const gating = findings.filter((f) => f.severity === 'error').map((f) => f.rule); + expect( + gating, + `os ${command} does not gate on ${testCase.rule} — it did not before #4409 either ` + + `(blind: ${testCase.blindTo.join(', ')}), which is the drift this registry exists to end.`, + ).toContain(testCase.rule); + } + }); + + it('the case table still covers every command as a blind spot', () => { + // Guard the guard: if the table drifted to cover only one direction, the + // suite above would keep passing while testing half the problem. The issue + // found holes in BOTH — `os build` publishing what `os lint` refuses, and + // `os lint` passing what `os build` rejects. + const blind = new Set(CASES.flatMap((c) => c.blindTo)); + expect([...blind].sort()).toEqual(['build', 'lint', 'validate']); + }); + + /** + * The issue's own repro, end to end through the real CLI: a flow whose + * expression approver does not parse used to exit 1 on `os lint` and 0 on + * both `os validate` and `os build` — so the build emitted an artifact for a + * flow the runtime refuses to run. + * + * Spawned rather than run in-process: the exit CODE is the contract CI reads, + * and only a real process produces it. + */ + it('the broken approval flow now exits non-zero on all three commands', () => { + const dir = mkdtempSync(join(tmpdir(), 'os-authoring-parity-')); + try { + // A plain literal config: no imports, so it resolves without a + // node_modules next to it. + writeFileSync( + join(dir, 'objectstack.config.mjs'), + `export default ${JSON.stringify(CASES[0].stack, null, 2)};\n`, + ); + + for (const command of ['lint', 'validate', 'build']) { + let exitCode = 0; + let output = ''; + try { + output = execFileSync(process.execPath, [cliBin, command], { cwd: dir, encoding: 'utf8', stdio: 'pipe' }); + } catch (error: any) { + exitCode = error.status ?? 1; + output = `${error.stdout ?? ''}${error.stderr ?? ''}`; + } + expect(exitCode, `os ${command} exited 0 on a flow whose approver expression does not parse`).toBe(1); + expect(output, `os ${command} rejected the stack without naming the rule`).toContain( + 'approval-expression-invalid', + ); + } + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, 120_000); +}); diff --git a/packages/cli/test/validate-build-gate-parity.test.ts b/packages/cli/test/validate-build-gate-parity.test.ts index 1f35a17c7c..caa3b532d5 100644 --- a/packages/cli/test/validate-build-gate-parity.test.ts +++ b/packages/cli/test/validate-build-gate-parity.test.ts @@ -7,75 +7,129 @@ import { join } from 'node:path'; /** * `os validate` is documented — and relied on by CI setups — as the READ-ONLY * SUPERSET of the gates `os build` runs: same checks, no artifact emitted. That - * contract has no enforcement, so it drifted (#3782): four authoring lints - * (`lintFlowPatterns`, `lintLivenessProperties`, `lintAutonumberFormats`, - * `lintViewRefs`) were wired into `compile.ts` only. Two of them already emitted - * `severity: 'error'`, so `os validate` reported a clean stack that `os build` - * then rejected — the precise failure the contract exists to prevent. + * contract had no enforcement, so it drifted (#3782): four authoring lints were + * wired into `compile.ts` only, two of them already emitting `severity: 'error'`, + * so `os validate` reported a clean stack that `os build` then rejected. * - * The drift was invisible because every OTHER gate is a `@objectstack/lint` - * import shared by both files, while these four are CLI-local `../utils/lint-*` - * modules that only `compile.ts` ever imported. + * ## What changed, and what this file still guards * - * This is a source-level gate rather than a behavioural one on purpose: it fails - * when a gate is ADDED to the build without being added to validate, which is - * the moment the mistake is cheap to fix — not later, when some app trips it. + * The metadata rules the two commands share now come from ONE table + * (`src/lint/authoring-rules.ts`, #4409), and its own ratchet — + * `src/commands/authoring-rule-wiring.test.ts` — proves all three authoring + * commands run the identical gating set. That is a stronger guarantee than the + * source diff this file used to do, and it covers `os lint` too. + * + * What the registry CANNOT cover is the gates that are not pure functions of the + * stack: the capability-provider preflight reads `node_modules`, the docs lint + * reads `src/docs/`, the access-matrix snapshot reads a file next to the config. + * Those are still hand-wired per command, so they can still drift — and one of + * them already had. `collectAndLintDocs` gated `os build` and never ran on + * `os validate`, invisible for the same reason the #3782 four were: the old + * scan keyed on the `lint*`/`validate*` naming convention, and this gate is + * named `collect*`. This file now names each shared gate explicitly instead of + * pattern-matching for them. + * + * Source-level rather than behavioural on purpose: it fails when a gate is ADDED + * to the build without being added to validate, which is the moment the mistake + * is cheap to fix — not later, when some app trips it. */ const COMMANDS_DIR = join(__dirname, '..', 'src', 'commands'); +/** + * Gates that are NOT registry rules (they need the filesystem or the emitted + * artifact) and that both commands must therefore wire by hand. + * + * Adding a gate to `compile.ts` means adding it here and to `validate.ts`, or + * to `BUILD_ONLY_GATES` below with a reason. There is no third option — that is + * the whole point of the file. + */ +const SHARED_NON_REGISTRY_GATES: readonly string[] = [ + // [#3366] Resolves each `requires` token's provider in the active edition. + 'preflightRequiredCapabilities', + // [#3786] The pre-parse undeclared-key diff, both halves. + 'lintUnknownStackKeys', + 'lintUnknownAuthoringKeys', + // [ADR-0046] Package docs: flatness, prefixed names, MDX/image ban, links. + 'collectAndLintDocs', +]; + /** * Gates `os build` may legitimately run that `os validate` does not. * - * Adding an entry here is a deliberate assertion that the check CANNOT be made - * read-only (it needs the emitted artifact, the bundler, the filesystem output). - * A gate that merely *reads* the parsed stack does not belong here — wire it - * into `validate.ts` instead. Empty today, and that is the healthy state. + * Each entry is a deliberate assertion that the check CANNOT be made read-only + * — it needs the emitted artifact, the bundler, or filesystem output. A gate + * that merely *reads* the parsed stack does not belong here; wire it into + * `validate.ts`, or better, register it in `src/lint/authoring-rules.ts` so all + * three authoring commands get it at once. */ -const BUILD_ONLY_GATES: readonly string[] = []; +const BUILD_ONLY_GATES: Readonly> = { + buildAccessMatrix: + '[ADR-0090 D6] The snapshot gate reads (and with --update-access-matrix WRITES) access-matrix.json ' + + 'next to the config. Rewriting a committed snapshot is not a read-only operation.', + diffAccessMatrix: 'The comparison half of the same D6 snapshot gate.', + lowerCallables: + 'Lowers inline `function` handlers to string refs so they survive JSON.stringify. It exists to ' + + 'produce the artifact; there is nothing to lower when nothing is emitted.', + buildRuntimeBundle: 'Emits the objectstack-runtime.{hash}.mjs sibling module. Artifact output by definition.', +}; + +const sourceOf = (file: string) => readFileSync(join(COMMANDS_DIR, file), 'utf8'); /** Every `lintFoo(`/`validateFoo(` call site in a command's source. */ function gateCallsIn(file: string): Set { - const src = readFileSync(join(COMMANDS_DIR, file), 'utf8'); - const calls = src.match(/\b(?:lint|validate)[A-Z]\w*(?=\s*\()/g) ?? []; + const calls = sourceOf(file).match(/\b(?:lint|validate)[A-Z]\w*(?=\s*\()/g) ?? []; return new Set(calls); } -describe('os validate is the read-only superset of os build (#3782)', () => { - it('runs every gate compile.ts runs', () => { +/** Is `name` invoked anywhere in this command's source? */ +const calls = (file: string, name: string) => new RegExp(String.raw`\b${name}\s*\(`).test(sourceOf(file)); + +describe('os validate is the read-only superset of os build (#3782, #4409)', () => { + it('both commands run the shared authoring-rule registry', () => { + for (const file of ['compile.ts', 'validate.ts']) { + expect(calls(file, 'runAuthoringRules'), `${file} must run the authoring-rule registry`).toBe(true); + } + }); + + it.each(SHARED_NON_REGISTRY_GATES)('both commands run %s', (gate) => { + // Guard the guard: a gate that has been renamed or deleted must fail here + // rather than pass vacuously on both sides. + expect(calls('compile.ts', gate), `compile.ts no longer calls ${gate} — is this list stale?`).toBe(true); + expect( + calls('validate.ts', gate), + `os build runs ${gate} and os validate does not, so a stack can pass 'os validate' and fail ` + + `'os build'. Wire it into packages/cli/src/commands/validate.ts (mirroring compile.ts's severity ` + + `handling), or — only if it genuinely cannot run without emitting an artifact — move it to ` + + `BUILD_ONLY_GATES in this file with a reason.`, + ).toBe(true); + }); + + it('compile.ts hand-wires no gate validate.ts is missing', () => { const compileGates = gateCallsIn('compile.ts'); const validateGates = gateCallsIn('validate.ts'); - // Guard the guard: if the extraction regex silently stops matching, the - // set-difference below passes vacuously and the gate goes quietly dead. - expect(compileGates.size).toBeGreaterThan(10); + // Non-vacuity: the extraction must still find the pre-parse key lints. + expect(compileGates.size).toBeGreaterThan(0); const missing = [...compileGates] .filter((g) => !validateGates.has(g)) - .filter((g) => !BUILD_ONLY_GATES.includes(g)) + .filter((g) => !(g in BUILD_ONLY_GATES)) .sort(); expect( missing, - `os build runs ${missing.length} gate(s) that os validate does not, so a stack ` + - `can pass 'os validate' and fail 'os build': ${missing.join(', ')}.\n` + - `Wire each into packages/cli/src/commands/validate.ts (mirroring the severity ` + - `handling in compile.ts), or — only if it genuinely cannot run without emitting ` + - `an artifact — add it to BUILD_ONLY_GATES in this file with a reason.`, + `os build runs ${missing.length} gate(s) that os validate does not: ${missing.join(', ')}.\n` + + `Register it in packages/cli/src/lint/authoring-rules.ts so all three authoring commands run ` + + `it, wire it into validate.ts by hand and add it to SHARED_NON_REGISTRY_GATES, or add it to ` + + `BUILD_ONLY_GATES with a reason.`, ).toEqual([]); }); - it('runs the four CLI-local authoring lints that regressed in #3782', () => { - const validateGates = gateCallsIn('validate.ts'); - - for (const gate of [ - 'lintFlowPatterns', - 'lintLivenessProperties', - 'lintAutonumberFormats', - 'lintViewRefs', - ]) { - expect(validateGates.has(gate), `validate.ts must call ${gate}`).toBe(true); - } + it('every BUILD_ONLY_GATES entry is still called by the build', () => { + // A ratchet nobody prunes rots into a permission slip. + const stale = Object.keys(BUILD_ONLY_GATES).filter((g) => !calls('compile.ts', g)); + expect(stale, `BUILD_ONLY_GATES entries compile.ts no longer calls: ${stale.join(', ')}`).toEqual([]); }); /** @@ -92,7 +146,7 @@ describe('os validate is the read-only superset of os build (#3782)', () => { */ it('both commands pass a conversion-notice sink to normalizeStackInput', () => { for (const file of ['compile.ts', 'validate.ts']) { - const src = readFileSync(join(COMMANDS_DIR, file), 'utf8'); + const src = sourceOf(file); const call = src.match(/normalizeStackInput\([\s\S]{0,400}?\)\s*;/); expect(call, `${file} must call normalizeStackInput`).not.toBeNull(); expect( From c14caaa7cd40ee1047c0605b4b52292b76a0a868 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 07:49:39 +0000 Subject: [PATCH 2/3] chore(changeset): author-time rule command-coverage registry (#4409) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Sz61GE1CWCSnGW4qShSEXs --- ...uthoring-rule-command-coverage-registry.md | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 .changeset/authoring-rule-command-coverage-registry.md diff --git a/.changeset/authoring-rule-command-coverage-registry.md b/.changeset/authoring-rule-command-coverage-registry.md new file mode 100644 index 0000000000..36c8785ee3 --- /dev/null +++ b/.changeset/authoring-rule-command-coverage-registry.md @@ -0,0 +1,68 @@ +--- +"@objectstack/cli": minor +--- + +fix(cli): every author-time rule that can gate runs on all three commands (#4409) + +`os validate`, `os build` and `os lint` each hand-wired their own subset of the +author-time rules. Nothing connected the three lists, so "which rules run here?" +was answerable only by diffing three 800-line files by eye — and the answer +drifted every time a rule landed. The audit found 23 of 26 rules running on some +strict subset, nine of them able to emit `error`. + +The worst direction was the least obvious. `os build` — the command that +PUBLISHES — was the weakest gate of the three: a flow whose expression approver +does not parse (`approval-expression-invalid`) built and published green, and +only `os lint` stopped it, while CI usually runs the other two. `os lint` +disagreed in *both* directions at once, running one gating rule neither other +command ran and missing six that both of them ran, which is worse than no +pre-flight — the remaining options are re-verifying everything or learning to +distrust the signal. + +This is the same failure mode's fifth appearance (#3583, #3782, #4384/#4394, +#4402). Each earlier repair removed an instance and left the MODE: a rule's +command coverage was whatever its author remembered to type, and forgetting was +silent. #4402's guard could not catch the rest — it filtered on the current +member names of one suite, so a rule hand-wired into two commands from outside +that suite passed it without a word. A name list only guards the names on it. + +**The registry.** `AUTHORING_RULES` declares all 26 rules as data: tier +(`gating`/`advisory`), which stack tier they read (pre-parse `normalized` vs +`parsed`), which commands run them, and a written reason for the one narrowing. +All three commands consume it through `runAuthoringRules()`, so adding a rule is +a one-line edit that reaches every command at once. The three command files +shrink by ~1000 lines between them. + +**The ratchet.** The wiring guard is no longer a name list: a `gating` rule on +fewer than three commands fails, a narrowed rule with no reason fails, a command +that calls or imports a registry rule directly fails, and an `advisory` claim is +checked against the rule's own source — so a gate cannot wear an advisory label +to buy itself partial coverage. That last check is the one #3760 needed, having +promoted a `lintFlowPatterns` rule from advisory to gating with nothing anywhere +asking whether its coverage should follow. Remaining direct calls are listed +with reasons, and a stale entry fails too, so the ratchet cannot rot into a +permanent permission slip. + +**The verdict, not just the wiring.** A separate test plants one defect per +previously-blind gating rule and asserts all three commands gate on it, plus the +issue's own repro driven end-to-end through the real CLI: exit 1 on all three +where it was 1/0/0. + +Two behaviour changes fall out of reporting every failing rule in one run +instead of exiting at the first failing gate: an author with three unrelated +problems now sees all three in one pass, and `--strict` covers every advisory +rather than the roughly half that happened to be printed inline. + +Also closes the same hole one gate over: `collectAndLintDocs` failed `os build` +and never ran on `os validate`, invisible because the parity guard keyed on the +`lint*`/`validate*` naming convention and that gate is called `collect*`. The +guard now names each shared non-registry gate explicitly instead of +pattern-matching for them. + +Cost is not what argued against any of this. The heavy dependencies +(`typescript` ~9 MB, `sucrase`) are already lazy and load only when a stack +carries the metadata that needs them, and the heaviest rule of the set has run +on all three commands as a reference-integrity suite member since #4340 without +anyone noticing. The one narrowed rule, `lintUniqueDeclarations`, is scoped +because `os lint` already reports it through `lintDataModel` — coverage +recorded, not coverage missing. From 29741ecef2f6d7f042e12d626faacd3969ed4b96 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 07:54:54 +0000 Subject: [PATCH 3/3] docs: name the ADR-0085 pointers by field, not by the reserved word (#4409) The role-word ratchet (ADR-0090 D3) counts occurrences per file, and two doc lines describing `validateSemanticRoles` reintroduced the banned word for a non-permission concept. Name what the rule actually checks instead: `stageField` / `highlightFields` pointers. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Sz61GE1CWCSnGW4qShSEXs --- content/docs/deployment/cli.mdx | 5 +++-- content/docs/deployment/validating-metadata.mdx | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 54c90ba576..1bd72f351f 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -452,8 +452,9 @@ stopping at the first, so one pass shows the whole hole. - Missing `manifest.namespace` (required for multi-app hosting) - No objects defined - No apps or plugins defined -- Every advisory the rule registry raised (dangling semantic-role pointers, - replay-unsafe seeds, ambiguous flow status, deprecated visibility aliases, …) +- Every advisory the rule registry raised (dangling `stageField` / + `highlightFields` pointers, replay-unsafe seeds, ambiguous flow status, + deprecated visibility aliases, …) `os validate`, `os build` and `os lint` share one rule registry, so a config that diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index 50db7b96c3..c78c1e3124 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -346,7 +346,7 @@ author-time rules, from one table — `AUTHORING_RULES` in | Autonumber `{field}` interpolation | ✓ | ✓ | ✓ | | View references — form targets, view-key collisions (#2554) | ✓ | ✓ | ✓ | | Flow authoring anti-patterns (#1874) | ✓ | ✓ | ✓ | -| Advisory: flow trigger wiring, record titles, semantic roles, seed replay/state safety, capability references, liveness, visibility aliases | ✓ | ✓ | ✓ | +| Advisory: flow trigger wiring, record titles, semantic field pointers (ADR-0085), seed replay/state safety, capability references, liveness, visibility aliases | ✓ | ✓ | ✓ | | Package docs — flatness, prefixes, links (ADR-0046) | ✓ | ✓ | ✓ | | Undeclared authoring keys — every metadata collection (#3786) and the stack's own top-level keys (#4167) | ✓ | ✓ | — | | Naming, labels, data-model conventions, i18n coverage | — | — | ✓ |