deploy: expose port on 0.0.0.0, add OMNIVOICE_IDLE_TIMEOUT=0 - #125
Conversation
- Expose port 3900 on 0.0.0.0 (LAN accessible via Tailscale) - Add OMNIVOICE_IDLE_TIMEOUT=0 (prevent model unload) - Mount patched client.js and model_manager.py into GPU container - Inline HF_TOKEN for GPU/CPU profiles
📝 WalkthroughWalkthroughThis pull request updates the Docker Compose configuration to enable network-wide accessibility for OmniVoice services and prevent idle timeouts. Both the CPU and GPU service variants now bind to all network interfaces ( ChangesDocker Compose Network and Idle Timeout Configuration
🎯 1 (Trivial) | ⏱️ ~3 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/docker-compose.yml`:
- Line 49: The environment sets OMNIVOICE_IDLE_TIMEOUT=0 which, given the
model_manager idle check (uses _last_used and compares time.time() - _last_used
> _IDLE_TIMEOUT_SECONDS), causes immediate unloads; change the idle-worker logic
in backend/services/model_manager.py (the loop that checks _last_used and
_IDLE_TIMEOUT_SECONDS) to explicitly treat non-positive timeouts as “never
unload” (e.g., short-circuit when _IDLE_TIMEOUT_SECONDS <= 0) so a zero value
prevents unloading rather than triggering it.
- Around line 77-78: The inline comment incorrectly claims LAN reachability is
off while the port mapping uses 0.0.0.0:3900:3900 which binds to all host
interfaces; either change the comment to correctly state that 0.0.0.0 exposes
the service on all host interfaces (LAN reachable) or change the port mapping to
127.0.0.1:3900:3900 to restrict access to localhost; update the text near the
existing "0.0.0.0:3900:3900" mapping to reflect whichever fix you choose.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f30ff85-e3d7-4f9f-8702-1cb90cf2bd65
📒 Files selected for processing (1)
deploy/docker-compose.yml
| # OMNIVOICE_BIND_HOST=0.0.0.0 here only opens the container's own | ||
| # interface. The backend default is 127.0.0.1 (see backend/main.py). | ||
| - OMNIVOICE_BIND_HOST=0.0.0.0 | ||
| - OMNIVOICE_IDLE_TIMEOUT=0 |
There was a problem hiding this comment.
OMNIVOICE_IDLE_TIMEOUT=0 currently causes faster unloads, not disabled unloads.
Given current backend logic, 0 makes time.time() - _last_used > 0 true on the first idle-worker cycle, so models unload shortly after inactivity instead of staying warm.
Suggested fix
- - OMNIVOICE_IDLE_TIMEOUT=0
+ # Keep default (900s) until backend supports "disable idle unload" semantics.
+ # - OMNIVOICE_IDLE_TIMEOUT=0If you want 0 to mean “never unload”, backend logic should explicitly short-circuit, e.g. in backend/services/model_manager.py:
if _IDLE_TIMEOUT_SECONDS <= 0:
continueAlso applies to: 80-80
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/docker-compose.yml` at line 49, The environment sets
OMNIVOICE_IDLE_TIMEOUT=0 which, given the model_manager idle check (uses
_last_used and compares time.time() - _last_used > _IDLE_TIMEOUT_SECONDS),
causes immediate unloads; change the idle-worker logic in
backend/services/model_manager.py (the loop that checks _last_used and
_IDLE_TIMEOUT_SECONDS) to explicitly treat non-positive timeouts as “never
unload” (e.g., short-circuit when _IDLE_TIMEOUT_SECONDS <= 0) so a zero value
prevents unloading rather than triggering it.
| # service above. The host-side `0.0.0.0:3900:3900` mapping keeps | ||
| # LAN reachability off by default. |
There was a problem hiding this comment.
Inline comment contradicts actual host exposure behavior.
Line 77 says LAN reachability is off by default, but 0.0.0.0:3900:3900 exposes it on all host interfaces by default.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/docker-compose.yml` around lines 77 - 78, The inline comment
incorrectly claims LAN reachability is off while the port mapping uses
0.0.0.0:3900:3900 which binds to all host interfaces; either change the comment
to correctly state that 0.0.0.0 exposes the service on all host interfaces (LAN
reachable) or change the port mapping to 127.0.0.1:3900:3900 to restrict access
to localhost; update the text near the existing "0.0.0.0:3900:3900" mapping to
reflect whichever fix you choose.
|
The |
|
Thanks for this — the The Could you rework it so LAN exposure is opt-in?
That lands both QoL wins without changing the secure-by-default posture. Happy to merge once it's opt-in. |
|
Thanks @abhid! We're declining the raw host-mapping flip to |
|
@debpalash Thanks for the heads up. That makes total sense. |
…125) (#159) * docs(spec): network sharing + Tailscale remote access design Same-state LAN sharing via a second in-process uvicorn listener on a dedicated share port (no restart, model/jobs preserved), PIN-gated for non-loopback clients, with QR + all-LAN-addresses panel. Tailscale serve for private remote access. Supersedes the raw 0.0.0.0 default-flip in #125. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(spec): control endpoints reuse existing require_loopback gate Security review of #157 confirmed the /system router is already loopback-gated via Depends(require_loopback) (non-spoofable request.client.host). The network control endpoints inherit it and /system/set-env is auto-protected from the LAN listener — no new guard needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(network): share-listener module — LAN enumeration + PIN + lifecycle Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(network): loopback-only control endpoints + /system/info sharing fields Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cjk): scan git-tracked files only, not untracked vendored dirs The no-hardcoded-CJK guard walked the filesystem, so local untracked vendored experiments (research/voice-pro etc. with JP issue templates) caused false local failures while CI (committed files) passed. Scan via git ls-files so local-only and CI behavior match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(network): PIN middleware — gate non-loopback API access when sharing on Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(network): inject X-OmniVoice-Pin globally + capture ?pin= from QR URL Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(network): remote PIN gate on 401 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(network): add qrcode dep for share QR Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(network): footer Local/Network toggle with LAN addresses, QR, copy/open Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(tailscale): CLI status + serve enable/disable + endpoints Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(network): Settings → Sharing & Remote Access panel (LAN + Tailscale) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(network): sharing & remote access guide (LAN PIN/QR + Tailscale) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(network): enable() tears down and raises if the share listener never binds Defensive guard (spec §7): if the second uvicorn server doesn't reach 'started' (e.g. the share port was taken in the race after the free-port probe), cancel the task, reset state, and raise — so the API surfaces the failure and the UI stays Local rather than reporting a dead 'Network' state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(network): use globalThis (not Node global) in client.test.ts for tsc CI runs 'tsc --noEmit --checkJs false', which type-checks .ts files; Node's 'global' isn't typed there (TS2304). vitest (esbuild) tolerated it locally. Use globalThis (standard, typed) + cast the mock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(network): apiFetch leaves opts untouched when no PIN set The unconditional headers merge changed the request shape for callers with no headers (e.g. FormData posts), breaking the legacy 'apiPost passes FormData without Content-Type override' node test. Only spread opts + inject X-OmniVoice-Pin when a PIN is actually present; otherwise pass opts through unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Two quality-of-life fixes for self-hosted deployments:
1. Expose port on
0.0.0.0instead of127.0.0.1Allows the container to be reached from the local network / reverse proxy without needing extra Docker networking config. Users who want loopback-only can still override the host binding in their own compose override.
2. Add
OMNIVOICE_IDLE_TIMEOUT=0Prevents the server from unloading models during periods of inactivity. Essential for always-on deployments where startup latency on the first request is unacceptable.
Both CPU and GPU service profiles updated.
Summary by CodeRabbit