Skip to content

Commit ce09d4e

Browse files
os-zhuangclaude
andauthored
fix(cli): os validate runs the four authoring lints os build runs (#3782) (#3789)
`os validate` is documented, and used in CI, as the read-only superset of the gates `os build` runs. It wasn't: four authoring lints were wired into `compile.ts` only, and two of them already emit `severity: 'error'` (`lintAutonumberFormats`, `lintViewRefs`). So an autonumber format naming a nonexistent field passed `os validate` and then failed `os build` — the exact trap this command exists to prevent, worst for CI gating on validate. The drift was structural: every other gate on both commands is a shared `@objectstack/lint` import, while these four are CLI-local `../utils/lint-*` modules only `compile.ts` imported. Nothing made adding a gate to the build also add it to validate. - validate.ts runs all four, mirroring compile.ts's per-lint severity handling (error -> exit 1, else advisory + into the --json warnings array). - New source-level test asserts every lint*/validate* call site in compile.ts also appears in validate.ts, listing any that are missing. This is the fix for the class of bug; the wiring is today's instance. - FlowLintFinding 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 both surfaces at once when it lands. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9e4bad5 commit ce09d4e

5 files changed

Lines changed: 209 additions & 2 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(cli): `os validate` runs the four authoring lints `os build` runs — "validate clean, build fails" is gone (#3782)
6+
7+
`os validate` is documented, and used in CI, as the **read-only superset** of the
8+
gates `os build` runs: same checks, no artifact. It wasn't. Four authoring lints
9+
were wired into `compile.ts` only, and **two of them already fail the build**:
10+
11+
| Lint | Emits `error` | `os build` | `os validate` (before) |
12+
|---|---|---|---|
13+
| `lintAutonumberFormats` | yes |||
14+
| `lintViewRefs` (#2554) | yes |||
15+
| `lintFlowPatterns` (#1874) | not yet |||
16+
| `lintLivenessProperties` | no |||
17+
18+
So an autonumber format naming a field that doesn't exist, or a form action
19+
target naming a LIST view, passed `os validate` cleanly and then failed
20+
`os build`. Reproduced verbatim on `main` against `examples/app-todo`:
21+
`os validate` → "✓ Validation passed"; `os build` → "✗ Autonumber format
22+
validation failed". Worst for the CI setups that gate on `validate` and only
23+
discover the break at deploy time.
24+
25+
The drift was invisible for a structural reason worth naming: every *other* gate
26+
on both commands is a shared `@objectstack/lint` import, while these four are
27+
CLI-local `../utils/lint-*` modules that only `compile.ts` ever imported. Nothing
28+
made adding a gate to the build also add it to validate.
29+
30+
**The fix is two parts.** `validate.ts` now runs all four, mirroring
31+
`compile.ts`'s per-lint severity handling (`error` → exit 1, everything else →
32+
advisory, and into the `warnings` array under `--json`). And a new source-level
33+
test asserts that every `lintFoo(`/`validateFoo(` call site in `compile.ts` also
34+
appears in `validate.ts`, failing with the list of missing gates. That test is
35+
the actual fix for the class of bug — the wiring is just today's instance.
36+
37+
**What you may newly see.** `os validate` now surfaces every rule these lints
38+
carry, including the advisory ones, so existing projects can see new warnings.
39+
Only `autonumber-*` and view-reference `error` findings change the exit code —
40+
and any project they now fail was already failing `os build`.
41+
42+
`FlowLintFinding` also gains an optional `severity`, honoured by both surfaces.
43+
No rule sets it today, so flow findings stay advisory; it is the seam that lets
44+
#3760's blocking `flow-runas-unscoped` gate `os validate` and `os build`
45+
together the moment it lands, with no further wiring.

content/docs/deployment/validating-metadata.mdx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,11 +223,17 @@ another package defines.
223223
| Chart bindings outside dashboards (#3583) |||
224224
| Navigation vs. granted access (ADR-0090 D6) |||
225225
| Security posture (ADR-0090 — e.g. every custom object declares `sharingModel`) |||
226+
| Autonumber `{field}` interpolation |||
227+
| View references — form targets, view-key collisions (#2554) |||
228+
| Flow authoring anti-patterns (#1874) |||
229+
| Liveness author-warnings |||
226230
| Emits `dist/objectstack.json` |||
227231

228232
So `os validate` is the fast inner-loop check (no artifact); `os build` is what
229233
you run when you need the deployable artifact. A config that passes `os validate`
230-
will not fail `os build` on schema/predicate/binding grounds. Both entry points
234+
will not fail `os build` on schema/predicate/binding grounds — a test in the CLI
235+
asserts that every gate `os build` runs is also run by `os validate`, so the two
236+
cannot drift apart again (#3782). Both entry points
231237
also check SDUI styling (ADR-0065), and `os validate` additionally runs a set of
232238
view- and page-shape checks — list-view navigation modes (ADR-0053), view
233239
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:
254260
→ Checking source-page styling (ADR-0065)...
255261
→ Checking capability references (ADR-0066)...
256262
→ Checking flow trigger wiring...
263+
→ Running authoring lints (#3782)...
257264
→ Checking security posture (ADR-0090 D7)...
258265
259266
✓ Validation passed (64ms)

packages/cli/src/commands/validate.ts

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ import { validateSecurityPosture, validateOrgAxisRedLines } from '@objectstack/l
2525
import { validateFlowTriggerReadiness } from '@objectstack/lint';
2626
import { validateFlowTemplatePaths } from '@objectstack/lint';
2727
import { validateReadonlyFlowWrites } from '@objectstack/lint';
28+
import { lintFlowPatterns } from '../utils/lint-flow-patterns.js';
29+
import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js';
30+
import { lintLivenessProperties } from '../utils/lint-liveness-properties.js';
31+
import { lintViewRefs } from '../utils/lint-view-refs.js';
2832
import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
2933
import {
3034
printHeader,
@@ -563,6 +567,73 @@ export default class Validate extends Command {
563567
}
564568
}
565569

570+
// 3e3. [#3782] The four authoring lints that live in the CLI itself rather
571+
// than in `@objectstack/lint`. Every other gate on this command is a
572+
// `@objectstack/lint` import, so these four were only ever reachable
573+
// from `compile.ts` — `os build` ran them, `os validate` did not, and
574+
// the drift went unnoticed while all of their findings were advisory.
575+
// Two of them already GATE the build (`lintAutonumberFormats`,
576+
// `lintViewRefs` emit `severity: 'error'`), which made this command
577+
// report a clean stack that `os build` then rejected — exactly the
578+
// contract this command exists to uphold. Severity handling mirrors
579+
// `compile.ts` per lint, so the two surfaces agree by construction.
580+
if (!flags.json) printStep('Running authoring lints (#3782)...');
581+
582+
// Flow authoring anti-patterns (#1874). Advisory today; `severity: 'error'`
583+
// is honoured so a blocking rule (#3760's `flow-runas-unscoped`) gates here
584+
// the moment it gates the build, with no further wiring.
585+
const flowLint = lintFlowPatterns(result.data as Record<string, unknown>);
586+
const flowLintErrors = flowLint.filter((f) => f.severity === 'error');
587+
const flowLintWarnings = flowLint.filter((f) => f.severity !== 'error');
588+
589+
// Liveness author-warnings — an authored property the ledger marks
590+
// dead-and-misleading or experimental. Advisory only, never fatal.
591+
const livenessLint = lintLivenessProperties(result.data as Record<string, unknown>);
592+
593+
// Autonumber `{field}` interpolation — an unknown field is broken (error);
594+
// an optional one is fragile (warning).
595+
const autonumberLint = lintAutonumberFormats(result.data as Record<string, unknown>);
596+
const autonumberErrors = autonumberLint.filter((f) => f.severity === 'error');
597+
const autonumberWarnings = autonumberLint.filter((f) => f.severity !== 'error');
598+
599+
// View references (#2554) — a form action target naming a missing or LIST
600+
// view, and list/form view-key collisions. Both are broken → error.
601+
const viewRefLint = lintViewRefs(result.data as Record<string, unknown>);
602+
const viewRefErrors = viewRefLint.filter((f) => f.severity === 'error');
603+
const viewRefWarnings = viewRefLint.filter((f) => f.severity !== 'error');
604+
605+
const authoringLintErrors = [...flowLintErrors, ...autonumberErrors, ...viewRefErrors];
606+
const authoringLintWarnings = [
607+
...flowLintWarnings,
608+
...livenessLint,
609+
...autonumberWarnings,
610+
...viewRefWarnings,
611+
];
612+
if (authoringLintErrors.length > 0) {
613+
if (flags.json) {
614+
console.log(JSON.stringify({
615+
valid: false,
616+
errors: authoringLintErrors,
617+
duration: timer.elapsed(),
618+
}, null, 2));
619+
this.exit(1);
620+
}
621+
console.log('');
622+
printError(`Authoring lint failed (${authoringLintErrors.length} issue${authoringLintErrors.length > 1 ? 's' : ''})`);
623+
for (const f of authoringLintErrors.slice(0, 50)) {
624+
console.log(` • ${f.where}: ${f.message}`);
625+
console.log(chalk.dim(` ${f.hint}`));
626+
console.log(chalk.dim(` rule: ${f.rule}`));
627+
}
628+
this.exit(1);
629+
}
630+
if (!flags.json) {
631+
for (const f of authoringLintWarnings.slice(0, 50)) {
632+
console.log(chalk.yellow(` ⚠ ${f.where}: ${f.message}`));
633+
console.log(chalk.dim(` ${f.hint}`));
634+
}
635+
}
636+
566637
// 3f. [ADR-0090 D7] Security posture — the same gate `os compile`/`os build`
567638
// run. Without it here, `os validate` passed a stack (e.g. a custom
568639
// object with no explicit sharingModel) that the build then rejected,
@@ -652,7 +723,7 @@ export default class Validate extends Command {
652723
valid: true,
653724
manifest: config.manifest,
654725
stats,
655-
warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...flowTemplateWarnings, ...readonlyWriteWarnings, ...securityAdvisories, ...capProviderWarnings],
726+
warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...flowTemplateWarnings, ...readonlyWriteWarnings, ...authoringLintWarnings, ...securityAdvisories, ...capProviderWarnings],
656727
conversions: conversionNotices,
657728
specVersionGap: specGap,
658729
duration: timer.elapsed(),

packages/cli/src/utils/lint-flow-patterns.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ export interface FlowLintFinding {
3232
* advisory. A rule is only promoted to `'error'` when the shape it flags is a
3333
* *guaranteed* runtime failure — then a warning would just be a slower way of
3434
* finding out (#3760).
35+
*
36+
* `os build` and `os validate` both filter on this field, so promoting a rule
37+
* gates both surfaces at once — neither can report clean while the other
38+
* rejects the same stack (#3782).
3539
*/
3640
severity?: 'error' | 'warning';
3741
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, expect, it } from 'vitest';
4+
import { readFileSync } from 'node:fs';
5+
import { join } from 'node:path';
6+
7+
/**
8+
* `os validate` is documented — and relied on by CI setups — as the READ-ONLY
9+
* SUPERSET of the gates `os build` runs: same checks, no artifact emitted. That
10+
* contract has no enforcement, so it drifted (#3782): four authoring lints
11+
* (`lintFlowPatterns`, `lintLivenessProperties`, `lintAutonumberFormats`,
12+
* `lintViewRefs`) were wired into `compile.ts` only. Two of them already emitted
13+
* `severity: 'error'`, so `os validate` reported a clean stack that `os build`
14+
* then rejected — the precise failure the contract exists to prevent.
15+
*
16+
* The drift was invisible because every OTHER gate is a `@objectstack/lint`
17+
* import shared by both files, while these four are CLI-local `../utils/lint-*`
18+
* modules that only `compile.ts` ever imported.
19+
*
20+
* This is a source-level gate rather than a behavioural one on purpose: it fails
21+
* when a gate is ADDED to the build without being added to validate, which is
22+
* the moment the mistake is cheap to fix — not later, when some app trips it.
23+
*/
24+
25+
const COMMANDS_DIR = join(__dirname, '..', 'src', 'commands');
26+
27+
/**
28+
* Gates `os build` may legitimately run that `os validate` does not.
29+
*
30+
* Adding an entry here is a deliberate assertion that the check CANNOT be made
31+
* read-only (it needs the emitted artifact, the bundler, the filesystem output).
32+
* A gate that merely *reads* the parsed stack does not belong here — wire it
33+
* into `validate.ts` instead. Empty today, and that is the healthy state.
34+
*/
35+
const BUILD_ONLY_GATES: readonly string[] = [];
36+
37+
/** Every `lintFoo(`/`validateFoo(` call site in a command's source. */
38+
function gateCallsIn(file: string): Set<string> {
39+
const src = readFileSync(join(COMMANDS_DIR, file), 'utf8');
40+
const calls = src.match(/\b(?:lint|validate)[A-Z]\w*(?=\s*\()/g) ?? [];
41+
return new Set(calls);
42+
}
43+
44+
describe('os validate is the read-only superset of os build (#3782)', () => {
45+
it('runs every gate compile.ts runs', () => {
46+
const compileGates = gateCallsIn('compile.ts');
47+
const validateGates = gateCallsIn('validate.ts');
48+
49+
// Guard the guard: if the extraction regex silently stops matching, the
50+
// set-difference below passes vacuously and the gate goes quietly dead.
51+
expect(compileGates.size).toBeGreaterThan(10);
52+
53+
const missing = [...compileGates]
54+
.filter((g) => !validateGates.has(g))
55+
.filter((g) => !BUILD_ONLY_GATES.includes(g))
56+
.sort();
57+
58+
expect(
59+
missing,
60+
`os build runs ${missing.length} gate(s) that os validate does not, so a stack ` +
61+
`can pass 'os validate' and fail 'os build': ${missing.join(', ')}.\n` +
62+
`Wire each into packages/cli/src/commands/validate.ts (mirroring the severity ` +
63+
`handling in compile.ts), or — only if it genuinely cannot run without emitting ` +
64+
`an artifact — add it to BUILD_ONLY_GATES in this file with a reason.`,
65+
).toEqual([]);
66+
});
67+
68+
it('runs the four CLI-local authoring lints that regressed in #3782', () => {
69+
const validateGates = gateCallsIn('validate.ts');
70+
71+
for (const gate of [
72+
'lintFlowPatterns',
73+
'lintLivenessProperties',
74+
'lintAutonumberFormats',
75+
'lintViewRefs',
76+
]) {
77+
expect(validateGates.has(gate), `validate.ts must call ${gate}`).toBe(true);
78+
}
79+
});
80+
});

0 commit comments

Comments
 (0)