Skip to content

Commit bf36edd

Browse files
fix(cli): os init and os compile render each refusal once, not once on stdout and again as oclif's Error block (#21542) (#21560)
Fixes #21542 Clause-②: no `os init` and `os compile` printed ten refusals twice: once as the command's own `✗` line on stdout, and again as oclif's `Error:` block on stderr, because the command handed the same sentence to `this.error`. Each now prints its `✗` line and the hint under it once, and ends in `this.exit(2)`, the status `this.error` raised. Stdout is unchanged; stderr no longer repeats it; every exit status is still 2. ## The shape, and why it is this one Triage's ruling asks for one shape at every site, through the split `isReportedError` guards, with no second mechanism and no per-site variant. The shape the family's text faces already use is: the command renders its own refusal with `printError`, and ends in `this.exit(n)`. `validate`, `info`, `diff`, `lint`, `verify`, `i18n check`, `i18n extract`, `generate` and `migrate meta` all end that way, and `isReportedError` is the guard a catch-all uses for a refusal a helper already wrote to stderr. This PR applies that shape at all ten sites. No helper is added, and `utils/format.ts` is read, not edited. - **Which stream the one copy is on.** The dispatch read the split as "the human text goes on stderr once". That is the helper half of it: `printErrorToStderr`'s docblock says a command that has already decided it renders the text face keeps `printError` on stdout, and stderr is for shared paths that cannot see `--json` (`resolveConfigPath`). `os init` declares no `--json` at all, and `os compile`'s `--json` branch returns above every site here. So the surviving copy stays where the `✗` line and its hint always were. Stdout already held everything stderr carried, so nothing is lost. - **Why `init.ts`'s catch-all has no `isReportedError` guard.** `compile.ts`'s catch-all already carries it, because `resolveConfigPath()` can reach it. Only `ConfigRefusalError` (`config.ts`) and the SDUI manifest error (`sdui-manifest.ts`) carry the marker, and nothing in `init.ts`'s `try` reaches either: it imports neither, and `validateScaffold` has its own loader. A guard there would be a branch no run can take. - **Why exit 2.** `this.error(msg)` exits 2 and `this.exit(2)` is the same status. The dispatch's A4 and the ruling's second pin both say the statuses stay, and the platform checklist already names oclif's 2 on `os compile`'s human path as legitimate. ## Population: a symbol walk, not a grep ts-morph over `packages/cli/src/commands` at `fd5a1cd597` with the type checker: calls resolving to oclif's `Command.error`, and calls resolving to `printError` or `printErrorToStderr` in `utils/format.ts`, grouped by outermost function. - 16 `this.error` call sites in 5 files; 57 command files call a refusal printer. Functions holding both: 2, `init.ts run()` and `compile.ts run()`. - Those two hold 10 `this.error` sites: the card's 8, plus init's scaffold self-test and dependency install refusals inside its `try`. The ruling's third bullet makes any pair beyond the 8 ride this landing, and they do. - The three `datasource` commands (`introspect`, `list-tables`, `validate`) hold the other 6 `this.error` sites and call no refusal printer. Their detail lines go through `this.log` and the one sentence is oclif's, so they are not pairs. - After this PR the same walk finds 0 functions holding both, and 6 `this.error` sites in 3 files. ## Readings at the public door Spawned `bin/run-dev.js` (the source entry; it shares the published entry's oclif `handle()` and `flush()`) from a scratch directory, before and after. Before, the source entry prints oclif's stack beneath the same sentence; the published entry prints `› Error: …`, as the card measured it. ```text os init demo -t bogus before: stdout " ✗ Unknown template: bogus" + " Available: app, plugin, empty" stderr "Error: Unknown template: bogus" + stack exit 2 after: stdout the same two lines exit 2, stderr empty os compile (config throws at load) before: stdout " ✗ probe: the config module threw at load" stderr "Error: probe: the config module threw at load" + stack exit 2 after: stdout the same line exit 2, stderr empty ``` All ten sites, each reached through a real run (fixtures are in the e2e pin's header). Copies of the sentence across both streams, and the exit status: | Site | Reached by | Copies | Exit | | --- | --- | --- | --- | | init: unknown template | `init demo -t bogus` | 2 to 1 | 2 to 2 | | init: invalid project name | `init Bad_Name` | 2 to 1 | 2 to 2 | | init: target not empty | `init demo` over a non-empty `demo/` | 2 to 1 | 2 to 2 | | init: current directory name invalid | `init` in a directory named `Bad Cwd` | 2 to 1 | 2 to 2 | | init: config already exists | `init` beside an `objectstack.config.ts` | 2 to 1 | 2 to 2 | | init: scaffold self-test rejects (in the try) | `init demo -p npm`, stub `npm` that installs nothing | 2 to 1 | 2 to 2 | | init: dependency install fails (in the try) | `init demo -p npm`, stub `npm` exiting 1 | 2 to 1 | 2 to 2 | | init: catch-all | `init --no-install` with a file named `src` | 2 to 1 | 2 to 2 | | compile: runtime bundle refused | `compile --runtime-bundle`, config importing `./helper` that only `helper.jsx` satisfies | 2 to 1 | 2 to 2 | | compile: catch-all | `compile`, config throwing at load | 2 to 1 | 2 to 2 | `os build` extends `Compile` and has no refusal of its own, so it inherits both compile rows. The published `bin/run.js`, built from this tree, was run for the init unknown-template and compile catch-all rows too: exit 2, stdout once, stderr empty. ## Pins - `packages/cli/test/refusal-renders-once.e2e.test.ts` (nightly): spawns the CLI through each of the ten sites and asserts the exit status is 2, the refusal's subject occurs once across both streams, exactly one `✗` line is printed, and the hint under it survives. Every spawn is paid in `beforeAll`; no case is clocked. - `packages/cli/test/refusal-renders-once.test.ts` (queue, unit): the structural half over every command module. No function that calls a refusal printer (an identifier bound by an import from `utils/format.js`, aliases followed) also calls `this.error`. Nine fixtures pin the scan, and three floors keep it from going vacuous (60 command modules, 50 files calling a printer, 3 files raising `this.error`), set under what the symbol walk counted on this tree: 65, 57 and 3. The population is discovered, so the next command that repeats the shape is in it. - `packages/cli/test/exit-signal.pin.test.ts`: its `this.error` real-site anchor followed the sites to `this.exit(2)` (init's two in-try refusals, compile's and build's bundling refusal). `SITE_FLOOR` stays 131 (both spellings are seeds), and the pin stays green; the `this.error` seed itself is still covered by its fixtures. ## Reverse verification Run on the committed fix, through `scripts/ablation-replace.mjs` in wrap mode under the lock, restoring one site's pairing at a time. Both halves read the source (`bin/run-dev.js` runs from `src/`, the structural pin reads text), so no `dist/` is on the path and no rebuild leg applies. - Leg A, `init.ts` unknown-template site back to `this.error(...)`: anchor 1 to 0, blob `929642fba1ff` to `edab7c113e65`. Structural pin red (1 failed, naming `init.ts run()`); e2e red on exactly that case (1 failed, 9 passed: "expected 2 to be 1"); exit-signal pin green. Restore proven: blob equals HEAD (`929642fba1ff`) and `git diff HEAD` is empty. - Leg B, `compile.ts` runtime-bundle site back to `this.error(err.message)`: anchor 1 to 0, blob `25d3ea870831` to `42e44c372e8e`. Structural pin red (2 failed: the pair, and the exit-signal anchor "each ends in `this.exit(2)`"); e2e red on exactly the bundling case (1 failed, 9 passed). Restore proven: blob equals HEAD (`25d3ea870831`), `git diff HEAD` empty, `git status` clean. - Direction observed: red in both legs, the normal one. ## Verification At `d83eb007b0` unless noted; every heavy run went through `os-verify-lock.sh`. - `pnpm --filter @objectstack/cli typecheck`: exit 0 (`tsc --noEmit`, and `check:test-typecheck` OK with its ledger unchanged: 3 files, 28 errors). - Unit: `refusal-renders-once.test.ts`, `exit-signal.pin.test.ts` and `vitest-tiers-partition.test.ts`: 3 files, 142 tests passed. Driven pin with `OS_TEST_TIERS=nightly`: 10 of 10 passed. - Every test file that drives `os init`, `os compile` or `os build`, selected by import of the command module or by spawn argv (run at `228259e373`; the later commit changes one comment line in a test file): 31 queue-tier files, 485 passed and 6 failed, all 6 in `published-subpath-console.pin.test.ts`. Those six failed because this worktree's `packages/cli/dist` had been built with `OS_SKIP_DTS=1` and carried no declarations; after a declaration build the file passed 14 of 14. 18 nightly e2e files with `OS_TEST_TIERS=nightly`: 210 passed. - Gates: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, no paths, at `d83eb007b0`: 66 derived, all 66 run. Three first answered exit 3 or 124 for environment reasons (`check:dual-build-cjs-loads` and `check:i18n-coverage` said PREREQUISITE NOT MET because packages outside the cli closure had no `dist/`; `check:type-check-debt` hit my own 420 s cap). After building the missing packages with declarations and a longer cap, all three exited 0. `--ran`: 66 derived, 66 run, 0 NOT-MEASURED, 0 UNRUN. - Lint, as a proven narrowing: ESLint with inline config disabled over the 5 touched TypeScript files, population read from ESLint's own config (`isPathIgnored` and `calculateConfigForFile`): 5 files in the JSON results, 0 errors, 0 warnings. The `.changeset` file is outside ESLint's population. Type-aware linting is off for each file (no `parserOptions.project`; the config header says it is never enabled), and the diff touches no ESLint config, so it cannot move a verdict on an untouched file. Whole-repo `pnpm lint` is left to CI. - Not re-derived on a fresh tree: the branch is based on `fd5a1cd597` and `origin/main` is 7 commits ahead. The derivation tool reports one input changed upstream, `scripts/engine-double-contract.pinned.json` (the `check:engine-double-contract` family, which ran green here; this diff adds no fake engine). The upstream commits touch `packages/cli` only under `migrate/` and two utils, and add no `printError` or `this.error` line under `src/commands`. ## Acceptance notes - The declaration in `utils/format.ts` above `CliExitCode` reads "The only two exit codes this CLI has: 0 success, 1 failure". Ten refusals exit 2, as they always did, and the platform checklist (`docs/qa/platform-checklist/areas/cli.json`) blesses oclif's 2 on `os compile`'s human path. This PR keeps 2, as the ruling's pin says. Noted, not filed: the docblock's scope is the `emitJson` slot's type. - The same checklist file describes that exit 2 as coming from `compile.ts`'s `this.error()` in seven strings. The status it blesses is unchanged; only the named mechanism now reads stale. Not edited here (outside this card's file surface, and the file carries a revision history of its own). - `os compile`'s catch-all still answers exit 1 for a refusal a helper already wrote (the `isReportedError` branch) and exit 2 for any other. That asymmetry predates this PR and is kept. --- _Generated by [Claude Code](https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f9f9f91 commit bf36edd

6 files changed

Lines changed: 578 additions & 24 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
fix(cli): `os init` and `os compile` render each refusal once, not once on stdout and again as oclif's `Error:` block on stderr (#21542)
6+
7+
Clause-②: no
8+
9+
`os init demo -t bogus` printed `✗ Unknown template: bogus` on stdout, then the same sentence as oclif's `Error:` block on stderr, and exited 2. Ten refusals did it: the five `os init` makes before it writes anything (an unknown template, a project name that is not valid, a target directory that is not empty, a current directory whose name is not a valid project name, an `objectstack.config.ts` that already exists), its scaffold self-test and dependency install, its catch-all, and `os compile`'s runtime-bundle refusal and catch-all (`os build` inherits both). Each printed its own `✗` line and then handed the sentence to `this.error`, which has oclif's entry point render it again.
10+
11+
Each now prints its `✗` line and the hint under it once, and ends in `this.exit(2)`: the status `this.error` raised, with nothing rendered by the entry point. Stdout carries the same lines as before; stderr no longer repeats them. Exit statuses are unchanged: 2 for all ten.
12+
13+
A script that read the sentence from stderr, from the `Error:` block, now finds it on stdout, on the `✗` line, which is where the full wording and the hint always were.

‎packages/cli/src/commands/compile.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,9 +1041,14 @@ export default class Compile extends Command {
10411041
await emitJson({ success: false, error: `runtime bundle failed: ${err.message}`, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
10421042
this.exit(1);
10431043
}
1044+
// The `✗` line below is this refusal's one rendering. It used to end
1045+
// in `this.error(err.message)`, which has oclif's entry point render
1046+
// the same message again as an `Error:` block on stderr; `this.exit(2)`
1047+
// raises the same signal with the status `this.error` raised, and
1048+
// renders nothing.
10441049
console.log('');
10451050
printError(`Runtime bundle failed: ${err.message}`);
1046-
this.error(err.message);
1051+
this.exit(2);
10471052
}
10481053
}
10491054
}
@@ -1204,17 +1209,20 @@ export default class Compile extends Command {
12041209
printAdvisoriesOnce();
12051210
// [#15547] `resolveConfigPath()` already wrote its refusal and hint lines
12061211
// to stderr before throwing, so this face has nothing left to render —
1207-
// and `this.error()` below is NOT a no-op for it: it re-renders the same
1208-
// sentence as an oclif `› Error:` block AND raises this face's exit
1212+
// and an oclif `this.error()` here is NOT a no-op for it: it re-renders
1213+
// the same sentence as an `› Error:` block AND raises this face's exit
12091214
// status from 1 to 2. Measured on the published entry, `os compile
12101215
// ./missing.ts` (and `os build`, which inherits this catch): exit 2 with
12111216
// 483 stderr bytes, where the other eight faces answer exit 1 with 296.
12121217
// `this.exit(1)` throws the ExitError the `--json` branch already relies
12131218
// on, so the status and the bytes both stay where they were.
12141219
if (isReportedError(error)) this.exit(1);
1220+
// Any other failure is rendered here, once, by `printError`, and ends in
1221+
// `this.exit(2)`: the status the `this.error()` that stood here raised
1222+
// (its entry-point `Error:` block was the same sentence a second time).
12151223
console.log('');
12161224
printError(error.message || String(error));
1217-
this.error(error.message || String(error));
1225+
this.exit(2);
12181226
}
12191227
}
12201228
}

‎packages/cli/src/commands/init.ts‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,10 +1148,18 @@ export default class Init extends Command {
11481148
const startCwd = process.cwd();
11491149
const template = TEMPLATES[flags.template];
11501150

1151+
// Every refusal in this command renders its sentence ONCE: the `✗` line it
1152+
// prints itself, with the hint under it, and then `this.exit(2)`. It used to
1153+
// end in `this.error(<the same sentence>)`, which hands oclif's entry point
1154+
// the sentence to render a second time as an `Error:` block on stderr — one
1155+
// refusal read twice across two streams. `this.exit(n)` raises the same
1156+
// signal and renders nothing, and `2` is the status `this.error` raised, so
1157+
// the exit status is unchanged. That is the split `isReportedError` guards
1158+
// (`utils/format.ts`): one rendering per refusal.
11511159
if (!template) {
11521160
printError(`Unknown template: ${flags.template}`);
11531161
console.log(chalk.dim(` Available: ${Object.keys(TEMPLATES).join(', ')}`));
1154-
this.error(`Unknown template: ${flags.template}`);
1162+
this.exit(2);
11551163
}
11561164

11571165
// Resolve target directory + project name.
@@ -1169,7 +1177,7 @@ export default class Init extends Command {
11691177
const nameError = validateProjectName(args.name);
11701178
if (nameError) {
11711179
printError(nameError);
1172-
this.error(nameError);
1180+
this.exit(2);
11731181
}
11741182
projectName = args.name;
11751183
targetDir = path.resolve(startCwd, args.name);
@@ -1179,7 +1187,7 @@ export default class Init extends Command {
11791187
const msg = `Target directory ${targetDir} is not empty`;
11801188
printError(msg);
11811189
console.log(chalk.dim(' Choose a different name or remove the existing directory first.'));
1182-
this.error(msg);
1190+
this.exit(2);
11831191
}
11841192
} else {
11851193
fs.mkdirSync(targetDir, { recursive: true });
@@ -1191,15 +1199,15 @@ export default class Init extends Command {
11911199
if (nameError) {
11921200
printError(`Current directory name "${projectName}" is not a valid project name. ${nameError}`);
11931201
console.log(chalk.dim(' Re-run with an explicit name: `objectstack init my-app`'));
1194-
this.error(nameError);
1202+
this.exit(2);
11951203
}
11961204
}
11971205

11981206
// Check for existing config
11991207
if (fs.existsSync(path.join(targetDir, 'objectstack.config.ts'))) {
12001208
printError(`objectstack.config.ts already exists in ${targetDir}`);
12011209
console.log(chalk.dim(' Use `objectstack generate` to add metadata to an existing project'));
1202-
this.error('objectstack.config.ts already exists');
1210+
this.exit(2);
12031211
}
12041212

12051213
// Convert the npm-name (which allows hyphens, dots, scopes) into a
@@ -1357,7 +1365,7 @@ export default class Init extends Command {
13571365

13581366
if (scaffoldRejected) {
13591367
console.log(chalk.dim(' This is a CLI bug — please report it at https://github.com/objectstack-ai/objectstack/issues'));
1360-
this.error('Scaffold validation failed');
1368+
this.exit(2);
13611369
}
13621370
}
13631371

@@ -1386,16 +1394,16 @@ export default class Init extends Command {
13861394
}
13871395
console.log(chalk.dim(` ${chosenPm} install`));
13881396
console.log('');
1389-
this.error('Dependency installation failed');
1397+
this.exit(2);
13901398
}
13911399

13921400
} catch (error: any) {
13931401
// The two refusals above (scaffold self-test, dependency install) already
1394-
// printed their `✗` line and raised the exit signal with `this.error`.
1402+
// printed their `✗` line and raised the exit signal with `this.exit(2)`.
13951403
// Re-reporting it here printed the refusal a second time.
13961404
if (isExitSignal(error)) throw error;
13971405
printError(error.message || String(error));
1398-
this.error(error.message || String(error));
1406+
this.exit(2);
13991407
}
14001408
}
14011409
}

‎packages/cli/test/exit-signal.pin.test.ts‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,9 @@
112112
* directory a scratch directory. Each case asserts the two things an operator
113113
* reads: the refusal is ONE `✗` line with no `EEXIT` anywhere in the output,
114114
* and the exit status, unchanged by the repair — 1 for the `this.exit(1)`
115-
* refusals, 2 for `os init`'s `this.error` ones.
115+
* refusals, 2 for `os init`'s `this.exit(2)` ones (the status its `this.error`
116+
* refusals raised, until they rendered their sentence once:
117+
* `refusal-renders-once.test.ts` and its `.e2e` twin).
116118
*
117119
* ## Tier
118120
*
@@ -480,7 +482,9 @@ const FLOW = new Map(
480482
* `bee8d1c62c` over the same 65 commands: the 127 `this.exit` sites, plus
481483
* four `this.error` sites — `compile.ts`'s bundling refusal, counted for
482484
* `os compile` and again for `os build` through it, and `os init`'s two
483-
* refusals inside its outer `try`.
485+
* refusals inside its outer `try`. Those four sites now spell `this.exit(2)`
486+
* (the status `this.error` raised, with the sentence rendered once): both
487+
* spellings are seeds, so the floor is unchanged.
484488
*/
485489
const POPULATION_FLOOR = 65;
486490
const SITE_FLOOR = 131;
@@ -648,14 +652,13 @@ describe('every command lets the exit signal through', () => {
648652
}
649653
});
650654

651-
it("the scan reaches the `this.error` seed — `os init`'s two refusals inside its outer try, and `os compile`'s bundling refusal", () => {
652-
const errorCalls = (id: string) => FLOW.get(id)?.sites.map((s) => s.call).filter((call) => call.startsWith('this.error(')) ?? [];
653-
expect(errorCalls('init')).toEqual(expect.arrayContaining([
654-
"this.error('Scaffold validation failed')",
655-
"this.error('Dependency installation failed')",
656-
]));
655+
it("the scan reaches `os init`'s two refusals inside its outer try, and `os compile`'s bundling refusal — each ends in `this.exit(2)`", () => {
656+
const refusalCalls = (id: string) => FLOW.get(id)?.sites.map((s) => s.call).filter((call) => call === 'this.exit(2)') ?? [];
657+
// Scaffold self-test and dependency install: both sit inside the outer `try`
658+
// whose `catch` must let the signal through, so both are sites.
659+
expect(refusalCalls('init'), 'os init').toHaveLength(2);
657660
// Inherited: `os build` is judged on `compile.ts`'s site as well.
658-
for (const id of ['compile', 'build']) expect(errorCalls(id), `os ${id}`).toContain('this.error(err.message)');
661+
for (const id of ['compile', 'build']) expect(refusalCalls(id), `os ${id}`).toHaveLength(1);
659662
});
660663

661664
it.each(POPULATION.map((c) => [c.id, c.faces.join(' | ')]))('os %s (%s)', (id) => {
@@ -961,8 +964,8 @@ async function driveText(cmd: Runnable, argv: string[], cwd?: string): Promise<D
961964

962965
/**
963966
* The refusal is reported ONCE, the signal is never named, and the status is
964-
* the one it always was: 1 for a `this.exit(1)` refusal, 2 for a `this.error`
965-
* one (oclif's default, which `os init` never overrides). `subject` is what
967+
* the one it always was: 1 for a `this.exit(1)` refusal, 2 for `os init`'s
968+
* `this.exit(2)` ones (what its `this.error` refusals raised). `subject` is what
966969
* the one line must be about — a path, a URL or an error the case chose, never
967970
* the refusal's wording.
968971
*/

0 commit comments

Comments
 (0)