mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: [BREAKING] Replace Hosted*Tool classes with tool methods (#3634)
* Replace Hosted*Tool classes with client static factory methods * fixed failing test * mypy fix * mypy fix 2 * declarative mypy fix * addressed comments * ToolProtocol removal * fixed test * agents mypy fix * fix failing tests * mypy fix * addressed comments * fixed tests * addressed comments + added factory method overrides for azureai v2 client * mypy fix * added kwargs to azureai tool methods * fixed in test * _sessions fix * test fix
This commit is contained in:
committed by
GitHub
Unverified
parent
d249473a6d
commit
7a88af0aef
@@ -5,19 +5,12 @@ from __future__ import annotations
|
||||
import sys
|
||||
from collections.abc import Callable, Mapping
|
||||
from pathlib import Path
|
||||
from typing import Any, Literal, cast
|
||||
from typing import Any, cast
|
||||
|
||||
import yaml
|
||||
from agent_framework import (
|
||||
Agent,
|
||||
Content,
|
||||
HostedCodeInterpreterTool,
|
||||
HostedFileSearchTool,
|
||||
HostedMCPSpecificApproval,
|
||||
HostedMCPTool,
|
||||
HostedWebSearchTool,
|
||||
SupportsChatGetResponse,
|
||||
ToolProtocol,
|
||||
)
|
||||
from agent_framework import (
|
||||
FunctionTool as AFFunctionTool,
|
||||
@@ -714,14 +707,14 @@ class AgentFactory:
|
||||
chat_options["additional_chat_options"] = options.additionalProperties
|
||||
return chat_options
|
||||
|
||||
def _parse_tools(self, tools: list[Tool] | None) -> list[ToolProtocol] | None:
|
||||
"""Parse tool resources into ToolProtocol instances."""
|
||||
def _parse_tools(self, tools: list[Tool] | None) -> list[AFFunctionTool | dict[str, Any]] | None:
|
||||
"""Parse tool resources into AFFunctionTool instances or dict-based tools."""
|
||||
if not tools:
|
||||
return None
|
||||
return [self._parse_tool(tool_resource) for tool_resource in tools]
|
||||
|
||||
def _parse_tool(self, tool_resource: Tool) -> ToolProtocol:
|
||||
"""Parse a single tool resource into a ToolProtocol instance."""
|
||||
def _parse_tool(self, tool_resource: Tool) -> AFFunctionTool | dict[str, Any]:
|
||||
"""Parse a single tool resource into an AFFunctionTool instance."""
|
||||
match tool_resource:
|
||||
case FunctionTool():
|
||||
func: Callable[..., Any] | None = None
|
||||
@@ -736,88 +729,81 @@ class AgentFactory:
|
||||
func=func,
|
||||
)
|
||||
case WebSearchTool():
|
||||
return HostedWebSearchTool(
|
||||
description=tool_resource.description, additional_properties=tool_resource.options
|
||||
)
|
||||
result: dict[str, Any] = {"type": "web_search_preview"}
|
||||
if tool_resource.description:
|
||||
result["description"] = tool_resource.description
|
||||
if tool_resource.options:
|
||||
result.update(tool_resource.options)
|
||||
return result
|
||||
case FileSearchTool():
|
||||
add_props: dict[str, Any] = {}
|
||||
result = {
|
||||
"type": "file_search",
|
||||
"vector_store_ids": tool_resource.vectorStoreIds or [],
|
||||
}
|
||||
if tool_resource.maximumResultCount is not None:
|
||||
result["max_num_results"] = tool_resource.maximumResultCount
|
||||
if tool_resource.description:
|
||||
result["description"] = tool_resource.description
|
||||
if tool_resource.ranker is not None:
|
||||
add_props["ranker"] = tool_resource.ranker
|
||||
result["ranker"] = tool_resource.ranker
|
||||
if tool_resource.scoreThreshold is not None:
|
||||
add_props["score_threshold"] = tool_resource.scoreThreshold
|
||||
result["score_threshold"] = tool_resource.scoreThreshold
|
||||
if tool_resource.filters:
|
||||
add_props["filters"] = tool_resource.filters
|
||||
return HostedFileSearchTool(
|
||||
inputs=[Content.from_hosted_vector_store(id) for id in tool_resource.vectorStoreIds or []],
|
||||
description=tool_resource.description,
|
||||
max_results=tool_resource.maximumResultCount,
|
||||
additional_properties=add_props,
|
||||
)
|
||||
result["filters"] = tool_resource.filters
|
||||
return result
|
||||
case CodeInterpreterTool():
|
||||
return HostedCodeInterpreterTool(
|
||||
inputs=[Content.from_hosted_file(file_id=file) for file in tool_resource.fileIds or []],
|
||||
description=tool_resource.description,
|
||||
)
|
||||
result = {"type": "code_interpreter"}
|
||||
if tool_resource.fileIds:
|
||||
result["file_ids"] = tool_resource.fileIds
|
||||
if tool_resource.description:
|
||||
result["description"] = tool_resource.description
|
||||
return result
|
||||
case McpTool():
|
||||
approval_mode: HostedMCPSpecificApproval | Literal["always_require", "never_require"] | None = None
|
||||
result = {
|
||||
"type": "mcp",
|
||||
"server_label": tool_resource.name.replace(" ", "_") if tool_resource.name else "",
|
||||
"server_url": str(tool_resource.url) if tool_resource.url else "",
|
||||
}
|
||||
if tool_resource.description:
|
||||
result["server_description"] = tool_resource.description
|
||||
if tool_resource.allowedTools:
|
||||
result["allowed_tools"] = list(tool_resource.allowedTools)
|
||||
|
||||
# Handle approval mode
|
||||
if tool_resource.approvalMode is not None:
|
||||
if tool_resource.approvalMode.kind == "always":
|
||||
approval_mode = "always_require"
|
||||
result["require_approval"] = "always"
|
||||
elif tool_resource.approvalMode.kind == "never":
|
||||
approval_mode = "never_require"
|
||||
result["require_approval"] = "never"
|
||||
elif isinstance(tool_resource.approvalMode, McpServerToolSpecifyApprovalMode):
|
||||
approval_mode = {}
|
||||
approval_config: dict[str, Any] = {}
|
||||
if tool_resource.approvalMode.alwaysRequireApprovalTools:
|
||||
approval_mode["always_require_approval"] = (
|
||||
tool_resource.approvalMode.alwaysRequireApprovalTools
|
||||
)
|
||||
approval_config["always"] = {
|
||||
"tool_names": list(tool_resource.approvalMode.alwaysRequireApprovalTools)
|
||||
}
|
||||
if tool_resource.approvalMode.neverRequireApprovalTools:
|
||||
approval_mode["never_require_approval"] = (
|
||||
tool_resource.approvalMode.neverRequireApprovalTools
|
||||
)
|
||||
if not approval_mode:
|
||||
approval_mode = None
|
||||
approval_config["never"] = {
|
||||
"tool_names": list(tool_resource.approvalMode.neverRequireApprovalTools)
|
||||
}
|
||||
if approval_config:
|
||||
result["require_approval"] = approval_config
|
||||
|
||||
# Handle connection settings
|
||||
headers: dict[str, str] | None = None
|
||||
additional_properties: dict[str, Any] | None = None
|
||||
|
||||
if tool_resource.connection is not None:
|
||||
match tool_resource.connection:
|
||||
case ApiKeyConnection():
|
||||
if tool_resource.connection.apiKey:
|
||||
headers = {"Authorization": f"Bearer {tool_resource.connection.apiKey}"}
|
||||
result["headers"] = {"Authorization": f"Bearer {tool_resource.connection.apiKey}"}
|
||||
case RemoteConnection():
|
||||
additional_properties = {
|
||||
"connection": {
|
||||
"kind": tool_resource.connection.kind,
|
||||
"name": tool_resource.connection.name,
|
||||
"authenticationMode": tool_resource.connection.authenticationMode,
|
||||
"endpoint": tool_resource.connection.endpoint,
|
||||
}
|
||||
}
|
||||
result["project_connection_id"] = tool_resource.connection.name
|
||||
case ReferenceConnection():
|
||||
additional_properties = {
|
||||
"connection": {
|
||||
"kind": tool_resource.connection.kind,
|
||||
"name": tool_resource.connection.name,
|
||||
"authenticationMode": tool_resource.connection.authenticationMode,
|
||||
}
|
||||
}
|
||||
result["project_connection_id"] = tool_resource.connection.name
|
||||
case AnonymousConnection():
|
||||
pass
|
||||
case _:
|
||||
raise ValueError(f"Unsupported connection kind: {tool_resource.connection.kind}")
|
||||
|
||||
return HostedMCPTool(
|
||||
name=tool_resource.name, # type: ignore
|
||||
description=tool_resource.description,
|
||||
url=tool_resource.url, # type: ignore
|
||||
allowed_tools=tool_resource.allowedTools,
|
||||
approval_mode=approval_mode,
|
||||
headers=headers,
|
||||
additional_properties=additional_properties,
|
||||
)
|
||||
return result
|
||||
case _:
|
||||
raise ValueError(f"Unsupported tool kind: {tool_resource.kind}")
|
||||
|
||||
|
||||
@@ -698,11 +698,9 @@ class TestAgentFactoryMcpToolConnection:
|
||||
"""Tests for MCP tool connection handling in AgentFactory._parse_tool."""
|
||||
|
||||
def _get_mcp_tools(self, agent):
|
||||
"""Helper to get MCP tools from agent's default_options."""
|
||||
from agent_framework import HostedMCPTool
|
||||
|
||||
"""Helper to get MCP dict tools from agent's default_options."""
|
||||
tools = agent.default_options.get("tools", [])
|
||||
return [t for t in tools if isinstance(t, HostedMCPTool)]
|
||||
return [t for t in tools if isinstance(t, dict) and t.get("type") == "mcp"]
|
||||
|
||||
def test_mcp_tool_with_api_key_connection_sets_headers(self):
|
||||
"""Test that MCP tool with ApiKeyConnection sets headers correctly."""
|
||||
@@ -735,11 +733,11 @@ tools:
|
||||
mcp_tool = mcp_tools[0]
|
||||
|
||||
# Verify headers are set with the API key
|
||||
assert mcp_tool.headers is not None
|
||||
assert mcp_tool.headers == {"Authorization": "Bearer my-secret-api-key"}
|
||||
assert mcp_tool.get("headers") is not None
|
||||
assert mcp_tool.get("headers") == {"Authorization": "Bearer my-secret-api-key"}
|
||||
|
||||
def test_mcp_tool_with_remote_connection_sets_additional_properties(self):
|
||||
"""Test that MCP tool with RemoteConnection sets additional_properties correctly."""
|
||||
"""Test that MCP tool with RemoteConnection sets project_connection_id correctly."""
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from agent_framework_declarative import AgentFactory
|
||||
@@ -769,16 +767,11 @@ tools:
|
||||
assert len(mcp_tools) == 1
|
||||
mcp_tool = mcp_tools[0]
|
||||
|
||||
# Verify additional_properties are set with connection info
|
||||
assert mcp_tool.additional_properties is not None
|
||||
assert "connection" in mcp_tool.additional_properties
|
||||
conn = mcp_tool.additional_properties["connection"]
|
||||
assert conn["kind"] == "remote"
|
||||
assert conn["authenticationMode"] == "oauth"
|
||||
assert conn["name"] == "github-mcp-oauth-connection"
|
||||
# Verify project_connection_id is set from connection name
|
||||
assert mcp_tool.get("project_connection_id") == "github-mcp-oauth-connection"
|
||||
|
||||
def test_mcp_tool_with_reference_connection_sets_additional_properties(self):
|
||||
"""Test that MCP tool with ReferenceConnection sets additional_properties correctly."""
|
||||
"""Test that MCP tool with ReferenceConnection sets project_connection_id correctly."""
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from agent_framework_declarative import AgentFactory
|
||||
@@ -808,15 +801,11 @@ tools:
|
||||
assert len(mcp_tools) == 1
|
||||
mcp_tool = mcp_tools[0]
|
||||
|
||||
# Verify additional_properties are set with connection info
|
||||
assert mcp_tool.additional_properties is not None
|
||||
assert "connection" in mcp_tool.additional_properties
|
||||
conn = mcp_tool.additional_properties["connection"]
|
||||
assert conn["kind"] == "reference"
|
||||
assert conn["name"] == "my-connection-ref"
|
||||
# Verify project_connection_id is set from connection name
|
||||
assert mcp_tool.get("project_connection_id") == "my-connection-ref"
|
||||
|
||||
def test_mcp_tool_with_anonymous_connection_no_headers_or_properties(self):
|
||||
"""Test that MCP tool with AnonymousConnection doesn't set headers or additional_properties."""
|
||||
"""Test that MCP tool with AnonymousConnection doesn't set headers or project_connection_id."""
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from agent_framework_declarative import AgentFactory
|
||||
@@ -844,9 +833,9 @@ tools:
|
||||
assert len(mcp_tools) == 1
|
||||
mcp_tool = mcp_tools[0]
|
||||
|
||||
# Verify no headers or additional_properties are set
|
||||
assert mcp_tool.headers is None
|
||||
assert mcp_tool.additional_properties is None
|
||||
# Verify no headers or project_connection_id are set
|
||||
assert mcp_tool.get("headers") is None
|
||||
assert mcp_tool.get("project_connection_id") is None
|
||||
|
||||
def test_mcp_tool_without_connection_preserves_existing_behavior(self):
|
||||
"""Test that MCP tool without connection works as before (no headers or additional_properties)."""
|
||||
@@ -877,14 +866,13 @@ tools:
|
||||
mcp_tool = mcp_tools[0]
|
||||
|
||||
# Verify tool is created correctly without connection
|
||||
assert mcp_tool.name == "simple-mcp-tool"
|
||||
assert str(mcp_tool.url) == "https://api.example.com/mcp"
|
||||
assert mcp_tool.approval_mode == "never_require"
|
||||
assert mcp_tool.headers is None
|
||||
assert mcp_tool.additional_properties is None
|
||||
assert mcp_tool["server_label"] == "simple-mcp-tool"
|
||||
assert mcp_tool["server_url"] == "https://api.example.com/mcp"
|
||||
assert mcp_tool.get("require_approval") == "never"
|
||||
assert mcp_tool.get("headers") is None
|
||||
|
||||
def test_mcp_tool_with_remote_connection_with_endpoint(self):
|
||||
"""Test that MCP tool with RemoteConnection including endpoint sets it in additional_properties."""
|
||||
"""Test that MCP tool with RemoteConnection including endpoint sets project_connection_id."""
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from agent_framework_declarative import AgentFactory
|
||||
@@ -915,7 +903,5 @@ tools:
|
||||
assert len(mcp_tools) == 1
|
||||
mcp_tool = mcp_tools[0]
|
||||
|
||||
# Verify additional_properties include endpoint
|
||||
assert mcp_tool.additional_properties is not None
|
||||
conn = mcp_tool.additional_properties["connection"]
|
||||
assert conn["endpoint"] == "https://auth.example.com"
|
||||
# Verify project_connection_id is set from connection name
|
||||
assert mcp_tool.get("project_connection_id") == "my-oauth-connection"
|
||||
|
||||
Reference in New Issue
Block a user