Skip to content

Commit 9888008

Browse files
fix(cli): os dev -p and os serve --port export the port as OS_PORT/PORT, so a config reads the port the server listens on (#26268)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XUzmgvQZMEYuBB7qUesPo6L)_ Fixes #26243 Clause-②: no Metadata-protocol: none ## What was wrong `os dev -p N` forwarded the port to its `serve` child as `--port` on argv, and `os serve --port N` handed it to its listener. Neither put it in the environment, so a config that reads `OS_PORT`/`PORT` while it loads saw the inherited value or the 3000 default. `examples/app-showcase`'s self URL is such a config. `os start --port` was already right, because it hands the flag to its child as `OS_PORT` + `PORT` (#12992). Measured on `examples/app-showcase` at `81bba6c3` (main), with no `OS_PORT`/`PORT`/`SHOWCASE_SELF_URL` in the environment. "ping" is `POST /api/v1/automation/showcase_declarative_connector_ping/trigger`, and "meta" is `providerConfig.baseUrl` from `GET /api/v1/meta/connector/showcase_status_api`. | boot | meta before | ping before | meta after (`af26658b`) | ping after | |---|---|---|---|---| | `os dev --fresh -p 41302` | `http://127.0.0.1:3000` | 400 `FLOW_FAILED … fetch failed` | `http://127.0.0.1:41302` | 200, `status: ok` | | `os serve --port 41303 objectstack.config.ts` (production) | `http://127.0.0.1:3000` | 400 `FLOW_FAILED … fetch failed` | `http://127.0.0.1:41303` | 200, `status: ok` | | `os start --port 41304` (control) | `http://127.0.0.1:41304` | 200 | unchanged | unchanged | `os serve --port` showed the same symptom, so it is fixed through the same source, per the ruling. ## The fix There is one writer for the port's environment, and all three doors call it. No door restates the precedence. - `packages/cli/src/utils/port-contract.ts`: `os start`'s `childPortEnv` moves here as `portFlagEnv(port)`, unchanged in behaviour. It writes `{ OS_PORT, PORT }` with one value when a flag was given, and nothing otherwise. Its docblock moves with it and now names all three callers. - `packages/cli/src/commands/dev.ts`: the port door (resolution plus refusal) moves ahead of the compile, because the compile is the first child that evaluates the config. Once a `-p` passes the door, `os dev` merges `portFlagEnv(parseRequestedPort(port))` into its own `process.env`. The compile, the watch recompiles and the `serve` child all run in that environment (the one #26240's `devCompilesAtBoot` relies on), so both copies of the stack read the flag's port. An invalid `-p` is now refused before the compile instead of after it. The argv forwarding is unchanged. - `packages/cli/src/commands/serve.ts`: once the port is validated, a `--port` given on argv (oclif's `setFromDefault` is not `true`) is merged into `process.env` the same way, before the config is loaded. A port that came from `OS_PORT`/`PORT` or the default leaves the environment as it is. - `packages/cli/src/commands/start.ts`: imports `portFlagEnv`; its two comment references are re-pointed. The behaviour is unchanged. - `examples/app-showcase/src/system/self-url.ts`: header comment only. Its claim that it reads "the same names … the CLI itself reads" is now true for the flag too, and it says how. The exported value is the parsed port, not the text: `--port 3e3` selects port 3, so it exports `3`. `os dev` keeps forwarding its flag text on argv, so the child's text-read notice still names `--port "3e3"`. Both channels come from one variable at one door. The `portFlagEnv` docblock says why that is deliberate, and why `os start`, whose integer flag has no such text, stays on the environment alone. ## Tests At `45116cbc` (the head; `af26658b` plus the changeset): - `packages/cli/test/dev-connector-env-parity.integration.test.ts` (#26240's file, reshaped): two boots of the built `bin/run.js` on the minimal host config. - `os serve objectstack.config.ts --port N`, production posture, with no `OS_PORT`. The materialized connector `baseUrl` must be `http://127.0.0.1:N`. - `os dev -p N` over an inherited stale `OS_PORT=41302` and an artifact built with `:3000`. The served and the materialized `baseUrl` must both be `:N`. This one boot holds both cards: before #26240 the served copy read `:3000`, and before this PR both copies read `:41302`. - `#26130`'s old boot was `-p 0` with `OS_PORT=41302` as the config's value. This PR makes `-p 0` win for the config too, so that shape no longer holds. Each boot now draws a non-zero port with `reservePort()`, because the config has to know the number before the boot. The draw is ledgered in `released-port-production-spawn.pin.test.ts` (2 draws, with the reason), and a lost race reads as `portDriftError`/`portContentionError`. - `start-port-forwarding-channel.pin.test.ts` now imports `portFlagEnv` from the contract module. Its five behavioural cases are unchanged. - Touched files together: `vitest run` on the integration file, `released-port-production-spawn.pin.test.ts`, `start-port-forwarding-channel.pin.test.ts` and `port-contract-single-source.test.ts` gave `Test Files 4 passed (4)`, `Tests 21 passed (21)`. - `pnpm --filter @objectstack/cli exec vitest run --project unit` gave `Test Files 334 passed (334)` and `Tests 4878 passed (4878)` (lock VERDICT command-exit 0). `pnpm --filter @objectstack/cli typecheck` exited 0. The integration tier ran locally for the touched file only; the rest is declared to CI. - Ablations, one per export, each through `scripts/ablation-replace.mjs`. In each case the CLI was rebuilt and `ablation-dist-preflight` found the marker in `packages/cli/dist/commands/*.js`. - The dev export was made a no-op: the served leg went red (`expected 'http://127.0.0.1:41302' to be 'http://127.0.0.1:33115'`) and the materialized leg stayed green. That shows why the parent has to export: the `serve` child re-exports from its own argv, but the compile only sees the parent's environment. - The serve export was made a no-op: the serve leg went red (`expected 'http://127.0.0.1:3000' to be 'http://127.0.0.1:44503'`) and both dev legs stayed green. - Both were restored (blob equals HEAD, `git status --porcelain` empty), the CLI was rebuilt, and `ablation-dist-preflight --absent` exited 0 for both markers. - A first attempt at the dev ablation did not compile (`TS2367` on the literal comparison). Nothing ran, and it is not counted. ## Gates Local subset: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `45116cbc` derived 73 families. `--ran` reconciliation: 72 ran with exit 0, 1 NOT MEASURED, 0 unrun. - NOT MEASURED, reason: prerequisite not met (exit 3). `pnpm check:dual-build-cjs-loads` reads a full-repo build that this container did not make. CI runs it. - `node scripts/check-plugin-teardown-shape.mjs --self-test` exited 3 on the shallow clone. After `git fetch origin 621a487…` (its positive control), it passed 48 cases. - `pnpm check:cli-test-child-env` passed (exit 0). The reshaped test is already in its census, and the new serve leg also spawns `bin/run.js` with `NODE_ENV` unset. - `pnpm exec eslint --no-inline-config` on the 8 touched source and test files exited 0. Neither `@objectstack/cli` nor the showcase has a `lint` script. ## Acceptance notes - **Dev auto-shift.** Measured at `af26658b` with `41310` held by another listener: `os dev --fresh -p 41310` printed `Port 41310 is in use — serving on 41311 instead`. Its `/meta` served `:41310` and the ping answered `400 FLOW_FAILED … fetch failed`. The config is evaluated before the bind, so it reads the requested port, not the shifted one. A bare `os dev` with 3000 busy has the same shape on `main`. This is the #7538 family, and this PR does not widen to it, per the ruling. It goes to the seat as a finding. - **`--port 0`** now exports `0`, so a config like the showcase's builds `:0` where it used to build `:3000` (or the inherited `OS_PORT`). Neither address serves anything. A kernel-assigned port cannot be known before the bind. - **Port text from the environment** (`OS_PORT=3e3`) is not normalized. Only a flag is exported, as `os start` already did, so an operator's own variables reach the config exactly as set. #26242 remains open; this PR does not address it. --- _Generated by [Claude Code](https://claude.ai/code/session_01LrUJpT4UGJT1bwN5uvT8dh)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 860df72 commit 9888008

9 files changed

Lines changed: 295 additions & 175 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
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
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
- **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`.
11+
- **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.
12+
- 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).

‎examples/app-showcase/src/system/self-url.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,12 @@
2323
* 2. `OS_PORT`, then its deprecated alias `PORT` — **the same names, in the
2424
* same order, that the CLI itself reads to choose the listen port**
2525
* (`packages/cli/src/commands/serve.ts`: `readEnvWithDeprecation('OS_PORT',
26-
* 'PORT') ?? '3000'`). Following the CLI's own inputs is what makes the
27-
* isolated-port boot self-ping correctly with no extra configuration — the
28-
* exact case #7538 was filed for.
26+
* 'PORT') ?? '3000'`). A `--port` flag reaches these names too: `os dev`,
27+
* `os serve` and `os start` export it as both before the config loads
28+
* (`portFlagEnv` in `packages/cli/src/utils/port-contract.ts`, #26243).
29+
* Following the CLI's own inputs is what makes the isolated-port boot
30+
* self-ping correctly with no extra configuration — the exact case #7538
31+
* was filed for.
2932
* 3. `http://127.0.0.1:3000` — the historical literal, unchanged, so a plain
3033
* `pnpm dev` behaves exactly as before.
3134
*

‎packages/cli/src/commands/dev.ts‎

Lines changed: 57 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import { OBJECTSTACK_MCP_SERVER_NAME } from '@objectstack/mcp';
3838
// command spawns (#12673). ⛔ Nothing about ports is declared in this file —
3939
// no range, no reader, no wording; a second copy of the bound is exactly what
4040
// #12620 and #12662 protected against.
41-
import { describePortSource, parseRequestedPort, formatInvalidPortNotice } from '../utils/port-contract.js';
41+
import { describePortSource, parseRequestedPort, formatInvalidPortNotice, portFlagEnv } from '../utils/port-contract.js';
4242
// The ONE dev-TLS contract, shared with the `serve` child this command spawns
4343
// (#16804). ⛔ Nothing about certificates is declared in this file, and ⛔ no
4444
// certificate is ever generated — the developer brings their own.
@@ -516,6 +516,62 @@ export default class Dev extends Command {
516516
printKV('Config', configPath, '📂');
517517
}
518518

519+
const port = flags.port ?? readEnvWithDeprecation('OS_PORT', 'PORT', { silent: true });
520+
521+
// ── dev's own door on the ONE port contract (#12673) ──────────────
522+
// Everything about the port — the range, the reader, the refusal prose —
523+
// is imported from `utils/port-contract.ts`; this command declares none
524+
// of it. That is the point of the card: `os dev` had no port validation
525+
// at all, so an impossible value travelled to the `serve` child, which
526+
// refused it under the name of the CHANNEL it arrived on. Measured on
527+
// `origin/main` before this door existed: `PORT=abc os dev` and
528+
// `OS_PORT=abc os dev` were both refused as `--port "abc"` — the one
529+
// spelling the operator had not used, because the forwarding below
530+
// renames every source to `--port`.
531+
//
532+
// ⭐ Placement: ahead of the compile below, which is the first child this
533+
// command starts and the first process that evaluates the config, so a
534+
// refused port starts nothing and a valid one is exported (below) before
535+
// anything reads it. The refusal covers exactly the text this command
536+
// FORWARDS: the `port ? …` guard in the spawn argv reads this same
537+
// variable, so the two cannot drift.
538+
//
539+
// ⛔ The `if (port)` is load-bearing, and MEASURED rather than assumed:
540+
// `os dev --port ""` boots today. An empty string is falsy, so the guard
541+
// below drops it and the child resolves its own default — so a door that
542+
// refused every non-parsing text would refuse a value that starts a
543+
// server, narrowing a published command's accept set, which this card is
544+
// forbidden to do. (`PORT=""` reaches the child by inheritance instead,
545+
// and `serve` refuses it there naming `PORT` — correctly, since that IS
546+
// the spelling the operator set.)
547+
if (port) {
548+
// `flags.port === undefined` answers the same question oclif's
549+
// `setFromDefault` answers for `serve`: did this come from argv? This
550+
// flag carries no `default`, so there is no oclif metadata to read —
551+
// and none to trust either, since oclif runs neither a flag `parse`
552+
// nor an integer `min`/`max` over a default (measured; see the
553+
// `describePortSource` docblock).
554+
const portSource = describePortSource(flags.port === undefined);
555+
if (parseRequestedPort(port) === null) {
556+
// stderr, like `serve`'s refusal and for the same #7915 reason: this
557+
// command's stdout is the fd the child's stdio MCP transport writes
558+
// JSON-RPC frames to.
559+
process.stderr.write(`${formatInvalidPortNotice(port, portSource)}\n`);
560+
process.exit(1);
561+
}
562+
// ── The flag's port is the port the config reads (#26243) ──────────
563+
// A config reads `OS_PORT`/`PORT` at module evaluation (the showcase's
564+
// self URL does), and it is evaluated in the compile below, the watch
565+
// recompiles and the `serve` child, which all run in THIS process's
566+
// environment. The flag reached none of them before: it travelled on
567+
// argv alone, so `os dev -p 41302` served and dispatched
568+
// `http://127.0.0.1:3000`. Exported here as `os start` hands it down
569+
// (`portFlagEnv`), so the flag overrides an inherited `OS_PORT` for the
570+
// config exactly as it does for the listener. With no flag, the
571+
// operator's own variables stay as they are.
572+
if (flags.port !== undefined) Object.assign(process.env, portFlagEnv(parseRequestedPort(port) ?? undefined));
573+
}
574+
519575
// Compile only when the boot's artifact is one this command builds — a
520576
// pinned artifact means "use this, don't rebuild" — and then by default
521577
// (#26130, `devCompilesAtBoot`).
@@ -765,52 +821,6 @@ export default class Dev extends Command {
765821
}
766822
printKV('Database', redactConnectionUrl(effectiveDb), '🗄️');
767823

768-
const port = flags.port ?? readEnvWithDeprecation('OS_PORT', 'PORT', { silent: true });
769-
770-
// ── dev's own door on the ONE port contract (#12673) ──────────────
771-
// Everything about the port — the range, the reader, the refusal prose —
772-
// is imported from `utils/port-contract.ts`; this command declares none
773-
// of it. That is the point of the card: `os dev` had no port validation
774-
// at all, so an impossible value travelled to the `serve` child, which
775-
// refused it under the name of the CHANNEL it arrived on. Measured on
776-
// `origin/main` before this door existed: `PORT=abc os dev` and
777-
// `OS_PORT=abc os dev` were both refused as `--port "abc"` — the one
778-
// spelling the operator had not used, because the forwarding below
779-
// renames every source to `--port`.
780-
//
781-
// ⭐ Placement is adjacent to `port` ON PURPOSE, not merely convenient.
782-
// The refusal has to cover exactly the text this command FORWARDS, and
783-
// the `port ? …` guard in the spawn argv below is what decides that. A
784-
// door hoisted to the top of `run()` would be a second copy of that
785-
// decision, free to drift from it; four lines apart, the two read one
786-
// variable. It is still ahead of every spawn, every socket and every
787-
// child process, which is all "before spawning" has ever meant here.
788-
//
789-
// ⛔ The `if (port)` is load-bearing, and MEASURED rather than assumed:
790-
// `os dev --port ""` boots today. An empty string is falsy, so the guard
791-
// below drops it and the child resolves its own default — so a door that
792-
// refused every non-parsing text would refuse a value that starts a
793-
// server, narrowing a published command's accept set, which this card is
794-
// forbidden to do. (`PORT=""` reaches the child by inheritance instead,
795-
// and `serve` refuses it there naming `PORT` — correctly, since that IS
796-
// the spelling the operator set.)
797-
if (port) {
798-
// `flags.port === undefined` answers the same question oclif's
799-
// `setFromDefault` answers for `serve`: did this come from argv? This
800-
// flag carries no `default`, so there is no oclif metadata to read —
801-
// and none to trust either, since oclif runs neither a flag `parse`
802-
// nor an integer `min`/`max` over a default (measured; see the
803-
// `describePortSource` docblock).
804-
const portSource = describePortSource(flags.port === undefined);
805-
if (parseRequestedPort(port) === null) {
806-
// stderr, like `serve`'s refusal and for the same #7915 reason: this
807-
// command's stdout is the fd the child's stdio MCP transport writes
808-
// JSON-RPC frames to.
809-
process.stderr.write(`${formatInvalidPortNotice(port, portSource)}\n`);
810-
process.exit(1);
811-
}
812-
}
813-
814824
const binPath = process.argv[1];
815825
const requestedPort = port ?? '3000';
816826

‎packages/cli/src/commands/serve.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ import {
8989
parseRequestedPort,
9090
formatInvalidPortNotice,
9191
portTextReadNotice,
92+
portFlagEnv,
9293
MAX_PORT,
9394
type PortInputSource,
9495
} from '../utils/port-contract.js';
@@ -2209,6 +2210,14 @@ export default class Serve extends Command {
22092210
this.exit(1);
22102211
}
22112212
const requestedPort = parsedPort;
2213+
// ── A `--port` on argv is the port the config reads (#26243) ──────────
2214+
// The config is loaded in this process further down, and a config may read
2215+
// `OS_PORT`/`PORT` at module evaluation (the showcase's self URL does). The
2216+
// flag reached only the listener before: `os serve --port 41303` with no
2217+
// `OS_PORT` served and dispatched `http://127.0.0.1:3000`. Exported here
2218+
// the way `os dev` and `os start` export theirs (`portFlagEnv`); a port
2219+
// that came from the environment or the default is left as it is.
2220+
if (metadata.flags.port?.setFromDefault !== true) Object.assign(process.env, portFlagEnv(requestedPort));
22122221

22132222
// ── The TLS pair, refused here for the same reason the port is (#16804) ──
22142223
// Ahead of every socket and every import that branches on the protocol, and

‎packages/cli/src/commands/start-port-forwarding-channel.pin.test.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@ import { describe, it, expect, afterEach } from 'vitest';
4747
import { readFileSync } from 'node:fs';
4848
import ts from 'typescript';
4949
import { readEnvWithDeprecation } from '@objectstack/types';
50-
import { parseRequestedPort } from '../utils/port-contract.js';
51-
import { childPortEnv } from './start.js';
50+
import { parseRequestedPort, portFlagEnv } from '../utils/port-contract.js';
5251

5352
/** The operator's own value, inherited by `start` and passed down. */
5453
const OPERATOR_OS_PORT = '41077';
@@ -68,7 +67,7 @@ afterEach(() => {
6867
* about: an inherited environment, plus whatever the port channel contributes.
6968
*/
7069
function composeChildEnv(flagPort: number | undefined, inherited: Record<string, string> = {}) {
71-
return { ...inherited, ...childPortEnv(flagPort) };
70+
return { ...inherited, ...portFlagEnv(flagPort) };
7271
}
7372

7473
/**
@@ -92,7 +91,7 @@ describe('`os start` forwards --port on the channel its child reads first', () =
9291
});
9392

9493
it('states the one value on the canonical name AND its legacy alias', () => {
95-
const env = childPortEnv(FLAG_PORT);
94+
const env = portFlagEnv(FLAG_PORT);
9695

9796
// Exact values, never containment — see the instrument note in the header.
9897
expect(env.OS_PORT).toBe(String(FLAG_PORT));
@@ -134,8 +133,8 @@ describe('`os start` forwards --port on the channel its child reads first', () =
134133
// ⭐ POSITIVE CONTROL first: the same probe, on the same helper, DOES see
135134
// the keys when a flag is given. Without this line the assertion below
136135
// would pass just as happily against a probe that can never see anything.
137-
expect(hasPortKey(childPortEnv(FLAG_PORT))).toBe(true);
138-
expect(hasPortKey(childPortEnv(undefined))).toBe(false);
136+
expect(hasPortKey(portFlagEnv(FLAG_PORT))).toBe(true);
137+
expect(hasPortKey(portFlagEnv(undefined))).toBe(false);
139138

140139
// …so an operator's own environment reaches the child untouched, under its
141140
// own names, which is what `start`'s refusal door one process earlier

0 commit comments

Comments
 (0)