From e0fa825189e5a2b3e54764ca395feda393fff67f Mon Sep 17 00:00:00 2001
From: Robert Helewka <r@helu.ca>
Date: Wed, 6 May 2026 19:47:52 -0400
Subject: [PATCH] auth: read tool name off context.message directly; trace
 call_next failures
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In FastMCP's on_call_tool hook the middleware context is already
MiddlewareContext[CallToolRequestParams] (per fastmcp's own
middleware.py:158), so tool name lives at context.message.name, not
at context.message.params.name — the latter always returned None,
silently breaking the PUBLIC_TOOLS bypass for get_health and making
the per-tool ACL short-circuit.

Also wrap call_next in a traced helper that logs any exception with
a full traceback and logs the success-path result type.  During the
Pallas↔Mnemosyne shakedown the tool results were coming back to
fast-agent as the literal string "object NoneType can't be used in
'await' expression" with no trace in either process — that's Python's
TypeError for 'await X' where X is None.  If that TypeError is raised
inside FastMCP dispatch we want the frame in Mnemosyne's own log
rather than having Pallas's aggregator turn it into a terse
CallToolResult(isError=True) with no stack.
---
 mnemosyne/mcp_server/auth.py | 61 +++++++++++++++++++++++++++++++++---
 1 file changed, 57 insertions(+), 4 deletions(-)

diff --git a/mnemosyne/mcp_server/auth.py b/mnemosyne/mcp_server/auth.py
index 4954529..c405261 100644
--- a/mnemosyne/mcp_server/auth.py
+++ b/mnemosyne/mcp_server/auth.py
@@ -252,7 +252,9 @@ class MCPAuthMiddleware(Middleware):
         )
 
         if require_auth and tool_name in self._PUBLIC_TOOLS:
-            return await call_next(context)
+            return await self._call_next_with_trace(
+                tool_name, call_next, context
+            )
 
         token_string = self._extract_token()
 
@@ -297,7 +299,40 @@ class MCPAuthMiddleware(Middleware):
             if claims is not None:
                 await fastmcp_ctx.set_state(STATE_KEY_CLAIMS, claims)
 
-        return await call_next(context)
+        return await self._call_next_with_trace(tool_name, call_next, context)
+
+    @staticmethod
+    async def _call_next_with_trace(tool_name, call_next, context):
+        """Run ``call_next`` and log any exception with a full traceback.
+
+        During the Pallas↔Mnemosyne shakedown we were seeing tool results
+        come back to fast-agent as the string ``"object NoneType can't be
+        used in 'await' expression"`` with no trace anywhere in either
+        process. That string is Python's ``TypeError`` for ``await X``
+        where ``X`` is ``None`` (i.e. someone awaited a non-coroutine).
+        If that TypeError is raised inside the FastMCP dispatch we want
+        the full traceback in Mnemosyne's own log rather than silently
+        letting it propagate back to Pallas where the aggregator turns
+        it into a ``CallToolResult(isError=True)`` and we lose the frame.
+
+        We also log the *type* of the successful return so we can verify
+        later that FastMCP is returning ``ToolResult`` / ``CallToolResult``
+        the way we expect. Keep at INFO until green, demote to DEBUG
+        afterwards.
+        """
+        try:
+            result = await call_next(context)
+        except Exception:
+            logger.exception(
+                "mcp_auth.call_next_failed tool=%s", tool_name
+            )
+            raise
+        logger.info(
+            "mcp_auth.call_next_ok tool=%s result_type=%s",
+            tool_name,
+            type(result).__name__,
+        )
+        return result
 
     @staticmethod
     def _extract_token() -> str | None:
@@ -345,9 +380,27 @@ class MCPAuthMiddleware(Middleware):
 
     @staticmethod
     def _extract_tool_name(context: MiddlewareContext) -> str | None:
+        """Pull the tool name off a FastMCP ``on_call_tool`` context.
+
+        In FastMCP middleware, ``context.message`` inside ``on_call_tool``
+        is already a ``CallToolRequestParams`` (see
+        ``fastmcp.server.middleware.middleware.Middleware.on_call_tool``
+        signature: ``MiddlewareContext[mt.CallToolRequestParams]``), so
+        ``name`` lives directly on ``context.message`` — there is no
+        nested ``.params``. The older ``message.params.name`` access we
+        had here always returned ``None``, which caused the public-tools
+        bypass to silently miss ``get_health`` and made the per-tool ACL
+        short-circuit. Fall back to ``.params.name`` only as a legacy
+        safety net in case the shape ever diverges.
+        """
         msg = getattr(context, "message", None)
-        params = getattr(msg, "params", None) if msg else None
-        return getattr(params, "name", None)
+        if msg is None:
+            return None
+        name = getattr(msg, "name", None)
+        if name:
+            return name
+        params = getattr(msg, "params", None)
+        return getattr(params, "name", None) if params is not None else None
 
 
 # --- Helpers ---------------------------------------------------------------