diff --git a/.changeset/validate-runs-build-authoring-lints.md b/.changeset/validate-runs-build-authoring-lints.md new file mode 100644 index 0000000000..a0eb230a22 --- /dev/null +++ b/.changeset/validate-runs-build-authoring-lints.md @@ -0,0 +1,45 @@ +--- +"@objectstack/cli": patch +--- + +fix(cli): `os validate` runs the four authoring lints `os build` runs — "validate clean, build fails" is gone (#3782) + +`os validate` is documented, and used in CI, as the **read-only superset** of the +gates `os build` runs: same checks, no artifact. It wasn't. Four authoring lints +were wired into `compile.ts` only, and **two of them already fail the build**: + +| Lint | Emits `error` | `os build` | `os validate` (before) | +|---|---|---|---| +| `lintAutonumberFormats` | yes | ✓ | — | +| `lintViewRefs` (#2554) | yes | ✓ | — | +| `lintFlowPatterns` (#1874) | not yet | ✓ | — | +| `lintLivenessProperties` | no | ✓ | — | + +So an autonumber format naming a field that doesn't exist, or a form action +target naming a LIST view, passed `os validate` cleanly and then failed +`os build`. Reproduced verbatim on `main` against `examples/app-todo`: +`os validate` → "✓ Validation passed"; `os build` → "✗ Autonumber format +validation failed". Worst for the CI setups that gate on `validate` and only +discover the break at deploy time. + +The drift was invisible for a structural reason worth naming: every *other* gate +on both commands is a shared `@objectstack/lint` import, while these four are +CLI-local `../utils/lint-*` modules that only `compile.ts` ever imported. Nothing +made adding a gate to the build also add it to validate. + +**The fix is two parts.** `validate.ts` now runs all four, mirroring +`compile.ts`'s per-lint severity handling (`error` → exit 1, everything else → +advisory, and into the `warnings` array under `--json`). And a new source-level +test asserts that every `lintFoo(`/`validateFoo(` call site in `compile.ts` also +appears in `validate.ts`, failing with the list of missing gates. That test is +the actual fix for the class of bug — the wiring is just today's instance. + +**What you may newly see.** `os validate` now surfaces every rule these lints +carry, including the advisory ones, so existing projects can see new warnings. +Only `autonumber-*` and view-reference `error` findings change the exit code — +and any project they now fail was already failing `os build`. + +`FlowLintFinding` also gains an optional `severity`, honoured by both surfaces. +No rule sets it today, so flow findings stay advisory; it is the seam that lets +#3760's blocking `flow-runas-unscoped` gate `os validate` and `os build` +together the moment it lands, with no further wiring. diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index b86d1c24a8..187789bc89 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -223,11 +223,17 @@ another package defines. | 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 | ✓ | ✓ | | 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. Both entry points +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 JSX/React page sources (ADR-0080/0081) — that catch UI @@ -254,6 +260,7 @@ A clean run walks each gate and reports timing: → 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)... ✓ Validation passed (64ms) diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index a0c75ba440..5ae4bb5dde 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -25,6 +25,10 @@ import { validateSecurityPosture, validateOrgAxisRedLines } from '@objectstack/l import { validateFlowTriggerReadiness } from '@objectstack/lint'; import { validateFlowTemplatePaths } from '@objectstack/lint'; import { validateReadonlyFlowWrites } from '@objectstack/lint'; +import { lintFlowPatterns } from '../utils/lint-flow-patterns.js'; +import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js'; +import { lintLivenessProperties } from '../utils/lint-liveness-properties.js'; +import { lintViewRefs } from '../utils/lint-view-refs.js'; import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js'; import { printHeader, @@ -563,6 +567,73 @@ export default class Validate extends Command { } } + // 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'); + + const authoringLintErrors = [...flowLintErrors, ...autonumberErrors, ...viewRefErrors]; + const authoringLintWarnings = [ + ...flowLintWarnings, + ...livenessLint, + ...autonumberWarnings, + ...viewRefWarnings, + ]; + if (authoringLintErrors.length > 0) { + if (flags.json) { + console.log(JSON.stringify({ + valid: false, + errors: authoringLintErrors, + duration: timer.elapsed(), + }, null, 2)); + 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}`)); + } + } + // 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, @@ -652,7 +723,7 @@ export default class Validate extends Command { valid: true, manifest: config.manifest, stats, - warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...flowTemplateWarnings, ...readonlyWriteWarnings, ...securityAdvisories, ...capProviderWarnings], + warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...flowTemplateWarnings, ...readonlyWriteWarnings, ...authoringLintWarnings, ...securityAdvisories, ...capProviderWarnings], conversions: conversionNotices, specVersionGap: specGap, duration: timer.elapsed(), diff --git a/packages/cli/src/utils/lint-flow-patterns.ts b/packages/cli/src/utils/lint-flow-patterns.ts index ffef3821f7..dffb470afd 100644 --- a/packages/cli/src/utils/lint-flow-patterns.ts +++ b/packages/cli/src/utils/lint-flow-patterns.ts @@ -32,6 +32,10 @@ export interface FlowLintFinding { * advisory. A rule is only promoted to `'error'` when the shape it flags is a * *guaranteed* runtime failure — then a warning would just be a slower way of * finding out (#3760). + * + * `os build` and `os validate` both filter on this field, so promoting a rule + * gates both surfaces at once — neither can report clean while the other + * rejects the same stack (#3782). */ severity?: 'error' | 'warning'; } diff --git a/packages/cli/test/validate-build-gate-parity.test.ts b/packages/cli/test/validate-build-gate-parity.test.ts new file mode 100644 index 0000000000..5df516e042 --- /dev/null +++ b/packages/cli/test/validate-build-gate-parity.test.ts @@ -0,0 +1,80 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'node:fs'; +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. + * + * 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. + * + * 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. + */ + +const COMMANDS_DIR = join(__dirname, '..', 'src', 'commands'); + +/** + * 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. + */ +const BUILD_ONLY_GATES: readonly string[] = []; + +/** 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) ?? []; + return new Set(calls); +} + +describe('os validate is the read-only superset of os build (#3782)', () => { + it('runs every gate compile.ts runs', () => { + 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); + + const missing = [...compileGates] + .filter((g) => !validateGates.has(g)) + .filter((g) => !BUILD_ONLY_GATES.includes(g)) + .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.`, + ).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); + } + }); +});