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
99 changes: 99 additions & 0 deletions packages/cli/test/helpers/serve-process.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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';
Expand All@@ -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:<want>`, print
* the port the kernel actually assigned, close. `want = 0` asks the kernel to
Expand Down
143 changes: 143 additions & 0 deletions packages/cli/test/serve-built-cli-prerequisite.test.ts
Original file line numberDiff line numberDiff line change
@@ -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);
}
});
});
54 changes: 15 additions & 39 deletions packages/cli/test/serve-mcp-capability-collision.e2e.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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), '..');
/**
Expand All@@ -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';
Expand DownExpand Up@@ -393,7 +369,7 @@ 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();
requireBuiltCli(RUN_JS_RESOLVES_FROM_DIST);

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