Skip to content

Commit e6fd1ca

Browse files
os-litantclaude
andauthored
fix(cli): name the missing build output instead of oclif's "command not found" (#13064)
* fix(cli): name the missing build output instead of "command not found" In a checkout where a workspace dependency has no dist/, @oclif/core import()s every command module while building its manifest, all of them fail, and the run ends on `Error: command <id> not found` with exit 2 -- while the command file is right there in src/commands/. A command whose module will not load is indistinguishable, to Config.runCommand, from one that does not exist. bin/run-dev.js now collects oclif's module-load warnings around run() and, when the failure was caused by a package this repo builds, prints the attribution and the one command that fixes it ahead of oclif's report. Both the classification and the remedy come from scripts/cli-build-prerequisite.mjs, which already answers this for the gates that shell out to the CLI. The collector is attached AFTER run() deliberately: @oclif/core installs its own warning listener only while process.listenerCount('warning') <= 1, so attaching first would silently drop its debug blocks from every failing run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd * refactor(cli): move the lead renderer under scripts/ with a declaration mirror Two facts forced the move, both measured rather than guessed: - a .ts test importing the untyped .mjs is TS7016 ("Could not find a declaration file"), and packages/cli's hidden test layer is ledgered in check-type-check-coverage.mjs with "the first new error in it should go red rather than be absorbed". The fix is the hand-written .d.mts this repo already uses three times over -- and check-declaration-mirrors only discovers scripts/**/*.d.mts, so a declaration under packages/cli/bin would have been exactly the unwatched drift that gate exists to prevent. - that gate import()s the module with bare node, so the module may not import a .ts. The CLI's name therefore arrives as a parameter; the shim already imports invocation.ts on its failure path and owns that coupling. Measured after the move: the two new test files add 0 errors to the layer, with a two-sided control (the same program reports TS7016 for the undeclared sibling import on the line above and nothing for this one). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd * chore(cli): declare the new cross-package test inputs check:cross-package-test-inputs named the import the moment the renderer moved under scripts/: a test whose real inputs are wider than its package is invisible to both the affected-subset filter and the turbo cache. Declared in the roster and mirrored into turbo.json, per package convention: - cli-unbuilt-workspace-lead.mjs -- the import itself; - its .d.mts -- a real input to the typecheck verdict, the same reason the js-comment-mask sibling is declared; - cli-build-prerequisite.mjs -- where both halves of the answer come from, including the remedy string the test pins character for character; - cross-package-test-inputs.mjs -- the mention shape, since the test now says where its inputs are declared. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 95c3703 commit e6fd1ca

9 files changed

Lines changed: 643 additions & 1 deletion
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(cli): name the missing build output instead of reporting "command not found" (#12964)
6+
7+
In a checkout where a workspace dependency has no `dist/`, `@oclif/core` `import()`s
8+
every command module while it builds its manifest, every one of them fails, and the run
9+
ends on
10+
11+
```
12+
Error: command i18n:extract:… not found
13+
```
14+
15+
with exit 2 — while the command file is right there in `src/commands/`. A command whose
16+
module will not load is indistinguishable, to `Config.runCommand`, from one that does not
17+
exist, so the only cause the reader is handed is the one cause that is definitely not
18+
true.
19+
20+
`packages/cli/bin/run-dev.js` — this repo's SOURCE entry point, run through `tsx` by its
21+
own gates and e2e suites, and not part of the published package — now collects oclif's
22+
module-load warnings and, when that failure was caused by a package this repo builds,
23+
prints the attribution and the single command that fixes it ahead of oclif's report:
24+
25+
```
26+
objectstack: NOT A MISSING COMMAND — @oclif/core reports a command module that failed to
27+
LOAD as "not found", and one did: Cannot find module '…/@objectstack/spec/dist/index.mjs'.
28+
The unmet precondition is @objectstack/spec's build output, not the invocation.
29+
objectstack: Fix: pnpm exec turbo run build --filter=@objectstack/spec
30+
```
31+
32+
Both the classification and the remedy come from `scripts/cli-build-prerequisite.mjs`, the
33+
module that already answers this question for the gates that shell out to the CLI, so
34+
there is no second verdict to keep in sync. Nothing is added to a run that succeeds, and a
35+
command that really is missing keeps oclif's reporting exactly as it was — the diagnosis
36+
requires BOTH oclif's "not found" and a module-load failure naming a workspace package.

packages/cli/bin/run-dev.js

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,77 @@ async function announceInvocationFailure(error) {
2525
}
2626
}
2727

28+
/**
29+
* Every module-load failure oclif reported while building its command table
30+
* (#12964), in emission order. Filled by the listener attached below.
31+
*
32+
* It HAS to be collected as it happens. `findCommand` `import()`s every command
33+
* module while `Config.load()` runs, warns on each one that will not load, and
34+
* then throws a plain "command … not found" that carries none of it — so by the
35+
* time the `.catch()` below holds the error, the only cause worth naming has
36+
* already gone past. `warning.detail` is where oclif puts the failing specifier.
37+
*/
38+
const moduleLoadFailures = [];
39+
40+
/**
41+
* The other reading of "command … not found": the command is there and its
42+
* MODULE would not load, because a workspace package this repo builds has no
43+
* usable `dist/`. See `scripts/cli-unbuilt-workspace-lead.mjs` for the whole
44+
* argument, including why the CLI's name is passed IN rather than imported
45+
* there.
46+
*
47+
* Lazy and `catch`-wrapped for the same reason as `announceInvocationFailure`:
48+
* a reporter that throws must never become the report.
49+
*/
50+
async function announceUnbuiltWorkspace(error) {
51+
try {
52+
const [{ unbuiltWorkspaceLines }, { INVOCATION_PREFIX }] = await Promise.all([
53+
import('../../../scripts/cli-unbuilt-workspace-lead.mjs'),
54+
import('../src/utils/invocation.ts'),
55+
]);
56+
for (const line of unbuiltWorkspaceLines(error, moduleLoadFailures, INVOCATION_PREFIX) ?? []) {
57+
process.stderr.write(`${line}\n`);
58+
}
59+
} catch {
60+
// Stay quiet rather than replacing oclif's report with an error about the
61+
// reporter itself.
62+
}
63+
}
64+
2865
process.env.NODE_ENV = 'development';
2966
settings.debug = true;
3067

31-
await run(process.argv.slice(2), import.meta.url)
68+
const running = run(process.argv.slice(2), import.meta.url);
69+
70+
// ⚠️ ATTACHED AFTER `run()`, and that order is load-bearing rather than style.
71+
// @oclif/core installs a `warning` listener of its own — `displayWarnings()` in
72+
// `config/config.js`, which is what prints the `Warning: ModuleLoadError` stack
73+
// plus `detail` under `settings.debug` — but it installs it ONLY when
74+
// `process.listenerCount('warning') <= 1`, i.e. only node's own default is
75+
// attached. A collector attached before `run()` makes that count 2, oclif
76+
// silently declines to install, and every failing run through this shim quietly
77+
// loses those blocks (measured on the #12964 repro: 1518 lines of report became
78+
// 476, with nothing saying why).
79+
//
80+
// `run()` reaches `Config.load()` — and `displayWarnings()` inside it — in its
81+
// SYNCHRONOUS prefix (`main.js`: `await Config.load(...)` is its first `await`;
82+
// `config.js`: `displayWarnings()` precedes `load()`'s first `await`), and
83+
// `process.emitWarning` defers to `nextTick`, so a listener attached here is
84+
// installed second and still sees every warning. `run-dev-unbuilt-workspace.e2e`
85+
// asserts oclif's blocks are still there, so a future oclif that moves that call
86+
// past an `await` fails a test instead of going quiet.
87+
process.on('warning', (warning) => {
88+
const detail = warning?.detail;
89+
if (typeof detail === 'string' && detail) moduleLoadFailures.push(detail);
90+
});
91+
92+
await running
3293
.then(async (result) => {
3394
flush();
3495
return result;
3596
})
3697
.catch(async (error) => {
3798
await announceInvocationFailure(error);
99+
await announceUnbuiltWorkspace(error);
38100
return handle(error);
39101
});
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* A BUILT checkout, made to answer like an unbuilt one for `@objectstack/spec`
5+
* and nothing else — the environment `run-dev-unbuilt-workspace.e2e.test.ts`
6+
* needs and CI cannot otherwise have (#12964).
7+
*
8+
* Loaded with `node --import`, so it is in place before `@oclif/core` walks the
9+
* command directory. It is a `resolve` hook and NOT a file operation on purpose:
10+
* this repo is worked by several agents in one container at a time, and a test
11+
* that renamed `packages/spec/dist` for a few seconds would break every other
12+
* run in the box. Nothing here touches the disk.
13+
*
14+
* ## Why it re-points the specifier instead of throwing
15+
*
16+
* The classifier this feeds (`looksLikeStaleWorkspaceDist`) reads node's OWN
17+
* sentence, so the sentence has to be node's. Two shapes were measured before
18+
* this one was kept:
19+
*
20+
* - `{ url, shortCircuit: true }` at a non-existent URL skips
21+
* `finalizeResolution`, so the failure surfaces from the LOAD step as
22+
* `ENOENT: no such file or directory, open '…'`. That is not the corpus and
23+
* the classifier correctly declines it — a green run that proves nothing.
24+
* - throwing a hand-built `ERR_MODULE_NOT_FOUND` would make the test assert
25+
* against a string this file authored, which is the one thing a fixture for
26+
* a text classifier must not do.
27+
*
28+
* Handing `nextResolve` an ABSOLUTE PATH that does not exist runs node's real
29+
* resolution against it, and node produces its real
30+
* `Cannot find module '…' imported from …`.
31+
*
32+
* ## Why the path is spelled through `packages/cli/node_modules`
33+
*
34+
* That is where an unbuilt tree's sentence points, and it is not cosmetic: pnpm
35+
* symlinks the workspace package in, and node only reports the pre-realpath
36+
* spelling when resolution FAILS (a successful resolve reports
37+
* `packages/spec/dist/index.mjs`, with no `@objectstack` in it at all). The
38+
* classifier keys on `node_modules/@objectstack/<pkg>` — deliberately, so it
39+
* never diagnoses a third party — so a realpath spelling would classify as
40+
* nothing and this fixture would silently stop simulating anything.
41+
*/
42+
43+
import { registerHooks } from 'node:module';
44+
import { dirname, resolve } from 'node:path';
45+
import { fileURLToPath } from 'node:url';
46+
47+
/** This file lives in `packages/cli/test/fixtures`, so `packages/cli` is two up. */
48+
const CLI_PKG = resolve(dirname(fileURLToPath(import.meta.url)), '../..');
49+
50+
/**
51+
* Where an unbuilt `@objectstack/spec` is looked for. The last segment is
52+
* deliberately not a real one — `dist/` itself is present in a built checkout,
53+
* and the whole point is a path that is missing.
54+
*/
55+
const UNBUILT_TARGET = resolve(CLI_PKG, 'node_modules/@objectstack/spec/dist/__unbuilt-simulation__/index.mjs');
56+
57+
/** Every `@objectstack/spec` subpath, so the simulation is not one entry deep. */
58+
const DENIED = '@objectstack/spec';
59+
60+
registerHooks({
61+
resolve(specifier, context, nextResolve) {
62+
if (specifier === DENIED || specifier.startsWith(`${DENIED}/`)) {
63+
return nextResolve(UNBUILT_TARGET, context);
64+
}
65+
return nextResolve(specifier, context);
66+
},
67+
});
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The WIRING half of #12964 — `bin/run-dev.js` really asks, on a real failing
5+
* run, whether "command … not found" is about a missing command at all.
6+
*
7+
* ```
8+
* $ pnpm i18n:extract # fresh worktree, pnpm install done, nothing built
9+
* …
10+
* Error: command i18n:extract:packages/platform-objects/scripts/i18n-extract.config.ts not found
11+
* $ echo $?
12+
* 2
13+
* ```
14+
*
15+
* The command file is right there in `src/commands/i18n/extract.ts`. oclif
16+
* `import()`s every command module while it builds its manifest, all of them
17+
* failed on a `@objectstack/spec` that had no `dist/`, and a command whose
18+
* module will not load is indistinguishable to `Config.runCommand` from one
19+
* that does not exist.
20+
*
21+
* ## Why this suite is spawned, and why it simulates
22+
*
23+
* The lead line is produced from a `process.on('warning')` collector installed
24+
* around `run()` — state that exists only inside a real CLI process, so an
25+
* in-process test cannot see it and `process.exit`-adjacent behaviour cannot be
26+
* asserted from a vitest worker at all.
27+
*
28+
* And CI's checkout is BUILT. ⚠️ That is the trap this file is written against:
29+
* an "unbuilt tree" test that runs in a built tree never enters the branch it
30+
* claims to cover, prints nothing, asserts nothing failed, and reads green
31+
* forever. So the unbuilt condition is MANUFACTURED for one child process
32+
* (`fixtures/unbuilt-spec-dist.hook.mjs`, a `--import` resolve hook that touches
33+
* no disk), and the three cases below are a POSITIVE CONTROL PAIR rather than a
34+
* single assertion:
35+
*
36+
* 1. hook on, real command id → the lead lines appear;
37+
* 2. hook off, THE SAME command id → the command module loads and runs, so
38+
* the run never reaches that branch at all;
39+
* 3. hook off, a command that really is missing → oclif's "not found" stands
40+
* exactly as it did, with nothing added.
41+
*
42+
* (1) without (2) would pass in a tree where every run happens to be diagnosed;
43+
* (2) and (3) without (1) are two zero readings. Together they say the branch is
44+
* reachable, is not always taken, and is taken for the right reason.
45+
*/
46+
47+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
48+
import { execFile } from 'node:child_process';
49+
import { mkdtempSync, rmSync } from 'node:fs';
50+
import { tmpdir } from 'node:os';
51+
import { join, resolve } from 'node:path';
52+
import { fileURLToPath, pathToFileURL } from 'node:url';
53+
import { childEnv } from './helpers/serve-process.js';
54+
55+
const HERE = resolve(fileURLToPath(import.meta.url), '..');
56+
/** The SOURCE entry point — this suite is about it, and it needs no `dist/`. */
57+
const CLI = resolve(HERE, '../bin/run-dev.js');
58+
const TSX = resolve(HERE, '../../../node_modules/.bin/tsx');
59+
const UNBUILT_HOOK = pathToFileURL(resolve(HERE, 'fixtures/unbuilt-spec-dist.hook.mjs')).href;
60+
61+
/**
62+
* A REAL command id, so "not found" is a lie rather than the truth. Its
63+
* argument names nothing: case 2 has to fail for its own reason (no config
64+
* file) instead of doing work, and the point there is only WHICH failure.
65+
*/
66+
const REAL_COMMAND = ['i18n', 'extract', 'nope.ts'];
67+
68+
/** oclif + tsx cold start, plus ~58 failing command imports in case 1. */
69+
const RUN_TIMEOUT_MS = 180_000;
70+
71+
interface Run {
72+
code: number;
73+
stdout: string;
74+
stderr: string;
75+
}
76+
77+
/**
78+
* `NODE_OPTIONS` is stated on every call, in both directions. `childEnv()`
79+
* strips the vitest-worker family and `NODE_PATH`, but not this one, so a
80+
* control leg that said nothing would silently inherit whatever the runner was
81+
* started with — and the control legs' whole job is to be un-simulated.
82+
*/
83+
function runCli(args: string[], cwd: string, nodeOptions: string | undefined): Promise<Run> {
84+
return new Promise((resolvePromise) => {
85+
execFile(TSX, [CLI, ...args], { cwd, maxBuffer: 32 * 1024 * 1024, env: childEnv({ NO_COLOR: '1', NODE_OPTIONS: nodeOptions }) }, (err, stdout, stderr) => {
86+
resolvePromise({
87+
// `err.code` is the real exit status; `null`/undefined means the child
88+
// was signalled — a failure of a different kind, never reported as 0.
89+
code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0,
90+
stdout: String(stdout),
91+
stderr: String(stderr),
92+
});
93+
});
94+
});
95+
}
96+
97+
/** The sentence this change exists to contradict. */
98+
const LEAD = 'objectstack: NOT A MISSING COMMAND';
99+
const FIX = 'objectstack: Fix: pnpm exec turbo run build --filter=@objectstack/spec';
100+
101+
let dir: string;
102+
let unbuilt: Run;
103+
let built: Run;
104+
let genuinelyMissing: Run;
105+
106+
beforeAll(async () => {
107+
dir = mkdtempSync(join(tmpdir(), 'os-run-dev-unbuilt-'));
108+
unbuilt = await runCli(REAL_COMMAND, dir, `--import ${UNBUILT_HOOK}`);
109+
built = await runCli(REAL_COMMAND, dir, undefined);
110+
genuinelyMissing = await runCli(['definitely-not-a-command'], dir, undefined);
111+
}, RUN_TIMEOUT_MS * 3);
112+
113+
afterAll(() => {
114+
rmSync(dir, { recursive: true, force: true });
115+
});
116+
117+
describe('run-dev.js on a workspace package with no build output', () => {
118+
it('reproduces the misdiagnosis it is fixing — oclif still says "not found"', () => {
119+
// The upstream line is deliberately NOT suppressed: nothing here changes
120+
// which arguments the CLI accepts or how oclif reports, only what is said
121+
// alongside. Asserting it also proves case 1 really reached that failure
122+
// rather than dying earlier for some unrelated reason.
123+
expect(unbuilt.stderr).toContain('Error: command i18n:extract:nope.ts not found');
124+
expect(unbuilt.code).toBe(2);
125+
});
126+
127+
it('names the real cause and the one command that fixes it', () => {
128+
expect(unbuilt.stderr).toContain(LEAD);
129+
expect(unbuilt.stderr).toContain('@objectstack/spec');
130+
expect(unbuilt.stderr).toContain(FIX);
131+
});
132+
133+
it('keeps the oclif debug warning blocks — the listener order is load-bearing', () => {
134+
// @oclif/core installs its `warning` listener only when
135+
// `process.listenerCount('warning') <= 1`. A collector attached BEFORE
136+
// `run()` makes that 2, oclif silently declines, and every failing run
137+
// through this shim loses these blocks with nothing saying why (measured:
138+
// 1518 lines of report became 476). `at Plugin.warn` is that listener's
139+
// output, so this case reds if the attachment ever moves back.
140+
expect(unbuilt.stderr).toContain('at Plugin.warn');
141+
});
142+
});
143+
144+
describe('the same probe, un-simulated (positive control)', () => {
145+
it('takes the other branch entirely: the command module loads and runs', () => {
146+
// Not "no lead line" alone — that is a zero reading. The command REACHED
147+
// its own argument handling, which is only possible if its module loaded.
148+
expect(`${built.stdout}${built.stderr}`).toContain('Config file not found');
149+
expect(built.stderr).not.toContain('Error: command');
150+
expect(built.stderr).not.toContain(LEAD);
151+
expect(built.code).toBe(1);
152+
});
153+
154+
it('leaves a command that really is missing exactly as it was', () => {
155+
expect(genuinelyMissing.stderr).toContain('Error: command definitely-not-a-command not found');
156+
expect(genuinelyMissing.stderr).not.toContain(LEAD);
157+
expect(genuinelyMissing.stderr).not.toContain('objectstack: Fix:');
158+
expect(genuinelyMissing.code).toBe(2);
159+
});
160+
});

0 commit comments

Comments
 (0)