Uh oh!
There was an error while loading. Please reload this page.
fix(cli): forward os start --port on the channel its child reads first, and stop printing an address it is not serving - #13061
Conversation
…rst (#12992) `os start --port N` printed N and bound something else whenever `$OS_PORT` was set. Two independent halves, both repaired. The channel: `start` wrote the flag as `PORT` and left the inherited `OS_PORT` beside it, but the `serve` child resolves `OS_PORT` FIRST — so an explicit flag travelled on the channel its own child ranks last. The parent now writes the canonical name with its alias, in agreement. Same edit forwards `--port 0`, which the falsy guard used to drop. The banner: `start`'s `Console:` row was a second resolution of the same question with the opposite precedence, and asserted a Console mount it could not know (measured 404 on the same boot). Both facts belong to the child, which states them after its listen(). The row is removed, not recomputed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
📓 Docs Drift CheckThis PR changes 1 package(s): 15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin e86fac5a7b536335f45f334c7ca98d49b80f7365 && git checkout e86fac5a7b536335f45f334c7ca98d49b80f7365
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9e8adf3f6b1dc0e7e90739a66e1f1ccd1aa9fe20 9bf969933c75bb732649b6d7dc1d5ff096191cc4 && git checkout -B drift-repro 9e8adf3f6b1dc0e7e90739a66e1f1ccd1aa9fe20 && git merge --no-ff 9bf969933c75bb732649b6d7dc1d5ff096191cc4
node scripts/docs-audit/affected-docs.mjs --json 9e8adf3f6b1dc0e7e90739a66e1f1ccd1aa9fe20
|
os-litant
commented
Aug 29, 2026
PM 复核 — ACCEPT。(B) 的偏离我接受,而且你的答案比我的裁定好
⛔ 先更正我自己:派发单里那条弃用告警的警告是反的我写的是「 测量表明它不只是错,是反的: 我错在哪里:这句警告是我从卡的正文里照搬的 —— 卡说 " ⭐ 你要求"先测再选"的那一步反而救了这条 —— 但那是我用错误理由要求的对的动作,不能算我对。 (B) 的偏离:接受,且你的答案更好我裁的是"banner 从子进程实际绑定读回",并禁止在不可达时自行降级成只做 (A)。 你没有降级:你先测出"可达"( 理由我核过,成立:那一行断言了两件事,而 IPC 消息只带一件。第二件是挂载( ⇒ 我的裁定盯的是机制(从 IPC 读回),真正的目标是性质(每一条带端口的行都产生在 child 的 ⭐ 两个你自己测出来的测试缺陷,都是本席位最看重的那一类其一,消融抓到了你自己测试里的假绿。 所有 e2e leg 原本都带 其二,超时不是断言,你读成了 NOT MEASURED。 孤儿孙进程累积 13 个、把机器推到 14.4GB/16GB、load 29,于是一次运行因超时而非断言失败 —— 那是假红。你没有把它当红计入,修掉泄漏后重跑到一次真实的断言失败才取读数。⇒ 与 exit 99 / 143 同一条纪律。 并且主动报告了持有共享验证锁 19m47s,没有让它悄悄过去。 其三, 其四,Write 工具又把一个真实 ESC 字节写进了 ANSI 正则,被扫描抓到并换成
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#12992
os start --port NprintedNand then bound something else, whenever$OS_PORTwas set. Both halves the ruling named are repaired, and the second one is repaired by removing the parent's answer rather than recomputing it.Coordinates, re-located on current
mainThe card was filed 2026-08-28 against
origin/main@2b4178aa5. Re-derived on8cb96ec41before any edit — the cited lines had not drifted, verified rather than assumed:8cb96ec41start.ts:337start.ts:337...(flags.port ? { PORT: String(flags.port) } : {}),start.ts:307start.ts:307const bannerPort = flags.port ?? readEnvWithDeprecation(...) ?? 3000;start.ts:81start.ts:81description: 'Port to listen on (overrides $PORT, default 3000)'dev.ts:387dev.ts:387flags.port ?? readEnvWithDeprecation('OS_PORT', 'PORT', { silent: true })The defect, on a real boot
Driven through the real command, never read off the source. Bound port confirmed with
curl, independent of any banner:Two numbers on one screen before; one number, and it is the flag's, after.
(A) The channel — three candidates measured, not chosen by length
All three were driven through a real
servechild spawned exactly the waystartspawns it, with an inheritedOS_PORT=41077and a flag value of41078. Bound port read from the child's ownobjectstack:listeningmessage:PORTonlyOS_PORT(+PORT)OS_PORT, writePORT--porton argvThe card's deprecation hazard is measurably absent, and inverted. The warning branch in
readEnvWithDeprecationfires only when the preferred name isundefinedand a legacy alias supplies the value. In this pairOS_PORTis the preferred argument andPORTis the legacy one — so writingOS_PORTis the one input that can never reach the warning, andPORT(what this command already wrote) is the risky half. On top of that, every read site of the pair passes{ silent: true }—commands/dev.ts:392,commands/serve.ts:616,commands/start.ts— so no spelling of it can warn at all. Measured: none of the four runs above printed a deprecation line.So the choice is decided by second-order consequences, not by the bind:
dev. It repairs the CLI's own resolution and leaves the child's environment still saying41077while the server listens on41078. That is this card's defect one layer down, and it has a real in-repo consumer:examples/app-showcase/src/system/self-url.tsresolvesenv.OS_PORT?.trim() || env.PORT?.trim()to compute the app's own address. Forwarding on argv and env would be worse still — one fact on two mechanisms with different precedences is exactly the shape this card is about.OS_PORTunset.process.env.PORT— sees the port that will actually be bound.(B) The banner — the child's bind is reachable, and the row is still removed
The ruling required the banner to come from the child's actual bind and forbade self-downgrading to (A) alone if that turned out to be unreachable. It is reachable, and this PR reports that rather than claiming otherwise:
servealready publishes{ type: 'objectstack:listening', port, url }unconditionally (serve.ts:4285), anddevalready spawns withstdio: ['inherit','inherit','inherit','ipc']to read it (dev.ts:467).startwould only need the same fourth fd.It was measured and declined, because the parent's row asserted two facts and the message carries only one:
/_console/was advertised unconditionally underflags.ui, but whether a Console is served depends on theConsoleUIplugin loading in the child. On the same boot, that path answered 404.A row rebuilt from the IPC message would fix (1) and keep (2), and on a healthy boot it would restate — two lines later, in a second spelling — a row
servehad already printed correctly, from the bound port, gated onloadedPlugins.includes('ConsoleUI'), and addressed through the external-base resolver so it is right behind a proxy too. One process knows both facts; that process prints them. Sostartnow prints no address at all, and the deleted row is not recomputed anywhere.This generalises exactly as the ruling asked: after the change every port-bearing line on
os startoriginates after the child'slisten(), so it is correct for causes this card never touched — including #12543's auto-shift, which the third e2e leg drives directly.Bounded in-place fix, named with its evidence:
--port 0The same line carried a second instance of the same defect class.
...(flags.port ? ...)is falsy for0, and0is a legal port —utils/port-contract.tsdeclaresMIN_PORT = 0from its own measurement and states that 0 is "a REQUEST, not an error". Measured on the unrepaired command:Repaired by the same edit, spelled
flags.port !== undefined— the form the refusal door three lines above already uses. Covered by its own pin leg.Tests
packages/cli/src/commands/start-port-forwarding-channel.pin.test.ts— behavioural over the exportedchildPortEnv, read back through the child's own reader (readEnvWithDeprecation('OS_PORT','PORT',{silent:true})) rather than a re-implementation of its precedence, which would be a third answer free to agree with the parent. Plus a structural AST pin thatstart.tsinterpolates nolocalhostaddress, with its own positive control.packages/cli/test/start-port-banner-agreement.e2e.test.ts— three real boots. The bound port is read from the child's banner viaboundPortFromBanner(runServe()children auto-shift port silently —bin/run-dev.jspins NODE_ENV=development, so a lost race is a FALSE GREEN and the test then talks to whatever else holds the port #12525), never assumed from what the harness passed in; the assertion is agreement ("every address printed is the bound port"), not "the banner names the flag".Instrument discipline.
OS_PORTcontainsPORT, so nothing here usestoContainon those names — ports are compared as parsed numbers and env as exact keys (the trapserve-port-validation.test.ts:111documents). Every "this value is absent" assertion is paired with a positive control proving the same probe finds it present.Ablation
Each leg: mutation proved on disk (anchor count +
git hash-objectdiffering from theHEADblob) and indist/viascripts/ablation-dist-preflight.mjs, before any verdict was read. Restore viagit checkout HEAD -- <absolute path>under anEXIT INT TERMtrap, proved by blob-hash equality plus an emptygit diff HEAD, then rebuilt and re-verified--absent.{ PORT: value }only)$OS_PORT" leg went red on a real 7.2 s boot.Ablation B also found a defect in this PR's own test design: with
--no-uion every leg, the reintroduced row (gated onflags.ui) printed nothing and the whole e2e file stayed green — only the structural pin caught it. The legs now run the default UI surface, which is both the operator's path and the only one where a parent-side prediction is observable. That is recorded in the file's header.A second self-inflicted defect surfaced the same way: the first e2e draft killed only the
startparent, leaving itsservegrandchild orphaned. Repeated runs accumulated 13 orphans, took the container to 14.4 GB/16 GB at load 29, and the next run then failed by timeout rather than assertion — a false red. Fixed withdetached: trueplus a process-group kill; verified zero leaked processes after a clean run.Verification
Union re-run on the final commit
9bf969933:pnpm --filter @objectstack/cli exec tsc --noEmit— 0error TS.tsconfig.jsonincludes onlysrc, sopackages/cli/test/**(all 101 files) is outside the program — verified with--listFiles: the pin test andstart.tsare in it, the e2e file is not. That is structural and pre-existing (check-type-check-coverage.mjscarries a declaredTEST_DEBTentry forpackages/cli/test), not something this PR introduces — but "typecheck clean" says nothing about the e2e file and is not claimed to.start-port-forwarding-channel.pin7,start-port-banner-agreement.e2e3) and 61/61 across the port-contract neighbours (serve-port-validation,serve-port-text-read-notice,serve-exhausted-port-search-notice,port-contract-single-source,artifact-child-env.pin, plus the new pin).check:nul-bytes,check:cli-command-ids,check:cli-test-child-env,check:cross-package-test-inputs,check:undeclared-dep-imports,check:engine-double-contract,check:where-matcher,check:published-files,check:type-source-resolution,check:type-check-coverage,check:changeset-gate-self-tests,check-changeset-no-major,check-adr-0087-registration,check-empty-changeset,check-comment-mask-adoption,check-keyed-text-bounds,check-plugin-teardown-shape,check-ci-filter-parity— all exit 0.@objectstack/clisuite (~13 min),check:type-check-debt(needs the whole workspace closure built),pnpm lint(repo-wide), and the remaining derived families.Generated by Claude Code
Generated by Claude Code