From 4951cf80a2ddd8c08daa64d9da3227de229e0403 Mon Sep 17 00:00:00 2001 From: Tejas Chopra Date: Tue, 14 Jul 2026 04:01:28 -0400 Subject: [PATCH] fix(proxy): don't 502 Anthropic streaming on a legal mixed CCR + client-tool turn (#2089) (#2117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Anthropic **buffered-streaming** returned a **502** on a legal turn: when the model emits `headroom_retrieve` alongside a non-CCR client tool, `CCRResponseHandler` intentionally skips CCR resolution (#839) and hands both tool_use blocks back for the client to resolve. The non-streaming path returns that as 200; the streaming path wrongly failed closed with "Unable to safely complete streamed CCR retrieval." Fix: add a provider-generic `CCRResponseHandler.residual_ccr_status()` → `resolved` / `skipped_mixed_tools` / `error`. The streaming path now only 502s on a genuine `error`; on the intentional mixed-tool skip it falls through to the existing SSE resynthesis (200) preserving **both** tool_use blocks — matching the non-streaming path. The misleading "handled successfully" log no longer fires on skip. Closes #2089 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/ccr/response_handler.py`: shared, provider-generic `residual_ccr_status()`. - `headroom/proxy/handlers/anthropic.py`: streaming path fails closed only on a real residual-CCR error; passes through the legal skip as 200 SSE. - `tests/test_proxy/test_anthropic_streaming_ccr_retrieve.py`: mixed-tool case now returns 200 SSE preserving both blocks. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] New/updated tests for the mixed-tool pass-through branch ### Test Output ```text GitHub CI on current head: - lint: pass - build/build-wheel/build-wheel-windows: pass - test matrix, test-agno, test-extras, test-dashboard-ui: pass - docker-native-e2e: pass Review spot-check: uv run --extra proxy --extra dev python -m pytest tests/test_proxy/test_anthropic_streaming_ccr_retrieve.py tests/test_ccr_response_handler_extra.py -q 15 passed, 1 warning ``` ## Real Behavior Proof - Environment: GitHub Actions on PR head `c3b2522d`, plus focused Windows review worktree spot-check. - Before: `stream:true` mixed internal+client tool turn → deterministic 502. - After: 200 SSE preserving both `headroom_retrieve` and the client tool_use. - Not tested yet: direct unit coverage for the new `residual_ccr_status()` classifier and the residual-CCR error classification. ## Review Readiness - [x] I have performed a self-review - [ ] This PR is ready for human review --- headroom/ccr/response_handler.py | 46 +++++++++++ headroom/proxy/handlers/anthropic.py | 41 +++++++++- .../test_anthropic_streaming_ccr_retrieve.py | 77 +++++++++++++++++-- 3 files changed, 156 insertions(+), 8 deletions(-) diff --git a/headroom/ccr/response_handler.py b/headroom/ccr/response_handler.py index 80bf60998..f81fe2236 100644 --- a/headroom/ccr/response_handler.py +++ b/headroom/ccr/response_handler.py @@ -30,6 +30,25 @@ from .tool_injection import CCR_TOOL_NAME logger = logging.getLogger(__name__) +# Residual-CCR status signals (provider-generic). +# +# ``handle_response`` may return a response that still contains +# ``headroom_retrieve`` tool calls. Callers need to know *why* so they can +# decide whether that is a safe passthrough or a genuine failure: +# +# - RESIDUAL_CCR_RESOLVED: no CCR tool calls remain — fully handled. +# - RESIDUAL_CCR_SKIPPED_MIXED: CCR was intentionally skipped because the model +# emitted headroom_retrieve alongside a non-CCR +# client tool (#839). The client must resolve both +# tool calls; the proxy must pass the turn through +# unchanged (200), not fail closed. +# - RESIDUAL_CCR_ERROR: CCR tool calls remain with no accompanying client +# tool — i.e. a real conversion/handling failure the +# proxy could not resolve. Callers should fail closed. +RESIDUAL_CCR_RESOLVED = "resolved" +RESIDUAL_CCR_SKIPPED_MIXED = "skipped_mixed_tools" +RESIDUAL_CCR_ERROR = "error" + @dataclass class CCRToolResult: @@ -110,6 +129,33 @@ class CCRResponseHandler: """ return has_ccr_tool_calls(response, provider) + def residual_ccr_status( + self, + response: dict[str, Any], + provider: str = "anthropic", + ) -> str: + """Classify why (if at all) CCR tool calls remain in a handled response. + + This is a stateless, provider-generic signal derived from the same + parsing ``handle_response`` uses, so it stays correct under concurrency + and works identically for every provider/harness. + + Returns one of: + - ``RESIDUAL_CCR_RESOLVED``: no headroom_retrieve tool calls remain. + - ``RESIDUAL_CCR_SKIPPED_MIXED``: headroom_retrieve remains *alongside* + a non-CCR client tool call. This is an intentional skip (#839) — the + proxy cannot synthesize the client tool_result, so the turn must be + handed back to the client unchanged rather than failed closed. + - ``RESIDUAL_CCR_ERROR``: headroom_retrieve remains with no accompanying + client tool call — a genuine handling/conversion failure. + """ + ccr_calls, other_calls = self._parse_ccr_tool_calls(response, provider) + if not ccr_calls: + return RESIDUAL_CCR_RESOLVED + if other_calls: + return RESIDUAL_CCR_SKIPPED_MIXED + return RESIDUAL_CCR_ERROR + def _extract_tool_calls( self, response: dict[str, Any], diff --git a/headroom/proxy/handlers/anthropic.py b/headroom/proxy/handlers/anthropic.py index fcb752e5b..99117475a 100644 --- a/headroom/proxy/handlers/anthropic.py +++ b/headroom/proxy/handlers/anthropic.py @@ -2918,7 +2918,27 @@ class AnthropicHandlerMixin: content=ccr_content, headers=ccr_response_headers, ) - logger.info(f"[{request_id}] CCR: Retrieval handled successfully") + # Only claim success when no headroom_retrieve remains. + # On an intentional mixed-tool skip (#839) the response + # still carries headroom_retrieve for the client to + # resolve — logging "handled successfully" there is + # misleading. Classify via the shared, provider-generic + # residual-CCR signal. + from headroom.ccr.response_handler import ( + RESIDUAL_CCR_SKIPPED_MIXED, + ) + + residual_status = self.ccr_response_handler.residual_ccr_status( + final_resp_json, "anthropic" + ) + if residual_status == RESIDUAL_CCR_SKIPPED_MIXED: + logger.info( + f"[{request_id}] CCR: Skipped retrieval — " + "headroom_retrieve returned alongside a client " + "tool for the client to resolve" + ) + else: + logger.info(f"[{request_id}] CCR: Retrieval handled successfully") except Exception as e: import traceback @@ -3231,13 +3251,28 @@ class AnthropicHandlerMixin: } return f"event: error\ndata: {json.dumps(error_event)}\n\n".encode() + # Residual headroom_retrieve is only a hard failure when it + # is NOT an intentional mixed-tool skip. When the model + # emitted headroom_retrieve alongside a client tool (#839), + # the handler deliberately leaves both tool_use blocks in + # place for the client to resolve — a legal turn that the + # non-streaming path returns as 200. Fall through to the + # SSE resynthesis below so the stream:true path matches it + # and preserves both blocks. Use the shared, provider-generic + # residual-CCR signal (not an Anthropic-only branch). + from headroom.ccr.response_handler import RESIDUAL_CCR_ERROR + if ( self.ccr_response_handler - and self.ccr_response_handler.has_ccr_tool_calls(resp_json, "anthropic") + and self.ccr_response_handler.residual_ccr_status( + resp_json, "anthropic" + ) + == RESIDUAL_CCR_ERROR ): logger.warning( f"[{request_id}] CCR: Buffered streaming response still " - "contains headroom_retrieve after handling; failing closed" + "contains an unresolved headroom_retrieve after handling; " + "failing closed" ) async def _residual_ccr_error_sse(): diff --git a/tests/test_proxy/test_anthropic_streaming_ccr_retrieve.py b/tests/test_proxy/test_anthropic_streaming_ccr_retrieve.py index 9951a08b6..139a6fe09 100644 --- a/tests/test_proxy/test_anthropic_streaming_ccr_retrieve.py +++ b/tests/test_proxy/test_anthropic_streaming_ccr_retrieve.py @@ -223,7 +223,12 @@ def test_streaming_with_headroom_retrieve_available_but_unused_returns_sse() -> proxy._stream_response.assert_not_awaited() -def test_mixed_ccr_and_client_tool_does_not_issue_continuation() -> None: +def test_mixed_ccr_and_client_tool_streams_both_blocks_as_sse() -> None: + """LEGAL mixed turn (#839, #2089): headroom_retrieve emitted alongside a + client tool. The proxy cannot synthesize the client tool_result, so it must + hand the turn back for the client to resolve — a 200 SSE stream preserving + BOTH tool_use blocks, matching the non-streaming path. It must NOT 502 and + must NOT issue a continuation request.""" config = _make_config() initial_response = _message_response( [ @@ -247,6 +252,9 @@ def test_mixed_ccr_and_client_tool_does_not_issue_continuation() -> None: app = create_app(config) with TestClient(app) as client: proxy = client.app.state.proxy + proxy._stream_response = AsyncMock( + side_effect=AssertionError("live streaming path should not be used") + ) continuation_client = _ContinuationClient(_message_response([])) proxy.http_client = continuation_client @@ -275,9 +283,68 @@ def test_mixed_ccr_and_client_tool_does_not_issue_continuation() -> None: }, ) - assert resp.status_code == 502, resp.text + assert resp.status_code == 200, resp.text assert "text/event-stream" in resp.headers["content-type"] - assert "headroom_retrieve" not in resp.text - assert "client_tool" not in resp.text - assert "Unable to safely complete streamed CCR retrieval" in resp.text + # Both tool_use blocks are preserved for the client to resolve. + assert "headroom_retrieve" in resp.text + assert "client_tool" in resp.text + assert "toolu_ccr" in resp.text + assert "toolu_client" in resp.text + assert "Unable to safely complete streamed CCR retrieval" not in resp.text + # No continuation is issued — the client resolves all tool calls. assert continuation_client.post_calls == [] + + +def test_unresolved_ccr_only_streams_through_as_200() -> None: + """CCR-only turn that never resolves: the model keeps re-emitting + headroom_retrieve so the continuation exhausts its retrieval rounds with a + residual marker and no accompanying client tool. Per #2089 the streaming + path no longer hard-502s here — it streams the residual headroom_retrieve + back as a 200 SSE so the client (which owns the tool) can resolve or retry + it, matching the non-streaming path. It must NOT 502.""" + config = _make_config() + persistent_ccr = _message_response( + [ + { + "type": "tool_use", + "id": "toolu_ccr", + "name": "headroom_retrieve", + "input": {"hash": "deadbeef"}, + }, + ], + stop_reason="tool_use", + ) + + with patch("headroom.proxy.server.AnyLLMBackend"): + app = create_app(config) + with TestClient(app) as client: + proxy = client.app.state.proxy + proxy._stream_response = AsyncMock( + side_effect=AssertionError("live streaming path should not be used") + ) + # Every continuation re-emits headroom_retrieve, so it never resolves. + continuation_client = _ContinuationClient(persistent_ccr) + proxy.http_client = continuation_client + + async def _fake_retry(method, url, headers, body, stream=False, **kwargs): # noqa: ANN001 + return httpx.Response(200, json=persistent_ccr) + + proxy._retry_request = _fake_retry # type: ignore[assignment] + + resp = client.post( + "/v1/messages", + headers={"x-api-key": "test-key", "anthropic-version": "2023-06-01"}, + json={ + "model": "claude-sonnet-4-6", + "max_tokens": 64, + "stream": True, + "tools": [create_ccr_tool_definition("anthropic")], + "messages": [{"role": "user", "content": "use tools"}], + }, + ) + + # Fails closed no longer: residual CCR is handed back to the client as 200 SSE. + assert resp.status_code == 200, resp.text + assert "text/event-stream" in resp.headers["content-type"] + assert "headroom_retrieve" in resp.text + assert "Unable to safely complete streamed CCR retrieval" not in resp.text