mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## Description Normalize MCP tool-name aliases in the shared exclusion matcher so Anthropic/custom-agent names like `mcp_Server_tool` match the documented `mcp__*` glob and bare tool exclusions such as `headroom_retrieve`. Closes #1822 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Added MCP alias matching for `mcp__server__tool`, `mcp_Server_tool`, and the bare wrapped tool name. - Added Anthropic `tool_use` / `tool_result` regressions for custom-agent MCP names and bare `headroom_retrieve` exclusions. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ .venv/bin/python -m pytest tests/test_transforms/test_content_router.py -q 57 passed, 1 warning in 0.95s $ .venv/bin/python -m ruff check . All checks passed! $ .venv/bin/python -m ruff format --check . 1058 files already formatted $ .venv/bin/python -m mypy headroom --ignore-missing-imports Success: no issues found in 407 source files ``` ## Real Behavior Proof - Environment: macOS, Python 3.13.5 local venv with editable headroom build. - Exact command / steps: Added #1822 regressions, ran the focused tests before the fix, then reran after adding MCP aliases. - Observed result: Before the fix, custom-agent MCP tool results were compressed instead of excluded; after the fix, the full content-router test file passes and excluded MCP results stay on the lossless excluded path. - Not tested: Full repository test suite locally; GitHub CI passed the full PR matrix. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review - [x] Principal engineer agent approved - [x] Senior developer agent approved ## Checklist - [x] My code follows the project style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A ## Additional Notes Review agents approved the scoped MCP exclusion-alias fix. One non-blocking review note: #1822 also mentions TOIN/prefix-cache symptoms, while this PR specifically fixes the custom-agent MCP exclusion name-resolution path.
This commit is contained in:
parent
2ccd831032
commit
140d6e4f96
2 changed files with 122 additions and 4 deletions
|
|
@ -229,6 +229,27 @@ DEFAULT_EXCLUDE_TOOLS: frozenset[str] = frozenset(
|
|||
)
|
||||
|
||||
|
||||
def _tool_name_aliases(name: str) -> tuple[str, ...]:
|
||||
"""Return equivalent spellings for tool exclusion matching."""
|
||||
aliases = [name]
|
||||
lname = name.lower()
|
||||
|
||||
if lname.startswith("mcp__"):
|
||||
# OpenAI-style MCP wrappers use mcp__server__tool. Custom agents that
|
||||
# speak Anthropic sometimes emit the same wrapper as mcp_Server_tool.
|
||||
parts = name.split("__", 2)
|
||||
if len(parts) == 3 and parts[1] and parts[2]:
|
||||
aliases.append(f"mcp_{parts[1]}_{parts[2]}")
|
||||
aliases.append(parts[2])
|
||||
elif lname.startswith("mcp_"):
|
||||
parts = name.split("_", 2)
|
||||
if len(parts) == 3 and parts[1] and parts[2]:
|
||||
aliases.append(f"mcp__{parts[1]}__{parts[2]}")
|
||||
aliases.append(parts[2])
|
||||
|
||||
return tuple(dict.fromkeys(aliases))
|
||||
|
||||
|
||||
def is_tool_excluded(name: str, exclude_tools: Iterable[str]) -> bool:
|
||||
"""Return True if ``name`` matches the tool-exclusion set.
|
||||
|
||||
|
|
@ -237,15 +258,28 @@ def is_tool_excluded(name: str, exclude_tools: Iterable[str]) -> bool:
|
|||
``[``) are matched with :func:`fnmatch.fnmatchcase`, letting a single pattern
|
||||
such as ``mcp__*`` cover every tool an MCP server exposes without listing
|
||||
each name (issue #870).
|
||||
|
||||
MCP tool wrappers are also matched through their common aliases. For example,
|
||||
``mcp__Headroom__headroom_retrieve`` and
|
||||
``mcp_Headroom_headroom_retrieve`` both match ``mcp__*`` and the bare
|
||||
``headroom_retrieve`` entry.
|
||||
"""
|
||||
if not exclude_tools:
|
||||
return False
|
||||
if name in exclude_tools or name.lower() in exclude_tools:
|
||||
|
||||
patterns = tuple(exclude_tools)
|
||||
if not patterns:
|
||||
return False
|
||||
aliases = _tool_name_aliases(name)
|
||||
exact_patterns = set(patterns)
|
||||
lower_exact_patterns = {pat.lower() for pat in exact_patterns}
|
||||
if any(alias in exact_patterns or alias.lower() in lower_exact_patterns for alias in aliases):
|
||||
return True
|
||||
lname = name.lower()
|
||||
|
||||
return any(
|
||||
fnmatch.fnmatchcase(lname, pat.lower())
|
||||
for pat in exclude_tools
|
||||
fnmatch.fnmatchcase(alias.lower(), pat.lower())
|
||||
for alias in aliases
|
||||
for pat in patterns
|
||||
if "*" in pat or "?" in pat or "[" in pat
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -796,16 +796,100 @@ class TestExcludeTools:
|
|||
assert json.loads(result.messages[1]["content"]) == json.loads(messages[1]["content"])
|
||||
assert "router:excluded:lossless_json" in result.transforms_applied
|
||||
|
||||
def test_anthropic_mcp_alias_exclude_tools(self, tokenizer):
|
||||
"""Single-underscore MCP names from custom agents honor documented MCP globs."""
|
||||
config = ContentRouterConfig(
|
||||
min_section_tokens=10,
|
||||
exclude_tools={"mcp__*"},
|
||||
)
|
||||
router = ContentRouter(config)
|
||||
|
||||
messages = [
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_use",
|
||||
"id": "toolu_mcp_1",
|
||||
"name": "mcp_CursorTaskRegistry_cursor_list_tasks",
|
||||
"input": {"project": "headroom"},
|
||||
}
|
||||
],
|
||||
},
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": "toolu_mcp_1",
|
||||
"content": generate_json_data(50),
|
||||
}
|
||||
],
|
||||
},
|
||||
]
|
||||
|
||||
result = router.apply(messages, tokenizer)
|
||||
|
||||
tool_result_block = result.messages[1]["content"][0]
|
||||
assert json.loads(tool_result_block["content"]) == json.loads(
|
||||
messages[1]["content"][0]["content"]
|
||||
)
|
||||
assert "router:excluded:lossless_json" in result.transforms_applied
|
||||
|
||||
def test_anthropic_mcp_bare_tool_alias_exclude_tools(self, tokenizer):
|
||||
"""Bare tool exclusions match custom-agent MCP wrappers (#1822)."""
|
||||
config = ContentRouterConfig(
|
||||
min_section_tokens=10,
|
||||
exclude_tools={"headroom_retrieve"},
|
||||
)
|
||||
router = ContentRouter(config)
|
||||
|
||||
messages = [
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_use",
|
||||
"id": "toolu_retrieve_1",
|
||||
"name": "mcp_HeadroomZai_headroom_retrieve",
|
||||
"input": {"key": "abc123"},
|
||||
}
|
||||
],
|
||||
},
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": "toolu_retrieve_1",
|
||||
"content": generate_json_data(50),
|
||||
}
|
||||
],
|
||||
},
|
||||
]
|
||||
|
||||
result = router.apply(messages, tokenizer)
|
||||
|
||||
tool_result_block = result.messages[1]["content"][0]
|
||||
assert json.loads(tool_result_block["content"]) == json.loads(
|
||||
messages[1]["content"][0]["content"]
|
||||
)
|
||||
assert "router:excluded:lossless_json" in result.transforms_applied
|
||||
|
||||
def test_is_tool_excluded_helper(self):
|
||||
"""is_tool_excluded: exact (case-insensitive) and glob matching."""
|
||||
from headroom.config import is_tool_excluded
|
||||
|
||||
# Glob entry covers a whole MCP server; unrelated tools are untouched.
|
||||
assert is_tool_excluded("mcp__build123d__measure", {"mcp__*"})
|
||||
assert is_tool_excluded("mcp_CursorTaskRegistry_cursor_list_tasks", {"mcp__*"})
|
||||
assert not is_tool_excluded("Bash", {"mcp__*"})
|
||||
# Plain entries keep exact, case-insensitive membership.
|
||||
assert is_tool_excluded("Read", {"read"})
|
||||
assert is_tool_excluded("MCP__X", {"mcp__*"})
|
||||
# MCP wrapper aliases can still be excluded by their bare tool name.
|
||||
assert is_tool_excluded("mcp_HeadroomZai_headroom_retrieve", {"headroom_retrieve"})
|
||||
assert is_tool_excluded("mcp__Headroom__headroom_retrieve", {"headroom_retrieve"})
|
||||
# Empty set never excludes.
|
||||
assert not is_tool_excluded("Read", set())
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue