From d1db00ab8697505b56b294c611cf93463b71810b Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Fri, 10 Jul 2026 23:38:27 -0400 Subject: [PATCH] fix(proxy): keep PRE_SEND from reintroducing empty tool arrays (#2015) ## Description The direct body-write fix for empty `tools: []` already landed, but the later OpenAI PRE_SEND write-back path still reintroduces the empty array. This aligns that guard with the existing direct-assignment contract so tools-free requests stay tools-free while explicit client `tools: []` stays preserved. Anthropic's current-main PRE_SEND path already had the equivalent empty-tools protection and needed no code change. Closes #1983 ## 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 - Mirror the direct `tools or _original_tools is not None` guard in the OpenAI PRE_SEND write-back path. - Leave Anthropic unchanged because current `main` already protects the empty-tools case there. - Extend the focused #728 regression file with PRE_SEND-specific coverage. - Add a changelog note for providers that reject empty `tools` arrays. ## Testing - [x] Unit tests pass - [x] Linting passes - [ ] Type checking passes - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_issue_728_empty_tools_injection.py -q 11 passed uv run ruff check headroom/proxy/handlers/openai.py tests/test_issue_728_empty_tools_injection.py All checks passed uv run ruff format --check headroom/proxy/handlers/openai.py tests/test_issue_728_empty_tools_injection.py 2 files already formatted ``` ## Real Behavior Proof - Environment: OpenAI-compatible provider that rejects empty `tools` arrays - Exact command / steps: send a request without `tools`, then repeat with explicit `tools: []` - Observed result: the OpenAI PRE_SEND path now skips `tools: []` when the client omitted tools, while the focused regression still preserves explicit client `tools: []` and deliberate clearing of a previously present tool list - Not tested: live provider run on this host - Scope: PRE_SEND request-body write-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 - [ ] 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 have updated the CHANGELOG.md if applicable ## Additional Notes The change is intentionally narrow. It only brings PRE_SEND write-back into parity with the direct-assignment guard that already exists. --- CHANGELOG.md | 1 + headroom/proxy/handlers/openai.py | 2 +- tests/test_issue_728_empty_tools_injection.py | 18 ++++++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcfdd1538..9b3630e1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Bug Fixes +* **proxy/openai:** stop PRE_SEND from reintroducing `tools: []` after the direct #728 fix. The OpenAI request handler now mirrors the existing `tools or _original_tools is not None` body-write guard during PRE_SEND write-back, so providers that reject empty tool arrays no longer see a tools field when the client omitted it, while explicit client `tools: []` remains preserved ([#1983](https://github.com/headroomlabs-ai/headroom/issues/1983)). * **proxy/openai:** keep the exact Responses function name `terminal` resident during OpenAI tool-search deferral so cache-mode optimization stops forwarding `terminal.terminal` and triggering the reserved-namespace 400 on Codex Responses ([#1946](https://github.com/headroomlabs-ai/headroom/issues/1946)). * **proxy/openai:** thread the savings-profile kwargs into the live `/v1/chat/completions` compression path. The chat handler called `openai_pipeline.apply()` without `proxy_pipeline_kwargs(config)`, so `HEADROOM_SAVINGS_PROFILE=agent-90` (and the individual `compress_user_messages`/`target_ratio`/`min_tokens_to_compress`/... knobs) were silently dropped — OpenAI-compatible clients like OpenCode kept protecting user messages and missed the configured profile. Both the token-mode and non-token chat branches now pass the profile kwargs, matching `handlers/anthropic.py` and the dedicated OpenAI compress endpoint ([#1534](https://github.com/headroomlabs-ai/headroom/issues/1534)). * **proxy:** forward Codex Desktop `/v1/responses` posts byte-faithfully so they stop returning upstream `400 {"detail":"Bad Request"}`. `handle_openai_responses` decoded the inbound body to inspect it but always re-serialized a canonical body on the way out, and it never stripped the inbound `content-encoding` header — so a `content-encoding: zstd` Codex Desktop request was forwarded as already-decoded JSON still advertising `zstd`, and the upstream ChatGPT Codex endpoint rejected it. The handler now keeps the original decoded bytes and forwards them verbatim whenever nothing (compression or memory injection) mutated the request, and drops the stale `content-encoding` header, mirroring the byte-faithful passthrough the chat and Anthropic paths already use ([#1542](https://github.com/headroomlabs-ai/headroom/issues/1542)). diff --git a/headroom/proxy/handlers/openai.py b/headroom/proxy/handlers/openai.py index df65095ed..133a5d58f 100644 --- a/headroom/proxy/handlers/openai.py +++ b/headroom/proxy/handlers/openai.py @@ -2856,7 +2856,7 @@ class OpenAIHandlerMixin: if presend_event.messages is not None: optimized_messages = presend_event.messages body["messages"] = optimized_messages - if presend_event.tools is not None: + if presend_event.tools or _original_tools is not None: tools = presend_event.tools body["tools"] = tools if presend_event.headers is not None: diff --git a/tests/test_issue_728_empty_tools_injection.py b/tests/test_issue_728_empty_tools_injection.py index 93b2d7f5f..aea99cbe8 100644 --- a/tests/test_issue_728_empty_tools_injection.py +++ b/tests/test_issue_728_empty_tools_injection.py @@ -40,6 +40,11 @@ def _should_set_body_tools(tools: list | None, original_tools: list | None) -> b return bool(tools or original_tools is not None) +def _should_apply_presend_tools(presend_tools: list | None, original_tools: list | None) -> bool: + """Mirror the fixed PRE_SEND write-back condition in the OpenAI handler.""" + return bool(presend_tools or original_tools is not None) + + def _sort_tools(tools: list | None) -> list | None: return AnthropicHandlerMixin._sort_tools_deterministically(tools) @@ -110,6 +115,19 @@ class TestHandlerGuardCondition: assert not _legacy_should_set_body_tools_after_sort(tools_after_helpers, original_tools) assert _should_set_body_tools_after_sort(tools_after_helpers, original_tools) + def test_presend_empty_list_stays_omitted_when_client_omitted_tools(self): + """PRE_SEND must not re-introduce ``tools: []`` for a tools-free request.""" + assert not _should_apply_presend_tools([], None) + + def test_presend_preserves_explicit_client_empty_tools(self): + """PRE_SEND must still preserve an explicit client ``tools: []`` field.""" + assert _should_apply_presend_tools([], []) + + def test_presend_can_clear_previously_present_tools(self): + """PRE_SEND may deliberately replace a real tool list with ``[]``.""" + original_tools = [{"type": "function", "function": {"name": "my_tool"}}] + assert _should_apply_presend_tools([], original_tools) + # --------------------------------------------------------------------------- # apply_session_sticky_ccr_tool behaviour with no existing tools