test: Testing smallwebrtc - #903
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds SmallWebRTC as a third Breeze Buddy voice transport with WebRTC execution modes, authenticated offer/PATCH signaling, in-process bot execution, ICE and ESP32 routing, runtime diagnostics, shutdown cleanup, configuration, codec dependencies, documentation, and unit tests. ChangesSmallWebRTC transport integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SmallWebRTCRouter
participant SmallWebRTCHandlers
participant WebRTCBot
Client->>SmallWebRTCRouter: POST /smallwebrtc/offer
SmallWebRTCRouter->>SmallWebRTCHandlers: validate lead and handle offer
SmallWebRTCHandlers->>WebRTCBot: spawn in-process bot
SmallWebRTCHandlers-->>Client: SDP answer
Client->>SmallWebRTCRouter: PATCH /smallwebrtc/offer with ICE candidates
SmallWebRTCRouter->>SmallWebRTCHandlers: route patch by pc_id
SmallWebRTCHandlers-->>Client: status ok
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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.
Pull request overview
Adds SmallWebRTC as an additional Breeze Buddy voice transport (alongside Daily + telephony), including server-side SDP offer/ICE handling, new execution modes, Docker/runtime deps, and unit tests + dev TURN tooling to support local testing (especially on locked-down macOS environments).
Changes:
- Adds
WEBRTC/WEBRTC_TESTexecution modes (schema + DB constraint) and wires SmallWebRTC router lifecycle into the FastAPI app. - Introduces SmallWebRTC offer/PATCH handlers that spawn an in-process bot using Pipecat’s SmallWebRTC request handler, with new env-driven ICE/TURN configuration.
- Updates dependencies (Pipecat
webrtcextra + aiortc ecosystem), Docker codec libs, and adds unit tests + dev coturn docs/config.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Locks new dependencies for Pipecat webrtc extra (aiortc stack, etc.). |
pyproject.toml |
Adds webrtc extra to pinned pipecat-ai==1.1.0. |
Dockerfile |
Installs libopus0 / libvpx7 to support aiortc codec/runtime needs. |
app/schemas/breeze_buddy/core.py |
Adds ExecutionMode.WEBRTC and ExecutionMode.WEBRTC_TEST. |
app/database/migrations/035_add_webrtc_execution_modes.sql |
Extends DB execution_mode CHECK constraint to include WebRTC modes. |
app/core/config/static.py |
Adds BB_WEBRTC_ICE_SERVERS, BB_MAX_CONCURRENT_WEBRTC_BOTS, BB_WEBRTC_ESP32_HOST. |
.env.example |
Documents new SmallWebRTC env vars and recommended values. |
app/api/routers/breeze_buddy/smallwebrtc/handlers.py |
Implements ICE parsing, lead validation, offer/patch handlers, and bot spawning. |
app/api/routers/breeze_buddy/smallwebrtc/__init__.py |
Adds authenticated POST/PATCH endpoints for /smallwebrtc/offer. |
app/api/routers/breeze_buddy/__init__.py |
Mounts the SmallWebRTC router under Breeze Buddy routes. |
app/main.py |
Closes SmallWebRTC handlers during lifespan shutdown to avoid leaking PCs on drain. |
app/core/logger/__init__.py |
Reduces noisy logging and prevents potential secret leakage from websockets.client debug logs. |
app/ai/voice/agents/breeze_buddy/agent/transport.py |
Adds "webrtc" transport params factory + transport constant + AIC model selection behavior. |
app/ai/voice/agents/breeze_buddy/agent/__init__.py |
Routes WebRTC through RTC setup path, adjusts teardown/transfer behavior, and adds webrtc_bot entrypoint. |
app/ai/voice/agents/breeze_buddy/handlers/internal/end_conversation.py |
Treats WebRTC like Daily for completion updates keyed by lead id. |
app/ai/voice/agents/breeze_buddy/agent/webrtc_input_patch.py |
Adds diagnostic instrumentation for investigating WebRTC input latency. |
tests/breeze_buddy/test_smallwebrtc_offer.py |
Unit tests for ICE parsing, lead_id parsing, handler routing, and lead validation behavior. |
scripts/dev-turn/turnserver.conf |
Local dev-only coturn config bound to loopback for Mac Stealth Mode environments. |
scripts/dev-turn/README.md |
Documentation for running/using the local loopback TURN relay. |
docs/SMALLWEBRTC_DEVICE_TRANSPORT_SPEC.md |
Spec describing architecture, constraints, and rollout considerations for SmallWebRTC device transport. |
docs/superpowers/plans/2026-07-16-smallwebrtc-transport.md |
Detailed implementation plan and operational notes for SmallWebRTC transport work. |
| # Reuse the daily module's completion + live-task tracking: completion just flips | ||
| # the lead to FINISHED by call_id==lead_id (nothing Daily-specific), and | ||
| # _track_live_bot is the strong-ref set that both counts live bots and prevents | ||
| # asyncio GC. | ||
| from app.ai.voice.agents.breeze_buddy.services.daily.daily import ( | ||
| _live_bot_tasks, | ||
| _track_live_bot, | ||
| daily_completion_function, | ||
| ) | ||
| from app.core.config.static import ( | ||
| BB_MAX_CONCURRENT_WEBRTC_BOTS, | ||
| BB_WEBRTC_ESP32_HOST, | ||
| BB_WEBRTC_ICE_SERVERS, | ||
| ) | ||
| from app.core.logger import logger | ||
| from app.core.transport.http_client import create_aiohttp_session |
| await validate_webrtc_lead(lead_id) | ||
| if len(_live_bot_tasks) >= BB_MAX_CONCURRENT_WEBRTC_BOTS: | ||
| raise HTTPException( | ||
| status_code=503, detail="Too many concurrent WebRTC sessions" | ||
| ) | ||
|
|
||
| async def _on_connection(conn: Any) -> None: | ||
| runner_args = SmallWebRTCRunnerArguments(webrtc_connection=conn) | ||
| runner_args.body = {"lead_id": lead_id} | ||
| session = create_aiohttp_session() | ||
| task = asyncio.create_task( | ||
| webrtc_bot(runner_args, daily_completion_function, session) | ||
| ) | ||
| _track_live_bot(task) | ||
| logger.info(f"Spawned in-process SmallWebRTC bot for lead_id: {lead_id}") |
| # DIAGNOSTIC (pipecat==1.1.0): confirmed the in-process pipeline blocks the | ||
| # shared event loop (~400ms), starving aiortc's on-loop audio receive -> | ||
| # ~2s speech->STT latency. The watchdog thread dumps the loop stack the | ||
| # instant it stalls, naming the exact blocking call. | ||
| from app.ai.voice.agents.breeze_buddy.agent.webrtc_input_patch import ( | ||
| apply_webrtc_input_diagnostics, | ||
| heartbeat_task, | ||
| start_loop_watchdog, | ||
| ) | ||
|
|
||
| apply_webrtc_input_diagnostics() | ||
| start_loop_watchdog() | ||
| hb_task = asyncio.create_task(heartbeat_task()) | ||
|
|
||
| agent = Agent( | ||
| transport_type=TRANSPORT_TYPE_WEBRTC, | ||
| aiohttp_session=aiohttp_session, | ||
| completion_function=completion_function, | ||
| ) | ||
| try: | ||
| await agent.run(runner_args) | ||
| finally: | ||
| hb_task.cancel() |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@app/api/routers/breeze_buddy/smallwebrtc/__init__.py`:
- Around line 19-37: Replace the raw Dict[str, Any] request and response
annotations in smallwebrtc_offer and smallwebrtc_patch with the appropriate
Pydantic request and response models. Define or reuse models matching the
handlers’ payloads, preserve the existing handler calls and request_host
behavior, and ensure FastAPI uses those models for validation and API
documentation.
In `@app/api/routers/breeze_buddy/smallwebrtc/handlers.py`:
- Around line 182-194: Fix the TOCTOU race between the limit check and task
registration in the WebRTC connection flow. Use a semaphore or
pending-connection counter acquired before handler.handle_web_request and
released when the connection attempt completes or fails, while retaining
_track_live_bot for active tasks; ensure every path releases the reservation so
BB_MAX_CONCURRENT_WEBRTC_BOTS is never exceeded.
- Around line 97-124: Bound the host-keyed handler cache to prevent arbitrary
Host headers from creating unlimited SmallWebRTCRequestHandler instances. Update
_get_handler and _esp32_handlers to use an LRU cache with a finite capacity, or
validate request_host against an explicit allowed-host list before caching;
preserve BB_WEBRTC_ESP32_HOST precedence and _all_handlers access to active
handlers.
In `@app/database/migrations/035_add_webrtc_execution_modes.sql`:
- Around line 12-14: Update the lead_call_tracker_execution_mode_check
constraint in migration 035 to add it with NOT VALID, then add a separate
statement to validate that constraint using its existing name. Preserve the
current execution_mode allowed values.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4101cd48-1d94-46c8-898e-09c1fb4819af
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.env.exampleDockerfileapp/ai/voice/agents/breeze_buddy/agent/__init__.pyapp/ai/voice/agents/breeze_buddy/agent/transport.pyapp/ai/voice/agents/breeze_buddy/agent/webrtc_input_patch.pyapp/ai/voice/agents/breeze_buddy/handlers/internal/end_conversation.pyapp/api/routers/breeze_buddy/__init__.pyapp/api/routers/breeze_buddy/smallwebrtc/__init__.pyapp/api/routers/breeze_buddy/smallwebrtc/handlers.pyapp/core/config/static.pyapp/core/logger/__init__.pyapp/database/migrations/035_add_webrtc_execution_modes.sqlapp/main.pyapp/schemas/breeze_buddy/core.pydocs/SMALLWEBRTC_DEVICE_TRANSPORT_SPEC.mddocs/superpowers/plans/2026-07-16-smallwebrtc-transport.mdpyproject.tomlscripts/dev-turn/README.mdscripts/dev-turn/turnserver.conftests/breeze_buddy/test_smallwebrtc_offer.py
| @router.post("/smallwebrtc/offer") | ||
| async def smallwebrtc_offer( | ||
| body: Dict[str, Any], | ||
| request: Request, | ||
| current_user: UserInfo = Depends(get_current_user_with_rbac), | ||
| ) -> Dict[str, Any]: | ||
| # request_host feeds ESP32 SDP munging when the client self-identifies as | ||
| # esp32 and no BB_WEBRTC_ESP32_HOST override is set. | ||
| return await smallwebrtc_offer_handler( | ||
| body, request_host=request.headers.get("host") | ||
| ) | ||
|
|
||
|
|
||
| @router.patch("/smallwebrtc/offer") | ||
| async def smallwebrtc_patch( | ||
| body: Dict[str, Any], | ||
| current_user: UserInfo = Depends(get_current_user_with_rbac), | ||
| ) -> Dict[str, Any]: | ||
| return await smallwebrtc_patch_handler(body) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use Pydantic models for API request and response schemas.
As per coding guidelines, use Pydantic models for all API request/response schemas and data transfer rather than raw Dict[str, Any]. This ensures built-in validation and better API documentation.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 23-23: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 35-35: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 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 `@app/api/routers/breeze_buddy/smallwebrtc/__init__.py` around lines 19 - 37,
Replace the raw Dict[str, Any] request and response annotations in
smallwebrtc_offer and smallwebrtc_patch with the appropriate Pydantic request
and response models. Define or reuse models matching the handlers’ payloads,
preserve the existing handler calls and request_host behavior, and ensure
FastAPI uses those models for validation and API documentation.
Source: Coding guidelines
| _esp32_handlers: Dict[str, SmallWebRTCRequestHandler] = {} | ||
|
|
||
|
|
||
| def _get_handler( | ||
| client_type: Optional[str], request_host: Optional[str] | ||
| ) -> SmallWebRTCRequestHandler: | ||
| if client_type != "esp32": | ||
| return _default_handler | ||
| # Munging host: env override wins (proxies/LBs may rewrite Host); else the | ||
| # address the device actually dialed. | ||
| host = BB_WEBRTC_ESP32_HOST or (request_host or "").split(":")[0] | ||
| if not host: | ||
| logger.warning( | ||
| "ESP32 client but no munging host available — using default handler" | ||
| ) | ||
| return _default_handler | ||
| if host not in _esp32_handlers: | ||
| _esp32_handlers[host] = SmallWebRTCRequestHandler( | ||
| ice_servers=_ice_servers, | ||
| connection_mode=ConnectionMode.MULTIPLE, | ||
| esp32_mode=True, | ||
| host=host, | ||
| ) | ||
| return _esp32_handlers[host] | ||
|
|
||
|
|
||
| def _all_handlers() -> List[SmallWebRTCRequestHandler]: | ||
| return [_default_handler, *_esp32_handlers.values()] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Unbounded cache growth leading to memory leak / DoS.
The _esp32_handlers dictionary lazily creates and caches a new SmallWebRTCRequestHandler for every unique host. When BB_WEBRTC_ESP32_HOST is not set, host defaults to the user-controlled HTTP Host header. An attacker can send requests with arbitrary Host headers to exhaust server memory.
Consider using an LRU cache for _esp32_handlers or validating the Host header against a list of allowed hosts before creating a new handler.
🤖 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 `@app/api/routers/breeze_buddy/smallwebrtc/handlers.py` around lines 97 - 124,
Bound the host-keyed handler cache to prevent arbitrary Host headers from
creating unlimited SmallWebRTCRequestHandler instances. Update _get_handler and
_esp32_handlers to use an LRU cache with a finite capacity, or validate
request_host against an explicit allowed-host list before caching; preserve
BB_WEBRTC_ESP32_HOST precedence and _all_handlers access to active handlers.
| if len(_live_bot_tasks) >= BB_MAX_CONCURRENT_WEBRTC_BOTS: | ||
| raise HTTPException( | ||
| status_code=503, detail="Too many concurrent WebRTC sessions" | ||
| ) | ||
|
|
||
| async def _on_connection(conn: Any) -> None: | ||
| runner_args = SmallWebRTCRunnerArguments(webrtc_connection=conn) | ||
| runner_args.body = {"lead_id": lead_id} | ||
| session = create_aiohttp_session() | ||
| task = asyncio.create_task( | ||
| webrtc_bot(runner_args, daily_completion_function, session) | ||
| ) | ||
| _track_live_bot(task) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
TOCTOU race condition on concurrent bot limits.
The check len(_live_bot_tasks) >= BB_MAX_CONCURRENT_WEBRTC_BOTS happens before the await handler.handle_web_request(request, _on_connection) call, but the task is only added to _live_bot_tasks inside the _on_connection callback. If multiple requests arrive concurrently, they can all pass the limit check before any tasks are created, exceeding the configured BB_MAX_CONCURRENT_WEBRTC_BOTS.
Consider tracking a count of "pending" connections or using a semaphore to strictly enforce the concurrency limit.
🤖 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 `@app/api/routers/breeze_buddy/smallwebrtc/handlers.py` around lines 182 - 194,
Fix the TOCTOU race between the limit check and task registration in the WebRTC
connection flow. Use a semaphore or pending-connection counter acquired before
handler.handle_web_request and released when the connection attempt completes or
fails, while retaining _track_live_bot for active tasks; ensure every path
releases the reservation so BB_MAX_CONCURRENT_WEBRTC_BOTS is never exceeded.
| ALTER TABLE lead_call_tracker | ||
| ADD CONSTRAINT lead_call_tracker_execution_mode_check | ||
| CHECK (execution_mode IN ('TELEPHONY', 'TELEPHONY_TEST', 'DAILY', 'DAILY_TEST', 'DAILY_STREAM', 'HOLD_TRANSFER', 'WEBRTC', 'WEBRTC_TEST')); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Prevent table locking by adding the constraint as NOT VALID.
Adding a CHECK constraint without NOT VALID requires a full table scan while holding an ACCESS EXCLUSIVE lock, which will block all reads and writes to lead_call_tracker until the scan completes. For large tables, this can cause significant production downtime.
Please add the constraint as NOT VALID first, and then validate it in a separate statement. Validating it later only requires a SHARE UPDATE EXCLUSIVE lock, allowing concurrent reads and writes.
⚡ Proposed fix for safe constraint creation
ALTER TABLE lead_call_tracker
ADD CONSTRAINT lead_call_tracker_execution_mode_check
-CHECK (execution_mode IN ('TELEPHONY', 'TELEPHONY_TEST', 'DAILY', 'DAILY_TEST', 'DAILY_STREAM', 'HOLD_TRANSFER', 'WEBRTC', 'WEBRTC_TEST'));
+CHECK (execution_mode IN ('TELEPHONY', 'TELEPHONY_TEST', 'DAILY', 'DAILY_TEST', 'DAILY_STREAM', 'HOLD_TRANSFER', 'WEBRTC', 'WEBRTC_TEST')) NOT VALID;
+
+ALTER TABLE lead_call_tracker
+VALIDATE CONSTRAINT lead_call_tracker_execution_mode_check;🧰 Tools
🪛 Squawk (2.59.0)
[warning] 13-14: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.
(constraint-missing-not-valid)
🤖 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 `@app/database/migrations/035_add_webrtc_execution_modes.sql` around lines 12 -
14, Update the lead_call_tracker_execution_mode_check constraint in migration
035 to add it with NOT VALID, then add a separate statement to validate that
constraint using its existing name. Preserve the current execution_mode allowed
values.
Source: Linters/SAST tools
c7a2afe to
c8ffdaf
Compare
c8ffdaf to
d6f0819
Compare
|
Unconditional WEBRTC-DIAG instrumentation runs on every Soniox STT call, not just WebRTC — The new async def run_stt(self, audio: bytes) -> AsyncGenerator[Frame | None, None]:
...
ws_open = self._websocket is not None and self._websocket.state is State.OPEN
if not ws_open:
if not self._diag_drop_logged:
self._diag_drop_logged = True
logger.warning(
"WEBRTC-DIAG soniox ws NOT OPEN — dropping audio silently"
)
else:
self._diag_drop_logged = False
if audio:
samples = np.frombuffer(audio, dtype=np.int16)
rms = float(np.sqrt(np.mean(np.square(samples, dtype=np.float64))))
...
Given this PR already gated a similar concern behind removal elsewhere (the old unconditional diagnostic instrumentation in Source commit: |
Summary by CodeRabbit
New Features
Bug Fixes
Tests