From c9103a24da00960015d90ffe76700dc2d54dcd56 Mon Sep 17 00:00:00 2001
From: James Pine <ijamespine@me.com>
Date: Thu, 23 Apr 2026 19:16:37 -0700
Subject: [PATCH] fix(mcp): stamp last_seen_at on /speak too + tighten path
 predicate
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

POST /speak is a REST wrapper around voicebox.speak for agents that
don't talk MCP (shell scripts, ACP, A2A). It reads X-Voicebox-Client-Id
and uses it for the same per-client profile resolution + default
personality lookup the MCP tool does (speak.py:39-64), so its callers
are first-class clients — but the ClientIdMiddleware only stamped
last_seen_at on /mcp* paths. REST speak callers showed up as "never
seen" in Settings → MCP despite actively acting on their bindings.

Widen the stamp predicate to an explicit ("/mcp", "/speak") prefix
list, and require a path boundary on match so future routes named
/mcpfoo or /speakers don't silently inherit the stamp via the prefix.
New test_client_id_middleware.py pins the scope with 17 parametrised
cases (both the allowed set and the overlap cases that must not match).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 backend/mcp_server/context.py              | 34 +++++++++++----
 backend/tests/test_client_id_middleware.py | 49 ++++++++++++++++++++++
 2 files changed, 75 insertions(+), 8 deletions(-)
 create mode 100644 backend/tests/test_client_id_middleware.py

diff --git a/backend/mcp_server/context.py b/backend/mcp_server/context.py
index 20809a77..cfeeb481 100644
--- a/backend/mcp_server/context.py
+++ b/backend/mcp_server/context.py
@@ -26,15 +26,27 @@ current_client_id: ContextVar[str | None] = ContextVar(
     "current_client_id", default=None
 )
 
+# Endpoints that consume X-Voicebox-Client-Id for its MCP-semantic
+# meaning (per-client profile resolution + per-client default_personality).
+# These are the paths where a stamp into last_seen_at is accurate.
+# Unrelated REST traffic that happens to set the header is intentionally
+# ignored so the Settings UI's "last heard from" column only reflects
+# calls that actually acted on the client's bindings.
+#
+# - /mcp — FastMCP tool calls (voicebox.speak, voicebox.transcribe, …)
+#   and the /mcp/bindings admin surface. The admin surface is never
+#   called with the header in practice (the frontend manages bindings
+#   over plain REST), so the `startswith("/mcp")` match doesn't cause
+#   false stamps.
+# - /speak — REST mirror of voicebox.speak for non-MCP agents (shell
+#   scripts, ACP, A2A). Uses the same per-client binding lookup, so its
+#   callers belong in the last-seen list too.
+_STAMPED_PATH_PREFIXES: tuple[str, ...] = ("/mcp", "/speak")
+
 
 class ClientIdMiddleware(BaseHTTPMiddleware):
-    """Copy X-Voicebox-Client-Id into a ContextVar and stamp last_seen_at.
-
-    Only stamps on MCP-endpoint requests (anything under ``/mcp``) so
-    unrelated REST traffic with the header set won't advance the
-    last-seen timestamp — the Settings UI uses that to show when each
-    client was last heard from.
-    """
+    """Copy X-Voicebox-Client-Id into a ContextVar and stamp last_seen_at
+    for requests that act on the caller's MCP bindings."""
 
     def __init__(self, app: ASGIApp) -> None:
         super().__init__(app)
@@ -47,11 +59,17 @@ class ClientIdMiddleware(BaseHTTPMiddleware):
         finally:
             current_client_id.reset(token)
 
-        if client_id and request.url.path.startswith("/mcp"):
+        if client_id and _is_stamped_path(request.url.path):
             _stamp_last_seen(client_id)
         return response
 
 
+def _is_stamped_path(path: str) -> bool:
+    # Require a path boundary so a future ``/speakers`` or ``/mcpfoo``
+    # route doesn't silently inherit the stamp from ``/speak`` / ``/mcp``.
+    return any(path == p or path.startswith(p + "/") for p in _STAMPED_PATH_PREFIXES)
+
+
 def _stamp_last_seen(client_id: str) -> None:
     """Update or create the MCPClientBinding row for this client_id."""
     try:
diff --git a/backend/tests/test_client_id_middleware.py b/backend/tests/test_client_id_middleware.py
new file mode 100644
index 00000000..c9d081b4
--- /dev/null
+++ b/backend/tests/test_client_id_middleware.py
@@ -0,0 +1,49 @@
+"""Unit tests for the ClientIdMiddleware path predicate.
+
+Locks down which endpoints advance ``last_seen_at`` on the
+``MCPClientBinding`` row. Getting this wrong is silent: the Settings UI
+just shows a stale "last heard from" timestamp and bindings never get
+auto-created for new REST callers.
+"""
+
+import pytest
+
+from backend.mcp_server.context import _is_stamped_path
+
+
+@pytest.mark.parametrize(
+    "path",
+    [
+        "/mcp",
+        "/mcp/",
+        "/mcp/tools/call",
+        "/mcp/bindings",  # admin REST; benign — frontend never sets the header
+        "/speak",
+        "/speak/",
+    ],
+)
+def test_mcp_semantic_paths_are_stamped(path: str) -> None:
+    assert _is_stamped_path(path) is True
+
+
+@pytest.mark.parametrize(
+    "path",
+    [
+        "/",
+        "/health",
+        "/generate",
+        "/captures",
+        "/profiles",
+        "/profiles/abc/compose",
+        "/events/speak",
+        "/tasks/active",
+        "/llm/generate",
+        # Prefix overlap should not match — /speakers is a hypothetical
+        # future endpoint that shouldn't leak the stamp.
+        "/speakers",
+        # Same for anything starting with /mcpfoo.
+        "/mcpfoo",
+    ],
+)
+def test_other_paths_are_not_stamped(path: str) -> None:
+    assert _is_stamped_path(path) is False