Skip to content

Commit 7b97895

Browse files
os-litantclaude
andauthored
feat(cli): one port contract, three doors — dev/start/serve refuse in the operator's own spelling (#12898)
Extract the port validation that #12662 landed in `serve.ts` — the range constants, the `parseInt`-based reader and the refusal prose — into `packages/cli/src/utils/port-contract.ts`, and have `dev`, `start` and `serve` each call it at their own door, before spawning anything. Before this, only `serve` validated. A value typed at `dev` or `start` reached the spawned child on a channel that renamed it, so the refusal named a spelling the operator had not used: `PORT=abc os dev` was refused as `--port "abc"`, and `os start --port 99999` as `PORT="99999"`. The range is declared in exactly one place in the repository, which is what the ruling protects: #12620 and #12662 both declined to copy it to a second entry point, and a shared single source is the front of that judgement. `start` deliberately does NOT gain `Flags.integer({ min, max })` — measured against @oclif/core 4.13.3, neither a flag `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, so a flag-layer bound would be inert on two of the three channels and would be a second copy of the range besides. The end-to-end accept set is unchanged, measured rather than argued: 18 port texts driven through all three real commands on all three channels, before and after, verdict-identical row for row. Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4a4287d commit 7b97895

8 files changed

Lines changed: 849 additions & 331 deletions

File tree

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

packages/cli/src/commands/dev.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ import {
1818
} from '../utils/dev-restart.js';
1919
import { childEnvWithResolvedArtifact } from '../utils/internal-artifact-channel.js';
2020
import { readEnvWithDeprecation, isMcpServerEnabled } from '@objectstack/types';
21+
// The ONE port contract, shared with `start` and with the `serve` child this
22+
// command spawns (#12673). ⛔ Nothing about ports is declared in this file —
23+
// no range, no reader, no wording; a second copy of the bound is exactly what
24+
// #12620 and #12662 protected against.
25+
import { describePortSource, parseRequestedPort, formatInvalidPortNotice } from '../utils/port-contract.js';
2126
import type { ResolvedProjectDatabaseUrl } from '@objectstack/runtime';
2227

2328
/**
@@ -385,6 +390,51 @@ export default class Dev extends Command {
385390
printKV('Database', redactConnectionUrl(effectiveDb), '🗄️');
386391

387392
const port = flags.port ?? readEnvWithDeprecation('OS_PORT', 'PORT', { silent: true });
393+
394+
// ── dev's own door on the ONE port contract (#12673) ──────────────
395+
// Everything about the port — the range, the reader, the refusal prose —
396+
// is imported from `utils/port-contract.ts`; this command declares none
397+
// of it. That is the point of the card: `os dev` had no port validation
398+
// at all, so an impossible value travelled to the `serve` child, which
399+
// refused it under the name of the CHANNEL it arrived on. Measured on
400+
// `origin/main` before this door existed: `PORT=abc os dev` and
401+
// `OS_PORT=abc os dev` were both refused as `--port "abc"` — the one
402+
// spelling the operator had not used, because the forwarding below
403+
// renames every source to `--port`.
404+
//
405+
// ⭐ Placement is adjacent to `port` ON PURPOSE, not merely convenient.
406+
// The refusal has to cover exactly the text this command FORWARDS, and
407+
// the `port ? …` guard in the spawn argv below is what decides that. A
408+
// door hoisted to the top of `run()` would be a second copy of that
409+
// decision, free to drift from it; four lines apart, the two read one
410+
// variable. It is still ahead of every spawn, every socket and every
411+
// child process, which is all "before spawning" has ever meant here.
412+
//
413+
// ⛔ The `if (port)` is load-bearing, and MEASURED rather than assumed:
414+
// `os dev --port ""` boots today. An empty string is falsy, so the guard
415+
// below drops it and the child resolves its own default — so a door that
416+
// refused every non-parsing text would refuse a value that starts a
417+
// server, narrowing a published command's accept set, which this card is
418+
// forbidden to do. (`PORT=""` reaches the child by inheritance instead,
419+
// and `serve` refuses it there naming `PORT` — correctly, since that IS
420+
// the spelling the operator set.)
421+
if (port) {
422+
// `flags.port === undefined` answers the same question oclif's
423+
// `setFromDefault` answers for `serve`: did this come from argv? This
424+
// flag carries no `default`, so there is no oclif metadata to read —
425+
// and none to trust either, since oclif runs neither a flag `parse`
426+
// nor an integer `min`/`max` over a default (measured; see the
427+
// `describePortSource` docblock).
428+
const portSource = describePortSource(flags.port === undefined);
429+
if (parseRequestedPort(port) === null) {
430+
// stderr, like `serve`'s refusal and for the same #7915 reason: this
431+
// command's stdout is the fd the child's stdio MCP transport writes
432+
// JSON-RPC frames to.
433+
process.stderr.write(`${formatInvalidPortNotice(port, portSource)}\n`);
434+
process.exit(1);
435+
}
436+
}
437+
388438
const binPath = process.argv[1];
389439
const requestedPort = port ?? '3000';
390440

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,11 @@ import {
6060
portTextReadNotice,
6161
formatInvalidPortNotice,
6262
type PortInputSource,
63-
} from './serve.js';
63+
// Moved out of `serve.ts` by #12673: the port contract is now ONE module that
64+
// `dev`, `start` and `serve` all import, so this suite reads it from its home
65+
// rather than through the command that used to declare it. The assertions are
66+
// unchanged — only the path is.
67+
} from '../utils/port-contract.js';
6468

6569
/** Seeded from `import.meta.url`, the spelling `check:cross-package-test-inputs` recognises. */
6670
const HERE = resolve(fileURLToPath(import.meta.url), '..');

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ import {
6363
describePortSource,
6464
formatInvalidPortNotice,
6565
type PortInputSource,
66-
} from './serve.js';
66+
// Moved out of `serve.ts` by #12673: the port contract is now ONE module that
67+
// `dev`, `start` and `serve` all import, so this suite reads it from its home
68+
// rather than through the command that used to declare it. The assertions are
69+
// unchanged — only the path is.
70+
} from '../utils/port-contract.js';
6771

6872
/** Seeded from `import.meta.url`, the spelling `check:cross-package-test-inputs` recognises. */
6973
const HERE = resolve(fileURLToPath(import.meta.url), '..');

0 commit comments

Comments
 (0)