Skip to content

Commit 583b024

Browse files
fix(cli): os dev shifts off a busy port before it compiles, so the config reads the port the server binds (#26330)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XBnXB7E8hqiCjc4qEdSqTRv)_ Fixes #26270 Clause-②: no Metadata-protocol: none ## What changed When `os dev` shifts off a busy port, the config now reads the port the server binds, through the one port channel #26243 set up (`portFlagEnv`, `OS_PORT`/`PORT`). No second copy of the precedence: the requested port is the expression `dev.ts` already had (`port ?? '3000'`), the walk is `serve.ts`'s own `getAvailablePort`, and the notice is `serve.ts`'s own wording, extracted into `formatDevPortShiftNotice`. - **`os dev` (`packages/cli/src/commands/dev.ts`).** Before the compile (the first process that evaluates the config), the parent runs the same walk from the same requested port. On a shift it prints the shift notice (stderr), exports the bound port with `portFlagEnv` (so the compile, the watch recompiles and the `serve` child read it), and forwards it as the child's `--port`, so the child binds it without walking again and a restart keeps it. An exhausted walk is left to the child, which repeats it and prints `formatExhaustedPortSearchNotice` as before. - **`os serve` (`packages/cli/src/commands/serve.ts`).** The development auto-shift branch now exports the port it shifted to with `portFlagEnv`, before `loadProjectEnvFiles`, the config load, and the direct `os serve --dev` boot compile (#26242). That is the same branch whether it was entered through `--dev` or `NODE_ENV=development`. - `utils/port-contract.ts`: `portFlagEnv`'s caller list names the shift. `examples/app-showcase/src/system/self-url.ts` is unchanged: its header claims stay true. ## Measured at the public door (`examples/app-showcase`, a TCP listener holding the requested port) | boot | `main` (after #26268) | this branch | |---|---|---| | `os dev --fresh -p 41310` | shift notice 41310 → 41311; `/api/v1/meta/connector/showcase_status_api` `baseUrl` `:41310`; `POST /api/v1/automation/showcase_declarative_connector_ping/trigger` **400 FLOW_FAILED** | (41330 → 41331, 41400 → 41401 after the main merge) `baseUrl` `:41331` / `:41401`; trigger **200**, `ping.body.data.status: "ok"` | | `OS_PORT=41320 os dev --fresh` (no flag) | 41320 → 41321; `baseUrl` `:41320`; trigger **400** | (41340 → 41341) `baseUrl` `:41341`; trigger **200** | | `os serve --dev -p 41370` | 41370 → 41371; trigger **400** | (41350 → 41351) trigger **200**; after merging #26289: (41390 → 41391) `baseUrl` `:41391`, trigger **200** | | `NODE_ENV=development os serve -p 41380` | 41380 → 41381; trigger **400** | (41360 → 41361) trigger **200** | The `main` column's failure text is `connector_action(showcase_status_api.request) failed: resilientFetch: request timed out after 30000ms` because the probe's holder accepts and never answers; with nothing answering, the card measured `fetch failed`. The ruling's scope note on `os serve` without `--dev`: it does shift, under `NODE_ENV=development` only (`portAutoShiftAllowed`), and it is the same branch through the same channel, so it is fixed by the same line. Production `os serve` still refuses a busy port, unchanged. ## The pin `packages/cli/test/dev-connector-env-parity.integration.test.ts` gains a third boot: `os dev -p N` with N held for the whole boot by an HTTP neighbour answering 503. The bound port is read from the child's own banner. Three arms: the materialized connector `baseUrl`, the served `/api/v1/meta/connector/NAME` `baseUrl`, and a flow pinging `/api/v1/health` through the connector (`fxp_ping`, the fixture's `showcase_declarative_connector_ping`) answering 200. - **Red first** (commit 1084d2e test + 774eb26 neighbour, fix sources restored from before bc2d71e, cli rebuilt): 3 failed, 3 passed. `expected 'http://127.0.0.1:37609' to be 'http://127.0.0.1:37610'` (twice) and `FLOW_FAILED … connector_action(fxp_self.ping) failed: health answered 503: expected 400 to be 200`. The earlier #26130/#26243 arms stayed green. - **Green** with the fix: 6/6, and 9/9 together with `serve-dev-connector-env-parity.integration.test.ts` after merging `main`. - The first red run hung: `holdPort()` from the shared helper never answers, so its `release()` waited on the CLOSE_WAIT socket the ping left behind (the hook timed out at 600 s). The file owns an answering neighbour instead, as `serve-port-drift-notice.e2e.test.ts` does; no draw is added (the port is held, not released, so the `released-port-production-spawn` ledger is unchanged). ## Tests and gates (head 42bd366) - `pnpm --filter @objectstack/cli exec vitest run --project unit --maxWorkers=2`: lock `VERDICT command-exit 0`, Test Files 334 passed, Tests 4884 passed, at 42bd366 (4878 before the main merge, at 4cb7cd3). - `--project integration` on the two parity files: 2 files, 9 tests passed. - `pnpm --filter @objectstack/cli typecheck` (tsc plus `check:test-typecheck`, whose project lists this test file): exit 0. - ESLint `--no-inline-config` on the four touched sources: exit 0. - `dispatch-gates --commands`: 69 families, all run, `--ran` reconciliation `69 derived … 68 run, 1 NOT-MEASURED`. Every run exited 0 except: `check:dual-build-cjs-loads` exit 3 (`Run pnpm build first`, NOT MEASURED, CI builds and runs it), and `check-plugin-teardown-shape --self-test` exit 3 on the shallow checkout, 0 after `git fetch origin 621a487` (its pinned fixture). ## Acceptance notes - **Residual window.** The parent's probe releases the port before the child binds it. If something takes it in between, the child shifts once more, says so, and exports its own port for its config, but the artifact the parent compiled (`/api/v1/meta`) keeps the probed port until the next recompile. Stated in the code and the changeset. - **`--port` text notice.** When the parent shifts, the child receives the bound port as a number, so `serve`'s `portTextReadNotice` (for text like `08080` read as 8080) does not print on that boot. It still prints whenever no shift happens. - **Test instrument.** `holdPort()` in `test/helpers/serve-process.ts` cannot be released once something has connected to it (CLOSE_WAIT, `server.close()` never calls back). Not filed: no product defect, no reach outside tests. Carrier: none. --- _Generated by [Claude Code](https://claude.ai/code/session_01NQ4sTujBVMFFGmdXcRmdd5)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent da99a4b commit 583b024

5 files changed

Lines changed: 241 additions & 18 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): when `os dev` shifts off a busy port, the config reads the port the server binds
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
- **Before.** The development auto-shift ran in the `serve` child, after `os dev`'s compile and the child's own config load had both read the requested port. On `examples/app-showcase` with 41310 held, `os dev -p 41310` printed `Port 41310 is in use — serving on 41311 instead`, yet `GET /api/v1/meta/connector/showcase_status_api` served `http://127.0.0.1:41310` and `showcase_declarative_connector_ping` answered `400 FLOW_FAILED`. The same held for a port from `OS_PORT`, and for a direct `os serve --dev` (or `os serve` under `NODE_ENV=development`), whose dispatch dialled the busy port.
11+
- **Now.** `os dev` runs the same walk before it compiles. On a shift it prints the same notice, exports the port it landed on as `OS_PORT`/`PORT` (the channel a port flag already uses) for the compile, the watch recompiles and the server, and hands the server that port. `os serve` exports its shifted port the same way before it loads the config and before a direct `os serve --dev` compiles. The same boots now serve and ping the port they bound, and the ping answers 200.
12+
- Not covered: a port taken between `os dev`'s probe and the server's bind makes the server shift once more. It says so and its own config reads the new port, but `/api/v1/meta` keeps the probed one until the next recompile.

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

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,10 @@ import {
5454
} from '../utils/dev-tls-contract.js';
5555
// The auth base-URL precedence chain, borrowed from the command that owns it
5656
// (#16734). ⛔ `dev` declares no chain of its own — see printMcpConnectHint.
57-
import { resolveAuthBaseUrl } from './serve.js';
57+
// The development auto-shift and its notice, borrowed from the command that owns
58+
// them (#26270): this process runs the same walk before it compiles, so the
59+
// config reads the port the `serve` child will bind.
60+
import { resolveAuthBaseUrl, getAvailablePort, formatDevPortShiftNotice } from './serve.js';
5861
import type { ResolvedProjectDatabaseUrl } from '@objectstack/runtime';
5962

6063
/**
@@ -475,7 +478,7 @@ export default class Dev extends Command {
475478
printKV('Config', configPath, '📂');
476479
}
477480

478-
const port = flags.port ?? readEnvWithDeprecation('OS_PORT', 'PORT', { silent: true });
481+
let port = flags.port ?? readEnvWithDeprecation('OS_PORT', 'PORT', { silent: true });
479482

480483
// ── dev's own door on the ONE port contract (#12673) ──────────────
481484
// Everything about the port — the range, the reader, the refusal prose —
@@ -531,6 +534,43 @@ export default class Dev extends Command {
531534
if (flags.port !== undefined) Object.assign(process.env, portFlagEnv(parseRequestedPort(port) ?? undefined));
532535
}
533536

537+
const requestedPort = port ?? '3000';
538+
539+
// ── The port the server will bind is the port the config reads (#26270) ──
540+
// The `serve` child shifts off a busy port, but it did so after the
541+
// compile below and its own config load had both read the requested
542+
// port. Measured on the showcase with 41310 held: `os dev -p 41310`
543+
// served on 41311 while `/api/v1/meta/connector/showcase_status_api`
544+
// named 41310 and the self-ping flow answered `400 FLOW_FAILED`.
545+
//
546+
// So this process runs the child's walk (`getAvailablePort`, from the
547+
// same requested port) ahead of the first config evaluation. On a shift
548+
// it says so in the child's words, exports the port on the one channel a
549+
// config reads (`portFlagEnv`, for the compile, the watch recompiles and
550+
// the child), and forwards it as the child's `--port`, so the child binds
551+
// it without walking again and every restart keeps it.
552+
//
553+
// ⚠️ The probe releases the port before the child binds it. A port taken
554+
// in that window makes the child shift once more: it says so, and exports
555+
// its own port for the config it loads (`serve.ts`), but the artifact
556+
// compiled here keeps this port until the next recompile. An exhausted
557+
// walk is left to the child, which repeats it and prints why.
558+
const askedFor = parseRequestedPort(requestedPort);
559+
if (askedFor !== null) {
560+
let bound = askedFor;
561+
try {
562+
bound = await getAvailablePort(askedFor);
563+
} catch {
564+
// The child's own walk reports this (`formatExhaustedPortSearchNotice`).
565+
}
566+
if (bound !== askedFor) {
567+
// stderr, for the #7915 reason the port refusal above gives.
568+
process.stderr.write(`${formatDevPortShiftNotice(askedFor, bound)}\n`);
569+
Object.assign(process.env, portFlagEnv(bound));
570+
port = String(bound);
571+
}
572+
}
573+
534574
// Compile only when the boot's artifact is one this command builds — a
535575
// pinned artifact means "use this, don't rebuild" — and then by default
536576
// (#26130, `devCompilesAtBoot`).
@@ -781,7 +821,6 @@ export default class Dev extends Command {
781821
printKV('Database', redactConnectionUrl(effectiveDb), '🗄️');
782822

783823
const binPath = process.argv[1];
784-
const requestedPort = port ?? '3000';
785824

786825
// ── Serve child under restart supervision (#5148) ───────────────
787826
// The watcher below rebuilds dist/objectstack.json, but a running

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

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,26 @@ export function formatExhaustedPortSearchNotice(requestedPort: number, cause: un
468468
);
469469
}
470470

471+
/**
472+
* The notice for a development auto-shift that succeeded (#12543): the port
473+
* asked for, that it could not be had, and the port taken instead.
474+
*
475+
* One function because two processes print it (#26270): `os serve` when its
476+
* own walk shifts, and the `os dev` parent, which runs the same walk before it
477+
* compiles so the config reads the port the server will bind. Whichever shifts
478+
* says so in these words; the other never shifts on that boot.
479+
*/
480+
export function formatDevPortShiftNotice(requestedPort: number, port: number): string {
481+
return (
482+
'\n'
483+
+ chalk.yellow(` ⚠ Port ${requestedPort} is in use — serving on ${port} instead.\n`)
484+
+ chalk.dim(` Development auto-shift: ${requestedPort} was not free, so this server took\n`)
485+
+ chalk.dim(` the next one that was (${port}). Anything still pointed at ${requestedPort} — a\n`)
486+
+ chalk.dim(' proxy, an OAuth callback URL, another terminal, a test harness — is\n')
487+
+ chalk.dim(` talking to whatever holds ${requestedPort}, not to this server.`)
488+
);
489+
}
490+
471491
/**
472492
* The port the HTTP server ACTUALLY bound, read off the transport that bound it
473493
* (#13062).
@@ -2344,14 +2364,16 @@ export default class Serve extends Command {
23442364
// line cannot be swallowed, and it survives a boot that dies later. A
23452365
// drifted port is a plausible CAUSE of such a death, so the notice has
23462366
// to outlive it.
2347-
printDiagnostic(
2348-
'\n'
2349-
+ chalk.yellow(` ⚠ Port ${requestedPort} is in use — serving on ${port} instead.\n`)
2350-
+ chalk.dim(` Development auto-shift: ${requestedPort} was not free, so this server took\n`)
2351-
+ chalk.dim(` the next one that was (${port}). Anything still pointed at ${requestedPort} — a\n`)
2352-
+ chalk.dim(' proxy, an OAuth callback URL, another terminal, a test harness — is\n')
2353-
+ chalk.dim(` talking to whatever holds ${requestedPort}, not to this server.`),
2354-
);
2367+
printDiagnostic(formatDevPortShiftNotice(requestedPort, port));
2368+
// ── …and the shifted port is the port the config reads (#26270) ──
2369+
// The config is loaded further down in this process, and it may read
2370+
// `OS_PORT`/`PORT` at module evaluation (the showcase's self URL does),
2371+
// as may the dev compile a direct `os serve --dev` runs. Without this
2372+
// they read the port that was busy: measured on the showcase,
2373+
// `/api/v1/meta/connector/showcase_status_api` named the held port and
2374+
// the self-ping flow answered `400 FLOW_FAILED`. Exported on the same
2375+
// two names a port flag is (`portFlagEnv`), so nothing restates them.
2376+
Object.assign(process.env, portFlagEnv(port));
23552377
}
23562378
} else if (!(await isPortAvailable(requestedPort))) {
23572379
// One write, for the reason spelled out at the "Nothing to serve" exit

‎packages/cli/src/utils/port-contract.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,10 @@ export function portTextReadNotice(
425425
* watch recompiles and the `serve` child all run in (#26243).
426426
* - `os serve` merges it into its own `process.env` before it loads the
427427
* config (#26243).
428+
* - A development auto-shift exports the port it shifted to the same way,
429+
* in the `os dev` parent before its compile and in `os serve` before its
430+
* config load, so the config reads the port bound rather than the busy
431+
* one it asked for (#26270).
428432
*
429433
* Measured before #26243, on `examples/app-showcase` with no `OS_PORT`/`PORT`:
430434
* `os dev -p 41302` and `os serve --port 41303` both served

0 commit comments

Comments
 (0)