Python: Fix MCP metadata and tool name handling

python/packages/core/AGENTS.md

@@ -80,8 +80,9 @@ agent_framework/
 
 - **`MCPTool`** - Base wrapper that owns the MCP `ClientSession` and exposes the remote server's tools as `FunctionTool`s.
 - **`MCPStdioTool`** / **`MCPStreamableHTTPTool`** / **`MCPWebsocketTool`** - Transport-specific subclasses.
-- **Argument allowlist (`_prepare_call_kwargs`)** - Before each `tools/call`, kwargs are filtered to an **allowlist** built from the tool's declared parameters (`inputSchema.properties`) plus any user-configured extras. Framework runtime kwargs injected through the function-invocation pipeline (e.g. `thread`, `conversation_id`, `chat_options`, `options`, `response_format`) are stripped by default rather than forwarded. A tool that declares no usable `properties` (including schemas with `additionalProperties: true`) forwards only the configured extras. The `_MCP_FRAMEWORK_DENYLIST` is a safety net for framework-named params a server *declares* in its schema (those are dropped); names explicitly opted in via `additional_tool_argument_names` always win. The reserved `_meta` key is extracted as MCP request metadata, never forwarded as an argument.
-- **`additional_tool_argument_names`** (constructor arg on all `MCPTool` subclasses) - Opt extra argument names back into the allowlist. Accepts a `Sequence[str]` (applied to every tool) or a `Mapping[str, Sequence[str]]` keyed by **remote tool name**, where the reserved key `"*"` denotes global extras. It is configured only in user code at construction; there is **no per-call/runtime override**, so a model-issued tool call cannot change which names pass through. To use a server that accepts `additionalProperties: true`, list the extra names here and then either (1) manually extend that tool's `inputSchema` (via the `.functions` list after connecting) so the model is prompted to supply them, or (2) supply the values yourself via `function_invocation_kwargs`. If a name is supplied by both the model and `function_invocation_kwargs`, the model-supplied value wins.
+- **Argument allowlist (`_prepare_call_kwargs`)** - Before each `tools/call`, kwargs are filtered to an **allowlist** built from the tool's declared parameters (`inputSchema.properties`) plus any user-configured extras. Framework runtime kwargs injected through the function-invocation pipeline (e.g. `thread`, `conversation_id`, `chat_options`, `options`, `response_format`) are stripped by default rather than forwarded. A tool that declares no usable `properties` (including schemas with `additionalProperties: true`) forwards only the configured extras. The `_MCP_FRAMEWORK_DENYLIST` is a safety net for framework-named params a server *declares* in its schema (those are dropped); names explicitly opted in via `additional_tool_argument_names` always win. The reserved `_meta` key is never forwarded as an argument; trusted caller/runtime `_meta` is validated as MCP request metadata, model-supplied `_meta` is discarded in generated MCP functions, and metadata precedence is caller/runtime < OpenTelemetry < tools/list metadata.
+- **`allowed_tools`** (constructor arg on all `MCPTool` subclasses) - Restricts exposed MCP tools by raw remote MCP tool identity. Prefixed local names remain accepted only when the raw remote name already matches its normalized form; normalized/local aliases do not authorize a different raw remote name. If multiple raw remote tool names map to the same local function name, tool loading raises `ToolExecutionException` instead of first-one-wins shadowing.
+- **`additional_tool_argument_names`** (constructor arg on all `MCPTool` subclasses) - Opt extra argument names back into the allowlist. Accepts a `Sequence[str]` (applied to every tool) or a `Mapping[str, Sequence[str]]` keyed by **remote tool name**, where the reserved key `"*"` denotes global extras. It is configured only in user code at construction; there is **no per-call/runtime override**, so a model-issued tool call cannot change which names pass through. To use a server that accepts `additionalProperties: true`, list the extra names here and then either (1) manually extend that tool's `inputSchema` (via the `.functions` list after connecting) so the model is prompted to supply them, or (2) supply the values yourself via `function_invocation_kwargs`. If a normal forwarded argument name is supplied by both the model and `function_invocation_kwargs`, the model-supplied value wins; `_meta` is the exception and only trusted runtime/caller metadata is used.
 - **Sampling guardrails** (`sampling_callback`) - Passing `client=` advertises `SamplingCapability` so the server can send `sampling/createMessage`. Because remote servers are untrusted (confused-deputy risk), the default `sampling_callback` is **deny-by-default** and applies, in order: a per-session rate limit (`sampling_max_requests`, default `_DEFAULT_SAMPLING_MAX_REQUESTS`), an approval gate (`sampling_approval_callback`), and a `maxTokens` cap (`sampling_max_tokens`, default `_DEFAULT_SAMPLING_MAX_TOKENS`). The approval callback (constructor arg on all subclasses; exported type alias `SamplingApprovalCallback`) receives the raw `CreateMessageRequestParams`, may be sync or async, and must return truthy to approve. When it is `None` (the default) every sampling request is denied; pass `lambda params: True` to restore legacy auto-approve as an explicit opt-in. Requests and denials are logged at WARNING (content is not logged). The per-session counter resets in `_reset_session_state`.
 - **`MCPTaskOptions`** (experimental, `MCP_LONG_RUNNING_TASKS` feature, **frozen**) - Per-tool-instance options controlling the SEP-2663 long-running task lifecycle. When the server advertises a tool with `execution.taskSupport == "required"`, `MCPTool.call_tool` transparently routes through `call_tool_as_task`, which sends an augmented `tools/call`, polls `tasks/get` until terminal, and reinterprets `tasks/result` as a normal `CallToolResult`. Instances are immutable; replace via `MCPTool.task_options = MCPTaskOptions(...)`. Fields:
   - `default_ttl: timedelta | None` — forwarded to the server as `params.task.ttl` (milliseconds). When `None`, the server's default applies.

python/packages/core/agent_framework/_mcp.py

@@ -74,6 +74,11 @@ class MCPSpecificApproval(TypedDict, total=False):
 # Reserved key in an ``additional_tool_argument_names`` mapping that applies its
 # values to every tool on the server rather than a single named tool.
 _MCP_GLOBAL_EXTRA_ARGS_KEY = "*"
+_MCP_META_LABEL_PATTERN = r"[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?"
+_MCP_META_KEY_PATTERN = re.compile(
+    rf"^(?:(?:{_MCP_META_LABEL_PATTERN})(?:\.{_MCP_META_LABEL_PATTERN})*/)?"
+    r"(?:[A-Za-z0-9](?:[A-Za-z0-9_.-]*[A-Za-z0-9])?)?$"
+)
 # Framework kwargs that flow through the function-invocation pipeline (via
 # ``FunctionInvocationContext.kwargs``) but must never be forwarded to an MCP
 # server: they are internal objects that the MCP SDK cannot serialize. They are
@@ -205,7 +210,42 @@ def _normalize_additional_tool_argument_names(
     return set(additional_tool_argument_names), {}
 
 
-def _inject_otel_into_mcp_meta(meta: dict[str, Any] | None = None) -> dict[str, Any] | None:
+def _mcp_config_candidate_names(*, local_name: str, normalized_name: str, remote_name: str) -> tuple[str, ...]:
+    """Return safe configuration names for MCP allow/approval matching."""
+    names = [remote_name]
+    if normalized_name == remote_name and local_name != remote_name:
+        names.append(local_name)
+    return tuple(names)
+
+
+def _validate_mcp_meta_key(key: str) -> None:
+    """Validate an MCP ``_meta`` key against the 2025-06-18 key-name format."""
+    if not _MCP_META_KEY_PATTERN.fullmatch(key):
+        raise ToolExecutionException(f"Invalid MCP _meta key name: {key!r}.")
+
+
+def _validate_mcp_meta(raw_meta: object | None) -> dict[str, Any] | None:
+    """Validate and copy MCP request metadata."""
+    if raw_meta is None:
+        return None
+    if not isinstance(raw_meta, dict):
+        raise ToolExecutionException("MCP tool metadata provided via _meta must be a dict.")
+
+    raw_meta_dict = cast(Mapping[object, Any], raw_meta)
+    meta: dict[str, Any] = {}
+    for key, value in raw_meta_dict.items():
+        if not isinstance(key, str):
+            raise ToolExecutionException("MCP tool metadata provided via _meta must use string keys.")
+        _validate_mcp_meta_key(key)
+        meta[key] = value
+    return meta
+
+
+def _inject_otel_into_mcp_meta(
+    meta: dict[str, Any] | None = None,
+    *,
+    overwrite: bool = False,
+) -> dict[str, Any] | None:
     """Inject OpenTelemetry trace context into MCP request _meta via the global propagator(s)."""
     carrier: dict[str, str] = {}
     propagate.inject(carrier)
@@ -215,7 +255,8 @@ def _inject_otel_into_mcp_meta(meta: dict[str, Any] | None = None) -> dict[str,
     if meta is None:
         meta = {}
     for key, value in carrier.items():
-        if key not in meta:
+        _validate_mcp_meta_key(key)
+        if overwrite or key not in meta:
             meta[key] = value
 
     return meta
@@ -381,7 +422,9 @@ def __init__(
             approval_mode: Whether approval is required to run tools.
             allowed_tools: Optional allow-list of MCP tool names to expose as functions.
                 ``None`` (the default) exposes every tool advertised by the MCP server.
-                A non-empty collection exposes only the tools whose names appear in it.
+                A non-empty collection exposes only the raw remote tools whose names appear in it. For
+                compatibility, the prefixed local function name is also accepted when the raw remote name already
+                matches its normalized form; normalized aliases do not authorize a different raw remote tool.
                 An empty collection (``[]``) exposes no tools — if you simply want to
                 disable tool execution, prefer ``load_tools=False`` instead. ``[]`` is
                 useful as a runtime guard or when you want to load tool metadata for
@@ -753,11 +796,14 @@ def functions(self) -> list[FunctionTool]:
             additional_properties = func.additional_properties or {}
             normalized_name = additional_properties.get(_MCP_NORMALIZED_NAME_KEY)
             remote_name = additional_properties.get(_MCP_REMOTE_NAME_KEY)
-            if (
-                func.name in allowed_names
-                or (isinstance(normalized_name, str) and normalized_name in allowed_names)
-                or (isinstance(remote_name, str) and remote_name in allowed_names)
-            ):
+            if not isinstance(normalized_name, str) or not isinstance(remote_name, str):
+                continue
+            candidate_names = _mcp_config_candidate_names(
+                local_name=func.name,
+                normalized_name=normalized_name,
+                remote_name=remote_name,
+            )
+            if any(name in allowed_names for name in candidate_names):
                 filtered_functions.append(func)
         return filtered_functions
 
@@ -1381,7 +1427,13 @@ async def _load_prompts_locked(self) -> None:
                     continue
 
                 input_model = _get_input_model_from_mcp_prompt(prompt)
-                approval_mode = self._determine_approval_mode(local_name, normalized_name, prompt.name)
+                approval_mode = self._determine_approval_mode(
+                    *_mcp_config_candidate_names(
+                        local_name=local_name,
+                        normalized_name=normalized_name,
+                        remote_name=prompt.name,
+                    )
+                )
                 func: FunctionTool = FunctionTool(
                     func=partial(self.get_prompt, prompt.name),
                     name=local_name,
@@ -1422,7 +1474,11 @@ async def _load_tools_locked(self) -> None:
             return
 
         # Track existing function names to prevent duplicates
-        existing_names = {func.name for func in self._functions}
+        existing_remote_by_local: dict[str, str] = {}
+        for func in self._functions:
+            remote_name = (func.additional_properties or {}).get(_MCP_REMOTE_NAME_KEY)
+            if isinstance(remote_name, str):
+                existing_remote_by_local[func.name] = remote_name
         tool_call_meta_by_name: dict[str, dict[str, Any]] = {}
         tool_task_support_by_name: dict[str, str] = {}
         tool_param_names_by_name: dict[str, set[str]] = {}
@@ -1462,7 +1518,7 @@ async def _load_tools_locked(self) -> None:
 
             for tool in tool_list.tools:
                 if tool.meta is not None:
-                    tool_call_meta_by_name[tool.name] = dict(tool.meta)
+                    tool_call_meta_by_name[tool.name] = _validate_mcp_meta(tool.meta) or {}
 
                 task_support = getattr(getattr(tool, "execution", None), "taskSupport", None)
                 if task_support is not None:
@@ -1490,19 +1546,38 @@ async def _load_tools_locked(self) -> None:
                 local_name = _build_prefixed_mcp_name(normalized_name, self.tool_name_prefix)
 
                 # Skip if already loaded
-                if local_name in existing_names:
+                if local_name in existing_remote_by_local:
+                    if existing_remote_by_local.get(local_name) != tool.name:
+                        raise ToolExecutionException(
+                            "MCP server advertised multiple tools that map to the same local function name: "
+                            f"{existing_remote_by_local[local_name]!r} and {tool.name!r} both map to "
+                            f"{local_name!r}."
+                        )
                     continue
 
-                approval_mode = self._determine_approval_mode(local_name, normalized_name, tool.name)
+                existing_remote_by_local[local_name] = tool.name
+
+                approval_mode = self._determine_approval_mode(
+                    *_mcp_config_candidate_names(
+                        local_name=local_name,
+                        normalized_name=normalized_name,
+                        remote_name=tool.name,
+                    )
+                )
 
                 async def _call_tool_with_runtime_kwargs(
                     ctx: FunctionInvocationContext,
                     *,
                     _remote_tool_name: str = tool.name,
                     **kwargs: Any,
                 ) -> str | list[Content]:
+                    trusted_meta = ctx.kwargs.get("_meta")
                     call_kwargs = dict(ctx.kwargs)
                     call_kwargs.update(kwargs)
+                    if trusted_meta is not None:
+                        call_kwargs["_meta"] = trusted_meta
+                    else:
+                        call_kwargs.pop("_meta", None)
                     return await self.call_tool(_remote_tool_name, **call_kwargs)
 
                 # Create FunctionTools out of each tool
@@ -1518,7 +1593,6 @@ async def _call_tool_with_runtime_kwargs(
                     },
                 )
                 self._functions.append(func)
-                existing_names.add(local_name)
 
             # Check if there are more pages
             if not tool_list.nextCursor:
@@ -1636,8 +1710,8 @@ async def call_tool(self, tool_name: str, **kwargs: Any) -> str | list[Content]:
         Keyword Args:
             _meta: Optional ``dict[str, Any]`` of MCP request metadata. This reserved key is passed as the
                 ``meta`` parameter of the underlying ``session.call_tool`` call rather than as a tool argument.
-                User-supplied keys override metadata from ``tools/list``; OpenTelemetry propagation fills in
-                non-conflicting keys.
+                OpenTelemetry propagation overrides caller-supplied keys, and metadata from ``tools/list``
+                overrides both.
             kwargs: Remaining arguments to pass to the tool.
 
         Returns:
@@ -1746,17 +1820,7 @@ def _prepare_call_kwargs(
         self, tool_name: str, kwargs: dict[str, Any]
     ) -> tuple[dict[str, Any], dict[str, Any] | None]:
         """Filter kwargs down to the tool's arguments and build the merged MCP request metadata."""
-        raw_user_meta: object | None = kwargs.get("_meta")
-        user_meta: dict[str, Any] | None = None
-        if raw_user_meta is not None and not isinstance(raw_user_meta, dict):
-            raise ToolExecutionException("MCP tool metadata provided via _meta must be a dict.")
-        if isinstance(raw_user_meta, dict):
-            raw_user_meta_dict = cast(Mapping[object, object], raw_user_meta)
-            user_meta = {}
-            for key, value in raw_user_meta_dict.items():
-                if not isinstance(key, str):
-                    raise ToolExecutionException("MCP tool metadata provided via _meta must use string keys.")
-                user_meta[key] = value
+        user_meta = _validate_mcp_meta(kwargs.get("_meta"))
 
         # Allowlist: forward only the tool's declared parameters (from inputSchema.properties)
         # plus any user-configured extra argument names. Everything else - notably the
@@ -1783,12 +1847,12 @@ def _prepare_call_kwargs(
         }
 
         # Some MCP proxies require their tools/list metadata to be echoed on tools/call.
-        tool_meta = self._tool_call_meta_by_name.get(tool_name)
-        request_meta = dict(tool_meta) if tool_meta is not None else None
-        if user_meta is not None:
-            request_meta = {**(request_meta or {}), **user_meta}
-        meta = _inject_otel_into_mcp_meta(request_meta)
-        return filtered_kwargs, meta
+        request_meta = dict(user_meta) if user_meta is not None else None
+        request_meta = _inject_otel_into_mcp_meta(request_meta, overwrite=True)
+        tool_meta = _validate_mcp_meta(self._tool_call_meta_by_name.get(tool_name))
+        if tool_meta is not None:
+            request_meta = {**(request_meta or {}), **tool_meta}
+        return filtered_kwargs, request_meta
 
     async def call_tool_as_task(self, tool_name: str, **kwargs: Any) -> str | list[Content]:
         """Call an MCP tool via the long-running task lifecycle (SEP-2663).