diff --git a/packages/cli/test/helpers/serve-process.ts b/packages/cli/test/helpers/serve-process.ts index 1e19e0dbe9..1774cee8bd 100644 --- a/packages/cli/test/helpers/serve-process.ts +++ b/packages/cli/test/helpers/serve-process.ts @@ -11,6 +11,7 @@ */ import { execFileSync, spawn } from 'node:child_process'; +import { existsSync, readFileSync } from 'node:fs'; import { createServer, type Server } from 'node:net'; import { resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -21,6 +22,104 @@ const HERE = resolve(fileURLToPath(import.meta.url), '..'); export const CLI = resolve(HERE, '../../bin/run-dev.js'); export const TSX = resolve(HERE, '../../../../node_modules/.bin/tsx'); +// ───────────────────────────────────────────────────────────────────────── +// THE OTHER ENTRYPOINT: `bin/run.js`, and the build state it silently needs +// +// `CLI` above is `bin/run-dev.js`, which pins `NODE_ENV=development` and runs +// the command from `src/` through tsx — so a file using `runServe()` needs no +// `packages/cli/dist` at all. A handful of e2e files deliberately spawn the +// OTHER entrypoint instead, because the thing they measure only exists when +// oclif resolves the command from the BUILT artifact. Those files, and only +// those, carry a build-state prerequisite, and it used to be invisible: an +// unbuilt worktree answered ` › Error: command serve not found`, the harness +// reported `serve exited 2 before "Server is ready"`, and nothing in either +// sentence said "run the build" (#12539). +// +// ⭐ The guard is here rather than in those files because it was written THREE +// times, byte-identical, 19 lines each (#11707 / PR #12459 swept three +// spawners in one edit and each got its own copy). Three copies of a refusal is +// the same defect the refusal exists to prevent, one level up. +// +// ⛔ It is NOT a general "is the CLI ready" preflight. `runServe()` must never +// call it: a tsx child reads `src/`, so `packages/cli/dist` is not that child's +// prerequisite and a guard that refused there would be a false red on a tree +// that can run the test perfectly well. +// ───────────────────────────────────────────────────────────────────────── + +/** + * Why a `bin/run.js` child needs `packages/cli/dist`, in the child's own terms. + * + * ⭐ Named and exported rather than defaulted inside `requireBuiltCli()`, which + * is the whole point: this sentence is true of the `bin/run.js` + unset- + * `NODE_ENV` spawn and of nothing else. A caller reaching a different way (a + * `bin/run-dev.js` + tsx child, a `pnpm` bin shim, a packed tarball) has a + * DIFFERENT reason, and pasting this one there would attach a false + * explanation to a true refusal — the failure class #12498 and #12561 were + * filed for. The identifier says `RUN_JS` so that misuse has to be deliberate. + */ +export const RUN_JS_RESOLVES_FROM_DIST = + '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.'; + +/** + * The refusal itself, separated from the check so its WORDING can be pinned. + * + * `requireBuiltCli()` can only produce this Error on an unbuilt tree, and no + * test can produce an unbuilt tree without breaking every neighbouring file in + * the same run. So the sentence a reader actually acts on would otherwise be + * the one part of this guard nothing checks — and a refusal that forgets to + * name the build command is exactly the false red #12539 exists to end. + * `serve-built-cli-prerequisite.test.ts` pins it through this function. + * + * @param commandFile the `dist/` command file that was looked for and missing + * @param mechanism why THIS caller's child needs it — see + * `RUN_JS_RESOLVES_FROM_DIST` for the only one in the tree + */ +export function unbuiltCliError(commandFile: string, mechanism: string): Error { + return new Error( + `packages/cli is not built: ${commandFile} does not exist.\n` + + `${mechanism}\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', + ); +} + +/** + * 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. + * + * `serve.js` is the probe because it is the command every caller of this guard + * spawns, and one `tsup` run emits the whole `dist/commands` directory — so its + * absence answers "this package was never built" for any of them. ⛔ It does + * not catch a `dist/` that is merely BEHIND its source; that residual is the + * honest cost of consuming the artifact and is stated in each caller's header. + * + * @param mechanism why this caller's child resolves the command from `dist/`. + * Required, with no default: see `RUN_JS_RESOLVES_FROM_DIST`. + */ +export function requireBuiltCli(mechanism: string): 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 unbuiltCliError(commandFile, mechanism); +} + /** * The bind probe, run in a throwaway Node process: bind `0.0.0.0:`, print * the port the kernel actually assigned, close. `want = 0` asks the kernel to diff --git a/packages/cli/test/serve-built-cli-prerequisite.test.ts b/packages/cli/test/serve-built-cli-prerequisite.test.ts new file mode 100644 index 0000000000..e48fbbf243 --- /dev/null +++ b/packages/cli/test/serve-built-cli-prerequisite.test.ts @@ -0,0 +1,143 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The `bin/run.js` build-state prerequisite says what to run, and says it in + * the CALLER's terms (#12539). + * + * ── What this file is for ──────────────────────────────────────────────── + * + * A handful of e2e files in this directory spawn `bin/run.js` rather than + * `bin/run-dev.js`, because what they measure only exists when oclif resolves + * the command from the BUILT artifact. On a worktree where only the dependency + * closure was built — `pnpm --filter '@objectstack/cli^...' build`, the + * documented first command — the child answers + * ` › Error: command serve not found` and the harness reports + * `serve exited 2 before "Server is ready"`. Neither sentence says "run the + * build", so a build-state prerequisite arrives dressed as a regression on a + * file with no visible connection to a build step. That is the card. + * + * `requireBuiltCli()` (`helpers/serve-process.ts`) is the answer, and the ONLY + * part of it a reader ever acts on is the sentence it throws. That sentence can + * only be produced on an unbuilt tree, and no test can produce an unbuilt tree + * without breaking every neighbouring file in the same run — so without this + * file the wording would be the one part of the guard nothing checks. A refusal + * that forgets to name the build command is the false red restated, not fixed. + * + * ── Why the mechanism is a PARAMETER, and why that is pinned here ───────── + * + * Until #12539 the guard lived as three byte-identical private copies, each + * carrying `"This file spawns bin/run.js with NODE_ENV unset…"` inline. That + * sentence is true of the `bin/run.js` + unset-`NODE_ENV` spawn and of nothing + * else: a `bin/run-dev.js` + tsx child reads `src/`, and a hoisted copy + * carrying the text outward would be a FALSE EXPLANATION attached to a true + * refusal — the class #12498 and #12561 were filed for. So the reason is the + * caller's to supply, and `it('carries the caller's mechanism…')` below is what + * keeps it that way: it passes a foreign mechanism and demands the `bin/run.js` + * sentence be absent. + * + * ⚠️ `helpers/serve-process.ts` is a TEST helper, so `check:cross-package-test- + * inputs` and the source-alias gate both already see it; nothing here reads + * outside `packages/cli`. + */ + +import { describe, it, expect } from 'vitest'; +import { existsSync, readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + RUN_JS_RESOLVES_FROM_DIST, + requireBuiltCli, + unbuiltCliError, +} from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +/** `packages/cli` — this package's own root, never another package's. */ +const PACKAGE_ROOT = resolve(HERE, '..'); + +/** A mechanism belonging to some OTHER entrypoint — the misuse the guard must not commit. */ +const FOREIGN_MECHANISM = 'This file execs the packed tarball, which has no src/ to fall back to.'; + +describe('#12539: the unbuilt-CLI refusal is legible', () => { + it('names the build command, so the reader knows what to run', () => { + const message = unbuiltCliError('/repo/packages/cli/dist/commands/serve.js', RUN_JS_RESOLVES_FROM_DIST).message; + // The whole point of the card. A refusal that stops at "not built" costs + // the reader the same round the false red did. + expect(message).toContain('Run: pnpm exec turbo run build --filter=@objectstack/cli'); + }); + + it('names the artifact it looked for, not just the package', () => { + const message = unbuiltCliError('/repo/packages/cli/dist/commands/serve.js', RUN_JS_RESOLVES_FROM_DIST).message; + expect(message).toContain('/repo/packages/cli/dist/commands/serve.js'); + expect(message).toContain('packages/cli is not built'); + }); + + it('says why CI never sees this, so a green CI is not read as a contradiction', () => { + const message = unbuiltCliError('/x/serve.js', RUN_JS_RESOLVES_FROM_DIST).message; + expect(message).toContain('@objectstack/cli#test dependsOn build'); + expect(message).toContain('a direct vitest run does not'); + }); + + // ── The instrument can say no ────────────────────────────────────────── + it("carries the CALLER's mechanism, and no other entrypoint's", () => { + const message = unbuiltCliError('/x/serve.js', FOREIGN_MECHANISM).message; + expect(message).toContain(FOREIGN_MECHANISM); + // ⛔ The load-bearing half. If the `bin/run.js` sentence is ever inlined + // back into the helper "so callers do not have to pass one", this goes red + // — which is the only thing standing between a shared refusal and a shared + // WRONG explanation. + expect(message).not.toContain('bin/run.js'); + expect(message).not.toContain('transpiling src/'); + }); + + it('reproduces, byte for byte, what the three private copies threw before the hoist', () => { + // #12539 moved this refusal out of three files; it did not reword it. The + // literal below is the message measured on `09b4f4e4e`, so a rewording has + // to be a deliberate edit here rather than a side effect of the move. + expect(unbuiltCliError('/repo/packages/cli/dist/commands/serve.js', RUN_JS_RESOLVES_FROM_DIST).message).toBe( + 'packages/cli is not built: /repo/packages/cli/dist/commands/serve.js 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', + ); + }); +}); + +describe('#12539: the guard probes the DECLARED command target, and is silent when it is there', () => { + /** + * The path the guard must be looking at, derived the same way it derives it — + * from `oclif.commands.target`, which is where `dist/commands` is decided. + * Restating `dist/commands` here would pin the guard to a path the CLI is + * free to move, which is the copy this whole card is about. + */ + const declared = (): string => { + const target = JSON.parse(readFileSync(resolve(PACKAGE_ROOT, 'package.json'), 'utf8'))?.oclif?.commands?.target; + expect(typeof target, 'oclif.commands.target is what the guard reads; a bare string breaks it').toBe('string'); + return resolve(PACKAGE_ROOT, String(target).replace(/^\.\//, ''), 'serve.js'); + }; + + it('reads the target off the CLI declaration rather than restating it', () => { + expect(declared().endsWith('serve.js')).toBe(true); + expect(declared().startsWith(PACKAGE_ROOT)).toBe(true); + }); + + /** + * ⭐ BOTH directions, decided by the tree this run is actually on — which is + * why it is one `it()` and not two. + * + * On CI, and on any tree where `@objectstack/cli` is built, this asserts the + * expensive half: the guard is SILENT. A guard that fires on a correctly + * built tree is strictly worse than the false red it replaces, because then + * every green in this directory is a coin flip. On an unbuilt tree it asserts + * the other half — that it fires, and names the artifact it looked for. + */ + it('is silent when the declared command file exists, and refuses naming it when it does not', () => { + const commandFile = declared(); + if (existsSync(commandFile)) { + expect(() => requireBuiltCli(RUN_JS_RESOLVES_FROM_DIST)).not.toThrow(); + } else { + expect(() => requireBuiltCli(RUN_JS_RESOLVES_FROM_DIST)).toThrow(commandFile); + } + }); +}); diff --git a/packages/cli/test/serve-mcp-capability-collision.e2e.test.ts b/packages/cli/test/serve-mcp-capability-collision.e2e.test.ts index e2bd58cbc8..ef3af2aab4 100644 --- a/packages/cli/test/serve-mcp-capability-collision.e2e.test.ts +++ b/packages/cli/test/serve-mcp-capability-collision.e2e.test.ts @@ -44,11 +44,17 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdtempSync, 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, childEnv, randomPort } from './helpers/serve-process.js'; +import { + E2E_SECRET_KEY, + RUN_JS_RESOLVES_FROM_DIST, + childEnv, + randomPort, + requireBuiltCli, +} from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); /** @@ -75,48 +81,18 @@ const HERE = resolve(fileURLToPath(import.meta.url), '..'); * 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 — + * so CI always builds `dist/` first; `requireBuiltCli()` — hoisted into + * `helpers/serve-process.ts` by #12539, with the reason it refuses supplied + * from HERE (`RUN_JS_RESOLVES_FROM_DIST`) because it is true of this + * entrypoint and not of the tsx one — 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. - */ -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'; const CONSUMER_CLASS_NAME = 'ConnectorMcpPlugin'; @@ -393,7 +369,7 @@ async function readFrame(res: Response, id: number): Promise { beforeAll(async () => { // Build prerequisite first: the spawns below resolve `serve` from `dist/`. - requireBuiltCli(); + requireBuiltCli(RUN_JS_RESOLVES_FROM_DIST); dir = mkdtempSync(join(tmpdir(), 'mcp-collision-e2e-')); writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8'); diff --git a/packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts b/packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts index 6a845fa6dc..2ed1fc8e32 100644 --- a/packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts +++ b/packages/cli/test/serve-mcp-stdio-answers.e2e.test.ts @@ -43,11 +43,17 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdtempSync, 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, childEnv, randomPort } from './helpers/serve-process.js'; +import { + E2E_SECRET_KEY, + RUN_JS_RESOLVES_FROM_DIST, + childEnv, + randomPort, + requireBuiltCli, +} from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); /** @@ -74,48 +80,18 @@ const HERE = resolve(fileURLToPath(import.meta.url), '..'); * 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 — + * so CI always builds `dist/` first; `requireBuiltCli()` — hoisted into + * `helpers/serve-process.ts` by #12539, with the reason it refuses supplied + * from HERE (`RUN_JS_RESOLVES_FROM_DIST`) because it is true of this + * entrypoint and not of the tsx one — 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. - */ -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 { manifest: { @@ -358,7 +334,7 @@ async function stop(child: ChildProcessWithoutNullStreams): Promise { 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(); + requireBuiltCli(RUN_JS_RESOLVES_FROM_DIST); dir = mkdtempSync(join(tmpdir(), 'mcp-stdio-e2e-')); writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8'); diff --git a/packages/cli/test/serve-node-env-production-default.e2e.test.ts b/packages/cli/test/serve-node-env-production-default.e2e.test.ts index 6acd2b1b48..926b18f47b 100644 --- a/packages/cli/test/serve-node-env-production-default.e2e.test.ts +++ b/packages/cli/test/serve-node-env-production-default.e2e.test.ts @@ -153,6 +153,12 @@ * file exists to measure unreachable — the pin would go green measuring * nothing. `bin/run.js` plus a genuinely built `dist/` is the only shape * that reaches the gate for this pin. + * + * ⭐ And that prerequisite is now STATED rather than discovered: `beforeAll` + * calls `requireBuiltCli()` (#12539), so an unbuilt worktree gets a sentence + * naming the build command instead of ` › Error: command serve not found` + * relayed as `serve exited 2 before "Server is ready"`. The reason it prints + * is this file's own — see `UNSET_LEG_MEASURES_THE_BUILT_DIST` below. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -161,7 +167,13 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; import type { Readable } from 'node:stream'; import { fileURLToPath } from 'node:url'; -import { childEnv, E2E_SECRET_KEY, portContentionError, reservePort } from './helpers/serve-process.js'; +import { + childEnv, + E2E_SECRET_KEY, + portContentionError, + requireBuiltCli, + reservePort, +} from './helpers/serve-process.js'; /** What `spawn(..., { stdio: ['ignore', 'pipe', 'pipe'] })` actually returns — no `stdin`. */ type ProbeChild = ChildProcessByStdio; @@ -170,6 +182,36 @@ const HERE = resolve(fileURLToPath(import.meta.url), '..'); /** `bin/run.js` — the SHIPPED entrypoint. See the file header for why this one, not `run-dev.js`. */ const CLI = resolve(HERE, '../bin/run.js'); +/** + * Why THIS file needs `packages/cli/dist`, in its own terms (#12539). + * + * ⛔ NOT `RUN_JS_RESOLVES_FROM_DIST`, the constant the three sibling spawners + * pass. That sentence ends `… and every boot below times out`, which holds for + * a file whose every boot goes through `bin/run.js` with `NODE_ENV` unset. + * This file is not one: of its three legs only the unset pin resolves from + * `dist/`, and the other two hand the child `development`/`test` — exactly the + * value that makes `@oclif/core`'s `isProd()` false and reroutes them to + * `src/commands` (the header above; both are pinned as `DELIBERATE_REROUTE` in + * `scripts/check-cli-test-child-env.mjs`). Passing the siblings' sentence here + * would be a true refusal carrying a false explanation — the class #12498, + * #12561 and #12563 were filed for. + * + * ⭐ It is also why the guard runs for the WHOLE file rather than for the + * unset leg alone. Measured on a closure-only tree before this guard existed, + * this file reported `3 tests | 1 failed`: the unset pin failed with + * `serve exited 2 before "Server is ready"`, and the two rerouted legs PASSED + * — from `src/`, reporting green for a program this file's header says it does + * not measure. A partial green that misreports which program it measured is + * worse than a red (#12561). + */ +const UNSET_LEG_MEASURES_THE_BUILT_DIST = + 'Only the unset-NODE_ENV pin here resolves from dist/: unset is the one value that leaves ' + + "@oclif/core's isProd() true, so that leg globs the real dist/commands and answers " + + '"command serve not found" on an unbuilt tree. The other two legs set NODE_ENV to ' + + 'development/test, which reroutes them to src/commands — they would PASS without a built ' + + 'dist/, reporting green for a program this file does not measure. So the whole file ' + + 'refuses, not just that leg.'; + /** * The fixture's parent directory sits INSIDE `packages/cli/test/`, not the * system tmpdir: the config below does a real, static @@ -387,6 +429,10 @@ async function stop(child: ProbeChild): Promise { describe('#11113: os serve defaults NODE_ENV to production when unset', () => { beforeAll(() => { + // Build prerequisite first (#12539): the unset pin below resolves `serve` + // from `dist/`, and the two rerouted legs must not report green without it. + requireBuiltCli(UNSET_LEG_MEASURES_THE_BUILT_DIST); + dir = mkdtempSync(join(FIXTURES_ROOT, 'tmp-node-env-default-')); writeFileSync( join(dir, 'package.json'), diff --git a/packages/cli/test/serve-stdio-stdout-purity.e2e.test.ts b/packages/cli/test/serve-stdio-stdout-purity.e2e.test.ts index 5af353594b..a57da4bd1a 100644 --- a/packages/cli/test/serve-stdio-stdout-purity.e2e.test.ts +++ b/packages/cli/test/serve-stdio-stdout-purity.e2e.test.ts @@ -40,11 +40,17 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdtempSync, 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, childEnv, randomPort } from './helpers/serve-process.js'; +import { + E2E_SECRET_KEY, + RUN_JS_RESOLVES_FROM_DIST, + childEnv, + randomPort, + requireBuiltCli, +} from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); /** @@ -71,48 +77,18 @@ const HERE = resolve(fileURLToPath(import.meta.url), '..'); * 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 — + * so CI always builds `dist/` first; `requireBuiltCli()` — hoisted into + * `helpers/serve-process.ts` by #12539, with the reason it refuses supplied + * from HERE (`RUN_JS_RESOLVES_FROM_DIST`) because it is true of this + * entrypoint and not of the tsx one — 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. - */ -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 { manifest: { @@ -366,7 +342,7 @@ function parseFrame(line: string): Record | undefined { describe('#7915: a stdio MCP boot writes nothing but protocol frames to stdout', () => { beforeAll(async () => { // Build prerequisite first: the spawns below resolve `serve` from `dist/`. - requireBuiltCli(); + requireBuiltCli(RUN_JS_RESOLVES_FROM_DIST); dir = mkdtempSync(join(tmpdir(), 'mcp-stdout-purity-e2e-')); writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8');