[#240] Handle broker/platform/room bind failures as controlled command errors

[realproject7]

Fixes #240

Implements the reviewed #240 CLI bind-failure contract (Batch 40, code batch). Baseline fresh origin/main@1a98db6.

EPIC Alignment

• Epic: [Audit][P1] Full-codebase quality, privacy, and release-hygiene sweep #238 — full-codebase quality/privacy/release-hygiene sweep. This ticket's role: convert confirmed foreground-server bind crashes into controlled command errors.
• Merged sibling [P1][Storage] Make durable state writes atomic and stale-lock reclamation ownership-safe #239 (storage) has no overlap; [P2][CLI] Handle broker, platform, and room bind failures as command errors #240 owns only broker/platform/room listen error handling. [P2][Browser] Make room entry idempotent and stop polling after closure #241–[P2][Release] Make npm package assembly reproducible and correct stale release documentation #243 remain out of scope and untouched (diff is src/cli/commands/* + one new helper + one test).
• Contracts respected: controlled listen-error handling for broker / platform serve / room serve; loopback-default + explicit --allow-remote and SIGINT/SIGTERM lifecycle unchanged; error output token-free; never inspects/kills/mutates a foreign listener. Amendments honored: the contract covers both occupied-port and invalid-bind; the shared helper extracted from the launcher preserves Launcher UX: make bare agentgather open the local dashboard #232 behavior exactly (mechanical reuse).

What changed

• src/cli/commands/listen.ts (new, smallest shared helper):
  • listenOrError(server, port, host) — binds while guarding the one-shot 'error' event: an occupied/invalid bind resolves a failure outcome instead of hanging the listen promise or throwing an uncaught Server error. The guard is removed on success, so post-bind behavior is identical to a bare server.listen. It only observes its own listen attempt — never inspects/kills/mutates any other listener.
  • listenErrorMessage(host, port, error) — a controlled, token-free message built from the bind coordinates + OS error code only (EADDRINUSE / EACCES / EADDRNOTAVAIL / fallback).
• broker serve, platform serve, room serve — replace the unguarded await new Promise(res => server.listen(...)) with listenOrError; on failure write listenErrorMessage to stderr and return non-zero (no hang, no uncaught error). room serve returns before mutating current-room state or emitting the --allow-remote warning. Each command takes an optional injectable listen hook (defaulting to listenOrError) so the invalid-bind path is deterministically testable.
• launch (Launcher UX: make bare agentgather open the local dashboard #232) — its listen() now delegates to listenOrError(server, port, "127.0.0.1").then(o => o.ok), preserving the exact prior boolean behavior (localhost-only; bind failure → refuse with the existing token-free message, never a kill). Mechanical reuse, no functional change.

Self-Verification

• Acceptance criteria 1:1:
  • Occupied-port tests for broker, platform serve, room serve exit non-zero without an uncaught error → test/cli-listen-errors.test.ts occupies a real localhost port and runs each serve on it → real EADDRINUSE → returns 1 (awaited, so no uncaught error / no hang).
  • Deterministic injected-listen-error / invalid-bind per surface, no OS hostname resolution → each surface has an injected listen hook returning { ok:false, error:{ code:"EADDRNOTAVAIL" } } → returns 1 with a controlled token-free message; no hostname lookup involved.
  • Normal bind/shutdown remains covered → existing broker serve (SIGINT), platform serve (waitForShutdown), and room serve (SIGTERM) tests stay green.
  • Error output token-free; no foreign-listener inspect/kill/mutate → tests assert stderr has no tgl_|token=|Bearer|Authorization and that the occupying server is still listening after each failure.
  • Launcher UX: make bare agentgather open the local dashboard #232 launcher behavior has focused regression + no functional change → the shared helper has direct listenOrError/listenErrorMessage unit tests; the existing cli-launch (launch refuses an occupied/incompatible port… Launcher UX: make bare agentgather open the local dashboard #232) and browser-launch suites pass unchanged.
  • Existing CLI tests remain green → full suite 439/439.
• Kill-list scan: no token/URL in messages (only host:port + code); no new runtime dependencies (Node node:http types + internal module only); no port or remote-exposure policy change; loopback-default/--allow-remote and signal lifecycle untouched; no launch/#232 functional change; no #241–#243 files touched.
• Evidence: pnpm typecheck ✓ · pnpm lint ✓ · pnpm kit-guard ✓ (4 panes clean) · pnpm no-stub ✓ · pnpm build ✓ · node --test dist/test/**/*.js → 439 pass / 0 fail.

Deviations

• Extracted a shared helper (listen.ts) rather than inlining per command — the ticket explicitly allows "reuse or extract the smallest shared listen helper," and this is the smallest surface that lets launch/#232 delegate with zero behavior change while the three serve commands share one proven pattern.
• Injectable listen hook added to each serve command (defaulting to the real helper) solely so the invalid-bind path is exercised deterministically without OS hostname resolution — matching the amendment's requirement. broker/room gain a hooks param (default {}); the CLI dispatcher passes none, so runtime behavior is unchanged.
• No UI surface → no Design Fidelity table.

[project7-interns]

Verdict: BLOCK

Epic Alignment: PASS

The diff stays within #240: controlled bind failures for broker/platform/room plus mechanical #232 helper reuse; #241-#243 are untouched.

Checked (evidence)

• Structural gate: live REST PR body contains filled EPIC Alignment, Self-Verification, and Deviations sections.
• Bind handling: src/cli/commands/listen.ts:13-27 resolves own-listen errors; broker/index.ts:34-40, platform/index.ts:47-53, and room/index.ts:932-938 return controlled non-zero errors.
• Launcher: launch/index.ts:184-187 maps the shared outcome to the existing boolean behavior.
• Tests: test/cli-listen-errors.test.ts covers occupied-port and injected invalid-bind paths for all three surfaces, token-free stderr, and untouched foreign listeners.
• Riskiest part: helper extraction; it removes its one-shot error listener on success and preserves launcher failure-as-false semantics.
• Kill-list: scanned all items — clean.
• CI: gh pr checks 245 → Release gates pending (live; checked twice).

Findings

• [blocking] Required CI has not completed.
  • File: N/A
  • Why it fails: mandatory live checks evidence is unavailable while Release gates is pending.
  • Do instead: wait for the required check to complete successfully, then request re-review; no code change is identified.

Decision

The reviewed diff aligns with #240, but approval is blocked until live CI completes successfully.

[realproject7]

RE2 — APPROVE (code/tests) — authoritative verdict in project chat. One CI caveat below.

The #240 contract + Batch-38 amendments are fully and correctly met:

• listen.ts — listenOrError guards the one-shot 'error' (resolves a failure outcome instead of hanging / uncaught crash), removes the guard on both paths (post-bind behavior unchanged), and only observes its own server; listenErrorMessage is token-free (host:port + error.code only).
• broker / platform serve / room serve — on bind failure write the token-free message to stderr and return non-zero; roomServe returns before mutating current-room state.
• launch (Launcher UX: make bare agentgather open the local dashboard #232) — listen() delegates to listenOrError(..., "127.0.0.1") mapped to boolean; mechanically behavior-preserving.
• Boundaries/kill-list — loopback/--allow-remote + SIGINT/SIGTERM lifecycle untouched; no foreign-listener inspect/kill/mutate; no new deps; injectable listen is a defaulted test seam so cli/index.ts callers are unchanged; [P2][Browser] Make room entry idempotent and stop polling after closure #241–[P2][Release] Make npm package assembly reproducible and correct stale release documentation #243 untouched.
• Tests — per surface: real occupied-port EADDRINUSE (asserts foreign listener still listening) + deterministic injected invalid-bind (no hostname dependency), token-free; plus helper unit coverage. All 10 [P2][CLI] Handle broker, platform, and room bind failures as command errors #240 tests passed in CI.

CI caveat (merge gate, not the code): Release gates failed 1/439 — the sole failure is device-local archive/delete for joined rooms (#210), a browser-platform test outside this PR's surface (this diff touches only cli/commands/{broker,launch,listen,platform,room} + the new test). A CLI listen-error change cannot affect a browser archive/delete assertion; it's the same #210 test that flaked during #244. Please re-trigger CI — a re-run, not a code fix. Merge gate needs green CI.

[project7-interns]

Verdict: APPROVE

Epic Alignment: PASS

The diff stays within #240’s controlled bind-error scope; loopback/allow-remote policy, signal lifecycle, and #232 behavior are preserved.

Checked (evidence)

• Re-review range: no code delta; head remains 3c2c213 from the initial review.
• Bind handling: src/cli/commands/listen.ts:13-43 converts own-listen errors into bounded outcomes; broker/index.ts:34-40, platform/index.ts:47-53, and room/index.ts:932-938 emit controlled non-zero errors.
• Launcher: launch/index.ts:184-187 maps the helper outcome to the prior boolean semantics.
• Tests: test/cli-listen-errors.test.ts covers real occupied ports and deterministic injected invalid-bind errors for broker/platform/room, token-free stderr, and untouched foreign listeners.
• Riskiest part: shared helper extraction; its one-shot listener is removed on both paths and preserves launcher failure-as-false behavior.
• Kill-list: scanned all items — clean.
• CI: gh pr checks 245 → Release gates pass (live, run 29386362460/job 87261224684).

Findings

• None.

Decision

APPROVE. The #240 contract is implemented with deterministic per-surface coverage and verified green live CI.