Skip to content

feat(cli): carry the computed advisory lists on every os build --json failure exit#12079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-zhuang merged 2 commits into from
Aug 25, 2026
Merged

feat(cli): carry the computed advisory lists on every os build --json failure exit #12079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c7bf12
feat(cli): carry the computed advisory lists on every `os build --jso…
claude Aug 25, 2026
6da672d
fix(cli): bind the hoisted advisory list to splitBySeverity's return …
claude Aug 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions .changeset/build-json-failure-payload-warnings.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
---
"@objectstack/cli": minor
---

feat(cli): `os build --json` carries the computed advisory lists on every failure exit, not the success payload alone (#11772)

**Machine-contract widening on the `--json` failure payloads.** A consumer that
today branches on `warnings` being ABSENT from an `os build --json` failure
payload sees a different shape after this change.

## What was wrong

The text face prints its advisory blocks before the gates that can stop the
run — the #11529 author-time advisories at step 3b, the #3786 undeclared
authoring-key findings at 3d — and both end in `— re-run with --json for the
full list`. But `warnings` lived on the TERMINAL SUCCESS payload only (plus,
for `ruleAdvisories` alone, the author-time-rules failure). On a tree with 60
undeclared authoring keys *and* a package-docs error:

```
os build Undeclared authoring keys (60) … 50 rows …
… and 10 more … — re-run with --json for the full list
os build --json {"success":false,"error":"docs validation failed","issues":[…]}
^ the 60 keys nowhere
```

The remedy the notice named returned a payload that did not contain the list,
and the author could not reach the withheld entries by any route until an
unrelated later failure was fixed — the "the remedy named is unreachable"
shape of #11643 and #11391.

## Which exits gain the field

All nine failure exits of `os build --json`. Six already had a payload of their
own; three more were found while enumerating (the filing card's table listed
six). `warnings` is now present on every one, alongside each exit's existing
keys, which are unchanged:

| exit (step) | existing keys | `warnings` before | after |
| --- | --- | --- | --- |
| `strict-body: missing body` (2b) | `issues` | absent | `[]` |
| protocol parse failure (3) | `errors` | absent | `[]` |
| `author-time rules failed` (3b) | `issues` | `ruleAdvisories` | unchanged |
| `capability provider preflight failed` (3c) | `issues` | absent | rule + capability |
| `access matrix drift` (3e) | `changes` | absent | rule + key + capability |
| `docs validation failed` (3f) | `issues` | absent | all four lists |
| `--no-runtime-bundle` refusal (4b) | `error` | absent | all four lists |
| `runtime bundle failed` (4b) | `error` | absent | all four lists |
| thrown / caught (bottom) | `error` | absent | what the run had computed |

The success payload is unchanged in content: its
`[...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings]`
spread — `os validate --json`'s order minus its trailing `structuralWarnings`
— moved to a single `warningsSoFar()` site that every exit now reads, so the
member order cannot drift between exits.

## What a consumer keying off its absence should do instead

⛔ `warnings` is no longer a signal of which exit produced the payload. Read
`success` (and `error` / `errors`) for that; a consumer that inferred "this is
a failure payload" from a missing `warnings` must switch to `success === false`.

⛔ `warnings: []` on a failure payload does NOT mean "this tree raises no
advisories". It means **this run stopped before those advisories were
computed** — the two early exits above (`strict-body`, protocol parse) run
before any advisory step, so their list is empty by construction. A consumer
that needs the full advisory set for a tree must read it from a run that
reaches at least the gate that computes it, or from `os validate --json`.

✅ `warnings` is always an array on every `os build --json` payload, success or
failure, so it can be read unconditionally — that shape constancy is the point
of the change (maintainer ruling 2026-08-25, option 1 of three; option 2,
"carry them only where the text face printed them", was rejected as the hardest
contract to declare).

Advisories stay CARRIED, never recomputed: each list is still computed at
exactly the step that owns it, so an exit upstream of a step legitimately
reports that list empty and no failure path pays for a computation it did not
already do.
107 changes: 81 additions & 26 deletions packages/cli/src/commands/compile.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,7 +19,7 @@ import { buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '@objectstack/lint';
import { resolveSduiManifest } from '../utils/sdui-manifest.js';
import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
import { collectAndLintDocs } from '../utils/collect-docs.js';
import { collectAndLintDocs, type DocIssue } from '../utils/collect-docs.js';
import { buildRuntimeBundle, cleanupOldRuntimeBundles } from '../utils/build-runtime.js';
import {
printHeader,
Expand DownExpand Up@@ -83,6 +83,59 @@ export default class Compile extends Command {
printHeader('Compile');
}

// [#11772] THE ADVISORY LISTS THIS RUN HAS COMPUTED SO FAR, hoisted out of
// the `try` so that EVERY `emitJson` exit can read them — not the terminal
// success payload alone.
//
// The defect: `warnings` lived on the success payload only (plus, for one
// list, the author-time-rules failure). The text face prints the #3786
// undeclared-authoring-key block at 3d ending `— re-run with --json for the
// full list`, and #11529's advisory printer ends the same way. A build that
// then failed at a LATER gate (access matrix 3e, package docs 3f, the
// runtime bundle, or a throw caught at the bottom) emitted that gate's
// failure payload, and none of those carried the list — so the author was
// told to re-run with `--json` and got a payload without the withheld
// entries in it. That is the "the remedy named is unreachable" shape of
// #11643 and #11391.
//
// Maintainer ruling 2026-08-25, option 1 of the three the card offered:
// every failure exit carries the lists the run has ALREADY COMPUTED, so
// `warnings` means the same thing on every exit and a machine consumer has
// exactly one way to read it. Option 2 — carry them only where the text
// face printed them, making the payload's SHAPE depend on how far the run
// got — was rejected as the hardest contract to declare. Option 3 (weaken
// the pointer) was rejected as making the product worse.
//
// ⛔ CARRYING, NOT COMPUTING. Every list stays computed at exactly the step
// that owns it; these bindings only make the value visible to the exits
// DOWNSTREAM of that step. An exit that runs before a given step therefore
// still sees that list empty, and that is the honest reading of "what the
// run has already computed": hoisting a computation earlier so an early
// exit looks fuller would be option 2 wearing option 1's clothes, and it
// would change what the command costs on its failure paths as well.
//
// ORDER IS `os validate --json`'s, stated ONCE here and read by the success
// payload too — the "one list cannot drift from itself" idiom #11643 and
// #11727 applied one list over. The spread used to be written out at the
// payload, so a tenth exit could have been added with a different order and
// nothing would have caught it.
// Typed off `splitBySeverity` and not by naming `AuthoringFinding`: the #4409
// import scan (packages/lint/src/authoring-rule-wiring.test.ts) reads every
// symbol this file names from `@objectstack/lint` and strips `type ` rather
// than exempting it, and `splitBySeverity` — which produces this list — is
// already ratcheted there. Binding the annotation to the producer is also the
// tighter statement: the list cannot disagree with the function that fills it.
let ruleAdvisories: ReturnType<typeof splitBySeverity>['advisories'] = [];
let capProviderWarnings: Array<{ token: string; message: string }> = [];
let unknownKeyWarnings: string[] = [];
let docWarnings: DocIssue[] = [];
const warningsSoFar = () => [
...ruleAdvisories,
...docWarnings,
...unknownKeyWarnings,
...capProviderWarnings,
];

try {
// 1. Load Configuration
if (!flags.json) printStep('Loading configuration...');
Expand DownExpand Up@@ -141,7 +194,7 @@ export default class Compile extends Command {
];
if (issues.length > 0) {
if (flags.json) {
await emitJson({ success: false, error: 'strict-body: missing body', issues }, 0, { compact: true });
await emitJson({ success: false, error: 'strict-body: missing body', issues, warnings: warningsSoFar() }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand DownExpand Up@@ -196,7 +249,7 @@ export default class Compile extends Command {

if (!result.success) {
if (flags.json) {
await emitJson({ success: false, errors: (result.error as unknown as ZodError).issues }, 0, { compact: true });
await emitJson({ success: false, errors: (result.error as unknown as ZodError).issues, warnings: warningsSoFar() }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand All@@ -223,7 +276,8 @@ export default class Compile extends Command {
parsed: result.data as Record<string, unknown>,
sduiManifest: resolveSduiManifest(),
});
const { errors: ruleErrors, advisories: ruleAdvisories } = splitBySeverity(findings);
const { errors: ruleErrors, advisories } = splitBySeverity(findings);
ruleAdvisories = advisories;

if (ruleAdvisories.length > 0 && !flags.json) {
console.log('');
Expand All@@ -237,7 +291,7 @@ export default class Compile extends Command {
// Every failing rule reports at once — see the note in `validate.ts`.
if (flags.json) {
await emitJson(
{ success: false, error: 'author-time rules failed', issues: ruleErrors, warnings: ruleAdvisories },
{ success: false, error: 'author-time rules failed', issues: ruleErrors, warnings: warningsSoFar() },
0,
{ compact: true },
);
Expand DownExpand Up@@ -277,7 +331,7 @@ export default class Compile extends Command {
// `{ token, message }` record beside its own preflight call, so
// mirroring it is what keeps the two commands from reporting
// different sets. One list cannot drift from itself.
const capProviderWarnings = capPreflight.warnings.map((c) => ({
capProviderWarnings = capPreflight.warnings.map((c) => ({
token: c.token,
message: renderCapabilityMessage(c),
}));
Expand All@@ -287,6 +341,7 @@ export default class Compile extends Command {
success: false,
error: 'capability provider preflight failed',
issues: capPreflight.errors.map((c) => ({ token: c.token, message: renderCapabilityMessage(c) })),
warnings: warningsSoFar(),
}, 0, { compact: true });
this.exit(1);
}
Expand DownExpand Up@@ -322,7 +377,7 @@ export default class Compile extends Command {
// its own `normalized` — so hoisting the formatting rather than
// restating it at the payload is what keeps the two faces from
// reporting different sets. One list cannot drift from itself.
const unknownKeyWarnings = [
unknownKeyWarnings = [
...lintUnknownStackKeys(normalized as Record<string, unknown>, ObjectStackDefinitionSchema),
...lintUnknownAuthoringKeys(normalized as Record<string, unknown>, ObjectStackDefinitionSchema),
].map(formatUnknownAuthoringKey);
Expand All@@ -334,18 +389,18 @@ export default class Compile extends Command {
// into the `--json` payload (`warnings`) a few lines below; it would
// have been a dead end before that landed.
//
// ⚠️ …and it resolves ON THE SUCCESS EXIT ONLY — the one conditional
// pointer of the nine. `warnings` lives in the terminal payload, so a
// build that fails at a LATER gate (access matrix 3e, package docs 3f,
// the runtime bundle) emits that gate's failure payload instead, and
// none of those carries this list: the author is told to re-run with
// `--json` and gets a payload without the withheld keys in it. The six
// error-path notices have no such gap — their `--json` branch sits in
// the same block as the text face. Filed as #11772; closing it means
// changing a `--json` payload shape, which is a machine-contract
// decision and not this card's. ⛔ Do not read the line above as
// unconditional — an unqualified claim that holds in one branch is the
// same shape as the silence this whole change is about.
// [#11772] …and it resolves on EVERY exit now, which is what makes the
// pointer above unconditional. It used to resolve on the SUCCESS exit
// alone: `warnings` lived in the terminal payload, so a build that
// failed at a LATER gate (access matrix 3e, package docs 3f, the
// runtime bundle, or a throw) emitted that gate's failure payload and
// none of those carried this list — the author was told to re-run with
// `--json` and got a payload without the withheld keys in it. Every
// `emitJson` exit reads `warningsSoFar()`, so this list now survives
// whichever later gate stops the run. ⛔ If a tenth exit is added, it
// carries the lists too, or this pointer goes back to being a claim
// that holds in one branch only — the same shape as the silence
// #11642 was about. `build-json-failure-warnings.e2e.test.ts` pins it.
printBulletList(unknownKeyWarnings, {
noun: 'undeclared authoring key(s)',
remedy: JSON_FULL_LIST_REMEDY,
Expand DownExpand Up@@ -382,7 +437,7 @@ export default class Compile extends Command {
const drift = diffAccessMatrix(committed, currentMatrix);
if (drift.length > 0) {
if (flags.json) {
await emitJson({ success: false, error: 'access matrix drift', changes: drift }, 0, { compact: true });
await emitJson({ success: false, error: 'access matrix drift', changes: drift, warnings: warningsSoFar() }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand DownExpand Up@@ -423,10 +478,10 @@ export default class Compile extends Command {
// `severity === 'warning'` and validate's `severity !== 'error'`
// select the same set: `DocIssue.severity` is `'error' | 'warning'`,
// so there is no third value for the two spellings to disagree about.
const docWarnings = docsResult.issues.filter((i) => i.severity === 'warning');
docWarnings = docsResult.issues.filter((i) => i.severity === 'warning');
if (docErrors.length > 0) {
if (flags.json) {
await emitJson({ success: false, error: 'docs validation failed', issues: docErrors }, 0, { compact: true });
await emitJson({ success: false, error: 'docs validation failed', issues: docErrors, warnings: warningsSoFar() }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand DownExpand Up@@ -480,7 +535,7 @@ export default class Compile extends Command {
// pipelines can guard against accidental regressions.
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.`;
if (flags.json) {
await emitJson({ success: false, error: msg }, 0, { compact: true });
await emitJson({ success: false, error: msg, warnings: warningsSoFar() }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand All@@ -504,7 +559,7 @@ export default class Compile extends Command {
cleanupOldRuntimeBundles(artifactDir, runtimeBundle.outputFileName);
} catch (err: any) {
if (flags.json) {
await emitJson({ success: false, error: `runtime bundle failed: ${err.message}` }, 0, { compact: true });
await emitJson({ success: false, error: `runtime bundle failed: ${err.message}`, warnings: warningsSoFar() }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand DownExpand Up@@ -584,7 +639,7 @@ export default class Compile extends Command {
// port. Measured on #11727 (this change) and split out as #11896,
// which is where that judgment is made — deliberately NOT this card,
// which #11727 closes.
warnings: [...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings],
warnings: warningsSoFar(),
// [#10678] Body-extraction failures that made a callable fall back to
// the legacy .mjs bundle. A SEPARATE key on purpose, and the reason is
// parity too — the opposite way round from `unknownKeyWarnings` just
Expand DownExpand Up@@ -642,7 +697,7 @@ export default class Compile extends Command {
} catch (error: any) {
if (isExitSignal(error)) throw error;
if (flags.json) {
await emitJson({ success: false, error: error.message }, 0, { compact: true });
await emitJson({ success: false, error: error.message, warnings: warningsSoFar() }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand Down
Loading
Loading