Skip to content

feat(cli): carry the computed conversions on every os validate --json and os build --json failure exit#12285

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-trump merged 3 commits into from
Aug 25, 2026
Merged

feat(cli): carry the computed conversions on every os validate --json and os build --json failure exit #12285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a10c6a1
feat(cli): carry the computed `conversions` on every `os validate --j…
claude Aug 25, 2026
bce8ef4
test(cli): drive the conversion fixtures with single-root JSX source
claude Aug 25, 2026
2c7f002
test(cli): make the fold regression guard green in BOTH states, as it…
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
91 changes: 91 additions & 0 deletions .changeset/cli-json-failure-payload-conversions.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
---
"@objectstack/cli": minor
---

feat(cli): `os validate --json` and `os build --json` carry the computed `conversions` on every failure exit, not the success payload alone (#12125)

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

## What was wrong

`conversionNotices` is filled by the `onConversionNotice` sink handed to
`normalizeStackInput`, which runs at **step 2** — above the schema parse and
above every later gate in both commands. The notice was therefore already in
hand when any failure exit fired, and was then discarded: `conversions:` was
published on the terminal SUCCESS payload alone.

An ADR-0087 D2 conversion notice is the one advisory class that carries an
**expiry** — `retiresIn` names the protocol major where the old shape stops
loading. So a CI job gating on `os validate --json` / `os build --json` could
not see that its tree depends on a conversion about to retire for as long as
the tree also tripped any unrelated gate — the notice was withheld exactly
while the tree was broken, which is when an author is most likely editing it.

This is the same "computed, then dropped on a failure exit" shape as the
`warnings` family (#11643 / #11391 / #11772 / #12047), one field over. #12079
added `warnings` to all nine `os build` failure exits and deliberately left
`conversions` untouched, so closing those cards did not close this one.

## What changed

`conversions` is now present on every `emitJson` exit of both commands —
6 in `validate.ts` (5 failure + success), 10 in `compile.ts` (9 failure +
success) — alongside each exit's existing keys, which are unchanged.

| command | exit | `conversions` before | after |
| --- | --- | --- | --- |
| `os validate --json` | protocol parse failure | absent | the computed notices |
| `os validate --json` | author-time rules failed | absent | the computed notices |
| `os validate --json` | capability provider check | absent | the computed notices |
| `os validate --json` | package docs failed | absent | the computed notices |
| `os validate --json` | thrown / caught | absent | what the run had computed |
| `os build --json` | all nine failure exits | absent | what the run had computed |

The success payloads are unchanged in content.

Notices are **carried, not recomputed**: the fix is a pure scope change — the
sink array is declared above the `try` so the catch-all exit can read it — and
`normalizeStackInput` still runs at exactly step 2. A run that throws in
`loadConfig`, above step 2, therefore reports `[]` honestly.

⭐ Note the two fields differ on `os build`'s two earliest exits. For `warnings`,
`--strict-body` and the protocol parse are empty by construction (nothing
advisory is computed that early); step 2 is **above** both, so `conversions` is
populated there. The field was measured per exit rather than inherited from the
sibling change.

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

⛔ `conversions` is no longer a signal of which exit produced the payload, nor
of success. Read `valid` (validate) / `success` (build), and `error` /
`errors`, for that; a consumer that inferred "this is a failure payload" from a
missing `conversions` must switch to the explicit status field.

⛔ `conversions: []` on a failure payload does NOT mean "this tree converts
nothing". It means **this run stopped before the conversion layer ran** — a
config that fails to load reports `[]` by construction. A consumer that needs
the true conversion set for a tree must read it from a run that reaches at
least step 2.

✅ `conversions` is always an array on every `os validate --json` and
`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 on #11772/#12047, option 1 of three, applied here under the
same-family rule; option 2, "carry it only where the text face printed it", was
rejected as the hardest contract to declare).

✅ Each entry keeps its structured fields — `conversionId`, `surface`, `from`,
`to`, `path`, `toMajor`, `retiresIn` — on failure exits exactly as on the
success payload, so a CI job can gate on `retiresIn` without a second run.

Exit codes are untouched: every failure exit still exits 1. `--strict` on
`os validate` still reads the text face's own list, which folds conversion
notices in, so `os validate --json --strict` reaches the same verdict it did
before.

`warnings` and `conversions` remain **separate fields**. Whether the two should
be folded into one is a live question raised on #12125 and not settled by the
ruling; this change deliberately mirrors the `warnings` shape rather than
merging either field into the other.
41 changes: 32 additions & 9 deletions packages/cli/src/commands/compile.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,6 +135,27 @@ export default class Compile extends Command {
...unknownKeyWarnings,
...capProviderWarnings,
];
// [#12125] The ADR-0087 D2 conversion notices, hoisted for the SAME reason
// and under the SAME ruling as the four lists above — one field over. The
// notices were computed at step 2 (below) and reached the terminal SUCCESS
// payload alone, so all nine failure exits dropped a list already in hand.
// #12079 added `warnings` to those nine and deliberately left this field
// untouched, which is why closing that card did not close this one.
//
// ⛔ CARRYING, NOT COMPUTING — and here that is a pure SCOPE change. This is
// the same `const` array the `onConversionNotice` sink pushes into, moved
// above the `try` only so the catch-all exit can read it. `normalizeStackInput`
// still runs at exactly step 2, so a run that throws in `loadConfig` — above
// it — reports `[]` honestly, exactly as `warningsSoFar()` does there.
//
// ⛔ NOT FOLDED INTO `warningsSoFar()`, in either direction. The success
// payload keeps these separate deliberately (see its note at `conversions:`
// below), and this field is the one advisory class carrying an EXPIRY —
// `retiresIn` names the protocol major where the source stops loading, which
// is structure a flattened warning string cannot carry. Whether the two
// should become one field is an open question this change was explicitly not
// given the authority to settle, so the shape is mirrored, not merged.
const conversionNotices: ConversionNotice[] = [];

try {
// 1. Load Configuration
Expand All@@ -157,7 +178,8 @@ export default class Compile extends Command {
// stops loading. Five conversions are live today (protocol 11 and 15),
// so the gap is real, not hypothetical.
if (!flags.json) printStep('Normalizing stack definition...');
const conversionNotices: ConversionNotice[] = [];
// The sink is declared above the `try` (see its note there); the CALL that
// fills it stays right here, at the step that owns it.
const normalized = normalizeStackInput(config as Record<string, unknown>, {
onConversionNotice: (n) => conversionNotices.push(n),
});
Expand DownExpand Up@@ -194,7 +216,7 @@ export default class Compile extends Command {
];
if (issues.length > 0) {
if (flags.json) {
await emitJson({ success: false, error: 'strict-body: missing body', issues, warnings: warningsSoFar() }, 0, { compact: true });
await emitJson({ success: false, error: 'strict-body: missing body', issues, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand DownExpand Up@@ -249,7 +271,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, warnings: warningsSoFar() }, 0, { compact: true });
await emitJson({ success: false, errors: (result.error as unknown as ZodError).issues, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand DownExpand Up@@ -291,7 +313,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: warningsSoFar() },
{ success: false, error: 'author-time rules failed', issues: ruleErrors, warnings: warningsSoFar(), conversions: conversionNotices },
0,
{ compact: true },
);
Expand DownExpand Up@@ -342,6 +364,7 @@ export default class Compile extends Command {
error: 'capability provider preflight failed',
issues: capPreflight.errors.map((c) => ({ token: c.token, message: renderCapabilityMessage(c) })),
warnings: warningsSoFar(),
conversions: conversionNotices,
}, 0, { compact: true });
this.exit(1);
}
Expand DownExpand Up@@ -437,7 +460,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, warnings: warningsSoFar() }, 0, { compact: true });
await emitJson({ success: false, error: 'access matrix drift', changes: drift, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand DownExpand Up@@ -481,7 +504,7 @@ export default class Compile extends Command {
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, warnings: warningsSoFar() }, 0, { compact: true });
await emitJson({ success: false, error: 'docs validation failed', issues: docErrors, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand DownExpand Up@@ -535,7 +558,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, warnings: warningsSoFar() }, 0, { compact: true });
await emitJson({ success: false, error: msg, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand All@@ -559,7 +582,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}`, warnings: warningsSoFar() }, 0, { compact: true });
await emitJson({ success: false, error: `runtime bundle failed: ${err.message}`, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand DownExpand Up@@ -697,7 +720,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, warnings: warningsSoFar() }, 0, { compact: true });
await emitJson({ success: false, error: error.message, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
this.exit(1);
}
console.log('');
Expand Down
40 changes: 39 additions & 1 deletion packages/cli/src/commands/validate.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -126,6 +126,31 @@ export default class Validate extends Command {
...capProviderWarnings,
...structuralWarnings,
];
// [#12125] The ADR-0087 D2 conversion notices, hoisted for the SAME reason
// and under the SAME ruling as the five lists above — one field over. The
// notices were computed at step 2 (below) and reached the terminal SUCCESS
// payload alone, so all five failure exits dropped a list already in hand.
//
// ⛔ CARRYING, NOT COMPUTING — and here that is a pure SCOPE change. This is
// the same `const` array the `onConversionNotice` sink pushes into, moved
// above the `try` only so the catch-all exit can read it. `normalizeStackInput`
// still runs at exactly step 2, so a run that throws in `loadConfig` — above
// it — reports `[]` honestly, exactly as `warningsSoFar()` does there.
//
// ⛔ NOT FOLDED INTO `warningsSoFar()`, in either direction. The two fields
// are separate on the success payload by an explicit decision recorded at
// that call site: the text face folds these notices into its `⚠` block (so
// `--strict` gates on them) while the payload carries them under their own
// key with their structured `conversionId`/`retiresIn` fields intact — the
// one advisory class that carries an EXPIRY. Whether the two should become
// one field is an open question this change was explicitly not given the
// authority to settle, so the shape is mirrored, not merged.
//
// No `conversionsSoFar()` wrapper: `warningsSoFar()` exists because five
// producers had to be concatenated in ONE stated order, and this list has
// exactly one producer. Reading the binding directly already is the "a list
// cannot drift from itself" idiom the wrapper was built to buy.
const conversionNotices: ConversionNotice[] = [];

try {
// 1. Load configuration
Expand All@@ -143,7 +168,8 @@ export default class Validate extends Command {
// the author knows the source still carries an old-shape key that will
// retire from the load path in a future major.
if (!flags.json) printStep('Validating against ObjectStack Protocol...');
const conversionNotices: ConversionNotice[] = [];
// The sink is declared above the `try` (see its note there); the CALL that
// fills it stays right here, at the step that owns it.
const normalized = normalizeStackInput(config as Record<string, unknown>, {
onConversionNotice: (n) => conversionNotices.push(n),
});
Expand All@@ -169,6 +195,9 @@ export default class Validate extends Command {
// called the strongest instance: the hoist exists so the finding
// SURVIVES a schema error, and this payload discarded it anyway.
warnings: warningsSoFar(),
// [#12125] Filled by `normalizeStackInput` two statements above this
// exit — the tightest instance of this card, and the one it measured.
conversions: conversionNotices,
duration: timer.elapsed(),
});
this.exit(1);
Expand DownExpand Up@@ -213,6 +242,8 @@ export default class Validate extends Command {
// the pre-parse `unknownKeyWarnings` — computed long before this
// gate — and keeps the member ORDER identical to every other exit.
warnings: warningsSoFar(),
// [#12125] Computed at step 2, above this gate.
conversions: conversionNotices,
duration: timer.elapsed(),
});
this.exit(1);
Expand DownExpand Up@@ -258,6 +289,8 @@ export default class Validate extends Command {
// `warnings` beside the two lists computed before this gate. The
// two classes being separate is the whole point of the split.
warnings: warningsSoFar(),
// [#12125] Computed at step 2, above this gate.
conversions: conversionNotices,
duration: timer.elapsed(),
});
this.exit(1);
Expand DownExpand Up@@ -294,6 +327,8 @@ export default class Validate extends Command {
// capability hints, and the pre-parse key findings were all in
// hand and none of them reached the payload.
warnings: warningsSoFar(),
// [#12125] Computed at step 2, above this gate.
conversions: conversionNotices,
duration: timer.elapsed(),
});
this.exit(1);
Expand DownExpand Up@@ -490,6 +525,9 @@ export default class Validate extends Command {
// is a FILE, say, which makes `readdirSync` raise ENOTDIR) carries
// the three lists already in hand.
warnings: warningsSoFar(),
// [#12125] Same reading, one field over: `[]` for a throw at load —
// step 2 had not run — and the notices in hand for any later throw.
conversions: conversionNotices,
duration: timer.elapsed(),
});
this.exit(1);
Expand Down
Loading
Loading