mirror of
https://github.com/usestrix/strix.git
synced 2026-08-28 05:25:00 +00:00
render MCP tool calls with the connection look again
This commit is contained in:
Jonathan Singer
parent
cb5691d6e2
commit
e3f95d6cfd
15 changed files with 212 additions and 152 deletions
|
|
@ -33,3 +33,15 @@ func renderMcpTool(connection, toolName string, args map[string]any, status stri
|
|||
b.WriteString(style.Render(icon))
|
||||
return b.String()
|
||||
}
|
||||
|
||||
// renderMcpInspect renders describe_mcp: a request to inspect one connection's
|
||||
// catalog rather than a call to a tool on it. There is no underlying tool, so
|
||||
// the connection is the whole subject and leads. Same icon and colors as a tool
|
||||
// call so the two read as one family while scrolling a transcript.
|
||||
func renderMcpInspect(connection, status string) string {
|
||||
var b strings.Builder
|
||||
b.WriteString(mcpIcon + Dim().Render("Inspecting MCP server ") + Bold(Mint).Render(connection) + "\n")
|
||||
icon, style := statusIcon(status)
|
||||
b.WriteString(style.Render(icon))
|
||||
return b.String()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -57,6 +57,11 @@ func Tool(data map[string]any) string {
|
|||
// nothing here. The tag is only ever set from the connections the run made,
|
||||
// so it is the one thing that can tell such a call apart from a built-in.
|
||||
if connection := StringValue(data["mcp_connection"]); connection != "" {
|
||||
// describe_mcp inspects a connection's catalog rather than calling a tool
|
||||
// on it, so there is no underlying tool and the connection is the subject.
|
||||
if name == "describe_mcp" {
|
||||
return renderMcpInspect(connection, status)
|
||||
}
|
||||
toolName := StringValue(data["mcp_tool"])
|
||||
if toolName == "" {
|
||||
toolName = name
|
||||
|
|
|
|||
|
|
@ -227,7 +227,9 @@ func TestGenericToolOmitsRawResult(t *testing.T) {
|
|||
}
|
||||
|
||||
func TestMcpToolLeadsWithActionAndNamesTheServer(t *testing.T) {
|
||||
data := tool("local_fs_read_file", map[string]any{"path": "/etc/hosts"}, "file body", "completed")
|
||||
// call_mcp is the dispatch tool; the connection and the server's own tool
|
||||
// name are tagged onto the event from its arguments.
|
||||
data := tool("call_mcp", map[string]any{"path": "/etc/hosts"}, "file body", "completed")
|
||||
data["mcp_connection"] = "local_fs"
|
||||
data["mcp_tool"] = "read_file"
|
||||
|
||||
|
|
@ -244,11 +246,26 @@ func TestMcpToolLeadsWithActionAndNamesTheServer(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestMcpToolWithoutTaggedNameFallsBackToFullName(t *testing.T) {
|
||||
data := tool("local_fs_read_file", nil, nil, "running")
|
||||
func TestMcpToolWithoutTaggedToolFallsBackToDispatchName(t *testing.T) {
|
||||
// A call_mcp whose underlying tool could not be read still renders as an MCP
|
||||
// row, falling back to the dispatch tool name.
|
||||
data := tool("call_mcp", nil, nil, "running")
|
||||
data["mcp_connection"] = "local_fs"
|
||||
|
||||
requireContains(t, ansi.Strip(Tool(data)), "local_fs_read_file", "In progress")
|
||||
requireContains(t, ansi.Strip(Tool(data)), mcpIcon+"call_mcp", "local_fs", "In progress")
|
||||
}
|
||||
|
||||
func TestMcpDescribeInspectsConnection(t *testing.T) {
|
||||
// describe_mcp inspects a connection; the connection is the subject and the
|
||||
// dispatch tool name is not shown as if it were a server tool.
|
||||
data := tool("describe_mcp", nil, nil, "completed")
|
||||
data["mcp_connection"] = "local_fs"
|
||||
|
||||
out := ansi.Strip(Tool(data))
|
||||
requireContains(t, out, mcpIcon, "Inspecting MCP server", "local_fs", "Done")
|
||||
if strings.Contains(out, "describe_mcp") {
|
||||
t.Fatalf("describe_mcp must read as inspecting the connection, not name the dispatch tool:\n%s", out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCollapseToolShellPreviewAndExpand(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -8,7 +8,6 @@ from typing import TYPE_CHECKING, Any
|
|||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Iterable
|
||||
from pathlib import Path
|
||||
|
||||
from agents.tool import ToolOutputImage
|
||||
|
|
@ -16,9 +15,10 @@ from agents.tool import ToolOutputImage
|
|||
from strix.core.paths import runtime_state_dir
|
||||
from strix.interface.tui.history import load_session_history
|
||||
|
||||
# Imported from the naming module rather than the mcp package so a projection
|
||||
# never pulls in the MCP client and the agents SDK behind it.
|
||||
from strix.tools.mcp.naming import resolve_mcp_tool
|
||||
|
||||
# Every MCP call the model makes goes through one of two dispatch tools, so a
|
||||
# call to a user's server is recognised by the tool's name alone.
|
||||
_MCP_DISPATCH_TOOLS = frozenset({"call_mcp", "describe_mcp"})
|
||||
|
||||
|
||||
class TuiLiveView:
|
||||
|
|
@ -31,27 +31,30 @@ class TuiLiveView:
|
|||
self._user_instruction: str | None = None
|
||||
self._user_instruction_at: str | None = None
|
||||
self._user_instruction_shown = False
|
||||
self._mcp_connections: tuple[str, ...] = ()
|
||||
|
||||
def set_mcp_connections(self, names: Iterable[str]) -> None:
|
||||
"""The MCP servers this run connected, so its tool calls can name theirs.
|
||||
|
||||
A server's tools are offered to the model under a name built from the
|
||||
connection name and the tool's own name. That name cannot be split back
|
||||
apart on its own, so tool calls are matched against these names instead.
|
||||
"""
|
||||
self._mcp_connections = tuple(str(name) for name in names)
|
||||
|
||||
def _mcp_tool_fields(self, tool_name: str) -> dict[str, str]:
|
||||
def _mcp_tool_fields(self, tool_name: str, args: dict[str, Any]) -> dict[str, str]:
|
||||
"""Event fields naming the MCP server a tool call went out to, if any.
|
||||
|
||||
Empty for every built-in tool, which is what tells an interface to render
|
||||
the call as one of its own rather than as a call to a user's server.
|
||||
Every MCP call the model makes goes through one of two dispatch tools:
|
||||
``call_mcp`` runs a named tool on a connection, and ``describe_mcp``
|
||||
inspects a connection's catalog. The connection, and for ``call_mcp`` the
|
||||
server's own name for the tool, ride in the call's arguments rather than
|
||||
in the tool name, so they are read from there. Empty for every other tool,
|
||||
which is what tells an interface to render the call as one of its own
|
||||
rather than as a call to a user's server.
|
||||
"""
|
||||
origin = resolve_mcp_tool(tool_name, self._mcp_connections)
|
||||
if origin is None:
|
||||
if tool_name not in _MCP_DISPATCH_TOOLS:
|
||||
return {}
|
||||
return {"mcp_connection": origin.connection, "mcp_tool": origin.tool}
|
||||
connection = args.get("connection")
|
||||
if not isinstance(connection, str) or not connection:
|
||||
return {}
|
||||
# describe_mcp has no underlying tool; an empty tool tells both renderers
|
||||
# to present the row as inspecting the connection itself.
|
||||
tool = args.get("tool") if tool_name == "call_mcp" else ""
|
||||
return {
|
||||
"mcp_connection": connection,
|
||||
"mcp_tool": tool if isinstance(tool, str) else "",
|
||||
}
|
||||
|
||||
def set_user_instruction(self, text: str | None, *, timestamp: str | None = None) -> None:
|
||||
"""Open the transcript with what the user asked for.
|
||||
|
|
@ -98,8 +101,7 @@ class TuiLiveView:
|
|||
|
||||
def hydrate_from_run_dir(self, run_dir: Path) -> None:
|
||||
# Armed before the agents are added so the root agent's arrival puts the
|
||||
# user's opening message ahead of the replayed history, and before the
|
||||
# history is replayed so its MCP tool calls are attributed too.
|
||||
# user's opening message ahead of the replayed history.
|
||||
self._load_run_record(run_dir)
|
||||
state_dir = runtime_state_dir(run_dir)
|
||||
agents_path = state_dir / "agents.json"
|
||||
|
|
@ -128,16 +130,13 @@ class TuiLiveView:
|
|||
self._hydrate_sdk_session_history(run_dir, statuses.keys())
|
||||
|
||||
def _load_run_record(self, run_dir: Path) -> None:
|
||||
"""Take the user's opening message and the run's MCP servers off the record."""
|
||||
"""Take the user's opening message off the record."""
|
||||
try:
|
||||
record = json.loads((run_dir / "run.json").read_text(encoding="utf-8"))
|
||||
except (OSError, json.JSONDecodeError):
|
||||
return
|
||||
if not isinstance(record, dict):
|
||||
return
|
||||
connections = record.get("mcp_connections")
|
||||
if isinstance(connections, list):
|
||||
self.set_mcp_connections(name for name in connections if isinstance(name, str))
|
||||
instruction = record.get("user_instruction")
|
||||
if not isinstance(instruction, str):
|
||||
return
|
||||
|
|
@ -348,7 +347,7 @@ class TuiLiveView:
|
|||
"status": "running",
|
||||
"agent_id": agent_id,
|
||||
"call_id": call_id,
|
||||
**self._mcp_tool_fields(call["tool_name"]),
|
||||
**self._mcp_tool_fields(call["tool_name"], call["args"]),
|
||||
}
|
||||
if existing is None:
|
||||
event = self._append_event(agent_id, "tool", tool_data, timestamp=timestamp)
|
||||
|
|
@ -371,6 +370,10 @@ class TuiLiveView:
|
|||
event_key = (agent_id, call_id)
|
||||
event = self._tool_event_by_agent_and_call_id.get(event_key)
|
||||
if event is None:
|
||||
# No prior call event to update, so its arguments are gone and the
|
||||
# connection an MCP call went out to cannot be recovered. The matching
|
||||
# call event, when there is one, already carries the MCP fields; this
|
||||
# arrives only when the call was never projected, so it stays generic.
|
||||
event = self._append_event(
|
||||
agent_id,
|
||||
"tool",
|
||||
|
|
@ -380,7 +383,6 @@ class TuiLiveView:
|
|||
"status": "completed",
|
||||
"agent_id": agent_id,
|
||||
"call_id": call_id,
|
||||
**self._mcp_tool_fields(output["tool_name"]),
|
||||
},
|
||||
timestamp=timestamp,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -207,23 +207,9 @@ class GoTuiRuntime:
|
|||
self.controller.notify_changed()
|
||||
|
||||
def capture_event(self, agent_id: str, event: Any) -> None:
|
||||
self._refresh_mcp_connections()
|
||||
self.live_view.ingest_sdk_event(agent_id, event)
|
||||
self.controller.notify_changed()
|
||||
|
||||
def _refresh_mcp_connections(self) -> None:
|
||||
"""Hand the projection the MCP servers the scan connected.
|
||||
|
||||
The scan records them as it connects, which is before the agent can call
|
||||
anything, and the projection needs them to say which server a tool call
|
||||
went out to. Read on the way in rather than pushed, so no tool call can
|
||||
be projected before they arrive.
|
||||
"""
|
||||
if self.report_state is None:
|
||||
return
|
||||
connections = self.report_state.run_record.get("mcp_connections") or []
|
||||
self.live_view.set_mcp_connections(connections)
|
||||
|
||||
async def _sync_agent_state(self) -> bool:
|
||||
parent_of, statuses, names, errors = await self.coordinator.graph_snapshot()
|
||||
changed = False
|
||||
|
|
|
|||
|
|
@ -47,13 +47,29 @@ export default function McpRenderer({
|
|||
const lines = argLines(args);
|
||||
const failed = status === "failed" || status === "error";
|
||||
const error = failed ? errorText(result) : null;
|
||||
// describe_mcp inspects a connection's catalog rather than calling a tool on
|
||||
// it, so the connection is the subject and there is no underlying tool.
|
||||
const inspecting = toolName === "describe_mcp";
|
||||
|
||||
return (
|
||||
<div>
|
||||
<div className="flex items-center gap-2 flex-wrap">
|
||||
<span className="font-mono text-teal-300 font-semibold text-sm">{mcpTool || toolName}</span>
|
||||
<span className="text-[13px] text-[#555]">via MCP server</span>
|
||||
{mcpConnection && <span className="text-[13px] text-teal-400/80">{mcpConnection}</span>}
|
||||
{inspecting ? (
|
||||
<>
|
||||
<span className="text-[13px] text-[#555]">Inspecting MCP server</span>
|
||||
{mcpConnection && (
|
||||
<span className="font-mono text-teal-300 font-semibold text-sm">{mcpConnection}</span>
|
||||
)}
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<span className="font-mono text-teal-300 font-semibold text-sm">
|
||||
{mcpTool || toolName}
|
||||
</span>
|
||||
<span className="text-[13px] text-[#555]">via MCP server</span>
|
||||
{mcpConnection && <span className="text-[13px] text-teal-400/80">{mcpConnection}</span>}
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{lines.length > 0 && (
|
||||
|
|
|
|||
|
|
@ -181,8 +181,9 @@ function resolveCategory(toolName: string): ToolCategory | null {
|
|||
|
||||
/**
|
||||
* A call to a tool from one of the user's MCP servers is placed by the
|
||||
* connection it was tagged with, ahead of every name-keyed lookup below: its
|
||||
* name belongs to that server and matches nothing in this table.
|
||||
* connection it was tagged with, ahead of every name-keyed lookup below. Every
|
||||
* MCP call goes through the `call_mcp` / `describe_mcp` dispatch tools, so the
|
||||
* connection tag, not the tool name, is what routes it to the MCP renderer.
|
||||
*/
|
||||
export function getToolRenderer(
|
||||
toolName: string,
|
||||
|
|
|
|||
|
|
@ -101,9 +101,10 @@ export interface ToolRendererProps {
|
|||
status: "running" | "completed" | "failed" | "error";
|
||||
/**
|
||||
* Set only on a call to a tool from an MCP server the user connected: the name
|
||||
* they gave that connection, and the server's own name for the tool. The
|
||||
* engine resolves both, because `toolName` is the two glued together and
|
||||
* cannot be split back apart here.
|
||||
* they gave that connection, and the server's own name for the tool. Every MCP
|
||||
* call goes through the `call_mcp` / `describe_mcp` dispatch tools, so the
|
||||
* engine reads both out of the call's arguments; `describe_mcp` inspects a
|
||||
* connection and leaves `mcpTool` empty.
|
||||
*/
|
||||
mcpConnection?: string | null;
|
||||
mcpTool?: string | null;
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -6,7 +6,7 @@
|
|||
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
|
||||
<meta name="color-scheme" content="dark" />
|
||||
<title>Strix Results</title>
|
||||
<script type="module" crossorigin src="./assets/index-C9c1WbvP.js"></script>
|
||||
<script type="module" crossorigin src="./assets/index-CYf9nnT3.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="./assets/index-D0453ODW.css">
|
||||
</head>
|
||||
<body>
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ from strix.tools.mcp.config import (
|
|||
McpConnectionConfig,
|
||||
)
|
||||
from strix.tools.mcp.loader import load_user_mcp_configs
|
||||
from strix.tools.mcp.naming import McpToolOrigin, namespaced_tool_name, resolve_mcp_tool
|
||||
from strix.tools.mcp.naming import namespaced_tool_name
|
||||
from strix.tools.mcp.registry import (
|
||||
MCP_REGISTRY_CONTEXT_KEY,
|
||||
McpConnectionEntry,
|
||||
|
|
@ -29,12 +29,10 @@ __all__ = [
|
|||
"McpConnectionEntry",
|
||||
"McpConnectionSummary",
|
||||
"McpRegistry",
|
||||
"McpToolOrigin",
|
||||
"call_mcp",
|
||||
"connect_mcp_servers",
|
||||
"describe_mcp",
|
||||
"load_user_mcp_configs",
|
||||
"mcp_inventory_context",
|
||||
"namespaced_tool_name",
|
||||
"resolve_mcp_tool",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -97,10 +97,11 @@ async def call_mcp(
|
|||
Args:
|
||||
connection: The connection name exactly as shown in the MCP inventory.
|
||||
tool: The tool name, exactly as reported by ``describe_mcp``.
|
||||
arguments: The tool's arguments as an object of names to values, or
|
||||
omitted/empty for a tool that takes none. ``arguments`` is passed
|
||||
through as-is, so its shape is whatever ``describe_mcp`` showed for
|
||||
the tool rather than a shape this tool fixes in advance.
|
||||
arguments: The tool's arguments as a JSON object of names to values (for
|
||||
example ``{"path": "app.py"}``), or omitted/empty for a tool that
|
||||
takes none. Pass an object, not a stringified one. Its shape is
|
||||
whatever ``describe_mcp`` showed for the tool rather than a shape this
|
||||
tool fixes in advance.
|
||||
"""
|
||||
registry = _registry_from_ctx(ctx)
|
||||
if registry is None or not registry:
|
||||
|
|
@ -108,11 +109,22 @@ async def call_mcp(
|
|||
entry = registry.get(connection)
|
||||
if entry is None:
|
||||
return _unknown_connection(connection, registry)
|
||||
invalid_arguments = (
|
||||
f"Invalid arguments for {connection!r}.{tool}: expected a JSON object of "
|
||||
"argument names to values, or none. Call describe_mcp for the input schema."
|
||||
)
|
||||
if isinstance(arguments, str):
|
||||
# The ``arguments`` parameter is schema-less (an open object is not
|
||||
# expressible as a strict tool schema), so some models serialize it as a
|
||||
# JSON string instead of a bare object. Accept a string that decodes to an
|
||||
# object so a correct call is not rejected over its encoding.
|
||||
stripped = arguments.strip()
|
||||
try:
|
||||
arguments = json.loads(stripped) if stripped else {}
|
||||
except json.JSONDecodeError:
|
||||
return invalid_arguments
|
||||
if arguments is not None and not isinstance(arguments, dict):
|
||||
return (
|
||||
f"Invalid arguments for {connection!r}.{tool}: expected a JSON object of "
|
||||
"argument names to values, or none. Call describe_mcp for the input schema."
|
||||
)
|
||||
return invalid_arguments
|
||||
available = await entry.server.list_tools()
|
||||
valid_names = {mcp_tool.name for mcp_tool in available}
|
||||
if tool not in valid_names:
|
||||
|
|
|
|||
|
|
@ -17,6 +17,8 @@ from __future__ import annotations
|
|||
import contextlib
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING, Any, NamedTuple, cast
|
||||
|
||||
from agents.mcp import (
|
||||
|
|
@ -27,6 +29,7 @@ from agents.mcp import (
|
|||
MCPServerStreamableHttpParams,
|
||||
create_static_tool_filter,
|
||||
)
|
||||
from mcp.client.stdio import stdio_client
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
|
|
@ -72,6 +75,35 @@ def _auth_headers(config: McpConnectionConfig) -> dict[str, str]:
|
|||
return {"Authorization": f"Bearer {auth.token}"}
|
||||
|
||||
|
||||
@contextlib.asynccontextmanager
|
||||
async def _quiet_stdio_streams(params: Any) -> Any:
|
||||
"""Run a stdio MCP server with its stderr sent to the void.
|
||||
|
||||
A stdio MCP server chats on stderr as it boots (the filesystem server, for
|
||||
one, prints ``Allowed directories: [ ... ]``). The mcp library forwards that
|
||||
stderr to the parent's ``sys.stderr`` by default, which is the terminal the
|
||||
TUI is drawing on, so the banner corrupts the display. Pointing ``errlog`` at
|
||||
``os.devnull`` drops that chatter. Connection failures are unaffected: they
|
||||
still raise from ``connect`` and are logged by :func:`connect_mcp_servers`.
|
||||
"""
|
||||
with Path(os.devnull).open("w", encoding="utf-8") as errlog:
|
||||
async with stdio_client(params, errlog=errlog) as streams:
|
||||
yield streams
|
||||
|
||||
|
||||
class _QuietMCPServerStdio(MCPServerStdio):
|
||||
"""``MCPServerStdio`` whose subprocess stderr is kept off the terminal.
|
||||
|
||||
The SDK's ``create_streams`` calls ``stdio_client(self.params)`` with no
|
||||
``errlog``, so the subprocess stderr defaults to ``sys.stderr`` and paints
|
||||
server banners over the running TUI. Overriding it lets us redirect that
|
||||
stream; everything else about the stdio transport is unchanged.
|
||||
"""
|
||||
|
||||
def create_streams(self) -> Any:
|
||||
return _quiet_stdio_streams(self.params)
|
||||
|
||||
|
||||
def _build_server(config: McpConnectionConfig) -> MCPServer:
|
||||
"""Construct (but do not connect) the SDK server for one connection.
|
||||
|
||||
|
|
@ -92,7 +124,7 @@ def _build_server(config: McpConnectionConfig) -> MCPServer:
|
|||
"args": config.args,
|
||||
"env": config.env,
|
||||
}
|
||||
return MCPServerStdio(
|
||||
return _QuietMCPServerStdio(
|
||||
params=stdio_params,
|
||||
name=config.name,
|
||||
tool_filter=tool_filter,
|
||||
|
|
|
|||
|
|
@ -1,18 +1,13 @@
|
|||
"""How an MCP server's tools are named for the model, and how to read that back.
|
||||
"""How an MCP server's tools are named for the model.
|
||||
|
||||
Kept apart from the client, and stdlib-only, so the interfaces can resolve which
|
||||
connection a tool call went to without importing the MCP client (and through it
|
||||
the agents SDK and every registered tool).
|
||||
Kept apart from the client, and stdlib-only, so a caller can build the
|
||||
model-facing name for a connection's tool without importing the MCP client (and
|
||||
through it the agents SDK and every registered tool).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from typing import TYPE_CHECKING, NamedTuple
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Iterable
|
||||
|
||||
|
||||
# A tool name offered to a model has to be letters, digits, underscores or
|
||||
|
|
@ -32,49 +27,3 @@ def namespaced_tool_name(connection: str, tool: str) -> str:
|
|||
which tool is invoked.
|
||||
"""
|
||||
return _INVALID_TOOL_NAME_CHARS.sub("_", f"{connection}_{tool}")
|
||||
|
||||
|
||||
class McpToolOrigin(NamedTuple):
|
||||
"""Where a model-facing tool name came from, for showing the user.
|
||||
|
||||
``connection`` is the name the user gave the connection in their config, so
|
||||
it reads the way they wrote it. ``tool`` is what is left of the model-facing
|
||||
name once the connection prefix is removed, which is the server's own name
|
||||
for the tool and the part a reader cares about.
|
||||
"""
|
||||
|
||||
connection: str
|
||||
tool: str
|
||||
|
||||
|
||||
def resolve_mcp_tool(tool_name: str, connections: Iterable[str]) -> McpToolOrigin | None:
|
||||
"""Split a model-facing tool name against the run's connections, or ``None``.
|
||||
|
||||
Matched against the connections the run actually made rather than by
|
||||
splitting the name on the separator: the connection name and the server's own
|
||||
tool name can both contain underscores, so a split is ambiguous and would
|
||||
attribute calls to a connection that does not exist. Each connection name is
|
||||
sanitized the same way :func:`namespaced_tool_name` sanitizes it before
|
||||
comparing, so a connection whose name has characters a model-facing name
|
||||
cannot carry still matches.
|
||||
|
||||
The longest match wins, so one connection whose name is a prefix of another's
|
||||
still resolves to the right one. The character after the prefix has to be a
|
||||
separator rather than more of a name, which any non-alphanumeric satisfies,
|
||||
so this holds whichever separator :func:`namespaced_tool_name` uses.
|
||||
"""
|
||||
best: McpToolOrigin | None = None
|
||||
best_length = 0
|
||||
for connection in connections:
|
||||
prefix = _INVALID_TOOL_NAME_CHARS.sub("_", connection)
|
||||
if not prefix or len(tool_name) <= len(prefix) or not tool_name.startswith(prefix):
|
||||
continue
|
||||
if tool_name[len(prefix)].isalnum():
|
||||
continue
|
||||
if len(prefix) > best_length:
|
||||
# Past the prefix and its single separator character is the tool's
|
||||
# own name; if a server named a tool nothing but separators, fall
|
||||
# back to the whole name so the row still says something.
|
||||
tool = tool_name[len(prefix) + 1 :] or tool_name
|
||||
best, best_length = McpToolOrigin(connection, tool), len(prefix)
|
||||
return best
|
||||
|
|
|
|||
|
|
@ -31,7 +31,6 @@ from strix.tools.mcp import (
|
|||
load_user_mcp_configs,
|
||||
mcp_inventory_context,
|
||||
namespaced_tool_name,
|
||||
resolve_mcp_tool,
|
||||
)
|
||||
from strix.tools.mcp import client as mcp_client
|
||||
|
||||
|
|
@ -417,6 +416,40 @@ async def test_call_mcp_defaults_missing_arguments_to_empty_object() -> None:
|
|||
assert server.calls == [("ping", {})]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_call_mcp_coerces_json_string_arguments() -> None:
|
||||
# Some models serialize the schema-less ``arguments`` object as a JSON string;
|
||||
# a correct call must not be rejected over that encoding.
|
||||
registry = McpRegistry()
|
||||
server = FakeMCPServer("fs", [_mcp_tool("read_file")])
|
||||
registry.add(name="fs", server=server, purpose=None, tool_count=1)
|
||||
|
||||
out = await call_mcp.on_invoke_tool(
|
||||
_ctx(registry),
|
||||
json.dumps(
|
||||
{"connection": "fs", "tool": "read_file", "arguments": '{"path": "/etc/hosts"}'}
|
||||
),
|
||||
)
|
||||
|
||||
assert server.calls == [("read_file", {"path": "/etc/hosts"})]
|
||||
assert out == {"type": "text", "text": "routed:read_file"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_call_mcp_errors_on_unparseable_string_arguments() -> None:
|
||||
registry = McpRegistry()
|
||||
server = FakeMCPServer("fs", [_mcp_tool("read_file")])
|
||||
registry.add(name="fs", server=server, purpose=None, tool_count=1)
|
||||
|
||||
out = await call_mcp.on_invoke_tool(
|
||||
_ctx(registry),
|
||||
json.dumps({"connection": "fs", "tool": "read_file", "arguments": "not json"}),
|
||||
)
|
||||
|
||||
assert "expected a JSON object" in out
|
||||
assert server.calls == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_call_mcp_errors_on_unknown_connection() -> None:
|
||||
registry = McpRegistry()
|
||||
|
|
@ -685,31 +718,9 @@ async def test_connect_cleans_up_when_cancelled_mid_connect(
|
|||
|
||||
|
||||
# --- reading a tool call back to the server it went out to -------------------
|
||||
# resolve_mcp_tool / namespaced_tool_name stay in strix.tools.mcp.naming: the
|
||||
# TUI reads them to attribute a call to its connection, and call_mcp builds the
|
||||
# result_transform label with namespaced_tool_name.
|
||||
|
||||
|
||||
def test_resolve_mcp_tool_splits_against_the_run_connections() -> None:
|
||||
assert resolve_mcp_tool("local_fs_read_file", ["github", "local_fs"]) == (
|
||||
"local_fs",
|
||||
"read_file",
|
||||
)
|
||||
|
||||
|
||||
def test_resolve_mcp_tool_prefers_the_longest_matching_connection() -> None:
|
||||
assert resolve_mcp_tool("files_main_list", ["files", "files_main"]) == ("files_main", "list")
|
||||
|
||||
|
||||
def test_resolve_mcp_tool_matches_a_connection_name_it_had_to_sanitize() -> None:
|
||||
tool_name = namespaced_tool_name("my server", "db.query")
|
||||
|
||||
assert resolve_mcp_tool(tool_name, ["my server"]) == ("my server", "db_query")
|
||||
|
||||
|
||||
def test_resolve_mcp_tool_ignores_tools_that_are_not_a_connection_s() -> None:
|
||||
assert resolve_mcp_tool("exec_command", ["local_fs"]) is None
|
||||
assert resolve_mcp_tool("local_fsx", ["local_fs"]) is None
|
||||
# namespaced_tool_name stays in strix.tools.mcp.naming so call_mcp can build the
|
||||
# result_transform label. The connection a call went out to is read off the
|
||||
# call's arguments by the TUI projection, not off the tool name.
|
||||
|
||||
|
||||
def test_namespaced_name_is_a_valid_tool_name() -> None:
|
||||
|
|
@ -721,13 +732,16 @@ def test_namespaced_name_is_a_valid_tool_name() -> None:
|
|||
assert re.fullmatch(r"[a-zA-Z0-9_-]{1,128}", name)
|
||||
|
||||
|
||||
def test_projected_tool_call_names_the_server_it_went_out_to() -> None:
|
||||
def test_projected_call_mcp_names_the_server_and_tool_from_its_args() -> None:
|
||||
view = TuiLiveView()
|
||||
view.set_mcp_connections(["local_fs"])
|
||||
|
||||
view._record_tool_call_data(
|
||||
"agent-1",
|
||||
{"call_id": "c1", "tool_name": "local_fs_read_file", "args": {"path": "/etc/hosts"}},
|
||||
{
|
||||
"call_id": "c1",
|
||||
"tool_name": "call_mcp",
|
||||
"args": {"connection": "local_fs", "tool": "read_file", "arguments": {"path": "/x"}},
|
||||
},
|
||||
)
|
||||
view._record_tool_call_data(
|
||||
"agent-1",
|
||||
|
|
@ -737,3 +751,18 @@ def test_projected_tool_call_names_the_server_it_went_out_to() -> None:
|
|||
mcp_call, built_in = (event["data"] for event in view.events)
|
||||
assert (mcp_call["mcp_connection"], mcp_call["mcp_tool"]) == ("local_fs", "read_file")
|
||||
assert "mcp_connection" not in built_in
|
||||
|
||||
|
||||
def test_projected_describe_mcp_names_the_connection_with_no_tool() -> None:
|
||||
view = TuiLiveView()
|
||||
|
||||
view._record_tool_call_data(
|
||||
"agent-1",
|
||||
{"call_id": "c1", "tool_name": "describe_mcp", "args": {"connection": "local_fs"}},
|
||||
)
|
||||
|
||||
(describe,) = (event["data"] for event in view.events)
|
||||
# An empty tool is what tells both renderers to present the row as inspecting
|
||||
# the connection rather than as a call to a tool on it.
|
||||
assert describe["mcp_connection"] == "local_fs"
|
||||
assert describe["mcp_tool"] == ""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue