Skip to content

Commit edaf3b2

Browse files
os-justinclaude
andauthored
fix(cli): os validate and os lint judge the ADR-0130 D4 union-folded stack (#17524)
* fix(cli): os validate and os lint judge the ADR-0130 D4 union-folded stack A project whose definitions live only in `packages[]` — the ADR-0130 D4 artifact shape — was judged by both commands as if it declared nothing: the input they handed the author-time rule table was an empty stack, so every rule reported nothing and both exited 0. `os build` folds the packages back in via `authoringRuleUnionStack` before running the same table and refuses the same stack. Both call sites now hand the rule table the stack that helper returns — the one fold `compile.ts` already calls, not a second one. A stack that still carries its collections comes back by identity, so single-package projects are unaffected by construction. Rule INPUT only: neither command's output, `--json` payload nor `scoreMetadata` sees the fold. Measured through the real binaries on the card's repro: before os validate 0 · os lint 0 (no finding) · os build 1 after os validate 1 · os lint 1 · os build 1, all three object-reference-unknown at objects[0].fields.ghost.reference Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt Co-authored-by: Claude <noreply@anthropic.com> * test(cli): pin that all three authoring commands judge the option-B stack Behavioural half — `test/union-fold-command-parity.test.ts` drives the card's repro through the three real binaries and asserts all three exit 1 naming `object-reference-unknown` at one path, plus the clean control that keeps the case from passing on a command that simply refuses every `packages[]` project. Source-level half — one more `it()` in the existing gate-parity file, over the same AUTHORING_COMMANDS list the #12297 lesson put there: every door must hand both rule tiers a `authoringRuleUnionStack(...)` stack. Carries a positive control on the helper so a rename fails loudly rather than turning the guard vacuous. Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt Co-authored-by: Claude <noreply@anthropic.com> * chore(changeset): patch @objectstack/cli for the union-fold wiring Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent efa2533 commit edaf3b2

5 files changed

Lines changed: 281 additions & 4 deletions

File tree

.changeset/spotty-jars-shave.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
`os validate` and `os lint` now judge the same stack `os build` judges when a project declares its metadata only in `packages[]`.
6+
7+
A project in the ADR-0130 D4 artifact shape — every definition inside `packages[]`, no collections at the top level — was handed to the author-time rule table as an **empty stack** by both commands, so all 44 rules reported nothing and both exited 0 having read none of the project. `os build` folds the packages back in first (`authoringRuleUnionStack`) and refuses the same stack. Two of the three authoring gates were certifying an unread project as clean, and `os validate` is the check an author runs before shipping.
8+
9+
Both commands now hand the rule table the stack that same helper returns — one fold, shared with `os build`, not a second implementation. It is a rule **input** only: neither command's output, `--json` payload nor `os lint`'s metadata score changes, and a stack that still carries its top-level collections is returned by identity, so single-package projects are unaffected by construction.
10+
11+
⚠️ **A project that was silently passing may now fail.** That is the defect surfacing, not a new rule: the finding was always there and `os build` was always reporting it. Run `os build` on the same tree to see the identical diagnostic.

packages/cli/src/commands/lint.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { collectAndLintDocs } from '../utils/collect-docs.js';
1414
import { scoreMetadata } from '../lint/score.js';
1515
import { checkHookBodyLowering } from '../lint/hook-body-lowering.js';
1616
import { lowerCallables } from '../utils/lower-callables.js';
17+
import { authoringRuleUnionStack } from '../utils/stack-collections.js';
1718
import { runMetadataEval } from '../lint/metadata-eval.js';
1819
import { DEFAULT_METADATA_EVAL_CORPUS } from '../lint/corpus.js';
1920
import {
@@ -511,10 +512,25 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
511512
// the family stays silent on it and `checkHookBodyLowering` is what
512513
// reports it, so no verdict is ever given about a body that was not
513514
// produced. Nothing here touches what `os build` accepts (#13838).
515+
//
516+
// ── Both tiers are handed the UNION-FOLDED stack (ADR-0130 D4, #17069) ──
517+
// Under option B every definition lives in `packages[]` and the top level
518+
// carries none, so both tiers above were handed an EMPTY stack: the whole
519+
// table reported nothing and `os lint` returned no finding of any severity
520+
// for a project `os build` refuses. `authoringRuleUnionStack` is the one
521+
// helper `compile.ts` calls for its union run — the same fold, not a second
522+
// one — and it fills only the collections the top level does not carry, so
523+
// a stack that still carries them comes back BY IDENTITY and every
524+
// single-package project lints exactly as before.
525+
//
526+
// Scoped to this call, as it is in `compile.ts`: the hand-written checks
527+
// above and `scoreMetadata` (which reaches `lintConfig` through
528+
// `lint/score.js`) keep reading the caller's own stack, so nothing about
529+
// what this function returns for a top-level stack moves.
514530
const { lowered, loweredHookRefs } = lowerCallables(config as Record<string, unknown>);
515531
for (const f of runAuthoringRules('lint', {
516-
normalized: config,
517-
parsed: lowered,
532+
normalized: authoringRuleUnionStack(config as Record<string, unknown>),
533+
parsed: authoringRuleUnionStack(lowered),
518534
sduiManifest: opts.sduiManifest,
519535
// [#16546] Same ref set `os build` computes from the same normalized
520536
// input — what lets `validateReadonlyHookWrites` / `validateHookBodyWrites`

packages/cli/src/commands/validate.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from '@objectstack/spec';
1515
import { loadConfig } from '../utils/config.js';
1616
import { lowerCallables } from '../utils/lower-callables.js';
17+
import { authoringRuleUnionStack } from '../utils/stack-collections.js';
1718
import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '@objectstack/lint';
1819
import { resolveSduiManifest } from '../utils/sdui-manifest.js';
1920
import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
@@ -300,9 +301,25 @@ export default class Validate extends Command {
300301
// is declared in `lint/authoring-rules.ts`. Do not add a call site here.
301302
const registered = authoringRulesFor('validate');
302303
if (!flags.json) printStep(`Running author-time rules (${registered.length})...`);
304+
// [ADR-0130 D4 / option B, #17069] Judged on the SAME folded stack
305+
// `os build` judges (`compile.ts` step 3b), through the one helper
306+
// both doors call. Under option B every definition lives in
307+
// `packages[]` and the top level carries none, so this run's input
308+
// was an EMPTY stack: `os validate` printed `✓ Validation passed` and
309+
// exited 0 having judged nothing, on a stack `os build` refuses. That
310+
// is the weakest-gate class #4409 was filed for, and the direction it
311+
// arrived in here is the worst one — the fast inner-loop check is
312+
// what an author runs BEFORE shipping, so its clean bill of health is
313+
// the strongest false assurance the three commands can give.
314+
//
315+
// Rule INPUT only, exactly as in `compile.ts`: this command emits no
316+
// artifact at all, and the folded stack reaches neither the metadata
317+
// stats below nor the `--json` payload. A stack that still carries
318+
// its collections is returned by identity, so every single-package
319+
// project is unaffected by construction.
303320
const findings = runAuthoringRules('validate', {
304-
normalized: normalized as Record<string, unknown>,
305-
parsed: result.data as Record<string, unknown>,
321+
normalized: authoringRuleUnionStack(normalized as Record<string, unknown>),
322+
parsed: authoringRuleUnionStack(result.data as Record<string, unknown>),
306323
sduiManifest: resolveSduiManifest(),
307324
// [#16546] Same ref set `os build` / `os lint` compute — keeps this
308325
// door's hook write-set findings at the same `path` as the other two.
Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #17069 — the three authoring commands judge the SAME STACK, on the ADR-0130
5+
* D4 / option-B shape.
6+
*
7+
* `authoring-rule-command-parity.test.ts` next door proves the three commands
8+
* reach the same verdict about the same rule TABLE. This file proves the other
9+
* half of the same promise, one layer earlier: that they hand that table the
10+
* same INPUT. A command can run every rule in the registry and still certify a
11+
* project clean — if what it hands the registry is an empty stack.
12+
*
13+
* ## The defect this file pins
14+
*
15+
* A project whose definitions live only in `packages[]` — the ADR-0130 D4
16+
* artifact shape, no collections at the top level — was judged by `os validate`
17+
* and `os lint` as if it declared NOTHING. `compile.ts` folds the packages back
18+
* in through `authoringRuleUnionStack` before it runs the table; neither
19+
* sibling command imported that helper, so each ran 44 rules over `{}`.
20+
*
21+
* Measured through the real binaries on this card's repro, before the fix:
22+
*
23+
* os validate EXIT=0 ✓ Validation passed
24+
* os lint EXIT=0 (no finding of any severity)
25+
* os build EXIT=1 object-reference-unknown
26+
*
27+
* ⭐ The dangling `ob_nowhere` is only the PROBE that makes the blindness
28+
* visible. The same silence covered every author-time rule, because the input
29+
* was empty — which is why the fix is the shared fold and not a rule.
30+
*
31+
* ## Why this is the p1 direction of the #4409 class
32+
*
33+
* #4409 was `os build` publishing what the other two refuse. This is the
34+
* mirror, and it is worse: `os validate` is the fast inner-loop check an author
35+
* runs BEFORE shipping, so its clean bill of health on a stack it read nothing
36+
* of is the strongest false assurance the three commands can give.
37+
* `content/docs/deployment/validating-metadata.mdx` states the parity as a
38+
* promise — *"anything that can fail a build fails `os lint` too"* — and on
39+
* this stack shape the promise was false in the loudest direction.
40+
*
41+
* ## Why it is spawned, and why BOTH fixtures are here
42+
*
43+
* Spawned because the exit CODE is the contract a CI pipeline reads, and only a
44+
* real process produces one; `os validate`'s rule run is an expression inside
45+
* the oclif command body, so there is no exported seam a probe could call
46+
* instead (the same reason the option-B acceptance pin could not reach it).
47+
*
48+
* The CLEAN fixture is not decoration. A fold wired in backwards — or a rule
49+
* that fires on any `packages[]` at all — would satisfy the failing case alone.
50+
* The pair asserts what the card actually claims: that the option-B stack is
51+
* READ, not that it is rejected.
52+
*/
53+
54+
import { describe, expect, it } from 'vitest';
55+
import { execFileSync } from 'node:child_process';
56+
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
57+
import { tmpdir } from 'node:os';
58+
import { join, resolve } from 'node:path';
59+
import { fileURLToPath } from 'node:url';
60+
import { childEnv } from './helpers/serve-process.js';
61+
62+
const HERE = resolve(fileURLToPath(import.meta.url), '..');
63+
const CLI = resolve(HERE, '../bin/run-dev.js');
64+
65+
/** The three commands the #4409 registry holds to one bar. */
66+
const AUTHORING_COMMANDS = ['validate', 'lint', 'build'] as const;
67+
68+
/** The rule the probe trips, and the path it reports it at. */
69+
const RULE = 'object-reference-unknown';
70+
const RULE_PATH = 'objects[0].fields.ghost.reference';
71+
72+
/**
73+
* The card's repro verbatim: no top-level `objects`, one `packages[]` entry
74+
* carrying an object whose `ghost` lookup points at an object that does not
75+
* exist. Every collection this project declares lives inside `packages[]`.
76+
*/
77+
const optionBStack = (reference: string): Record<string, unknown> => ({
78+
manifest: { id: 'com.example.ob', name: 'ob', version: '1.0.0', type: 'app', namespace: 'ob' },
79+
packages: [
80+
{
81+
manifest: {
82+
id: 'com.example.ob',
83+
name: 'ob',
84+
version: '1.0.0',
85+
type: 'app',
86+
namespace: 'ob',
87+
objects: [
88+
{
89+
name: 'ob_order',
90+
label: 'Order',
91+
sharingModel: 'private',
92+
fields: {
93+
number: { type: 'text', label: 'Number' },
94+
ghost: { type: 'lookup', label: 'Ghost', reference },
95+
},
96+
},
97+
],
98+
},
99+
},
100+
],
101+
});
102+
103+
interface Run {
104+
code: number;
105+
output: string;
106+
}
107+
108+
/**
109+
* One authoring command over one option-B project, as a shell sees it.
110+
*
111+
* A plain literal config with no imports, so it resolves with no `node_modules`
112+
* next to it — the `authoring-rule-command-parity.test.ts` pattern.
113+
*/
114+
function runCommand(command: string, stack: Record<string, unknown>): Run {
115+
const dir = mkdtempSync(join(tmpdir(), 'os-union-fold-'));
116+
try {
117+
writeFileSync(join(dir, 'objectstack.config.mjs'), `export default ${JSON.stringify(stack, null, 2)};\n`);
118+
try {
119+
const stdout = execFileSync(process.execPath, [CLI, command], {
120+
cwd: dir,
121+
encoding: 'utf8',
122+
stdio: 'pipe',
123+
// Every spawned child under this directory declares its environment at
124+
// the call site (#11595).
125+
env: childEnv({ NO_COLOR: '1' }),
126+
});
127+
return { code: 0, output: String(stdout) };
128+
} catch (error: any) {
129+
return { code: error.status ?? 1, output: `${error.stdout ?? ''}${error.stderr ?? ''}` };
130+
}
131+
} finally {
132+
rmSync(dir, { recursive: true, force: true });
133+
}
134+
}
135+
136+
describe('#17069 — os validate and os lint judge the option-B stack, not an empty one', () => {
137+
it.each(AUTHORING_COMMANDS)(
138+
'os %s refuses a packages[]-only project whose lookup target does not exist',
139+
(command) => {
140+
const run = runCommand(command, optionBStack('ob_nowhere'));
141+
expect(
142+
run.code,
143+
`os ${command} exited ${run.code} on a project whose definitions live only in packages[]. ` +
144+
`Before #17069 os validate and os lint both exited 0 here — they handed the author-time ` +
145+
`rule table an EMPTY stack, so all 44 rules reported nothing and the project was certified ` +
146+
`clean without being read. Hand the table authoringRuleUnionStack(...) as compile.ts does.` +
147+
`\n--- output ---\n${run.output}`,
148+
).toBe(1);
149+
expect(run.output, `os ${command} refused the stack without naming the rule`).toContain(RULE);
150+
expect(
151+
run.output,
152+
`os ${command} named ${RULE} at a different path than the other doors — the three commands ` +
153+
`must report one finding one way`,
154+
).toContain(RULE_PATH);
155+
},
156+
180_000,
157+
);
158+
159+
it.each(AUTHORING_COMMANDS)(
160+
'control: os %s passes the identical option-B project once the lookup resolves',
161+
(command) => {
162+
const run = runCommand(command, optionBStack('ob_order'));
163+
expect(
164+
run.code,
165+
`os ${command} exited ${run.code} on a CLEAN option-B project. The fold folds packages[] ` +
166+
`back in as the rule table's INPUT — it does not make a packages[]-only project fail. ` +
167+
`Without this control the case above would pass on a command that refuses every ` +
168+
`multi-package stack.\n--- output ---\n${run.output}`,
169+
).toBe(0);
170+
expect(run.output, `os ${command} reported ${RULE} on a stack whose lookup resolves`).not.toContain(RULE);
171+
},
172+
180_000,
173+
);
174+
});

packages/cli/test/validate-build-gate-parity.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,13 +112,72 @@ function gateCallsIn(file: string): Set<string> {
112112
/** Is `name` invoked anywhere in this command's source? */
113113
const calls = (file: string, name: string) => new RegExp(String.raw`\b${name}\s*\(`).test(sourceOf(file));
114114

115+
/**
116+
* The `runAuthoringRules(...)` call in one command's source, from the call
117+
* through to its closing brace — the object literal whose `normalized` and
118+
* `parsed` members ARE the stack the rule table judges.
119+
*/
120+
function ruleTableCallIn(file: string): string {
121+
const src = sourceOf(file);
122+
const at = src.indexOf('runAuthoringRules(');
123+
return at === -1 ? '' : src.slice(at, src.indexOf('})', at));
124+
}
125+
115126
describe('os validate is the read-only superset of os build (#3782, #4409)', () => {
116127
it('both commands run the shared authoring-rule registry', () => {
117128
for (const file of ['compile.ts', 'validate.ts']) {
118129
expect(calls(file, 'runAuthoringRules'), `${file} must run the authoring-rule registry`).toBe(true);
119130
}
120131
});
121132

133+
/**
134+
* The same drift one layer EARLIER than every check in this file: not "does
135+
* this command run the table" but "what stack does it hand the table".
136+
*
137+
* ⭐ [#17069] A project whose definitions live only in `packages[]` — the
138+
* ADR-0130 D4 / option-B shape — carries no collections at the top level.
139+
* `compile.ts` folds them back in with `authoringRuleUnionStack` before it
140+
* runs the table; `validate.ts` and `lint.ts` did not, so each ran all 44
141+
* rules over an EMPTY stack and reported a clean bill of health for a project
142+
* it had read nothing of, at exit 0. That is the #4409 weakest-gate class
143+
* arriving through the INPUT rather than the rule set — and in its worst
144+
* direction, because `os validate` is the check an author runs before
145+
* shipping.
146+
*
147+
* Source-level for the same reason as the gate check above: it fails when a
148+
* door drops the fold, which is the moment it is cheap to fix. The
149+
* behavioural half — the card's own repro through the three real binaries —
150+
* is `test/union-fold-command-parity.test.ts`.
151+
*/
152+
it('all three authoring commands hand the rule table the union-folded stack', () => {
153+
// Positive control FIRST: the helper must still exist and still be the ONE
154+
// fold. Without it, deleting `authoringRuleUnionStack` outright would
155+
// satisfy nothing below — but renaming it would make every assertion here
156+
// fail for the wrong reason, and this line says which.
157+
const helper = readFileSync(join(UTILS_DIR, 'stack-collections.ts'), 'utf8');
158+
expect(
159+
/export function authoringRuleUnionStack\b/.test(helper),
160+
'src/utils/stack-collections.ts must export authoringRuleUnionStack — if it moved, move this ' +
161+
'guard with it. ⛔ Do not answer a red here by writing a second fold.',
162+
).toBe(true);
163+
164+
for (const file of AUTHORING_COMMANDS) {
165+
const call = ruleTableCallIn(file);
166+
expect(call, `${file} must call runAuthoringRules`).not.toBe('');
167+
for (const tier of ['normalized', 'parsed']) {
168+
expect(
169+
new RegExp(String.raw`\b${tier}\s*:\s*authoringRuleUnionStack\s*\(`).test(call),
170+
`${file} hands the authoring-rule table a '${tier}' stack that has NOT been through ` +
171+
`authoringRuleUnionStack(). On an ADR-0130 D4 / option-B project — every definition in ` +
172+
`packages[], none at the top level — that input is an EMPTY stack, so every rule in the ` +
173+
`table reports nothing and this command certifies an unread project as clean at exit 0. ` +
174+
`Wrap it, as compile.ts does. ⛔ Do not reimplement the fold here — import the one in ` +
175+
`src/utils/stack-collections.ts.`,
176+
).toBe(true);
177+
}
178+
}
179+
});
180+
122181
it.each(SHARED_NON_REGISTRY_GATES)('both commands run %s', (gate) => {
123182
// Guard the guard: a gate that has been renamed or deleted must fail here
124183
// rather than pass vacuously on both sides.

0 commit comments

Comments
 (0)