Skip to content

fix(serve): prefer explicit --port over stale WS fallback (#8535)#9005

Open
nwparker wants to merge 1 commit into
mainfrom
nwparker/fix-8535-serve-port-pin
Open

fix(serve): prefer explicit --port over stale WS fallback (#8535)#9005
nwparker wants to merge 1 commit into
mainfrom
nwparker/fix-8535-serve-port-pin

Conversation

@nwparker

Copy link
Copy Markdown
Contributor

Summary

Fixes #8535: when orca serve --port <P> is set explicitly, bind <P> before any port stored in mobile-ws-fallback-port.json. Default desktop / auto port paths keep STA-1511 fallback-first binding so existing mobile pairings stay stable.

Description

WebSocketTransport.start() previously always tried candidatePorts = [persistedFallbackPort, preferredPort]. After a one-time EADDRINUSE on the preferred port, STA-1511 persistence wrote mobile-ws-fallback-port.json, and every later launch bound that stale free fallback instead of the CLI pin — stranding clients (and health checks) that dial <P>.

This change:

  • Adds preferPinnedPort to WebSocketTransport (preferred first, fallback second when set).
  • Plumbs preferPinnedWsPort from explicit serve CLI (--serve-port / orca serve --port) through OrcaRuntimeRpcServer.
  • Leaves default 6768 / dev 6769 / GUI launches on fallback-first (STA-1511).
  • Still falls through to the persisted fallback when the pin is busy, and still persists a new fallback when the resolved port diverges.

Evidence

Focused vitest:

pnpm exec vitest run src/main/runtime/rpc/ws-transport.test.ts src/main/runtime/rpc/ws-fallback-port-store.test.ts
✓ 27 passed (including new preferPinnedPort order tests)

New coverage:

  • binds preferred first when preferPinnedPort is set and both ports free
  • falls back when preferPinnedPort is set but preferred is taken
  • existing STA-1511 “fallback even when preferred free” still asserts default order

ELI5

If you say “always use door 6768”, Orca should open door 6768 when it’s free — not a sticky note from last time that said “use door 45175 instead”. Phone pairing that didn’t pin a door still trusts the sticky note first so old phones keep finding the house.

User-regression-tradeoffs

Path Behavior after this PR
orca serve --port <P> Pin first. Free pin always wins over stale fallback. If pin is busy, still try fallback then OS port. Clients dialing <P> get the pin when free.
Default desktop / no --port Unchanged: fallback-first (STA-1511). Paired mobile devices that stored the fallback endpoint stay connected across restarts.
pnpm dev (6769) Unchanged: still fallback-first.

Tradeoff (explicit pin only): after a port conflict, mobile devices that only know the old fallback may need one re-pair / reconnect if serve later successfully binds the pin again (because we no longer prefer the fallback when free). That is intentional: an explicit pin means operators and health checks own that port. New pairing QR still advertises resolvedPort. Default auto path does not take this tradeoff.

Screenshots

No visual change.

Testing

  • Focused vitest: ws-transport.test.ts, ws-fallback-port-store.test.ts (27 passed)
  • pnpm lint (pre-commit oxlint/oxfmt on touched files)
  • pnpm typecheck
  • pnpm test
  • pnpm build
  • Added/updated high-quality tests that would catch regressions

AI Review Report

Reviewed for:

  • Bind-order regression on STA-1511 (default path still fallback-first).
  • Explicit-port distinguishability from DEFAULT_WS_PORT (flag, not port equality).
  • Error fall-through: preferred non-EADDRINUSE still fatal; fallback failures still degrade.
  • Cross-platform: listen/EADDRINUSE/EACCES path is Node HTTP server (macOS/Linux/Windows); no keyboard shortcuts, path separators, or shell changes.

No residual flags after plumbing + tests.

Security Audit

No new auth, secrets, IPC, or command execution. Only TCP bind candidate order changes when an operator explicitly requests a port. Fallback persistence and E2EE/device-token auth unchanged.

Notes

  • Flag is set only when serveOptions.wsPort !== undefined (CLI passed --serve-port), not for default 6768 or dev 6769.
  • Workaround for unfixed builds remains: delete mobile-ws-fallback-port.json.

When `orca serve --port <P>` is set, bind the pinned port before any
persisted mobile-ws-fallback-port.json entry so clients dialing <P>
are not stranded. Default auto/desktop keep STA-1511 fallback-first.
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 69983d89-1510-4525-b997-d9bd5aecb057

📥 Commits

Reviewing files that changed from the base of the PR and between cf02cc4 and 6ec0c1a.

📒 Files selected for processing (4)
  • src/main/index.ts
  • src/main/runtime/rpc/ws-transport.test.ts
  • src/main/runtime/rpc/ws-transport.ts
  • src/main/runtime/runtime-rpc.ts

📝 Walkthrough

Walkthrough

Adds a preferPinnedPort option to WebSocketTransport and uses it to prioritize the configured port over a persisted fallback port. OrcaRuntimeRpcServer exposes and forwards the corresponding preference, while serve-mode initialization enables it for explicitly supplied WebSocket ports. Tests cover both preferred-port selection and fallback when the preferred port is occupied.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: explicit serve ports now win over stale WebSocket fallback ports.
Description check ✅ Passed The description matches the template well and includes summary, screenshots, testing, AI review, security, and notes sections.
Linked Issues check ✅ Passed The changes implement #8535 by preferring explicitly pinned ports, preserving fallback when pinned is busy, and keeping default paths fallback-first.
Out of Scope Changes check ✅ Passed The PR stays focused on WebSocket port selection and regression tests, with no obvious unrelated code changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nwparker

Copy link
Copy Markdown
Contributor Author

Agent handoff

Fixes: #8535

Reproduce (before / regression after merge)

Branch with kits: nwparker/bug-repro-handoffs

Full issue kit (summary + re-run + file links): see the Agent handoff — reproduction kit comment on #8535.

Writeup: https://github.com/stablyai/orca/blob/nwparker/bug-repro-handoffs/docs/bug-reproductions/8535.md

This PR

Already includes Description / Evidence / ELI5 / User-regression-tradeoffs in the body. Review that section first; re-run the issue's repro-* test after pulling this branch to confirm the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: explicitly pinned "orca serve --port <P>" is overridden by a stale persisted mobile-ws fallback port

1 participant