fix(cli): one port contract, three doors — dev/start/serve refuse in the operator's own spelling (#12673)

.changeset/single-source-port-contract.md

@@ -0,0 +1,45 @@
+---
+'@objectstack/cli': patch
+---
+
+`os dev` / `os start` / `os serve` refuse an impossible port at their own door, from one shared contract
+
+The port range, the reader that turns operator text into a port, and the refusal
+prose now live in a single module (`packages/cli/src/utils/port-contract.ts`)
+that all three commands import. Before this, only `serve` validated: a value
+typed at `dev` or `start` travelled to the spawned `serve` child and was refused
+one process later, under the name of the CHANNEL it arrived on rather than the
+spelling the operator had used.
+
+**FROM → TO — what changes, stated precisely.**
+
+- **The end-to-end accept set does not change.** Since #12662 every value listed
+ below already ended in a refusal; what moves is WHERE the refusal happens and
+ WHAT it names. Measured before and after by driving a table of 18 port texts
+ through all three real commands on all three channels (`--port`, `$PORT`,
+ `$OS_PORT`) — 162 runs per side, every row identical in verdict. Values that
+ boot today still boot, on the same port: ` 3000`, `3000 `, `+3000`, `08080`,
+ `3e3`, `0x0BB8`, `3000.0`, `3000abc` and `0b111` are all accepted, exactly as
+ before, and `parseInt`'s tolerance is deliberately preserved — a strict-decimal
+ reader would refuse six values that start a server today.
+- **The refusal moves earlier: from the `serve` child to the parent's own door,
+ before anything is spawned and before any socket exists.**
+- **The refusal names the operator's spelling.**
+ - `PORT=abc os dev` — FROM `✗ Invalid port: --port "abc"` TO
+ `✗ Invalid port: PORT="abc"`.
+ - `OS_PORT=abc os dev` — FROM `✗ Invalid port: --port "abc"` TO
+ `✗ Invalid port: OS_PORT="abc"`.
+ - `os start --port 99999` — FROM `✗ Invalid port: PORT="99999"` TO
+ `✗ Invalid port: --port "99999"`.
+ - `os serve` is unchanged in every respect; it already named what it could see.
+- **`os dev --port ""` is unchanged: still dropped, not refused.** An empty
+ string is falsy, so `dev` forwards nothing and the child resolves its own
+ default — measured, and preserved deliberately, because refusing it would
+ narrow a published command's accept set.
+
+No new flag, no new environment variable, no new configuration key. The range is
+declared in exactly one place in the repository; `os start`'s `--port` did NOT
+gain `Flags.integer({ min, max })`, because that bound would be a second copy of
+the range and, measured against `@oclif/core` 4.13.3, neither a flag's `parse`
+nor an integer `min`/`max` runs over a value supplied by a flag's `default` —
+which is how `$PORT` and `$OS_PORT` reach the CLI.

packages/cli/src/commands/dev.ts

@@ -18,6 +18,11 @@ import {
 } from '../utils/dev-restart.js';
 import { childEnvWithResolvedArtifact } from '../utils/internal-artifact-channel.js';
 import { readEnvWithDeprecation, isMcpServerEnabled } from '@objectstack/types';
+// The ONE port contract, shared with `start` and with the `serve` child this
+// 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 type { ResolvedProjectDatabaseUrl } from '@objectstack/runtime';
 
 /**
@@ -385,6 +390,51 @@ 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';
 

packages/cli/src/commands/serve-port-text-read-notice.test.ts

@@ -60,7 +60,11 @@ import {
  portTextReadNotice,
  formatInvalidPortNotice,
  type PortInputSource,
-} from './serve.js';
+// Moved out of `serve.ts` by #12673: the port contract is now ONE module that
+// `dev`, `start` and `serve` all import, so this suite reads it from its home
+// rather than through the command that used to declare it. The assertions are
+// unchanged — only the path is.
+} from '../utils/port-contract.js';
 
 /** Seeded from `import.meta.url`, the spelling `check:cross-package-test-inputs` recognises. */
 const HERE = resolve(fileURLToPath(import.meta.url), '..');

packages/cli/src/commands/serve-port-validation.test.ts

@@ -63,7 +63,11 @@ import {
  describePortSource,
  formatInvalidPortNotice,
  type PortInputSource,
-} from './serve.js';
+// Moved out of `serve.ts` by #12673: the port contract is now ONE module that
+// `dev`, `start` and `serve` all import, so this suite reads it from its home
+// rather than through the command that used to declare it. The assertions are
+// unchanged — only the path is.
+} from '../utils/port-contract.js';
 
 /** Seeded from `import.meta.url`, the spelling `check:cross-package-test-inputs` recognises. */
 const HERE = resolve(fileURLToPath(import.meta.url), '..');