Skip to content

Commit 139c7cb

Browse files
chopratejasTejas Chopraclaude
authored
fix(ccr): send Accept: application/json on a buffered stream:false turn (#3102)
## Description Server-side CCR retrieval flips a `stream: true` turn to `stream: false` so the whole upstream reply is in hand before answering. The **body** was rewritten; the client's `Accept: text/event-stream` was **not**. The request that went on the wire therefore contradicted itself — *"answer as JSON"* in the body, *"I only accept SSE"* in the headers. Anthropic's first-party API tolerates that, which is why this never surfaced against it. GitHub Copilot's Anthropic-compatible gateway does not, and answers with a generic `api_error`. That is the reported shape exactly. An OpenCode session's **first** call succeeds — no marker exists yet, so nothing is buffered. The **second** call is the first to carry a redeemable `<<ccr:…>>` marker, so it is the first to be flipped to buffered, and it fails. The reporter's own logs show the correlation: every failed request carries `mutation_reasons=…,ccr_streaming_retrieve_buffered_non_stream`. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/proxy/handlers/anthropic.py`: when the buffered CCR path flips `stream` to `false`, the outgoing `Accept` header is set to `application/json` to match. The lookup is case-insensitive and **replaces** the existing header rather than appending, so exactly one `Accept` goes upstream. - `headroom/proxy/handlers/openai.py`: the **same fix on the `/v1/responses` buffered path**, which has an identical `stream: false` flip with no matching `Accept`. This handler is a GitHub Copilot path — it calls `apply_copilot_api_auth` — so leaving it would have left the reported bug live on a route the reporter can hit. Found during self-review, not in the original diff. - Same treatment for the Anthropic CCR continuation request, which is non-streaming for the same reason and previously fixed only `Content-Type`. Its header strip is now case-insensitive for `Content-Type` as well, removing a latent duplicate-header path. - `tests/test_buffered_ccr_accept_header.py`: 6 tests — the buffered turn asks for JSON, exactly one `Accept` survives, mixed-case `Accept` is replaced, a client sending no `Accept` still gets one, a non-buffered streaming turn keeps `text/event-stream` untouched, and the OpenAI `/v1/responses` buffered turn asks for JSON too. ## Testing - [x] Unit tests pass (`pytest`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text tests/test_buffered_ccr_accept_header.py ...... [100%] 6 passed CCR-adjacent suites on this branch: tests/test_buffered_ccr_accept_header.py, test_buffered_ccr_salvage.py, test_buffered_ccr_grace_window.py, test_anthropic_streaming_ccr_retrieve.py, test_ccr_buffered_stream_signed_thinking.py 41 passed Full suite on this branch: 3 failed, 11216 passed, 581 skipped in 414.81s ``` The 3 failures are pre-existing and environmental, identical to a plain-`main` baseline run on the same machine: no `cargo` installed (`test_no_native_tls_in_wheel_build_tree`), no `codex` CLI (`test_learn/test_integration.py`), and `test_run_server_installs_cancelled_error_filter`, which fails under full-suite ordering on `main` too. ## Real Behavior Proof - Environment: macOS (darwin 25.4.0), Python 3.12.13, worktree off `main` @ `7ef736fb`, `HEADROOM_SKIP_UPSTREAM_CHECK=1` - Exact command / steps: Drove one streaming `/v1/messages` turn through `create_app()` carrying a redeemable `<<ccr:…>>` marker and `headroom_retrieve` in `tools` (so the buffered path engages), with the client sending `Accept: text/event-stream`, and captured the exact headers and body handed to the upstream call. - Observed result: Before — `body.stream=False` sent together with `accept: text/event-stream`, the self-contradicting request. After — `body.stream=False` with `accept: application/json`, and a turn that is not flipped still sends `accept: text/event-stream` unchanged. Reverting only `headroom/proxy/handlers/anthropic.py` fails 3 of the new tests; reverting the OpenAI hunk alone fails the `/v1/responses` test with `['text/event-stream'] != ['application/json']`. Restoring both passes all 6. - Not tested: No live GitHub Copilot gateway call — I have no Copilot credentials here, so the claim that Copilot rejects the contradictory request is inferred from the reporter's logs plus the header mismatch, not observed against their upstream. Confirmation from @mars-peng-lb on a real OpenCode + Copilot session is still wanted before treating #3078 as fully closed. Separately noted while reviewing, **not fixed here**: `_should_buffer_openai_responses_stream_ccr` has no redeemable-marker requirement, so the `/v1/responses` path still buffers on mere tool presence — the #3071/#3092 narrowing was never mirrored from the Anthropic handler. Worth its own issue. ## Runtime Rollout Safety - Rollout-managed feature(s): None — no rollout channel gates this. - Minimum rollout channel: n/a - Stable/default behavior changed: Only on the buffered CCR path, and only the `Accept` header, which is made consistent with the `stream: false` body already being sent. Non-buffered turns are byte-identical, pinned by a test. - Kill switch / disable path: `--no-ccr` / `HEADROOM_NO_CCR` disables the buffered path entirely (see #3082), as does `ccr_handle_responses=False`. - Unsafe override required: No. - Qualification impact: None. - Rollback path: Revert this commit; the buffered path returns to forwarding the client's `Accept` unchanged. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review Closes #3078 Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 131b119 commit 139c7cb

3 files changed

Lines changed: 266 additions & 2 deletions

File tree

headroom/proxy/handlers/anthropic.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3501,6 +3501,20 @@ def _count_tool_tokens(value: object) -> int:
35013501
body_mutation_tracker.mark_mutated(
35023502
"ccr_streaming_retrieve_buffered_non_stream"
35033503
)
3504+
# The body now asks for a non-streaming reply, so the
3505+
# client's ``Accept: text/event-stream`` no longer describes
3506+
# the response being requested. Forwarding it unchanged
3507+
# sends upstream a self-contradicting request: "answer as
3508+
# JSON" in the body, "I only accept SSE" in the headers.
3509+
#
3510+
# Anthropic tolerates that. Stricter Anthropic-compatible
3511+
# gateways do not: GitHub Copilot's returns a generic
3512+
# ``api_error``, which is why a session's first call
3513+
# succeeded and the next one — the first to carry a
3514+
# redeemable marker, and so the first to be buffered —
3515+
# failed (#3078).
3516+
_accept_key = next((k for k in headers if k.lower() == "accept"), "accept")
3517+
headers[_accept_key] = "application/json"
35043518
logger.info(
35053519
f"[{request_id}] CCR: stream:true request has "
35063520
"headroom_retrieve available; using buffered stream:false "
@@ -3903,10 +3917,16 @@ async def api_call_fn(
39033917
body_mutated=True,
39043918
)
39053919
)
3920+
# A continuation is a non-streaming call, so it
3921+
# needs a matching Accept for the same reason the
3922+
# buffered flip above does (#3078).
39063923
ccr_outbound_headers = {
3907-
**continuation_headers,
3908-
"content-type": "application/json",
3924+
k: v
3925+
for k, v in continuation_headers.items()
3926+
if k.lower() not in ("accept", "content-type")
39093927
}
3928+
ccr_outbound_headers["content-type"] = "application/json"
3929+
ccr_outbound_headers["accept"] = "application/json"
39103930
log_outbound_request(
39113931
forwarder="anthropic_ccr_continuation",
39123932
method="POST",

headroom/proxy/handlers/openai.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5616,6 +5616,12 @@ async def handle_openai_responses(
56165616
if body.get("stream") is not False:
56175617
body["stream"] = False
56185618
body_mutation_tracker.mark_mutated("ccr_streaming_retrieve_buffered_non_stream")
5619+
# Same contradiction as the Anthropic path: the body now asks for a
5620+
# non-streaming reply while the client's Accept still says SSE. This
5621+
# handler serves GitHub Copilot (see apply_copilot_api_auth below),
5622+
# whose gateway is one of the strict ones (#3078).
5623+
_accept_key = next((k for k in headers if k.lower() == "accept"), "accept")
5624+
headers[_accept_key] = "application/json"
56195625
logger.info(
56205626
f"[{request_id}] CCR: stream:true /v1/responses request has "
56215627
"headroom_retrieve available; using buffered stream:false "
Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
"""A buffered CCR turn must not ask for SSE it no longer wants (#3078).
2+
3+
Server-side retrieval flips a ``stream: true`` turn to ``stream: false`` so the
4+
whole reply is in hand before answering. The body was rewritten; the client's
5+
``Accept: text/event-stream`` was not, so the request that went on the wire
6+
contradicted itself — "answer as JSON" in the body, "I only accept SSE" in the
7+
headers.
8+
9+
Anthropic tolerates that, which is why it never showed up against the first-party
10+
API. GitHub Copilot's Anthropic-compatible gateway does not, and answers with a
11+
generic ``api_error``. That produced the reported shape exactly: the first call
12+
of an OpenCode session succeeds (no marker yet, so no buffering), and the next
13+
one — the first to carry a redeemable marker, and so the first to be flipped —
14+
fails.
15+
"""
16+
17+
from __future__ import annotations
18+
19+
import json
20+
21+
import pytest
22+
23+
fastapi = pytest.importorskip("fastapi")
24+
httpx = pytest.importorskip("httpx")
25+
26+
from fastapi.testclient import TestClient # noqa: E402
27+
28+
from headroom.cache.backends import InMemoryBackend # noqa: E402
29+
from headroom.cache.compression_store import ( # noqa: E402
30+
get_compression_store,
31+
reset_compression_store,
32+
)
33+
from headroom.ccr.tool_injection import create_ccr_tool_definition # noqa: E402
34+
from headroom.proxy.server import ProxyConfig, create_app # noqa: E402
35+
36+
37+
@pytest.fixture(autouse=True)
38+
def _store():
39+
reset_compression_store()
40+
get_compression_store(backend=InMemoryBackend())
41+
try:
42+
yield
43+
finally:
44+
reset_compression_store()
45+
46+
47+
def _drive(*, with_marker: bool, accept: str | None) -> dict[str, object]:
48+
"""Run one streaming turn; report the body `stream` and headers sent upstream."""
49+
marker = get_compression_store().store(
50+
original=json.dumps({"earlier": "tool output"}),
51+
compressed="{}",
52+
original_item_count=400,
53+
)
54+
config = ProxyConfig(
55+
optimize=False,
56+
cache_enabled=False,
57+
rate_limit_enabled=False,
58+
memory_enabled=False,
59+
ccr_inject_tool=True,
60+
ccr_handle_responses=True,
61+
ccr_context_tracking=False,
62+
image_optimize=False,
63+
)
64+
seen: dict[str, object] = {}
65+
app = create_app(config)
66+
with TestClient(app) as client:
67+
proxy = client.app.state.proxy
68+
69+
async def _fake_retry(method, url, headers, body, stream=False, **kwargs): # noqa: ANN001
70+
sent = json.loads(body) if isinstance(body, (str, bytes)) else body
71+
seen["stream"] = sent.get("stream")
72+
seen["headers"] = dict(headers or {})
73+
return httpx.Response(
74+
200,
75+
json={
76+
"id": "msg_1",
77+
"type": "message",
78+
"role": "assistant",
79+
"model": "claude-sonnet-4-6",
80+
"content": [{"type": "text", "text": "ok"}],
81+
"stop_reason": "end_turn",
82+
"usage": {
83+
"input_tokens": 10,
84+
"output_tokens": 5,
85+
"cache_read_input_tokens": 0,
86+
"cache_creation_input_tokens": 0,
87+
},
88+
},
89+
)
90+
91+
proxy._retry_request = _fake_retry # type: ignore[assignment]
92+
93+
# A turn that is *not* flipped never reaches `_retry_request` — the plain
94+
# streaming path has its own upstream call — so capture that one too.
95+
async def _fake_stream(url, headers, body, *args, **kwargs): # noqa: ANN001
96+
from fastapi.responses import StreamingResponse
97+
98+
sent = json.loads(body) if isinstance(body, (str, bytes)) else body
99+
seen["stream"] = sent.get("stream")
100+
seen["headers"] = dict(headers or {})
101+
102+
async def _gen():
103+
yield b'event: message_stop\ndata: {"type":"message_stop"}\n\n'
104+
105+
return StreamingResponse(_gen(), media_type="text/event-stream")
106+
107+
proxy._stream_response = _fake_stream # type: ignore[assignment]
108+
headers = {"x-api-key": "test-key", "anthropic-version": "2023-06-01"}
109+
if accept is not None:
110+
headers["accept"] = accept
111+
content = f"go <<ccr:{marker}>>" if with_marker else "go"
112+
client.post(
113+
"/v1/messages",
114+
json={
115+
"model": "claude-sonnet-4-6",
116+
"max_tokens": 64,
117+
"stream": True,
118+
"tools": [create_ccr_tool_definition("anthropic")],
119+
"messages": [{"role": "user", "content": content}],
120+
},
121+
headers=headers,
122+
)
123+
return seen
124+
125+
126+
def _accepts(headers: dict) -> list[str]:
127+
return [v for k, v in headers.items() if k.lower() == "accept"]
128+
129+
130+
def test_buffered_turn_asks_for_json() -> None:
131+
seen = _drive(with_marker=True, accept="text/event-stream")
132+
133+
# Precondition: this turn really was flipped to buffered.
134+
assert seen["stream"] is False
135+
assert _accepts(seen["headers"]) == ["application/json"] # type: ignore[arg-type]
136+
137+
138+
def test_buffered_turn_leaves_exactly_one_accept_header() -> None:
139+
"""Replaced, never appended — two Accept values is its own bug."""
140+
seen = _drive(with_marker=True, accept="text/event-stream")
141+
142+
assert len(_accepts(seen["headers"])) == 1 # type: ignore[arg-type]
143+
144+
145+
def test_accept_header_is_replaced_regardless_of_casing() -> None:
146+
"""Header names are case-insensitive; the SSE value must not survive."""
147+
seen = _drive(with_marker=True, accept="TEXT/EVENT-STREAM")
148+
149+
values = _accepts(seen["headers"]) # type: ignore[arg-type]
150+
assert values == ["application/json"]
151+
assert not any("event-stream" in v.lower() for v in values)
152+
153+
154+
def test_buffered_turn_without_a_client_accept_still_asks_for_json() -> None:
155+
seen = _drive(with_marker=True, accept=None)
156+
157+
assert seen["stream"] is False
158+
assert _accepts(seen["headers"]) == ["application/json"] # type: ignore[arg-type]
159+
160+
161+
def test_a_streaming_turn_keeps_its_sse_accept() -> None:
162+
"""No marker means no flip, so nothing about the request should change."""
163+
seen = _drive(with_marker=False, accept="text/event-stream")
164+
165+
assert seen["stream"] is not False
166+
assert _accepts(seen["headers"]) == ["text/event-stream"] # type: ignore[arg-type]
167+
168+
169+
# --------------------------------------------------------------------------- #
170+
# The same flip exists on the OpenAI Responses path, which serves Copilot
171+
# --------------------------------------------------------------------------- #
172+
def _drive_responses(*, accept: str) -> dict[str, object]:
173+
"""Run one streaming /v1/responses turn and report what went upstream."""
174+
from headroom.ccr import CCR_TOOL_NAME
175+
176+
config = ProxyConfig(
177+
optimize=False,
178+
cache_enabled=False,
179+
rate_limit_enabled=False,
180+
memory_enabled=False,
181+
ccr_inject_tool=True,
182+
ccr_handle_responses=True,
183+
ccr_context_tracking=False,
184+
image_optimize=False,
185+
)
186+
seen: dict[str, object] = {}
187+
app = create_app(config)
188+
with TestClient(app) as client:
189+
proxy = client.app.state.proxy
190+
191+
async def _fake_retry(method, url, headers, body, stream=False, **kwargs): # noqa: ANN001
192+
sent = json.loads(body) if isinstance(body, (str, bytes)) else body
193+
seen["stream"] = sent.get("stream")
194+
seen["headers"] = dict(headers or {})
195+
return httpx.Response(
196+
200,
197+
json={
198+
"id": "resp_1",
199+
"object": "response",
200+
"model": "gpt-4o",
201+
"status": "completed",
202+
"output": [
203+
{
204+
"type": "message",
205+
"role": "assistant",
206+
"content": [{"type": "output_text", "text": "ok"}],
207+
}
208+
],
209+
"usage": {"input_tokens": 5, "output_tokens": 2, "total_tokens": 7},
210+
},
211+
)
212+
213+
proxy._retry_request = _fake_retry # type: ignore[assignment]
214+
client.post(
215+
"/v1/responses",
216+
json={
217+
"model": "gpt-4o",
218+
"stream": True,
219+
# Responses tool defs are flat, not nested under "function".
220+
"tools": [{"type": "function", "name": CCR_TOOL_NAME}],
221+
"input": "go",
222+
},
223+
headers={
224+
"authorization": "Bearer test-key",
225+
"accept": accept,
226+
"content-type": "application/json",
227+
},
228+
)
229+
return seen
230+
231+
232+
def test_responses_buffered_turn_asks_for_json() -> None:
233+
"""This handler serves GitHub Copilot, the gateway that rejects the mismatch."""
234+
seen = _drive_responses(accept="text/event-stream")
235+
236+
assert seen.get("stream") is False, "precondition: the turn must be buffered"
237+
assert _accepts(seen["headers"]) == ["application/json"] # type: ignore[arg-type]
238+
assert len(_accepts(seen["headers"])) == 1 # type: ignore[arg-type]

0 commit comments

Comments
 (0)