Skip to content

Commit 2f4d001

Browse files
authored
fix(proxy): keep prefixed core tools resident (#3046)
## Description Headroom's Tool Search deferral lowercased core tool names but did not account for client namespace prefixes. Oh My Pi sends built-ins such as `_read`, `_edit`, `_write`, and `_bash`, so those core tools were incorrectly marked `defer_loading=True`. This change centralizes resident-name normalization for both the Anthropic and OpenAI paths. It lowercases names and removes only leading underscores, preserving internal separators such as `mcp__server__read` so unrelated tools do not become resident. Closes #3031 ## 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 a shared resident-tool name normalizer in `headroom/proxy/helpers.py`. - Applied the same normalization to Anthropic and OpenAI Tool Search deferral. - Added a regression test for Oh My Pi's exact 12-tool surface at the deferral threshold. - Added OpenAI coverage for prefixed resident tools and negative namespace cases. ## Testing <!-- Check what you actually ran, then paste the real command output below. --> - [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 $ uv run --no-sync pytest --noconftest -q tests/test_openai_tool_search_deferral.py tests/test_issue_746_tool_search.py -k 'not normalize_tool_search_mode and not configure_' 72 passed, 23 deselected in 0.25s $ uv run --no-sync ruff check . All checks passed! $ uv run --no-sync ruff format --check . 1499 files already formatted $ UV_CACHE_DIR=/tmp/headroom-uv-cache uv run --no-sync mypy headroom Success: no issues found in 520 source files ``` ## Real Behavior Proof - Environment: Linux x86_64 sandbox; Python 3.12.13; uv 0.11.33; no provider credentials. - Exact command / steps: Exercised the exact 12-tool Oh My Pi fixture through the Anthropic deferral helper and prefixed resident plus negative names through the OpenAI helper. - Observed result: Anthropic kept `_edit`, `_task`, `_read`, `_bash`, `_glob`, `_grep`, `_write`, `computer`, and `web_search` resident while deferring `_hub`, `_todo`, and `_eval`. OpenAI kept prefixed core tools resident while `mcp__server__read` and `terminal_helper` remained deferred. - Not tested: Live Oh My Pi traffic against Anthropic, provider E2E tests, and the full native-backed pytest suite. ## Runtime Rollout Safety - Rollout-managed feature(s): Existing server-side Tool Search deferral for Anthropic and OpenAI. - Minimum rollout channel: N/A; targeted bug fix to existing behavior. - Stable/default behavior changed: Yes. Leading-underscore names that normalize to known resident names now remain resident. - Kill switch / disable path: Set `HEADROOM_TOOL_SEARCH=0`. - Unsafe override required: No. - Qualification impact: Prefixed core tools remain immediately available; non-core and MCP namespace behavior is unchanged. - Rollback path: Revert this commit or disable Tool Search with `HEADROOM_TOOL_SEARCH=0`. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's 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 - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [x] I did **not** edit `CHANGELOG.md` — it is generated by release-please from my Conventional Commit PR title (a CI guard enforces this) ## Screenshots (if applicable) N/A ## Additional Notes
1 parent 322425c commit 2f4d001

3 files changed

Lines changed: 68 additions & 11 deletions

File tree

headroom/proxy/helpers.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2811,6 +2811,13 @@ def reset_tool_search_hint_state() -> None:
28112811
_TOOL_SEARCH_MIN_TOOLS = 12
28122812

28132813

2814+
def _tool_search_resident_key(name: Any) -> str:
2815+
"""Normalize a client tool name for resident-tool membership checks."""
2816+
# Oh My Pi prefixes every built-in with ``_``. Strip only leading namespace
2817+
# markers so internal separators such as ``mcp__server__read`` stay intact.
2818+
return str(name or "").lower().lstrip("_")
2819+
2820+
28142821
def anthropic_first_party_tool_search_supported(api_base_url: str | None) -> bool:
28152822
"""Return whether Anthropic server-side tool search is valid for this upstream."""
28162823
from headroom.providers.claude.runtime import is_custom_anthropic_base_url
@@ -2872,17 +2879,17 @@ def inject_tool_search_deferral(
28722879
last_resident_real: dict[str, Any] | None = None
28732880
resident_has_cache_control = False
28742881

2875-
# Clients disagree on casing for the same tool: Claude Code sends ``Bash`` /
2876-
# ``ToolSearch`` where opencode sends ``bash``. Compare case-insensitively so
2877-
# the exemption applies to both — an exact match silently deferred *every*
2878-
# tool for PascalCase clients, including their own tool-search tool.
2879-
core_lower = {name.lower() for name in core_tools}
2882+
# Clients disagree on casing and leading namespace markers for the same tool:
2883+
# Claude Code sends ``Bash``, opencode sends ``bash``, and Oh My Pi sends
2884+
# ``_bash``. Normalize both the configured names and each candidate so the
2885+
# exemption applies consistently across clients.
2886+
core_keys = {_tool_search_resident_key(name) for name in core_tools}
28802887

28812888
for tool in tools:
28822889
if (
28832890
not isinstance(tool, dict)
28842891
or tool.get("type")
2885-
or str(tool.get("name") or "").lower() in core_lower
2892+
or _tool_search_resident_key(tool.get("name")) in core_keys
28862893
):
28872894
# Non-dict, server/typed tools (web_search, computer, …), and core
28882895
# tools stay resident and unchanged.
@@ -3259,10 +3266,10 @@ def inject_tool_search_deferral_openai(
32593266

32603267
out: list[Any] = [{"type": _OPENAI_TOOL_SEARCH_TYPE}]
32613268
deferred = 0
3262-
# Case-insensitive for the same reason as the Anthropic path above: the
3263-
# resident-name sets are lowercase, clients are not required to be.
3264-
resident_lower = {name.lower() for name in core_tools} | {
3265-
name.lower() for name in _OPENAI_TOOL_SEARCH_RESIDENT_NAMES
3269+
# Normalize for the same reason as the Anthropic path above: clients may use
3270+
# different casing or a leading namespace marker for the same resident tool.
3271+
resident_keys = {_tool_search_resident_key(name) for name in core_tools} | {
3272+
_tool_search_resident_key(name) for name in _OPENAI_TOOL_SEARCH_RESIDENT_NAMES
32663273
}
32673274
for tool in tools:
32683275
if not isinstance(tool, dict):
@@ -3273,7 +3280,7 @@ def inject_tool_search_deferral_openai(
32733280
# trained to search namespaces / MCP servers). Everything else — core
32743281
# coding tools and other hosted tools — stays resident.
32753282
deferrable = (
3276-
ttype == "function" and str(tool.get("name") or "").lower() not in resident_lower
3283+
ttype == "function" and _tool_search_resident_key(tool.get("name")) not in resident_keys
32773284
) or ttype == "mcp"
32783285
if deferrable and not tool.get("defer_loading"):
32793286
new_tool = dict(tool)

tests/test_issue_746_tool_search.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,42 @@ def test_resident_real_tool_survives_pascal_case_surface() -> None:
350350
assert any(not t.get("type") and not t.get("defer_loading") for t in out)
351351

352352

353+
def _omp_tools() -> list[dict]:
354+
"""Oh My Pi's 12-tool surface: underscore-prefixed built-ins plus typed tools."""
355+
named = [
356+
"_hub",
357+
"_edit",
358+
"_task",
359+
"_todo",
360+
"_eval",
361+
"_read",
362+
"_bash",
363+
"_glob",
364+
"_grep",
365+
"_write",
366+
]
367+
return [
368+
*[{"name": name, "description": name, "input_schema": {}} for name in named],
369+
{"type": "computer_20250124", "name": "computer"},
370+
{"type": "web_search_20250305", "name": "web_search"},
371+
]
372+
373+
374+
def test_core_tools_match_leading_underscore_namespace() -> None:
375+
tools = _omp_tools()
376+
assert len(tools) == _TOOL_SEARCH_MIN_TOOLS
377+
378+
out = inject_tool_search_deferral(tools)
379+
380+
by_name = {tool.get("name"): tool for tool in out if isinstance(tool, dict)}
381+
for name in ("_edit", "_task", "_read", "_bash", "_glob", "_grep", "_write"):
382+
assert by_name[name].get("defer_loading") is None, name
383+
for name in ("_hub", "_todo", "_eval"):
384+
assert by_name[name].get("defer_loading") is True, name
385+
for name in ("computer", "web_search"):
386+
assert by_name[name].get("defer_loading") is None, name
387+
388+
353389
# ---------------------------------------------------------------------------
354390
# Tool-search history repair (#2805)
355391
#

tests/test_openai_tool_search_deferral.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,20 @@ def test_terminal_helper_remains_deferrable():
123123
assert helper.get("defer_loading") is True
124124

125125

126+
def test_prefixed_core_and_terminal_names_stay_resident():
127+
resident = ["_bash", "_read", "_write", "_edit", "_glob", "_grep", "_terminal"]
128+
noncore = ["_hub", "_todo", "_eval", "mcp__server__read", "terminal_helper"]
129+
tools = [_fn(name) for name in resident + noncore]
130+
131+
out = inject_tool_search_deferral_openai(tools, "gpt-5.6-terra")
132+
133+
by_name = {tool["name"]: tool for tool in out if tool.get("type") == "function"}
134+
for name in resident:
135+
assert by_name[name].get("defer_loading") is None, name
136+
for name in noncore:
137+
assert by_name[name].get("defer_loading") is True, name
138+
139+
126140
def test_defers_mcp_server():
127141
tools = [_fn(n) for n in _CORE] + [{"type": "mcp", "server_label": "sentry"}]
128142
tools += [_fn(f"x{i}") for i in range(8)]

0 commit comments

Comments
 (0)