Skip to content

Commit e2d1f80

Browse files
os-litantclaude
andauthored
test(cli): one built-CLI prerequisite for all four bin/run.js spawners, each refusing in its own terms (#12617)
* wip: hoist requireBuiltCli into the serve-process helper * test(cli): extend the built-CLI prerequisite to the fourth bin/run.js spawner `serve-node-env-production-default.e2e.test.ts` is the one spawner of the built entrypoint that had no `requireBuiltCli()` preflight. It gets one -- with its OWN mechanism string rather than the siblings' `RUN_JS_RESOLVES_FROM_DIST`. Only this file's unset-NODE_ENV leg resolves from `dist/`; its other two legs hand the child `development`/`test`, which flips oclif's `isProd()` and reroutes them to `src/commands`. The siblings' sentence ("every boot below times out") is therefore false here, and a true refusal carrying a false explanation is the defect class #12498 / #12561 / #12563 were filed for. The guard is file-scoped deliberately: measured on a closure-only tree, this file reported `3 tests | 1 failed` -- the unset pin failing while the two rerouted legs PASSED from `src/`, reporting green for a program the file's own header says it does not measure. 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 1ca763b commit e2d1f80

6 files changed

Lines changed: 334 additions & 118 deletions

‎packages/cli/test/helpers/serve-process.ts‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
*/
1212

1313
import { execFileSync, spawn } from 'node:child_process';
14+
import { existsSync, readFileSync } from 'node:fs';
1415
import { createServer, type Server } from 'node:net';
1516
import { resolve } from 'node:path';
1617
import { fileURLToPath } from 'node:url';
@@ -21,6 +22,104 @@ const HERE = resolve(fileURLToPath(import.meta.url), '..');
2122
export const CLI = resolve(HERE, '../../bin/run-dev.js');
2223
export const TSX = resolve(HERE, '../../../../node_modules/.bin/tsx');
2324

25+
// ─────────────────────────────────────────────────────────────────────────
26+
// THE OTHER ENTRYPOINT: `bin/run.js`, and the build state it silently needs
27+
//
28+
// `CLI` above is `bin/run-dev.js`, which pins `NODE_ENV=development` and runs
29+
// the command from `src/` through tsx — so a file using `runServe()` needs no
30+
// `packages/cli/dist` at all. A handful of e2e files deliberately spawn the
31+
// OTHER entrypoint instead, because the thing they measure only exists when
32+
// oclif resolves the command from the BUILT artifact. Those files, and only
33+
// those, carry a build-state prerequisite, and it used to be invisible: an
34+
// unbuilt worktree answered ` › Error: command serve not found`, the harness
35+
// reported `serve exited 2 before "Server is ready"`, and nothing in either
36+
// sentence said "run the build" (#12539).
37+
//
38+
// ⭐ The guard is here rather than in those files because it was written THREE
39+
// times, byte-identical, 19 lines each (#11707 / PR #12459 swept three
40+
// spawners in one edit and each got its own copy). Three copies of a refusal is
41+
// the same defect the refusal exists to prevent, one level up.
42+
//
43+
// ⛔ It is NOT a general "is the CLI ready" preflight. `runServe()` must never
44+
// call it: a tsx child reads `src/`, so `packages/cli/dist` is not that child's
45+
// prerequisite and a guard that refused there would be a false red on a tree
46+
// that can run the test perfectly well.
47+
// ─────────────────────────────────────────────────────────────────────────
48+
49+
/**
50+
* Why a `bin/run.js` child needs `packages/cli/dist`, in the child's own terms.
51+
*
52+
* ⭐ Named and exported rather than defaulted inside `requireBuiltCli()`, which
53+
* is the whole point: this sentence is true of the `bin/run.js` + unset-
54+
* `NODE_ENV` spawn and of nothing else. A caller reaching a different way (a
55+
* `bin/run-dev.js` + tsx child, a `pnpm` bin shim, a packed tarball) has a
56+
* DIFFERENT reason, and pasting this one there would attach a false
57+
* explanation to a true refusal — the failure class #12498 and #12561 were
58+
* filed for. The identifier says `RUN_JS` so that misuse has to be deliberate.
59+
*/
60+
export const RUN_JS_RESOLVES_FROM_DIST =
61+
'This file spawns bin/run.js with NODE_ENV unset, which is what makes oclif resolve the ' +
62+
'command from dist/ instead of transpiling src/ — so on an unbuilt tree the child answers ' +
63+
'"command serve not found" and every boot below times out.';
64+
65+
/**
66+
* The refusal itself, separated from the check so its WORDING can be pinned.
67+
*
68+
* `requireBuiltCli()` can only produce this Error on an unbuilt tree, and no
69+
* test can produce an unbuilt tree without breaking every neighbouring file in
70+
* the same run. So the sentence a reader actually acts on would otherwise be
71+
* the one part of this guard nothing checks — and a refusal that forgets to
72+
* name the build command is exactly the false red #12539 exists to end.
73+
* `serve-built-cli-prerequisite.test.ts` pins it through this function.
74+
*
75+
* @param commandFile the `dist/` command file that was looked for and missing
76+
* @param mechanism why THIS caller's child needs it — see
77+
* `RUN_JS_RESOLVES_FROM_DIST` for the only one in the tree
78+
*/
79+
export function unbuiltCliError(commandFile: string, mechanism: string): Error {
80+
return new Error(
81+
`packages/cli is not built: ${commandFile} does not exist.\n` +
82+
`${mechanism}\n` +
83+
'CI declares the build (turbo: @objectstack/cli#test dependsOn build); a direct vitest run does not.\n' +
84+
'Run: pnpm exec turbo run build --filter=@objectstack/cli',
85+
);
86+
}
87+
88+
/**
89+
* Refuse to run against an unbuilt `packages/cli`, in a sentence rather than as
90+
* oclif's "command serve not found".
91+
*
92+
* The command target is read from the CLI's own `oclif.commands.target` rather
93+
* than restated here: that declaration is where `dist/commands` is decided, and
94+
* a copy keeps probing the old path after someone moves it — the argument
95+
* `scripts/cli-build-prerequisite.mjs` makes for the gates that shell out to
96+
* this CLI. Only that one declared shape is read; anything else (unreadable,
97+
* or `oclif.commands` written as a bare string) DEFERS rather than failing, so
98+
* a checkout this cannot understand never turns red here and the spawn's own
99+
* output stays the fallback — the same fail-open direction those gates take.
100+
*
101+
* `serve.js` is the probe because it is the command every caller of this guard
102+
* spawns, and one `tsup` run emits the whole `dist/commands` directory — so its
103+
* absence answers "this package was never built" for any of them. ⛔ It does
104+
* not catch a `dist/` that is merely BEHIND its source; that residual is the
105+
* honest cost of consuming the artifact and is stated in each caller's header.
106+
*
107+
* @param mechanism why this caller's child resolves the command from `dist/`.
108+
* Required, with no default: see `RUN_JS_RESOLVES_FROM_DIST`.
109+
*/
110+
export function requireBuiltCli(mechanism: string): void {
111+
let target: unknown;
112+
try {
113+
target = JSON.parse(readFileSync(resolve(HERE, '../../package.json'), 'utf8'))?.oclif?.commands?.target;
114+
} catch {
115+
return;
116+
}
117+
if (typeof target !== 'string' || !target) return;
118+
const commandFile = resolve(HERE, '../..', target.replace(/^\.\//, ''), 'serve.js');
119+
if (existsSync(commandFile)) return;
120+
throw unbuiltCliError(commandFile, mechanism);
121+
}
122+
24123
/**
25124
* The bind probe, run in a throwaway Node process: bind `0.0.0.0:<want>`, print
26125
* the port the kernel actually assigned, close. `want = 0` asks the kernel to
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The `bin/run.js` build-state prerequisite says what to run, and says it in
5+
* the CALLER's terms (#12539).
6+
*
7+
* ── What this file is for ────────────────────────────────────────────────
8+
*
9+
* A handful of e2e files in this directory spawn `bin/run.js` rather than
10+
* `bin/run-dev.js`, because what they measure only exists when oclif resolves
11+
* the command from the BUILT artifact. On a worktree where only the dependency
12+
* closure was built — `pnpm --filter '@objectstack/cli^...' build`, the
13+
* documented first command — the child answers
14+
* ` › Error: command serve not found` and the harness reports
15+
* `serve exited 2 before "Server is ready"`. Neither sentence says "run the
16+
* build", so a build-state prerequisite arrives dressed as a regression on a
17+
* file with no visible connection to a build step. That is the card.
18+
*
19+
* `requireBuiltCli()` (`helpers/serve-process.ts`) is the answer, and the ONLY
20+
* part of it a reader ever acts on is the sentence it throws. That sentence can
21+
* only be produced on an unbuilt tree, and no test can produce an unbuilt tree
22+
* without breaking every neighbouring file in the same run — so without this
23+
* file the wording would be the one part of the guard nothing checks. A refusal
24+
* that forgets to name the build command is the false red restated, not fixed.
25+
*
26+
* ── Why the mechanism is a PARAMETER, and why that is pinned here ─────────
27+
*
28+
* Until #12539 the guard lived as three byte-identical private copies, each
29+
* carrying `"This file spawns bin/run.js with NODE_ENV unset…"` inline. That
30+
* sentence is true of the `bin/run.js` + unset-`NODE_ENV` spawn and of nothing
31+
* else: a `bin/run-dev.js` + tsx child reads `src/`, and a hoisted copy
32+
* carrying the text outward would be a FALSE EXPLANATION attached to a true
33+
* refusal — the class #12498 and #12561 were filed for. So the reason is the
34+
* caller's to supply, and `it('carries the caller's mechanism…')` below is what
35+
* keeps it that way: it passes a foreign mechanism and demands the `bin/run.js`
36+
* sentence be absent.
37+
*
38+
* ⚠️ `helpers/serve-process.ts` is a TEST helper, so `check:cross-package-test-
39+
* inputs` and the source-alias gate both already see it; nothing here reads
40+
* outside `packages/cli`.
41+
*/
42+
43+
import { describe, it, expect } from 'vitest';
44+
import { existsSync, readFileSync } from 'node:fs';
45+
import { resolve } from 'node:path';
46+
import { fileURLToPath } from 'node:url';
47+
import {
48+
RUN_JS_RESOLVES_FROM_DIST,
49+
requireBuiltCli,
50+
unbuiltCliError,
51+
} from './helpers/serve-process.js';
52+
53+
const HERE = resolve(fileURLToPath(import.meta.url), '..');
54+
/** `packages/cli` — this package's own root, never another package's. */
55+
const PACKAGE_ROOT = resolve(HERE, '..');
56+
57+
/** A mechanism belonging to some OTHER entrypoint — the misuse the guard must not commit. */
58+
const FOREIGN_MECHANISM = 'This file execs the packed tarball, which has no src/ to fall back to.';
59+
60+
describe('#12539: the unbuilt-CLI refusal is legible', () => {
61+
it('names the build command, so the reader knows what to run', () => {
62+
const message = unbuiltCliError('/repo/packages/cli/dist/commands/serve.js', RUN_JS_RESOLVES_FROM_DIST).message;
63+
// The whole point of the card. A refusal that stops at "not built" costs
64+
// the reader the same round the false red did.
65+
expect(message).toContain('Run: pnpm exec turbo run build --filter=@objectstack/cli');
66+
});
67+
68+
it('names the artifact it looked for, not just the package', () => {
69+
const message = unbuiltCliError('/repo/packages/cli/dist/commands/serve.js', RUN_JS_RESOLVES_FROM_DIST).message;
70+
expect(message).toContain('/repo/packages/cli/dist/commands/serve.js');
71+
expect(message).toContain('packages/cli is not built');
72+
});
73+
74+
it('says why CI never sees this, so a green CI is not read as a contradiction', () => {
75+
const message = unbuiltCliError('/x/serve.js', RUN_JS_RESOLVES_FROM_DIST).message;
76+
expect(message).toContain('@objectstack/cli#test dependsOn build');
77+
expect(message).toContain('a direct vitest run does not');
78+
});
79+
80+
// ── The instrument can say no ──────────────────────────────────────────
81+
it("carries the CALLER's mechanism, and no other entrypoint's", () => {
82+
const message = unbuiltCliError('/x/serve.js', FOREIGN_MECHANISM).message;
83+
expect(message).toContain(FOREIGN_MECHANISM);
84+
// ⛔ The load-bearing half. If the `bin/run.js` sentence is ever inlined
85+
// back into the helper "so callers do not have to pass one", this goes red
86+
// — which is the only thing standing between a shared refusal and a shared
87+
// WRONG explanation.
88+
expect(message).not.toContain('bin/run.js');
89+
expect(message).not.toContain('transpiling src/');
90+
});
91+
92+
it('reproduces, byte for byte, what the three private copies threw before the hoist', () => {
93+
// #12539 moved this refusal out of three files; it did not reword it. The
94+
// literal below is the message measured on `09b4f4e4e`, so a rewording has
95+
// to be a deliberate edit here rather than a side effect of the move.
96+
expect(unbuiltCliError('/repo/packages/cli/dist/commands/serve.js', RUN_JS_RESOLVES_FROM_DIST).message).toBe(
97+
'packages/cli is not built: /repo/packages/cli/dist/commands/serve.js does not exist.\n' +
98+
'This file spawns bin/run.js with NODE_ENV unset, which is what makes oclif resolve the ' +
99+
'command from dist/ instead of transpiling src/ — so on an unbuilt tree the child answers ' +
100+
'"command serve not found" and every boot below times out.\n' +
101+
'CI declares the build (turbo: @objectstack/cli#test dependsOn build); a direct vitest run does not.\n' +
102+
'Run: pnpm exec turbo run build --filter=@objectstack/cli',
103+
);
104+
});
105+
});
106+
107+
describe('#12539: the guard probes the DECLARED command target, and is silent when it is there', () => {
108+
/**
109+
* The path the guard must be looking at, derived the same way it derives it —
110+
* from `oclif.commands.target`, which is where `dist/commands` is decided.
111+
* Restating `dist/commands` here would pin the guard to a path the CLI is
112+
* free to move, which is the copy this whole card is about.
113+
*/
114+
const declared = (): string => {
115+
const target = JSON.parse(readFileSync(resolve(PACKAGE_ROOT, 'package.json'), 'utf8'))?.oclif?.commands?.target;
116+
expect(typeof target, 'oclif.commands.target is what the guard reads; a bare string breaks it').toBe('string');
117+
return resolve(PACKAGE_ROOT, String(target).replace(/^\.\//, ''), 'serve.js');
118+
};
119+
120+
it('reads the target off the CLI declaration rather than restating it', () => {
121+
expect(declared().endsWith('serve.js')).toBe(true);
122+
expect(declared().startsWith(PACKAGE_ROOT)).toBe(true);
123+
});
124+
125+
/**
126+
* ⭐ BOTH directions, decided by the tree this run is actually on — which is
127+
* why it is one `it()` and not two.
128+
*
129+
* On CI, and on any tree where `@objectstack/cli` is built, this asserts the
130+
* expensive half: the guard is SILENT. A guard that fires on a correctly
131+
* built tree is strictly worse than the false red it replaces, because then
132+
* every green in this directory is a coin flip. On an unbuilt tree it asserts
133+
* the other half — that it fires, and names the artifact it looked for.
134+
*/
135+
it('is silent when the declared command file exists, and refuses naming it when it does not', () => {
136+
const commandFile = declared();
137+
if (existsSync(commandFile)) {
138+
expect(() => requireBuiltCli(RUN_JS_RESOLVES_FROM_DIST)).not.toThrow();
139+
} else {
140+
expect(() => requireBuiltCli(RUN_JS_RESOLVES_FROM_DIST)).toThrow(commandFile);
141+
}
142+
});
143+
});

‎packages/cli/test/serve-mcp-capability-collision.e2e.test.ts‎

Lines changed: 15 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,17 @@
4444

4545
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
4646
import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
47-
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
47+
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
4848
import { tmpdir } from 'node:os';
4949
import { join, resolve } from 'node:path';
5050
import { fileURLToPath } from 'node:url';
51-
import { E2E_SECRET_KEY, childEnv, randomPort } from './helpers/serve-process.js';
51+
import {
52+
E2E_SECRET_KEY,
53+
RUN_JS_RESOLVES_FROM_DIST,
54+
childEnv,
55+
randomPort,
56+
requireBuiltCli,
57+
} from './helpers/serve-process.js';
5258

5359
const HERE = resolve(fileURLToPath(import.meta.url), '..');
5460
/**
@@ -75,48 +81,18 @@ const HERE = resolve(fileURLToPath(import.meta.url), '..');
7581
* STATE as well as about the source in the checkout, which is the trade
7682
* `scripts/check-test-source-alias.mjs` argues against for in-process imports.
7783
* `turbo.json` declares `@objectstack/cli#test` `dependsOn: ["build"]` (#11268)
78-
* so CI always builds `dist/` first; `requireBuiltCli()` below is what a
79-
* developer running `vitest` directly gets instead of oclif's "command serve
80-
* not found". Neither catches a `dist/` that is merely BEHIND its source —
84+
* so CI always builds `dist/` first; `requireBuiltCli()` — hoisted into
85+
* `helpers/serve-process.ts` by #12539, with the reason it refuses supplied
86+
* from HERE (`RUN_JS_RESOLVES_FROM_DIST`) because it is true of this
87+
* entrypoint and not of the tsx one — is what a developer running `vitest`
88+
* directly gets instead of oclif's "command serve not found". Neither catches
89+
* a `dist/` that is merely BEHIND its source —
8190
* that residual is the honest cost of consuming the artifact, and
8291
* `serve-node-env-production-default.e2e.test.ts` (which has consumed `dist/`
8392
* since #11113) carries exactly the same one.
8493
*/
8594
const CLI = resolve(HERE, '../bin/run.js');
8695

87-
/**
88-
* Refuse to run against an unbuilt `packages/cli`, in a sentence rather than as
89-
* oclif's "command serve not found".
90-
*
91-
* The command target is read from the CLI's own `oclif.commands.target` rather
92-
* than restated here: that declaration is where `dist/commands` is decided, and
93-
* a copy keeps probing the old path after someone moves it — the argument
94-
* `scripts/cli-build-prerequisite.mjs` makes for the gates that shell out to
95-
* this CLI. Only that one declared shape is read; anything else (unreadable,
96-
* or `oclif.commands` written as a bare string) DEFERS rather than failing, so
97-
* a checkout this cannot understand never turns red here and the spawn's own
98-
* output stays the fallback — the same fail-open direction those gates take.
99-
*/
100-
function requireBuiltCli(): void {
101-
let target: unknown;
102-
try {
103-
target = JSON.parse(readFileSync(resolve(HERE, '../package.json'), 'utf8'))?.oclif?.commands?.target;
104-
} catch {
105-
return;
106-
}
107-
if (typeof target !== 'string' || !target) return;
108-
const commandFile = resolve(HERE, '..', target.replace(/^\.\//, ''), 'serve.js');
109-
if (existsSync(commandFile)) return;
110-
throw new Error(
111-
`packages/cli is not built: ${commandFile} does not exist.\n` +
112-
'This file spawns bin/run.js with NODE_ENV unset, which is what makes oclif resolve the ' +
113-
'command from dist/ instead of transpiling src/ — so on an unbuilt tree the child answers ' +
114-
'"command serve not found" and every boot below times out.\n' +
115-
'CI declares the build (turbo: @objectstack/cli#test dependsOn build); a direct vitest run does not.\n' +
116-
'Run: pnpm exec turbo run build --filter=@objectstack/cli',
117-
);
118-
}
119-
12096
/** The consumer's real identity — see `serve-capability-identity.test.ts`. */
12197
const CONSUMER_PLUGIN_ID = 'com.objectstack.connector.mcp';
12298
const CONSUMER_CLASS_NAME = 'ConnectorMcpPlugin';
@@ -393,7 +369,7 @@ async function readFrame(res: Response, id: number): Promise<Record<string, unkn
393369
describe('#7652: an app loading the MCP client connector still gets the MCP server', () => {
394370
beforeAll(async () => {
395371
// Build prerequisite first: the spawns below resolve `serve` from `dist/`.
396-
requireBuiltCli();
372+
requireBuiltCli(RUN_JS_RESOLVES_FROM_DIST);
397373

398374
dir = mkdtempSync(join(tmpdir(), 'mcp-collision-e2e-'));
399375
writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8');

0 commit comments

Comments
 (0)