Skip to content

Commit 39985e5

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 39985e5

File tree

2 files changed

+33
-20
lines changed

2 files changed

+33
-20
lines changed

src/adcp/server/mcp_tools.py

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,10 @@
66
from __future__ import annotations
77

88
from collections.abc import Callable
9-
from typing import TYPE_CHECKING, Any
9+
from typing import Any
1010

1111
from adcp.server.base import ADCPHandler, ToolContext
1212

13-
if TYPE_CHECKING:
14-
pass
15-
16-
1713
# Tool definitions for all ADCP operations
1814
ADCP_TOOL_DEFINITIONS: list[dict[str, Any]] = [
1915
# Core Catalog Operations
@@ -589,25 +585,34 @@
589585
"ADCPHandler": {tool["name"] for tool in ADCP_TOOL_DEFINITIONS},
590586
}
591587

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

593-
def get_tools_for_handler(handler_class_name: str) -> list[dict[str, Any]]:
595+
def get_tools_for_handler(handler: ADCPHandler | type[ADCPHandler]) -> list[dict[str, Any]]:
594596
"""Return tool definitions filtered by handler type.
595597
596-
Specialized handlers only get their own tools plus protocol discovery.
598+
Walks the MRO to find the matching handler base class, so subclasses
599+
(e.g. MyGovernanceAgent(GovernanceHandler)) get the correct tool set.
597600
ADCPHandler gets all tools. Unknown handlers get only protocol discovery
598601
(minimum privilege).
599602
600603
Args:
601-
handler_class_name: The handler class name (e.g. "GovernanceHandler")
604+
handler: The handler instance or class
602605
603606
Returns:
604607
Filtered list of tool definitions
605608
"""
606-
if handler_class_name not in _HANDLER_TOOLS:
607-
return [tool for tool in ADCP_TOOL_DEFINITIONS if tool["name"] in _PROTOCOL_TOOLS]
609+
cls = handler if isinstance(handler, type) else type(handler)
610+
for base in cls.__mro__:
611+
if base.__name__ in _HANDLER_TOOLS:
612+
allowed = _HANDLER_TOOLS[base.__name__] | _PROTOCOL_TOOLS
613+
return [tool for tool in ADCP_TOOL_DEFINITIONS if tool["name"] in allowed]
608614

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

612617

613618
def create_tool_caller(
@@ -649,7 +654,7 @@ def __init__(self, handler: ADCPHandler):
649654
handler: ADCP handler instance
650655
"""
651656
self.handler = handler
652-
self._filtered_definitions = get_tools_for_handler(type(handler).__name__)
657+
self._filtered_definitions = get_tools_for_handler(handler)
653658
self._tools: dict[str, Callable[[dict[str, Any]], Any]] = {}
654659

655660
# 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)