mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
7 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Joseph Benno
|
2f4d001c9f
|
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 |
||
|
Copilot
|
8ea87e7804
|
fix: tool_search_tool_regex deferred and falsely resolved on direct-Anthropic path (#2971)
## Description Direct Anthropic users could receive `400 Tool reference 'tool_search_tool_regex' not found in available tools` when Claude Code sent a typeless `tool_search_tool_regex` entry. Headroom treated it as an ordinary deferrable tool, injected a typed search tool with the same name, and later mistook that typed search mechanism for a valid target of the stale `tool_reference`. This change prevents the duplicate injection and repairs already-poisoned transcripts without stripping valid references to ordinary deferred tools. It addresses the first-party Anthropic regression reported in [PR #2539's follow-up](https://github.com/headroomlabs-ai/headroom/pull/2539#issuecomment-5280259642) and complements the history repair from #2805. ## 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 - recognize typeless, case-insensitive `tool_search_tool_*` names as an existing client tool-search surface and skip Headroom's duplicate injection - exclude typed Anthropic search mechanisms from the set of valid `tool_reference` targets - preserve valid regular deferred-tool references and the normal deferral path for similar non-reserved names - add a first-party Anthropic handler regression that proves the outbound tools remain unchanged and stale search bookkeeping is removed - rebase onto #2996, which prevents the native detector from hanging the full CI test shard ## 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/Scripts/python.exe -m pytest \ tests/test_issue_746_tool_search.py \ tests/test_anthropic_stage_timings.py \ tests/test_cache_control_ttl_order.py \ tests/test_cache_ttl_preserved.py \ tests/test_proxy/test_tool_search_repair_after_turn_hooks.py \ tests/test_transforms/test_detect_fallback_1123.py \ tests/test_transforms_content_detection.py \ tests/test_transforms_content_router.py \ -q --disable-warnings --maxfail=1 170 passed, 1 warning in 10.27s .venv/Scripts/ruff.exe check . All checks passed! .venv/Scripts/ruff.exe format --check . 1411 files already formatted pre-commit run mypy --all-files Success: no issues found in 519 source files git diff --check (no output) ``` ## Real Behavior Proof - Environment: Windows, Python 3.13.3, first-party Anthropic handler test with `HEADROOM_TOOL_SEARCH` at its default enabled setting - Exact command / steps: run the same helper-level payload against the pre-fix base and this branch, then run `test_anthropic_direct_path_repairs_typeless_tool_search_regression` through `handle_anthropic_messages()` with 20 ordinary tools, one typeless `tool_search_tool_regex`, and a stale self-reference - Observed result: before the fix, Headroom injected a second typed search tool, deferred the typeless client tool, and removed 0 stale blocks; on this branch, it skips duplicate injection, preserves the client tools array, and removes the paired `server_tool_use` and `tool_search_tool_result` blocks before forwarding - Not tested: a live request against a paid Anthropic account; the production handler's outbound body is captured before the network boundary instead ## Runtime Rollout Safety - Rollout-managed feature(s): Anthropic server-side tool-search deferral (`HEADROOM_TOOL_SEARCH`) - Minimum rollout channel: standard CI; narrow corrective change to an existing default-on path - Stable/default behavior changed: yes; reserved typeless client search tools now suppress duplicate injection, and typed search mechanisms no longer satisfy deferred-tool references - Kill switch / disable path: set `HEADROOM_TOOL_SEARCH=0` to disable new injection; history repair remains unconditional so existing poisoned sessions can recover - Unsafe override required: no - Qualification impact: no new rollout surface or configuration; focused handler, helper, cache-control, and hook-order regressions cover the affected path - Rollback path: revert this PR; operators can set `HEADROOM_TOOL_SEARCH=0` while rolling back ## 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 - [x] 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 — proxy request transformation only. ## Additional Notes - Documentation is not changed because this fixes internal request classification and transcript repair without adding a user-facing option or workflow. - Anthropic documents `tool_search_tool_regex` / `tool_search_tool_bm25` as server search mechanisms; deferred definitions, rather than the search mechanism itself, are the valid `tool_reference` targets. - Rebased onto #2996, which fixes the unrelated native-detector hang that timed out shard 4 on the prior merge commit. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
Rod Boev
|
7f6950be34
|
fix(anthropic): strip first-party tool search on custom upstreams (#2539)
## Description Third-party Anthropic-compatible upstreams can reject Headroom-routed Claude requests before generation starts because the forwarded `tools[]` array still contains the first-party Anthropic server tool type `tool_search_tool_regex_20251119`. That path is valid when the upstream really is Anthropic, but DeepSeek-style Anthropic-compatible gateways reject it with a 400 and never reach model execution. This change strips first-party Anthropic `tool_search_tool_*` entries only when Headroom forwards an Anthropic-wire request to a third-party upstream selected through `anthropic_api_url`. Direct Anthropic behavior stays intact, and unrelated typed or untyped tools keep their existing forwarding contract. Closes #2526. ## 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 - add a narrow Anthropic helper that strips first-party `tool_search_tool_*` entries from client-supplied tool lists when the outbound target is a third-party Anthropic-compatible upstream - wire the sanitizer into the Anthropic handler's third-party forwarding path without changing the first-party `HEADROOM_TOOL_SEARCH` injector branch - add focused helper coverage for third-party stripping, first-party preservation, and typed-tool negative space - add a production-path regression through `handle_anthropic_messages()` that captures the custom-upstream request body and verifies the sanitizer wiring ## Testing - [x] Unit tests pass (`uv run pytest tests/test_issue_746_tool_search.py tests/test_anthropic_stage_timings.py -q`) - [x] Linting passes (`uv run ruff check headroom/proxy/helpers.py headroom/proxy/handlers/anthropic.py tests/test_issue_746_tool_search.py tests/test_anthropic_stage_timings.py`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality when applicable - [x] Manual testing performed ### Test Output ```text uv run pytest tests/test_issue_746_tool_search.py tests/test_anthropic_stage_timings.py -q 50 passed in 0.72s uv run ruff check headroom/proxy/helpers.py headroom/proxy/handlers/anthropic.py tests/test_issue_746_tool_search.py tests/test_anthropic_stage_timings.py All checks passed! uv run ruff format headroom/proxy/helpers.py headroom/proxy/handlers/anthropic.py tests/test_issue_746_tool_search.py tests/test_anthropic_stage_timings.py --check 4 files already formatted git diff --check (no output) ``` ## Real Behavior Proof - Environment: focused Headroom worktree with Anthropic-wire regression tests - Exact command / steps: use the issue reproduction at https://github.com/headroomlabs-ai/headroom/issues/2526, then run the focused helper and handler tests; the handler regression calls `handle_anthropic_messages()` with a DeepSeek-compatible upstream and captures the outbound request body - Observed result: the base repro printed `FAIL issue2526 third-party sanitize -> [{'type': 'tool_search_tool_regex_20251119', 'name': 'tool_search_tool_regex'}, {'name': 'Bash', 'description': 'run a command', 'input_schema': {}}]`, while the head repro printed `PASS issue2526 third-party sanitize -> [{'name': 'Bash', 'description': 'run a command', 'input_schema': {}}]`; the handler-level test captured the same removal while preserving `Bash` and `web_search_20250305`, and the combined focused run passed 50 tests - Not tested: live DeepSeek account on this host ## 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 - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A - proxy forwarding change only. ## Additional Notes - `CHANGELOG.md` stays untouched because Headroom's release automation generates it from conventional commits. - The narrow slice strips only first-party Anthropic server tool-search entries on third-party Anthropic-compatible upstreams. It does not invent or translate third-party search-tool semantics. --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: JD Davis <jd@jds-macbook-air.tail2a279.ts.net> |
||
|
Tejas Chopra
|
0237cbffbb
|
fix(proxy): enable tool search by default and repair poisoned transcripts (#2807)
## Description Server-side tool search poisons the Claude Code transcript: once the proxy injects deferral and the model runs one search, Anthropic's `server_tool_use` + `tool_search_tool_result` pair lives in the message history forever. Upstream validates **every `tool_reference` in that history against the *current* request's `tools` array** — and Claude Code replays one transcript across requests with wildly different tools arrays (main loop: hundreds of tools; prompt-type Stop hook evaluator, `/compact`, other side-requests: a handful). Every one of those side-requests 400s with `Tool reference 'X' not found in available tools`. This PR keeps tool search **on** — it's the whole point of the feature, and the default `coding` savings profile already turned it on at proxy startup — and instead repairs the transcript per request, statelessly. The issue author's preferred fix (never inject for Claude Code clients) would disable the feature for its main audience. A session-sticky approach was also considered and rejected: it needs session state, it can't re-add ~500 tool definitions to a 5-tool side-request without erasing the savings, and it can't heal transcripts already poisoned before the upgrade. Closes #2805 ## 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) - [x] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - **`headroom/proxy/helpers.py`** — new `strip_unsupported_tool_search_blocks(messages, tools)`. Builds the set of names this request can resolve, drops any `tool_search_tool_result` whose `tool_reference` entries aren't all resolvable (or when no search tool is present at all), and drops the paired `server_tool_use` by `tool_use_id`. Other server tools (`web_search`, code execution) are untouched. Turns left with zero content blocks are removed rather than forwarded empty. Copy-on-write: returns the **original** `messages` object by identity when nothing was removed. - **`headroom/proxy/handlers/anthropic.py`** — runs the repair right after the injection block, so the tool just injected counts as present and the main loop is a no-op with a byte-identical prefix. Deliberately **not** gated on `HEADROOM_TOOL_SEARCH`, so transcripts poisoned before an upgrade (or before someone sets the flag to `0`) still recover. Logs and tags `router:tool_search_repair:Nblocks` when it fires. - **`headroom/proxy/handlers/anthropic.py`** — `HEADROOM_TOOL_SEARCH` now defaults to `1`. This matches the posture `seed_proxy_env_defaults()` already established for the default `coding` profile; the flip only affects entry points that never seeded. - **`docs/content/docs/proxy.mdx`** — documents on-by-default plus `HEADROOM_TOOL_SEARCH=0` as the opt-out. - **`tests/test_issue_746_tool_search.py`** — 6 tests covering the repair. ### Answering the issue's open question > we could not determine what enables it — `/proc/<pid>/environ` shows no `HEADROOM_TOOL_SEARCH` `seed_proxy_env_defaults()` calls `os.environ.setdefault("HEADROOM_TOOL_SEARCH", "1")` at proxy startup because the default savings profile is `coding`, which has `tool_search=True` (`headroom/agent_savings.py`). In-process mutation of `os.environ` never appears in the process's environ snapshot, which is why the flag looked unset. ## 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 $ python -m pytest tests/test_issue_746_tool_search.py -q 45 passed, 1 warning in 1.56s $ python -m pytest tests/test_*anthropic*.py tests/test_*tool*.py -q 4 failed, 459 passed, 2 skipped, 7 warnings in 27.40s # the 4 failures are in tests/test_bedrock_tool_result_cache_and_streaming_stats.py # and reproduce identically on this branch's merge-base with the changes stashed: # 4 failed, 9 passed, 5 warnings in 3.02s $ ruff check headroom/proxy/helpers.py headroom/proxy/handlers/anthropic.py tests/test_issue_746_tool_search.py All checks passed! $ ruff format --check <same three files> 3 files already formatted $ mypy --python-version 3.12 headroom/proxy/helpers.py headroom/proxy/handlers/anthropic.py Success: no issues found in 2 source files # --python-version 3.12 only to skip a pre-existing numpy-stub syntax error that # the repo's python_version = "3.10" setting triggers on this machine. ``` New tests: | Test | Asserts | |---|---| | `test_repair_drops_blocks_the_hook_evaluator_cannot_resolve` | small tools array → both blocks dropped, surrounding assistant text survives | | `test_repair_is_noop_on_the_main_loop` | search tool + referenced tool present → `removed == 0` and `messages is transcript` (prefix cache untouched) | | `test_repair_drops_a_turn_left_with_no_blocks` | a turn that was *only* the search round-trip is removed, not forwarded empty | | `test_repair_leaves_other_server_tools_alone` | `web_search` `server_tool_use` blocks survive | | `test_repair_is_idempotent` | second pass over a repaired transcript removes nothing | | `test_repair_strips_search_history_when_only_the_tool_is_missing` | references resolvable but no search tool in the array → still stripped | ## Real Behavior Proof - **Environment:** macOS 25.4.0, Python 3.12 venv, live `api.anthropic.com`, `claude-sonnet-4-6`, local proxy on `127.0.0.1:8799` built from this branch. - **Exact command / steps:** one request body — a poisoned transcript (`server_tool_use` + `tool_search_tool_result` referencing `AskUserQuestion`) with a **1-tool** `tools` array (`Read`), exactly the shape a Claude Code side-request replays — sent twice: once straight to `https://api.anthropic.com`, once to the proxy. ```text $ python /tmp/hr-2805-repro.py https://api.anthropic.com HTTP 400 {"type": "invalid_request_error", "message": "Tool reference 'AskUserQuestion' not found in available tools"} $ python /tmp/hr-2805-repro.py http://127.0.0.1:8799 HTTP 200 content: [{"type": "text", "text": "OK"}] ``` - **Observed result:** the exact 400 from the issue reproduces against upstream; the identical body through the proxy returns 200. The proxy's savings event for that request records `before: 133, after: 32, saved: 101` tokens — the two dropped blocks. The one-tool array is below `_TOOL_SEARCH_MIN_TOOLS = 12`, so no injection ran; the repair alone is what made the request valid. - **Not tested:** a full end-to-end Claude Code session with a real Stop hook (the synthetic replay above is the same request shape the hook evaluator produces); non-Anthropic providers, which don't have server-side tool search. ## 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 - [x] 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 - [x] New and existing unit tests pass locally with my changes - [x] I did **not** edit `CHANGELOG.md` ## Screenshots (if applicable) N/A — proxy-side behavior, covered by the command output above. ## Additional Notes - **Cache cost is zero on the hot path.** The repair only rewrites requests whose transcripts reference tools they don't carry — request families that were 400ing anyway. The main loop takes the identity path and its prefix stays byte-identical. - **Out of scope, spotted while here:** `run-all-plugins.sh` exports `HEADROOM_TOOL_SEARCH_MIN_TOOLS=5`, but nothing in Python reads it — `_TOOL_SEARCH_MIN_TOOLS` is a hardcoded `12`. Worth a follow-up. |
||
|
Fabien Culpo
|
1d29738818
|
fix(proxy): keep core tools and the client's ToolSearch resident for PascalCase clients (#2647)
## Description `_TOOL_SEARCH_CORE_TOOLS` is spelled in lowercase, but the membership test compared the raw tool name, so the core-tool exemption never fired for clients that send PascalCase names. For Claude Code (`Bash`, `Read`, `Edit`, `ToolSearch`) **every** tool in the request body was deferred. The damaging part is that Claude Code's own `ToolSearch` was deferred. It is the schema fetcher for tools the client keeps in its local registry and never sends in the body — `TaskCreate`, `TaskUpdate`, `TaskList`, `WebFetch`, `EnterPlanMode`, `Monitor`, `LSP`, `Cron*`, `SendMessage`. Hiding it makes all of them permanently uncallable: advertised to the model in a `<system-reminder>`, but no search can return their schemas, because the injected `tool_search_tool_regex` only indexes what is in the request body. Closes #2646 ## 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 - Compare tool names against the core set case-insensitively in `inject_tool_search_deferral` (`helpers.py`). - Add `"toolsearch"` to `_TOOL_SEARCH_CORE_TOOLS` so a client's own schema-fetch tool is never deferred. - Apply the same case-insensitive comparison to `inject_tool_search_deferral_openai`, which had the identical exact-match bug (including against `_OPENAI_TOOL_SEARCH_RESIDENT_NAMES = {"terminal"}`). - Add 3 tests on the Anthropic path and 1 on the OpenAI path. Both source changes are required: case-folding alone does not help `ToolSearch` (it was not in the set), and adding it alone does not help `Bash`/`Read`/`Edit`. **The token saving is unchanged** — MCP tools are still deferred. This is not a request to disable the feature. Beyond the stranded tools, the old behaviour also meant (a) routine `Bash`/`Read`/`Edit` loops each paid a search round-trip, the exact cost the core set exists to avoid, and (b) zero resident *real* tools remained, silently violating the invariant documented on `inject_tool_search_deferral` — the injected search tool is typed and does not satisfy it — which risks an upstream 400. The existing assertion for that invariant passes today only because its fixture uses lowercase names. ## Testing - [x] Unit tests pass (`pytest`) — the two affected files; see scope note below - [ ] Linting passes (`ruff check .`) — see note - [ ] Type checking passes (`mypy headroom`) — could not run, see note - [x] New tests added for new functionality - [x] Manual testing performed `ruff check .` reports 4 findings repo-wide, **all pre-existing and unrelated** (`plugins/headroom-oauth2/`), confirmed identical on unmodified `main`. Zero findings in the three files this PR touches, and `ruff format --check` is clean on all three. Left unchecked because the repo-wide command does not exit 0. `mypy headroom` could not run in my environment (numpy stubs error out under the resolved Python version before checking begins). Not attempted further — CI should be the authority. ### Test Output ```text $ python -m pytest tests/test_issue_746_tool_search.py tests/test_openai_tool_search_deferral.py -q 65 passed, 1 warning in 0.70s # Baseline on those two files before this PR: 62 (36 + 26). # The 3 new Anthropic tests + 1 new OpenAI test bring it to 65. # Red before the source change (tests written first): tests/test_issue_746_tool_search.py::test_core_tools_match_case_insensitively FAILED AssertionError: Bash assert True is None where {'name': 'Bash', ..., 'defer_loading': True}.get('defer_loading') tests/test_issue_746_tool_search.py::test_client_tool_search_tool_is_never_deferred FAILED AssertionError: assert True is None where {'name': 'ToolSearch', ..., 'defer_loading': True}.get('defer_loading') tests/test_issue_746_tool_search.py::test_resident_real_tool_survives_pascal_case_surface FAILED assert any(not t.get("type") and not t.get("defer_loading") for t in out) assert False 3 failed, 36 deselected $ python -m ruff check headroom/proxy/helpers.py tests/test_issue_746_tool_search.py tests/test_openai_tool_search_deferral.py All checks passed! $ python -m ruff format --check <same three files> 3 files already formatted ``` ## Real Behavior Proof - Environment: headroom 0.32.1 installed / 0.32.0 source, Python 3.13, macOS 15 (Darwin 25.5.0), Claude Code 2.1.220 with `ENABLE_TOOL_SEARCH=true` and `ANTHROPIC_BASE_URL=http://localhost:8787`, first-party Anthropic upstream, `HEADROOM_TOOL_SEARCH` truthy - Exact command / steps: build a Claude Code tool surface and pass it through the injector — `names = ["Bash","Read","Write","Edit","Glob","Grep","ToolSearch"] + [f"mcp__srv__t{i}" for i in range(12)]`, `tools = [{"name": n, "description": n, "input_schema": {}} for n in names]`, then `inject_tool_search_deferral(tools)` and print which entries carry `defer_loading` - Observed result: before the fix `resident real tools: []` with `ToolSearch deferred: True` (every built-in deferred). After the fix `resident real tools: ['Bash','Edit','Glob','Grep','Read','ToolSearch','Write']` with all 12 `mcp__srv__t*` still deferred, so the saving is retained. This matches a live session: the proxy logged `router:tool_search_deferral:25tools:22182tok ... client=claude-code` and `tool_search_tool_regex` could resolve only `mcp__*` tools — `TaskCreate`/`WebFetch`/`EnterPlanMode` returned no match until `ToolSearch` was recovered by regex-searching for it and then calling `select:TaskCreate,...` - Not tested: the full pytest suite (164 modules fail collection with `ModuleNotFoundError: No module named 'headroom._core'` because my environment imports the package via `PYTHONPATH` without building the Rust extension; identical failure confirmed on unmodified `main`, so it is environmental). `mypy headroom` not runnable here. No end-to-end run against a live upstream through a rebuilt proxy — verification is at the function boundary plus the live-session log evidence above. The OpenAI Responses path is covered by unit test only, not exercised against a real gpt-5.4+ deployment. ## 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) ## Additional Notes - **Documentation**: N/A — no user-facing surface changes; behaviour returns to what the existing comments and docstring already describe. - **"New and existing unit tests pass locally"**: left unchecked deliberately. The tests covering the changed symbols pass (65), but I cannot run the whole suite locally without the compiled `headroom._core`. Not claiming more than I verified. - **Scope**: the OpenAI-path fix rides along because it is the identical three-line comparison bug in the sibling function. Happy to split it into its own PR if you would rather keep this Anthropic-only. - **Deliberately not done**: I did not add a `client != "claude-code"` gate at `handlers/anthropic.py`, even though the feature's own comment block scopes it to non-Claude-Code clients and `client=claude-code` is already known there (it appears in the `transforms=` log line). Gating there would forfeit the ~22k tokens/request currently saved on Claude Code's eagerly-shipped MCP schemas; keeping the meta-tool resident preserves both the saving and reachability. Flagging in case you would prefer to gate as well. - **Adjacent blind spot, out of scope**: `claude_code_tool_search_inactive` already checks both the tools array *and* the `anthropic-beta` header, but the injector's early-return guard checks only the array. That is why a plain-function `ToolSearch` slips past it and the injection runs on a client that is already deferring. Co-authored-by: Fabien Culpo <fabien.culpo@dawex.com> |
||
|
Tejas Chopra
|
0ba5065d40
|
Tejas/tool search deferral (#1885)
## Description <!-- Briefly explain the change and why it is needed. --> Closes # ## Type of Change - [ ] 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 - ## Testing <!-- Check what you actually ran, then paste the real command output below. --> - [ ] Unit tests pass (`pytest`) - [ ] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [ ] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text # Paste relevant command output or artifact links here ``` ## Real Behavior Proof - Environment: - Exact command / steps: - Observed result: - Not tested: ## Review Readiness - [ ] I have performed a self-review - [ ] This PR is ready for human review ## Checklist - [ ] My code follows the project's style guidelines - [ ] I have performed a self-review of my code - [ ] 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 - [ ] 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 - [ ] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) Add screenshots to help explain your changes. ## Additional Notes <!-- Mention any N/A checklist items, tradeoffs, follow-ups, or maintainer context. --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Tejas Chopra
|
1c8c538780
|
fix: preserve Claude Code tool-search deferral through the proxy (#746) (#753)
Routing Claude Code through the proxy disabled its on-demand tool loading: with a custom ANTHROPIC_BASE_URL and ENABLE_TOOL_SEARCH unset, Claude Code stops deferring MCP/system tool schemas behind the server-side Tool Search Tool and materializes them all into local context (~25K tokens) — the opposite of what a context-optimization proxy should do. Root cause is a client-side gate in Claude Code (isToolSearchEnabledOptimistic): deferral is disabled when ENABLE_TOOL_SEARCH is unset AND provider is first-party AND the base-URL host is not api.anthropic.com. It is a one-way URL check, not a capability handshake, so no proxy/response header can flip it. The only lever is the ENABLE_TOOL_SEARCH env var Claude Code reads at startup. Changes: - wrap claude: inject ENABLE_TOOL_SEARCH into the launched Claude Code env (default "true"; --tool-search true|auto|auto:N|false; a pre-set env value is respected; blank is treated as unset). Keeps deferral on through the proxy. - proxy: emit a one-time, actionable hint when a Claude Code request is detected loading tools eagerly (for users who run `claude` manually). Gated on a cheap one-shot flag and wrapped so it can never fail a request. - docs: troubleshooting section with before/after verification. - tests: 30 unit tests (value validation, injection precedence, detection, hint content, one-shot guard). |