feat(network): user-configurable backend / LAN-share / UI ports - #163
Conversation
Single-source the backend port from OMNIVOICE_PORT and derive the LAN-share base from it (OMNIVOICE_SHARE_PORT override). Previously network_share.py hardcoded BACKEND_PORT=3900, so a user running the backend on a custom port got LAN-share/Tailscale pointed at the wrong port. - network_share: replace BACKEND_PORT constant with backend_port() / share_port_base() helpers (env-driven, never-throw fallback to defaults); enable() probes from share_port_base(). - tailscale: serve_enable(port=None) defaults to network_share.backend_port(). - main.py: direct-run + --health-check ports and HEALTH_URL read OMNIVOICE_PORT; CORS default origins use OMNIVOICE_UI_PORT (default 3901). - /system/info + SystemInfoResponse: expose backend_port, share_port_base, ui_port (both success and never-throw except branches). - set-env: persist OMNIVOICE_PORT/SHARE_PORT/UI_PORT; validate numeric and 1024-65535, reject otherwise with 400. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Push OMNIVOICE_PORT=backend_port() onto the spawned Python child's env so network_share.backend_port() always agrees with the uvicorn --port Rust passes. Without this, a user-set OMNIVOICE_PORT would move the LAN-share / Tailscale target while the listener stayed on the Rust-resolved port. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- vite.config.js: dev-server port reads OMNIVOICE_UI_PORT (default 3901). - SharingPanel: new Ports subsection reads /system/info and displays backend_port / ui_port (with their env-var names + restart-to-apply note) and makes the LAN-share port editable — Save POSTs OMNIVOICE_SHARE_PORT to /system/set-env (persisted), "applies next time you enable sharing". - Tests: extend SharingPanel.test.jsx for the ports subsection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR introduces environment-driven port configuration for the backend, UI dev server, and LAN-share listener. Port values transition from hardcoded constants to configurable environment variables with safe fallbacks, exposed via the ChangesPort configuration feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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. Comment |
|
| Filename | Overview |
|---|---|
| backend/services/network_share.py | Replaces BACKEND_PORT constant with env-driven backend_port()/share_port_base() helpers; _find_free_port() can throw OverflowError when scanning past port 65535. |
| backend/api/routers/system.py | Adds port keys to PERSISTENT_KEYS, validates port range in set-env, and exposes backend_port/share_port_base/ui_port in /system/info; _ui_port() duplicates the same helper already in main.py. |
| backend/main.py | Switches hardcoded 3900 port to network_share.backend_port() in health check and uvicorn.run; CORS origins now use OMNIVOICE_UI_PORT at module-load time. |
| frontend/src-tauri/src/backend.rs | Pushes OMNIVOICE_PORT env var into Python child process so Rust and Python always agree on the listening port; straightforward and correct. |
| frontend/src/components/settings/SharingPanel.jsx | Adds Ports subsection with read-only display for backend/UI ports and an editable LAN-share port input; cancel-safe fetch on mount, optimistic local state update on save. |
| frontend/vite.config.js | Uses OMNIVOICE_UI_PORT env var for Vite dev server port; Number(...) |
| backend/api/schemas.py | Adds backend_port, share_port_base, and ui_port fields to SystemInfoResponse with sensible defaults. |
| tests/test_network_share.py | Adds thorough unit tests for the new port helpers and set-env validation. |
| tests/test_tailscale_service.py | Adds test verifying serve_enable() proxies to the configured OMNIVOICE_PORT rather than hardcoded 3900. |
Sequence Diagram
sequenceDiagram
participant Rust as backend.rs
participant Python as main.py
participant NS as network_share.py
participant TS as tailscale.py
participant UI as SharingPanel.jsx
Note over Rust: Reads OMNIVOICE_PORT env var
Rust->>Python: spawn with OMNIVOICE_PORT in env
Python->>Python: CORS origins built from _ui_port()
Python->>Python: "uvicorn.run(port=backend_port())"
UI->>Python: GET /system/info
Python->>NS: backend_port(), share_port_base()
Python-->>UI: backend_port, share_port_base, ui_port
UI->>Python: POST /system/set-env OMNIVOICE_SHARE_PORT
Python->>Python: validate 1024-65535, persist to prefs
Python-->>UI: set: true
UI->>Python: POST /system/network/enable
Python->>NS: enable(app)
NS->>NS: _find_free_port(share_port_base())
NS->>NS: uvicorn.Server(0.0.0.0, free_port)
UI->>Python: POST /system/tailscale/enable
Python->>TS: serve_enable()
TS->>NS: backend_port()
TS->>TS: tailscale serve http://127.0.0.1:port
Comments Outside Diff (1)
-
backend/services/network_share.py, line 87-88 (link)_find_free_portscansrange(base, base + 20), butsocket.bindraisesOverflowError(notOSError) for port numbers above 65535. Because theset-envvalidator allows values up to 65535, a user who setsOMNIVOICE_SHARE_PORT=65525and finds all of 65525–65535 in use will trigger an uncaughtOverflowErrorthat bypasses theexcept OSErrorclause and propagates as a 500 to the enable endpoint. Capping the scan range at 65535 prevents the crash.
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def _ui_port() -> int: | ||
| """The Vite UI dev-server port, single-sourced from OMNIVOICE_UI_PORT. | ||
|
|
||
| Mirrors the resolver in main.py (kept local to avoid importing the app | ||
| module). Falls back to 3901 on a missing or malformed value. | ||
| """ | ||
| raw = os.environ.get("OMNIVOICE_UI_PORT") | ||
| if raw is None: | ||
| return 3901 | ||
| try: | ||
| return int(raw) | ||
| except (TypeError, ValueError): | ||
| return 3901 |
There was a problem hiding this comment.
_ui_port() is defined identically in both system.py (here) and main.py. The comment says it "mirrors" main.py to avoid a circular import, but the two copies can silently drift if one is updated without the other. Moving _ui_port into network_share alongside backend_port() would give a single source of truth without the import problem.
| def _ui_port() -> int: | |
| """The Vite UI dev-server port, single-sourced from OMNIVOICE_UI_PORT. | |
| Mirrors the resolver in main.py (kept local to avoid importing the app | |
| module). Falls back to 3901 on a missing or malformed value. | |
| """ | |
| raw = os.environ.get("OMNIVOICE_UI_PORT") | |
| if raw is None: | |
| return 3901 | |
| try: | |
| return int(raw) | |
| except (TypeError, ValueError): | |
| return 3901 | |
| def _ui_port() -> int: | |
| """The Vite UI dev-server port, single-sourced from OMNIVOICE_UI_PORT. | |
| Moved to network_share to keep all port helpers together and avoid | |
| duplication with main.py. Falls back to 3901 on a missing or malformed | |
| value — never throws. | |
| """ | |
| from services import network_share as _ns # local import avoids circular dep | |
| return _ns.ui_port() |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Lets users adapt OmniVoice to their own networks instead of hardcoded 3900/3901. Env vars are the source of truth (the ports are read at startup by the Rust sidecar before any in-app config loads); Settings shows them and makes the runtime LAN-share port editable.
What changed
OMNIVOICE_PORT(backend) was already read by the Rust sidecar; now Python honors it too. Fixes the core bug:network_share.pyhardcoded3900, so a custom backend port left LAN-share + Tailscale still proxying:3900.network_share.backend_port()/share_port_base()are now env-driven.OMNIVOICE_SHARE_PORT(defaultbackend+1) — the LAN-share listener base; editable in Settings (persists via the existing prefs mechanism, applies on next enable).OMNIVOICE_UI_PORT(default3901) — Vite dev port + CORS origins.backend.rsnow pushesOMNIVOICE_PORTonto the spawned child's env, so Python'sbackend_port()always equals the--portRust used — the listener and the LAN-share/Tailscale target can never diverge./system/infoexposesbackend_port/share_port_base/ui_port; Settings → Sharing has a Ports subsection (displays backend/UI ports + env-var names + "restart to apply"; LAN-share port editable). set-env validates port keys (numeric, 1024–65535).Verification (full CI-equivalent, all green locally)
Backend 32 passed ·
cargo check✓ ·typecheck:ci✓ ·test:legacy36 ✓ · vitest 91 ✓ · build ✓.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests