mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-26 17:38:36 +00:00
feat(mcp): fnmatch glob support in tools.include/exclude filters
The include/exclude filter matched exact names only — glob-style entries
('*_radar_*') silently matched nothing, so a Cloudflare flat-mode config
meant to trim 3,320 tools to ~1,900 actually registered 3,319. Unmatched
patterns produced no warning.
- matches_name_filter(): exact membership first (O(1) for literal lists),
then fnmatch.fnmatchcase for entries containing * ? [ — same pattern
semantics as approvals.deny. Entries without metacharacters stay
strictly literal ('docs' never matches 'docs_search').
- _should_register() uses it for both include and exclude (symmetric)
- hermes mcp tools picker (mcp_config.py) pre-selection uses the same
matcher so the UI agrees with runtime registration
E2E against the live Cloudflare capture with the real exclude list:
3,320 -> 1,905 surviving (1,415 excluded); radar/DLP gone,
purge_cache/dns_records kept. 220/220 mcp_tool tests (4 new).
This commit is contained in:
parent
e9fe060ebf
commit
e7172ab1ba
4 changed files with 110 additions and 10 deletions
|
|
@ -986,15 +986,25 @@ def cmd_mcp_configure(args):
|
|||
|
||||
tool_names = [t[0] for t in all_tools]
|
||||
|
||||
# Same matching semantics as runtime registration (tools/mcp_tool.py):
|
||||
# exact names or fnmatch globs.
|
||||
try:
|
||||
from tools.mcp_tool import matches_name_filter
|
||||
except ImportError: # pragma: no cover — defensive fallback
|
||||
def matches_name_filter(tool_name, patterns):
|
||||
return tool_name in patterns
|
||||
|
||||
if include and isinstance(include, list):
|
||||
include_set = set(include)
|
||||
include_set = {str(p) for p in include}
|
||||
pre_selected = {
|
||||
i for i, tn in enumerate(tool_names) if tn in include_set
|
||||
i for i, tn in enumerate(tool_names)
|
||||
if matches_name_filter(tn, include_set)
|
||||
}
|
||||
elif exclude and isinstance(exclude, list):
|
||||
exclude_set = set(exclude)
|
||||
exclude_set = {str(p) for p in exclude}
|
||||
pre_selected = {
|
||||
i for i, tn in enumerate(tool_names) if tn not in exclude_set
|
||||
i for i, tn in enumerate(tool_names)
|
||||
if not matches_name_filter(tn, exclude_set)
|
||||
}
|
||||
else:
|
||||
pre_selected = set(range(len(all_tools)))
|
||||
|
|
|
|||
|
|
@ -3874,6 +3874,52 @@ class TestMCPSelectiveToolLoading:
|
|||
"mcp__ink_exclude__list_services",
|
||||
]
|
||||
|
||||
def test_exclude_filter_supports_globs(self):
|
||||
"""fnmatch globs in exclude — the Cloudflare flat-mode shape
|
||||
(``*_radar_*`` etc.). Previously silently matched nothing."""
|
||||
config = {
|
||||
"url": "https://mcp.example.com",
|
||||
"tools": {"exclude": ["*_radar_*", "delete_*"]},
|
||||
}
|
||||
registered, _ = self._run_discover(
|
||||
"ink_glob",
|
||||
["get_radar_summary", "get_accounts_radar_http", "delete_service",
|
||||
"create_service", "list_services"],
|
||||
config,
|
||||
session=SimpleNamespace(),
|
||||
)
|
||||
assert registered == [
|
||||
"mcp__ink_glob__create_service",
|
||||
"mcp__ink_glob__list_services",
|
||||
]
|
||||
|
||||
def test_include_filter_supports_globs(self):
|
||||
"""Globs work symmetrically on the include whitelist."""
|
||||
config = {
|
||||
"url": "https://mcp.example.com",
|
||||
"tools": {"include": ["get_zones_*"]},
|
||||
}
|
||||
registered, _ = self._run_discover(
|
||||
"ink_glob_inc",
|
||||
["get_zones_dns_records", "get_zones_settings", "delete_zone",
|
||||
"get_accounts"],
|
||||
config,
|
||||
session=SimpleNamespace(),
|
||||
)
|
||||
assert registered == [
|
||||
"mcp__ink_glob_inc__get_zones_dns_records",
|
||||
"mcp__ink_glob_inc__get_zones_settings",
|
||||
]
|
||||
|
||||
def test_exact_names_still_match_exactly(self):
|
||||
"""No-metachar entries stay literal — 'docs' must not glob-match
|
||||
'docs_search', and exact matching is unchanged."""
|
||||
from tools.mcp_tool import matches_name_filter
|
||||
assert matches_name_filter("docs", {"docs"})
|
||||
assert not matches_name_filter("docs_search", {"docs"})
|
||||
assert matches_name_filter("docs_search", {"docs*"})
|
||||
assert not matches_name_filter("anything", set())
|
||||
|
||||
def test_include_filter_skips_utility_tools_without_capabilities(self):
|
||||
config = {
|
||||
"url": "https://mcp.example.com",
|
||||
|
|
|
|||
|
|
@ -93,6 +93,7 @@ import asyncio
|
|||
import contextvars
|
||||
import concurrent.futures
|
||||
import errno
|
||||
import fnmatch
|
||||
import inspect
|
||||
import json
|
||||
import logging
|
||||
|
|
@ -5307,7 +5308,12 @@ def _build_utility_schemas(server_name: str) -> List[dict]:
|
|||
|
||||
|
||||
def _normalize_name_filter(value: Any, label: str) -> set[str]:
|
||||
"""Normalize include/exclude config to a set of tool names."""
|
||||
"""Normalize include/exclude config to a set of tool-name patterns.
|
||||
|
||||
Entries may be exact tool names or fnmatch-style globs
|
||||
(``*_radar_*``, ``get_zones_*``). Matching happens in
|
||||
:func:`matches_name_filter`.
|
||||
"""
|
||||
if value is None:
|
||||
return set()
|
||||
if isinstance(value, str):
|
||||
|
|
@ -5318,6 +5324,25 @@ def _normalize_name_filter(value: Any, label: str) -> set[str]:
|
|||
return set()
|
||||
|
||||
|
||||
def matches_name_filter(tool_name: str, patterns: set[str]) -> bool:
|
||||
"""True if ``tool_name`` matches any entry in ``patterns``.
|
||||
|
||||
Exact names match literally; entries containing fnmatch metacharacters
|
||||
(``*``, ``?``, ``[``) match as case-sensitive globs — the same pattern
|
||||
semantics as ``approvals.deny``. Exact membership is checked first so
|
||||
large literal lists stay O(1).
|
||||
"""
|
||||
if not patterns:
|
||||
return False
|
||||
if tool_name in patterns:
|
||||
return True
|
||||
return any(
|
||||
fnmatch.fnmatchcase(tool_name, p)
|
||||
for p in patterns
|
||||
if "*" in p or "?" in p or "[" in p
|
||||
)
|
||||
|
||||
|
||||
def _parse_boolish(value: Any, default: bool = True) -> bool:
|
||||
"""Parse a bool-like config value with safe fallback."""
|
||||
if value is None:
|
||||
|
|
@ -5481,9 +5506,10 @@ def _register_server_tools(name: str, server: MCPServerTask, config: dict) -> Li
|
|||
toolset_name = f"mcp-{name}"
|
||||
|
||||
# Selective tool loading: honour include/exclude lists from config.
|
||||
# Rules (matching issue #690 spec):
|
||||
# tools.include — whitelist: only these tool names are registered
|
||||
# tools.exclude — blacklist: all tools EXCEPT these are registered
|
||||
# Rules (matching issue #690 spec, extended with glob support):
|
||||
# tools.include — whitelist: only matching tool names are registered
|
||||
# tools.exclude — blacklist: all tools EXCEPT matching ones are registered
|
||||
# entries may be exact names or fnmatch globs (e.g. "*_radar_*")
|
||||
# include takes precedence over exclude
|
||||
# Neither set → register all tools (backward-compatible default)
|
||||
tools_filter = config.get("tools") or {}
|
||||
|
|
@ -5492,9 +5518,9 @@ def _register_server_tools(name: str, server: MCPServerTask, config: dict) -> Li
|
|||
|
||||
def _should_register(tool_name: str) -> bool:
|
||||
if include_set:
|
||||
return tool_name in include_set
|
||||
return matches_name_filter(tool_name, include_set)
|
||||
if exclude_set:
|
||||
return tool_name not in exclude_set
|
||||
return not matches_name_filter(tool_name, exclude_set)
|
||||
return True
|
||||
|
||||
for mcp_tool in server._tools:
|
||||
|
|
|
|||
|
|
@ -460,6 +460,24 @@ mcp_servers:
|
|||
|
||||
All server tools are registered except the excluded ones.
|
||||
|
||||
### Glob patterns
|
||||
|
||||
Both lists accept fnmatch-style globs alongside exact names — essential for
|
||||
huge flat surfaces like Cloudflare's API MCP (`?codemode=false`, ~3,300
|
||||
tools) where excluding product areas one endpoint at a time is impractical:
|
||||
|
||||
```yaml
|
||||
mcp_servers:
|
||||
cloudflare:
|
||||
url: "https://mcp.cloudflare.com/mcp?codemode=false"
|
||||
auth: oauth
|
||||
tools:
|
||||
exclude: ["*_radar_*", "*_accounts_dlp_*", "*_zones_web3_*"]
|
||||
```
|
||||
|
||||
Entries without glob metacharacters (`*`, `?`, `[`) match exactly — `docs`
|
||||
excludes only the tool named `docs`, never `docs_search`.
|
||||
|
||||
### Precedence rule
|
||||
|
||||
If both are present:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue