From a10c6a1d58489fd4ca98487a7dcc61547aef7dd1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 16:13:03 +0000 Subject: [PATCH 1/3] feat(cli): carry the computed `conversions` on every `os validate --json` and `os build --json` failure exit (#12125) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `conversionNotices` is filled by the `onConversionNotice` sink handed to `normalizeStackInput` at step 2 — above the schema parse and above every later gate in both commands — and was then published on the terminal success payload alone. All five `os validate` failure exits and all nine `os build` failure exits dropped a list already in hand. An ADR-0087 D2 notice is the one advisory class carrying an expiry, so a CI job gating on either `--json` face could not see that its tree depends on a conversion about to retire until every unrelated failure was fixed first. The fix is a pure SCOPE change: the sink array is declared above the `try` so the catch-all exit can read it. `normalizeStackInput` still runs at exactly step 2, so a run that throws in `loadConfig` reports `[]` honestly — carrying, never computing earlier. `warnings` and `conversions` are deliberately NOT folded: whether they should become one field is a live question the ruling did not address. Part of #12125 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HbG3rGVLjZStHQxHDtzJdJ --- .../cli-json-failure-payload-conversions.md | 91 ++++ packages/cli/src/commands/compile.ts | 41 +- packages/cli/src/commands/validate.ts | 40 +- ...build-json-failure-conversions.e2e.test.ts | 454 ++++++++++++++++++ ...idate-json-failure-conversions.e2e.test.ts | 449 +++++++++++++++++ 5 files changed, 1065 insertions(+), 10 deletions(-) create mode 100644 .changeset/cli-json-failure-payload-conversions.md create mode 100644 packages/cli/test/build-json-failure-conversions.e2e.test.ts create mode 100644 packages/cli/test/validate-json-failure-conversions.e2e.test.ts diff --git a/.changeset/cli-json-failure-payload-conversions.md b/.changeset/cli-json-failure-payload-conversions.md new file mode 100644 index 0000000000..d9100bae33 --- /dev/null +++ b/.changeset/cli-json-failure-payload-conversions.md @@ -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. diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index d1a43cbb6d..2dd9f9278d 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -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 @@ -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, { onConversionNotice: (n) => conversionNotices.push(n), }); @@ -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(''); @@ -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(''); @@ -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 }, ); @@ -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); } @@ -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(''); @@ -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(''); @@ -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(''); @@ -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(''); @@ -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(''); diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 24409611ae..2253413c8e 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -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 @@ -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, { onConversionNotice: (n) => conversionNotices.push(n), }); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); diff --git a/packages/cli/test/build-json-failure-conversions.e2e.test.ts b/packages/cli/test/build-json-failure-conversions.e2e.test.ts new file mode 100644 index 0000000000..dde9350886 --- /dev/null +++ b/packages/cli/test/build-json-failure-conversions.e2e.test.ts @@ -0,0 +1,454 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #12125 — `os build --json`'s FAILURE payloads dropped the `conversions` field + * the run had ALREADY COMPUTED, on all nine of its failure exits. + * + * The same "computed, then dropped on a failure exit" shape as the `warnings` + * family, one FIELD over. #12079 (for #11772) added `warnings` to all nine of + * these exits and deliberately left `conversions` untouched, so closing that + * card did not close this one. + * + * `conversionNotices` is filled by the `onConversionNotice` sink handed to + * `normalizeStackInput` at step 2, and `conversions:` was published on the + * terminal success payload alone. + * + * ## ⭐ Where this measurement DIFFERS from the `warnings` pins next door + * + * For `warnings`, compile's two earliest exits (2b `--strict-body` and 3 the + * protocol parse) carry `[]` by construction: nothing advisory is computed that + * early. `conversions` is NOT in that position — step 2 runs ABOVE both of them + * — so those two exits carry the notice, and every one of the nine failure + * exits below does. The only honestly-empty exit is a throw at LOAD, above step + * 2 itself. Measured per exit rather than inherited from the sibling card. + * + * ## The ruling these pins encode + * + * Maintainer, 2026-08-25 on #11772/#12047, applied here to `conversions` under + * the same-family rule: every failure exit carries the lists the run has + * ALREADY COMPUTED, so the field means the same thing on every exit. + * + * ⛔ CARRYING, NOT COMPUTING. The fix is a pure SCOPE change — the sink array + * moved above the `try` so the catch-all can read it — and `normalizeStackInput` + * still runs at exactly step 2. The `throw at load` pin is the half that holds + * that line. + * + * ## ⛔ WHAT THESE PINS DO NOT DECIDE + * + * Whether `warnings` and `conversions` should be FOLDED into one field is an + * open question this card had no authority to settle. `fields stay separate` is + * a REGRESSION GUARD recording the shape as-shipped — green before and after — + * ⛔ not an argument that folding is wrong. + * + * ## WHAT THESE PINS ASSERT — "what the run computed", not "the key exists" + * + * ⭐ A pin asserting `'conversions' in payload` passes against a `conversions: + * []` hard-coded at every exit. So every fixture drives a LIVE conversion + * (`page-kind-jsx-to-html`, ADR-0087 D2, protocol 11, on `pages[0].kind: + * 'jsx'`) and each exit is asserted to carry exactly that one notice — no + * fewer, and NO MORE, the array being asserted whole. `converts nothing` runs + * the same exit with the canonical `kind: 'html'` and requires `[]`, which is + * the negative whose positive is every other test here. + * + * ## Why no `dist/` sits on the measured path + * + * These run the CLI through `bin/run-dev.js`, the SOURCE entry point (src/ via + * tsx), so `compile.ts` is loaded from source and an ablation of it is measured + * without a rebuild. Its DEPENDENCY `@objectstack/spec` — which owns the + * conversion — resolves through `exports` to `dist/`, and is untouched here. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync, mkdirSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); +const COMPILE_TS = resolve(HERE, '../src/commands/compile.ts'); + +/** The live ADR-0087 D2 conversion these fixtures drive (protocol 11). */ +const CONVERSION_ID = 'page-kind-jsx-to-html'; +/** The FATAL capability token — `unavailable` whatever is installed. */ +const FATAL_TOKEN = 'governance'; + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...args], + { cwd, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) }, + (err, stdout, stderr) => { + resolvePromise({ + code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +function payloadOf(run: Run, label: string): Record { + try { + return JSON.parse(run.stdout) as Record; + } catch { + throw new Error(`${label}: stdout was not one JSON document (exit ${run.code})\n${run.stdout}\n${run.stderr}`); + } +} + +/** + * A stack whose `pages[0].kind` drives the live conversion. `pageKind` is a + * parameter so the negative control can run the identical shape with the + * CANONICAL spelling, where there is nothing to convert. + */ +function stack(ns: string, opts: { pageKind?: string; requires?: string[]; extraFields?: string; extraTop?: string } = {}): string { + const { pageKind = 'jsx', requires = [], extraFields = '', extraTop = '' } = opts; + return ` +export default { + manifest: { id: 'com.example.${ns}', name: '${ns}', version: '1.0.0', type: 'app', namespace: '${ns}' }, + requires: [${requires.map((r) => `'${r}'`).join(', ')}], + pages: [{ name: 'landing', label: 'Landing', kind: '${pageKind}', source: 'hi' }], + objects: [ + { + name: '${ns}_ticket', + label: 'Ticket', + sharingModel: 'private', + fields: { + title: { type: 'text', label: 'Title' },${extraFields} + }, + }, + ],${extraTop} +}; +`; +} + +/** A hook whose body cannot be lowered — `require()` is refused (#10678). */ +function unlowerableHook(ns: string): string { + return ` + hooks: [{ + name: '${ns}_hook', + object: '${ns}_ticket', + events: ['beforeInsert'], + handler: async (ctx: any) => { + const os = require('node:os'); + return os.platform(); + }, + }],`; +} + +const DOC_PLAIN = `--- +title: Wrong namespace +--- + +Body text. +`; + +/** The `conversions` field, as a list, whatever the payload shipped. */ +function conversionsOf(payload: Record): unknown[] { + return Array.isArray(payload.conversions) ? (payload.conversions as unknown[]) : []; +} + +/** + * The one notice this fixture family must produce, asserted by IDENTITY — + * `conversionId` plus the converted path — not by arity alone. + */ +const THE_NOTICE = { + conversionId: CONVERSION_ID, + surface: 'page.kind', + from: 'jsx', + to: 'html', + path: 'pages[0].kind', +}; + +/** Asserts EXACTLY the one computed notice. `toEqual` is the "and NO MORE" half. */ +function expectTheOneNotice(payload: Record, label: string): void { + expect(conversionsOf(payload), `${label}: expected exactly the one computed conversion notice`).toEqual([ + expect.objectContaining(THE_NOTICE), + ]); +} + +const dirs: Record = {}; +let root = ''; + +beforeAll(() => { + root = mkdtempSync(join(tmpdir(), 'os-build-fail-conversions-')); + const make = (name: string, config: string, docs: Array<[string, string]> = []): string => { + const dir = join(root, name); + mkdirSync(join(dir, 'src', 'docs'), { recursive: true }); + writeFileSync(join(dir, 'objectstack.config.ts'), config); + for (const [file, body] of docs) writeFileSync(join(dir, 'src', 'docs', file), body); + dirs[name] = dir; + return dir; + }; + + // 2b — `--strict-body`. ⭐ For `warnings` this exit is empty by construction; + // for `conversions` it is NOT, because step 2 runs above it. + make('strictbody', stack('sbody', { extraTop: unlowerableHook('sbody') })); + + // 3 — the protocol parse itself fails, likewise BELOW step 2. + make('zodfail', stack('zfail', { extraFields: ` + broken: { type: 'this_is_not_a_field_type', label: 'Broken' },` })); + + // ⭐ the negative control — the SAME exit, the CANONICAL page kind. + make('zodfail_nc', stack('znc', { pageKind: 'html', extraFields: ` + broken: { type: 'this_is_not_a_field_type', label: 'Broken' },` })); + + // 3b — an author-time rule ERROR. + make('rulefail', stack('rfail', { extraFields: ` + subject: { type: 'text', label: 'Subject', visibleWhen: { dialect: 'cel', source: 'record.zzz_no_such_field' } },` })); + + // 3c — the FATAL capability token. + make('capfail', stack('cfail', { requires: [FATAL_TOKEN] })); + + // 3e — a committed snapshot naming a permission set the stack does not grant. + const amx = make('amx', stack('amx')); + writeFileSync( + join(amx, 'access-matrix.json'), + JSON.stringify({ + version: 1, + entries: [{ + permissionSet: 'ghost_ps', object: 'amx_ticket', + create: false, read: true, edit: false, delete: false, + viewAllRecords: false, modifyAllRecords: false, + }], + }) + '\n', + ); + + // 3f — a doc ERROR (missing namespace prefix). + make('docsfail', stack('dfail'), [['otherns_guide.md', DOC_PLAIN]]); + + // 4b — `--no-runtime-bundle` over a callable that could not be lowered. + make('latefail', stack('lfail', { extraTop: unlowerableHook('lfail') })); + + // bottom — the artifact path is a DIRECTORY, so the write throws and the + // catch reports. Step 2 ran long before. + const thrown = make('thrown', stack('tfail')); + mkdirSync(join(thrown, 'out', 'artifact.json'), { recursive: true }); + + // catch-all, AT LOAD — the config throws on import, ABOVE step 2. + make('earlythrow', ` +throw new Error('zzz_config_module_threw'); +export default {}; +`); + + // The control — the same shape, reaching SUCCESS. + make('control', stack('ctrl')); +}); + +afterAll(() => { + if (root) rmSync(root, { recursive: true, force: true }); +}); + +describe('#12125 — every `os build --json` failure exit carries the conversions the run computed', () => { + it('control — the fixture shape DOES drive a live conversion on the success exit', async () => { + // ⭐ Anti-vacuity for every assertion below. + const run = await runCli(['build', '--json'], dirs.control); + expect(run.code, `expected the control to build:\n${run.stdout}${run.stderr}`).toBe(0); + const payload = payloadOf(run, 'control'); + expect(payload.success).toBe(true); + expectTheOneNotice(payload, 'control'); + expect(typeof (conversionsOf(payload)[0] as { retiresIn?: unknown }).retiresIn).toBe('number'); + }, 180_000); + + it('2b (--strict-body) — ⭐ carries the notice, where `warnings` is empty by construction', async () => { + // The measured difference from the sibling card. This exit is BELOW step 2 + // and ABOVE every advisory computation, so the two fields disagree here — + // which is exactly why `conversions` needed its own measurement. + const run = await runCli(['build', '--json', '--strict-body'], dirs.strictbody); + expect(run.code, `expected --strict-body to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'strictbody'); + expect(payload.success).toBe(false); + expect(String(payload.error)).toContain('strict-body'); + expect(payload.warnings, 'no advisory is computed this early — the sibling pins this too').toEqual([]); + expectTheOneNotice(payload, 'strictbody'); + }, 180_000); + + it('3 (protocol parse) — ⭐ carries the notice, where `warnings` is empty by construction', async () => { + const run = await runCli(['build', '--json'], dirs.zodfail); + expect(run.code, `expected the parse to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'zodfail'); + expect(payload.success).toBe(false); + expect(Array.isArray(payload.errors)).toBe(true); + expect(payload.warnings).toEqual([]); + expectTheOneNotice(payload, 'zodfail'); + }, 180_000); + + it('⭐ converts nothing — the SAME exit reports `[]`, so the field tracks the run', async () => { + const run = await runCli(['build', '--json'], dirs.zodfail_nc); + expect(run.code, `expected the parse to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'zodfail_nc'); + expect(payload.success).toBe(false); + expect('conversions' in payload, 'the field must be PRESENT even when empty').toBe(true); + expect(payload.conversions, 'a canonical page kind converts nothing').toEqual([]); + }, 180_000); + + it('3b (author-time rules) — the notice rides the rule gate', async () => { + const run = await runCli(['build', '--json'], dirs.rulefail); + expect(run.code, `expected the rule gate to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'rulefail'); + expect(String(payload.error)).toContain('author-time rules failed'); + expectTheOneNotice(payload, 'rulefail'); + }, 180_000); + + it('3c (capability preflight) — the notice rides the preflight gate', async () => { + const run = await runCli(['build', '--json'], dirs.capfail); + expect(run.code, `expected the capability gate to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'capfail'); + expect(String(payload.error)).toContain('capability provider preflight failed'); + expectTheOneNotice(payload, 'capfail'); + }, 180_000); + + it('3e (access-matrix drift) — the notice rides the drift gate', async () => { + const run = await runCli(['build', '--json'], dirs.amx); + expect(run.code, `expected the drift gate to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'amx'); + expect(String(payload.error)).toContain('access matrix drift'); + expectTheOneNotice(payload, 'amx'); + }, 180_000); + + it('3f (package docs) — the notice rides the docs gate', async () => { + const run = await runCli(['build', '--json'], dirs.docsfail); + expect(run.code, `expected the docs gate to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'docsfail'); + expect(String(payload.error)).toContain('docs validation failed'); + expectTheOneNotice(payload, 'docsfail'); + }, 180_000); + + it('4b (--no-runtime-bundle) — a late exit past every step carries the notice', async () => { + const run = await runCli(['build', '--json', '--no-runtime-bundle'], dirs.latefail); + expect(run.code, `expected --no-runtime-bundle to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'latefail'); + expect(String(payload.error)).toContain('--no-runtime-bundle'); + expectTheOneNotice(payload, 'latefail'); + }, 180_000); + + it('the catch-all — a THROWN failure still reports the computed notice', async () => { + // Before this change the sink was declared INSIDE the `try`, so the + // catch-all could not read it at all — structurally unreachable from this + // exit rather than merely omitted. + const run = await runCli(['build', '--json', '-o', 'out/artifact.json'], dirs.thrown); + expect(run.code, `expected the artifact write to throw:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'thrown'); + expect(payload.success).toBe(false); + expectTheOneNotice(payload, 'thrown'); + }, 180_000); + + it('catch-all (throw at load) — `conversions` is PRESENT and empty, because step 2 never ran', async () => { + // ⛔ The line that holds "carrying, not computing": this exit is ABOVE the + // normalize call, so `[]` is the honest reading. Hoisting the computation + // up to make it look fuller turns this red. + const run = await runCli(['build', '--json'], dirs.earlythrow); + expect(run.code, `expected the config module to throw:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'earlythrow'); + expect(String(payload.error)).toContain('zzz_config_module_threw'); + expect('conversions' in payload, 'the load-failure exit omits `conversions` entirely').toBe(true); + expect(payload.conversions).toEqual([]); + }, 180_000); + + it('REGRESSION GUARD — `warnings` and `conversions` stay separate fields', async () => { + // ⛔ NOT an argument that folding is wrong. Whether the two should become + // one field is an OPEN question this card was not given authority to + // settle; this records the shape as-shipped so a fold happens deliberately. + const run = await runCli(['build', '--json'], dirs.docsfail); + const payload = payloadOf(run, 'docsfail'); + const warnings = Array.isArray(payload.warnings) ? payload.warnings : []; + expect( + warnings.filter((w) => typeof w === 'object' && w !== null && 'conversionId' in (w as object)), + 'a conversion notice appeared inside `warnings` — the two fields were folded', + ).toEqual([]); + expect(conversionsOf(payload)).toHaveLength(1); + }, 180_000); +}); + +// ── Exhaustiveness, read off the source ───────────────────────────────────── + +/** + * Every `emitJson` payload literal in a command file, extracted by brace + * matching from the `{` that opens the first argument. `${…}` inside a template + * literal is balanced, so the depth arithmetic survives a payload built from + * one. (Same extractor as the `warnings` pins one field over.) + */ +function payloadLiterals(src: string): string[] { + const out: string[] = []; + const NEEDLE = 'await emitJson('; + let i = src.indexOf(NEEDLE); + while (i !== -1) { + const open = src.indexOf('{', i + NEEDLE.length); + if (open === -1) break; + let depth = 0; + let j = open; + for (; j < src.length; j++) { + if (src[j] === '{') depth++; + else if (src[j] === '}') { + depth--; + if (depth === 0) break; + } + } + out.push(src.slice(open, j + 1)); + i = src.indexOf(NEEDLE, j); + } + return out; +} + +describe('#12125 — the contract is exhaustive over `compile.ts`, not just over the exits pinned above', () => { + const SRC = readFileSync(COMPILE_TS, 'utf8'); + + it('the extractor produces a POSITIVE before its negative is trusted', () => { + const SYNTHETIC = [ + "await emitJson({ success: false, errors }, 0, { compact: true });", + "await emitJson({ success: false, error: `x: ${e.message}`, conversions: conversionNotices }, 0, { compact: true });", + ].join('\n'); + const found = payloadLiterals(SYNTHETIC); + expect(found).toHaveLength(2); + expect(found.filter((p) => !p.includes('conversions:'))).toHaveLength(1); + expect(found[1]).toContain('conversions: conversionNotices'); + }); + + it('all 10 `emitJson` exits carry `conversions` — 9 failure exits and the success payload', () => { + const literals = payloadLiterals(SRC); + expect(literals, 'the `emitJson` exit count moved — a new exit must carry `conversions` too').toHaveLength(10); + expect(literals.filter((p) => p.includes('success: false'))).toHaveLength(9); + expect(literals.filter((p) => p.includes('success: true'))).toHaveLength(1); + + const bare = literals.filter((p) => !p.includes('conversions:')); + expect( + bare, + 'an `os build --json` exit publishes no `conversions`, so a consumer cannot tell ' + + '"this tree converts nothing" from "this run stopped early" through it (#12125)', + ).toEqual([]); + }); + + it('every exit reads the ONE sink, and the sink outlives the `try`', () => { + expect(SRC).toMatch(/const conversionNotices: ConversionNotice\[\] = \[\];\s*\n\s*\n\s*try \{/); + for (const literal of payloadLiterals(SRC)) { + expect(literal, 'an exit spells its own conversion list instead of reading the shared sink').toContain( + 'conversions: conversionNotices', + ); + } + }); + + it('⛔ the conversion layer still runs at step 2 — carrying, not computing', () => { + const declAt = SRC.indexOf('const conversionNotices: ConversionNotice[] = [];'); + const tryAt = SRC.indexOf('\n try {'); + const loadAt = SRC.indexOf('await loadConfig('); + const normalizeAt = SRC.indexOf('normalizeStackInput('); + expect(declAt, 'the sink declaration was not found').toBeGreaterThan(-1); + expect(declAt, 'the sink must be declared ABOVE the `try` so `catch` can read it').toBeLessThan(tryAt); + expect(normalizeAt, 'the normalize call must stay INSIDE the `try`').toBeGreaterThan(tryAt); + expect( + normalizeAt, + 'the normalize call moved above `loadConfig` — that is computing earlier, not carrying', + ).toBeGreaterThan(loadAt); + }); +}); diff --git a/packages/cli/test/validate-json-failure-conversions.e2e.test.ts b/packages/cli/test/validate-json-failure-conversions.e2e.test.ts new file mode 100644 index 0000000000..0efa71d198 --- /dev/null +++ b/packages/cli/test/validate-json-failure-conversions.e2e.test.ts @@ -0,0 +1,449 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #12125 — `os validate --json`'s FAILURE payloads dropped the `conversions` + * field the run had ALREADY COMPUTED, on all five of its failure exits. + * + * The same "computed, then dropped on a failure exit" shape as the `warnings` + * family (#11643 / #11391 / #11772 / #12047), one FIELD over. Measured by the + * filer at `origin/main` 2ba4329, two runs over the same stack differing only + * in one field's `type`: + * + * converts + otherwise VALID → keys include `conversions`, one notice in it + * converts + parse error → keys are [duration, errors, valid, warnings] + * + * `conversionNotices` is filled by the `onConversionNotice` sink handed to + * `normalizeStackInput`, which runs at step 2 — ABOVE `safeParse` and above + * every later gate. So on each failure exit the notice was already in hand and + * was then discarded; `conversions` reached the terminal success payload alone. + * + * ## Why the dropped field matters more than its size suggests + * + * An auto-converted key is the one advisory class carrying an EXPIRY: + * `retiresIn` names the protocol major where the old shape stops loading. A CI + * job gating on `os validate --json` therefore could not see that the tree + * depends on a conversion about to retire until every unrelated failure in the + * tree was fixed first — the notice was withheld exactly while the tree was + * broken, which is when an author is most likely to be editing it. + * + * ## The ruling these pins encode + * + * Maintainer, 2026-08-25 on #11772/#12047, applied here to `conversions` under + * the same-family rule: every failure exit carries the lists the run has + * ALREADY COMPUTED, so the field means the same thing on every exit and a + * machine consumer has exactly one way to read it. + * + * ⛔ CARRYING, NOT COMPUTING. The fix is a pure SCOPE change — the sink array + * moved above the `try` so the catch-all can read it — and `normalizeStackInput` + * still runs at exactly step 2. The `throw at load` pin below is the half that + * holds that line: an exit ABOVE the computation reports `[]`, and if anyone + * hoists the normalize call to make that exit look fuller, it goes red. + * + * ## ⛔ WHAT THESE PINS DO NOT DECIDE + * + * Whether `warnings` and `conversions` should be FOLDED into one field is an + * open question (raised by the filer, not addressed by the ruling) and this + * card had no authority to settle it. `fields stay separate` below is a + * REGRESSION GUARD recording the shape as-shipped — green before and after this + * change — ⛔ not an argument that folding is wrong. If the fold is later + * decided, that pin is the one to revisit, deliberately. + * + * ## WHAT THESE PINS ASSERT — "what the run computed", not "the key exists" + * + * ⭐ A pin asserting `'conversions' in payload` passes against a `conversions: + * []` hard-coded at every exit — the defect with a lid on it. So every fixture + * here drives a LIVE conversion (`page-kind-jsx-to-html`, ADR-0087 D2, protocol + * 11, on `pages[0].kind: 'jsx'`) and each exit is asserted to carry exactly the + * notice the run had computed by then — no fewer, and NO MORE: + * + * exit | conversions + * ----------------------+--------------------------------- + * parse failure | the one notice + * rule errors | the one notice + * capability errors | the one notice + * doc errors | the one notice + * catch-all (late) | the one notice + * catch-all (at load) | [] — step 2 never ran + * success (control) | the one notice + * + * The "NO MORE" half is the array being asserted whole, so a second live + * conversion firing from a fixture would be caught rather than absorbed. + * + * ## Both directions of the instrument are proven + * + * `converts nothing` runs the SAME failure exit with `kind: 'html'` — already + * canonical, nothing to convert — and requires `[]`. Without it, a payload with + * the notice hard-coded in would satisfy every assertion above; with it, the + * field is shown to track what the run actually computed. That is the negative + * whose positive is every other test in this file. + * + * ## Why no `dist/` sits on the measured path + * + * These run the CLI through `bin/run-dev.js`, "the SOURCE entry point — same + * CLI, run from `src/` through tsx". `validate.ts` is loaded from source by the + * child, so an ablation of that file is measured without a rebuild. Its + * DEPENDENCY `@objectstack/spec` — which owns the conversion itself — does + * resolve through `exports` to `dist/`, and this change does not touch it. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync, mkdirSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); +const VALIDATE_TS = resolve(HERE, '../src/commands/validate.ts'); + +/** The live ADR-0087 D2 conversion these fixtures drive (protocol 11). */ +const CONVERSION_ID = 'page-kind-jsx-to-html'; +/** The FATAL capability token — `{package: null, edition: 'cloud'}` in the spec + * registry, so it classifies `unavailable` whatever is installed. */ +const FATAL_TOKEN = 'governance'; + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...args], + { cwd, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) }, + (err, stdout, stderr) => { + resolvePromise({ + code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +function payloadOf(run: Run, label: string): Record { + try { + return JSON.parse(run.stdout) as Record; + } catch { + throw new Error(`${label}: stdout was not one JSON document (exit ${run.code})\n${run.stdout}\n${run.stderr}`); + } +} + +/** + * A stack whose `pages[0].kind` drives the live conversion. `pageKind` is a + * parameter so the negative control can run the identical shape with the + * CANONICAL spelling, where there is nothing to convert. + */ +function stack(ns: string, opts: { pageKind?: string; requires?: string[]; extraFields?: string } = {}): string { + const { pageKind = 'jsx', requires = [], extraFields = '' } = opts; + return ` +export default { + manifest: { id: 'com.example.${ns}', name: '${ns}', version: '1.0.0', type: 'app', namespace: '${ns}' }, + requires: [${requires.map((r) => `'${r}'`).join(', ')}], + pages: [{ name: 'landing', label: 'Landing', kind: '${pageKind}', source: 'hi' }], + objects: [ + { + name: '${ns}_ticket', + label: 'Ticket', + sharingModel: 'private', + fields: { + title: { type: 'text', label: 'Title' },${extraFields} + }, + }, + ], +}; +`; +} + +const DOC_PLAIN = `--- +title: Wrong namespace +--- + +Body text. +`; + +/** The `conversions` field, as a list, whatever the payload shipped. */ +function conversionsOf(payload: Record): unknown[] { + return Array.isArray(payload.conversions) ? (payload.conversions as unknown[]) : []; +} + +/** + * The one notice this fixture family must produce, asserted by IDENTITY — + * `conversionId` plus the converted path — rather than by arity alone, so a + * different conversion firing could not satisfy it. + */ +const THE_NOTICE = { + conversionId: CONVERSION_ID, + surface: 'page.kind', + from: 'jsx', + to: 'html', + path: 'pages[0].kind', +}; + +/** + * Asserts the payload carries EXACTLY the one computed notice. `toEqual` over + * the whole array is the "and NO MORE" half. + */ +function expectTheOneNotice(payload: Record, label: string): void { + expect(conversionsOf(payload), `${label}: expected exactly the one computed conversion notice`).toEqual([ + expect.objectContaining(THE_NOTICE), + ]); +} + +const dirs: Record = {}; +let root = ''; + +beforeAll(() => { + root = mkdtempSync(join(tmpdir(), 'os-validate-fail-conversions-')); + const make = (name: string, config: string, docs: Array<[string, string]> = []): string => { + const dir = join(root, name); + mkdirSync(dir, { recursive: true }); + if (docs.length > 0) mkdirSync(join(dir, 'src', 'docs'), { recursive: true }); + writeFileSync(join(dir, 'objectstack.config.ts'), config); + for (const [file, body] of docs) writeFileSync(join(dir, 'src', 'docs', file), body); + dirs[name] = dir; + return dir; + }; + + // parse failure — a second field whose `type` is not a field type. The + // conversion runs at step 2, above `safeParse`. + make('parsefail', stack('pfail', { extraFields: ` + broken: { type: 'this_is_not_a_field_type', label: 'Broken' },` })); + + // ⭐ the negative control — the SAME exit, the CANONICAL page kind. Nothing + // to convert, so `conversions` must be empty. + make('parsefail_nc', stack('pnc', { pageKind: 'html', extraFields: ` + broken: { type: 'this_is_not_a_field_type', label: 'Broken' },` })); + + // rule errors — an expression naming a field that does not resolve. + make('rulefail', stack('rfail', { extraFields: ` + subject: { type: 'text', label: 'Subject', visibleWhen: { dialect: 'cel', source: 'record.zzz_no_such_field' } },` })); + + // capability errors — the FATAL token. + make('capfail', stack('cfail', { requires: [FATAL_TOKEN] })); + + // doc errors — a doc whose name carries no namespace prefix. + make('docsfail', stack('dfail'), [['otherns_guide.md', DOC_PLAIN]]); + + // catch-all, LATE — `src/docs` is a FILE, so `readdirSync` raises ENOTDIR + // inside `collectAndLintDocs`, well below step 2. + const thrown = make('thrown', stack('tfail')); + mkdirSync(join(thrown, 'src'), { recursive: true }); + writeFileSync(join(thrown, 'src', 'docs'), 'not a directory\n'); + + // catch-all, AT LOAD — the config throws on import, ABOVE step 2. + make('earlythrow', ` +throw new Error('zzz_config_module_threw'); +export default {}; +`); + + // The control — the same shape, reaching SUCCESS. + make('control', stack('ctrl')); +}); + +afterAll(() => { + if (root) rmSync(root, { recursive: true, force: true }); +}); + +describe('#12125 — every `os validate --json` failure exit carries the conversions the run computed', () => { + it('control — the fixture shape DOES drive a live conversion on the success exit', async () => { + // ⭐ Anti-vacuity for every assertion below: without this, a fixture that + // never converted anything would satisfy the failure-exit pins for the + // wrong reason. This is also the run the filer measured. + const run = await runCli(['validate', '--json'], dirs.control); + expect(run.code, `expected the control to pass:\n${run.stdout}${run.stderr}`).toBe(0); + const payload = payloadOf(run, 'control'); + expect(payload.valid).toBe(true); + expectTheOneNotice(payload, 'control'); + // The expiry is the reason this field cannot just be dropped into prose. + expect(typeof (conversionsOf(payload)[0] as { retiresIn?: unknown }).retiresIn).toBe('number'); + }, 120_000); + + it('parse failure — THE HEADLINE: the notice computed at step 2 survives the schema error', async () => { + const run = await runCli(['validate', '--json'], dirs.parsefail); + expect(run.code, `expected the parse to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'parsefail'); + expect(payload.valid).toBe(false); + expect(Array.isArray(payload.errors), 'the parse exit reports under `errors`').toBe(true); + expectTheOneNotice(payload, 'parsefail'); + }, 120_000); + + it('⭐ converts nothing — the SAME exit reports `[]`, so the field tracks the run', async () => { + // The instrument's negative, whose positive is every other test here. A + // `conversions: [THE_NOTICE]` hard-coded at the exits would pass all of + // them and fail this one. + const run = await runCli(['validate', '--json'], dirs.parsefail_nc); + expect(run.code, `expected the parse to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'parsefail_nc'); + expect(payload.valid).toBe(false); + expect('conversions' in payload, 'the field must be PRESENT even when empty').toBe(true); + expect(payload.conversions, 'a canonical page kind converts nothing').toEqual([]); + }, 120_000); + + it('rule errors — the notice rides the author-time gate', async () => { + const run = await runCli(['validate', '--json'], dirs.rulefail); + expect(run.code, `expected the rule gate to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'rulefail'); + expect(payload.valid).toBe(false); + expect((payload.errors as Array<{ rule: string }>).map((i) => i.rule)).toContain('expression-invalid'); + expectTheOneNotice(payload, 'rulefail'); + }, 120_000); + + it('capability errors — the notice rides the #3366 preflight gate', async () => { + const run = await runCli(['validate', '--json'], dirs.capfail); + expect(run.code, `expected the capability gate to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'capfail'); + expect(payload.valid).toBe(false); + expect((payload.errors as Array<{ token: string }>).map((i) => i.token)).toEqual([FATAL_TOKEN]); + expectTheOneNotice(payload, 'capfail'); + }, 120_000); + + it('doc errors — the notice rides the ADR-0046 docs gate', async () => { + const run = await runCli(['validate', '--json'], dirs.docsfail); + expect(run.code, `expected the docs gate to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'docsfail'); + expect(payload.valid).toBe(false); + expect((payload.errors as Array<{ rule: string }>).map((i) => i.rule)).toEqual(['docs/namespace-prefix']); + expectTheOneNotice(payload, 'docsfail'); + }, 120_000); + + it('catch-all (late throw) — a THROWN failure still reports the computed notice', async () => { + // Before this change the sink was declared INSIDE the `try`, so the + // catch-all could not read it at all — the field was structurally + // unreachable from this exit rather than merely omitted. + const run = await runCli(['validate', '--json'], dirs.thrown); + expect(run.code, `expected the docs read to throw:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'thrown'); + expect(String(payload.error)).toContain('ENOTDIR'); + expectTheOneNotice(payload, 'thrown'); + }, 120_000); + + it('catch-all (throw at load) — `conversions` is PRESENT and empty, because step 2 never ran', async () => { + // ⛔ The line that holds "carrying, not computing": this exit is ABOVE the + // normalize call, so `[]` is the honest reading. Hoisting the computation + // up to make it look fuller turns this red. + // + // ⛔ Empty here is NOT "this tree converts nothing" — it is "this run + // stopped before the conversion layer ran", and that distinction is what + // the changeset tells consumers. + const run = await runCli(['validate', '--json'], dirs.earlythrow); + expect(run.code, `expected the config module to throw:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'earlythrow'); + expect(String(payload.error)).toContain('zzz_config_module_threw'); + expect('conversions' in payload, 'the load-failure exit omits `conversions` entirely').toBe(true); + expect(payload.conversions).toEqual([]); + }, 120_000); + + it('REGRESSION GUARD — `warnings` and `conversions` stay separate fields', async () => { + // ⛔ NOT an argument that folding is wrong. Whether the two should become + // one field is an OPEN question this card was not given authority to + // settle; this records the shape as-shipped so a fold happens deliberately + // rather than as a side effect. Green both before and after #12125. + const run = await runCli(['validate', '--json'], dirs.docsfail); + const payload = payloadOf(run, 'docsfail'); + const warnings = Array.isArray(payload.warnings) ? payload.warnings : []; + expect( + warnings.filter((w) => typeof w === 'object' && w !== null && 'conversionId' in (w as object)), + 'a conversion notice appeared inside `warnings` — the two fields were folded', + ).toEqual([]); + expect(conversionsOf(payload)).toHaveLength(1); + }, 120_000); +}); + +// ── Exhaustiveness, read off the source ───────────────────────────────────── + +/** + * Every `emitJson` payload literal in a command file, extracted by brace + * matching from the `{` that opens the first argument. `${…}` inside a template + * literal is balanced, so the depth arithmetic survives a payload built from + * one. (Same extractor as the `warnings` pins one field over.) + */ +function payloadLiterals(src: string): string[] { + const out: string[] = []; + const NEEDLE = 'await emitJson('; + let i = src.indexOf(NEEDLE); + while (i !== -1) { + const open = src.indexOf('{', i + NEEDLE.length); + if (open === -1) break; + let depth = 0; + let j = open; + for (; j < src.length; j++) { + if (src[j] === '{') depth++; + else if (src[j] === '}') { + depth--; + if (depth === 0) break; + } + } + out.push(src.slice(open, j + 1)); + i = src.indexOf(NEEDLE, j); + } + return out; +} + +describe('#12125 — the contract is exhaustive over `validate.ts`, not just over the exits pinned above', () => { + const SRC = readFileSync(VALIDATE_TS, 'utf8'); + + it('the extractor produces a POSITIVE before its negative is trusted', () => { + // ⭐ A "no payload lacks `conversions`" pass is worthless from an instrument + // that finds no payloads, or that cannot see a missing key. Both halves are + // demonstrated on synthetic input first. + const SYNTHETIC = [ + "await emitJson({ valid: false, errors, duration: timer.elapsed() });", + "await emitJson({ valid: false, error: `x: ${e.message}`, conversions: conversionNotices });", + ].join('\n'); + const found = payloadLiterals(SYNTHETIC); + expect(found).toHaveLength(2); + expect(found.filter((p) => !p.includes('conversions:'))).toHaveLength(1); + // …and the template literal's `${…}` did not break the brace matching. + expect(found[1]).toContain('conversions: conversionNotices'); + }); + + it('all 6 `emitJson` exits carry `conversions` — 5 failure exits and the success payload', () => { + const literals = payloadLiterals(SRC); + expect(literals, 'the `emitJson` exit count moved — a new exit must carry `conversions` too').toHaveLength(6); + expect(literals.filter((p) => p.includes('valid: false'))).toHaveLength(5); + expect(literals.filter((p) => p.includes('valid: true'))).toHaveLength(1); + + const bare = literals.filter((p) => !p.includes('conversions:')); + expect( + bare, + 'an `os validate --json` exit publishes no `conversions`, so a consumer cannot tell ' + + '"this tree converts nothing" from "this run stopped early" through it (#12125)', + ).toEqual([]); + }); + + it('every exit reads the ONE sink, and the sink outlives the `try`', () => { + // The scope change is the whole fix for the catch-all exit: a sink declared + // inside the `try` is unreachable from `catch`, so that exit could not have + // carried the field however the payload was written. + expect(SRC).toMatch(/const conversionNotices: ConversionNotice\[\] = \[\];\s*\n\s*\n\s*try \{/); + for (const literal of payloadLiterals(SRC)) { + expect(literal, 'an exit spells its own conversion list instead of reading the shared sink').toContain( + 'conversions: conversionNotices', + ); + } + }); + + it('⛔ the conversion layer still runs at step 2 — carrying, not computing', () => { + // The static half of the `throw at load` pin: the sink is DECLARED above + // the `try`, but the call that fills it stays inside, below `loadConfig`. + const declAt = SRC.indexOf('const conversionNotices: ConversionNotice[] = [];'); + const tryAt = SRC.indexOf('\n try {'); + const loadAt = SRC.indexOf('await loadConfig('); + const normalizeAt = SRC.indexOf('normalizeStackInput('); + expect(declAt, 'the sink declaration was not found').toBeGreaterThan(-1); + expect(declAt, 'the sink must be declared ABOVE the `try` so `catch` can read it').toBeLessThan(tryAt); + expect(normalizeAt, 'the normalize call must stay INSIDE the `try`').toBeGreaterThan(tryAt); + expect( + normalizeAt, + 'the normalize call moved above `loadConfig` — that is computing earlier, not carrying', + ).toBeGreaterThan(loadAt); + }); +}); From bce8ef4654fdea694f0e8127671d78a77c2076fa Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 16:33:38 +0000 Subject: [PATCH 2/3] test(cli): drive the conversion fixtures with single-root JSX source The card measured its fixture with `source: 'hi'`, which no longer reaches the success exit at current `main`: the `jsx-no-root` authoring rule rejects it ("Expected a single root element"), so the control fixture stopped at the rule gate instead of proving the conversion fires on a passing run. `
hi
` is the same source the ADR-0087 conversion registry uses in its own fixture for `page-kind-jsx-to-html`. Part of #12125 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HbG3rGVLjZStHQxHDtzJdJ --- packages/cli/test/build-json-failure-conversions.e2e.test.ts | 2 +- packages/cli/test/validate-json-failure-conversions.e2e.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/test/build-json-failure-conversions.e2e.test.ts b/packages/cli/test/build-json-failure-conversions.e2e.test.ts index dde9350886..6792329071 100644 --- a/packages/cli/test/build-json-failure-conversions.e2e.test.ts +++ b/packages/cli/test/build-json-failure-conversions.e2e.test.ts @@ -118,7 +118,7 @@ function stack(ns: string, opts: { pageKind?: string; requires?: string[]; extra export default { manifest: { id: 'com.example.${ns}', name: '${ns}', version: '1.0.0', type: 'app', namespace: '${ns}' }, requires: [${requires.map((r) => `'${r}'`).join(', ')}], - pages: [{ name: 'landing', label: 'Landing', kind: '${pageKind}', source: 'hi' }], + pages: [{ name: 'landing', label: 'Landing', kind: '${pageKind}', source: '
hi
' }], objects: [ { name: '${ns}_ticket', diff --git a/packages/cli/test/validate-json-failure-conversions.e2e.test.ts b/packages/cli/test/validate-json-failure-conversions.e2e.test.ts index 0efa71d198..a2ae61a394 100644 --- a/packages/cli/test/validate-json-failure-conversions.e2e.test.ts +++ b/packages/cli/test/validate-json-failure-conversions.e2e.test.ts @@ -147,7 +147,7 @@ function stack(ns: string, opts: { pageKind?: string; requires?: string[]; extra export default { manifest: { id: 'com.example.${ns}', name: '${ns}', version: '1.0.0', type: 'app', namespace: '${ns}' }, requires: [${requires.map((r) => `'${r}'`).join(', ')}], - pages: [{ name: 'landing', label: 'Landing', kind: '${pageKind}', source: 'hi' }], + pages: [{ name: 'landing', label: 'Landing', kind: '${pageKind}', source: '
hi
' }], objects: [ { name: '${ns}_ticket', From 2c7f00270803deb48abba3d56725447cd9568d07 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 16:46:12 +0000 Subject: [PATCH 3/3] test(cli): make the fold regression guard green in BOTH states, as its name claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard also asserted that the failure payload carries the one notice, which is red before the fix — so a pin named REGRESSION GUARD was in fact red-before evidence. Measured in the ablated tree: it failed for that assertion alone. That claim is the `doc errors` / `3f (package docs)` test's job, on this very fixture. The guard now asserts the FOLD property and nothing else. Part of #12125 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HbG3rGVLjZStHQxHDtzJdJ --- .../cli/test/build-json-failure-conversions.e2e.test.ts | 7 ++++++- .../cli/test/validate-json-failure-conversions.e2e.test.ts | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/cli/test/build-json-failure-conversions.e2e.test.ts b/packages/cli/test/build-json-failure-conversions.e2e.test.ts index 6792329071..32482c63e1 100644 --- a/packages/cli/test/build-json-failure-conversions.e2e.test.ts +++ b/packages/cli/test/build-json-failure-conversions.e2e.test.ts @@ -367,7 +367,12 @@ describe('#12125 — every `os build --json` failure exit carries the conversion warnings.filter((w) => typeof w === 'object' && w !== null && 'conversionId' in (w as object)), 'a conversion notice appeared inside `warnings` — the two fields were folded', ).toEqual([]); - expect(conversionsOf(payload)).toHaveLength(1); + // ⛔ Deliberately asserts the FOLD property and nothing else, so this pin is + // green in BOTH states — that is what makes it a regression guard rather + // than evidence for this change. "the payload carries the one notice" is + // the `3f (package docs)` test's job, on this very fixture; asserting it + // here too made this pin red-before and its name a lie. (Measured: it + // failed in the ablated tree for that reason alone.) }, 180_000); }); diff --git a/packages/cli/test/validate-json-failure-conversions.e2e.test.ts b/packages/cli/test/validate-json-failure-conversions.e2e.test.ts index a2ae61a394..90e760ff95 100644 --- a/packages/cli/test/validate-json-failure-conversions.e2e.test.ts +++ b/packages/cli/test/validate-json-failure-conversions.e2e.test.ts @@ -353,7 +353,12 @@ describe('#12125 — every `os validate --json` failure exit carries the convers warnings.filter((w) => typeof w === 'object' && w !== null && 'conversionId' in (w as object)), 'a conversion notice appeared inside `warnings` — the two fields were folded', ).toEqual([]); - expect(conversionsOf(payload)).toHaveLength(1); + // ⛔ Deliberately asserts the FOLD property and nothing else, so this pin is + // green in BOTH states — that is what makes it a regression guard rather + // than evidence for this change. "the payload carries the one notice" is + // the `doc errors` test's job, on this very fixture; asserting it here too + // made this pin red-before and its name a lie. (Measured: it failed in the + // ablated tree for that reason alone.) }, 120_000); });