From cd57c3f74048573cf85e9983b15f7307216ba390 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 11:31:51 +0000 Subject: [PATCH 1/4] wip(cli): resolveConfigPath refusals throw; os verify gains its catch-all --- packages/cli/src/commands/compile.ts | 10 +- packages/cli/src/commands/diff.ts | 10 +- packages/cli/src/commands/generate.ts | 14 ++- packages/cli/src/commands/i18n/check.ts | 10 +- packages/cli/src/commands/i18n/extract.ts | 10 +- packages/cli/src/commands/info.ts | 10 +- packages/cli/src/commands/lint.ts | 10 +- packages/cli/src/commands/migrate/meta.ts | 6 +- packages/cli/src/commands/validate.ts | 10 +- packages/cli/src/commands/verify.ts | 57 ++++++++++ packages/cli/src/utils/config.ts | 130 ++++++++++++++++------ packages/cli/src/utils/format.ts | 25 +++++ 12 files changed, 251 insertions(+), 51 deletions(-) diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index 035379744f..d98efc2ed9 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -42,6 +42,7 @@ import { emitJson, isExitSignal, errorCodeFields, + isReportedError, } from '../utils/format.js'; import { checkProtocolVersionGap } from '../utils/protocol-version-gap.js'; // [#14553] The compile-time half of the navigation-contribution group ruling. @@ -915,8 +916,13 @@ export default class Compile extends Command { await emitJson({ success: false, error: error.message, ...errorCodeFields(error), warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true }); this.exit(1); } - console.log(''); - printError(error.message || String(error)); + // [#15547] `resolveConfigPath()` already wrote its refusal and hint + // lines to stderr before throwing; printing the sentence again here + // would put a second copy on stdout. + if (!isReportedError(error)) { + console.log(''); + printError(error.message || String(error)); + } this.error(error.message || String(error)); } } diff --git a/packages/cli/src/commands/diff.ts b/packages/cli/src/commands/diff.ts index 16518ee6cf..9da78f517a 100644 --- a/packages/cli/src/commands/diff.ts +++ b/packages/cli/src/commands/diff.ts @@ -14,6 +14,7 @@ import { createTimer, emitJson, errorCodeFields, + isReportedError, } from '../utils/format.js'; // ─── Types ────────────────────────────────────────────────────────── @@ -310,8 +311,13 @@ export default class Diff extends Command { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); process.exit(1); } - console.log(''); - printError(error.message || String(error)); + // [#15547] `resolveConfigPath()` already wrote its refusal and hint + // lines to stderr before throwing; printing the sentence again here + // would put a second copy on stdout. + if (!isReportedError(error)) { + console.log(''); + printError(error.message || String(error)); + } process.exit(1); } } diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index fe8d04fd50..e5d9786501 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -18,7 +18,7 @@ import type { FieldType } from '@objectstack/spec/data'; // these, so the part that can be shared is shared and only the part that // genuinely lives on `driver-sql` is mirrored. import { isTenancyDisabled, isUniqueDeclared } from '@objectstack/spec/data'; -import { printHeader, printSuccess, printError, printInfo, printStep, createTimer, CLI_ALIAS } from '../utils/format.js'; +import { printHeader, printSuccess, printError, printInfo, printStep, createTimer, isReportedError, CLI_ALIAS } from '../utils/format.js'; import { metadataFileName } from '../utils/metadata-file-name.js'; import { findEmissionParseFailures } from '../utils/emitted-source-parses.js'; @@ -866,7 +866,9 @@ async function runTypesGeneration(configPath: string | undefined, flags: { outpu console.log(''); } catch (error: any) { - printError(error.message || String(error)); + // [#15547] `resolveConfigPath()` already reported its refusal on stderr + // before throwing; a second copy on stdout is what this guards. + if (!isReportedError(error)) printError(error.message || String(error)); process.exit(1); } } @@ -1007,7 +1009,9 @@ async function runClientGeneration(configPath: string | undefined, flags: { outp console.log(''); } catch (error: any) { - printError(error.message || String(error)); + // [#15547] `resolveConfigPath()` already reported its refusal on stderr + // before throwing; a second copy on stdout is what this guards. + if (!isReportedError(error)) printError(error.message || String(error)); process.exit(1); } } @@ -2065,7 +2069,9 @@ async function runMigrationGeneration(configPath: string | undefined, flags: { o console.log(''); } catch (error: any) { - printError(error.message || String(error)); + // [#15547] `resolveConfigPath()` already reported its refusal on stderr + // before throwing; a second copy on stdout is what this guards. + if (!isReportedError(error)) printError(error.message || String(error)); process.exit(1); } } diff --git a/packages/cli/src/commands/i18n/check.ts b/packages/cli/src/commands/i18n/check.ts index f966173925..4ebbcdf62a 100644 --- a/packages/cli/src/commands/i18n/check.ts +++ b/packages/cli/src/commands/i18n/check.ts @@ -15,6 +15,7 @@ import { emitJson, isExitSignal, errorCodeFields, + isReportedError, } from '../../utils/format.js'; import { computeI18nCoverage, COVERAGE_SURFACE_PHRASE } from '../../utils/i18n-coverage.js'; @@ -186,8 +187,13 @@ export default class I18nCheck extends Command { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); process.exit(1); } - console.log(''); - printError(error.message || String(error)); + // [#15547] `resolveConfigPath()` already wrote its refusal and hint + // lines to stderr before throwing; printing the sentence again here + // would put a second copy on stdout. + if (!isReportedError(error)) { + console.log(''); + printError(error.message || String(error)); + } process.exit(1); } } diff --git a/packages/cli/src/commands/i18n/extract.ts b/packages/cli/src/commands/i18n/extract.ts index 068a1fc0de..61d6350fba 100644 --- a/packages/cli/src/commands/i18n/extract.ts +++ b/packages/cli/src/commands/i18n/extract.ts @@ -16,6 +16,7 @@ import { emitJson, isExitSignal, errorCodeFields, + isReportedError, } from '../../utils/format.js'; import { extractTranslations, @@ -807,8 +808,13 @@ export default class I18nExtract extends Command { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); process.exit(1); } - console.log(''); - printError(error.message || String(error)); + // [#15547] `resolveConfigPath()` already wrote its refusal and hint + // lines to stderr before throwing; printing the sentence again here + // would put a second copy on stdout. + if (!isReportedError(error)) { + console.log(''); + printError(error.message || String(error)); + } process.exit(1); } } diff --git a/packages/cli/src/commands/info.ts b/packages/cli/src/commands/info.ts index 2d473a9f10..70209ce6c2 100644 --- a/packages/cli/src/commands/info.ts +++ b/packages/cli/src/commands/info.ts @@ -15,6 +15,7 @@ import { printMetadataStats, emitJson, errorCodeFields, + isReportedError, } from '../utils/format.js'; export default class Info extends Command { @@ -119,8 +120,13 @@ export default class Info extends Command { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); process.exit(1); } - console.log(''); - printError(error.message || String(error)); + // [#15547] `resolveConfigPath()` already wrote its refusal and hint + // lines to stderr before throwing; printing the sentence again here + // would put a second copy on stdout. + if (!isReportedError(error)) { + console.log(''); + printError(error.message || String(error)); + } process.exit(1); } } diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index ab85602c55..3714c77e43 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -28,6 +28,7 @@ import { emitJson, isExitSignal, errorCodeFields, + isReportedError, } from '../utils/format.js'; // ─── Types ────────────────────────────────────────────────────────── @@ -895,8 +896,13 @@ export default class Lint extends Command { ); process.exit(1); } - console.log(''); - printError(error.message || String(error)); + // [#15547] `resolveConfigPath()` already wrote its refusal and hint + // lines to stderr before throwing; printing the sentence again here + // would put a second copy on stdout. + if (!isReportedError(error)) { + console.log(''); + printError(error.message || String(error)); + } process.exit(1); } } diff --git a/packages/cli/src/commands/migrate/meta.ts b/packages/cli/src/commands/migrate/meta.ts index 058a5bb796..1ff44b256b 100644 --- a/packages/cli/src/commands/migrate/meta.ts +++ b/packages/cli/src/commands/migrate/meta.ts @@ -26,6 +26,7 @@ import { createTimer, emitJson, errorCodeFields, + isReportedError, } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; @@ -435,7 +436,10 @@ export default class MigrateMeta extends Command { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } - printError(error.message || String(error)); + // [#15547] `resolveConfigPath()` already wrote its refusal and hint + // lines to stderr before throwing; printing the sentence again here + // would put a second copy on stdout. + if (!isReportedError(error)) printError(error.message || String(error)); this.exit(1); } } diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index eb92efbf82..4cccf0fc09 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -37,6 +37,7 @@ import { emitJson, isExitSignal, errorCodeFields, + isReportedError, } from '../utils/format.js'; import { checkProtocolVersionGap } from '../utils/protocol-version-gap.js'; // [#14553] The navigation-contribution group check, shared with `os compile`. @@ -636,8 +637,13 @@ export default class Validate extends Command { }); this.exit(1); } - console.log(''); - printError(error.message || String(error)); + // [#15547] `resolveConfigPath()` already wrote its refusal and hint + // lines to stderr before throwing; printing the sentence again here + // would put a second copy on stdout. + if (!isReportedError(error)) { + console.log(''); + printError(error.message || String(error)); + } this.exit(1); } } diff --git a/packages/cli/src/commands/verify.ts b/packages/cli/src/commands/verify.ts index 41d431805e..de2b7a09ef 100644 --- a/packages/cli/src/commands/verify.ts +++ b/packages/cli/src/commands/verify.ts @@ -20,6 +20,13 @@ import { type RlsPositionPersonaInput, } from '@objectstack/verify'; import { loadConfig } from '../utils/config.js'; +import { + printError, + emitJson, + isExitSignal, + errorCodeFields, + isReportedError, +} from '../utils/format.js'; /** * Should this `os verify` run boot an org-scoped (multi-tenant) stack? @@ -86,9 +93,59 @@ export default class Verify extends Command { json: Flags.boolean({ description: 'Emit the structured report as JSON', default: false }), }; + /** + * The catch-all this command did not have (#15547). + * + * Every one of its nine `--json` siblings wraps its whole body in one `try` + * and answers a throw with an envelope; `os verify` wrapped nothing, so a + * throw walked out of `run()` and oclif rendered it. Measured on the + * published entry before this landed, against a config module that throws at + * evaluation: + * + * os verify --json → exit 1, stdout 0 B, stderr ` Error: …` + * os validate --json → exit 1, stdout `{"valid":false,"error":…}` + * os info --json → exit 1, stdout `{"error":…}` + * + * That mattered the moment `resolveConfigPath()` started throwing instead of + * exiting: this face would have been the one command turned INTO a crash + * dump by a change that fixed the other nine. So the `try` lands with the + * throw, never after it. + * + * The body moves into {@link runVerification} verbatim rather than being + * re-indented under a `try` here — the guard is the change, and a 120-line + * whitespace diff would bury it. + */ async run(): Promise { const { flags } = await this.parse(Verify); + try { + await this.runVerification(flags); + } catch (error: any) { + // `this.exit()` THROWS (see `isExitSignal`) — including the exit 0 this + // command's success path takes — so the signal is re-thrown before + // anything is described as a failure. + if (isExitSignal(error)) throw error; + if (flags.json) { + await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); + this.exit(1); + } + // [#15547] `resolveConfigPath()` already wrote its refusal and hint lines + // to stderr before throwing; printing the sentence again here would put a + // second copy on stdout. + if (!isReportedError(error)) { + console.log(''); + printError(error.message || String(error)); + } + this.exit(1); + } + } + + private async runVerification(flags: { + app?: string; + rls: boolean; + 'multi-tenant': boolean; + json: boolean; + }): Promise { const { config, absolutePath } = await loadConfig(flags.app); const multiTenant = resolveVerifyMultiTenant(flags); diff --git a/packages/cli/src/utils/config.ts b/packages/cli/src/utils/config.ts index 9a3f0e38b8..b7ffa1aa4d 100644 --- a/packages/cli/src/utils/config.ts +++ b/packages/cli/src/utils/config.ts @@ -39,52 +39,118 @@ export const BUNDLE_REQUIRE_EXTERNALS: (string | RegExp)[] = [ '@libsql/client', ]; +/** + * The refusal `resolveConfigPath()` throws when no config file can be resolved. + * + * Three fields, each with exactly one consumer, and the split is the point: + * + * • `message` — PLAIN. It is what every `--json` catch-all copies into its + * envelope, so it must not carry terminal decoration: `chalk.white(abs)` + * in a payload is `…` inside a JSON string the moment + * the run happens to have colour on. + * • `display` — the same sentence WITH that decoration, for the stream a + * human reads. Defaults to `message` where there is nothing to decorate. + * • `hints` — the lines printed under the refusal. Carried on the error + * rather than printed and forgotten, so the renderer below is the only + * place that knows their shape. + * + * `reportedToStderr` is read structurally by {@link isReportedError}, never + * through `instanceof`: a command's catch-all must not print this refusal a + * second time on stdout, and a structural marker survives a tree where `dist/` + * and `src/` copies of this module can both be live. + * + * ⛔ No `code` and no `httpStatus` field, deliberately. `errorCodeFields()` + * reads exactly those two names off a thrown error, so adding either here + * would mint an ADR-0112 code for this refusal through the back door — the one + * thing the #15547 ruling forbids. What a bare `{ error }` with neither field + * should look like is #15549's question, not this file's. + */ +export class ConfigRefusalError extends Error { + /** The refusal sentence with the decoration the text face has always shown. */ + readonly display: string; + /** The dim lines printed under the refusal, in order. */ + readonly hints: readonly string[]; + /** Already written to stderr at the throw site — do not render it twice. */ + readonly reportedToStderr = true; + + constructor(message: string, hints: readonly string[], display: string = message) { + super(message); + this.name = 'ConfigRefusalError'; + this.display = display; + this.hints = hints; + } +} + +/** + * Report a config refusal on stderr and throw it. + * + * The four writes are the ones this helper has always made, in the same order, + * on the same stream, byte for byte — they are just driven off the error object + * now instead of off four literals. Rendering here rather than in the ten + * catch-alls is what keeps the diagnostic in BOTH faces: a `--json` run still + * shows its operator the refusal on stderr while the machine reads the envelope + * on stdout, which is the shape #15692 established and this change must not + * undo. + */ +function refuseConfig(message: string, hints: readonly string[], display?: string): never { + const error = new ConfigRefusalError(message, hints, display); + printErrorToStderr(error.display); + console.error(''); + for (const hint of error.hints) console.error(chalk.dim(hint)); + throw error; +} + /** * Resolve the config file path. Supports: * - explicit path (objectstack.config.ts) * - auto-detection (searches for objectstack.config.{ts,js,mjs}) * - * ## Both refusals go to STDERR, and that is not cosmetic (#15547) + * ## Both refusals THROW, and go to stderr on the way out (#15547) * * This helper is reached by ten published `--json` faces — `os validate`, * `info`, `diff`, `lint`, `compile`, `build` (a subclass of `compile`), * `verify`, `migrate meta`, `i18n check`, `i18n extract` — and it has no way * to know which run is a `--json` run: the flag is parsed in the command, and - * `loadConfig()` passes it nothing. It used to print through `printError` and - * `console.log`, **both of which write to stdout**, and then call - * `process.exit(1)`. + * `loadConfig()` passes it nothing. * - * That put human text on the one stream `--json` reserves for the machine - * (`utils/json-stdout.ts`). Measured on the published entry `bin/run.js` with - * `NO_COLOR=1` and the streams captured separately: every one of the ten faces - * answered a missing config with **exit 1, an unparseable stdout and an empty - * stderr**, on both branches below. And because the exit is called directly, - * nothing throws — so every command's catch-all `--json` error exit, which - * sits downstream of a throw, never ran. + * It used to print through `printError` and `console.log` — **both stdout** — + * and then call `process.exit(1)`. #15692 moved the bytes to stderr; the exit + * stayed, and with it the real defect: **nothing was thrown**, so every + * command's catch-all `--json` error exit — all of which sit downstream of a + * throw — never ran, and ten faces answered a missing config with an EMPTY + * stdout where each of them has already declared it emits an envelope. * - * ⚠️ Moving the bytes is the whole change. The exit code stays 1, the wording - * stays identical, and no payload is invented here: what a `--json` consumer - * should receive on this path is an envelope question that touches ten - * published faces at once, and it is deliberately left open (see - * {@link printErrorToStderr} and the PR for #15547). What is settled is that - * the machine's channel no longer carries prose. + * ⇒ The refusals now throw {@link ConfigRefusalError}. That is not a new + * contract; it is this path being pulled back onto the contract its callers + * already published, which is why it adds **zero** accept-set members and + * **zero** error codes. * - * ⛔ Do not "fix" this by making the helper throw without settling that - * question first. Measured on the callers: `os verify` has no `try` around its - * `loadConfig()` at all, so a throw becomes an oclif crash dump rather than a - * payload; and `errorCodeFields()` deliberately mints no `code` for a plain - * `Error`, so the other nine would emit a bare `{ error }` — the exact shape - * #15549 is an open card about. + * Three properties hold it in place, and each has a pin: + * + * 1. **No face becomes a crash dump.** `os verify` had no `try` at all — + * measured, a throw through it produced an oclif error line and no + * payload — so it gained the catch-all its nine siblings already had, in + * the same landing as the throw. + * 2. **The text face does not narrow.** The refusal and both hint lines are + * still written here, to stderr, byte-identical; the catch-alls skip + * re-printing via {@link isReportedError}. + * 3. **No code is minted.** The thrown error carries neither `code` nor + * `httpStatus`, so `errorCodeFields()` contributes nothing and the + * envelope is a bare `{ error }`. Whether that shape is right is + * **#15549's** open question — ⛔ do not answer it by adding a field here. */ export function resolveConfigPath(source?: string): string { if (source) { const abs = path.resolve(process.cwd(), source); if (!fs.existsSync(abs)) { - printErrorToStderr(`Config file not found: ${chalk.white(abs)}`); - console.error(''); - console.error(chalk.dim(' Hint: Run this command from a directory with objectstack.config.ts')); - console.error(chalk.dim(' Or specify the path: objectstack path/to/config.ts')); - process.exit(1); + refuseConfig( + `Config file not found: ${abs}`, + [ + ' Hint: Run this command from a directory with objectstack.config.ts', + ' Or specify the path: objectstack path/to/config.ts', + ], + `Config file not found: ${chalk.white(abs)}`, + ); } return abs; } @@ -101,10 +167,10 @@ export function resolveConfigPath(source?: string): string { if (fs.existsSync(abs)) return abs; } - printErrorToStderr('No objectstack.config.{ts,js,mjs} found in current directory'); - console.error(''); - console.error(chalk.dim(' Hint: Run `objectstack init` to create a new project')); - process.exit(1); + refuseConfig( + 'No objectstack.config.{ts,js,mjs} found in current directory', + [' Hint: Run `objectstack init` to create a new project'], + ); } /** diff --git a/packages/cli/src/utils/format.ts b/packages/cli/src/utils/format.ts index b4364b553c..bdfd31948e 100644 --- a/packages/cli/src/utils/format.ts +++ b/packages/cli/src/utils/format.ts @@ -131,6 +131,31 @@ export function isExitSignal(error: unknown): boolean { return e?.code === 'EEXIT' || typeof e?.oclif?.exit === 'number'; } +/** + * True for an error whose diagnostic was ALREADY written to stderr by the code + * that threw it — so a command's TEXT face must not render it a second time. + * + * `resolveConfigPath()` is the case this exists for (#15547). It refuses with a + * sentence plus hint lines no catch-all could reconstruct — the hints are the + * helper's, not the command's — so it writes them itself, on stderr, and + * throws. Without this predicate the text face would then print the same + * sentence again through `printError`, on **stdout**, and the operator would + * read one refusal twice across two streams. + * + * ⚠️ It gates the TEXT branch only. The `--json` branch sits above it and is + * unaffected: the envelope is the machine's copy of that failure and the prose + * on stderr is the human's, which is the split #15692 established and #15547 + * kept. + * + * Structural, like {@link isExitSignal}, and deliberately not `instanceof`: + * this CLI runs from `dist/` in production and from `src/` under `tsx` in the + * gates, and a marker that depends on which copy of a class the error came from + * fails silently in exactly the tree the pins run in. + */ +export function isReportedError(error: unknown): boolean { + return (error as { reportedToStderr?: unknown } | null | undefined)?.reportedToStderr === true; +} + /** * [#13347] The ADR-0112 carriers a `--format json` failure envelope adds * beside its `error` sentence — `{ code, httpStatus }`, and only the ones the From 6ff603f29a8d796795f4077d0b527d1cf56309f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 11:41:38 +0000 Subject: [PATCH 2/4] test(cli): widen the pre-boot json purity population; pin the text face by bytes --- ...onfig-refusal-throws-so-json-faces-emit.md | 48 +++ packages/cli/src/commands/compile.ts | 19 +- .../config-miss-stdout-purity.e2e.test.ts | 316 ++++++++---------- .../cli/test/helpers/config-miss-family.ts | 163 +++++++++ .../cli/test/json-stdout-purity.e2e.test.ts | 25 ++ 5 files changed, 383 insertions(+), 188 deletions(-) create mode 100644 .changeset/config-refusal-throws-so-json-faces-emit.md create mode 100644 packages/cli/test/helpers/config-miss-family.ts diff --git a/.changeset/config-refusal-throws-so-json-faces-emit.md b/.changeset/config-refusal-throws-so-json-faces-emit.md new file mode 100644 index 0000000000..6466423d5e --- /dev/null +++ b/.changeset/config-refusal-throws-so-json-faces-emit.md @@ -0,0 +1,48 @@ +--- +"@objectstack/cli": patch +--- + +fix(cli): `resolveConfigPath` throws its two refusals so the ten `--json` faces emit their envelopes, and `os verify` gains the catch-all it never had (#15547) + +Every `--json` face in this CLI declares that it answers an error path with a +payload. `resolveConfigPath()` was the one path that bypassed that declaration: +it wrote its refusal and then called `process.exit(1)` **directly**, so nothing +was thrown and the catch-all each command already carries — all of which sit +downstream of a throw — never ran. Ten published faces answered a missing config +file with an empty stdout. + +Measured before this change on the published entry `packages/cli/bin/run.js`, +`NO_COLOR=1`, streams captured separately, exit read before any pipe — ten faces +(`build` · `compile` · `diff` · `i18n check` · `i18n extract` · `info` · `lint` · +`migrate meta` · `validate` · `verify`) across both branches of the helper, 19 +runs: **exit 1, stdout 0 bytes, stderr 296 B (explicit path) / 123 B +(auto-detect)** — and `JSON.parse` on that stdout throws in all 19. After: the +same 19 runs answer **exit 1 with a parseable document on stdout**, stderr +unchanged byte for byte. + +The refusals now throw `ConfigRefusalError`. That is not a new contract — it is +this path being pulled back onto the one its callers had already published, so +it adds **zero** accept-set members and **zero** error codes. + +Three properties hold it in place: + +- **No face becomes a crash dump.** `os verify` had no `try` at all — measured, + a throw through it produced an oclif error line and no payload where every + sibling emitted an envelope — so it gains the catch-all its nine siblings + already had, in this same change rather than after it. +- **The text face does not narrow.** The refusal and both hint lines are still + written by the helper, to stderr, byte-identical: all 19 non-`--json` runs + compare equal before and after on stdout, on stderr and on exit status. The + catch-alls skip re-rendering the sentence a second time on stdout. +- **No error code is minted.** The thrown error carries neither `code` nor + `httpStatus`, so `errorCodeFields()` contributes nothing and each face emits + its own bare `{ error }`. Whether that shape is right is **#15549**'s open + question, and this change deliberately does not answer it. + +The `--json` stdout-purity instrument is widened with the fix rather than after +it: the pre-boot family's discovery moves into a shared module, the pin that +drives it now demands a document (empty stdout no longer passes) and compares +the text face's stderr as a whole string, and `json-stdout-purity.e2e.test.ts` +— whose own discovery is `bootSchemaStack`-based and cannot see a command that +fails above the kernel — reconciles against that population so neither half can +be lost silently. diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index d98efc2ed9..a649678381 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -916,13 +916,18 @@ export default class Compile extends Command { await emitJson({ success: false, error: error.message, ...errorCodeFields(error), warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true }); this.exit(1); } - // [#15547] `resolveConfigPath()` already wrote its refusal and hint - // lines to stderr before throwing; printing the sentence again here - // would put a second copy on stdout. - if (!isReportedError(error)) { - console.log(''); - printError(error.message || String(error)); - } + // [#15547] `resolveConfigPath()` already wrote its refusal and hint lines + // to stderr before throwing, so this face has nothing left to render — + // and `this.error()` below is NOT a no-op for it: it re-renders the same + // sentence as an oclif `› Error:` block AND raises this face's exit + // status from 1 to 2. Measured on the published entry, `os compile + // ./missing.ts` (and `os build`, which inherits this catch): exit 2 with + // 483 stderr bytes, where the other eight faces answer exit 1 with 296. + // `this.exit(1)` throws the ExitError the `--json` branch already relies + // on, so the status and the bytes both stay where they were. + if (isReportedError(error)) this.exit(1); + console.log(''); + printError(error.message || String(error)); this.error(error.message || String(error)); } } diff --git a/packages/cli/test/config-miss-stdout-purity.e2e.test.ts b/packages/cli/test/config-miss-stdout-purity.e2e.test.ts index 080b9f9edb..7ab16c441f 100644 --- a/packages/cli/test/config-miss-stdout-purity.e2e.test.ts +++ b/packages/cli/test/config-miss-stdout-purity.e2e.test.ts @@ -1,8 +1,8 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * `--json` ⇒ nothing unparseable on stdout when the CONFIG FILE IS MISSING, - * for the whole `resolveConfigPath` family (#15547). + * `--json` ⇒ ONE JSON DOCUMENT on stdout when the CONFIG FILE IS MISSING, for + * the whole `resolveConfigPath` family — and the text face unchanged (#15547). * * ## The blind spot this exists to close * @@ -12,159 +12,63 @@ * runs **before** any kernel boots, so that pin structurally cannot see this * path and stayed green through the whole defect. * - * What it was green through: `resolveConfigPath()` printed its refusal through - * `printError` and `console.log` — both stdout — and then called - * `process.exit(1)` directly. Ten published `--json` faces therefore answered a - * missing config with human text on the machine's channel, an EMPTY stderr, and - * no payload; and because nothing was thrown, every command's catch-all - * `--json` error exit — all of which sit downstream of a throw — never ran. + * What it was green through: `resolveConfigPath()` printed its refusal to + * stdout and then called `process.exit(1)` directly. Ten published `--json` + * faces therefore answered a missing config with human text on the machine's + * channel; and because nothing was thrown, every command's catch-all `--json` + * error exit — all of which sit downstream of a throw — never ran. * - * ⇒ The instrument was as broken as the code. A fix that repaired the helper - * without widening the discovery would leave the next pre-boot stdout leak just - * as invisible, which is why this file exists rather than a fixture edit. + * ⇒ The instrument was as broken as the code, so the population is widened + * across a PAIR of files rather than left to one: the discovery lives in + * `helpers/config-miss-family.ts`, this file drives it, and the sibling pin + * reconciles against it. See that helper's header for why it is not inlined. * - * ## Why the whole family, from one expectation + * ## What changed here when the refusals started throwing * - * Same discipline as the sibling pin: the family is READ OFF THE SOURCE, not - * remembered, and reconciled against {@link FAMILY}. Add a command that offers - * `--json` and reaches the config helper and this file goes red until it is - * listed here and passes; drop one and it goes red until it is removed. + * This file's first version asserted only that stdout carried nothing a + * machine could not read — empty passed, one JSON document passed, prose + * failed — because whether these faces should EMIT anything was still an open + * question then. * - * The discovery has two halves because the reach has two shapes: + * That question is now ruled: the refusals throw, the ten catch-alls emit the + * envelopes they had already declared, and **empty stdout no longer passes**. + * The assertion is tightened to a bare `JSON.parse` accordingly — a face that + * regressed to exiting with no payload slipped straight through the old form. * - * • DIRECT — the module declares `json: Flags.boolean(` and imports from - * `utils/config.js`. - * • ALIAS — the module's default export `extends` a command in the direct - * set, so it inherits both the flag and the reach without naming either. - * `os build` is exactly this (`class Build extends Compile`), and a - * one-half discovery would have missed it: the original card's static - * reading listed nine modules, and `build` is the tenth face. + * ⛔ Still NOT pinned here: the envelope's SHAPE. The thrown error carries no + * `code` and no `httpStatus`, so `errorCodeFields()` contributes nothing and + * each face emits its own bare `{ error }`. Whether that is the right shape is + * **#15549**'s open question; this file asserts that a document arrives and + * that it names the refusal, never what else is in it, so settling #15549 + * changes the payload without touching this file. * - * ## What is asserted — and what is deliberately NOT + * ## The text face is pinned by BYTES, not by containment * - * ⛔ This file does NOT pin an error-payload shape. Whether `--json` should - * emit an envelope on this path is an open question touching ten published - * faces at once, entangled with #15549 (`os lint --eval --json`'s bare - * `{ error }` with no `code` and no `httpStatus`), and settling it is above - * this pin's authority. - * - * So the assertion is the half that needs no ruling: **stdout carries nothing a - * machine cannot read.** Empty passes, one JSON document passes, prose fails. - * That holds under the shape shipped today AND under any future envelope, so - * whoever settles the question changes the payload without touching this file. - * - * The other two halves are the ones a "just silence it" regression would break: - * the diagnostic must still reach the operator, on **stderr**, and the exit - * status must still be 1. + * The route ruled out for this card was "make it throw and let the catch-alls + * render it" — which deletes the helper's hint lines from all ten text faces, + * because a catch-all can only re-render `error.message`. A `toContain('Hint:')` + * assertion does not see that loss when one hint line survives and the other + * does not, so the whole stderr string is compared instead. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { execFile } from 'node:child_process'; -import { mkdtempSync, rmSync, readFileSync, readdirSync, statSync } from 'node:fs'; +import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join, resolve, relative, sep } from 'node:path'; +import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { childEnv } from './helpers/serve-process.js'; +import { + CONFIG_MISS_FAMILY, + CONFIG_MISS_REFUSAL, + MISSING_CONFIG, + discoverConfigMissFamily, + expectedRefusalStderr, +} from './helpers/config-miss-family.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 COMMANDS_DIR = resolve(HERE, '../src/commands'); - -/** A path that cannot exist, driving the EXPLICIT-PATH branch of the helper. */ -const MISSING = './nope-does-not-exist.ts'; - -/** - * The family, and the argv each member needs to reach `resolveConfigPath()`. - * - * `autoDetect: false` marks the one member with no auto-detect branch to drive: - * `os diff` requires two config paths, so there is no bare form that reaches - * the helper without one. - */ -interface Member { - /** argv that reaches the helper with an EXPLICIT missing path. */ - explicit: string[]; - /** argv that reaches the helper with NO path, or `null` when there is none. */ - auto: string[] | null; -} - -const FAMILY: Record = { - // `build` is `class Build extends Compile` — the alias half of the discovery. - build: { explicit: [MISSING], auto: [] }, - compile: { explicit: [MISSING], auto: [] }, - diff: { explicit: [MISSING, MISSING], auto: null }, - 'i18n check': { explicit: [MISSING], auto: [] }, - 'i18n extract': { explicit: [MISSING], auto: [] }, - info: { explicit: [MISSING], auto: [] }, - lint: { explicit: [MISSING], auto: [] }, - // `--from` because `--stored` is its only other way past the flag parser, and - // `--stored` boots a kernel — a different family, already pinned elsewhere. - 'migrate meta': { explicit: [MISSING, '--from', '4'], auto: ['--from', '4'] }, - validate: { explicit: [MISSING], auto: [] }, - // The config path is a FLAG here, not a positional. - verify: { explicit: ['--app', MISSING], auto: [] }, -}; - -/** The refusal text, one line per branch — what must be on stderr, never stdout. */ -const REFUSAL = { - explicit: 'Config file not found', - auto: 'No objectstack.config.{ts,js,mjs} found in current directory', -} as const; - -/** Every `.ts` under `src/commands`, excluding tests. */ -function commandFiles(dir: string): string[] { - const out: string[] = []; - for (const entry of readdirSync(dir)) { - const abs = join(dir, entry); - if (statSync(abs).isDirectory()) { - out.push(...commandFiles(abs)); - continue; - } - if (!entry.endsWith('.ts') || entry.endsWith('.test.ts')) continue; - out.push(abs); - } - return out; -} - -/** `src/commands/i18n/check.ts` → `i18n check`, the id oclif dispatches on. */ -function commandId(abs: string): string { - const rel = relative(COMMANDS_DIR, abs).replace(/\.ts$/, ''); - return rel.split(sep).filter((p) => p !== 'index').join(' '); -} - -/** - * The family, read off the source rather than remembered: a command belongs iff - * it offers a machine-readable mode AND reaches the config helper — directly, - * or by extending a command that does. - */ -function discoverFamily(): string[] { - const files = commandFiles(COMMANDS_DIR); - const sources = new Map(files.map((abs) => [abs, readFileSync(abs, 'utf-8')])); - - const direct = new Set(); - for (const [abs, src] of sources) { - if (!/\bjson:\s*Flags\.boolean\(/.test(src)) continue; - if (!/from '(?:\.\.\/)+utils\/config\.js'/.test(src)) continue; - direct.add(abs); - } - - // An alias inherits the flag and the reach from the class it extends, and - // names neither itself. Resolve `extends ` back to the module the - // identifier was imported from, and take the member if that module is in the - // direct set. One level is enough for the aliases in this tree and a deeper - // chain would show up as a discovery mismatch rather than pass silently. - const alias = new Set(); - for (const [abs, src] of sources) { - const ext = /export default class \w+ extends (\w+)\b/.exec(src); - if (!ext) continue; - const imported = new RegExp(`import ${ext[1]} from '(\\.[^']+)\\.js'`).exec(src); - if (!imported) continue; - const target = resolve(abs, '..', `${imported[1]}.ts`); - if (direct.has(target)) alias.add(abs); - } - - return [...direct, ...alias].map(commandId).sort(); -} interface Run { key: string; @@ -194,34 +98,23 @@ function runCli(argv: string[], cwd: string): Promise> { }); } -/** - * Whether stdout is something a program can read: nothing at all, or exactly - * one JSON document. Prose is the failure — see the header for why the choice - * between the two passing shapes is deliberately left open. - */ -function stdoutIsMachineReadable(stdout: string): boolean { - if (stdout.trim() === '') return true; - try { - JSON.parse(stdout); - return true; - } catch { - return false; - } -} - /** `[key, argv]` for every branch of every member — 19 runs across 10 faces. */ function cases(): [string, string[]][] { const out: [string, string[]][] = []; - for (const [id, member] of Object.entries(FAMILY)) { + for (const [id, member] of Object.entries(CONFIG_MISS_FAMILY)) { const argv = id.split(' '); - out.push([`${id} (explicit path)`, [...argv, ...member.explicit, '--json']]); - if (member.auto) out.push([`${id} (auto-detect)`, [...argv, ...member.auto, '--json']]); + out.push([`${id} (explicit path)`, [...argv, ...member.explicit]]); + if (member.auto) out.push([`${id} (auto-detect)`, [...argv, ...member.auto]]); } return out; } +const branchOf = (key: string): 'explicit' | 'auto' => + (key.endsWith('(auto-detect)') ? 'auto' : 'explicit'); + let dir: string; -let runs: Run[]; +let jsonRuns: Run[]; +let textRuns: Run[]; beforeAll(async () => { // Deliberately EMPTY — no `objectstack.config.*` here, so the auto-detect @@ -229,11 +122,18 @@ beforeAll(async () => { dir = mkdtempSync(join(tmpdir(), 'os-config-miss-e2e-')); // Sequential: nineteen `tsx` starts at once is the kind of load that makes a - // shared box report timeouts instead of verdicts. - runs = []; + // shared box report timeouts instead of verdicts. BOTH faces are driven — + // the `--json` half is the contract this card repaired, the text half is the + // one it had to leave untouched, and only driving the second proves it. + jsonRuns = []; + for (const [key, argv] of cases()) { + const { code, stdout, stderr } = await runCli([...argv, '--json'], dir); + jsonRuns.push({ key, code, stdout, stderr }); + } + textRuns = []; for (const [key, argv] of cases()) { const { code, stdout, stderr } = await runCli(argv, dir); - runs.push({ key, code, stdout, stderr }); + textRuns.push({ key, code, stdout, stderr }); } }, 900_000); @@ -243,51 +143,105 @@ afterAll(() => { describe('the family this contract has to hold across', () => { it('is exactly the set listed here — a new member goes red until it is driven too', () => { - expect(discoverFamily()).toEqual(Object.keys(FAMILY).sort()); + expect(discoverConfigMissFamily()).toEqual(Object.keys(CONFIG_MISS_FAMILY).sort()); }); - it('includes the alias face, which declares neither the flag nor the import', () => { - // Guards the alias half specifically: a discovery that regressed to - // "grep the module" would still pass the reconciliation above only by - // ALSO dropping `build` from FAMILY, and this makes that a second red. - expect(discoverFamily()).toContain('build'); + it('is TEN faces, and names the two a static reading loses', () => { + // The population is load-bearing rather than incidental: the original card + // reached nine modules by reading imports, and `os build` declares neither + // the flag nor the import. A discovery that quietly shrank back to nine + // would still satisfy the reconciliation above if FAMILY shrank with it, + // so the count and the two interesting members are asserted directly. + expect(discoverConfigMissFamily()).toHaveLength(10); + expect(discoverConfigMissFamily()).toContain('build'); + expect(discoverConfigMissFamily()).toContain('verify'); + }); + + it('drives both branches of the helper — 19 runs, not 10', () => { + expect(cases()).toHaveLength(19); }); }); describe.each(cases())('os %s --json, config missing', (key) => { const runOf = () => { - const run = runs.find((r) => r.key === key); + const run = jsonRuns.find((r) => r.key === key); if (!run) throw new Error(`no run captured for '${key}'`); return run; }; - it('leaves nothing on stdout that a machine cannot read', () => { + it('emits ONE JSON document on stdout — a bare JSON.parse, no extraction', () => { const run = runOf(); - // Under the defect this was 206 bytes of ` ✗ Config file not found: …` - // plus two hint lines — on the one stream `--json` reserves for the - // machine, with stderr completely empty. - expect(stdoutIsMachineReadable(run.stdout)).toBe(true); + // Under the original defect this was 296 bytes of prose; after #15692 it + // was ZERO bytes, which `JSON.parse` rejects just as loudly. Both are the + // failure this asserts against. + const payload = JSON.parse(run.stdout); + expect(payload).toBeTypeOf('object'); + expect(payload).not.toBeNull(); + }); + + it('names the refusal in the payload, so the machine is told WHY', () => { + const payload = JSON.parse(runOf().stdout) as { error?: unknown }; + expect(payload.error).toBeTypeOf('string'); + expect(String(payload.error)).toContain(CONFIG_MISS_REFUSAL[branchOf(key)]); + }); + + it('carries no terminal decoration into the payload', () => { + // The refusal line the operator reads wraps the path in `chalk.white`. The + // envelope must carry the PLAIN sentence: an escape sequence inside a JSON + // string is a defect a consumer cannot see coming, and it would appear + // only in runs that happen to have colour on. + expect(runOf().stdout).not.toContain('['); }); it('keeps the human refusal off stdout entirely', () => { const run = runOf(); // Asserted separately from the parse so a regression names its cause - // rather than only `Unexpected token`. - expect(run.stdout).not.toContain(REFUSAL.explicit); - expect(run.stdout).not.toContain(REFUSAL.auto); + // rather than only `Unexpected token`. The payload's `error` sentence is + // not this: `Hint:` and the glyph are prose only the text renderer writes. expect(run.stdout).not.toContain('Hint:'); + expect(run.stdout).not.toContain('✗'); }); - it('still shows the operator the refusal — on stderr', () => { - const run = runOf(); - // Diagnostics are MOVED, never destroyed: a regression toward silencing - // this path goes red here. - const expected = key.endsWith('(auto-detect)') ? REFUSAL.auto : REFUSAL.explicit; - expect(run.stderr).toContain(expected); - expect(run.stderr).toContain('Hint:'); + it('still shows the operator the refusal — on stderr, byte for byte', () => { + // Diagnostics are MOVED, never destroyed: the envelope is the machine's + // copy of this failure and the prose is the human's, and a `--json` run + // keeps both. A regression toward silencing this path goes red here. + expect(runOf().stderr).toBe(expectedRefusalStderr(branchOf(key), resolve(dir, MISSING_CONFIG))); }); it('still exits 1', () => { expect(runOf().code).toBe(1); }); }); + +describe.each(cases())('os %s (text face), config missing', (key) => { + const runOf = () => { + const run = textRuns.find((r) => r.key === key); + if (!run) throw new Error(`no text run captured for '${key}'`); + return run; + }; + + it('writes the refusal and every hint line to stderr, byte for byte', () => { + // Full-string equality, deliberately not `toContain`. The route ruled out + // for this card kept the first line and dropped the hints; every + // containment assertion anyone would reach for passes through that loss. + expect(runOf().stderr).toBe(expectedRefusalStderr(branchOf(key), resolve(dir, MISSING_CONFIG))); + }); + + it('does not print the refusal a SECOND time, on stdout', () => { + // The helper reports on stderr and throws. A catch-all that then rendered + // `error.message` through `printError` would put the same sentence on + // stdout, and the operator would read one failure twice, on two streams. + const run = runOf(); + expect(run.stdout).not.toContain(CONFIG_MISS_REFUSAL[branchOf(key)]); + expect(run.stdout).not.toContain('✗'); + }); + + it('still exits 1 — not 2', () => { + // `os compile` (and `os build`, which inherits its catch) ends its text + // branch in oclif's `this.error()`, which exits 2 and re-renders the + // sentence as a `› Error:` block. Measured at 483 stderr bytes and exit + // 2 while that path was unguarded, against 296 and exit 1 everywhere else. + expect(runOf().code).toBe(1); + }); +}); diff --git a/packages/cli/test/helpers/config-miss-family.ts b/packages/cli/test/helpers/config-miss-family.ts new file mode 100644 index 0000000000..16475983c6 --- /dev/null +++ b/packages/cli/test/helpers/config-miss-family.ts @@ -0,0 +1,163 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The PRE-BOOT `--json` family: the commands that refuse at + * `resolveConfigPath()`, before any kernel exists (#15547). + * + * ## Why this is a shared module and not a constant in one test file + * + * `json-stdout-purity.e2e.test.ts` pins "stdout is exactly one JSON document" + * and DISCOVERS its family as the commands that call `bootSchemaStack`. The + * commands here never boot one — they fail above it — so that discovery + * structurally cannot see them, and the whole `--json`-stdout contract was + * being watched by an instrument blind to ten of its faces. + * + * Widening that pin's own discovery to include them would drive ten commands + * that emit no boot diagnostics against assertions about boot diagnostics. So + * the POPULATION is widened across the pair instead: this module owns the + * pre-boot discovery, `config-miss-stdout-purity.e2e.test.ts` drives it, and + * `json-stdout-purity.e2e.test.ts` reconciles against it — which makes losing + * the pre-boot half a red in BOTH files rather than a silent gap in neither. + * + * ⛔ Do not inline either export back into a test file. The blind spot this + * closes was created exactly by a discovery that only one file could see. + */ + +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { join, resolve, relative, sep } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); + +/** `packages/cli/src/commands` — the tree both discoveries read. */ +export const COMMANDS_DIR = resolve(HERE, '../../src/commands'); + +/** A path that cannot exist, driving the EXPLICIT-PATH branch of the helper. */ +export const MISSING_CONFIG = './nope-does-not-exist.ts'; + +/** + * How to drive one member into the config helper. + * + * `auto: null` marks the one member with no auto-detect branch to drive: + * `os diff` requires two config paths, so there is no bare form that reaches + * the helper without one. + */ +export interface ConfigMissMember { + /** argv that reaches the helper with an EXPLICIT missing path. */ + explicit: string[]; + /** argv that reaches the helper with NO path, or `null` when there is none. */ + auto: string[] | null; +} + +export const CONFIG_MISS_FAMILY: Record = { + // `build` is `class Build extends Compile` — the alias half of the discovery. + build: { explicit: [MISSING_CONFIG], auto: [] }, + compile: { explicit: [MISSING_CONFIG], auto: [] }, + diff: { explicit: [MISSING_CONFIG, MISSING_CONFIG], auto: null }, + 'i18n check': { explicit: [MISSING_CONFIG], auto: [] }, + 'i18n extract': { explicit: [MISSING_CONFIG], auto: [] }, + info: { explicit: [MISSING_CONFIG], auto: [] }, + lint: { explicit: [MISSING_CONFIG], auto: [] }, + // `--from` because `--stored` is its only other way past the flag parser, and + // `--stored` boots a kernel — a different family, already pinned elsewhere. + 'migrate meta': { explicit: [MISSING_CONFIG, '--from', '4'], auto: ['--from', '4'] }, + validate: { explicit: [MISSING_CONFIG], auto: [] }, + // The config path is a FLAG here, not a positional. + verify: { explicit: ['--app', MISSING_CONFIG], auto: [] }, +}; + +/** The refusal text, one line per branch — what must be on stderr, never stdout. */ +export const CONFIG_MISS_REFUSAL = { + explicit: 'Config file not found', + auto: 'No objectstack.config.{ts,js,mjs} found in current directory', +} as const; + +/** + * The exact stderr a refusal writes, byte for byte. + * + * Held here rather than asserted with `toContain` because the two HINT lines + * are the half a "make the helper throw" change silently deletes: a catch-all + * that re-renders `error.message` reproduces the first line and nothing else, + * and a containment assertion on the first line passes through that loss. The + * text face of these ten commands is pinned to this whole string (#15547). + */ +export function expectedRefusalStderr(branch: 'explicit' | 'auto', absentPath: string): string { + if (branch === 'explicit') { + return ( + ` ✗ Config file not found: ${absentPath}\n` + + '\n' + + ' Hint: Run this command from a directory with objectstack.config.ts\n' + + ' Or specify the path: objectstack path/to/config.ts\n' + ); + } + return ( + ' ✗ No objectstack.config.{ts,js,mjs} found in current directory\n' + + '\n' + + ' Hint: Run `objectstack init` to create a new project\n' + ); +} + +/** Every `.ts` under `src/commands`, excluding tests. */ +function commandFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir)) { + const abs = join(dir, entry); + if (statSync(abs).isDirectory()) { + out.push(...commandFiles(abs)); + continue; + } + if (!entry.endsWith('.ts') || entry.endsWith('.test.ts')) continue; + out.push(abs); + } + return out; +} + +/** `src/commands/i18n/check.ts` → `i18n check`, the id oclif dispatches on. */ +function commandId(abs: string): string { + const rel = relative(COMMANDS_DIR, abs).replace(/\.ts$/, ''); + return rel.split(sep).filter((p) => p !== 'index').join(' '); +} + +/** + * The family, read off the source rather than remembered: a command belongs iff + * it offers a machine-readable mode AND reaches the config helper — directly, + * or by extending a command that does. + * + * The discovery has two halves because the reach has two shapes: + * + * • DIRECT — the module declares `json: Flags.boolean(` and imports from + * `utils/config.js`. + * • ALIAS — the module's default export `extends` a command in the direct + * set, so it inherits both the flag and the reach without naming either. + * `os build` is exactly this (`class Build extends Compile`), and a + * one-half discovery would have missed it: the original card's static + * reading listed nine modules, and `build` is the tenth face. + */ +export function discoverConfigMissFamily(): string[] { + const files = commandFiles(COMMANDS_DIR); + const sources = new Map(files.map((abs) => [abs, readFileSync(abs, 'utf-8')])); + + const direct = new Set(); + for (const [abs, src] of sources) { + if (!/\bjson:\s*Flags\.boolean\(/.test(src)) continue; + if (!/from '(?:\.\.\/)+utils\/config\.js'/.test(src)) continue; + direct.add(abs); + } + + // An alias inherits the flag and the reach from the class it extends, and + // names neither itself. Resolve `extends ` back to the module the + // identifier was imported from, and take the member if that module is in the + // direct set. One level is enough for the aliases in this tree and a deeper + // chain would show up as a discovery mismatch rather than pass silently. + const alias = new Set(); + for (const [abs, src] of sources) { + const ext = /export default class \w+ extends (\w+)\b/.exec(src); + if (!ext) continue; + const imported = new RegExp(`import ${ext[1]} from '(\\.[^']+)\\.js'`).exec(src); + if (!imported) continue; + const target = resolve(abs, '..', `${imported[1]}.ts`); + if (direct.has(target)) alias.add(abs); + } + + return [...direct, ...alias].map(commandId).sort(); +} diff --git a/packages/cli/test/json-stdout-purity.e2e.test.ts b/packages/cli/test/json-stdout-purity.e2e.test.ts index af39210b78..1a6e822c5c 100644 --- a/packages/cli/test/json-stdout-purity.e2e.test.ts +++ b/packages/cli/test/json-stdout-purity.e2e.test.ts @@ -63,6 +63,7 @@ import { tmpdir } from 'node:os'; import { join, resolve, relative, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; import { childEnv } from './helpers/serve-process.js'; +import { CONFIG_MISS_FAMILY, discoverConfigMissFamily } from './helpers/config-miss-family.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); const CLI = resolve(HERE, '../bin/run-dev.js'); @@ -207,6 +208,30 @@ describe('the family this contract has to hold across', () => { it('is exactly the set listed here — a new member goes red until it is driven too', () => { expect(discoverFamily()).toEqual(Object.keys(FAMILY).sort()); }); + + it('and the commands that fail BEFORE boot are covered too, by the sibling pin', () => { + // [#15547] THIS file's discovery is `bootSchemaStack`-based, so it is + // structurally blind to a command that refuses at `resolveConfigPath()` — + // above the kernel, with no boot to discover. Ten published `--json` faces + // sat in that gap while `--json` stdout was, on paper, pinned. + // + // ⛔ The repair is NOT to widen the discovery above: these ten emit no boot + // diagnostics, so driving them through `BOOT_DIAGNOSTICS` would assert + // lines they never write. The POPULATION is widened across the PAIR + // instead — `config-miss-stdout-purity.e2e.test.ts` drives them, this + // reconciliation names them, and the shared discovery lives in one module + // so neither file can lose the other's half silently. Delete the sibling's + // helper and this import stops resolving; shrink the discovery and this + // assertion goes red. + const preBoot = discoverConfigMissFamily(); + expect(preBoot).toEqual(Object.keys(CONFIG_MISS_FAMILY).sort()); + expect(preBoot).toHaveLength(10); + + // Disjoint by construction, and worth asserting: a command appearing in + // BOTH would be driven twice under contradictory expectations, and the two + // files would start disagreeing about which one owns its verdict. + expect(preBoot.filter((id) => id in FAMILY)).toEqual([]); + }); }); describe.each(Object.keys(FAMILY))('os %s --json', (id) => { From 1e1e7ae8402fc1a32abcc0c331a129731f5de8a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 12:00:37 +0000 Subject: [PATCH 3/4] =?UTF-8?q?test(cli):=20the=20two=20purity=20families?= =?UTF-8?q?=20overlap=20on=20migrate=20meta=20=E2=80=94=20pin=20the=20argv?= =?UTF-8?q?,=20not=20disjointness?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cli/test/json-stdout-purity.e2e.test.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/cli/test/json-stdout-purity.e2e.test.ts b/packages/cli/test/json-stdout-purity.e2e.test.ts index 1a6e822c5c..7958e0a1c3 100644 --- a/packages/cli/test/json-stdout-purity.e2e.test.ts +++ b/packages/cli/test/json-stdout-purity.e2e.test.ts @@ -227,10 +227,21 @@ describe('the family this contract has to hold across', () => { expect(preBoot).toEqual(Object.keys(CONFIG_MISS_FAMILY).sort()); expect(preBoot).toHaveLength(10); - // Disjoint by construction, and worth asserting: a command appearing in - // BOTH would be driven twice under contradictory expectations, and the two - // files would start disagreeing about which one owns its verdict. - expect(preBoot.filter((id) => id in FAMILY)).toEqual([]); + // The two families are NOT disjoint, and measuring that was worth more + // than assuming it: `os migrate meta` is in both, legitimately and by + // design — it boots a kernel under `--stored` and refuses at + // `resolveConfigPath()` under `--from N`. So a shared MEMBER is fine and + // pinned; what must never happen is the two files driving the same + // INVOCATION and disagreeing about its verdict, which is an argv question. + const overlap = preBoot.filter((id) => id in FAMILY); + expect(overlap).toEqual(['migrate meta']); + for (const id of overlap) { + const bootArgv = FAMILY[id].join(' '); + const preBootMember = CONFIG_MISS_FAMILY[id]; + for (const argv of [preBootMember.explicit, preBootMember.auto]) { + if (argv) expect(argv.join(' ')).not.toBe(bootArgv); + } + } }); }); From 2004e5a6c675817f28648a4b15798211059f43c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 12:36:16 +0000 Subject: [PATCH 4/4] fix(cli): write the ESC byte as an escape sequence, not the raw byte --- packages/cli/src/utils/config.ts | 3 ++- packages/cli/test/config-miss-stdout-purity.e2e.test.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/utils/config.ts b/packages/cli/src/utils/config.ts index b7ffa1aa4d..5b1c97865b 100644 --- a/packages/cli/src/utils/config.ts +++ b/packages/cli/src/utils/config.ts @@ -46,7 +46,8 @@ export const BUNDLE_REQUIRE_EXTERNALS: (string | RegExp)[] = [ * * • `message` — PLAIN. It is what every `--json` catch-all copies into its * envelope, so it must not carry terminal decoration: `chalk.white(abs)` - * in a payload is `…` inside a JSON string the moment + * in a payload is an ESC-bracket-37m / ESC-bracket-39m pair inside a JSON + * string the moment * the run happens to have colour on. * • `display` — the same sentence WITH that decoration, for the stream a * human reads. Defaults to `message` where there is nothing to decorate. diff --git a/packages/cli/test/config-miss-stdout-purity.e2e.test.ts b/packages/cli/test/config-miss-stdout-purity.e2e.test.ts index 7ab16c441f..99a7f22b7a 100644 --- a/packages/cli/test/config-miss-stdout-purity.e2e.test.ts +++ b/packages/cli/test/config-miss-stdout-purity.e2e.test.ts @@ -190,7 +190,8 @@ describe.each(cases())('os %s --json, config missing', (key) => { // envelope must carry the PLAIN sentence: an escape sequence inside a JSON // string is a defect a consumer cannot see coming, and it would appear // only in runs that happen to have colour on. - expect(runOf().stdout).not.toContain('['); + // eslint-disable-next-line no-control-regex + expect(runOf().stdout).not.toMatch(/\u001b\[/); }); it('keeps the human refusal off stdout entirely', () => {