From f25c51804f7c8b7e63c7cf5538a425149ca9070b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 06:23:03 +0000 Subject: [PATCH 1/3] fix(cli): os init and os compile render each refusal once, not once on stdout and again as oclif's Error block Ten sites printed their sentence with printError and then handed it to this.error, which has oclif's entry point render it a second time on stderr. Each now ends in this.exit(2): the status this.error raised, with no second rendering. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz --- packages/cli/src/commands/compile.ts | 16 ++++++++++++---- packages/cli/src/commands/init.ts | 26 +++++++++++++++++--------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index 7d24c878b83..25d3ea87083 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -1041,9 +1041,14 @@ export default class Compile extends Command { await emitJson({ success: false, error: `runtime bundle failed: ${err.message}`, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true }); this.exit(1); } + // The `✗` line below is this refusal's one rendering. It used to end + // in `this.error(err.message)`, which has oclif's entry point render + // the same message again as an `Error:` block on stderr; `this.exit(2)` + // raises the same signal with the status `this.error` raised, and + // renders nothing. console.log(''); printError(`Runtime bundle failed: ${err.message}`); - this.error(err.message); + this.exit(2); } } } @@ -1204,17 +1209,20 @@ export default class Compile extends Command { printAdvisoriesOnce(); // [#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 + // and an oclif `this.error()` here is NOT a no-op for it: it re-renders + // the same sentence as an `› 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); + // Any other failure is rendered here, once, by `printError`, and ends in + // `this.exit(2)`: the status the `this.error()` that stood here raised + // (its entry-point `Error:` block was the same sentence a second time). console.log(''); printError(error.message || String(error)); - this.error(error.message || String(error)); + this.exit(2); } } } diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index 4930c989f59..929642fba1f 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -1148,10 +1148,18 @@ export default class Init extends Command { const startCwd = process.cwd(); const template = TEMPLATES[flags.template]; + // Every refusal in this command renders its sentence ONCE: the `✗` line it + // prints itself, with the hint under it, and then `this.exit(2)`. It used to + // end in `this.error()`, which hands oclif's entry point + // the sentence to render a second time as an `Error:` block on stderr — one + // refusal read twice across two streams. `this.exit(n)` raises the same + // signal and renders nothing, and `2` is the status `this.error` raised, so + // the exit status is unchanged. That is the split `isReportedError` guards + // (`utils/format.ts`): one rendering per refusal. if (!template) { printError(`Unknown template: ${flags.template}`); console.log(chalk.dim(` Available: ${Object.keys(TEMPLATES).join(', ')}`)); - this.error(`Unknown template: ${flags.template}`); + this.exit(2); } // Resolve target directory + project name. @@ -1169,7 +1177,7 @@ export default class Init extends Command { const nameError = validateProjectName(args.name); if (nameError) { printError(nameError); - this.error(nameError); + this.exit(2); } projectName = args.name; targetDir = path.resolve(startCwd, args.name); @@ -1179,7 +1187,7 @@ export default class Init extends Command { const msg = `Target directory ${targetDir} is not empty`; printError(msg); console.log(chalk.dim(' Choose a different name or remove the existing directory first.')); - this.error(msg); + this.exit(2); } } else { fs.mkdirSync(targetDir, { recursive: true }); @@ -1191,7 +1199,7 @@ export default class Init extends Command { if (nameError) { printError(`Current directory name "${projectName}" is not a valid project name. ${nameError}`); console.log(chalk.dim(' Re-run with an explicit name: `objectstack init my-app`')); - this.error(nameError); + this.exit(2); } } @@ -1199,7 +1207,7 @@ export default class Init extends Command { if (fs.existsSync(path.join(targetDir, 'objectstack.config.ts'))) { printError(`objectstack.config.ts already exists in ${targetDir}`); console.log(chalk.dim(' Use `objectstack generate` to add metadata to an existing project')); - this.error('objectstack.config.ts already exists'); + this.exit(2); } // Convert the npm-name (which allows hyphens, dots, scopes) into a @@ -1357,7 +1365,7 @@ export default class Init extends Command { if (scaffoldRejected) { console.log(chalk.dim(' This is a CLI bug — please report it at https://github.com/objectstack-ai/objectstack/issues')); - this.error('Scaffold validation failed'); + this.exit(2); } } @@ -1386,16 +1394,16 @@ export default class Init extends Command { } console.log(chalk.dim(` ${chosenPm} install`)); console.log(''); - this.error('Dependency installation failed'); + this.exit(2); } } catch (error: any) { // The two refusals above (scaffold self-test, dependency install) already - // printed their `✗` line and raised the exit signal with `this.error`. + // printed their `✗` line and raised the exit signal with `this.exit(2)`. // Re-reporting it here printed the refusal a second time. if (isExitSignal(error)) throw error; printError(error.message || String(error)); - this.error(error.message || String(error)); + this.exit(2); } } } From 228259e373c5a082fd07c56aa0c3eb33bee94b62 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 06:29:56 +0000 Subject: [PATCH 2/3] test(cli): pin that each os init and os compile refusal renders once and keeps exit 2 A structural half over every command module (no refusal printer paired with this.error) and a driven half that spawns the CLI through all ten sites. The exit-signal pin's this.error anchor follows the sites to this.exit(2). Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz --- .changeset/21542-refusal-renders-once.md | 13 + packages/cli/test/exit-signal.pin.test.ts | 25 +- .../cli/test/refusal-renders-once.e2e.test.ts | 277 ++++++++++++++++++ .../cli/test/refusal-renders-once.test.ts | 245 ++++++++++++++++ 4 files changed, 549 insertions(+), 11 deletions(-) create mode 100644 .changeset/21542-refusal-renders-once.md create mode 100644 packages/cli/test/refusal-renders-once.e2e.test.ts create mode 100644 packages/cli/test/refusal-renders-once.test.ts diff --git a/.changeset/21542-refusal-renders-once.md b/.changeset/21542-refusal-renders-once.md new file mode 100644 index 00000000000..3eb38729483 --- /dev/null +++ b/.changeset/21542-refusal-renders-once.md @@ -0,0 +1,13 @@ +--- +'@objectstack/cli': patch +--- + +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) + +Clause-②: no + +`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. + +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. + +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. diff --git a/packages/cli/test/exit-signal.pin.test.ts b/packages/cli/test/exit-signal.pin.test.ts index a405f9530ca..b3332ffc9ea 100644 --- a/packages/cli/test/exit-signal.pin.test.ts +++ b/packages/cli/test/exit-signal.pin.test.ts @@ -112,7 +112,9 @@ * directory a scratch directory. Each case asserts the two things an operator * reads: the refusal is ONE `✗` line with no `EEXIT` anywhere in the output, * and the exit status, unchanged by the repair — 1 for the `this.exit(1)` - * refusals, 2 for `os init`'s `this.error` ones. + * refusals, 2 for `os init`'s `this.exit(2)` ones (the status its `this.error` + * refusals raised, until they rendered their sentence once: + * `refusal-renders-once.test.ts`). * * ## Tier * @@ -480,7 +482,9 @@ const FLOW = new Map( * `bee8d1c62c` over the same 65 commands: the 127 `this.exit` sites, plus * four `this.error` sites — `compile.ts`'s bundling refusal, counted for * `os compile` and again for `os build` through it, and `os init`'s two - * refusals inside its outer `try`. + * refusals inside its outer `try`. Those four sites now spell `this.exit(2)` + * (the status `this.error` raised, with the sentence rendered once): both + * spellings are seeds, so the floor is unchanged. */ const POPULATION_FLOOR = 65; const SITE_FLOOR = 131; @@ -648,14 +652,13 @@ describe('every command lets the exit signal through', () => { } }); - it("the scan reaches the `this.error` seed — `os init`'s two refusals inside its outer try, and `os compile`'s bundling refusal", () => { - const errorCalls = (id: string) => FLOW.get(id)?.sites.map((s) => s.call).filter((call) => call.startsWith('this.error(')) ?? []; - expect(errorCalls('init')).toEqual(expect.arrayContaining([ - "this.error('Scaffold validation failed')", - "this.error('Dependency installation failed')", - ])); + 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)`", () => { + const refusalCalls = (id: string) => FLOW.get(id)?.sites.map((s) => s.call).filter((call) => call === 'this.exit(2)') ?? []; + // Scaffold self-test and dependency install: both sit inside the outer `try` + // whose `catch` must let the signal through, so both are sites. + expect(refusalCalls('init'), 'os init').toHaveLength(2); // Inherited: `os build` is judged on `compile.ts`'s site as well. - for (const id of ['compile', 'build']) expect(errorCalls(id), `os ${id}`).toContain('this.error(err.message)'); + for (const id of ['compile', 'build']) expect(refusalCalls(id), `os ${id}`).toHaveLength(1); }); 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 install` sites put a stub `npm` first on `PATH` (the command runs + * `execSync('npm install')`): one that exits 1, and one that exits 0 having + * created an EMPTY `node_modules/@objectstack/spec`, so the scaffold's own + * self-test cannot resolve the protocol package — on this tree or any other, + * since a directory that exists shadows every copy further up. The catch-all + * is reached by a FILE named `src` where the template needs a directory. The + * runtime-bundle refusal is reached by a config that imports `./helper`, which + * only `helper.jsx` satisfies: the config loader resolves `.jsx`, the runtime + * bundle's `resolveExtensions` does not, so the load succeeds and the bundle + * step refuses. If that stops being true the case fails on its first + * assertion (exit 0, `Build complete`) and says why, rather than passing on a + * refusal it never reached. + * + * ## Tier + * + * Nightly by name, `integration` by behaviour (`vitest-tiers.ts`): each case is + * a spawn. Every spawn is paid in `beforeAll`, sequentially — ten tsx starts at + * once is the load that turns a shared box's verdicts into timeouts — and no + * case is clocked. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { delimiter, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { childEnv } from './helpers/serve-process.js'; +import { linkSpec } from './helpers/define-stack-fixture.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(argv: string[], cwd: string, env: Record = {}): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...argv], + { cwd, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1', ...env }) }, + (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), + }); + }, + ); + }); +} + +let root: string; + +/** A fresh empty directory under the scratch root. */ +function dir(name: string): string { + const d = join(root, name); + mkdirSync(d, { recursive: true }); + return d; +} + +/** A directory holding an executable `npm` — put it first on PATH to stand in for the package manager. */ +function stubNpm(name: string, body: string): Record { + const bin = dir(name); + const file = join(bin, 'npm'); + writeFileSync(file, `#!/bin/sh\n${body}\n`); + chmodSync(file, 0o755); + return { PATH: `${bin}${delimiter}${process.env.PATH ?? ''}` }; +} + +const OBJECT = "{ name: 'rr_ticket', label: 'Ticket', sharingModel: 'private', fields: { title: { type: 'text', label: 'Title' } } }"; + +function stackConfig(hooks: string): string { + return `import { defineStack } from '@objectstack/spec'; +export default defineStack({ + manifest: { id: 'com.example.rr', name: 'rr', version: '1.0.0', type: 'app' }, + objects: [${OBJECT}], + hooks: [${hooks}], +}, { strict: false }); +`; +} + +interface Site { + /** `os : ` — the case's name. */ + name: string; + /** What the case needs on disk and on the command line; runs once, in `beforeAll`. */ + prepare: () => { argv: string[]; cwd: string; env?: Record }; + /** A fragment of the refusal, distinctive enough to count: one occurrence across both streams. */ + subject: RegExp; + /** The hint printed under the `✗` line, when the site prints one. */ + hint?: string; +} + +const SITES: Site[] = [ + { + name: 'os init: an unknown template', + prepare: () => ({ argv: ['init', 'demo', '-t', 'bogus'], cwd: dir('unknown-template') }), + subject: /Unknown template: bogus/g, + hint: 'Available: app, plugin, empty', + }, + { + name: 'os init: a project name that is not valid', + prepare: () => ({ argv: ['init', 'Bad_Name'], cwd: dir('bad-name') }), + subject: /Project name must be lowercase/g, + }, + { + name: 'os init: a target directory that is not empty', + prepare: () => { + const cwd = dir('target-not-empty'); + mkdirSync(join(cwd, 'demo')); + writeFileSync(join(cwd, 'demo', 'keep.txt'), 'already here'); + return { argv: ['init', 'demo'], cwd }; + }, + subject: /is not empty/g, + hint: 'Choose a different name or remove the existing directory first.', + }, + { + name: 'os init: the current directory name is not a valid project name', + prepare: () => ({ argv: ['init'], cwd: dir('Bad Cwd') }), + // The `✗` line carries this sentence inside a longer one; the entry point's copy carried it alone. + subject: /Project name must be lowercase/g, + hint: 'Re-run with an explicit name', + }, + { + name: 'os init: an objectstack.config.ts that already exists', + prepare: () => { + const cwd = dir('config-exists'); + writeFileSync(join(cwd, 'objectstack.config.ts'), '// already here\n'); + return { argv: ['init'], cwd }; + }, + subject: /objectstack\.config\.ts already exists/g, + hint: 'Use `objectstack generate` to add metadata to an existing project', + }, + { + name: 'os init: a scaffold its own self-test rejects (refused inside the try)', + prepare: () => ({ + argv: ['init', 'demo', '-p', 'npm'], + cwd: dir('scaffold-rejected'), + // An install that "succeeds" and leaves the protocol package unresolvable. + env: stubNpm('npm-installs-nothing', 'mkdir -p node_modules/@objectstack/spec'), + }), + subject: /Scaffold validation failed/g, + hint: 'This is a CLI bug', + }, + { + name: 'os init: a dependency install that fails (refused inside the try)', + prepare: () => ({ + argv: ['init', 'demo', '-p', 'npm'], + cwd: dir('install-failed'), + env: stubNpm('npm-fails', 'exit 1'), + }), + subject: /dependency installation failed/gi, + hint: 'To finish setup:', + }, + { + name: 'os init: any other failure (the catch-all)', + prepare: () => { + const cwd = dir('catch-all'); + // A file where the template needs the `src` directory. + writeFileSync(join(cwd, 'src'), 'a file where a directory is needed'); + return { argv: ['init', '--no-install'], cwd }; + }, + subject: /ENOTDIR/g, + }, + { + name: 'os compile: a runtime bundle that cannot be built', + prepare: () => { + const cwd = dir('bundle-refused'); + linkSpec(cwd); + writeFileSync(join(cwd, 'helper.jsx'), 'export const x = 1;\n'); + writeFileSync( + join(cwd, 'objectstack.config.ts'), + `import { x } from './helper';\n${stackConfig("{ name: 'rr_hook', object: 'rr_ticket', events: ['beforeInsert'], handler: async (ctx: any) => [x, ctx] }")}`, + ); + return { argv: ['compile', '--runtime-bundle'], cwd }; + }, + subject: /Could not resolve "\.\/helper"/g, + }, + { + name: 'os compile: any other failure (the catch-all)', + prepare: () => { + const cwd = dir('compile-catch-all'); + linkSpec(cwd); + writeFileSync(join(cwd, 'objectstack.config.ts'), "throw new Error('refusal-renders-once: the config module threw at load');\n"); + return { argv: ['compile'], cwd }; + }, + subject: /the config module threw at load/g, + }, +]; + +const RUNS = new Map(); + +beforeAll(async () => { + root = mkdtempSync(join(tmpdir(), 'os-refusal-once-')); + for (const site of SITES) { + const { argv, cwd, env } = site.prepare(); + RUNS.set(site.name, await runCli(argv, cwd, env)); + } +}, 900_000); + +afterAll(() => { + if (root) rmSync(root, { recursive: true, force: true }); +}); + +const occurrences = (text: string, subject: RegExp): number => text.match(subject)?.length ?? 0; + +describe('each refusal renders its sentence once, and keeps its exit status', () => { + it.each(SITES.map((s) => [s.name, s] as const))('%s', (name, site) => { + const run = RUNS.get(name)!; + const output = `${run.stdout}\n${run.stderr}`; + const shown = `\n--- stdout\n${run.stdout}\n--- stderr\n${run.stderr}`; + + // 1. The fixture reached the refusal, and the status is the one `this.error` raised. + expect(run.code, `the case never reached its refusal${shown}`).toBe(2); + + // 2. One rendering, not two across the two streams. + expect(occurrences(output, site.subject), `the sentence must be read once${shown}`).toBe(1); + + // 3. The copy that survives is the command's `✗` line. + expect(output.split('\n').filter((line) => /^\s*✗ /.test(line)), `exactly one \`✗\` line${shown}`).toHaveLength(1); + + // 4. Nothing the site printed under it was lost. + if (site.hint !== undefined) expect(output, `the hint under the refusal${shown}`).toContain(site.hint); + }); +}); diff --git a/packages/cli/test/refusal-renders-once.test.ts b/packages/cli/test/refusal-renders-once.test.ts new file mode 100644 index 00000000000..1455f399f3d --- /dev/null +++ b/packages/cli/test/refusal-renders-once.test.ts @@ -0,0 +1,245 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * PIN — a command that renders its refusal itself never also hands the sentence + * to oclif: no `printError(msg)` + `this.error(msg)` pair under `src/commands`. + * + * ## The defect + * + * `printError` writes the `✗` line on stdout. `this.error(msg)` throws a + * `CLIError`, and oclif's entry point renders that as its own `Error:` block on + * stderr. A refusal that did both was read twice, across two streams — + * `os init demo -t bogus` printed `✗ Unknown template: bogus` and then oclif's + * block with the same sentence, exit 2. Ten sites in `init.ts` and `compile.ts` + * did it; each now renders its `✗` line once and ends in `this.exit(2)`, the + * status `this.error` raised, which renders nothing. + * + * ## What this half pins, and what the other half pins + * + * This file is the STRUCTURAL half, over the WHOLE population: every function + * under `src/commands` that calls one of `utils/format.ts`'s refusal printers + * (`printError`, `printErrorToStderr`) must not also call `this.error`. It is + * decided from the syntax tree — a call to an identifier bound by an import + * from `utils/format.js` (aliases followed), and `this.error(…)` — never a text + * match, and the population is discovered, not listed, so a command added later + * is in it the moment its module exists. A pin naming only `init` and `compile` + * would stay green while the next command repeated the shape. + * + * The DRIVEN half is `refusal-renders-once.e2e.test.ts`: it spawns the CLI + * through each of the ten sites and counts the sentence across both streams and + * the exit status. That half is nightly (a spawn costs seconds apiece); this + * half costs milliseconds and runs in the queue, so the mechanism cannot come + * back between nights. + * + * ## What it does NOT judge + * + * A function that prints a refusal line and ends in `this.exit(n)` is the + * shape, and is not seen. A `this.error` in a function that prints nothing + * through those printers is the other legal shape — oclif renders the sentence + * and the command renders none — and is not seen either (`os datasource + * introspect` is one). Only the pair is the defect. + * + * ## Tier + * + * `unit` (`vitest-tiers.ts`): it reads source text and parses it; nothing is + * spawned and no kernel boots. + */ + +import { describe, it, expect } from 'vitest'; +import { readdirSync, readFileSync } from 'node:fs'; +import { dirname, join, relative, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const PKG_ROOT = resolve(HERE, '..'); +const COMMANDS_DIR = join(PKG_ROOT, 'src', 'commands'); + +const FORMAT_MODULE = /(?:^|\/)utils\/format\.js$/; +/** The `utils/format.ts` printers that write a refusal line (`✗ …`). */ +const REFUSAL_PRINTERS: ReadonlySet = new Set(['printError', 'printErrorToStderr']); + +interface RefusalPair { + /** The outermost method or function holding both calls. */ + fn: string; + printerLines: number[]; + errorLines: number[]; +} + +interface Scan { + pairs: RefusalPair[]; + /** `this.error(…)` calls seen, paired or not — the detector's own evidence. */ + errorCalls: number; + /** Calls to a refusal printer seen, paired or not. */ + printerCalls: number; +} + +/** + * Every function in `text` that calls a refusal printer AND `this.error`. + * Calls are grouped under the outermost method / function declaration they sit + * in, so a callback inside `run()` counts toward `run()`. + */ +function scanRefusalPairs(fileName: string, text: string): Scan { + const sf = ts.createSourceFile(fileName, text, ts.ScriptTarget.Latest, true); + + // The local names the refusal printers are bound to, alias-aware: a call to a + // function that merely happens to be called `printError` is not one. + const printers = new Set(); + for (const stmt of sf.statements) { + if (!ts.isImportDeclaration(stmt) || !ts.isStringLiteral(stmt.moduleSpecifier)) continue; + if (!FORMAT_MODULE.test(stmt.moduleSpecifier.text)) continue; + const bindings = stmt.importClause?.namedBindings; + if (!bindings || !ts.isNamedImports(bindings)) continue; + for (const el of bindings.elements) { + if (REFUSAL_PRINTERS.has((el.propertyName ?? el.name).text)) printers.add(el.name.text); + } + } + + const groups = new Map(); + let errorCalls = 0; + let printerCalls = 0; + + const visit = (node: ts.Node, owner: string): void => { + let next = owner; + if (owner === '') { + if ((ts.isMethodDeclaration(node) || ts.isFunctionDeclaration(node) || ts.isPropertyDeclaration(node)) && node.name) { + next = node.name.getText(sf); + } else if ( + ts.isVariableDeclaration(node) + && node.initializer + && (ts.isArrowFunction(node.initializer) || ts.isFunctionExpression(node.initializer)) + ) { + next = node.name.getText(sf); + } + } + if (ts.isCallExpression(node)) { + const callee = node.expression; + const line = sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1; + const group = groups.get(next) ?? groups.set(next, { printerLines: [], errorLines: [] }).get(next)!; + if (ts.isIdentifier(callee) && printers.has(callee.text)) { + group.printerLines.push(line); + printerCalls++; + } else if ( + ts.isPropertyAccessExpression(callee) + && callee.expression.kind === ts.SyntaxKind.ThisKeyword + && callee.name.text === 'error' + ) { + group.errorLines.push(line); + errorCalls++; + } + } + ts.forEachChild(node, (child) => visit(child, next)); + }; + visit(sf, ''); + + const pairs: RefusalPair[] = []; + for (const [fn, g] of groups) { + if (g.printerLines.length > 0 && g.errorLines.length > 0) pairs.push({ fn, ...g }); + } + return { pairs, errorCalls, printerCalls }; +} + +// --------------------------------------------------------------------------- +// 1. The scan, against fixtures — it must be able to fail +// --------------------------------------------------------------------------- + +const IMPORT = "import { printError } from '../utils/format.js';"; + +const FIXTURES: Array<{ name: string; src: string; pairs: number }> = [ + { + name: 'a refusal printed and then raised with `this.error` (the defect)', + src: `${IMPORT} class C { async run() { printError('no'); this.error('no'); } }`, + pairs: 1, + }, + { + name: 'the stderr printer is a refusal printer too', + src: "import { printErrorToStderr } from '../utils/format.js'; class C { async run() { printErrorToStderr('no'); this.error('no'); } }", + pairs: 1, + }, + { + name: 'an aliased import is followed', + src: "import { printError as fail } from '../utils/format.js'; class C { async run() { fail('no'); this.error('no'); } }", + pairs: 1, + }, + { + name: 'a call inside a callback counts toward the method that holds it', + src: `${IMPORT} class C { async run() { [1].forEach(() => { printError('no'); }); this.error('no'); } }`, + pairs: 1, + }, + { + name: 'the shape: print the refusal, then `this.exit(n)`', + src: `${IMPORT} class C { async run() { printError('no'); this.exit(2); } }`, + pairs: 0, + }, + { + name: 'the other legal shape: `this.error` and nothing printed through the refusal printers', + src: `class C { async run() { this.log('checked'); this.error('no'); } }`, + pairs: 0, + }, + { + name: 'a printer and a `this.error` in different methods', + src: `${IMPORT} class C { async a() { printError('no'); } async b() { this.error('no'); } }`, + pairs: 0, + }, + { + name: 'a local function that is merely named `printError` is not the refusal printer', + src: `function printError(m) {} class C { async run() { printError('no'); this.error('no'); } }`, + pairs: 0, + }, + { + name: 'an `error` method on something other than `this`', + src: `${IMPORT} class C { async run() { printError('no'); logger.error('no'); } }`, + pairs: 0, + }, +]; + +describe('the scan decides the fixtures it was written against', () => { + it.each(FIXTURES)('$name', ({ src, pairs }) => { + expect(scanRefusalPairs('src/commands/fixture.ts', src).pairs).toHaveLength(pairs); + }); +}); + +// --------------------------------------------------------------------------- +// 2. The population — every command module, structurally +// --------------------------------------------------------------------------- + +/** Every non-test `.ts` module under `src/commands`, the files oclif's command table is built from. */ +function commandFiles(dir: string): string[] { + return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const full = join(dir, entry.name); + if (entry.isDirectory()) return commandFiles(full); + return /\.ts$/.test(entry.name) && !/\.(?:test|d)\.ts$/.test(entry.name) ? [full] : []; + }); +} + +const FILES = commandFiles(COMMANDS_DIR); +const SCANS = FILES.map((file) => ({ file: relative(PKG_ROOT, file), scan: scanRefusalPairs(file, readFileSync(file, 'utf8')) })); + +/** + * Floors, measured on `fd5a1cd597`: 65 command modules, 57 of them calling a + * refusal printer, and `this.error` raised from 3 (`os datasource introspect`, + * `list-tables`, `validate`) once `init.ts` and `compile.ts` stopped. A scan + * that silently finds nothing reports zero pairs, and zero passes every + * assertion below; these are what notice. A drop below them is a broken + * detector or a deliberate removal — say which when you lower one. + */ +const FILE_FLOOR = 60; +const PRINTER_FILE_FLOOR = 50; +const ERROR_FILE_FLOOR = 3; + +describe('no command renders a refusal and also raises it with this.error', () => { + it('the population is not vacuous — the floors hold', () => { + expect(FILES.length, 'fewer command modules than this pin was written over').toBeGreaterThanOrEqual(FILE_FLOOR); + expect(SCANS.filter((s) => s.scan.printerCalls > 0).length, 'refusal printers not found').toBeGreaterThanOrEqual(PRINTER_FILE_FLOOR); + expect(SCANS.filter((s) => s.scan.errorCalls > 0).length, '`this.error` not found').toBeGreaterThanOrEqual(ERROR_FILE_FLOOR); + }); + + it('every command module is clear of the pair', () => { + const pairs = SCANS.flatMap(({ file, scan }) => + scan.pairs.map( + (p) => `${file} ${p.fn}(): refusal printed at line ${p.printerLines.join(', ')}, then \`this.error\` at line ${p.errorLines.join(', ')} — oclif renders the sentence a second time on stderr; end in \`this.exit(n)\` instead`, + ), + ); + expect(pairs).toEqual([]); + }); +}); From d83eb007b0e95dbdbb2519742a4ee8523e7ea9fc Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 07:03:01 +0000 Subject: [PATCH 3/3] test(cli): name both halves of the refusal-renders-once pin from the exit-signal pin's comment Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz --- packages/cli/test/exit-signal.pin.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/test/exit-signal.pin.test.ts b/packages/cli/test/exit-signal.pin.test.ts index b3332ffc9ea..752ef174360 100644 --- a/packages/cli/test/exit-signal.pin.test.ts +++ b/packages/cli/test/exit-signal.pin.test.ts @@ -114,7 +114,7 @@ * and the exit status, unchanged by the repair — 1 for the `this.exit(1)` * refusals, 2 for `os init`'s `this.exit(2)` ones (the status its `this.error` * refusals raised, until they rendered their sentence once: - * `refusal-renders-once.test.ts`). + * `refusal-renders-once.test.ts` and its `.e2e` twin). * * ## Tier *