From a4b6282a21dcb2a6ed13e352bcbdd1d0e9c4dec8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 06:14:05 +0000 Subject: [PATCH 1/5] fix(cli): os serve defaults NODE_ENV to production when unset os start already forces NODE_ENV='production' on the unset case, but on the child environment it assembles for its spawn. os serve runs in-process, so there was no equivalent write, and the whole family of NODE_ENV !== 'production' gates across the tree read the raw undefined and took the non-production branch on a boot that never declared itself anything else. Adds the same default os serve already applies for --dev, at the same early point in run() - before any dynamically-imported runtime module and before every gate downstream reads the variable. An explicitly-set NODE_ENV (development, test, anything else) is never overridden. --- .../serve-node-env-production-default.md | 62 +++++ packages/cli/src/commands/serve.ts | 23 +- ...ve-node-env-production-default.e2e.test.ts | 229 ++++++++++++++++++ 3 files changed, 312 insertions(+), 2 deletions(-) create mode 100644 .changeset/serve-node-env-production-default.md create mode 100644 packages/cli/test/serve-node-env-production-default.e2e.test.ts diff --git a/.changeset/serve-node-env-production-default.md b/.changeset/serve-node-env-production-default.md new file mode 100644 index 0000000000..cebfb66f22 --- /dev/null +++ b/.changeset/serve-node-env-production-default.md @@ -0,0 +1,62 @@ +--- +"@objectstack/cli": minor +--- + +fix(cli): `os serve` defaults `NODE_ENV` to `production` when unset, exactly as `os start` already does (#11113) + +**BREAKING for a deployment that runs `os serve` with `NODE_ENV` unset and +relies on a development-class convenience surviving into a real boot.** +Shipped as `minor` under the repo's launch-window convention for breaking +changes, not `patch` — this is a deliberate default flip, not a bugfix that +restores previously-intended behaviour. + +`os start` has forced `NODE_ENV='production'` on the unset case since #5673, +but it does so on the child environment it assembles for its **spawn** +(`start.ts:347`). `os serve` runs **in-process** — there was no equivalent +write, so the whole family of `NODE_ENV !== 'production'` gates across the +tree read the raw `undefined` and took the non-production branch on a boot +that never declared itself anything else. Filed as #11113, the declared +residual of #10366 (which closed the same gate's *set-but-wrong* case and +left this one for its own card, per the disposition precedent on #11035). + +One line: `serve.ts` now defaults `process.env.NODE_ENV` to `'production'` +when unset, at the same point it already defaults it to `'development'` under +`--dev` — before any of the runtime modules it dynamically imports, and before +every gate downstream reads the variable. An explicitly-set `NODE_ENV` +(`development`, `test`, anything else) is never overridden. + +The full behaviour-flip survey — every `NODE_ENV`-reading predicate in the +tree, which ones flip and which don't, and why — is in the PR body (#11113), +not repeated here. Highlights of what an unset-`NODE_ENV` `os serve` boot now +gets, that it did not before: + +- plugin-auth's localhost trusted-origin CSRF substitution closes (the + regression this card pins). +- plugin-auth's CSRF Origin/Referer synthesis for headerless requests closes. +- plugin-auth's missing-`OS_AUTH_SECRET` fallback to a forgeable + `dev-secret-` becomes a refusal to boot instead. +- plugin-auth stops printing invitation / magic-link URLs and OTP codes to + logs. +- plugin-dev's ADR-0115 D6 boot guard now refuses to initialize the dev + assembly (well-known auth secret, seeded dev admin) instead of loading it. +- the SQL driver's auto-DDL guard stops silently applying `safe` schema drift. +- the seed loader stops seeding dev-scoped datasets into what it previously + could not tell apart from production. +- service-settings' local crypto provider now requires a stable key instead + of tolerating an auto-generated / ephemeral one. + +Every one of those is the intended tightening this card exists to make: an +operator (or an AI-authored deploy script) that never exported `NODE_ENV` is +running a real deployment, and the safe direction is to treat it as one, loud +failures included, rather than silently keep a development-class door open. +`NODE_ENV=development` / `NODE_ENV=test` — including the flows `os dev` and +`os serve --dev` already carry — are unaffected; only the unset case moves. + + diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index ba779d8282..561cbfc066 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -1095,8 +1095,27 @@ export default class Serve extends Command { // time) see development mode. We deliberately do NOT inherit // NODE_ENV from the parent `os dev` spawn — see the note in // commands/dev.ts for why. - if (flags.dev && !process.env.NODE_ENV) { - process.env.NODE_ENV = 'development'; + // + // The `else` branch is `os serve`'s side of #11113. `os start` already + // defaults NODE_ENV to 'production' on the unset case, but it does so on + // `localEnv` — a child environment assembled for a SPAWN (start.ts:347). + // `serve` runs in-process, so there is no child env to default; the + // equivalent has to mutate `process.env.NODE_ENV` itself. It has to + // happen HERE, at the same point the --dev branch above already sets it, + // and for the identical reason: every `await import(...)` below, and + // every `NODE_ENV !== 'production'` (or equivalent) gate downstream — + // plugin-auth's localhost trusted-origin CSRF substitution, its Origin + // synthesis, its auth-secret fallback and OTP-deliverability check; + // plugin-dev's production boot guard; the SQL driver's auto-DDL guard; + // service-settings' crypto-key mode; the seed loader's env scoping — must + // observe the default, not the raw unset value. Full survey in #11113. + // An operator who never exported NODE_ENV is booting a real deployment, + // not asking to be treated as development — this is `os serve` agreeing + // with `os start` on that, one line, for the whole gate family at once. + if (flags.dev) { + if (!process.env.NODE_ENV) process.env.NODE_ENV = 'development'; + } else if (!process.env.NODE_ENV) { + process.env.NODE_ENV = 'production'; } const requestedPort = parseInt(flags.port); 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 new file mode 100644 index 0000000000..7d65f1b2e3 --- /dev/null +++ b/packages/cli/test/serve-node-env-production-default.e2e.test.ts @@ -0,0 +1,229 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #11113 — `os serve` with an UNSET `NODE_ENV` must be treated as production, + * exactly as `os start` already is. + * + * `os start` defaults `NODE_ENV` to `'production'` on the unset case, but it + * does so on `localEnv` — a child environment assembled for a SPAWN + * (`start.ts:347`). `os serve` runs IN-PROCESS, so there is no child env to + * default; the equivalent has to mutate `process.env.NODE_ENV` itself, early + * enough that every `NODE_ENV !== 'production'` gate downstream — starting + * with plugin-auth's localhost trusted-origin CSRF substitution (#10366) — + * observes the default rather than the raw unset value. + * + * WHY THIS FILE SPAWNS THE REAL, BUILT CLI (`bin/run.js`, not + * `bin/run-dev.js`). `run-dev.js` unconditionally sets + * `process.env.NODE_ENV = 'development'` before it even parses argv — it is + * the tsx source-loader shim `pnpm dev` and the other e2e fixtures use, and + * it would make the exact input this defect is about (a truly UNSET + * `NODE_ENV`) unreachable no matter what the test passes to `spawn()`'s `env`. + * Only the shipped, built entrypoint leaves `NODE_ENV` exactly as the spawn + * environment supplies it — which is also why a plain in-process unit test + * (`auth-manager.test.ts`'s "substitutes the trio when NODE_ENV is unset") + * cannot stand in for this one: it proves the GATE reads unset the open way, + * not that `serve.ts`'s boot sets `process.env.NODE_ENV` EARLY ENOUGH for the + * gate to observe the default before it is first read. A fix that set the + * default after the imports/gates below would pass that unit test and still + * ship with the door open — that ordering is what only a real boot can catch. + * + * WHY THE PROBE IS AN ORIGIN CHECK, NOT A SIGN-IN. The trusted-origin + * substitution is consulted by better-auth's CSRF/origin middleware BEFORE + * the sign-in handler ever looks at the credentials in the body (see + * `origin-check.mjs`'s `validateFormCsrf` → `validateOrigin`), so a request + * with bogus credentials still tells the two states apart: gate OPEN answers + * with whatever `sign-in/email` says about the (wrong) credentials, gate + * CLOSED never reaches that logic and answers `403 INVALID_ORIGIN` first. + * `OS_AUTH_SECRET` is set explicitly in every boot below so the outcome is + * never confused with `serve.ts`'s separate "AuthPlugin skipped, no secret" + * warning path (`serve.ts` ~2444) — that path is orthogonal to this gate and + * unaffected by this fix (it is keyed on `isDev`, not on the raw variable). + * + * The probed Origin is `http://localhost:` — + * different from the port `serve` itself binds to — so a pass can only be + * explained by the wildcard `http://localhost:*` substitution, never by + * better-auth's own default trust of the deployment's own origin. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +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'); + +const CONFIG = ` +export default { + manifest: { + id: 'com.example.nodeenvdefault', + namespace: 'nodeenvdefault', + version: '1.0.0', + type: 'app', + name: 'NODE_ENV production-default probe', + }, + objects: [{ + name: 'nodeenvdefault_task', + label: 'Task', + sharingModel: 'public', + fields: { title: { type: 'text', label: 'Title' } }, + }], +}; +`; + +let dir: string; +const children: ChildProcessWithoutNullStreams[] = []; + +/** A random high port, so a run never contends with another agent's dev server on this host. */ +function randomPort(): number { + return 41000 + Math.floor(Math.random() * 19000); +} + +interface OriginCheckResult { + status: number; + body: any; +} + +/** + * Boot `os serve` for real against the shipped entrypoint, wait for the ready + * banner, POST a sign-in attempt carrying an untrusted-looking localhost + * Origin and no cookie, then shut the child down. Never leaves a child + * running past its own test. + * + * `env.NODE_ENV` may be `undefined` to leave the variable truly UNSET for the + * child — Node's `spawn()` omits `undefined`-valued env entries rather than + * stringifying them, so this is not the same as `NODE_ENV=''`. + */ +async function probeOriginCheck(env: Record): Promise { + const port = randomPort(); + const untrustedOriginPort = port + 1; + + const child = spawn(process.execPath, [CLI, 'serve', '-p', String(port)], { + cwd: dir, + stdio: ['ignore', 'pipe', 'pipe'], + env: { + ...process.env, + NO_COLOR: '1', + OS_LOG_LEVEL: '', + OS_DISABLE_CONSOLE: '1', + OS_DATABASE_URL: ':memory:', + // Explicit and real, so the boot never takes the orthogonal "AuthPlugin + // skipped — no OS_AUTH_SECRET" path (serve.ts) regardless of which + // NODE_ENV state this call is probing. + OS_AUTH_SECRET: 'e2e-node-env-default-probe-secret-not-for-real-use', + // The base default for every call: truly unset, unless overridden by + // `env` below. Node's spawn omits an `undefined`-valued entry rather + // than inheriting whatever this test RUNNER's own process (vitest sets + // NODE_ENV=test) happened to have. + NODE_ENV: undefined, + ...env, + }, + }) as ChildProcessWithoutNullStreams; + children.push(child); + + let out = ''; + let err = ''; + + await new Promise((readyResolve, readyReject) => { + const timer = setTimeout(() => { + readyReject(new Error(`serve never reached "Server is ready"\n--- stdout ---\n${out}\n--- stderr ---\n${err}`)); + }, 150_000); + const onData = () => { + if (/Server is ready/.test(out + err)) { + clearTimeout(timer); + readyResolve(); + } + }; + child.stdout.on('data', (d) => { out += String(d); onData(); }); + child.stderr.on('data', (d) => { err += String(d); onData(); }); + child.on('exit', (code) => { + clearTimeout(timer); + readyReject(new Error(`serve exited ${code} before "Server is ready"\n--- stdout ---\n${out}\n--- stderr ---\n${err}`)); + }); + }); + + try { + const res = await fetch(`http://localhost:${port}/api/v1/auth/sign-in/email`, { + method: 'POST', + headers: { + 'content-type': 'application/json', + // No cookie header — this is the shape `validateFormCsrf` forces an + // origin check for when neither Sec-Fetch-* nor a cookie is present. + origin: `http://localhost:${untrustedOriginPort}`, + }, + body: JSON.stringify({ email: 'nobody@example.com', password: 'definitely-wrong-password' }), + }); + let body: any = null; + try { body = await res.json(); } catch { /* non-JSON error body, fall through with null */ } + return { status: res.status, body }; + } finally { + await stop(child); + } +} + +async function stop(child: ChildProcessWithoutNullStreams): Promise { + if (child.exitCode !== null || child.signalCode !== null) return; + await new Promise((done) => { + const give = setTimeout(() => { + try { child.kill('SIGKILL'); } catch { /* already gone */ } + done(); + }, 10_000); + child.once('exit', () => { clearTimeout(give); done(); }); + try { + child.kill('SIGTERM'); + } catch { + clearTimeout(give); + done(); + } + }); +} + +describe('#11113: os serve defaults NODE_ENV to production when unset', () => { + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'serve-node-env-default-e2e-')); + writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8'); + writeFileSync( + join(dir, 'package.json'), + JSON.stringify({ name: 'serve-node-env-default-e2e-fixture', private: true, type: 'module' }, null, 2), + 'utf8', + ); + }); + + afterAll(async () => { + for (const child of children) await stop(child); + if (dir) rmSync(dir, { recursive: true, force: true }); + }, 60_000); + + it( + 'NODE_ENV unset: the localhost trusted-origin convenience gate is CLOSED (regression pin)', + async () => { + const { status, body } = await probeOriginCheck({}); + expect(status).toBe(403); + expect(body?.code).toBe('INVALID_ORIGIN'); + }, + 180_000, + ); + + it( + 'NODE_ENV=development (explicit): the gate stays OPEN — unaffected by the production default', + async () => { + const { status, body } = await probeOriginCheck({ NODE_ENV: 'development' }); + expect(status).not.toBe(403); + expect(body?.code).not.toBe('INVALID_ORIGIN'); + }, + 180_000, + ); + + it( + 'NODE_ENV=test (explicit): the gate stays OPEN — unaffected by the production default', + async () => { + const { status, body } = await probeOriginCheck({ NODE_ENV: 'test' }); + expect(status).not.toBe(403); + expect(body?.code).not.toBe('INVALID_ORIGIN'); + }, + 180_000, + ); +}); From 2d57ffee5a207cdcbfce162254146b0125216ce8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 07:01:15 +0000 Subject: [PATCH 2/5] fix(cli): correct the NODE_ENV production-default regression pin's fixture Measured: os serve's own auto-injected AuthPlugin wiring always seeds trustedOrigins with the resolved baseUrl origin and carries its own isDev-gated localhost wildcard, so auth-manager.ts's NODE_ENV-gated substitution is unreachable through that path regardless of NODE_ENV. The pin now uses a host-authored AuthPlugin construction, which is the shape that actually reaches the gate, and reverse-verifies against the pre-fix tree. Also strips the vitest worker's own TEST env var from the spawned child: better-auth 1.7.1 reads TEST directly (independent of NODE_ENV) to decide whether to skip origin validation entirely, and leaving it inherited made the pin pass regardless of the fix. --- ...ve-node-env-production-default.e2e.test.ts | 101 +++++++++++++++--- 1 file changed, 89 insertions(+), 12 deletions(-) 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 7d65f1b2e3..7cc9467ec9 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 @@ -27,6 +27,45 @@ * default after the imports/gates below would pass that unit test and still * ship with the door open — that ordering is what only a real boot can catch. * + * ## WHY THE FIXTURE CONSTRUCTS `AuthPlugin` ITSELF, RATHER THAN LEAVING + * `os serve` TO AUTO-INJECT IT — measured, and it contradicts the naive + * reading of the regression-pin wording + * + * `os serve`'s OWN auto-injection wiring (`serve.ts`, the `!hasAuthPlugin && + * tierEnabled('auth')` block) does two things that make + * `plugin-auth/auth-manager.ts`'s `NODE_ENV`-gated substitution (line ~1927) + * UNREACHABLE through that path, regardless of `NODE_ENV`, before or after + * this fix: + * + * 1. It ALWAYS pushes the resolved `baseUrl`'s origin into `trustedOrigins` + * before handing the array to `AuthPlugin` — so `this.config.trustedOrigins` + * inside `auth-manager.ts` is NEVER empty, and that substitution is + * itself gated on `!origins.length`. + * 2. It has its OWN, separate localhost-wildcard convenience + * (`if (isDev && …) trustedOrigins.push('http://localhost:*')`), gated on + * `isDev = flags.dev || NODE_ENV === 'development'` — an EQUALITY test + * against `'development'` that was never open on unset `NODE_ENV` to + * begin with (unset `!== 'development'`, same as `'production'`). + * + * MEASURED on real boots, both ways: spawning `os serve` with NO declared + * `plugins` (letting the CLI auto-inject `AuthPlugin`) answers `403 + * INVALID_ORIGIN` to an untrusted-localhost-origin probe with UNSET + * `NODE_ENV` — identically — on the pre-fix tree AND the post-fix tree. That + * is not this fix working; it is `serve.ts`'s own, unrelated `isDev` gate, + * which was already closed. A pin built on the auto-injected path would have + * been GREEN with the fix reverted — the exact vacuity this card's own + * anti-vacuity section warns against, just one layer further down than the + * one it names. + * + * The gate this card is actually about — `auth-manager.ts`'s own + * `NODE_ENV`-gated substitution — IS reached by a host app that constructs + * `AuthPlugin` ITSELF (a supported, real shape: `serve.ts`'s `hasAuthPlugin` + * check exists precisely to detect and defer to it), without pre-populating + * `trustedOrigins`. So that is what this fixture does. Reverse-verified: on + * the pre-fix tree, this exact fixture with unset `NODE_ENV` answers `401 + * INVALID_EMAIL_OR_PASSWORD` (origin accepted, gate OPEN) to the same probe + * that gets `403 INVALID_ORIGIN` post-fix. + * * WHY THE PROBE IS AN ORIGIN CHECK, NOT A SIGN-IN. The trusted-origin * substitution is consulted by better-auth's CSRF/origin middleware BEFORE * the sign-in handler ever looks at the credentials in the body (see @@ -34,10 +73,10 @@ * with bogus credentials still tells the two states apart: gate OPEN answers * with whatever `sign-in/email` says about the (wrong) credentials, gate * CLOSED never reaches that logic and answers `403 INVALID_ORIGIN` first. - * `OS_AUTH_SECRET` is set explicitly in every boot below so the outcome is - * never confused with `serve.ts`'s separate "AuthPlugin skipped, no secret" - * warning path (`serve.ts` ~2444) — that path is orthogonal to this gate and - * unaffected by this fix (it is keyed on `isDev`, not on the raw variable). + * `OS_AUTH_SECRET` is set explicitly in every boot below and threaded into + * the fixture's own `AuthPlugin({ secret: … })` — `AuthPlugin.init()` throws + * `'AuthPlugin: secret is required'` synchronously otherwise, which is a + * boot failure, not a signal about this gate. * * The probed Origin is `http://localhost:` — * different from the port `serve` itself binds to — so a pass can only be @@ -48,7 +87,6 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; -import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -56,7 +94,21 @@ 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'); -const CONFIG = ` +/** + * The fixture's parent directory sits INSIDE `packages/cli/test/`, not the + * system tmpdir: the config below does a real, static + * `import { AuthPlugin } from '@objectstack/plugin-auth'`, and that only + * resolves because `packages/cli/node_modules/@objectstack/plugin-auth` + * (a real dependency of this package) is reachable by Node's ordinary + * upward `node_modules` walk from wherever the config file lives. A fixture + * rooted in `os.tmpdir()` has no such ancestor and the import fails. + */ +const FIXTURES_ROOT = HERE; + +function configFor(port: number): string { + return ` +import { AuthPlugin } from '@objectstack/plugin-auth'; + export default { manifest: { id: 'com.example.nodeenvdefault', @@ -71,8 +123,17 @@ export default { sharingModel: 'public', fields: { title: { type: 'text', label: 'Title' } }, }], + // Constructed here, by the HOST — not left to os serve's own auto-inject. + // See the file header for why that distinction is load-bearing for this pin. + plugins: [ + new AuthPlugin({ + secret: process.env.OS_AUTH_SECRET, + baseUrl: 'http://localhost:${port}', + }), + ], }; `; +} let dir: string; const children: ChildProcessWithoutNullStreams[] = []; @@ -100,6 +161,7 @@ interface OriginCheckResult { async function probeOriginCheck(env: Record): Promise { const port = randomPort(); const untrustedOriginPort = port + 1; + writeFileSync(join(dir, 'objectstack.config.ts'), configFor(port), 'utf8'); const child = spawn(process.execPath, [CLI, 'serve', '-p', String(port)], { cwd: dir, @@ -107,18 +169,33 @@ async function probeOriginCheck(env: Record): Promis env: { ...process.env, NO_COLOR: '1', - OS_LOG_LEVEL: '', + OS_LOG_LEVEL: 'warn', OS_DISABLE_CONSOLE: '1', OS_DATABASE_URL: ':memory:', - // Explicit and real, so the boot never takes the orthogonal "AuthPlugin - // skipped — no OS_AUTH_SECRET" path (serve.ts) regardless of which - // NODE_ENV state this call is probing. + // Explicit and real, threaded into the fixture's own `new AuthPlugin({ + // secret: … })` — so the boot never takes the orthogonal + // "AuthPlugin.init() throws: secret is required" path regardless of + // which NODE_ENV state this call is probing. OS_AUTH_SECRET: 'e2e-node-env-default-probe-secret-not-for-real-use', // The base default for every call: truly unset, unless overridden by // `env` below. Node's spawn omits an `undefined`-valued entry rather // than inheriting whatever this test RUNNER's own process (vitest sets // NODE_ENV=test) happened to have. NODE_ENV: undefined, + // MEASURED TRAP, worth stating explicitly: `...process.env` above is + // THIS FILE's own process env — the vitest WORKER's — and vitest's + // worker carries `TEST=true` (and `VITEST=true`) regardless of + // `NODE_ENV`. better-auth 1.7.1 reads `TEST` directly, independent of + // `NODE_ENV`: `create-context.mjs` defaults + // `skipOriginCheck: … isTest() ? true : false`, and + // `isTest = () => nodeENV === 'test' || toBoolean(env.TEST)`. Left + // alone, that inherited `TEST=true` makes better-auth skip origin + // validation ENTIRELY — a false GREEN that has nothing to do with + // `serve.ts`'s own gate and stays green with the fix reverted, which is + // exactly the vacuity this card's anti-vacuity section warns against, + // one layer further down than the one it names. Unset it the same way + // `NODE_ENV` is unset above, for the same reason. + TEST: undefined, ...env, }, }) as ChildProcessWithoutNullStreams; @@ -183,13 +260,13 @@ async function stop(child: ChildProcessWithoutNullStreams): Promise { describe('#11113: os serve defaults NODE_ENV to production when unset', () => { beforeAll(() => { - dir = mkdtempSync(join(tmpdir(), 'serve-node-env-default-e2e-')); - writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8'); + dir = mkdtempSync(join(FIXTURES_ROOT, 'tmp-node-env-default-')); writeFileSync( join(dir, 'package.json'), JSON.stringify({ name: 'serve-node-env-default-e2e-fixture', private: true, type: 'module' }, null, 2), 'utf8', ); + // objectstack.config.ts is written per-probe (configFor embeds the port). }); afterAll(async () => { From 61810a5a0e9754143b3952f61a144957254d3ed4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 07:19:06 +0000 Subject: [PATCH 3/5] fix(cli): correct the pin's ChildProcess type to match its own stdio config spawn(..., { stdio: ['ignore', 'pipe', 'pipe'] }) returns ChildProcessByStdio (no stdin), not ChildProcessWithoutNullStreams. TS2352 on the `as` cast was the one raw tsc error the TEST_DEBT re-measure ledger caught (146 -> 147) that the package's own `typecheck` script cannot see, since packages/cli hides its test/ tree from tsc. --- .../serve-node-env-production-default.e2e.test.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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 7cc9467ec9..6fe9c8a9ef 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 @@ -85,11 +85,15 @@ */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; +import { spawn, type ChildProcessByStdio } from 'node:child_process'; import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; +import type { Readable } from 'node:stream'; import { fileURLToPath } from 'node:url'; +/** What `spawn(..., { stdio: ['ignore', 'pipe', 'pipe'] })` actually returns — no `stdin`. */ +type ProbeChild = ChildProcessByStdio; + 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'); @@ -136,7 +140,7 @@ export default { } let dir: string; -const children: ChildProcessWithoutNullStreams[] = []; +const children: ProbeChild[] = []; /** A random high port, so a run never contends with another agent's dev server on this host. */ function randomPort(): number { @@ -198,7 +202,7 @@ async function probeOriginCheck(env: Record): Promis TEST: undefined, ...env, }, - }) as ChildProcessWithoutNullStreams; + }) as ProbeChild; children.push(child); let out = ''; @@ -241,7 +245,7 @@ async function probeOriginCheck(env: Record): Promis } } -async function stop(child: ChildProcessWithoutNullStreams): Promise { +async function stop(child: ProbeChild): Promise { if (child.exitCode !== null || child.signalCode !== null) return; await new Promise((done) => { const give = setTimeout(() => { From adb1b9ea8c6188893d4661aaf4af743d826aa6bb Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 08:19:55 +0000 Subject: [PATCH 4/5] fix(cli): address the two CI failures on #11268 (test-source-alias gate + spawn-load flake) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lint & Repo Gates: check:test-source-alias flagged @objectstack/plugin-auth as a new unaliased artifact import — the gate's dependency-free text scanner cannot tell a fixture-config string (written to a file for a spawned child process) apart from a real import in this file. Added the gate's own prescribed remedy: an anchored resolve.alias entry in packages/cli/vitest.config.ts. Verified locally: `pnpm check:test-source-alias` green. Test Core (2/6): the pin's first sub-test failed once with oclif's own "Error: command serve not found" before reaching "Server is ready" — 1824 of 1825 other tests in the same shard passed, and the ordering fix in serve.ts runs strictly after command resolution, so it cannot be the cause. This package's own vitest.config.ts header documents ~20 files (this one now among them) that spawn the built CLI as a real child process under heavy concurrent load as the suite's dominant cost (56.1% of wall time) — a shape this repo's own scripts/cli-build-prerequisite.mjs names as the canonical signature of a `dist/commands` glob read that transiently looks incomplete under load (no oclif.manifest.json cache in this repo — every invocation re-globs). Added a bounded, signature-scoped retry (bootServeWithRetry) that retries EXACTLY ONCE, and only when the failure text matches that exact oclif sentence (looksLikeMissingCliCommand, replicating cli-build-prerequisite.mjs's line-flattening) — any other failure shape (real assertion, real crash, a timeout with a different tail) still fails unretried, so this cannot mask a genuine regression. Verified locally (this commit, workspace closure rebuilt first): - `pnpm --filter @objectstack/cli build` — clean - `pnpm --filter @objectstack/cli typecheck` — clean - `pnpm --filter @objectstack/cli exec vitest run test/serve-node-env-production-default.e2e.test.ts` — 3/3 passed - `pnpm --filter @objectstack/cli exec eslint test/serve-node-env-production-default.e2e.test.ts vitest.config.ts` — clean Not yet done at push time: a dedicated local reproduction of the CI spawn-load flake (an in-progress 24-file concurrent-spawn run was interrupted, inconclusive either way — this remains a documented-but-unreproduced-locally diagnosis, not a confirmed one) and check:type-check-debt --re-measure on this exact diff. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- ...ve-node-env-production-default.e2e.test.ts | 113 ++++++++++++++++-- packages/cli/vitest.config.ts | 16 +++ 2 files changed, 118 insertions(+), 11 deletions(-) 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 6fe9c8a9ef..544666d061 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 @@ -82,6 +82,29 @@ * different from the port `serve` itself binds to — so a pass can only be * explained by the wildcard `http://localhost:*` substitution, never by * better-auth's own default trust of the deployment's own origin. + * + * ## The bounded retry — a documented CI infrastructure shape, not a mask + * + * CI measured (run 32625390225, `Test Core (2/6)`): the FIRST leg of this + * file's suite failed with oclif's own `Error: command serve not found` + * before ever reaching "Server is ready" — the other two legs, and 1824 other + * tests in the same shard, passed. That sentence is `scripts/ + * cli-build-prerequisite.mjs`'s own documented signature for a `dist/` that + * reads unbuilt-or-half-built to oclif's live `dist/commands/**` glob scan + * (no manifest cache — `packages/cli` ships no `oclif.manifest.json`, so + * EVERY invocation re-globs). `pnpm --filter @objectstack/cli typecheck` and + * a full local run of this file were both clean; `packages/cli`'s own suite + * spawns the real built CLI from ~20 other files, so a transient read of a + * just-finished, cold `tsc` build under this package's heavy concurrent-spawn + * load (measured 485s wall, 1825 tests, dozens of real child processes) is a + * documented shape of THIS suite specifically, not a property of the fix + * under test — the ordering proof above already establishes that + * `serve.ts`'s ENTIRE ELSE branch runs after the command was already found + * and `run()` already entered, so it cannot be the cause of a failure to find + * the command in the first place. `bootServeWithRetry` below retries once, + * scoped EXACTLY to that one oclif sentence — any other failure (a real + * assertion, a real crash, a real timeout with a different tail) still fails + * on the first attempt, unretried. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -162,11 +185,40 @@ interface OriginCheckResult { * child — Node's `spawn()` omits `undefined`-valued env entries rather than * stringifying them, so this is not the same as `NODE_ENV=''`. */ -async function probeOriginCheck(env: Record): Promise { - const port = randomPort(); - const untrustedOriginPort = port + 1; - writeFileSync(join(dir, 'objectstack.config.ts'), configFor(port), 'utf8'); +/** + * oclif's own "command not found" — what its live `dist/commands/**` glob + * answers with when it scans a target directory that is unbuilt OR (the shape + * measured against this file, see the retry below) TRANSIENTLY appears + * incomplete under this package's own heavy concurrent-spawn test load. + * `scripts/cli-build-prerequisite.mjs` names this exact signature as the + * shared detector several of this repo's own CI gates already carry for + * commands shelled out to the built CLI, including oclif's own line-wrapping + * of the sentence across ` › `-prefixed lines — flattened here the same way, + * not re-imported (that module lives at the repo root for GATES to share; + * this is a package test, a different resolution domain). + */ +function looksLikeMissingCliCommand(text: string): string { + const flattened = text + .split('\n') + .map((line) => line.replace(/^\s*›\s*/, '')) + .join('') + .replace(/\s+/g, ' '); + return flattened.match(/Error:\s*command\b.*?\bnot found\b/)?.[0] ?? ''; +} + +class BootFailure extends Error { + constructor(public readonly stdout: string, public readonly stderr: string) { + super(`serve did not reach "Server is ready"\n--- stdout ---\n${stdout}\n--- stderr ---\n${stderr}`); + } +} +/** + * Spawn `os serve` once and resolve when it prints "Server is ready", or + * reject with a {@link BootFailure} carrying everything it wrote. Never + * retries — that policy lives one level up, in {@link bootServeWithRetry}, + * where it can be scoped to the one failure shape it exists for. + */ +function bootServeOnce(port: number, env: Record): { child: ProbeChild; ready: Promise } { const child = spawn(process.execPath, [CLI, 'serve', '-p', String(port)], { cwd: dir, stdio: ['ignore', 'pipe', 'pipe'], @@ -207,10 +259,9 @@ async function probeOriginCheck(env: Record): Promis let out = ''; let err = ''; - - await new Promise((readyResolve, readyReject) => { + const ready = new Promise((readyResolve, readyReject) => { const timer = setTimeout(() => { - readyReject(new Error(`serve never reached "Server is ready"\n--- stdout ---\n${out}\n--- stderr ---\n${err}`)); + readyReject(new BootFailure(out, err)); }, 150_000); const onData = () => { if (/Server is ready/.test(out + err)) { @@ -220,20 +271,60 @@ async function probeOriginCheck(env: Record): Promis }; child.stdout.on('data', (d) => { out += String(d); onData(); }); child.stderr.on('data', (d) => { err += String(d); onData(); }); - child.on('exit', (code) => { + child.on('exit', () => { clearTimeout(timer); - readyReject(new Error(`serve exited ${code} before "Server is ready"\n--- stdout ---\n${out}\n--- stderr ---\n${err}`)); + readyReject(new BootFailure(out, err)); }); }); + return { child, ready }; +} + +/** + * {@link bootServeOnce}, retried EXACTLY ONCE, and only when the failure is + * oclif's own documented "command not found" — never on any other shape, + * which would mask a real regression instead of absorbing a known + * infrastructure characteristic. See {@link looksLikeMissingCliCommand}'s + * header for what that signature means and why this package's own suite is + * positioned to hit it: ~20 files in this same suite spawn the real built CLI + * (this file among them), so a transient read of a just-built `dist/commands` + * under concurrent load is a documented shape here, not a hypothesis reached + * for to explain away a failure. + */ +async function bootServeWithRetry(port: number, env: Record): Promise { + const first = bootServeOnce(port, env); + try { + await first.ready; + return first.child; + } catch (e) { + if (!(e instanceof BootFailure) || !looksLikeMissingCliCommand(e.stderr)) throw e; + await stop(first.child); + const retryPort = randomPort(); + writeFileSync(join(dir, 'objectstack.config.ts'), configFor(retryPort), 'utf8'); + const second = bootServeOnce(retryPort, env); + await second.ready; + return second.child; + } +} + +async function probeOriginCheck(env: Record): Promise { + const port = randomPort(); + writeFileSync(join(dir, 'objectstack.config.ts'), configFor(port), 'utf8'); + + const child = await bootServeWithRetry(port, env); + // `bootServeWithRetry` may have rebound to a different port on its retry + // leg — read the port the child actually reports itself as, from the last + // arg it was spawned with, so the probe below always targets the live one. + const boundPort = Number(child.spawnargs[child.spawnargs.length - 1]); + const boundUntrustedOriginPort = boundPort + 1; try { - const res = await fetch(`http://localhost:${port}/api/v1/auth/sign-in/email`, { + const res = await fetch(`http://localhost:${boundPort}/api/v1/auth/sign-in/email`, { method: 'POST', headers: { 'content-type': 'application/json', // No cookie header — this is the shape `validateFormCsrf` forces an // origin check for when neither Sec-Fetch-* nor a cookie is present. - origin: `http://localhost:${untrustedOriginPort}`, + origin: `http://localhost:${boundUntrustedOriginPort}`, }, body: JSON.stringify({ email: 'nobody@example.com', password: 'definitely-wrong-password' }), }); diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index 58a912ac49..88067a5a3a 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -150,6 +150,22 @@ export default defineConfig({ find: /^create-objectstack\/created-summary$/, replacement: path.resolve(__dirname, '../create-objectstack/src/created-summary.ts'), }, + // `test/serve-node-env-production-default.e2e.test.ts` (#11113) writes a + // FIXTURE config file whose text is `import { AuthPlugin } from + // '@objectstack/plugin-auth'` — real code, but code the fixture's own + // SPAWNED CHILD process resolves via bundle-require, never through this + // Vite config. `check-test-source-alias` is a dependency-free text + // reader (this file's own header explains why); it cannot tell that + // occurrence apart from a real import in THIS file, and flags it as an + // unaliased artifact import the same way it would a genuine one. This + // entry satisfies the gate; it is inert for the actual e2e run (the + // child's own dist/-resolving `exports` lookup is what that test + // deliberately exercises — see the file's header for why testing the + // BUILT artifact is the point there). + { + find: /^@objectstack\/plugin-auth$/, + replacement: path.resolve(__dirname, '../plugins/plugin-auth/src/index.ts'), + }, ], }, }); From 98e8399f57052e71242e32fddfd974f03ed099b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 11:03:43 +0000 Subject: [PATCH 5/5] fix(cli): declare @objectstack/cli#test's dependency on its own build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CI red on #11268 was one test file failing with oclif's own "Error: command serve not found" before serve.ts ran a line. Re-derived rather than inherited, and the mechanism is this card's own subject matter turned back on the harness. turbo.json declared "@objectstack/cli#test": { dependsOn: ["^build"] } — dependencies only, never the package's own build. Measured with `turbo run test --filter=@objectstack/cli --dry=json`: 58 tasks, 57 of them #build, and @objectstack/cli#build absent from the test task's resolved dependencies. So packages/cli/dist does not exist when the Test Core shard runs, and it never has. Five other files in this package name bin/run.js. Two only assert the path as a string. The three that spawn it pass NODE_ENV: 'development' to the child for its --dev admin seed — and that is also the value that makes @oclif/core's tsPath() rewrite the command target from the declared ./dist/commands to ./src/commands and auto-transpile (lib/util/util.js: isProd = () => !['development','test'].includes(process.env.NODE_ENV ?? '')). Those three have never touched dist/, so the undeclared build dependency stayed invisible. 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 off. So it is the only file in packages/cli that genuinely consumes dist/, and the first to depend on a prerequisite the graph did not declare. Fixed at the seam, not the symptom: dependsOn ["build"] (turbo's own build task already dependsOn ^build, so this is a superset). Same shape as "@objectstack/metadata#test" 38 lines above. Marginal cost measured at 9s on a tree whose dependency closure is already built — the shard's exact state — and ~0 on a turbo cache hit. Also removes bootServeWithRetry / looksLikeMissingCliCommand, added one commit earlier on the theory that the failure was a transient dist/commands read under concurrent spawn load. Removed because the theory is falsified by measurement, not because a rule forbids retry-wrapping: that retry was already live in the failing job 97166275854 and did not change the outcome. The failure is deterministic — with dist absent, unset and production both answer "command serve not found", test and development both resolve from src/. The file returns to its reviewed pre-retry shape. Verified in a dedicated worktree: - turbo graph after the change: 59 tasks, @objectstack/cli#test depends on @objectstack/cli#build. - End-to-end through the real mechanism: `rm -rf packages/cli/dist` then `turbo run test --filter=@objectstack/cli -- test/serve-node-env-production-default.e2e.test.ts` -> 57 tasks, 0 cached, `Test Files 1 passed (1)` / `Tests 3 passed (3)`. - Reverse-verified both legs on this exact tree, each rebuilt, the mutation confirmed on disk by anchored grep counts and in dist/ by ablation-dist-preflight.mjs: fix ablated -> `AssertionError: expected 401 to be 403`, `Tests 1 failed | 2 passed (3)`; restored -> `Tests 3 passed (3)`. - pnpm --filter @objectstack/cli typecheck / eslint — clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --- ...ve-node-env-production-default.e2e.test.ts | 149 ++++++------------ turbo.json | 2 +- 2 files changed, 50 insertions(+), 101 deletions(-) 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 544666d061..bfd3e838ae 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 @@ -83,28 +83,45 @@ * explained by the wildcard `http://localhost:*` substitution, never by * better-auth's own default trust of the deployment's own origin. * - * ## The bounded retry — a documented CI infrastructure shape, not a mask + * ## WHY THIS FILE IS THE REASON `@objectstack/cli#test` DECLARES `build` * - * CI measured (run 32625390225, `Test Core (2/6)`): the FIRST leg of this - * file's suite failed with oclif's own `Error: command serve not found` - * before ever reaching "Server is ready" — the other two legs, and 1824 other - * tests in the same shard, passed. That sentence is `scripts/ - * cli-build-prerequisite.mjs`'s own documented signature for a `dist/` that - * reads unbuilt-or-half-built to oclif's live `dist/commands/**` glob scan - * (no manifest cache — `packages/cli` ships no `oclif.manifest.json`, so - * EVERY invocation re-globs). `pnpm --filter @objectstack/cli typecheck` and - * a full local run of this file were both clean; `packages/cli`'s own suite - * spawns the real built CLI from ~20 other files, so a transient read of a - * just-finished, cold `tsc` build under this package's heavy concurrent-spawn - * load (measured 485s wall, 1825 tests, dozens of real child processes) is a - * documented shape of THIS suite specifically, not a property of the fix - * under test — the ordering proof above already establishes that - * `serve.ts`'s ENTIRE ELSE branch runs after the command was already found - * and `run()` already entered, so it cannot be the cause of a failure to find - * the command in the first place. `bootServeWithRetry` below retries once, - * scoped EXACTLY to that one oclif sentence — any other failure (a real - * assertion, a real crash, a real timeout with a different tail) still fails - * on the first attempt, unretried. + * 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. + * + * `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. + * 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. + * + * 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 + * off — so oclif globs the real `dist/commands`, and on an unbuilt tree + * answers ` › Error: command serve not found` before `serve.ts` runs a + * single line. Measured, deterministic, not load-dependent: unset and + * `production` both fail that way, `test` and `development` both resolve + * from `src/`. An earlier revision of this file wrapped the boot in a + * signature-scoped retry on the theory that the failure was a transient + * `dist/commands` read under concurrent spawn load; that retry shipped, ran + * in the failing job, and changed nothing — which is the measurement that + * removed it again. The prerequisite is declared in the build graph now, + * where it is true for whoever writes the next built-CLI test here. + * + * ⛔ Do NOT "fix" a `command serve not found` here by switching the spawn to + * `bin/run-dev.js`. That shim sets `NODE_ENV=development` unconditionally + * before argv is parsed, which makes the unset-`NODE_ENV` input this whole + * 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. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -185,40 +202,11 @@ interface OriginCheckResult { * child — Node's `spawn()` omits `undefined`-valued env entries rather than * stringifying them, so this is not the same as `NODE_ENV=''`. */ -/** - * oclif's own "command not found" — what its live `dist/commands/**` glob - * answers with when it scans a target directory that is unbuilt OR (the shape - * measured against this file, see the retry below) TRANSIENTLY appears - * incomplete under this package's own heavy concurrent-spawn test load. - * `scripts/cli-build-prerequisite.mjs` names this exact signature as the - * shared detector several of this repo's own CI gates already carry for - * commands shelled out to the built CLI, including oclif's own line-wrapping - * of the sentence across ` › `-prefixed lines — flattened here the same way, - * not re-imported (that module lives at the repo root for GATES to share; - * this is a package test, a different resolution domain). - */ -function looksLikeMissingCliCommand(text: string): string { - const flattened = text - .split('\n') - .map((line) => line.replace(/^\s*›\s*/, '')) - .join('') - .replace(/\s+/g, ' '); - return flattened.match(/Error:\s*command\b.*?\bnot found\b/)?.[0] ?? ''; -} - -class BootFailure extends Error { - constructor(public readonly stdout: string, public readonly stderr: string) { - super(`serve did not reach "Server is ready"\n--- stdout ---\n${stdout}\n--- stderr ---\n${stderr}`); - } -} +async function probeOriginCheck(env: Record): Promise { + const port = randomPort(); + const untrustedOriginPort = port + 1; + writeFileSync(join(dir, 'objectstack.config.ts'), configFor(port), 'utf8'); -/** - * Spawn `os serve` once and resolve when it prints "Server is ready", or - * reject with a {@link BootFailure} carrying everything it wrote. Never - * retries — that policy lives one level up, in {@link bootServeWithRetry}, - * where it can be scoped to the one failure shape it exists for. - */ -function bootServeOnce(port: number, env: Record): { child: ProbeChild; ready: Promise } { const child = spawn(process.execPath, [CLI, 'serve', '-p', String(port)], { cwd: dir, stdio: ['ignore', 'pipe', 'pipe'], @@ -259,9 +247,10 @@ function bootServeOnce(port: number, env: Record): { let out = ''; let err = ''; - const ready = new Promise((readyResolve, readyReject) => { + + await new Promise((readyResolve, readyReject) => { const timer = setTimeout(() => { - readyReject(new BootFailure(out, err)); + readyReject(new Error(`serve never reached "Server is ready"\n--- stdout ---\n${out}\n--- stderr ---\n${err}`)); }, 150_000); const onData = () => { if (/Server is ready/.test(out + err)) { @@ -271,60 +260,20 @@ function bootServeOnce(port: number, env: Record): { }; child.stdout.on('data', (d) => { out += String(d); onData(); }); child.stderr.on('data', (d) => { err += String(d); onData(); }); - child.on('exit', () => { + child.on('exit', (code) => { clearTimeout(timer); - readyReject(new BootFailure(out, err)); + readyReject(new Error(`serve exited ${code} before "Server is ready"\n--- stdout ---\n${out}\n--- stderr ---\n${err}`)); }); }); - return { child, ready }; -} - -/** - * {@link bootServeOnce}, retried EXACTLY ONCE, and only when the failure is - * oclif's own documented "command not found" — never on any other shape, - * which would mask a real regression instead of absorbing a known - * infrastructure characteristic. See {@link looksLikeMissingCliCommand}'s - * header for what that signature means and why this package's own suite is - * positioned to hit it: ~20 files in this same suite spawn the real built CLI - * (this file among them), so a transient read of a just-built `dist/commands` - * under concurrent load is a documented shape here, not a hypothesis reached - * for to explain away a failure. - */ -async function bootServeWithRetry(port: number, env: Record): Promise { - const first = bootServeOnce(port, env); - try { - await first.ready; - return first.child; - } catch (e) { - if (!(e instanceof BootFailure) || !looksLikeMissingCliCommand(e.stderr)) throw e; - await stop(first.child); - const retryPort = randomPort(); - writeFileSync(join(dir, 'objectstack.config.ts'), configFor(retryPort), 'utf8'); - const second = bootServeOnce(retryPort, env); - await second.ready; - return second.child; - } -} - -async function probeOriginCheck(env: Record): Promise { - const port = randomPort(); - writeFileSync(join(dir, 'objectstack.config.ts'), configFor(port), 'utf8'); - - const child = await bootServeWithRetry(port, env); - // `bootServeWithRetry` may have rebound to a different port on its retry - // leg — read the port the child actually reports itself as, from the last - // arg it was spawned with, so the probe below always targets the live one. - const boundPort = Number(child.spawnargs[child.spawnargs.length - 1]); - const boundUntrustedOriginPort = boundPort + 1; try { - const res = await fetch(`http://localhost:${boundPort}/api/v1/auth/sign-in/email`, { + const res = await fetch(`http://localhost:${port}/api/v1/auth/sign-in/email`, { method: 'POST', headers: { 'content-type': 'application/json', // No cookie header — this is the shape `validateFormCsrf` forces an // origin check for when neither Sec-Fetch-* nor a cookie is present. - origin: `http://localhost:${boundUntrustedOriginPort}`, + origin: `http://localhost:${untrustedOriginPort}`, }, body: JSON.stringify({ email: 'nobody@example.com', password: 'definitely-wrong-password' }), }); diff --git a/turbo.json b/turbo.json index 8d9ddda0e7..909fb72877 100644 --- a/turbo.json +++ b/turbo.json @@ -56,7 +56,7 @@ ] }, "@objectstack/cli#test": { - "dependsOn": ["^build"], + "dependsOn": ["build"], "outputs": [], "inputs": [ "$TURBO_DEFAULT$",