From af26658b8b5061e8cd32a3f9a83937fa2443a6b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 17:34:40 +0000 Subject: [PATCH 1/2] fix(cli): os dev -p and os serve --port export the flag's port to the config's environment `os dev -p N` and `os serve --port N` handed the flag to the listener only, so a config reading OS_PORT/PORT at module evaluation (the showcase's self URL) saw the inherited value or the 3000 default: the self-ping connector failed `fetch failed` and /meta served :3000. `os start` already exported its flag as OS_PORT/PORT; that writer moves into utils/port-contract.ts as `portFlagEnv`, and dev (into its own environment, before the compile) and serve (before it loads the config) call it too. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01LrUJpT4UGJT1bwN5uvT8dh --- examples/app-showcase/src/system/self-url.ts | 9 +- packages/cli/src/commands/dev.ts | 104 +++++++------ packages/cli/src/commands/serve.ts | 9 ++ .../start-port-forwarding-channel.pin.test.ts | 11 +- packages/cli/src/commands/start.ts | 90 +----------- packages/cli/src/utils/port-contract.ts | 92 ++++++++++++ ...v-connector-env-parity.integration.test.ts | 137 +++++++++++++----- ...released-port-production-spawn.pin.test.ts | 6 + 8 files changed, 283 insertions(+), 175 deletions(-) diff --git a/examples/app-showcase/src/system/self-url.ts b/examples/app-showcase/src/system/self-url.ts index 0e6b78185f1..cf7a3b0043f 100644 --- a/examples/app-showcase/src/system/self-url.ts +++ b/examples/app-showcase/src/system/self-url.ts @@ -23,9 +23,12 @@ * 2. `OS_PORT`, then its deprecated alias `PORT` — **the same names, in the * same order, that the CLI itself reads to choose the listen port** * (`packages/cli/src/commands/serve.ts`: `readEnvWithDeprecation('OS_PORT', - * 'PORT') ?? '3000'`). Following the CLI's own inputs is what makes the - * isolated-port boot self-ping correctly with no extra configuration — the - * exact case #7538 was filed for. + * 'PORT') ?? '3000'`). A `--port` flag reaches these names too: `os dev`, + * `os serve` and `os start` export it as both before the config loads + * (`portFlagEnv` in `packages/cli/src/utils/port-contract.ts`, #26243). + * Following the CLI's own inputs is what makes the isolated-port boot + * self-ping correctly with no extra configuration — the exact case #7538 + * was filed for. * 3. `http://127.0.0.1:3000` — the historical literal, unchanged, so a plain * `pnpm dev` behaves exactly as before. * diff --git a/packages/cli/src/commands/dev.ts b/packages/cli/src/commands/dev.ts index 7e10fb38230..6d3dd0ca09d 100644 --- a/packages/cli/src/commands/dev.ts +++ b/packages/cli/src/commands/dev.ts @@ -38,7 +38,7 @@ import { OBJECTSTACK_MCP_SERVER_NAME } from '@objectstack/mcp'; // command spawns (#12673). ⛔ Nothing about ports is declared in this file — // no range, no reader, no wording; a second copy of the bound is exactly what // #12620 and #12662 protected against. -import { describePortSource, parseRequestedPort, formatInvalidPortNotice } from '../utils/port-contract.js'; +import { describePortSource, parseRequestedPort, formatInvalidPortNotice, portFlagEnv } from '../utils/port-contract.js'; // The ONE dev-TLS contract, shared with the `serve` child this command spawns // (#16804). ⛔ Nothing about certificates is declared in this file, and ⛔ no // certificate is ever generated — the developer brings their own. @@ -516,6 +516,62 @@ export default class Dev extends Command { printKV('Config', configPath, '📂'); } + const port = flags.port ?? readEnvWithDeprecation('OS_PORT', 'PORT', { silent: true }); + + // ── dev's own door on the ONE port contract (#12673) ────────────── + // Everything about the port — the range, the reader, the refusal prose — + // is imported from `utils/port-contract.ts`; this command declares none + // of it. That is the point of the card: `os dev` had no port validation + // at all, so an impossible value travelled to the `serve` child, which + // refused it under the name of the CHANNEL it arrived on. Measured on + // `origin/main` before this door existed: `PORT=abc os dev` and + // `OS_PORT=abc os dev` were both refused as `--port "abc"` — the one + // spelling the operator had not used, because the forwarding below + // renames every source to `--port`. + // + // ⭐ Placement: ahead of the compile below, which is the first child this + // command starts and the first process that evaluates the config, so a + // refused port starts nothing and a valid one is exported (below) before + // anything reads it. The refusal covers exactly the text this command + // FORWARDS: the `port ? …` guard in the spawn argv reads this same + // variable, so the two cannot drift. + // + // ⛔ The `if (port)` is load-bearing, and MEASURED rather than assumed: + // `os dev --port ""` boots today. An empty string is falsy, so the guard + // below drops it and the child resolves its own default — so a door that + // refused every non-parsing text would refuse a value that starts a + // server, narrowing a published command's accept set, which this card is + // forbidden to do. (`PORT=""` reaches the child by inheritance instead, + // and `serve` refuses it there naming `PORT` — correctly, since that IS + // the spelling the operator set.) + if (port) { + // `flags.port === undefined` answers the same question oclif's + // `setFromDefault` answers for `serve`: did this come from argv? This + // flag carries no `default`, so there is no oclif metadata to read — + // and none to trust either, since oclif runs neither a flag `parse` + // nor an integer `min`/`max` over a default (measured; see the + // `describePortSource` docblock). + const portSource = describePortSource(flags.port === undefined); + if (parseRequestedPort(port) === null) { + // stderr, like `serve`'s refusal and for the same #7915 reason: this + // command's stdout is the fd the child's stdio MCP transport writes + // JSON-RPC frames to. + process.stderr.write(`${formatInvalidPortNotice(port, portSource)}\n`); + process.exit(1); + } + // ── The flag's port is the port the config reads (#26243) ────────── + // A config reads `OS_PORT`/`PORT` at module evaluation (the showcase's + // self URL does), and it is evaluated in the compile below, the watch + // recompiles and the `serve` child, which all run in THIS process's + // environment. The flag reached none of them before: it travelled on + // argv alone, so `os dev -p 41302` served and dispatched + // `http://127.0.0.1:3000`. Exported here as `os start` hands it down + // (`portFlagEnv`), so the flag overrides an inherited `OS_PORT` for the + // config exactly as it does for the listener. With no flag, the + // operator's own variables stay as they are. + if (flags.port !== undefined) Object.assign(process.env, portFlagEnv(parseRequestedPort(port) ?? undefined)); + } + // Compile only when the boot's artifact is one this command builds — a // pinned artifact means "use this, don't rebuild" — and then by default // (#26130, `devCompilesAtBoot`). @@ -765,52 +821,6 @@ export default class Dev extends Command { } printKV('Database', redactConnectionUrl(effectiveDb), '🗄️'); - const port = flags.port ?? readEnvWithDeprecation('OS_PORT', 'PORT', { silent: true }); - - // ── dev's own door on the ONE port contract (#12673) ────────────── - // Everything about the port — the range, the reader, the refusal prose — - // is imported from `utils/port-contract.ts`; this command declares none - // of it. That is the point of the card: `os dev` had no port validation - // at all, so an impossible value travelled to the `serve` child, which - // refused it under the name of the CHANNEL it arrived on. Measured on - // `origin/main` before this door existed: `PORT=abc os dev` and - // `OS_PORT=abc os dev` were both refused as `--port "abc"` — the one - // spelling the operator had not used, because the forwarding below - // renames every source to `--port`. - // - // ⭐ Placement is adjacent to `port` ON PURPOSE, not merely convenient. - // The refusal has to cover exactly the text this command FORWARDS, and - // the `port ? …` guard in the spawn argv below is what decides that. A - // door hoisted to the top of `run()` would be a second copy of that - // decision, free to drift from it; four lines apart, the two read one - // variable. It is still ahead of every spawn, every socket and every - // child process, which is all "before spawning" has ever meant here. - // - // ⛔ The `if (port)` is load-bearing, and MEASURED rather than assumed: - // `os dev --port ""` boots today. An empty string is falsy, so the guard - // below drops it and the child resolves its own default — so a door that - // refused every non-parsing text would refuse a value that starts a - // server, narrowing a published command's accept set, which this card is - // forbidden to do. (`PORT=""` reaches the child by inheritance instead, - // and `serve` refuses it there naming `PORT` — correctly, since that IS - // the spelling the operator set.) - if (port) { - // `flags.port === undefined` answers the same question oclif's - // `setFromDefault` answers for `serve`: did this come from argv? This - // flag carries no `default`, so there is no oclif metadata to read — - // and none to trust either, since oclif runs neither a flag `parse` - // nor an integer `min`/`max` over a default (measured; see the - // `describePortSource` docblock). - const portSource = describePortSource(flags.port === undefined); - if (parseRequestedPort(port) === null) { - // stderr, like `serve`'s refusal and for the same #7915 reason: this - // command's stdout is the fd the child's stdio MCP transport writes - // JSON-RPC frames to. - process.stderr.write(`${formatInvalidPortNotice(port, portSource)}\n`); - process.exit(1); - } - } - const binPath = process.argv[1]; const requestedPort = port ?? '3000'; diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 80458419a16..e437d84994d 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -89,6 +89,7 @@ import { parseRequestedPort, formatInvalidPortNotice, portTextReadNotice, + portFlagEnv, MAX_PORT, type PortInputSource, } from '../utils/port-contract.js'; @@ -2209,6 +2210,14 @@ export default class Serve extends Command { this.exit(1); } const requestedPort = parsedPort; + // ── A `--port` on argv is the port the config reads (#26243) ────────── + // The config is loaded in this process further down, and a config may read + // `OS_PORT`/`PORT` at module evaluation (the showcase's self URL does). The + // flag reached only the listener before: `os serve --port 41303` with no + // `OS_PORT` served and dispatched `http://127.0.0.1:3000`. Exported here + // the way `os dev` and `os start` export theirs (`portFlagEnv`); a port + // that came from the environment or the default is left as it is. + if (metadata.flags.port?.setFromDefault !== true) Object.assign(process.env, portFlagEnv(requestedPort)); // ── The TLS pair, refused here for the same reason the port is (#16804) ── // Ahead of every socket and every import that branches on the protocol, and diff --git a/packages/cli/src/commands/start-port-forwarding-channel.pin.test.ts b/packages/cli/src/commands/start-port-forwarding-channel.pin.test.ts index 943d2d46cde..f429e9b1e3a 100644 --- a/packages/cli/src/commands/start-port-forwarding-channel.pin.test.ts +++ b/packages/cli/src/commands/start-port-forwarding-channel.pin.test.ts @@ -47,8 +47,7 @@ import { describe, it, expect, afterEach } from 'vitest'; import { readFileSync } from 'node:fs'; import ts from 'typescript'; import { readEnvWithDeprecation } from '@objectstack/types'; -import { parseRequestedPort } from '../utils/port-contract.js'; -import { childPortEnv } from './start.js'; +import { parseRequestedPort, portFlagEnv } from '../utils/port-contract.js'; /** The operator's own value, inherited by `start` and passed down. */ const OPERATOR_OS_PORT = '41077'; @@ -68,7 +67,7 @@ afterEach(() => { * about: an inherited environment, plus whatever the port channel contributes. */ function composeChildEnv(flagPort: number | undefined, inherited: Record = {}) { - return { ...inherited, ...childPortEnv(flagPort) }; + return { ...inherited, ...portFlagEnv(flagPort) }; } /** @@ -92,7 +91,7 @@ describe('`os start` forwards --port on the channel its child reads first', () = }); it('states the one value on the canonical name AND its legacy alias', () => { - const env = childPortEnv(FLAG_PORT); + const env = portFlagEnv(FLAG_PORT); // Exact values, never containment — see the instrument note in the header. expect(env.OS_PORT).toBe(String(FLAG_PORT)); @@ -134,8 +133,8 @@ describe('`os start` forwards --port on the channel its child reads first', () = // ⭐ POSITIVE CONTROL first: the same probe, on the same helper, DOES see // the keys when a flag is given. Without this line the assertion below // would pass just as happily against a probe that can never see anything. - expect(hasPortKey(childPortEnv(FLAG_PORT))).toBe(true); - expect(hasPortKey(childPortEnv(undefined))).toBe(false); + expect(hasPortKey(portFlagEnv(FLAG_PORT))).toBe(true); + expect(hasPortKey(portFlagEnv(undefined))).toBe(false); // …so an operator's own environment reaches the child untouched, under its // own names, which is what `start`'s refusal door one process earlier diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 733683e27b6..6d6e1dbc16e 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -27,7 +27,7 @@ import { readEnvWithDeprecation } from '@objectstack/types'; // command spawns (#12673). ⛔ Nothing about ports is declared in this file — // no range, no reader, no wording; a second copy of the bound is exactly what // #12620 and #12662 protected against. -import { describePortSource, parseRequestedPort, formatInvalidPortNotice } from '../utils/port-contract.js'; +import { describePortSource, parseRequestedPort, formatInvalidPortNotice, portFlagEnv } from '../utils/port-contract.js'; import type { ResolvedProjectDatabaseUrl } from '@objectstack/runtime'; /** @@ -347,7 +347,7 @@ export default class Start extends Command { printKV('Environment', environmentId, '🎯'); // The port TEXT this command was given, for the refusal door below — // ⛔ never for a URL. Nothing in this parent may print a port: see - // {@link childPortEnv} for the channel, and the spawn below for why the + // `portFlagEnv` (`utils/port-contract.ts`) for the channel, and the spawn below for why the // address this command used to advertise is now the child's to state. const envPort = readEnvWithDeprecation('OS_PORT', 'PORT', { silent: true }); @@ -372,7 +372,7 @@ export default class Start extends Command { // // ⚠️ The text validated is the text FORWARDED. `Flags.integer` has already // normalised argv by this point (`--port 08080` parses to `8080`), and - // {@link childPortEnv} writes `String(flags.port)` — so `String(flags.port)` + // `portFlagEnv` (`utils/port-contract.ts`) writes `String(flags.port)` — so `String(flags.port)` // is literally what the child will read, not a reconstruction of it. The env // branch needs no such care: when no flag is given `start` writes neither // port variable, the child inherits `$PORT`/`$OS_PORT` under their own @@ -449,7 +449,7 @@ export default class Start extends Command { OS_HOME: homeDir, OS_ENVIRONMENT_ID: environmentId, OS_DATABASE_URL: databaseUrl, - ...childPortEnv(flags.port), + ...portFlagEnv(flags.port), ...(flags['database-driver'] ? { OS_DATABASE_DRIVER: flags['database-driver'] } : {}), ...(flags['database-auth-token'] ? { OS_DATABASE_AUTH_TOKEN: flags['database-auth-token'] } : {}), AUTH_SECRET: authSecret, @@ -544,88 +544,6 @@ export default class Start extends Command { } } -/** - * The port `start` hands its `serve` child, as environment (#12992). - * - * ## ⭐ The channel is the defect, not the value - * - * `start` used to write the flag as `{ PORT: String(flags.port) }` and leave an - * inherited `$OS_PORT` in place beside it. The child resolves - * `readEnvWithDeprecation('OS_PORT', 'PORT')` — `OS_PORT` FIRST — so an explicit - * `--port` was handed down on the channel its own child ranks LAST and lost to - * an environment variable the flag's help text says it overrides. Measured on a - * real boot before this function existed: - * - * ``` - * OS_PORT=41077 os start --port 41078 → curl finds the server on 41077 - * ``` - * - * The child's precedence is CORRECT and is not what changed. The parent now - * writes the canonical name, so the explicit flag arrives first in the order the - * child already reads. - * - * ## Why BOTH names, and why that is one statement rather than two channels - * - * `OS_PORT` alone would satisfy the CLI, because the CLI reads the pair through - * one reader with a declared precedence. But the child's environment is read by - * more than the CLI: app code and third-party libraries read `process.env.PORT` - * directly, and this repo has such a consumer in - * `examples/app-showcase/src/system/self-url.ts` - * (`env.OS_PORT?.trim() || env.PORT?.trim()`). Leaving a stale `PORT` behind - * would repair the bind and leave the app computing its own address from a port - * nothing is listening on — this card's defect, one layer down. So the pair is - * written together and always agrees: ONE value, on a canonical name and its own - * documented alias, not two channels that could ever disagree. - * - * ⛔ NOT forwarded as `--port` on argv, though `dev` does exactly that and it - * would also make the flag win. Argv and environment are two mechanisms with - * DIFFERENT precedences, and a `start` that stated the same port on both would - * be the shape this card is about: if they ever drifted, argv would silently win - * while the environment — the thing the app reads — lied. `dev` forwards on argv - * because it writes no port into the child's environment at all; one command, - * one channel, in both cases. - * - * ## The deprecation hazard the card flagged: MEASURED ABSENT, and inverted - * - * The card warned that writing `OS_PORT` might surface a deprecation notice the - * operator never caused. It cannot, for two independent reasons: - * - * - `OS_PORT` is the **preferred** argument of `readEnvWithDeprecation`, not - * the legacy one. The warning branch fires only when the preferred name is - * `undefined` and a LEGACY alias supplies the value, so setting the preferred - * name is the one input that can never reach it. `PORT` — what this command - * used to write, and still writes beside it — is the legacy half of the pair. - * - Every read site of this pair in the repository passes `{ silent: true }` - * (`commands/dev.ts`, `commands/serve.ts`, and the door in this file), which - * `env.ts` documents as the setting for aliases that are "accepted - * conventions rather than true legacy names — e.g. `PORT`, which PaaS - * platforms inject automatically". So no spelling of this pair can warn. - * - * Driven through a real `serve` child, all four shapes were silent: today's - * `PORT`-only write (bound the WRONG port, 41077), writing `OS_PORT`, deleting - * `OS_PORT`, and argv — the last three all bound 41078 and none printed a - * deprecation line. - * - * ## ⛔ `!== undefined`, never `flags.port ?` - * - * The falsy guard this replaces DROPPED `--port 0`, and 0 is a legal, useful - * port: `utils/port-contract.ts` declares `MIN_PORT = 0` from a measurement and - * states that 0 is "a REQUEST, not an error" — it asks the kernel for any free - * port. Measured on the unrepaired command, `OS_PORT=41077 os start --port 0` - * printed `http://localhost:0/_console/` and bound **41077**: the flag was never - * forwarded at all. The refusal door a few lines up already spells the test - * `flags.port !== undefined`; this is the same question asked the same way. - * - * @param flagPort `flags.port` — oclif has already normalised it to an integer. - * @returns the keys to merge into the child env; EMPTY when no flag was given, - * so an operator's own `$PORT`/`$OS_PORT` are inherited untouched. - */ -export function childPortEnv(flagPort: number | undefined): Record { - if (flagPort === undefined) return {}; - const value = String(flagPort); - return { OS_PORT: value, PORT: value }; -} - /** * Resolve the database URL for `objectstack start` — start's flag surface * mapped onto the ONE shared resolution (`resolveProjectDatabaseUrl`, #6469) diff --git a/packages/cli/src/utils/port-contract.ts b/packages/cli/src/utils/port-contract.ts index 6beee874007..737cbdb9192 100644 --- a/packages/cli/src/utils/port-contract.ts +++ b/packages/cli/src/utils/port-contract.ts @@ -406,3 +406,95 @@ export function portTextReadNotice( + chalk.dim(fix) ); } + +/** + * The environment a port given on argv is exported as: `OS_PORT` and `PORT`, + * one value (#12992, #26243). + * + * ## Who calls it, and why one writer + * + * A port flag has two readers: the listener, and whatever reads the port from + * the environment, which includes a config evaluated at module load + * (`examples/app-showcase/src/system/self-url.ts` builds its self URL from + * `env.OS_PORT?.trim() || env.PORT?.trim()`). Each door that takes `--port` + * hands the flag to the second reader through this function, so all three + * write the same pair the same way: + * + * - `os start` merges it into its `serve` child's environment (#12992). + * - `os dev` merges it into its own `process.env`, which the compile, the + * watch recompiles and the `serve` child all run in (#26243). + * - `os serve` merges it into its own `process.env` before it loads the + * config (#26243). + * + * Measured before #26243, on `examples/app-showcase` with no `OS_PORT`/`PORT`: + * `os dev -p 41302` and `os serve --port 41303` both served + * `/api/v1/meta/connector/showcase_status_api` with `http://127.0.0.1:3000`, and + * the self-ping flow answered `400 FLOW_FAILED … fetch failed`; `os start + * --port 41304`, the one door that already called this, served `:41304` and the + * ping answered 200. + * + * ## ⭐ The channel was the defect, not the value (#12992) + * + * `start` used to write the flag as `{ PORT: String(flags.port) }` and leave an + * inherited `$OS_PORT` in place beside it. The child resolves + * `readEnvWithDeprecation('OS_PORT', 'PORT')` — `OS_PORT` FIRST — so an explicit + * `--port` was handed down on the channel its own child ranks LAST and lost to + * an environment variable the flag's help text says it overrides. Measured on a + * real boot before this function existed: + * + * ``` + * OS_PORT=41077 os start --port 41078 → curl finds the server on 41077 + * ``` + * + * The child's precedence is CORRECT and is not what changed. The parent writes + * the canonical name, so the explicit flag arrives first in the order the child + * already reads. + * + * ## Why BOTH names, and why that is one statement rather than two channels + * + * `OS_PORT` alone would satisfy the CLI, because the CLI reads the pair through + * one reader with a declared precedence. But the environment is read by more + * than the CLI: app code and third-party libraries read `process.env.PORT` + * directly, as the showcase's self URL does. Leaving a stale `PORT` behind would + * repair the bind and leave the app computing its own address from a port + * nothing is listening on. So the pair is written together and always agrees: + * ONE value, on a canonical name and its own documented alias. + * + * ## The value is the port, not the text + * + * Callers pass {@link parseRequestedPort}'s answer (`os start`'s + * `Flags.integer` has already parsed it). `--port 3e3` selects port 3, so the + * environment says `3`, the port the listener asks for; the text `3e3` would + * give a config a URL nothing serves. ⚠️ So `os dev`, which keeps forwarding its + * flag TEXT to the `serve` child on argv, states one port on two channels. + * That is deliberate and cannot drift: both come from one variable at one + * door, the environment carries the port the argv text selects, and the argv + * text is what lets the child's {@link portTextReadNotice} tell the operator + * that `3e3` was read as 3, under the `--port` spelling they typed. `os start` + * forwards on the environment alone; its integer flag has no such text. + * + * ## The deprecation hazard: MEASURED ABSENT (#12992) + * + * - `OS_PORT` is the **preferred** argument of `readEnvWithDeprecation`, not + * the legacy one. The warning branch fires only when the preferred name is + * `undefined` and a LEGACY alias supplies the value, so setting the preferred + * name is the one input that can never reach it. + * - Every read site of this pair in the repository passes `{ silent: true }` + * (`commands/dev.ts`, `commands/serve.ts`, `commands/start.ts`). + * + * ## ⛔ `!== undefined`, never a falsy test + * + * 0 is a legal port ({@link MIN_PORT}; it asks the kernel for any free one). A + * falsy guard once DROPPED `os start --port 0`: measured, `OS_PORT=41077 os + * start --port 0` bound **41077**, the flag never forwarded at all. + * + * @param flagPort the port the flag selected; `undefined` when no flag was + * given. + * @returns the keys to merge into the environment; EMPTY when no flag was + * given, so an operator's own `$PORT`/`$OS_PORT` stay as they are. + */ +export function portFlagEnv(flagPort: number | undefined): Record { + if (flagPort === undefined) return {}; + const value = String(flagPort); + return { OS_PORT: value, PORT: value }; +} diff --git a/packages/cli/test/dev-connector-env-parity.integration.test.ts b/packages/cli/test/dev-connector-env-parity.integration.test.ts index 7642d114196..bd5cbbb62e0 100644 --- a/packages/cli/test/dev-connector-env-parity.integration.test.ts +++ b/packages/cli/test/dev-connector-env-parity.integration.test.ts @@ -31,17 +31,39 @@ * `self-url.ts` builds it, a host plugin contributing that connector's * provider (its factory records the `providerConfig` it was handed, which is * what dispatch calls), and a `dist/objectstack.json` standing for a build - * whose environment had no `OS_PORT`. One `os dev` boot with `OS_PORT` set; - * the served `baseUrl` and the materialized one must both read that port. + * whose environment had no `OS_PORT`. + * + * ## #26243 — the port flag is the port the config reads + * + * `os dev -p N` and `os serve --port N` handed the flag to the listener alone, + * so a config reading `OS_PORT` saw the inherited value or none. Measured on + * the showcase with no `OS_PORT`/`PORT`: `os dev -p 41302` and `os serve + * --port 41303` both served `/api/v1/meta/connector/showcase_status_api` with + * `http://127.0.0.1:3000`, and the self-ping flow answered `400 FLOW_FAILED … + * fetch failed`. Both doors now export the flag's port as `OS_PORT`/`PORT` + * (`portFlagEnv`) before anything evaluates the config. + * + * So the `os dev` boot here passes `-p N` over an inherited, stale `OS_PORT` + * and the build-time artifact: the served `baseUrl` and the materialized one + * must both read N, the port the boot is listening on. That one boot holds + * both cards: before #26130's fix the served copy read the artifact's `:3000`, + * and before #26243's both copies read the stale `OS_PORT`. The `os serve + * --port N` boot, in production posture (no artifact door, so the config + * module is the only copy), has no `OS_PORT` at all, and the materialized + * `baseUrl` must read N. * * ## Spawn shape * * `bin/run.js` with `NODE_ENV` unset, the shipped entry resolving commands from * `dist/` (`requireBuiltCli`), in its own process group (`dev` supervises a - * `serve` grandchild). The listen port is kernel-assigned (`-p 0`), which the - * flag gives the child; `OS_PORT` is only the value the config reads, exactly - * as an isolated-port boot hands it to a config. The boot runs in `beforeAll` - * and every `it` reads what it recorded. + * `serve` grandchild). Each boot asks for a NON-ZERO port drawn with + * `reservePort()`, because that number is the subject: the config must read + * the port the flag names, before the boot, and a kernel-assigned `-p 0` read + * back afterwards cannot be known to the config (the draw is ledgered in + * `released-port-production-spawn.pin.test.ts`). A lost race on that draw is + * reported as one (`portContentionError` / `portDriftError`), not as a + * verdict. The boots run in `beforeAll` and every `it` reads what they + * recorded. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -52,11 +74,13 @@ import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { childEnv, + E2E_AUTH_SECRET, E2E_SECRET_KEY, - KERNEL_ASSIGNED_PORT, + portContentionError, + portDriftError, probeThroughChild, - readKernelAssignedPort, requireBuiltCli, + reservePort, RUN_JS_RESOLVES_FROM_DIST, } from './helpers/serve-process.js'; import { linkSpec } from './helpers/define-stack-fixture.js'; @@ -69,10 +93,11 @@ const CLI = resolve(HERE, '../bin/run.js'); const READY = /Press Ctrl\+C to stop/; const BOOT_TIMEOUT_MS = 180_000; -/** The port the boot's environment names, other than the 3000 default. */ -const LIVE_PORT = '41302'; -const LIVE_URL = `http://127.0.0.1:${LIVE_PORT}`; +/** An `OS_PORT` the `os dev` boot inherits and its `-p` must override. */ +const STALE_PORT = '41302'; const BUILD_URL = 'http://127.0.0.1:3000'; +/** The self URL a config builds for `port`, the way `CONFIG_SOURCE` builds it. */ +const selfUrlFor = (port: number | string) => `http://127.0.0.1:${port}`; const CONNECTOR = 'fxp_self'; /** Where the fixture provider's factory records the `providerConfig` it was handed. */ const MATERIALIZED_FILE = 'materialized-provider-config.json'; @@ -133,6 +158,8 @@ export default defineStack({ `; interface Reading { + /** The non-zero port the boot asked for, and bound. */ + port?: number; served?: unknown; materialized?: unknown; error?: Error; @@ -140,7 +167,8 @@ interface Reading { const groups: ChildProcess[] = []; let dir: string; -let reading: Reading = {}; +let devReading: Reading = {}; +let serveReading: Reading = {}; interface Live { child: ChildProcess; @@ -148,11 +176,15 @@ interface Live { output: () => string; } -function bootDev(env: Record): Promise { +/** + * Boot `bin/run.js ARGS` asking for `port`, resolved once the banner tail shows + * the child listening on exactly that port. + */ +function boot(what: string, args: string[], port: number, env: Record): Promise { return new Promise((resolveBoot, rejectBoot) => { const child = spawn( process.execPath, - [CLI, 'dev', '--no-watch', '--no-restart', '-d', 'file:dev.db', '-p', KERNEL_ASSIGNED_PORT], + [CLI, ...args], { cwd: dir, // `childEnv`, never a bare `...process.env` — see its header. `NODE_ENV` @@ -168,6 +200,7 @@ function bootDev(env: Record): Promise { OS_ARTIFACT_PATH: undefined, OS_MODE: undefined, NODE_ENV: undefined, + OS_PORT: undefined, PORT: undefined, ...env, }), @@ -186,18 +219,21 @@ function bootDev(env: Record): Promise { else resolveBoot({ child, base: `http://localhost:${verdict}`, output: () => out }); }; const timer = setTimeout( - () => settle(new Error(`os dev never printed ${READY}\n--- output ---\n${out.slice(-4000)}`)), + () => settle(new Error(`${what} never printed ${READY}\n--- output ---\n${out.slice(-4000)}`)), BOOT_TIMEOUT_MS, ); const onData = (d: unknown) => { out += String(d); - // The child is the authority on the port it bound: a kernel-assigned one (#24342). - if (READY.test(out)) settle(readKernelAssignedPort(out, 'os dev')); + // A boot on another port than the drawn one lost the draw's race; say so. + if (READY.test(out)) settle(portDriftError(out, what, port) ?? port); }; child.stdout?.on('data', onData); child.stderr?.on('data', onData); child.on('exit', (code) => - settle(new Error(`os dev exited ${String(code)} before ${READY}\n--- output ---\n${out.slice(-4000)}`)), + settle( + portContentionError(out, what, port) + ?? new Error(`${what} exited ${String(code)} before ${READY}\n--- output ---\n${out.slice(-4000)}`), + ), ); }); } @@ -261,9 +297,41 @@ beforeAll(async () => { 'utf8', ); + const materializedPath = join(dir, MATERIALIZED_FILE); + /** What the fxp provider's factory was handed on this boot, or why there is nothing. */ + const materialized = (live: Live): unknown => existsSync(materializedPath) + ? (JSON.parse(readFileSync(materializedPath, 'utf8')) as { baseUrl?: unknown }).baseUrl + : `(the fxp provider never materialized ${CONNECTOR})\n--- output ---\n${live.output().slice(-3000)}`; + + // `os serve --port N`, production posture, no `OS_PORT`: the config module is + // the only copy, and materialization reads it at boot. let live: Live | undefined; try { - live = await bootDev({ OS_PORT: LIVE_PORT }); + const port = reservePort(); + live = await boot( + 'os serve --port', + ['serve', 'objectstack.config.ts', '--port', String(port)], + port, + { OS_AUTH_SECRET: E2E_AUTH_SECRET, OS_DATABASE_URL: 'file:serve.db' }, + ); + serveReading = { port, materialized: materialized(live) }; + } catch (error) { + serveReading = { error: error instanceof Error ? error : new Error(String(error)) }; + } finally { + if (live) await stopGroup(live.child); + live = undefined; + rmSync(materializedPath, { force: true }); + } + + // `os dev -p N` over a stale inherited `OS_PORT` and the build-time artifact. + try { + const port = reservePort(); + live = await boot( + 'os dev -p', + ['dev', '--no-watch', '--no-restart', '-d', 'file:dev.db', '-p', String(port)], + port, + { OS_PORT: STALE_PORT }, + ); const signIn = await http(live, 'POST', '/api/v1/auth/sign-in/email', '', { email: EMAIL, password: PASSWORD }); const token = signIn.body?.token; if (signIn.status !== 200 || typeof token !== 'string') { @@ -273,36 +341,39 @@ beforeAll(async () => { if (meta.status !== 200) { throw new Error(`GET /api/v1/meta/connector/${CONNECTOR} answered ${meta.status}: ${JSON.stringify(meta.body)}`); } - const materializedPath = join(dir, MATERIALIZED_FILE); - reading = { - served: servedBaseUrl(meta.body), - materialized: existsSync(materializedPath) - ? (JSON.parse(readFileSync(materializedPath, 'utf8')) as { baseUrl?: unknown }).baseUrl - : `(the fxp provider never materialized ${CONNECTOR})\n--- output ---\n${live.output().slice(-3000)}`, - }; + devReading = { port, served: servedBaseUrl(meta.body), materialized: materialized(live) }; } catch (error) { - reading = { error: error instanceof Error ? error : new Error(String(error)) }; + devReading = { error: error instanceof Error ? error : new Error(String(error)) }; } finally { if (live) await stopGroup(live.child); } -}, BOOT_TIMEOUT_MS + 60_000); +}, 2 * BOOT_TIMEOUT_MS + 60_000); afterAll(async () => { for (const child of groups) await stopGroup(child); try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } }, 60_000); -function recorded(): Required> { +function recorded(reading: Reading): Required> { if (reading.error) throw reading.error; return reading as Required>; } -describe('#26130 — `os dev` with OS_PORT serves the connector baseUrl dispatch uses', () => { - it('dispatch materializes the connector on the boot environment\'s port (unchanged)', () => { - expect(recorded().materialized).toBe(LIVE_URL); +describe('#26130 + #26243 — `os dev -p N` serves and dispatches the connector baseUrl on N', () => { + it('dispatch materializes the connector on the flag\'s port, not the inherited OS_PORT', () => { + const { port, materialized } = recorded(devReading); + expect(materialized).toBe(selfUrlFor(port)); }); it('/api/v1/meta/connector/NAME serves that same baseUrl, not the build-time one', () => { - expect(recorded().served).toBe(LIVE_URL); + const { port, served } = recorded(devReading); + expect(served).toBe(selfUrlFor(port)); + }); +}); + +describe('#26243 — `os serve --port N` with no OS_PORT', () => { + it('dispatch materializes the connector on the flag\'s port, not the 3000 default', () => { + const { port, materialized } = recorded(serveReading); + expect(materialized).toBe(selfUrlFor(port)); }); }); diff --git a/packages/cli/test/released-port-production-spawn.pin.test.ts b/packages/cli/test/released-port-production-spawn.pin.test.ts index f5a67e3bdfd..76bdbfb716c 100644 --- a/packages/cli/test/released-port-production-spawn.pin.test.ts +++ b/packages/cli/test/released-port-production-spawn.pin.test.ts @@ -83,6 +83,12 @@ const HARNESS_MODULE = /(?:^|\/)serve-process(?:\.[cm]?[jt]s)?$/; * closed alternative (a port read back afterwards, or one held for the whole boot). */ const RELEASED_DRAW_LEDGER: Readonly> = { + 'dev-connector-env-parity.integration.test.ts': { + draws: 2, + why: 'each boot (`os serve --port N`, `os dev -p N`) is about the config reading the port the flag ' + + 'names BEFORE the boot; a kernel-assigned port read back afterwards is unknown to the config, ' + + 'and a lost race is read out of the child\'s own drift or in-use diagnostic instead of failing on it', + }, 'serve-port-drift-notice.e2e.test.ts': { draws: 1, why: 'the NEGATIVE arm is a boot that binds exactly the non-zero port it asked for and says ' From 45116cbc104801a811f4ee0399e3fef69fe5f806 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 17:37:36 +0000 Subject: [PATCH 2/2] chore(changeset): cli patch for the port flag reaching the config's environment Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01LrUJpT4UGJT1bwN5uvT8dh --- .changeset/26243-port-flag-reaches-config.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/26243-port-flag-reaches-config.md diff --git a/.changeset/26243-port-flag-reaches-config.md b/.changeset/26243-port-flag-reaches-config.md new file mode 100644 index 00000000000..6c8f5a80988 --- /dev/null +++ b/.changeset/26243-port-flag-reaches-config.md @@ -0,0 +1,12 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): `os dev -p N` and `os serve --port N` export the port as `OS_PORT`/`PORT`, so a config that reads the port sees the one the server listens on + +Clause-②: no +Metadata-protocol: none + +- **Before.** The port flag reached the listener only. A config that builds a URL from `OS_PORT`/`PORT` while it loads, such as `examples/app-showcase`'s self URL, got the inherited value or the 3000 default. On the showcase with no `OS_PORT`/`PORT`, `os dev -p 41302` and `os serve --port 41303` both served `GET /api/v1/meta/connector/showcase_status_api` with `http://127.0.0.1:3000`, and the self-ping flow answered `400 FLOW_FAILED … fetch failed`. +- **Now.** `os dev` and `os serve` export the port the flag selected as both `OS_PORT` and `PORT` before anything loads the config, the way `os start` already handed it to its server. Under `os dev` that covers the compile, the watch recompiles and the server. The flag overrides an inherited `OS_PORT` for the config, as it already did for the listener. The same two boots now serve `:41302` / `:41303`, and the ping answers 200. +- With no port flag, the environment is left as it is. The exported value is the parsed port (`--port 3e3` exports `3`, the port it selects).