Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions backend/api/routers/mcp_bindings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
"""REST CRUD for per-agent MCP voice bindings (Wave 2.2 / Spec 2).

Loopback-gated — the Settings UI manages bindings here. The MCP tools
themselves resolve voices via ``services.mcp_bindings.resolve_voice``.
"""

from __future__ import annotations

from fastapi import APIRouter, Depends, HTTPException
from pydantic import BaseModel, Field

from api.dependencies import require_loopback
from services import mcp_bindings

router = APIRouter(
prefix="/api/mcp",
tags=["mcp"],
dependencies=[Depends(require_loopback)],
)


class _BindingBody(BaseModel):
client_id: str = Field(..., min_length=1, max_length=128)
label: str | None = None
profile_id: str | None = None
default_engine: str | None = None


@router.get("/bindings")
def list_bindings():
"""All per-agent voice bindings, most-recently-seen first."""
return mcp_bindings.list_bindings()


@router.put("/bindings")
def upsert_binding(body: _BindingBody):
"""Create or update the binding for an MCP client id."""
try:
return mcp_bindings.upsert_binding(
body.client_id,
label=body.label,
profile_id=body.profile_id,
default_engine=body.default_engine,
)
except ValueError as e:
raise HTTPException(status_code=400, detail=str(e))
Comment on lines +45 to +46

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve exception cause when mapping validation errors to HTTP 400.

On Line 46, use explicit exception chaining so traceback origin stays intact during debugging (raise ... from e).

Suggested patch
-    except ValueError as e:
-        raise HTTPException(status_code=400, detail=str(e))
+    except ValueError as e:
+        raise HTTPException(status_code=400, detail=str(e)) from e
🧰 Tools
🪛 Ruff (0.15.15)

[warning] 46-46: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🤖 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 `@backend/api/routers/mcp_bindings.py` around lines 45 - 46, Replace the bare
re-raise of HTTPException with explicit exception chaining so the original
ValueError traceback is preserved: in the except ValueError as e block where
HTTPException is raised, change the raise to use "from e" (i.e., raise
HTTPException(status_code=400, detail=str(e)) from e) to maintain the original
exception context for debugging; this references the except ValueError as e
handler and the HTTPException construction in mcp_bindings.py.

Source: Linters/SAST tools



@router.delete("/bindings/{client_id}")
def delete_binding(client_id: str):
if not mcp_bindings.delete_binding(client_id):
raise HTTPException(status_code=404, detail="No binding for that client id")
return {"deleted": client_id}
13 changes: 13 additions & 0 deletions backend/core/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,19 @@ def db_conn():
value TEXT NOT NULL,
updated_at REAL NOT NULL
);

-- Wave 2.2: per-agent MCP voice bindings. An MCP client (Claude Code,
-- Cursor, …) identified by the X-OmniVoice-Client-Id header it sends is
-- bound to a default voice profile / engine. Fresh installs create it
-- here; v0.3.x upgrades get it via alembic 0004.
CREATE TABLE IF NOT EXISTS mcp_client_bindings (
client_id TEXT PRIMARY KEY,
label TEXT NOT NULL DEFAULT '',
profile_id TEXT,
default_engine TEXT,
last_seen_at REAL,
created_at REAL
);
"""

# Only tables/columns this module is allowed to ALTER. Prevents SQL injection via
Expand Down
39 changes: 38 additions & 1 deletion backend/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,23 @@ def _warm():
capture_preload_task = asyncio.create_task(_preload_capture_asr())
else:
logger.info("Capture ASR preload disabled; dictation ASR will load on first use.")
yield

# ── MCP session manager (Wave 2.2) ────────────────────────────────────
# FastMCP's Streamable-HTTP transport needs its session manager running
# for the lifetime of the app. It's created lazily by streamable_http_app()
# (called in mount_mcp below), so we stack its `run()` context into ours
# via AsyncExitStack rather than replacing this lifespan. Best-effort: a
# missing/broken MCP layer must never stop the rest of the backend.
from contextlib import AsyncExitStack
async with AsyncExitStack() as _mcp_stack:
_sm = getattr(app.state, "mcp_session_manager", None)
if _sm is not None:
try:
await _mcp_stack.enter_async_context(_sm.run())
logger.info("MCP server mounted at /mcp")
except Exception as e:
logger.warning("MCP session manager failed to start: %s", e)
yield
Comment on lines +438 to +447

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard MCP teardown so it cannot bypass the backend’s own shutdown path.

Because yield sits inside async with AsyncExitStack(), any exception from _sm.run().__aexit__() aborts the function before Lines 448-479 run. That leaves worker tasks alive, skips model/VRAM release, and can strand the next launch behind orphaned state. Close the MCP stack in its own guarded step instead of letting AsyncExitStack unwind unhandled around the yield boundary.

Suggested structure
-    from contextlib import AsyncExitStack
-    async with AsyncExitStack() as _mcp_stack:
-        _sm = getattr(app.state, "mcp_session_manager", None)
-        if _sm is not None:
-            try:
-                await _mcp_stack.enter_async_context(_sm.run())
-                logger.info("MCP server mounted at /mcp")
-            except Exception as e:
-                logger.warning("MCP session manager failed to start: %s", e)
-        yield
+    from contextlib import AsyncExitStack
+    _mcp_stack = AsyncExitStack()
+    try:
+        _sm = getattr(app.state, "mcp_session_manager", None)
+        if _sm is not None:
+            try:
+                await _mcp_stack.enter_async_context(_sm.run())
+                logger.info("MCP server mounted at /mcp")
+            except Exception as e:
+                logger.warning("MCP session manager failed to start: %s", e)
+        yield
+    finally:
+        try:
+            await _mcp_stack.aclose()
+        except Exception as e:
+            logger.warning("MCP session manager failed to stop: %s", e)
🧰 Tools
🪛 Ruff (0.15.15)

[warning] 445-445: Do not catch blind exception: Exception

(BLE001)

🤖 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 `@backend/main.py` around lines 438 - 447, The current context manager uses
AsyncExitStack() so if _sm.run().__aexit__() raises during the implicit exit it
can bypass the rest of shutdown; change the structure so the MCP context is
entered and exited explicitly and guarded rather than relying on the async-with
to span the yield: obtain _mcp_stack = AsyncExitStack() and enter _sm.run() with
await _mcp_stack.enter_async_context(_sm.run()) before the yield, then after the
yield close the MCP stack in its own try/except/finally block (call await
_mcp_stack.aclose() inside a protected block and log any exceptions) so teardown
of _mcp_stack and subsequent backend shutdown steps always run even if the MCP
teardown raises; reference symbols: AsyncExitStack, _mcp_stack, _sm, _sm.run(),
and the yield boundary.

# ── Graceful shutdown (SIGTERM from Tauri, Ctrl+C, etc.) ────────────
logger.info("Shutdown: cleaning up…")
idle_task.cancel()
Expand Down Expand Up @@ -749,6 +765,27 @@ def health():
app.include_router(marketplace.router)
app.include_router(sonitranslate.router)
app.include_router(settings_router.router) # Phase 1 AUTH-03 endpoints
from api.routers import mcp_bindings as _mcp_bindings_router # noqa: E402
app.include_router(_mcp_bindings_router.router) # Wave 2.2 per-agent voice bindings

# ── Mount the MCP server (Wave 2.2) ───────────────────────────────────────
# FastMCP's Streamable-HTTP app is sub-mounted at /mcp; its session manager is
# stashed on app.state for the lifespan above to run. Opt-out via
# OMNIVOICE_MCP_DISABLE=1; best-effort so a missing mcp package or a build
# without it never breaks startup.
if os.environ.get("OMNIVOICE_MCP_DISABLE", "").strip().lower() not in ("1", "true", "yes", "on"):
try:
from mcp_server import create_mcp_server

_mcp = create_mcp_server()
_mcp_app = _mcp.streamable_http_app()
app.state.mcp_session_manager = _mcp.session_manager
app.mount("/mcp", _mcp_app)
logging.getLogger("omnivoice.api").info("MCP app mounted at /mcp")
except Exception as _mcp_err: # noqa: BLE001
logging.getLogger("omnivoice.api").info(
"MCP server not mounted (%s); /mcp disabled.", _mcp_err
)

frontend_path = os.path.join(os.path.dirname(__file__), "..", "frontend", "dist")
if os.path.exists(frontend_path):
Expand Down
65 changes: 64 additions & 1 deletion backend/mcp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@
"voice design, and video dubbing in 646 languages."
),
)
# Serve the Streamable-HTTP transport at the app root so mounting the whole
# app at "/mcp" on the main FastAPI yields the endpoint at "/mcp". FastMCP's
# default path is "/mcp", which would double-prefix to "/mcp/mcp" when
# sub-mounted. Harmless for the standalone CLI run() path.
try:
mcp.settings.streamable_http_path = "/"
except Exception:

Check notice

Code scanning / CodeQL

Empty except Note

'except' clause does nothing but pass and there is no explanatory comment.
pass
Comment on lines +60 to +63

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Silent path misconfiguration leaves MCP dead with no diagnostic

If mcp.settings.streamable_http_path = "/" raises for any reason — validation error, attribute renamed in a FastMCP patch, settings becoming read-only — the assignment is silently swallowed. The FastMCP default path remains /mcp. After mounting the sub-app at /mcp on the main FastAPI, Starlette strips the /mcp prefix before forwarding, so the only reachable endpoint is /mcp/mcp. Every client connection fails with 404/405 while the startup log still says "MCP app mounted at /mcp". Replace the bare pass with at least a logger.warning so operators can diagnose the double-prefix scenario.

Fix in Claude Code


# ── Helpers ─────────────────────────────────────────────────────────

Expand All @@ -75,6 +83,21 @@

# ── Tools ───────────────────────────────────────────────────────────

def _current_client_id() -> str | None:
"""The X-OmniVoice-Client-Id of the calling MCP client, if any.

FastMCP exposes the HTTP request via its request context on the
Streamable-HTTP transport; stdio clients (and any version where the
accessor differs) simply resolve to None and fall back to the
global default voice."""
try:
req = mcp.get_context().request_context.request
if req is not None:
return req.headers.get("x-omnivoice-client-id")
except Exception:

Check notice

Code scanning / CodeQL

Empty except Note

'except' clause does nothing but pass and there is no explanatory comment.
pass
return None

@mcp.tool()
async def generate_speech(
text: str,
Expand All @@ -89,7 +112,8 @@
Args:
text: The text to synthesize into speech.
language: Target language (ISO code or 'Auto'). 646 languages supported.
profile_id: ID of a saved voice profile to clone. Omit for voice design mode.
profile_id: ID of a saved voice profile to clone. Omit to use this
agent's bound voice (Settings → MCP), else the global default.
instruct: Style instruction (e.g. 'whisper', 'excited', 'narrator').
speed: Speech speed multiplier (0.5–2.0, default 1.0).
steps: Diffusion steps (8=fast/draft, 16=balanced, 32=quality).
Expand All @@ -98,6 +122,17 @@
JSON with audio_id, generation_time, audio_duration, and
base64-encoded WAV data.
"""
# Per-agent voice binding (Wave 2.2): explicit arg wins; otherwise
# resolve this client's bound profile, then the global default.
client_id = _current_client_id()
try:
from services import mcp_bindings
resolved = mcp_bindings.resolve_voice(client_id, profile_id)
profile_id = resolved.get("profile_id")
mcp_bindings.touch_last_seen(client_id) if client_id else None
except Exception:
pass # binding layer unavailable — use whatever was passed

form = {
"text": text,
"language": language,
Expand Down Expand Up @@ -159,6 +194,34 @@
'],"note":"Pass any ISO 639 code or set language=Auto for detection."}'
)

@mcp.tool()
async def transcribe(audio_base64: str, language: str | None = None) -> str:
"""Transcribe spoken audio to text.

Args:
audio_base64: Base64-encoded audio bytes (wav/mp3/webm/m4a).
language: Optional language hint; omit for auto-detect.

Returns:
JSON with the recognized text, language, and duration.
"""
try:
raw = base64.b64decode(audio_base64, validate=True)
except Exception:
return '{"error":"audio_base64 is not valid base64"}'
# 200 MB cap — same spirit as voicebox's transcribe gate. Keeps a
# buggy/hostile agent from posting an unbounded blob.
if len(raw) > 200 * 1024 * 1024:
return '{"error":"audio exceeds 200 MB limit"}'
Comment on lines +208 to +215

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reject oversized audio before b64decode().

Line 209 allocates the full decoded blob before the 200 MB guard on Line 214 runs, so a buggy or hostile client can still force a very large allocation and take down the mounted backend. Preflight the encoded length first, or stream-decode with a hard cap.

⚙️ Suggested fix
+        max_raw = 200 * 1024 * 1024
+        max_b64 = ((max_raw + 2) // 3) * 4
+        if len(audio_base64) > max_b64:
+            return '{"error":"audio exceeds 200 MB limit"}'
         try:
             raw = base64.b64decode(audio_base64, validate=True)
         except Exception:
             return '{"error":"audio_base64 is not valid base64"}'
-        # 200 MB cap — same spirit as voicebox's transcribe gate. Keeps a
-        # buggy/hostile agent from posting an unbounded blob.
-        if len(raw) > 200 * 1024 * 1024:
+        if len(raw) > max_raw:
             return '{"error":"audio exceeds 200 MB limit"}'

As per coding guidelines, "the backend serves loopback HTTP: treat every query/path/form param as hostile."

🧰 Tools
🪛 Ruff (0.15.15)

[warning] 210-210: Do not catch blind exception: Exception

(BLE001)

🤖 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 `@backend/mcp_server.py` around lines 208 - 215, Reject oversized input before
decoding: compute a safe maximum encoded length from the 200 MB raw cap (e.g.
max_encoded = int(200 * 1024 * 1024 * 4 / 3) + some padding for base64
padding/newlines) and check len(audio_base64) against that and return the error
if exceeded before calling base64.b64decode; then proceed to decode into raw and
keep the existing len(raw) check as a secondary guard. Use the existing variable
names audio_base64 and raw (and the same error message) so you only add the
preflight length check and early return to avoid allocating huge decoded blobs.

Source: Coding guidelines

Comment on lines +208 to +215

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Size guard fires after the 200 MB allocation, not before

The comment explicitly says this cap "Keeps a buggy/hostile agent from posting an unbounded blob," but base64.b64decode runs before the check. A 200 MB audio file encodes to ~267 MB of base64; the server fully allocates the ~200 MB decoded bytes object and then rejects it. A buggy agent looping the call, or one injecting large payloads, can trigger repeated 200 MB heap allocations before any guard fires. Add an encoded-length pre-check (base64 expands by ~4/3) so the blob is never decoded when it would exceed the cap.

Suggested change
try:
raw = base64.b64decode(audio_base64, validate=True)
except Exception:
return '{"error":"audio_base64 is not valid base64"}'
# 200 MB cap — same spirit as voicebox's transcribe gate. Keeps a
# buggy/hostile agent from posting an unbounded blob.
if len(raw) > 200 * 1024 * 1024:
return '{"error":"audio exceeds 200 MB limit"}'
# 200 MB cap — same spirit as voicebox's transcribe gate. Keeps a
# buggy/hostile agent from posting an unbounded blob.
# Check the encoded length first (~4/3 overhead) so we never allocate
# the decoded blob when we'd immediately reject it anyway.
if len(audio_base64) > 200 * 1024 * 1024 * 4 // 3 + 64:
return '{"error":"audio exceeds 200 MB limit"}'
try:
raw = base64.b64decode(audio_base64, validate=True)
except Exception:
return '{"error":"audio_base64 is not valid base64"}'
if len(raw) > 200 * 1024 * 1024:
return '{"error":"audio exceeds 200 MB limit"}'

Fix in Claude Code

data = {}
if language:
data["language"] = language
r = await _api_post_form(
"/transcribe", data=data,
files={"audio": ("audio.wav", raw, "application/octet-stream")},
)
return str(r.json())
Comment on lines +219 to +223

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Return actual JSON from the new tool.

Line 223 uses str(r.json()), which yields Python repr with single quotes rather than parseable JSON. That breaks the documented tool contract for any MCP client expecting structured JSON back from transcribe().

🛠️ Suggested fix
-        return str(r.json())
+        return json.dumps(r.json(), ensure_ascii=False)

Add import json at the top of the module.

🤖 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 `@backend/mcp_server.py` around lines 219 - 223, The code currently returns
str(r.json()) which produces a Python repr instead of valid JSON; import json at
the top of the module and replace that return with json.dumps(r.json()) so the
transcribe endpoint (the block calling _api_post_form and returning r.json())
emits proper JSON text that clients can parse.


@mcp.tool()
async def check_health() -> str:
"""Check if the OmniVoice backend is running and what GPU device is active."""
Expand Down
1 change: 1 addition & 0 deletions backend/mcp_shim/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""omnivoice-mcp — stdio MCP shim for clients that only speak stdio."""
Loading
Loading