mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## 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
234 lines
8.6 KiB
Python
234 lines
8.6 KiB
Python
"""Server-side Tool Search deferral for OpenAI Responses (gpt-5.4+).
|
|
|
|
The OpenAI-side analogue of the Anthropic path (issue #746): mark non-core
|
|
function / MCP tools ``defer_loading: true`` and inject ``{"type": "tool_search"}``
|
|
so OpenAI keeps their heavy parameter schemas out of the model's context until
|
|
searched. Gated to gpt-5.4+ (older models 400 on the fields).
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import copy
|
|
|
|
import pytest
|
|
|
|
from headroom.proxy.helpers import (
|
|
_model_supports_openai_tool_search,
|
|
inject_tool_search_deferral_openai,
|
|
openai_tool_search_client_supported,
|
|
)
|
|
|
|
|
|
def _fn(name: str) -> dict:
|
|
return {"type": "function", "name": name, "parameters": {"type": "object", "properties": {}}}
|
|
|
|
|
|
_CORE = ["bash", "read", "write", "edit", "grep", "glob"]
|
|
_NONCORE = [f"slack_{i}" for i in range(10)] # 6 core + 10 non-core = 16 tools (>= min 12)
|
|
|
|
|
|
def _tools() -> list[dict]:
|
|
return [_fn(n) for n in _CORE + _NONCORE]
|
|
|
|
|
|
# --- model gating ------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.parametrize("model", ["gpt-5.4", "gpt-5.5", "gpt-5.4-2026-02-01", "gpt-6", "gpt-6.2"])
|
|
def test_model_supported(model):
|
|
assert _model_supports_openai_tool_search(model) is True
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"model", ["gpt-4o", "gpt-4.1", "gpt-5", "gpt-5.3", "o3", "", None, "claude-opus-4-8"]
|
|
)
|
|
def test_model_unsupported(model):
|
|
assert _model_supports_openai_tool_search(model) is False
|
|
|
|
|
|
def test_env_override_wins_then_falls_back(monkeypatch):
|
|
monkeypatch.setenv("HEADROOM_OPENAI_TOOL_SEARCH_MODELS", r"^my-model")
|
|
assert _model_supports_openai_tool_search("my-model-v1") is True
|
|
assert _model_supports_openai_tool_search("gpt-5.4") is False # override replaces the gate
|
|
# a malformed regex must not crash — fall back to the version gate.
|
|
monkeypatch.setenv("HEADROOM_OPENAI_TOOL_SEARCH_MODELS", "[unclosed")
|
|
assert _model_supports_openai_tool_search("gpt-5.4") is True
|
|
|
|
|
|
# --- deferral behavior -------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
("client", "supported"),
|
|
[(None, True), ("codex", False), (" CODEX ", False), ("opencode", False), ("claude", True)],
|
|
)
|
|
def test_client_supported(client, supported):
|
|
assert openai_tool_search_client_supported(client) is supported
|
|
|
|
|
|
def test_codex_client_does_not_inject():
|
|
tools = _tools()
|
|
|
|
out = inject_tool_search_deferral_openai(tools, "gpt-5.5", client="codex")
|
|
|
|
assert out is tools
|
|
assert all(tool.get("type") != "tool_search" for tool in out)
|
|
assert all("defer_loading" not in tool for tool in out)
|
|
|
|
|
|
@pytest.mark.parametrize("client", [None, "claude-code"])
|
|
def test_supported_clients_still_inject(client):
|
|
tools = _tools()
|
|
|
|
out = inject_tool_search_deferral_openai(tools, "gpt-5.5", client=client)
|
|
|
|
assert out is not tools
|
|
assert out[0] == {"type": "tool_search"}
|
|
assert any(tool.get("defer_loading") is True for tool in out)
|
|
|
|
|
|
def test_defers_non_core_and_injects_search_tool():
|
|
tools = _tools()
|
|
out = inject_tool_search_deferral_openai(tools, "gpt-5.5")
|
|
assert out is not tools # new list
|
|
assert out[0] == {"type": "tool_search"} # search tool injected, first, once
|
|
assert sum(1 for t in out if t.get("type") == "tool_search") == 1
|
|
by_name = {t["name"]: t for t in out if t.get("type") == "function"}
|
|
for c in _CORE:
|
|
assert not by_name[c].get("defer_loading") # core stays resident
|
|
for n in _NONCORE:
|
|
assert by_name[n].get("defer_loading") is True # non-core deferred
|
|
|
|
|
|
def test_terminal_reserved_namespace_stays_resident():
|
|
terminal = _fn("terminal")
|
|
tools = [terminal] + [_fn(f"peer_{i}") for i in range(11)]
|
|
snapshot = copy.deepcopy(tools)
|
|
|
|
out = inject_tool_search_deferral_openai(tools, "gpt-5.6-terra")
|
|
|
|
forwarded = next(t for t in out if t.get("name") == "terminal")
|
|
assert forwarded == terminal
|
|
assert "defer_loading" not in forwarded
|
|
assert next(t for t in out if t.get("name") == "peer_0").get("defer_loading") is True
|
|
assert tools == snapshot
|
|
|
|
|
|
def test_terminal_helper_remains_deferrable():
|
|
tools = [_fn("terminal_helper")] + [_fn(f"peer_{i}") for i in range(11)]
|
|
|
|
out = inject_tool_search_deferral_openai(tools, "gpt-5.6-terra")
|
|
|
|
helper = next(t for t in out if t.get("name") == "terminal_helper")
|
|
assert helper.get("defer_loading") is True
|
|
|
|
|
|
def test_prefixed_core_and_terminal_names_stay_resident():
|
|
resident = ["_bash", "_read", "_write", "_edit", "_glob", "_grep", "_terminal"]
|
|
noncore = ["_hub", "_todo", "_eval", "mcp__server__read", "terminal_helper"]
|
|
tools = [_fn(name) for name in resident + noncore]
|
|
|
|
out = inject_tool_search_deferral_openai(tools, "gpt-5.6-terra")
|
|
|
|
by_name = {tool["name"]: tool for tool in out if tool.get("type") == "function"}
|
|
for name in resident:
|
|
assert by_name[name].get("defer_loading") is None, name
|
|
for name in noncore:
|
|
assert by_name[name].get("defer_loading") is True, name
|
|
|
|
|
|
def test_defers_mcp_server():
|
|
tools = [_fn(n) for n in _CORE] + [{"type": "mcp", "server_label": "sentry"}]
|
|
tools += [_fn(f"x{i}") for i in range(8)]
|
|
out = inject_tool_search_deferral_openai(tools, "gpt-5.5")
|
|
mcp = next(t for t in out if t.get("type") == "mcp")
|
|
assert mcp.get("defer_loading") is True
|
|
|
|
|
|
def test_hosted_tools_stay_resident():
|
|
tools = [_fn(n) for n in _CORE] + [{"type": "web_search"}, {"type": "code_interpreter"}]
|
|
tools += [_fn(f"x{i}") for i in range(8)]
|
|
out = inject_tool_search_deferral_openai(tools, "gpt-5.5")
|
|
ws = next(t for t in out if t.get("type") == "web_search")
|
|
ci = next(t for t in out if t.get("type") == "code_interpreter")
|
|
assert "defer_loading" not in ws # hosted tools can't be deferred
|
|
assert "defer_loading" not in ci
|
|
|
|
|
|
def test_does_not_mutate_input():
|
|
tools = _tools()
|
|
snapshot = copy.deepcopy(tools)
|
|
inject_tool_search_deferral_openai(tools, "gpt-5.5")
|
|
assert tools == snapshot # deferred tools are copies; the input is untouched
|
|
|
|
|
|
# --- no-op guards ------------------------------------------------------------
|
|
|
|
|
|
def test_noop_for_unsupported_model():
|
|
tools = _tools()
|
|
assert inject_tool_search_deferral_openai(tools, "gpt-4o") is tools
|
|
|
|
|
|
def test_noop_below_min_tools():
|
|
tools = [_fn(f"x{i}") for i in range(5)] # < 12
|
|
assert inject_tool_search_deferral_openai(tools, "gpt-5.5") is tools
|
|
|
|
|
|
def test_noop_when_tool_search_already_present():
|
|
tools = [{"type": "tool_search"}] + [_fn(f"x{i}") for i in range(15)]
|
|
assert inject_tool_search_deferral_openai(tools, "gpt-5.5") is tools
|
|
|
|
|
|
def test_noop_when_nothing_deferrable():
|
|
tools = [_fn(n) for n in _CORE * 3] # 18 core tools, none deferrable
|
|
assert inject_tool_search_deferral_openai(tools, "gpt-5.5") is tools
|
|
|
|
|
|
def test_noop_for_non_list():
|
|
assert inject_tool_search_deferral_openai(None, "gpt-5.5") is None
|
|
|
|
|
|
def test_resident_names_match_case_insensitively():
|
|
# The resident-name sets are lowercase; clients are not required to be. An
|
|
# exact match deferred every tool for a PascalCase client, including its own
|
|
# tool-search tool. Mirrors the Anthropic-side fix.
|
|
tools = [_fn(n) for n in ("Bash", "Read", "Edit", "Terminal", "ToolSearch")] + [
|
|
_fn(f"slack_{i}") for i in range(10)
|
|
]
|
|
out = inject_tool_search_deferral_openai(tools, "gpt-5.5")
|
|
by_name = {t.get("name"): t for t in out if "name" in t}
|
|
for name in ("Bash", "Read", "Edit", "Terminal", "ToolSearch"):
|
|
assert by_name[name].get("defer_loading") is None, name
|
|
assert by_name["slack_0"].get("defer_loading") is True
|
|
|
|
|
|
# --- client-harness exclusion (GH #2660) -------------------------------------
|
|
|
|
|
|
def test_noop_for_a_client_that_cannot_execute_the_search_tool():
|
|
# GH #2660 reports opencode resolving tool calls against its own registry
|
|
# and rejecting the injected tool as unavailable, so its tools stay resident
|
|
# and untouched.
|
|
tools = _tools()
|
|
snapshot = copy.deepcopy(tools)
|
|
|
|
out = inject_tool_search_deferral_openai(tools, "gpt-5.5", client="opencode")
|
|
|
|
assert out is tools
|
|
assert tools == snapshot
|
|
assert not any(t.get("type") == "tool_search" for t in out)
|
|
assert not any(t.get("defer_loading") for t in out)
|
|
|
|
|
|
def test_supported_clients_keep_the_existing_deferral():
|
|
# The exclusion is per-client, not a global default flip: anything that can
|
|
# search still gets the same payload it got before.
|
|
tools = _tools()
|
|
|
|
explicit = inject_tool_search_deferral_openai(tools, "gpt-5.5", client="claude-code")
|
|
implicit = inject_tool_search_deferral_openai(tools, "gpt-5.5")
|
|
|
|
assert explicit == implicit
|
|
assert implicit[0] == {"type": "tool_search"}
|
|
assert any(t.get("defer_loading") for t in implicit)
|