Skip to content

Commit adbf51c

Browse files
bokelleyclaude
andcommitted
fix: walk MRO for tool filtering, add tool name validation
- get_tools_for_handler now accepts instance or class, walks MRO to find the matching handler base class. Fixes filtering for user subclasses (e.g. MyGovernanceAgent(GovernanceHandler) now correctly gets only governance tools). - Add module-level assertion that all tool names in _HANDLER_TOOLS reference real tools in ADCP_TOOL_DEFINITIONS. - Add test for subclass MRO-based filtering. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 12f1080 commit adbf51c

2 files changed

Lines changed: 32 additions & 15 deletions

File tree

src/adcp/server/mcp_tools.py

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -589,25 +589,34 @@
589589
"ADCPHandler": {tool["name"] for tool in ADCP_TOOL_DEFINITIONS},
590590
}
591591

592+
# Validate that all handler tool names reference real tools
593+
_ALL_TOOL_NAMES = {t["name"] for t in ADCP_TOOL_DEFINITIONS}
594+
for _handler_name, _tools in _HANDLER_TOOLS.items():
595+
_unknown = _tools - _ALL_TOOL_NAMES
596+
assert not _unknown, f"{_handler_name} references unknown tools: {_unknown}"
592597

593-
def get_tools_for_handler(handler_class_name: str) -> list[dict[str, Any]]:
598+
599+
def get_tools_for_handler(handler: ADCPHandler | type[ADCPHandler]) -> list[dict[str, Any]]:
594600
"""Return tool definitions filtered by handler type.
595601
596-
Specialized handlers only get their own tools plus protocol discovery.
602+
Walks the MRO to find the matching handler base class, so subclasses
603+
(e.g. MyGovernanceAgent(GovernanceHandler)) get the correct tool set.
597604
ADCPHandler gets all tools. Unknown handlers get only protocol discovery
598605
(minimum privilege).
599606
600607
Args:
601-
handler_class_name: The handler class name (e.g. "GovernanceHandler")
608+
handler: The handler instance or class
602609
603610
Returns:
604611
Filtered list of tool definitions
605612
"""
606-
if handler_class_name not in _HANDLER_TOOLS:
607-
return [tool for tool in ADCP_TOOL_DEFINITIONS if tool["name"] in _PROTOCOL_TOOLS]
613+
cls = handler if isinstance(handler, type) else type(handler)
614+
for base in cls.__mro__:
615+
if base.__name__ in _HANDLER_TOOLS:
616+
allowed = _HANDLER_TOOLS[base.__name__] | _PROTOCOL_TOOLS
617+
return [tool for tool in ADCP_TOOL_DEFINITIONS if tool["name"] in allowed]
608618

609-
allowed = _HANDLER_TOOLS[handler_class_name] | _PROTOCOL_TOOLS
610-
return [tool for tool in ADCP_TOOL_DEFINITIONS if tool["name"] in allowed]
619+
return [tool for tool in ADCP_TOOL_DEFINITIONS if tool["name"] in _PROTOCOL_TOOLS]
611620

612621

613622
def create_tool_caller(
@@ -649,7 +658,7 @@ def __init__(self, handler: ADCPHandler):
649658
handler: ADCP handler instance
650659
"""
651660
self.handler = handler
652-
self._filtered_definitions = get_tools_for_handler(type(handler).__name__)
661+
self._filtered_definitions = get_tools_for_handler(handler)
653662
self._tools: dict[str, Callable[[dict[str, Any]], Any]] = {}
654663

655664
# Create tool callers only for filtered tools

tests/test_spec_coverage.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,12 @@ def test_tool_filtering_by_handler_type():
7777
all_tool_names = {tool["name"] for tool in ADCP_TOOL_DEFINITIONS}
7878

7979
# GovernanceHandler: governance tools + protocol discovery
80-
gov_tools = {t["name"] for t in get_tools_for_handler("GovernanceHandler")}
80+
from adcp.server.base import ADCPHandler
81+
from adcp.server.content_standards import ContentStandardsHandler
82+
from adcp.server.governance import GovernanceHandler
83+
from adcp.server.sponsored_intelligence import SponsoredIntelligenceHandler
84+
85+
gov_tools = {t["name"] for t in get_tools_for_handler(GovernanceHandler)}
8186
assert gov_tools == {
8287
"get_creative_features", "sync_plans", "check_governance",
8388
"report_plan_outcome", "get_plan_audit_logs",
@@ -87,7 +92,7 @@ def test_tool_filtering_by_handler_type():
8792
}
8893

8994
# ContentStandardsHandler: content standards tools + protocol discovery
90-
cs_tools = {t["name"] for t in get_tools_for_handler("ContentStandardsHandler")}
95+
cs_tools = {t["name"] for t in get_tools_for_handler(ContentStandardsHandler)}
9196
assert cs_tools == {
9297
"create_content_standards", "get_content_standards",
9398
"list_content_standards", "update_content_standards",
@@ -97,20 +102,23 @@ def test_tool_filtering_by_handler_type():
97102
}
98103

99104
# SponsoredIntelligenceHandler: SI tools + protocol discovery
100-
si_tools = {t["name"] for t in get_tools_for_handler("SponsoredIntelligenceHandler")}
105+
si_tools = {t["name"] for t in get_tools_for_handler(SponsoredIntelligenceHandler)}
101106
assert si_tools == {
102107
"si_get_offering", "si_initiate_session",
103108
"si_send_message", "si_terminate_session",
104109
"get_adcp_capabilities",
105110
}
106111

107112
# ADCPHandler: all tools (no filtering)
108-
adcp_tools = {t["name"] for t in get_tools_for_handler("ADCPHandler")}
113+
adcp_tools = {t["name"] for t in get_tools_for_handler(ADCPHandler)}
109114
assert adcp_tools == all_tool_names
110115

111-
# Unknown handler: protocol-only tools (minimum privilege)
112-
unknown_tools = {t["name"] for t in get_tools_for_handler("SomeCustomHandler")}
113-
assert unknown_tools == {"get_adcp_capabilities"}
116+
# Subclass of GovernanceHandler gets governance tools (MRO walk)
117+
class MyGovernanceAgent(GovernanceHandler):
118+
pass
119+
120+
subclass_tools = {t["name"] for t in get_tools_for_handler(MyGovernanceAgent)}
121+
assert subclass_tools == gov_tools
114122

115123

116124
def _collect_all_properties(schema: dict[str, Any]) -> set[str]:

0 commit comments

Comments
 (0)