Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 82 additions & 24 deletions packages/cli/test/serve-mcp-capability-collision.e2e.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,31 +44,78 @@

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

const HERE = resolve(fileURLToPath(import.meta.url), '..');
/**
* `bin/run-dev.js` through `tsx` — the SOURCE entrypoint, like the ~20 sibling
* e2e files in this directory.
* `bin/run.js` through plain `node` — the SHIPPED entrypoint, and this file
* genuinely reaches it (#11707).
*
* ⛔ NOT `bin/run.js`. This file used to spell that one and call it "the SHIPPED
* entrypoint"; the claim was never true here (#11317). The boot below pins
* `NODE_ENV=development` on the child for the `--dev` admin seed, and
* @oclif/core 4.13.3 skips its TypeScript path lookup only when `isProd()` —
* `!['development', 'test'].includes(process.env.NODE_ENV ?? '')`. Under that
* value oclif rewrites the command target from the declared `./dist/commands`
* to `./src/commands` and transpiles, so `packages/cli/dist` is never consulted
* whichever stub is named. `serve-node-env-production-default.e2e.test.ts` is
* the file that genuinely reaches the built artifact, and it gets there by
* leaving `NODE_ENV` UNSET — the value that disables the reroute. Restoring
* `bin/run.js` here without dropping the `NODE_ENV` pin below is a no-op with a
* false comment attached.
* Both halves of that are load-bearing and neither works alone. @oclif/core
* 4.13.3 skips its TypeScript path lookup only when `isProd()` —
* `!['development', 'test'].includes(process.env.NODE_ENV ?? '')` — so under a
* child `NODE_ENV` of `development` or `test` it rewrites the command target
* from the declared `./dist/commands` to `./src/commands` and transpiles, and
* `packages/cli/dist` goes unread whichever stub is named. This file used to
* pin `NODE_ENV=development` on the child (#11317); the boot below leaves it
* UNSET instead — the value that disables the reroute — and the `--dev` admin
* seed survives that, for the reason spelled out at the `NODE_ENV` entry below.
*
* ⛔ Do not restore either half on its own. `bin/run-dev.js` assigns
* `NODE_ENV = 'development'` before argv is parsed, so it reroutes
* unconditionally; and re-pinning `NODE_ENV` on the child while naming
* `bin/run.js` is the self-cancelling pair #11317 found here — it promises the
* built artifact, delivers source, and says nothing.
*
* PRICE, stated because it is real. This file is now a verdict about BUILD
* STATE as well as about the source in the checkout, which is the trade
* `scripts/check-test-source-alias.mjs` argues against for in-process imports.
* `turbo.json` declares `@objectstack/cli#test` `dependsOn: ["build"]` (#11268)
* so CI always builds `dist/` first; `requireBuiltCli()` below is what a
* developer running `vitest` directly gets instead of oclif's "command serve
* not found". Neither catches a `dist/` that is merely BEHIND its source —
* that residual is the honest cost of consuming the artifact, and
* `serve-node-env-production-default.e2e.test.ts` (which has consumed `dist/`
* since #11113) carries exactly the same one.
*/
const CLI = resolve(HERE, '../bin/run-dev.js');
const CLI = resolve(HERE, '../bin/run.js');

/**
* Refuse to run against an unbuilt `packages/cli`, in a sentence rather than as
* oclif's "command serve not found".
*
* The command target is read from the CLI's own `oclif.commands.target` rather
* than restated here: that declaration is where `dist/commands` is decided, and
* a copy keeps probing the old path after someone moves it — the argument
* `scripts/cli-build-prerequisite.mjs` makes for the gates that shell out to
* this CLI. Only that one declared shape is read; anything else (unreadable,
* or `oclif.commands` written as a bare string) DEFERS rather than failing, so
* a checkout this cannot understand never turns red here and the spawn's own
* output stays the fallback — the same fail-open direction those gates take.
*/
function requireBuiltCli(): void {
let target: unknown;
try {
target = JSON.parse(readFileSync(resolve(HERE, '../package.json'), 'utf8'))?.oclif?.commands?.target;
} catch {
return;
}
if (typeof target !== 'string' || !target) return;
const commandFile = resolve(HERE, '..', target.replace(/^\.\//, ''), 'serve.js');
if (existsSync(commandFile)) return;
throw new Error(
`packages/cli is not built: ${commandFile} does not exist.\n` +
'This file spawns bin/run.js with NODE_ENV unset, which is what makes oclif resolve the ' +
'command from dist/ instead of transpiling src/ — so on an unbuilt tree the child answers ' +
'"command serve not found" and every boot below times out.\n' +
'CI declares the build (turbo: @objectstack/cli#test dependsOn build); a direct vitest run does not.\n' +
'Run: pnpm exec turbo run build --filter=@objectstack/cli',
);
}

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

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

describe('#7652: an app loading the MCP client connector still gets the MCP server', () => {
beforeAll(async () => {
// Build prerequisite first: the spawns below resolve `serve` from `dist/`.
requireBuiltCli();

dir = mkdtempSync(join(tmpdir(), 'mcp-collision-e2e-'));
writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8');
writeFileSync(
Expand Down
107 changes: 82 additions & 25 deletions packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,31 +43,78 @@

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

const HERE = resolve(fileURLToPath(import.meta.url), '..');
/**
* `bin/run-dev.js` through `tsx` — the SOURCE entrypoint, like the ~20 sibling
* e2e files in this directory.
* `bin/run.js` through plain `node` — the SHIPPED entrypoint, and this file
* genuinely reaches it (#11707).
*
* ⛔ NOT `bin/run.js`. This file used to spell that one and call it "the SHIPPED
* entrypoint"; the claim was never true here (#11317). The boot below pins
* `NODE_ENV=development` on the child for the `--dev` admin seed, and
* @oclif/core 4.13.3 skips its TypeScript path lookup only when `isProd()` —
* `!['development', 'test'].includes(process.env.NODE_ENV ?? '')`. Under that
* value oclif rewrites the command target from the declared `./dist/commands`
* to `./src/commands` and transpiles, so `packages/cli/dist` is never consulted
* whichever stub is named. `serve-node-env-production-default.e2e.test.ts` is
* the file that genuinely reaches the built artifact, and it gets there by
* leaving `NODE_ENV` UNSET — the value that disables the reroute. Restoring
* `bin/run.js` here without dropping the `NODE_ENV` pin below is a no-op with a
* false comment attached.
* Both halves of that are load-bearing and neither works alone. @oclif/core
* 4.13.3 skips its TypeScript path lookup only when `isProd()` —
* `!['development', 'test'].includes(process.env.NODE_ENV ?? '')` — so under a
* child `NODE_ENV` of `development` or `test` it rewrites the command target
* from the declared `./dist/commands` to `./src/commands` and transpiles, and
* `packages/cli/dist` goes unread whichever stub is named. This file used to
* pin `NODE_ENV=development` on the child (#11317); the boot below leaves it
* UNSET instead — the value that disables the reroute — and the `--dev` admin
* seed survives that, for the reason spelled out at the `NODE_ENV` entry below.
*
* ⛔ Do not restore either half on its own. `bin/run-dev.js` assigns
* `NODE_ENV = 'development'` before argv is parsed, so it reroutes
* unconditionally; and re-pinning `NODE_ENV` on the child while naming
* `bin/run.js` is the self-cancelling pair #11317 found here — it promises the
* built artifact, delivers source, and says nothing.
*
* PRICE, stated because it is real. This file is now a verdict about BUILD
* STATE as well as about the source in the checkout, which is the trade
* `scripts/check-test-source-alias.mjs` argues against for in-process imports.
* `turbo.json` declares `@objectstack/cli#test` `dependsOn: ["build"]` (#11268)
* so CI always builds `dist/` first; `requireBuiltCli()` below is what a
* developer running `vitest` directly gets instead of oclif's "command serve
* not found". Neither catches a `dist/` that is merely BEHIND its source —
* that residual is the honest cost of consuming the artifact, and
* `serve-node-env-production-default.e2e.test.ts` (which has consumed `dist/`
* since #11113) carries exactly the same one.
*/
const CLI = resolve(HERE, '../bin/run.js');

/**
* Refuse to run against an unbuilt `packages/cli`, in a sentence rather than as
* oclif's "command serve not found".
*
* The command target is read from the CLI's own `oclif.commands.target` rather
* than restated here: that declaration is where `dist/commands` is decided, and
* a copy keeps probing the old path after someone moves it — the argument
* `scripts/cli-build-prerequisite.mjs` makes for the gates that shell out to
* this CLI. Only that one declared shape is read; anything else (unreadable,
* or `oclif.commands` written as a bare string) DEFERS rather than failing, so
* a checkout this cannot understand never turns red here and the spawn's own
* output stays the fallback — the same fail-open direction those gates take.
*/
const CLI = resolve(HERE, '../bin/run-dev.js');
function requireBuiltCli(): void {
let target: unknown;
try {
target = JSON.parse(readFileSync(resolve(HERE, '../package.json'), 'utf8'))?.oclif?.commands?.target;
} catch {
return;
}
if (typeof target !== 'string' || !target) return;
const commandFile = resolve(HERE, '..', target.replace(/^\.\//, ''), 'serve.js');
if (existsSync(commandFile)) return;
throw new Error(
`packages/cli is not built: ${commandFile} does not exist.\n` +
'This file spawns bin/run.js with NODE_ENV unset, which is what makes oclif resolve the ' +
'command from dist/ instead of transpiling src/ — so on an unbuilt tree the child answers ' +
'"command serve not found" and every boot below times out.\n' +
'CI declares the build (turbo: @objectstack/cli#test dependsOn build); a direct vitest run does not.\n' +
'Run: pnpm exec turbo run build --filter=@objectstack/cli',
);
}

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

describe('#7645: the stdio MCP transport answers over a spawned CLI process', () => {
beforeAll(async () => {
// Build prerequisite first: the spawns below resolve `serve` from `dist/`.
requireBuiltCli();

dir = mkdtempSync(join(tmpdir(), 'mcp-stdio-e2e-'));
writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8');
writeFileSync(
Expand Down
26 changes: 20 additions & 6 deletions packages/cli/test/serve-node-env-production-default.e2e.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -85,23 +85,37 @@
*
* ## WHY THIS FILE IS THE REASON `@objectstack/cli#test` DECLARES `build`
*
* This is the only file in `packages/cli` that genuinely consumes
* `packages/cli/dist`, and it is the only one that can be, for a reason that
* is this card's own subject matter turned back on the test harness.
* This file was the FIRST in `packages/cli` to genuinely consume
* `packages/cli/dist`, and for a while the only one, for a reason that is
* this card's own subject matter turned back on the test harness. Four files
* consume it today: this one, and the three named below that #11707 moved
* onto the built artifact. What did not change is that the `dependsOn`
* declaration this section is about is what makes any of them safe.
*
* `turbo.json` used to declare `"@objectstack/cli#test": { dependsOn:
* ["^build"] }` — dependencies only, never this package's own build. So
* `packages/cli/dist` does not exist when the `Test Core` shard runs. Five
* other files here name `bin/run.js`; two only assert the path as a string,
* and the three that actually spawn it (`serve-mcp-stdio-answers`,
* `serve-mcp-capability-collision`, `serve-stdio-stdout-purity`) pass
* `NODE_ENV: 'development'` to the child so its `--dev` admin seed runs.
* `serve-mcp-capability-collision`, `serve-stdio-stdout-purity`) used to pass
* `NODE_ENV: 'development'` to the child so its `--dev` admin seed ran.
* That value is also what makes `@oclif/core`'s `tsPath()` rewrite the
* command target from the declared `./dist/commands` to `./src/commands` and
* auto-transpile: `lib/util/util.js` defines `isProd = () =>
* !['development','test'].includes(process.env.NODE_ENV ?? '')`, and the
* lookup is skipped only when that is true. Those three therefore never
* touch `dist/` at all, and the missing build stayed invisible.
* touched `dist/` at all, and the missing build stayed invisible.
*
* THEY REACH IT NOW, and the declaration below is the whole reason that is
* allowed. #11707 dropped that `NODE_ENV` pin: all three leave the variable
* unset and spawn `bin/run.js` through plain `node` — this file's own shape —
* and the `--dev` seed survives because `serve.ts` assigns
* `process.env.NODE_ENV = 'development'` IN-PROCESS for `--dev` before
* `runtime.start()`, which is after oclif has already resolved the command.
* Measured when they moved, with a distinct marker planted in each tree:
* `node bin/run.js` + unset executes `dist/commands`, `node bin/run.js` +
* `NODE_ENV=development` executes `src/commands`. The pair is as silent as it
* ever was; what removed the hazard is that `dependsOn` reads `["build"]`.
*
* This pin cannot dodge it. Unset `NODE_ENV` is the input under test, and
* unset is exactly the value that leaves `isProd()` true and the reroute
Expand Down
Loading
Loading