Skip to content

Commit 3e14556

Browse files
claudeclaude
committed
test(cli): reach the built CLI in the three serve e2e spawners
Adopt the honest-and-fast spawn shape measured in #11462: spawn `bin/run.js` with plain `node` and leave the child's `NODE_ENV` UNSET, which is the value that keeps @oclif/core's TypeScript path lookup disabled so the command resolves from `packages/cli/dist/commands`. The `--dev` admin seed these fixtures sign in as survives an unset `NODE_ENV`: `serve.ts` assigns `process.env.NODE_ENV = 'development'` in-process for `--dev` before `runtime.start()`, and plugin-auth's `isDevAdminSeedArmed()` reads it at call time inside the `kernel:ready` hook, after that assignment. Each file gains a `requireBuiltCli()` preflight so an unbuilt tree fails with a sentence and a build command instead of oclif's cryptic "command serve not found". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
1 parent 8515954 commit 3e14556

3 files changed

Lines changed: 246 additions & 73 deletions

File tree

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

Lines changed: 82 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -44,31 +44,78 @@
4444

4545
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
4646
import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
47-
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
47+
import { existsSync, mkdtempSync, readFileSync, 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, TSX, childEnv, randomPort } from './helpers/serve-process.js';
51+
import { E2E_SECRET_KEY, childEnv, randomPort } from './helpers/serve-process.js';
5252

5353
const HERE = resolve(fileURLToPath(import.meta.url), '..');
5454
/**
55-
* `bin/run-dev.js` through `tsx` — the SOURCE entrypoint, like the ~20 sibling
56-
* e2e files in this directory.
55+
* `bin/run.js` through plain `node` — the SHIPPED entrypoint, and this file
56+
* genuinely reaches it (#11707).
5757
*
58-
* ⛔ NOT `bin/run.js`. This file used to spell that one and call it "the SHIPPED
59-
* entrypoint"; the claim was never true here (#11317). The boot below pins
60-
* `NODE_ENV=development` on the child for the `--dev` admin seed, and
61-
* @oclif/core 4.13.3 skips its TypeScript path lookup only when `isProd()` —
62-
* `!['development', 'test'].includes(process.env.NODE_ENV ?? '')`. Under that
63-
* value oclif rewrites the command target from the declared `./dist/commands`
64-
* to `./src/commands` and transpiles, so `packages/cli/dist` is never consulted
65-
* whichever stub is named. `serve-node-env-production-default.e2e.test.ts` is
66-
* the file that genuinely reaches the built artifact, and it gets there by
67-
* leaving `NODE_ENV` UNSET — the value that disables the reroute. Restoring
68-
* `bin/run.js` here without dropping the `NODE_ENV` pin below is a no-op with a
69-
* false comment attached.
58+
* Both halves of that are load-bearing and neither works alone. @oclif/core
59+
* 4.13.3 skips its TypeScript path lookup only when `isProd()` —
60+
* `!['development', 'test'].includes(process.env.NODE_ENV ?? '')` — so under a
61+
* child `NODE_ENV` of `development` or `test` it rewrites the command target
62+
* from the declared `./dist/commands` to `./src/commands` and transpiles, and
63+
* `packages/cli/dist` goes unread whichever stub is named. This file used to
64+
* pin `NODE_ENV=development` on the child (#11317); the boot below leaves it
65+
* UNSET instead — the value that disables the reroute — and the `--dev` admin
66+
* seed survives that, for the reason spelled out at the `NODE_ENV` entry below.
67+
*
68+
* ⛔ Do not restore either half on its own. `bin/run-dev.js` assigns
69+
* `NODE_ENV = 'development'` before argv is parsed, so it reroutes
70+
* unconditionally; and re-pinning `NODE_ENV` on the child while naming
71+
* `bin/run.js` is the self-cancelling pair #11317 found here — it promises the
72+
* built artifact, delivers source, and says nothing.
73+
*
74+
* PRICE, stated because it is real. This file is now a verdict about BUILD
75+
* STATE as well as about the source in the checkout, which is the trade
76+
* `scripts/check-test-source-alias.mjs` argues against for in-process imports.
77+
* `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 —
81+
* that residual is the honest cost of consuming the artifact, and
82+
* `serve-node-env-production-default.e2e.test.ts` (which has consumed `dist/`
83+
* since #11113) carries exactly the same one.
7084
*/
71-
const CLI = resolve(HERE, '../bin/run-dev.js');
85+
const CLI = resolve(HERE, '../bin/run.js');
86+
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+
}
72119

73120
/** The consumer's real identity — see `serve-capability-identity.test.ts`. */
74121
const CONSUMER_PLUGIN_ID = 'com.objectstack.connector.mcp';
@@ -122,7 +169,7 @@ const children: ChildProcessWithoutNullStreams[] = [];
122169

123170
function boot(env: Record<string, string | undefined>, waitFor: RegExp): Promise<ChildProcessWithoutNullStreams> {
124171
return new Promise((resolveBoot, rejectBoot) => {
125-
const child = spawn(TSX, [CLI, 'serve', '-p', port, '--dev'], {
172+
const child = spawn(process.execPath, [CLI, 'serve', '-p', port, '--dev'], {
126173
cwd: dir,
127174
stdio: ['pipe', 'pipe', 'pipe'],
128175
// `childEnv`, not a bare `...process.env`: the vitest worker exports
@@ -141,12 +188,20 @@ function boot(env: Record<string, string | undefined>, waitFor: RegExp): Promise
141188
// boot from writing to the runner's home directory and from coupling
142189
// itself to whatever other test got there first.
143190
OS_SECRET_KEY: E2E_SECRET_KEY,
144-
// The dev-admin seed the key mint signs in as is gated on this, and
145-
// vitest exports `test`.
146-
// Still passed explicitly although `bin/run-dev.js` assigns it too: the
147-
// shim's assignment runs after its own static imports have evaluated,
148-
// so only the child env pins the value for the whole process lifetime.
149-
NODE_ENV: 'development',
191+
// UNSET, not `development` — and `undefined` rather than `''`, because
192+
// Node's `spawn()` omits an undefined-valued entry rather than
193+
// stringifying it. Unset is what keeps oclif's ts-path reroute OFF, so
194+
// `CLI` above resolves the command from `dist/`; the vitest worker
195+
// exports `NODE_ENV=test`, which would switch the reroute back on, so
196+
// the entry has to be here to remove it rather than merely omitted.
197+
//
198+
// The `--dev` admin seed this boot depends on still runs. `serve.ts`
199+
// assigns `process.env.NODE_ENV = 'development'` IN-PROCESS for `--dev`
200+
// when the variable is unset, before `runtime.start()`, and
201+
// plugin-auth's `isDevAdminSeedArmed()` reads it at CALL time inside
202+
// the `kernel:ready` hook — after that assignment. Both halves
203+
// re-measured on this tree, not inherited from the card.
204+
NODE_ENV: undefined,
150205
...env,
151206
}),
152207
}) as ChildProcessWithoutNullStreams;
@@ -247,6 +302,9 @@ async function readFrame(res: Response, id: number): Promise<Record<string, unkn
247302

248303
describe('#7652: an app loading the MCP client connector still gets the MCP server', () => {
249304
beforeAll(async () => {
305+
// Build prerequisite first: the spawns below resolve `serve` from `dist/`.
306+
requireBuiltCli();
307+
250308
dir = mkdtempSync(join(tmpdir(), 'mcp-collision-e2e-'));
251309
writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8');
252310
writeFileSync(

‎packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts‎

Lines changed: 82 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -43,31 +43,78 @@
4343

4444
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
4545
import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
46-
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
46+
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
4747
import { tmpdir } from 'node:os';
4848
import { join, resolve } from 'node:path';
4949
import { fileURLToPath } from 'node:url';
50-
import { E2E_SECRET_KEY, TSX, childEnv, randomPort } from './helpers/serve-process.js';
50+
import { E2E_SECRET_KEY, childEnv, randomPort } from './helpers/serve-process.js';
5151

5252
const HERE = resolve(fileURLToPath(import.meta.url), '..');
5353
/**
54-
* `bin/run-dev.js` through `tsx` — the SOURCE entrypoint, like the ~20 sibling
55-
* e2e files in this directory.
54+
* `bin/run.js` through plain `node` — the SHIPPED entrypoint, and this file
55+
* genuinely reaches it (#11707).
5656
*
57-
* ⛔ NOT `bin/run.js`. This file used to spell that one and call it "the SHIPPED
58-
* entrypoint"; the claim was never true here (#11317). The boot below pins
59-
* `NODE_ENV=development` on the child for the `--dev` admin seed, and
60-
* @oclif/core 4.13.3 skips its TypeScript path lookup only when `isProd()` —
61-
* `!['development', 'test'].includes(process.env.NODE_ENV ?? '')`. Under that
62-
* value oclif rewrites the command target from the declared `./dist/commands`
63-
* to `./src/commands` and transpiles, so `packages/cli/dist` is never consulted
64-
* whichever stub is named. `serve-node-env-production-default.e2e.test.ts` is
65-
* the file that genuinely reaches the built artifact, and it gets there by
66-
* leaving `NODE_ENV` UNSET — the value that disables the reroute. Restoring
67-
* `bin/run.js` here without dropping the `NODE_ENV` pin below is a no-op with a
68-
* false comment attached.
57+
* Both halves of that are load-bearing and neither works alone. @oclif/core
58+
* 4.13.3 skips its TypeScript path lookup only when `isProd()` —
59+
* `!['development', 'test'].includes(process.env.NODE_ENV ?? '')` — so under a
60+
* child `NODE_ENV` of `development` or `test` it rewrites the command target
61+
* from the declared `./dist/commands` to `./src/commands` and transpiles, and
62+
* `packages/cli/dist` goes unread whichever stub is named. This file used to
63+
* pin `NODE_ENV=development` on the child (#11317); the boot below leaves it
64+
* UNSET instead — the value that disables the reroute — and the `--dev` admin
65+
* seed survives that, for the reason spelled out at the `NODE_ENV` entry below.
66+
*
67+
* ⛔ Do not restore either half on its own. `bin/run-dev.js` assigns
68+
* `NODE_ENV = 'development'` before argv is parsed, so it reroutes
69+
* unconditionally; and re-pinning `NODE_ENV` on the child while naming
70+
* `bin/run.js` is the self-cancelling pair #11317 found here — it promises the
71+
* built artifact, delivers source, and says nothing.
72+
*
73+
* PRICE, stated because it is real. This file is now a verdict about BUILD
74+
* STATE as well as about the source in the checkout, which is the trade
75+
* `scripts/check-test-source-alias.mjs` argues against for in-process imports.
76+
* `turbo.json` declares `@objectstack/cli#test` `dependsOn: ["build"]` (#11268)
77+
* so CI always builds `dist/` first; `requireBuiltCli()` below is what a
78+
* developer running `vitest` directly gets instead of oclif's "command serve
79+
* not found". Neither catches a `dist/` that is merely BEHIND its source —
80+
* that residual is the honest cost of consuming the artifact, and
81+
* `serve-node-env-production-default.e2e.test.ts` (which has consumed `dist/`
82+
* since #11113) carries exactly the same one.
83+
*/
84+
const CLI = resolve(HERE, '../bin/run.js');
85+
86+
/**
87+
* Refuse to run against an unbuilt `packages/cli`, in a sentence rather than as
88+
* oclif's "command serve not found".
89+
*
90+
* The command target is read from the CLI's own `oclif.commands.target` rather
91+
* than restated here: that declaration is where `dist/commands` is decided, and
92+
* a copy keeps probing the old path after someone moves it — the argument
93+
* `scripts/cli-build-prerequisite.mjs` makes for the gates that shell out to
94+
* this CLI. Only that one declared shape is read; anything else (unreadable,
95+
* or `oclif.commands` written as a bare string) DEFERS rather than failing, so
96+
* a checkout this cannot understand never turns red here and the spawn's own
97+
* output stays the fallback — the same fail-open direction those gates take.
6998
*/
70-
const CLI = resolve(HERE, '../bin/run-dev.js');
99+
function requireBuiltCli(): void {
100+
let target: unknown;
101+
try {
102+
target = JSON.parse(readFileSync(resolve(HERE, '../package.json'), 'utf8'))?.oclif?.commands?.target;
103+
} catch {
104+
return;
105+
}
106+
if (typeof target !== 'string' || !target) return;
107+
const commandFile = resolve(HERE, '..', target.replace(/^\.\//, ''), 'serve.js');
108+
if (existsSync(commandFile)) return;
109+
throw new Error(
110+
`packages/cli is not built: ${commandFile} does not exist.\n` +
111+
'This file spawns bin/run.js with NODE_ENV unset, which is what makes oclif resolve the ' +
112+
'command from dist/ instead of transpiling src/ — so on an unbuilt tree the child answers ' +
113+
'"command serve not found" and every boot below times out.\n' +
114+
'CI declares the build (turbo: @objectstack/cli#test dependsOn build); a direct vitest run does not.\n' +
115+
'Run: pnpm exec turbo run build --filter=@objectstack/cli',
116+
);
117+
}
71118

72119
const CONFIG = `
73120
export default {
@@ -117,7 +164,7 @@ interface Booted {
117164
*/
118165
function boot(env: Record<string, string | undefined>, waitFor: RegExp): Promise<Booted> {
119166
return new Promise((resolveBoot, rejectBoot) => {
120-
const child = spawn(TSX, [CLI, 'serve', '-p', port, '--dev'], {
167+
const child = spawn(process.execPath, [CLI, 'serve', '-p', port, '--dev'], {
121168
cwd: dir,
122169
stdio: ['pipe', 'pipe', 'pipe'],
123170
// `childEnv`, not a bare `...process.env`: the vitest worker exports
@@ -136,13 +183,20 @@ function boot(env: Record<string, string | undefined>, waitFor: RegExp): Promise
136183
// boot from writing to the runner's home directory and from coupling
137184
// itself to whatever other test got there first.
138185
OS_SECRET_KEY: E2E_SECRET_KEY,
139-
// Explicit, not inherited: the dev-admin seed this fixture signs in as
140-
// is hard-gated on `NODE_ENV === 'development'`, and vitest exports
141-
// `test`, which would leave the DB user-less and the mint unauthorized.
142-
// Still passed explicitly although `bin/run-dev.js` assigns it too: the
143-
// shim's assignment runs after its own static imports have evaluated,
144-
// so only the child env pins the value for the whole process lifetime.
145-
NODE_ENV: 'development',
186+
// UNSET, not `development` — and `undefined` rather than `''`, because
187+
// Node's `spawn()` omits an undefined-valued entry rather than
188+
// stringifying it. Unset is what keeps oclif's ts-path reroute OFF, so
189+
// `CLI` above resolves the command from `dist/`; the vitest worker
190+
// exports `NODE_ENV=test`, which would switch the reroute back on, so
191+
// the entry has to be here to remove it rather than merely omitted.
192+
//
193+
// The `--dev` admin seed this boot depends on still runs. `serve.ts`
194+
// assigns `process.env.NODE_ENV = 'development'` IN-PROCESS for `--dev`
195+
// when the variable is unset, before `runtime.start()`, and
196+
// plugin-auth's `isDevAdminSeedArmed()` reads it at CALL time inside
197+
// the `kernel:ready` hook — after that assignment. Both halves
198+
// re-measured on this tree, not inherited from the card.
199+
NODE_ENV: undefined,
146200
...env,
147201
}),
148202
}) as ChildProcessWithoutNullStreams;
@@ -213,6 +267,9 @@ async function stop(child: ChildProcessWithoutNullStreams): Promise<void> {
213267

214268
describe('#7645: the stdio MCP transport answers over a spawned CLI process', () => {
215269
beforeAll(async () => {
270+
// Build prerequisite first: the spawns below resolve `serve` from `dist/`.
271+
requireBuiltCli();
272+
216273
dir = mkdtempSync(join(tmpdir(), 'mcp-stdio-e2e-'));
217274
writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8');
218275
writeFileSync(

0 commit comments

Comments
 (0)