Skip to content

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

Merged
os-litant merged 2 commits into
mainfrom
claude/issue-12673-single-source-port-contract
Aug 28, 2026
Merged

fix(cli): one port contract, three doors — dev/start/serve refuse in the operator's own spelling (#12673)#12898
os-litant merged 2 commits into
mainfrom
claude/issue-12673-single-source-port-contract

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes#12673

Maintainer ruling 2026-08-28 (option 甲, comment 5448628200), answering the
2026-08-27 instruction 「12673 不要临时打补丁,给我考虑长远合理的方案」: the port
validation #12662 landed in serve.ts becomes ONE shared module, and dev /
start / serve each refuse at their own door, in the operator's own spelling,
before spawning. The child keeps validating from the same import.

What moved

packages/cli/src/utils/port-contract.ts now owns the range constants, the
parseInt-based reader, the source discriminator and both notices. All three
commands import it. serve's own call site is byte-identical to before, so the
placement pins #12662 left behind (guard ahead of the auto-shift branch, refusal
written through printDiagnostic) still hold what they held.

Criterion 1 — exactly one copy of the range, falsified rather than asserted

The ruling is explicit that a shared single source is the FRONT of the #12620 /
#12662 judgement, not its reverse: "still exactly one copy of the range, which
is what #12620/#12662 protected"
. Measured across the working tree, tracked and
untracked, excluding dist and node_modules:

$ grep -rnE "(const|let|var)[[:space:]]+(MIN_PORT|MAX_PORT|PORT_MIN|PORT_MAX)\b" --include='*.ts' ...
./packages/cli/src/utils/port-contract.ts:75:export const MIN_PORT = 0;
./packages/cli/src/utils/port-contract.ts:76:export const MAX_PORT = 65535;
POSITIVE CONTROL — the same scan, same corpus, a constant known to exist:
$ grep -rnE "(const|let|var)[[:space:]]+PORT_SEARCH_SPAN\b" --include='*.ts' ...
./packages/cli/src/commands/serve.ts:234:const PORT_SEARCH_SPAN = 100;

The control is not a substring of either term under test, and neither term is a
substring of it — asserted mechanically in the new test, not eyeballed, because
this card touches PORT and OS_PORT where one contains the other.

The zero-assertion caught a bad grep of its own on the way: the first run used
git grep, which reads only TRACKED files, so it returned nothing about a
brand-new untracked module. The control is what exposed it.

Precision the claim needs: 65535 does appear elsewhere in the repo, and those
are DIFFERENT contracts, not copies of this one — an SMTP client port bound in
plugin-email and service-settings (floor 1, not 0, deliberately) and SQL row
byte budgets in driver-sql. The claim is about the CLI listen-port range.

New test packages/cli/src/utils/port-contract-single-source.test.ts keeps the
zero, the control and the door wiring pinned. Ablation, from the committed
state, restoring with git checkout HEAD -- under a trap and proving the
restore by blob hash plus an empty git diff HEAD:

LEG A a second MAX_PORT declaration injected into dev.ts -> 3 failed | 4 passed
LEG B dev's door deleted -> 3 failed | 4 passed
CONTROL unmutated tree -> 7 passed

Criterion 2 — the end-to-end accept set does not change, measured

A table of 18 port texts driven through all three REAL commands on all three
channels (--port, $PORT, $OS_PORT) — 162 rows — before the change and
again after, compared row by row:

rows: before=162 after=162
ACCEPT-SET VERDICT DIFFERENCES: 0
REFUSAL SPELLING CHANGES: 13
before verdict counts { ACCEPTED: 101, REFUSED: 61 }
after verdict counts { ACCEPTED: 101, REFUSED: 61 }

And 18 REAL binds (the server actually listening, port read off its own banner),
covering the coercing spellings, all identical before and after:

serve/dev "41011" -> 41011 " 41012" -> 41012 "+41013" -> 41013
"041014" -> 41014 "0x0A05F" -> 41055 "41016abc" -> 41016
start identical on every row it accepts; Flags.integer refuses the rest,
exactly as it did before
bind rows compared: 18 differences: 0

The 13 spelling changes are the repair:

invocationFROMTO
PORT=abc os devInvalid port: --port "abc"Invalid port: PORT="abc"
OS_PORT=abc os devInvalid port: --port "abc"Invalid port: OS_PORT="abc"
os start --port 99999Invalid port: PORT="99999"Invalid port: --port "99999"

os serve changed on zero rows: it already named what it could see.

One accept-set row is load-bearing and deliberately preserved: os dev --port ""
BOOTS today. The empty string is falsy, so dev forwards nothing and the child
resolves its own default. A door that refused every text the reader rejects
would refuse a value that starts a server, so dev's door shares the forwarding
guard rather than restating it.

The oclif behaviour, re-measured

Driving Parser.parse directly against this checkout's @oclif/core 4.13.3:

string flag, value from argv -> parse RAN
string flag, value from `default` -> parse did NOT run
string flag, `default` is a FUNCTION
reading process.env -> parse did NOT run (serve's shape)
string flag, value from the `env:` option -> parse RAN
integer flag {min:0,max:65535} argv 99999 -> REFUSED
integer flag {min:0,max:65535} default 99999 -> ACCEPTED, value 99999
integer flag {min:0,max:65535} `env:` 99999 -> REFUSED

So the warning holds and is wider than stated: Flags.integer's min/max is
inert over a default too, not only parse. $PORT / $OS_PORT reach the CLI
through exactly such a default, so a validator hung on the flag layer would guard
--port alone. The ruling's engineering fallback is therefore the route taken:
an explicit pre-spawn call of the shared module at each door. Note also that
oclif's own env: OPTION is a different channel from a default that reads
process.env — the first is validated, the second is not, and no command here
uses the first.

Verification

Run at the final commit cbc34b972.

  • pnpm --filter @objectstack/cli typecheck — clean. Confirmed with
    tsc --listFiles that all three edited/added files are in the program (2 test
    files plus the new module), so the green covers them rather than skipping them.
  • pnpm --filter @objectstack/cli exec vitest run over the 4 port suites plus 3
    neighbouring dev/start pins — Test Files 7 passed (7) · Tests 66 passed (66).
  • pnpm lint — the FULL repo run (eslint . --no-inline-config), exit 0 in 93s.
    No narrowing claimed.
  • Gate union, each exit code captured before any pipe: check:nul-bytes,
    check:cross-package-test-inputs, check-comment-mask-adoption,
    check-empty-changeset, check-changeset-no-major,
    check-adr-0087-registration, check:pm-half-states,
    check:objectui-changeset, check:changeset-gate-self-tests,
    check:engine-double-contract, check:where-matcher,
    check:query-options-erasure, check:test-source-alias,
    check:type-source-resolution, check:route-envelope,
    check:published-files, check:page-declaration-shape, check:slot-lookup,
    check:objectql-double-limit, check-plugin-teardown-shape,
    check-ci-filter-parity, check-affected-docs — all exit 0.
  • DECLARED NARROWING: check:type-check-coverage and
    check:type-check-debt --re-measure were NOT run locally. They need the whole
    workspace closure built and re-measure tsc per ledger entry; CI runs them on
    every PR. What that green would have covered and this run does not: whether the
    new test file lands outside every tsc program accounting for its package — the
    --listFiles check above is the same question asked of this package only.

check:nul-bytes earned its keep here: the first draft of the new test wrote an
ANSI escape as a regex literal and the editing tool materialised a real 0x1b
byte. The neighbouring suite already builds that byte with String.fromCharCode
for exactly this reason, and this file now does too.

Out of scope, found on the way

Neither is repaired in this PR, and neither is filed: the dedup channel returns
403 from this seat, so both are reported upward for the PM to file.

  1. os start --port is silently overridden by $OS_PORT. start writes the
    flag into the child's environment as PORT, but the child's flag default
    prefers $OS_PORT over $PORT and start never clears it. Measured:
    OS_PORT=41077 os start --port 41078 prints Console: http://localhost:41078/_console/
    and then binds 41077. Pre-existing, untouched by this diff, and separate from
    this card (a precedence/forwarding question, not the range or the spelling).
  2. The SMTP port bound 1-65535 is hand-written three times — the enforcement and
    its own message text in plugins/plugin-email/src/transports/smtp.ts, and the
    form bound in services/service-settings/src/manifests/mail.manifest.ts. Same
    defect class this card repairs, different family.

Generated by Claude Code

… the operator's own spelling
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.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 12 documentable anchor(s).

33 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 96732b03a1dd387f427dd65fdc9a84dc9695f50a.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 96732b03a1dd387f427dd65fdc9a84dc9695f50apackageMentionDocs.

Which tree this was computed on

This run read content/docs from f57ae266669ac7442c06cf39cf50b3efe412ed49 — the merge of head cbc34b9723377e6e95d672f845cf9d39093c3074 into base 96732b03a1dd387f427dd65fdc9a84dc9695f50a, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin f57ae266669ac7442c06cf39cf50b3efe412ed49 && git checkout f57ae266669ac7442c06cf39cf50b3efe412ed49
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 96732b03a1dd387f427dd65fdc9a84dc9695f50a cbc34b9723377e6e95d672f845cf9d39093c3074 && git checkout -B drift-repro 96732b03a1dd387f427dd65fdc9a84dc9695f50a && git merge --no-ff cbc34b9723377e6e95d672f845cf9d39093c3074
node scripts/docs-audit/affected-docs.mjs --json 96732b03a1dd387f427dd65fdc9a84dc9695f50a

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 96732b03a1dd387f427dd65fdc9a84dc9695f50a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 28, 2026
@os-litant
os-litant marked this pull request as ready for review August 28, 2026 07:29
@os-litant
os-litant added this pull request to the merge queueAug 28, 2026
Merged via the queue into main with commit 7b97895Aug 28, 2026
34 checks passed
@os-litant
os-litant deleted the claude/issue-12673-single-source-port-contract branch August 28, 2026 13:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/xlteststooling

Projects

None yet

2 participants

@os-litant@claude