From 07ce160a7e1231906d8396a4f7ca771e942cbd64 Mon Sep 17 00:00:00 2001 From: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> Date: Fri, 30 Oct 2026 09:14:42 +0530 Subject: [PATCH] Python: dispatch MCP approval modes by kind in the declarative tool The tool parsed approvalMode with the base class, which only carries the kind, so specify mode with its tool lists raised a TypeError and the simplified string parsed into the base class that the loader never matches. Parsing now dispatches by kind like the top level component path, and an unknown mode name is rejected. Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> --- .../agent_framework_declarative/_models.py | 10 ++++--- .../tests/test_declarative_models.py | 26 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/python/packages/declarative/agent_framework_declarative/_models.py b/python/packages/declarative/agent_framework_declarative/_models.py index d748dd65862..02b869334a9 100644 --- a/python/packages/declarative/agent_framework_declarative/_models.py +++ b/python/packages/declarative/agent_framework_declarative/_models.py @@ -806,11 +806,15 @@ def __init__( self.serverName = _try_powerfx_eval(serverName) self.serverDescription = _try_powerfx_eval(serverDescription) if not isinstance(approvalMode, McpServerApprovalMode) and approvalMode is not None: - # Handle simplified string format: "always" -> {"kind": "always"} + # Handle simplified string format: "always" -> {"kind": "always"}. + # Dispatch by kind so specify mode parses into its own subclass with + # the tool lists; the base class only carries the kind. if isinstance(approvalMode, str): - approvalMode = McpServerApprovalMode.from_dict({"kind": approvalMode}) + approvalMode = agent_schema_dispatch({"kind": approvalMode}) else: - approvalMode = McpServerApprovalMode.from_dict(approvalMode) + approvalMode = agent_schema_dispatch(approvalMode) + if not isinstance(approvalMode, McpServerApprovalMode): + raise ValueError(f"Unsupported approval mode: {approvalMode!r}") self.approvalMode = approvalMode self.allowedTools = allowedTools or [] self.url = _try_powerfx_eval(url) diff --git a/python/packages/declarative/tests/test_declarative_models.py b/python/packages/declarative/tests/test_declarative_models.py index 043fb372792..3462d33f077 100644 --- a/python/packages/declarative/tests/test_declarative_models.py +++ b/python/packages/declarative/tests/test_declarative_models.py @@ -765,6 +765,32 @@ def test_mcp_tool_from_dict(self): assert tool.kind == "mcp" assert isinstance(tool.approvalMode, McpServerApprovalMode) + def test_mcp_tool_specify_approval_mode_parses_with_tool_lists(self): + """The specify mode with its tool lists must parse into its own subclass.""" + tool = McpTool.from_dict({ + "name": "mcp_tool", + "kind": "mcp", + "serverName": "test-server", + "approvalMode": { + "kind": "specify", + "alwaysRequireApprovalTools": ["alpha"], + "neverRequireApprovalTools": ["beta"], + }, + }) + assert isinstance(tool.approvalMode, McpServerToolSpecifyApprovalMode) + assert tool.approvalMode.alwaysRequireApprovalTools == ["alpha"] + assert tool.approvalMode.neverRequireApprovalTools == ["beta"] + + def test_mcp_tool_simplified_specify_approval_mode_parses(self): + """The simplified string form must produce the specify subclass, not the base mode.""" + tool = McpTool.from_dict({"name": "mcp_tool", "kind": "mcp", "approvalMode": "specify"}) + assert isinstance(tool.approvalMode, McpServerToolSpecifyApprovalMode) + + def test_mcp_tool_rejects_an_unknown_approval_mode(self): + """An unknown approval mode name is rejected instead of parsing as the base mode.""" + with pytest.raises(ValueError, match="Unsupported approval mode"): + McpTool.from_dict({"name": "mcp_tool", "kind": "mcp", "approvalMode": "sometimes"}) + def test_mcp_tool_with_simplified_approval_mode(self): """Test McpTool with simplified string format for approvalMode.""" # Test simplified string format: approvalMode: "always"