From 8c7bf124940846679614c7bbb094b0706d7ffde2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 06:51:33 +0000 Subject: [PATCH 1/2] feat(cli): carry the computed advisory lists on every `os build --json` failure exit (#11772) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `warnings` lived on the terminal success payload only (plus `ruleAdvisories` alone on the author-time-rules failure), while the text face prints its advisory blocks — the #11529 author-time advisories at 3b and the #3786 undeclared authoring-key findings at 3d — several gates earlier, each ending in `— re-run with --json for the full list`. A build that then failed at a later gate emitted that gate's failure payload, and none of those carried the list: the remedy the notice named returned a payload without the withheld entries in it (the #11643 / #11391 "the remedy named is unreachable" shape). Maintainer ruling 2026-08-25, option 1 of three: every `emitJson` failure exit carries the lists the run has ALREADY COMPUTED, so `warnings` means the same thing on every exit. Option 2 (shape depends on how far the run got) and option 3 (weaken the pointer) were both rejected. Nine failure exits, three more than the filing card's table listed — it missed the protocol-parse exit, the `--no-runtime-bundle` refusal and the bottom catch-all. Every one now reads a single `warningsSoFar()` site, which the success payload reads too, so the member order (`os validate --json`'s, minus its trailing `structuralWarnings`) cannot drift between exits. Carrying, not computing: each list stays computed at the step that owns it, so an exit upstream of a step reports that list empty rather than paying for a computation it had not already done. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --- .../build-json-failure-payload-warnings.md | 79 +++ packages/cli/src/commands/compile.ts | 103 +++- .../build-json-failure-warnings.e2e.test.ts | 531 ++++++++++++++++++ .../test/truncation-remainder-notices.test.ts | 11 +- 4 files changed, 695 insertions(+), 29 deletions(-) create mode 100644 .changeset/build-json-failure-payload-warnings.md create mode 100644 packages/cli/test/build-json-failure-warnings.e2e.test.ts diff --git a/.changeset/build-json-failure-payload-warnings.md b/.changeset/build-json-failure-payload-warnings.md new file mode 100644 index 0000000000..fa6a2b8195 --- /dev/null +++ b/.changeset/build-json-failure-payload-warnings.md @@ -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. diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index d8e8bbca45..d7914d7aac 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -16,10 +16,10 @@ import { import { loadConfig } from '../utils/config.js'; import { lowerCallables } from '../utils/lower-callables.js'; import { buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint'; -import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '@objectstack/lint'; +import { runAuthoringRules, splitBySeverity, authoringRulesFor, type AuthoringFinding } 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, @@ -83,6 +83,53 @@ 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. + let ruleAdvisories: AuthoringFinding[] = []; + 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...'); @@ -141,7 +188,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(''); @@ -196,7 +243,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(''); @@ -223,7 +270,8 @@ export default class Compile extends Command { parsed: result.data as Record, 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(''); @@ -237,7 +285,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 }, ); @@ -277,7 +325,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), })); @@ -287,6 +335,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); } @@ -322,7 +371,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, ObjectStackDefinitionSchema), ...lintUnknownAuthoringKeys(normalized as Record, ObjectStackDefinitionSchema), ].map(formatUnknownAuthoringKey); @@ -334,18 +383,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, @@ -382,7 +431,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(''); @@ -423,10 +472,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(''); @@ -480,7 +529,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(''); @@ -504,7 +553,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(''); @@ -584,7 +633,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 @@ -642,7 +691,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(''); diff --git a/packages/cli/test/build-json-failure-warnings.e2e.test.ts b/packages/cli/test/build-json-failure-warnings.e2e.test.ts new file mode 100644 index 0000000000..a59ad6c615 --- /dev/null +++ b/packages/cli/test/build-json-failure-warnings.e2e.test.ts @@ -0,0 +1,531 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #11772 — `os build --json`'s FAILURE payloads carried no `warnings`, so the + * text face's `— re-run with --json for the full list` pointer was a dead end + * whenever a later gate failed. + * + * The text face prints its advisory blocks well before the gates that can stop + * the run: the #11529 author-time advisories at 3b, the #3786 undeclared-key + * findings at 3d — both ending in `JSON_FULL_LIST_REMEDY`. `warnings` then + * lived on the TERMINAL SUCCESS payload only (plus, for one list, the + * author-time-rules failure). So 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",…} + * ^ the 60 keys nowhere + * + * The remedy the notice named returned a payload that did not contain the + * list — the "the remedy named is unreachable" shape of #11643 and #11391. + * The author could not see the withheld entries by any route until an + * UNRELATED later failure was fixed. + * + * ## The ruling these pins encode + * + * Maintainer, 2026-08-25, option 1 of the three the card offered: every + * `emitJson` failure exit carries the advisory lists the run has ALREADY + * COMPUTED, so `warnings` means the same thing on every exit. 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. + * + * ## WHAT THESE PINS ASSERT — "what the run computed", not "the key exists" + * + * A pin that only asserted `'warnings' in payload` would pass against a + * `warnings: []` hard-coded at every exit, which is the defect with a lid on + * it. So each exit below is driven by a fixture carrying a KNOWN advisory of + * each class, and the payload is asserted to carry exactly the classes the run + * had reached — no fewer, and NO MORE: + * + * exit (step) | rule | doc | key | cap + * -----------------------------------+------+-----+-----+----- + * strict-body (2b) | - | - | - | - + * protocol parse (3) | - | - | - | - + * author-time rules (3b) | ✓ | - | - | - + * capability preflight (3c) | ✓ | - | - | ✓ + * access-matrix drift (3e) | ✓ | - | ✓ | ✓ + * package docs (3f) | ✓ | ✓ | ✓ | ✓ + * --no-runtime-bundle (4b) | ✓ | ✓ | ✓ | ✓ + * thrown / caught (bottom) | ✓ | ✓ | ✓ | ✓ + * + * The "NO MORE" half is what tells option 1 apart from a change that hoisted + * the COMPUTATIONS earlier to make every exit look full — which would be + * option 2 wearing option 1's clothes, and would also change what the command + * costs on its failure paths. The 3c and 3e rows are where that is measured: + * a doc advisory appearing at 3c, or at 3e, means a computation moved. + * + * The 3b row is a REGRESSION GUARD, not evidence: that exit already published + * `warnings: ruleAdvisories` before this change, and at that point in the run + * `warningsSoFar()` is exactly `[...ruleAdvisories]`. It is pinned so that the + * refactor to a shared ordering site cannot quietly drop it. + * + * Detection is STRUCTURAL — a capability hint is a record carrying `token`, a + * doc advisory a record whose `rule` is namespaced `docs/`, an undeclared-key + * finding the formatted STRING, an authoring-rule advisory the remaining + * record. Deliberately not a substring of the planted prose: a check spelled + * as a fragment of the term under test can match for reasons that have nothing + * to do with the behaviour, in both directions. + * + * ## Why no `dist/` sits on the measured path + * + * These run the CLI through `bin/run-dev.js`, whose own header states it is + * "the SOURCE entry point — same CLI, run from `src/` through tsx, used by + * this repo's gates and e2e suites so they do not depend on + * `packages/cli/dist` having been built". `compile.ts` is therefore loaded + * from source by the child, and an ablation of that file is measured without + * a rebuild. (Its DEPENDENCIES — `@objectstack/spec`, `@objectstack/lint` — + * do resolve through `exports` to their `dist/`, but this change touches + * neither.) + */ + +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'); + +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}`); + } +} + +/** The planted capability token — classified `unknown`, i.e. an ADVISORY. */ +const PLANTED_TOKEN = 'zzz_unknown_capability_token'; +/** + * The planted FATAL capability token. `governance` is `{package: null, edition: + * 'cloud'}` in the spec registry, so it classifies `unavailable` no matter what + * is installed — the 3c failure exit without depending on the environment. + */ +const FATAL_TOKEN = 'governance'; +/** The planted undeclared authoring key. Matched by name, never as a fragment. */ +const PLANTED_KEY = 'zzzUndeclaredProbeKey'; + +/** + * A stack raising ONE advisory of each of the four classes while parsing + * cleanly: a bare `unique: true` index (authoring-RULE advisory), an unknown + * `requires` token (#3366 capability hint), an undeclared key inside + * `visibleWhen` (#3786 finding), plus — via `plantDocs` — a doc whose `tags:` + * scalar the reader cannot parse (ADR-0046 doc advisory). + * + * The key sits inside `visibleWhen` rather than on the object or field itself: + * an undeclared key in either of those positions has been a hard PARSE error + * since #4001, which would stop the run at 3 and never reach the gate under + * test. + */ +function stack(ns: string, requires: string[], extra = ''): string { + return ` +export default { + manifest: { id: 'com.example.${ns}', name: '${ns}', version: '1.0.0', type: 'app', namespace: '${ns}' }, + requires: [${requires.map((r) => `'${r}'`).join(', ')}], + objects: [ + { + name: '${ns}_ticket', + label: 'Ticket', + sharingModel: 'private', + indexes: [{ name: '${ns}_title_idx', fields: ['title'], unique: true }], + fields: { + title: { + type: 'text', + label: 'Title', + visibleWhen: { dialect: 'cel', source: 'true', ${PLANTED_KEY}: 1 }, + }, + }, + }, + ],${extra} +}; +`; +} + +/** A hook whose body cannot be lowered — `require()` is refused (#10678). */ +const UNLOWERABLE_HOOK = ` + hooks: [{ + name: 'lf_hook', + object: 'lfail_ticket', + events: ['beforeInsert'], + handler: async (ctx: any) => { + const os = require('node:os'); + return os.platform(); + }, + }],`; + +const DOC_BAD_TAGS = `--- +title: Guide +tags: not-a-list +--- + +Body text. +`; + +const DOC_PLAIN = `--- +title: Wrong namespace +--- + +Body text. +`; + +// ── Structural classifiers over one `warnings` list ───────────────────────── + +function asList(warnings: unknown): unknown[] { + return Array.isArray(warnings) ? warnings : []; +} +/** #3366 capability hints: records carrying a `token`. */ +function capHints(warnings: unknown): Array> { + return asList(warnings).filter( + (w): w is Record => typeof w === 'object' && w !== null && 'token' in w, + ); +} +/** ADR-0046 doc advisories: records whose `rule` is namespaced `docs/`. */ +function docAdvisories(warnings: unknown): Array> { + return asList(warnings).filter( + (w): w is Record => + typeof w === 'object' && w !== null && + typeof (w as { rule?: unknown }).rule === 'string' && + ((w as { rule: string }).rule).startsWith('docs/'), + ); +} +/** #3786 undeclared-key findings: the formatted STRINGS. */ +function keyFindings(warnings: unknown): string[] { + return asList(warnings).filter((w): w is string => typeof w === 'string'); +} +/** Author-time RULE advisories: the records that are neither of the above. */ +function ruleAdvisories(warnings: unknown): Array> { + const cap = new Set(capHints(warnings)); + const doc = new Set(docAdvisories(warnings)); + return asList(warnings).filter( + (w): w is Record => + typeof w === 'object' && w !== null && !cap.has(w) && !doc.has(w), + ); +} + +/** The four class counts, as one comparable tuple. */ +function classes(warnings: unknown): Record { + return { + rule: ruleAdvisories(warnings).length, + doc: docAdvisories(warnings).length, + key: keyFindings(warnings).length, + cap: capHints(warnings).length, + }; +} + +const dirs: Record = {}; +let root = ''; + +beforeAll(() => { + root = mkdtempSync(join(tmpdir(), 'os-fail-warnings-')); + 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`, before any advisory has been computed. + make('strictbody', ` +export default { + manifest: { id: 'com.example.sbody', name: 'sbody', version: '1.0.0', type: 'app', namespace: 'sbody' }, + requires: ['${PLANTED_TOKEN}'], + objects: [{ name: 'sb_ticket', label: 'Ticket', sharingModel: 'private', + fields: { title: { type: 'text', label: 'Title' } } }], + hooks: [{ name: 'sb_hook', object: 'sb_ticket', events: ['beforeInsert'], + handler: async (ctx: any) => { const os = require('node:os'); return os.platform(); } }], +}; +`); + + // 3 — the protocol parse itself fails, likewise before any advisory. + make('zodfail', ` +export default { + manifest: { id: 'com.example.zfail', name: 'zfail', version: '1.0.0', type: 'app', namespace: 'zfail' }, + requires: ['${PLANTED_TOKEN}'], + objects: [{ name: 'zf_ticket', label: 'Ticket', sharingModel: 'private', + fields: { title: { type: 'this_is_not_a_field_type', label: 'Title' } } }], +}; +`); + + // 3b — an author-time rule ERROR (a `record.` that does not resolve), + // raised alongside the bare-`unique` advisory. + make('rulefail', ` +export default { + manifest: { id: 'com.example.rfail', name: 'rfail', version: '1.0.0', type: 'app', namespace: 'rfail' }, + requires: ['${PLANTED_TOKEN}'], + objects: [{ name: 'rf_ticket', label: 'Ticket', sharingModel: 'private', + indexes: [{ name: 'rf_title_idx', fields: ['title'], unique: true }], + fields: { title: { type: 'text', label: 'Title', + visibleWhen: { dialect: 'cel', source: 'record.zzz_no_such_field' } } } }], +}; +`); + + // 3c — one FATAL capability token beside the advisory one. + make('capfail', stack('capfail', [FATAL_TOKEN, PLANTED_TOKEN]), [['capfail_guide.md', DOC_BAD_TAGS]]); + + // 3e — a committed snapshot naming a permission set the stack does not grant. + const amx = make('amx', stack('amx', [PLANTED_TOKEN]), [['amx_guide.md', DOC_BAD_TAGS]]); + 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 — the card's headline: a doc ERROR (missing namespace prefix) reached + // with every one of the four advisory lists already computed. + make('docsfail', stack('dfail', [PLANTED_TOKEN]), [ + ['dfail_guide.md', DOC_BAD_TAGS], + ['otherns_guide.md', DOC_PLAIN], + ]); + + // 4b — `--no-runtime-bundle` over a callable that could not be lowered. + make('latefail', stack('lfail', [PLANTED_TOKEN], UNLOWERABLE_HOOK), [['lfail_guide.md', DOC_BAD_TAGS]]); + + // bottom — the artifact path is a DIRECTORY, so the write throws and the + // catch reports. Everything is computed by then. + const thrown = make('thrown', stack('tfail', [PLANTED_TOKEN]), [['tfail_guide.md', DOC_BAD_TAGS]]); + mkdirSync(join(thrown, 'out', 'artifact.json'), { recursive: true }); +}); + +afterAll(() => { + if (root) rmSync(root, { recursive: true, force: true }); +}); + +describe('#11772 — every `os build --json` failure exit carries the advisory lists the run computed', () => { + it('3f (package docs) — the card\'s headline: all four lists ride the failure payload', 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(payload.success).toBe(false); + expect(payload.error).toBe('docs validation failed'); + + // The gate's own findings are untouched by this change. + expect((payload.issues as Array<{ rule: string }>).map((i) => i.rule)).toEqual(['docs/namespace-prefix']); + + // …and the advisories the author was pointed at are now reachable HERE. + expect( + classes(payload.warnings), + 'the docs failure payload dropped an advisory list the run had already computed', + ).toEqual({ rule: 1, doc: 1, key: 1, cap: 1 }); + + // Identity, not just arity — this is the list the truncation notice names. + expect(keyFindings(payload.warnings)[0]).toContain(PLANTED_KEY); + expect(capHints(payload.warnings)[0]?.token).toBe(PLANTED_TOKEN); + expect(docAdvisories(payload.warnings)[0]?.rule).toBe('docs/frontmatter-tags'); + }, 120_000); + + it('3e (access-matrix drift) — carries the three lists computed by then and NOT the doc advisory', async () => { + const run = await runCli(['build', '--json'], dirs.amx); + expect(run.code, `expected the access-matrix gate to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'amx'); + expect(payload.error).toBe('access matrix drift'); + expect(payload.changes).toEqual(["'ghost_ps' loses ALL access to 'amx_ticket' (entry removed)"]); + + // The fixture SHIPS a doc that raises an advisory — and 3f has not run, so + // that advisory must NOT be here. This is the assertion that tells "carry + // what was computed" apart from "compute everything at every exit". + expect( + classes(payload.warnings), + 'the 3e payload carries a list the run had not computed yet — a computation moved', + ).toEqual({ rule: 1, doc: 0, key: 1, cap: 1 }); + expect(keyFindings(payload.warnings)[0]).toContain(PLANTED_KEY); + }, 120_000); + + it('3c (capability preflight) — carries rule + capability only; the 3d key finding is not computed yet', 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(payload.error).toBe('capability provider preflight failed'); + expect((payload.issues as Array<{ token: string }>).map((i) => i.token)).toEqual([FATAL_TOKEN]); + + // The fixture plants an undeclared key AND a doc advisory, neither of which + // 3c has reached. Only the fatal token is an `issue`; the advisory token + // rides `warnings`, which is the whole point of the two being separate. + expect( + classes(payload.warnings), + 'the 3c payload carries a list the run had not computed yet — a computation moved', + ).toEqual({ rule: 1, doc: 0, key: 0, cap: 1 }); + expect(capHints(payload.warnings)[0]?.token).toBe(PLANTED_TOKEN); + }, 120_000); + + it('4b (--no-runtime-bundle) — a late exit past every advisory step carries all four', async () => { + const run = await runCli(['build', '--json', '--no-runtime-bundle'], dirs.latefail); + expect(run.code, `expected the bundle gate to fail:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'latefail'); + expect(String(payload.error)).toContain('--no-runtime-bundle requires every callable to have a metadata body'); + expect(classes(payload.warnings)).toEqual({ rule: 1, doc: 1, key: 1, cap: 1 }); + }, 120_000); + + it('the catch-all — a THROWN failure still reports what the run had computed', async () => { + // The artifact path is a directory, so `writeFileSync` throws EISDIR and the + // bottom `catch` reports. Before this change that payload was `{success, + // error}` alone: an author whose build died on an unwritable artifact got + // the truncation notice's remedy and a payload with nothing in it. + const run = await runCli(['build', '--json', '-o', 'out/artifact.json'], dirs.thrown); + expect(run.code, `expected the write to throw:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'thrown'); + expect(String(payload.error)).toContain('EISDIR'); + expect( + classes(payload.warnings), + 'the catch-all payload dropped the lists the run had already computed', + ).toEqual({ rule: 1, doc: 1, key: 1, cap: 1 }); + }, 120_000); + + it('2b (--strict-body) — `warnings` is PRESENT and empty, because nothing is computed yet', async () => { + // The shape-constancy half of the ruling: the key is there on every exit, so + // a consumer has one way to read it. ⛔ Empty here is not "this tree is + // clean" — it is "this run stopped before any advisory was computed", and + // that distinction is what the changeset tells consumers. + const run = await runCli(['build', '--json', '--strict-body'], dirs.strictbody); + expect(run.code, `expected --strict-body to refuse:\n${run.stdout}${run.stderr}`).toBe(1); + const payload = payloadOf(run, 'strictbody'); + expect(payload.error).toBe('strict-body: missing body'); + expect('warnings' in payload, 'the strict-body exit omits `warnings` entirely').toBe(true); + expect(payload.warnings).toEqual([]); + }, 120_000); + + it('3 (protocol parse) — `warnings` is PRESENT and empty on the parse failure too', 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(Array.isArray(payload.errors), 'the parse exit reports under `errors`').toBe(true); + expect('warnings' in payload, 'the parse exit omits `warnings` entirely').toBe(true); + expect(payload.warnings).toEqual([]); + }, 120_000); + + it('3b (author-time rules) — REGRESSION GUARD: this exit already carried `ruleAdvisories`', async () => { + // Green before this change as well, and named as such: at 3b + // `warningsSoFar()` is exactly `[...ruleAdvisories]`, so the refactor to a + // single ordering site must leave this exit byte-identical. ⛔ Never read + // this one as evidence that the change does anything. + 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(payload.error).toBe('author-time rules failed'); + expect((payload.issues as Array<{ rule: string }>).map((i) => i.rule)).toEqual(['expression-invalid']); + + // `cap: 0` although the fixture DECLARES the unknown token: the #3366 + // preflight is step 3c, one step past this exit, so the hint does not exist + // yet. Measured, not assumed — the first draft of this pin expected `cap: 1` + // and this assertion is what corrected it. It is the earliest of the three + // "and NO MORE" readings, and the tightest. + expect(classes(payload.warnings)).toEqual({ rule: 1, doc: 0, key: 0, cap: 0 }); + }, 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 the `runtime bundle + * failed: ${err.message}` payload. + */ +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('#11772 — 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', () => { + // ⭐ A "no payload lacks `warnings`" 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({ success: false, error: 'a', issues }, 0, { compact: true });", + "await emitJson({ success: false, error: `x: ${e.message}`, warnings: warningsSoFar() }, 0, { compact: true });", + ].join('\n'); + const found = payloadLiterals(SYNTHETIC); + expect(found).toHaveLength(2); + expect(found.filter((p) => !p.includes('warnings:'))).toHaveLength(1); + // …and the template literal's `${…}` did not break the brace matching. + expect(found[1]).toContain('warnings: warningsSoFar()'); + }); + + it('all 10 `emitJson` exits carry `warnings` — 9 failure exits and the success payload', () => { + const literals = payloadLiterals(SRC); + // The enumeration measured on this card, three MORE than the filing card's + // table listed: it missed the protocol-parse exit, the `--no-runtime-bundle` + // refusal, and the bottom catch-all. + expect(literals, 'the `emitJson` exit count moved — a new exit must carry `warnings` 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('warnings:')); + expect( + bare, + 'an `os build --json` exit publishes no `warnings`, so the text face\'s ' + + '`re-run with --json for the full list` pointer is a dead end through it (#11772)', + ).toEqual([]); + }); + + it('the order lives at ONE site, which the success payload reads too', () => { + // Before this change the spread was written out at the success payload. A + // tenth exit could have been added with a different member order and + // nothing would have caught it; now every exit reads `warningsSoFar()`. + expect(SRC).toContain('const warningsSoFar = () => ['); + expect(SRC).toMatch(/\.\.\.ruleAdvisories,\s*\n\s*\.\.\.docWarnings,\s*\n\s*\.\.\.unknownKeyWarnings,\s*\n\s*\.\.\.capProviderWarnings,/); + for (const literal of payloadLiterals(SRC)) { + expect(literal, 'an exit spells its own `warnings` list instead of reading the shared one').toContain( + 'warnings: warningsSoFar()', + ); + } + }); +}); diff --git a/packages/cli/test/truncation-remainder-notices.test.ts b/packages/cli/test/truncation-remainder-notices.test.ts index 14fc60f66d..fc069caee9 100644 --- a/packages/cli/test/truncation-remainder-notices.test.ts +++ b/packages/cli/test/truncation-remainder-notices.test.ts @@ -407,7 +407,7 @@ describe('[#11642] a pointer is only offered where it resolves', () => { // asserted in prose: the key each list is published under, on the very // exit whose text face carries the notice. const carried: Array<[string, string]> = [ - ['compile.ts', "error: 'strict-body: missing body', issues }"], + ['compile.ts', "error: 'strict-body: missing body', issues,"], ['compile.ts', "error: 'author-time rules failed', issues: ruleErrors"], ['compile.ts', "error: 'access matrix drift', changes: drift"], ['compile.ts', "error: 'docs validation failed', issues: docErrors"], @@ -417,7 +417,14 @@ describe('[#11642] a pointer is only offered where it resolves', () => { // hints and the ADR-0046 doc advisories joined the two already here, so // "re-run with `--json` for the full list" resolves for four lists // rather than two. - ['compile.ts', 'warnings: [...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings],'], + // + // [#11772] Spelling updated again, claim STRONGER again: the four-list + // spread moved to `warningsSoFar()`, which EVERY `emitJson` exit reads — + // so the notice's remedy now resolves whichever later gate stops the run, + // not only on the exit that reaches the end. The per-exit payloads are + // pinned in `build-json-failure-warnings.e2e.test.ts`; what is checked + // here is that the notice still sits at an exit that publishes the list. + ['compile.ts', 'warnings: warningsSoFar(),'], ['validate.ts', 'errors: ruleErrors,'], ['validate.ts', 'errors: docErrors,'], ]; From 6da672d43dd14e8623eba1dbefc1c1cec9a3b44b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 08:44:06 +0000 Subject: [PATCH 2/2] fix(cli): bind the hoisted advisory list to splitBySeverity's return type (#11772) `Test Core (2/6)` failed on `packages/lint/src/authoring-rule-wiring.test.ts:291` -- "os build imports no unratcheted symbol from @objectstack/lint": AssertionError: compile.ts imports AuthoringFinding from @objectstack/lint directly. Register the rule in AUTHORING_RULES, or add the symbol to LINT_IMPORT_RATCHET with a reason. expected [ 'AuthoringFinding' ] to deeply equal [] WHY THIS CARD TRIPPED IT. Hoisting the four advisory lists out of the `try` so every `emitJson` failure exit can read `warningsSoFar()` turned one inferred binding into a declared one. Before the hoist the list arrived destructured -- `const { errors: ruleErrors, advisories: ruleAdvisories } = splitBySeverity(...)` -- and its type was inferred, so no name was imported. A `let` declared ahead of the assignment needs a written type, and the first spelling reached for the underlying finding type, adding `AuthoringFinding` to compile.ts's import list. The #4409 import scan reads that list and had never been told about the symbol. `import type` does not escape the scan, and that is deliberate: `lintImportsIn()` matches `/import\s+(?:type\s+)?\{([^}]*)\}\s*from\s*['"]@objectstack\/lint['"]/g` and then strips a per-name `type ` prefix, so both the statement-level and the inline modifier are seen. The guard is asking which SYMBOLS this command file names, not which of them survive to runtime. THE FIX. Bind the annotation to the function that produces the value: let ruleAdvisories: ReturnType['advisories'] = []; `splitBySeverity` is already an import this file makes and already carries a LINT_IMPORT_RATCHET entry ("Pure partition of a finding list into gating vs advisory. Carries no rule identity at all."), so no symbol is added and no exemption is widened. It resolves to exactly the same type the old annotation spelled -- `AuthoringFinding[]`, the function is sync so no `Awaited` is involved -- and `tsc --noEmit` is clean. WHY THIS IS NOT A GATE BYPASS. Nothing was added to LINT_IMPORT_RATCHET or to AUTHORING_RULES, no assertion in authoring-rule-wiring.test.ts was touched, and no test was skipped or relaxed. The ratchet's exemption set is byte-identical before and after; the import list it scans is what shrank. The annotation is strictly tighter than the one it replaces: it is now pinned to `splitBySeverity`'s declared shape, so if that function's `advisories` member ever changes type this binding follows it instead of silently disagreeing -- the same "one list cannot drift from itself" idiom this card applied to the member ORDER of `warningsSoFar()`, now applied to the list's TYPE. Behaviour is unchanged: type-only edit, no emitted JS differs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --- packages/cli/src/commands/compile.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index d7914d7aac..d1a43cbb6d 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -16,7 +16,7 @@ import { import { loadConfig } from '../utils/config.js'; import { lowerCallables } from '../utils/lower-callables.js'; import { buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint'; -import { runAuthoringRules, splitBySeverity, authoringRulesFor, type AuthoringFinding } 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, type DocIssue } from '../utils/collect-docs.js'; @@ -119,7 +119,13 @@ export default class Compile extends Command { // #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. - let ruleAdvisories: AuthoringFinding[] = []; + // 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['advisories'] = []; let capProviderWarnings: Array<{ token: string; message: string }> = []; let unknownKeyWarnings: string[] = []; let docWarnings: DocIssue[] = [];