From 727db8c21a6a2bb0f791e25f6b42beab5a92126d Mon Sep 17 00:00:00 2001 From: Bionic711 Date: Fri, 7 Aug 2026 08:41:06 -0500 Subject: [PATCH 1/2] Normalize outbound MCP tool arguments Unwrap schema-aware kwargs wrappers before outbound MCP validation and invocation so standards-compliant MCP servers receive required parameters at the top level. Add regression coverage for plugin and factory call paths while preserving legitimate kwargs fields. Fixes #1163 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- .../single_app/functions_mcp_operations.py | 27 ++ .../semantic_kernel_plugins/mcp_plugin.py | 19 +- .../mcp_plugin_factory.py | 28 +- docs/explanation/release_notes.md | 9 + .../test_mcp_tool_argument_normalization.py | 297 ++++++++++++++++++ 6 files changed, 373 insertions(+), 9 deletions(-) create mode 100644 functional_tests/test_mcp_tool_argument_normalization.py diff --git a/application/single_app/config.py b/application/single_app/config.py index ffb5634e..fe8f93be 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -96,7 +96,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.250.126" +VERSION = "0.250.127" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/single_app/functions_mcp_operations.py b/application/single_app/functions_mcp_operations.py index 4d21a77d..60d5e9f2 100644 --- a/application/single_app/functions_mcp_operations.py +++ b/application/single_app/functions_mcp_operations.py @@ -477,6 +477,33 @@ def build_mcp_tool_metadata_warnings(tools, additional_fields=None): return warnings[:20] +def _mcp_tool_schema_defines_property(tool, property_name): + if not isinstance(tool, dict): + return False + + input_schema = tool.get("input_schema") if isinstance(tool.get("input_schema"), dict) else {} + properties = input_schema.get("properties") + return isinstance(properties, dict) and property_name in properties + + +def normalize_mcp_tool_call_arguments(tool, arguments): + """Return MCP tool arguments in the top-level shape expected by tools/call.""" + if arguments is None: + return {} + if not isinstance(arguments, dict): + return arguments + + wrapper_value = arguments.get("kwargs") + if ( + set(arguments.keys()) == {"kwargs"} + and isinstance(wrapper_value, dict) + and not _mcp_tool_schema_defines_property(tool, "kwargs") + ): + return wrapper_value + + return arguments + + def validate_mcp_tool_arguments(tool, arguments): """Return validation errors for MCP tool arguments against cached input schema.""" if not isinstance(tool, dict): diff --git a/application/single_app/semantic_kernel_plugins/mcp_plugin.py b/application/single_app/semantic_kernel_plugins/mcp_plugin.py index 5f6ededc..52f64efa 100644 --- a/application/single_app/semantic_kernel_plugins/mcp_plugin.py +++ b/application/single_app/semantic_kernel_plugins/mcp_plugin.py @@ -13,6 +13,7 @@ classify_mcp_exception, normalize_mcp_additional_fields, normalize_mcp_tool_metadata, + normalize_mcp_tool_call_arguments, validate_mcp_tool_arguments, ) from semantic_kernel_plugins.base_plugin import BasePlugin @@ -157,12 +158,16 @@ async def call_tool(self, tool_name: str, arguments: Optional[dict] = None) -> d "error_type": "not_configured", "configured_tools": sorted(configured_tool_names), } + configured_tool = next( + (tool for tool in self._tools if tool.get("original_name") == normalized_tool_name), + None, + ) + normalized_arguments = normalize_mcp_tool_call_arguments( + configured_tool, + arguments if arguments is not None else {}, + ) if self._additional_fields.get("validate_tool_arguments"): - configured_tool = next( - (tool for tool in self._tools if tool.get("original_name") == normalized_tool_name), - None, - ) - validation_errors = validate_mcp_tool_arguments(configured_tool, arguments or {}) + validation_errors = validate_mcp_tool_arguments(configured_tool, normalized_arguments) if validation_errors: return { "success": False, @@ -171,7 +176,7 @@ async def call_tool(self, tool_name: str, arguments: Optional[dict] = None) -> d "validation_errors": validation_errors, } - return await self.invoke_tool(normalized_tool_name, arguments or {}) + return await self.invoke_tool(normalized_tool_name, normalized_arguments or {}) async def invoke_tool(self, tool_name: str, arguments: Optional[dict] = None) -> dict: """Invoke an MCP tool through the factory's native MCP connector.""" @@ -180,7 +185,7 @@ async def invoke_tool(self, tool_name: str, arguments: Optional[dict] = None) -> f"[MCP_PLUGIN] Invoking MCP tool tool_name={tool_name} " f"transport={self._additional_fields.get('transport')} " f"endpoint_present={bool(str(self.manifest.get('endpoint') or '').strip())} " - f"argument_keys={sorted((arguments or {}).keys())}" + f"argument_keys={sorted(arguments.keys()) if isinstance(arguments, dict) else []}" ) from semantic_kernel_plugins.mcp_plugin_factory import McpPluginFactory diff --git a/application/single_app/semantic_kernel_plugins/mcp_plugin_factory.py b/application/single_app/semantic_kernel_plugins/mcp_plugin_factory.py index 344c587c..00a74db0 100644 --- a/application/single_app/semantic_kernel_plugins/mcp_plugin_factory.py +++ b/application/single_app/semantic_kernel_plugins/mcp_plugin_factory.py @@ -26,6 +26,7 @@ get_mcp_custom_header_validation_errors, is_valid_mcp_header_name, normalize_mcp_additional_fields, + normalize_mcp_tool_call_arguments, normalize_mcp_tool_metadata, validate_mcp_endpoint_for_transport, ) @@ -249,10 +250,35 @@ async def call_tool_from_config( arguments: Optional[Dict[str, Any]] = None, ) -> Dict[str, Any]: """Connect to an MCP server, invoke one tool, and normalize the result.""" + configured_tool = cls._find_configured_tool(config, tool_name) + normalized_arguments = ( + normalize_mcp_tool_call_arguments(configured_tool, arguments) + if configured_tool + else arguments + ) return await cls._run_with_retries( config, "tool_call", - lambda: cls._call_tool_once(config, tool_name, arguments), + lambda: cls._call_tool_once(config, tool_name, normalized_arguments), + ) + + @classmethod + def _find_configured_tool(cls, config: Dict[str, Any], tool_name: str) -> Optional[Dict[str, Any]]: + """Return cached MCP tool metadata for a factory invocation.""" + normalized_tool_name = str(tool_name or "").strip() + if not normalized_tool_name: + return None + + additional_fields = normalize_mcp_additional_fields((config or {}).get("additionalFields", {})) + configured_tools = normalize_mcp_tool_metadata(additional_fields.get("mcp_tools", [])) + return next( + ( + tool + for tool in configured_tools + if tool.get("original_name") == normalized_tool_name + or tool.get("function_name") == normalized_tool_name + ), + None, ) @classmethod diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 4085165f..89c60868 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,15 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/). +### **(v0.250.127)** + +#### Bug Fixes + +* **Outbound MCP Tool Argument Normalization** + * Fixed outbound MCP tool calls that could wrap parameters inside a `kwargs` object, preventing standards-compliant MCP servers from seeing required top-level fields such as `type`. + * Added schema-aware normalization before MCP argument validation and invocation while preserving tools that explicitly define a real top-level `kwargs` property. + * (Ref: #1163, MCP `tools/call` arguments, `functions_mcp_operations.py`, `mcp_plugin.py`, `mcp_plugin_factory.py`) + ### **(v0.250.126)** #### Bug Fixes diff --git a/functional_tests/test_mcp_tool_argument_normalization.py b/functional_tests/test_mcp_tool_argument_normalization.py new file mode 100644 index 00000000..1d9c3661 --- /dev/null +++ b/functional_tests/test_mcp_tool_argument_normalization.py @@ -0,0 +1,297 @@ +# test_mcp_tool_argument_normalization.py +#!/usr/bin/env python3 +""" +Functional test for outbound MCP tool argument normalization. +Version: 0.250.127 +Implemented in: 0.250.127 + +This test ensures wrapped Semantic Kernel kwargs are normalized before outbound +MCP tool validation and invocation, while legitimate kwargs tool fields remain +unchanged. +""" + +import asyncio +import sys +import types +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[1] +APP_DIR = REPO_ROOT / "application" / "single_app" +sys.path.insert(0, str(APP_DIR)) +sys.path.insert(0, str(REPO_ROOT / "functional_tests")) + + +def _noop(*_args, **_kwargs): + return None + + +class _NoopLogger: + def __getattr__(self, _name): + return _noop + + +class _KernelPlugin: + def __init__(self, functions): + self.functions = functions + + @classmethod + def from_object(cls, _plugin_name, functions, description=None): + return cls(functions) + + +def _kernel_function(*_args, **_kwargs): + def decorator(func): + return func + + return decorator + + +sys.modules.setdefault( + "functions_appinsights", + types.SimpleNamespace( + log_event=_noop, + get_appinsights_logger=lambda: _NoopLogger(), + ), +) +sys.modules.setdefault( + "functions_authentication", + types.SimpleNamespace(get_current_user_id=lambda: "functional-test-user"), +) +sys.modules.setdefault( + "functions_debug", + types.SimpleNamespace(debug_print=lambda *args, **kwargs: None), +) +sys.modules.setdefault("semantic_kernel", types.SimpleNamespace()) +sys.modules.setdefault( + "semantic_kernel.functions", + types.SimpleNamespace(kernel_function=_kernel_function), +) +sys.modules.setdefault( + "semantic_kernel.functions.kernel_plugin", + types.SimpleNamespace(KernelPlugin=_KernelPlugin), +) +sys.modules.setdefault( + "semantic_kernel.connectors", + types.SimpleNamespace(), +) +sys.modules.setdefault( + "semantic_kernel.connectors.mcp", + types.SimpleNamespace( + MCPSsePlugin=object, + MCPStdioPlugin=object, + MCPStreamableHttpPlugin=object, + MCPWebsocketPlugin=object, + ), +) + +from functions_mcp_operations import ( # noqa: E402 + MCP_PLUGIN_TYPE, + normalize_mcp_tool_call_arguments, + validate_mcp_tool_arguments, +) +from semantic_kernel_plugins.mcp_plugin import McpPlugin # noqa: E402 +from semantic_kernel_plugins.mcp_plugin_factory import McpPluginFactory # noqa: E402 +from test_support.versioning import assert_app_version_at_least # noqa: E402 + + +def _splunk_type_tool(): + return { + "original_name": "get-splunk-objects", + "function_name": "get_splunk_objects", + "description": "Get Splunk objects by type.", + "input_schema": { + "type": "object", + "properties": { + "type": {"type": "string"}, + "count": {"type": "integer"}, + }, + "required": ["type"], + }, + } + + +def _mcp_manifest(tool, validate_arguments=True): + return { + "name": "splunk_mcp", + "type": MCP_PLUGIN_TYPE, + "endpoint": "https://splunk.example.com/mcp", + "auth": {"type": "NoAuth"}, + "additionalFields": { + "transport": "streamable_http", + "auth_method": "none", + "server_profile": "splunk", + "validate_tool_arguments": validate_arguments, + "mcp_tools": [tool], + }, + } + + +def test_wrapped_kwargs_arguments_are_unwrapped_for_required_schema(): + """Validate the Splunk-style kwargs wrapper becomes top-level MCP arguments.""" + tool = _splunk_type_tool() + wrapped_arguments = {"kwargs": {"type": "savedsearch", "count": 25}} + + wrapped_validation_errors = validate_mcp_tool_arguments(tool, wrapped_arguments) + assert any("'type' is a required property" in error for error in wrapped_validation_errors) + + normalized_arguments = normalize_mcp_tool_call_arguments(tool, wrapped_arguments) + + assert normalized_arguments == {"type": "savedsearch", "count": 25} + assert validate_mcp_tool_arguments(tool, normalized_arguments) == [] + + +def test_direct_arguments_are_preserved(): + """Validate already-correct MCP arguments are left untouched.""" + tool = _splunk_type_tool() + direct_arguments = {"type": "savedsearch"} + + normalized_arguments = normalize_mcp_tool_call_arguments(tool, direct_arguments) + + assert normalized_arguments == direct_arguments + + +def test_legitimate_kwargs_tool_property_is_preserved(): + """Validate tools that define a real kwargs field do not get unwrapped.""" + tool = { + "original_name": "kwargs-tool", + "function_name": "kwargs_tool", + "input_schema": { + "type": "object", + "properties": { + "kwargs": { + "type": "object", + "properties": {"type": {"type": "string"}}, + }, + }, + "required": ["kwargs"], + }, + } + arguments = {"kwargs": {"type": "intended-field"}} + + normalized_arguments = normalize_mcp_tool_call_arguments(tool, arguments) + + assert normalized_arguments == arguments + assert validate_mcp_tool_arguments(tool, normalized_arguments) == [] + + +def test_none_arguments_normalize_to_empty_object(): + """Validate no-parameter MCP tool calls keep the standard empty object shape.""" + assert normalize_mcp_tool_call_arguments({}, None) == {} + + +def test_mcp_plugin_call_tool_normalizes_before_validation_and_invocation(): + """Validate McpPlugin.call_tool forwards normalized args to invoke_tool.""" + plugin = McpPlugin(_mcp_manifest(_splunk_type_tool())) + captured_call = {} + + async def fake_invoke_tool(tool_name, arguments=None): + captured_call["tool_name"] = tool_name + captured_call["arguments"] = arguments + return {"success": True, "received_arguments": arguments} + + plugin.invoke_tool = fake_invoke_tool + result = asyncio.run( + plugin.call_tool( + "get-splunk-objects", + {"kwargs": {"type": "savedsearch", "count": 25}}, + ) + ) + + assert result["success"] is True + assert captured_call["tool_name"] == "get-splunk-objects" + assert captured_call["arguments"] == {"type": "savedsearch", "count": 25} + + +def test_factory_call_tool_normalizes_cached_tool_arguments(): + """Validate direct factory callers also receive top-level MCP arguments.""" + captured_call = {} + + async def fake_run_with_retries(cls, _config, _operation, operation_factory): + return await operation_factory() + + async def fake_call_tool_once(cls, _config, tool_name, arguments=None): + captured_call["tool_name"] = tool_name + captured_call["arguments"] = arguments + return {"success": True, "received_arguments": arguments} + + original_run_with_retries = McpPluginFactory.__dict__["_run_with_retries"] + original_call_tool_once = McpPluginFactory.__dict__["_call_tool_once"] + McpPluginFactory._run_with_retries = classmethod(fake_run_with_retries) + McpPluginFactory._call_tool_once = classmethod(fake_call_tool_once) + try: + result = asyncio.run( + McpPluginFactory.call_tool_from_config( + _mcp_manifest(_splunk_type_tool()), + "get-splunk-objects", + {"kwargs": {"type": "savedsearch", "count": 25}}, + ) + ) + finally: + McpPluginFactory._run_with_retries = original_run_with_retries + McpPluginFactory._call_tool_once = original_call_tool_once + + assert result["success"] is True + assert captured_call["tool_name"] == "get-splunk-objects" + assert captured_call["arguments"] == {"type": "savedsearch", "count": 25} + + +def test_factory_preserves_wrapper_without_cached_tool_metadata(): + """Validate factory normalization stays conservative without schema metadata.""" + captured_call = {} + manifest = _mcp_manifest(_splunk_type_tool()) + manifest["additionalFields"]["mcp_tools"] = [] + wrapped_arguments = {"kwargs": {"type": "intended-wrapper"}} + + async def fake_run_with_retries(cls, _config, _operation, operation_factory): + return await operation_factory() + + async def fake_call_tool_once(cls, _config, tool_name, arguments=None): + captured_call["tool_name"] = tool_name + captured_call["arguments"] = arguments + return {"success": True, "received_arguments": arguments} + + original_run_with_retries = McpPluginFactory.__dict__["_run_with_retries"] + original_call_tool_once = McpPluginFactory.__dict__["_call_tool_once"] + McpPluginFactory._run_with_retries = classmethod(fake_run_with_retries) + McpPluginFactory._call_tool_once = classmethod(fake_call_tool_once) + try: + result = asyncio.run( + McpPluginFactory.call_tool_from_config( + manifest, + "unknown-tool", + wrapped_arguments, + ) + ) + finally: + McpPluginFactory._run_with_retries = original_run_with_retries + McpPluginFactory._call_tool_once = original_call_tool_once + + assert result["success"] is True + assert captured_call["tool_name"] == "unknown-tool" + assert captured_call["arguments"] == wrapped_arguments + + +def main(): + """Run tests as a standalone functional test script.""" + assert_app_version_at_least("0.250.127") + tests = [ + test_wrapped_kwargs_arguments_are_unwrapped_for_required_schema, + test_direct_arguments_are_preserved, + test_legitimate_kwargs_tool_property_is_preserved, + test_none_arguments_normalize_to_empty_object, + test_mcp_plugin_call_tool_normalizes_before_validation_and_invocation, + test_factory_call_tool_normalizes_cached_tool_arguments, + test_factory_preserves_wrapper_without_cached_tool_metadata, + ] + for test in tests: + try: + test() + except Exception as ex: + print(f"{test.__name__} failed: {ex}") + raise + print("MCP tool argument normalization tests passed.") + + +if __name__ == "__main__": + main() From 21a080528c404531d3a0dcc768df4134bbdf087b Mon Sep 17 00:00:00 2001 From: Bionic711 Date: Fri, 7 Aug 2026 16:09:57 -0500 Subject: [PATCH 2/2] Address MCP PR CodeQL test cleanup Replace the unnecessary App Insights logger lambda in the MCP argument normalization functional test with the named no-op logger helper pattern used by nearby MCP tests. Update branch version metadata for the follow-up change. Refs #1163 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- docs/explanation/release_notes.md | 2 +- .../test_mcp_tool_argument_normalization.py | 15 +++++++++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/application/single_app/config.py b/application/single_app/config.py index fe8f93be..44bdd95e 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -96,7 +96,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.250.127" +VERSION = "0.250.128" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 89c60868..d7799a43 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,7 +2,7 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/). -### **(v0.250.127)** +### **(v0.250.128)** #### Bug Fixes diff --git a/functional_tests/test_mcp_tool_argument_normalization.py b/functional_tests/test_mcp_tool_argument_normalization.py index 1d9c3661..8a5cee20 100644 --- a/functional_tests/test_mcp_tool_argument_normalization.py +++ b/functional_tests/test_mcp_tool_argument_normalization.py @@ -2,8 +2,8 @@ #!/usr/bin/env python3 """ Functional test for outbound MCP tool argument normalization. -Version: 0.250.127 -Implemented in: 0.250.127 +Version: 0.250.128 +Implemented in: 0.250.128 This test ensures wrapped Semantic Kernel kwargs are normalized before outbound MCP tool validation and invocation, while legitimate kwargs tool fields remain @@ -31,6 +31,13 @@ def __getattr__(self, _name): return _noop +_NOOP_LOGGER = _NoopLogger() + + +def _get_noop_logger(): + return _NOOP_LOGGER + + class _KernelPlugin: def __init__(self, functions): self.functions = functions @@ -51,7 +58,7 @@ def decorator(func): "functions_appinsights", types.SimpleNamespace( log_event=_noop, - get_appinsights_logger=lambda: _NoopLogger(), + get_appinsights_logger=_get_noop_logger, ), ) sys.modules.setdefault( @@ -274,7 +281,7 @@ async def fake_call_tool_once(cls, _config, tool_name, arguments=None): def main(): """Run tests as a standalone functional test script.""" - assert_app_version_at_least("0.250.127") + assert_app_version_at_least("0.250.128") tests = [ test_wrapped_kwargs_arguments_are_unwrapped_for_required_schema, test_direct_arguments_are_preserved,