Skip to content

Commit d114d5e

Browse files
os-zhuangclaude
os-zhuang
and
claude
authored
feat(cli): carry the computed advisory lists on every os build --json failure exit (#12079)
* feat(cli): carry the computed advisory lists on every `os build --json` failure exit (#11772) `warnings` lived on the terminal success payload only (plus `ruleAdvisories` alone on the author-time-rules failure), while the text face prints its advisory blocks — the #11529 author-time advisories at 3b and the #3786 undeclared authoring-key findings at 3d — several gates earlier, each ending in `— re-run with --json for the full list`. A build that then failed at a later gate emitted that gate's failure payload, and none of those carried the list: the remedy the notice named returned a payload without the withheld entries in it (the #11643 / #11391 "the remedy named is unreachable" shape). Maintainer ruling 2026-08-25, option 1 of three: every `emitJson` failure exit carries the lists the run has ALREADY COMPUTED, so `warnings` means the same thing on every exit. Option 2 (shape depends on how far the run got) and option 3 (weaken the pointer) were both rejected. Nine failure exits, three more than the filing card's table listed — it missed the protocol-parse exit, the `--no-runtime-bundle` refusal and the bottom catch-all. Every one now reads a single `warningsSoFar()` site, which the success payload reads too, so the member order (`os validate --json`'s, minus its trailing `structuralWarnings`) cannot drift between exits. Carrying, not computing: each list stays computed at the step that owns it, so an exit upstream of a step reports that list empty rather than paying for a computation it had not already done. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR * fix(cli): bind the hoisted advisory list to splitBySeverity's return type (#11772) `Test Core (2/6)` failed on `packages/lint/src/authoring-rule-wiring.test.ts:291` -- "os build imports no unratcheted symbol from @objectstack/lint": AssertionError: compile.ts imports AuthoringFinding from @objectstack/lint directly. Register the rule in AUTHORING_RULES, or add the symbol to LINT_IMPORT_RATCHET with a reason. expected [ 'AuthoringFinding' ] to deeply equal [] WHY THIS CARD TRIPPED IT. Hoisting the four advisory lists out of the `try` so every `emitJson` failure exit can read `warningsSoFar()` turned one inferred binding into a declared one. Before the hoist the list arrived destructured -- `const { errors: ruleErrors, advisories: ruleAdvisories } = splitBySeverity(...)` -- and its type was inferred, so no name was imported. A `let` declared ahead of the assignment needs a written type, and the first spelling reached for the underlying finding type, adding `AuthoringFinding` to compile.ts's import list. The #4409 import scan reads that list and had never been told about the symbol. `import type` does not escape the scan, and that is deliberate: `lintImportsIn()` matches `/import\s+(?:type\s+)?\{([^}]*)\}\s*from\s*['"]@objectstack\/lint['"]/g` and then strips a per-name `type ` prefix, so both the statement-level and the inline modifier are seen. The guard is asking which SYMBOLS this command file names, not which of them survive to runtime. THE FIX. Bind the annotation to the function that produces the value: let ruleAdvisories: ReturnType<typeof splitBySeverity>['advisories'] = []; `splitBySeverity` is already an import this file makes and already carries a LINT_IMPORT_RATCHET entry ("Pure partition of a finding list into gating vs advisory. Carries no rule identity at all."), so no symbol is added and no exemption is widened. It resolves to exactly the same type the old annotation spelled -- `AuthoringFinding[]`, the function is sync so no `Awaited` is involved -- and `tsc --noEmit` is clean. WHY THIS IS NOT A GATE BYPASS. Nothing was added to LINT_IMPORT_RATCHET or to AUTHORING_RULES, no assertion in authoring-rule-wiring.test.ts was touched, and no test was skipped or relaxed. The ratchet's exemption set is byte-identical before and after; the import list it scans is what shrank. The annotation is strictly tighter than the one it replaces: it is now pinned to `splitBySeverity`'s declared shape, so if that function's `advisories` member ever changes type this binding follows it instead of silently disagreeing -- the same "one list cannot drift from itself" idiom this card applied to the member ORDER of `warningsSoFar()`, now applied to the list's TYPE. Behaviour is unchanged: type-only edit, no emitted JS differs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 71f9cd1 commit d114d5e

4 files changed

Lines changed: 700 additions & 28 deletions

File tree

‎.changeset/build-json-failure-payload-warnings.md‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
feat(cli): `os build --json` carries the computed advisory lists on every failure exit, not the success payload alone (#11772)
6+
7+
**Machine-contract widening on the `--json` failure payloads.** A consumer that
8+
today branches on `warnings` being ABSENT from an `os build --json` failure
9+
payload sees a different shape after this change.
10+
11+
## What was wrong
12+
13+
The text face prints its advisory blocks before the gates that can stop the
14+
run — the #11529 author-time advisories at step 3b, the #3786 undeclared
15+
authoring-key findings at 3d — and both end in `— re-run with --json for the
16+
full list`. But `warnings` lived on the TERMINAL SUCCESS payload only (plus,
17+
for `ruleAdvisories` alone, the author-time-rules failure). On a tree with 60
18+
undeclared authoring keys *and* a package-docs error:
19+
20+
```
21+
os build Undeclared authoring keys (60) … 50 rows …
22+
… and 10 more … — re-run with --json for the full list
23+
os build --json {"success":false,"error":"docs validation failed","issues":[…]}
24+
^ the 60 keys nowhere
25+
```
26+
27+
The remedy the notice named returned a payload that did not contain the list,
28+
and the author could not reach the withheld entries by any route until an
29+
unrelated later failure was fixed — the "the remedy named is unreachable"
30+
shape of #11643 and #11391.
31+
32+
## Which exits gain the field
33+
34+
All nine failure exits of `os build --json`. Six already had a payload of their
35+
own; three more were found while enumerating (the filing card's table listed
36+
six). `warnings` is now present on every one, alongside each exit's existing
37+
keys, which are unchanged:
38+
39+
| exit (step) | existing keys | `warnings` before | after |
40+
| --- | --- | --- | --- |
41+
| `strict-body: missing body` (2b) | `issues` | absent | `[]` |
42+
| protocol parse failure (3) | `errors` | absent | `[]` |
43+
| `author-time rules failed` (3b) | `issues` | `ruleAdvisories` | unchanged |
44+
| `capability provider preflight failed` (3c) | `issues` | absent | rule + capability |
45+
| `access matrix drift` (3e) | `changes` | absent | rule + key + capability |
46+
| `docs validation failed` (3f) | `issues` | absent | all four lists |
47+
| `--no-runtime-bundle` refusal (4b) | `error` | absent | all four lists |
48+
| `runtime bundle failed` (4b) | `error` | absent | all four lists |
49+
| thrown / caught (bottom) | `error` | absent | what the run had computed |
50+
51+
The success payload is unchanged in content: its
52+
`[...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings]`
53+
spread — `os validate --json`'s order minus its trailing `structuralWarnings`
54+
— moved to a single `warningsSoFar()` site that every exit now reads, so the
55+
member order cannot drift between exits.
56+
57+
## What a consumer keying off its absence should do instead
58+
59+
⛔ `warnings` is no longer a signal of which exit produced the payload. Read
60+
`success` (and `error` / `errors`) for that; a consumer that inferred "this is
61+
a failure payload" from a missing `warnings` must switch to `success === false`.
62+
63+
⛔ `warnings: []` on a failure payload does NOT mean "this tree raises no
64+
advisories". It means **this run stopped before those advisories were
65+
computed** — the two early exits above (`strict-body`, protocol parse) run
66+
before any advisory step, so their list is empty by construction. A consumer
67+
that needs the full advisory set for a tree must read it from a run that
68+
reaches at least the gate that computes it, or from `os validate --json`.
69+
70+
✅ `warnings` is always an array on every `os build --json` payload, success or
71+
failure, so it can be read unconditionally — that shape constancy is the point
72+
of the change (maintainer ruling 2026-08-25, option 1 of three; option 2,
73+
"carry them only where the text face printed them", was rejected as the hardest
74+
contract to declare).
75+
76+
Advisories stay CARRIED, never recomputed: each list is still computed at
77+
exactly the step that owns it, so an exit upstream of a step legitimately
78+
reports that list empty and no failure path pays for a computation it did not
79+
already do.

‎packages/cli/src/commands/compile.ts‎

Lines changed: 81 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
1919
import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '@objectstack/lint';
2020
import { resolveSduiManifest } from '../utils/sdui-manifest.js';
2121
import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
22-
import { collectAndLintDocs } from '../utils/collect-docs.js';
22+
import { collectAndLintDocs, type DocIssue } from '../utils/collect-docs.js';
2323
import { buildRuntimeBundle, cleanupOldRuntimeBundles } from '../utils/build-runtime.js';
2424
import {
2525
printHeader,
@@ -83,6 +83,59 @@ export default class Compile extends Command {
8383
printHeader('Compile');
8484
}
8585

86+
// [#11772] THE ADVISORY LISTS THIS RUN HAS COMPUTED SO FAR, hoisted out of
87+
// the `try` so that EVERY `emitJson` exit can read them — not the terminal
88+
// success payload alone.
89+
//
90+
// The defect: `warnings` lived on the success payload only (plus, for one
91+
// list, the author-time-rules failure). The text face prints the #3786
92+
// undeclared-authoring-key block at 3d ending `— re-run with --json for the
93+
// full list`, and #11529's advisory printer ends the same way. A build that
94+
// then failed at a LATER gate (access matrix 3e, package docs 3f, the
95+
// runtime bundle, or a throw caught at the bottom) emitted that gate's
96+
// failure payload, and none of those carried the list — so the author was
97+
// told to re-run with `--json` and got a payload without the withheld
98+
// entries in it. That is the "the remedy named is unreachable" shape of
99+
// #11643 and #11391.
100+
//
101+
// Maintainer ruling 2026-08-25, option 1 of the three the card offered:
102+
// every failure exit carries the lists the run has ALREADY COMPUTED, so
103+
// `warnings` means the same thing on every exit and a machine consumer has
104+
// exactly one way to read it. Option 2 — carry them only where the text
105+
// face printed them, making the payload's SHAPE depend on how far the run
106+
// got — was rejected as the hardest contract to declare. Option 3 (weaken
107+
// the pointer) was rejected as making the product worse.
108+
//
109+
// ⛔ CARRYING, NOT COMPUTING. Every list stays computed at exactly the step
110+
// that owns it; these bindings only make the value visible to the exits
111+
// DOWNSTREAM of that step. An exit that runs before a given step therefore
112+
// still sees that list empty, and that is the honest reading of "what the
113+
// run has already computed": hoisting a computation earlier so an early
114+
// exit looks fuller would be option 2 wearing option 1's clothes, and it
115+
// would change what the command costs on its failure paths as well.
116+
//
117+
// ORDER IS `os validate --json`'s, stated ONCE here and read by the success
118+
// payload too — the "one list cannot drift from itself" idiom #11643 and
119+
// #11727 applied one list over. The spread used to be written out at the
120+
// payload, so a tenth exit could have been added with a different order and
121+
// nothing would have caught it.
122+
// Typed off `splitBySeverity` and not by naming `AuthoringFinding`: the #4409
123+
// import scan (packages/lint/src/authoring-rule-wiring.test.ts) reads every
124+
// symbol this file names from `@objectstack/lint` and strips `type ` rather
125+
// than exempting it, and `splitBySeverity` — which produces this list — is
126+
// already ratcheted there. Binding the annotation to the producer is also the
127+
// tighter statement: the list cannot disagree with the function that fills it.
128+
let ruleAdvisories: ReturnType<typeof splitBySeverity>['advisories'] = [];
129+
let capProviderWarnings: Array<{ token: string; message: string }> = [];
130+
let unknownKeyWarnings: string[] = [];
131+
let docWarnings: DocIssue[] = [];
132+
const warningsSoFar = () => [
133+
...ruleAdvisories,
134+
...docWarnings,
135+
...unknownKeyWarnings,
136+
...capProviderWarnings,
137+
];
138+
86139
try {
87140
// 1. Load Configuration
88141
if (!flags.json) printStep('Loading configuration...');
@@ -141,7 +194,7 @@ export default class Compile extends Command {
141194
];
142195
if (issues.length > 0) {
143196
if (flags.json) {
144-
await emitJson({ success: false, error: 'strict-body: missing body', issues }, 0, { compact: true });
197+
await emitJson({ success: false, error: 'strict-body: missing body', issues, warnings: warningsSoFar() }, 0, { compact: true });
145198
this.exit(1);
146199
}
147200
console.log('');
@@ -196,7 +249,7 @@ export default class Compile extends Command {
196249

197250
if (!result.success) {
198251
if (flags.json) {
199-
await emitJson({ success: false, errors: (result.error as unknown as ZodError).issues }, 0, { compact: true });
252+
await emitJson({ success: false, errors: (result.error as unknown as ZodError).issues, warnings: warningsSoFar() }, 0, { compact: true });
200253
this.exit(1);
201254
}
202255
console.log('');
@@ -223,7 +276,8 @@ export default class Compile extends Command {
223276
parsed: result.data as Record<string, unknown>,
224277
sduiManifest: resolveSduiManifest(),
225278
});
226-
const { errors: ruleErrors, advisories: ruleAdvisories } = splitBySeverity(findings);
279+
const { errors: ruleErrors, advisories } = splitBySeverity(findings);
280+
ruleAdvisories = advisories;
227281

228282
if (ruleAdvisories.length > 0 && !flags.json) {
229283
console.log('');
@@ -237,7 +291,7 @@ export default class Compile extends Command {
237291
// Every failing rule reports at once — see the note in `validate.ts`.
238292
if (flags.json) {
239293
await emitJson(
240-
{ success: false, error: 'author-time rules failed', issues: ruleErrors, warnings: ruleAdvisories },
294+
{ success: false, error: 'author-time rules failed', issues: ruleErrors, warnings: warningsSoFar() },
241295
0,
242296
{ compact: true },
243297
);
@@ -277,7 +331,7 @@ export default class Compile extends Command {
277331
// `{ token, message }` record beside its own preflight call, so
278332
// mirroring it is what keeps the two commands from reporting
279333
// different sets. One list cannot drift from itself.
280-
const capProviderWarnings = capPreflight.warnings.map((c) => ({
334+
capProviderWarnings = capPreflight.warnings.map((c) => ({
281335
token: c.token,
282336
message: renderCapabilityMessage(c),
283337
}));
@@ -287,6 +341,7 @@ export default class Compile extends Command {
287341
success: false,
288342
error: 'capability provider preflight failed',
289343
issues: capPreflight.errors.map((c) => ({ token: c.token, message: renderCapabilityMessage(c) })),
344+
warnings: warningsSoFar(),
290345
}, 0, { compact: true });
291346
this.exit(1);
292347
}
@@ -322,7 +377,7 @@ export default class Compile extends Command {
322377
// its own `normalized` — so hoisting the formatting rather than
323378
// restating it at the payload is what keeps the two faces from
324379
// reporting different sets. One list cannot drift from itself.
325-
const unknownKeyWarnings = [
380+
unknownKeyWarnings = [
326381
...lintUnknownStackKeys(normalized as Record<string, unknown>, ObjectStackDefinitionSchema),
327382
...lintUnknownAuthoringKeys(normalized as Record<string, unknown>, ObjectStackDefinitionSchema),
328383
].map(formatUnknownAuthoringKey);
@@ -334,18 +389,18 @@ export default class Compile extends Command {
334389
// into the `--json` payload (`warnings`) a few lines below; it would
335390
// have been a dead end before that landed.
336391
//
337-
// ⚠️ …and it resolves ON THE SUCCESS EXIT ONLY — the one conditional
338-
// pointer of the nine. `warnings` lives in the terminal payload, so a
339-
// build that fails at a LATER gate (access matrix 3e, package docs 3f,
340-
// the runtime bundle) emits that gate's failure payload instead, and
341-
// none of those carries this list: the author is told to re-run with
342-
// `--json` and gets a payload without the withheld keys in it. The six
343-
// error-path notices have no such gap — their `--json` branch sits in
344-
// the same block as the text face. Filed as #11772; closing it means
345-
// changing a `--json` payload shape, which is a machine-contract
346-
// decision and not this card's. ⛔ Do not read the line above as
347-
// unconditional — an unqualified claim that holds in one branch is the
348-
// same shape as the silence this whole change is about.
392+
// [#11772] …and it resolves on EVERY exit now, which is what makes the
393+
// pointer above unconditional. It used to resolve on the SUCCESS exit
394+
// alone: `warnings` lived in the terminal payload, so a build that
395+
// failed at a LATER gate (access matrix 3e, package docs 3f, the
396+
// runtime bundle, or a throw) emitted that gate's failure payload and
397+
// none of those carried this list — the author was told to re-run with
398+
// `--json` and got a payload without the withheld keys in it. Every
399+
// `emitJson` exit reads `warningsSoFar()`, so this list now survives
400+
// whichever later gate stops the run. ⛔ If a tenth exit is added, it
401+
// carries the lists too, or this pointer goes back to being a claim
402+
// that holds in one branch only — the same shape as the silence
403+
// #11642 was about. `build-json-failure-warnings.e2e.test.ts` pins it.
349404
printBulletList(unknownKeyWarnings, {
350405
noun: 'undeclared authoring key(s)',
351406
remedy: JSON_FULL_LIST_REMEDY,
@@ -382,7 +437,7 @@ export default class Compile extends Command {
382437
const drift = diffAccessMatrix(committed, currentMatrix);
383438
if (drift.length > 0) {
384439
if (flags.json) {
385-
await emitJson({ success: false, error: 'access matrix drift', changes: drift }, 0, { compact: true });
440+
await emitJson({ success: false, error: 'access matrix drift', changes: drift, warnings: warningsSoFar() }, 0, { compact: true });
386441
this.exit(1);
387442
}
388443
console.log('');
@@ -423,10 +478,10 @@ export default class Compile extends Command {
423478
// `severity === 'warning'` and validate's `severity !== 'error'`
424479
// select the same set: `DocIssue.severity` is `'error' | 'warning'`,
425480
// so there is no third value for the two spellings to disagree about.
426-
const docWarnings = docsResult.issues.filter((i) => i.severity === 'warning');
481+
docWarnings = docsResult.issues.filter((i) => i.severity === 'warning');
427482
if (docErrors.length > 0) {
428483
if (flags.json) {
429-
await emitJson({ success: false, error: 'docs validation failed', issues: docErrors }, 0, { compact: true });
484+
await emitJson({ success: false, error: 'docs validation failed', issues: docErrors, warnings: warningsSoFar() }, 0, { compact: true });
430485
this.exit(1);
431486
}
432487
console.log('');
@@ -480,7 +535,7 @@ export default class Compile extends Command {
480535
// pipelines can guard against accidental regressions.
481536
const msg = `--no-runtime-bundle requires every callable to have a metadata body (${stillNeeded} missing, ${lowering.bodyExtractionWarnings.length} extraction warning(s)). Re-run with --strict-body to see details, or omit --no-runtime-bundle.`;
482537
if (flags.json) {
483-
await emitJson({ success: false, error: msg }, 0, { compact: true });
538+
await emitJson({ success: false, error: msg, warnings: warningsSoFar() }, 0, { compact: true });
484539
this.exit(1);
485540
}
486541
console.log('');
@@ -504,7 +559,7 @@ export default class Compile extends Command {
504559
cleanupOldRuntimeBundles(artifactDir, runtimeBundle.outputFileName);
505560
} catch (err: any) {
506561
if (flags.json) {
507-
await emitJson({ success: false, error: `runtime bundle failed: ${err.message}` }, 0, { compact: true });
562+
await emitJson({ success: false, error: `runtime bundle failed: ${err.message}`, warnings: warningsSoFar() }, 0, { compact: true });
508563
this.exit(1);
509564
}
510565
console.log('');
@@ -584,7 +639,7 @@ export default class Compile extends Command {
584639
// port. Measured on #11727 (this change) and split out as #11896,
585640
// which is where that judgment is made — deliberately NOT this card,
586641
// which #11727 closes.
587-
warnings: [...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings],
642+
warnings: warningsSoFar(),
588643
// [#10678] Body-extraction failures that made a callable fall back to
589644
// the legacy .mjs bundle. A SEPARATE key on purpose, and the reason is
590645
// parity too — the opposite way round from `unknownKeyWarnings` just
@@ -642,7 +697,7 @@ export default class Compile extends Command {
642697
} catch (error: any) {
643698
if (isExitSignal(error)) throw error;
644699
if (flags.json) {
645-
await emitJson({ success: false, error: error.message }, 0, { compact: true });
700+
await emitJson({ success: false, error: error.message, warnings: warningsSoFar() }, 0, { compact: true });
646701
this.exit(1);
647702
}
648703
console.log('');

0 commit comments

Comments
 (0)