Skip to content

Commit 131b119

Browse files
chopratejasTejas Chopraclaude
authored
fix(ccr): make --no-ccr disable server-side response handling too (#3101)
## Description `--no-ccr` advertises **"Disable CCR entirely"**, and its help text names the case it exists for: *"streaming / non-MCP clients that can't resolve an injected tool."* It mapped onto only two of the three CCR subsystems — markers and tool injection — leaving `ccr_handle_responses` on. That field has no flag and no env var of its own, so under `--no-ccr` it was always `True`. That mattered because the buffered `stream: false` path keys off `headroom_retrieve` being present in the **request's** tools, and the client can put it there itself — the bundled OpenCode plugin registers it unconditionally. So `--no-ccr` left the buffered path fully armed for exactly the clients it was recommended to, and any turn whose history still held a redeemable marker kept being flipped to buffered. This is why the workaround handed out in #2952 / #3017 / #3079 did nothing for `headroom wrap opencode`. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/cli/proxy.py`: `--no-ccr` / `HEADROOM_NO_CCR` now also sets `ccr_handle_responses=False`, so the switch covers all three CCR subsystems rather than two. - Rewrote the inline comment, which claimed the flag "disables both halves at once" — there were three. - `tests/test_no_ccr_disables_response_handling.py`: 5 tests covering the flag→config mapping (flag, env var, and the untouched default), plus the behaviour it buys — a client-advertised `headroom_retrieve` with a redeemable marker no longer flips the turn to buffered. ## Testing - [x] Unit tests pass (`pytest`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text tests/test_no_ccr_disables_response_handling.py ..... [100%] 5 passed Full suite (both Tier 1 fixes applied): 3 failed, 11220 passed, 581 skipped in 428.90s ``` 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()` with the `--no-ccr` posture (`ccr_inject_tool=False`, `ccr_inject_marker=False`), a client-supplied `headroom_retrieve` in `tools`, and a redeemable `<<ccr:…>>` marker in the message — then recorded the `stream` value that reached the upstream stub. - Observed result: Before — upstream received `stream=False`; the turn was buffered despite `--no-ccr`. Only setting `ccr_handle_responses=False` stopped it. After — `headroom proxy --no-ccr` and `HEADROOM_NO_CCR=1` both produce `ccr_handle_responses=False`, and the same turn keeps streaming. Reverting just `headroom/cli/proxy.py` fails the two mapping tests and passes them again with it restored. - Not tested: No live OpenCode + GitHub Copilot session; the reporter's end-to-end confirmation is still wanted. The OpenCode plugin still registers `headroom_retrieve` unconditionally — deliberately left alone, since with this fix an advertised tool no longer causes buffering. ## Runtime Rollout Safety - Rollout-managed feature(s): None — no rollout channel gates this. - Minimum rollout channel: n/a - Stable/default behavior changed: No. Default (no flag) keeps `ccr_handle_responses=True`, pinned by a test. - Kill switch / disable path: This *is* the kill switch; the change makes it work as documented. - Unsafe override required: No. - Qualification impact: None. - Rollback path: Revert this commit; `--no-ccr` returns to disabling two of three subsystems. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review Closes #3082 Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 7ef736f commit 131b119

2 files changed

Lines changed: 178 additions & 4 deletions

File tree

headroom/cli/proxy.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,12 +1330,22 @@ def proxy(
13301330
protect_recent=_get_env_int_optional("HEADROOM_PROTECT_RECENT"),
13311331
protect_analysis_context=_get_env_bool_optional("HEADROOM_PROTECT_ANALYSIS_CONTEXT"),
13321332
accuracy_guard=os.environ.get("HEADROOM_ACCURACY_GUARD") or None,
1333-
# CCR opt-out: --no-ccr disables both halves at once (markers in content
1334-
# AND the injected retrieve tool). Markers without a tool — or a tool
1335-
# without markers — are useless, so it is a single switch. Default keeps
1336-
# CCR fully on.
1333+
# CCR opt-out: --no-ccr disables every half at once — markers in
1334+
# content, the injected retrieve tool, AND server-side response
1335+
# handling. Markers without a tool, or a tool without markers, are
1336+
# useless, so it is a single switch. Default keeps CCR fully on.
1337+
#
1338+
# Response handling has to be part of it. The buffered stream:false
1339+
# path keys off ``headroom_retrieve`` being present in the *request's*
1340+
# tools, and a client can advertise that tool on its own — the bundled
1341+
# OpenCode plugin registers it unconditionally. So with response
1342+
# handling left on, `--no-ccr` silently kept flipping streaming turns
1343+
# to buffered whenever history still held a redeemable marker, and the
1344+
# documented escape hatch for the CCR buffered-stream bugs did nothing
1345+
# for exactly the clients told to use it (#3082).
13371346
ccr_inject_tool=not no_ccr,
13381347
ccr_inject_marker=not no_ccr,
1348+
ccr_handle_responses=not no_ccr,
13391349
ccr_resolve_markers_inline=ccr_inline_resolve,
13401350
lossless=lossless,
13411351
ccr_proactive_expansion=not no_ccr_proactive_expansion,
Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
"""`--no-ccr` has to disable server-side CCR handling too (#3082).
2+
3+
The flag advertises "Disable CCR entirely", and names the case it exists for:
4+
streaming / non-MCP clients that cannot resolve an injected tool. It mapped onto
5+
only two of the three CCR subsystems, though — markers and tool injection —
6+
leaving ``ccr_handle_responses`` on, which has no flag and no env var of its own.
7+
8+
That mattered because the buffered ``stream: false`` path keys off
9+
``headroom_retrieve`` being present in the *request's* tools, and the client can
10+
put it there itself: the bundled OpenCode plugin registers the tool
11+
unconditionally. So `--no-ccr` left the buffered path fully armed for exactly
12+
the clients it was recommended to, and a turn whose history still held a
13+
redeemable marker kept being flipped to buffered.
14+
"""
15+
16+
from __future__ import annotations
17+
18+
import json
19+
from unittest.mock import patch
20+
21+
import pytest
22+
23+
click = pytest.importorskip("click")
24+
pytest.importorskip("fastapi")
25+
httpx = pytest.importorskip("httpx")
26+
27+
from click.testing import CliRunner # noqa: E402
28+
from fastapi.testclient import TestClient # noqa: E402
29+
30+
from headroom.cache.backends import InMemoryBackend # noqa: E402
31+
from headroom.cache.compression_store import ( # noqa: E402
32+
get_compression_store,
33+
reset_compression_store,
34+
)
35+
from headroom.ccr.tool_injection import create_ccr_tool_definition # noqa: E402
36+
from headroom.cli.main import main # noqa: E402
37+
from headroom.proxy.server import ProxyConfig, create_app # noqa: E402
38+
39+
40+
def _config_for(args: list[str], env: dict[str, str] | None = None) -> ProxyConfig:
41+
"""Run `headroom proxy ...` far enough to capture the ProxyConfig it builds."""
42+
captured: dict[str, ProxyConfig] = {}
43+
44+
def mock_run_server(config, **kwargs): # noqa: ANN001, ANN003
45+
captured["config"] = config
46+
47+
with patch("headroom.proxy.server.run_server", mock_run_server):
48+
result = CliRunner().invoke(main, args, env=env or {}, catch_exceptions=False)
49+
50+
assert result.exit_code == 0, result.output
51+
return captured["config"]
52+
53+
54+
# --------------------------------------------------------------------------- #
55+
# The flag -> config mapping
56+
# --------------------------------------------------------------------------- #
57+
def test_no_ccr_flag_disables_response_handling() -> None:
58+
config = _config_for(["proxy", "--no-ccr"])
59+
60+
assert config.ccr_inject_tool is False
61+
assert config.ccr_inject_marker is False
62+
# The half that used to survive the switch.
63+
assert config.ccr_handle_responses is False
64+
65+
66+
def test_no_ccr_env_var_disables_response_handling() -> None:
67+
config = _config_for(["proxy"], env={"HEADROOM_NO_CCR": "1"})
68+
69+
assert config.ccr_inject_tool is False
70+
assert config.ccr_inject_marker is False
71+
assert config.ccr_handle_responses is False
72+
73+
74+
def test_default_keeps_ccr_fully_on() -> None:
75+
"""The switch must not leak into the default posture."""
76+
config = _config_for(["proxy"])
77+
78+
assert config.ccr_inject_tool is True
79+
assert config.ccr_inject_marker is True
80+
assert config.ccr_handle_responses is True
81+
82+
83+
# --------------------------------------------------------------------------- #
84+
# What that mapping buys: no buffered flip, even when the client offers the tool
85+
# --------------------------------------------------------------------------- #
86+
@pytest.fixture
87+
def _store():
88+
reset_compression_store()
89+
get_compression_store(backend=InMemoryBackend())
90+
try:
91+
yield
92+
finally:
93+
reset_compression_store()
94+
95+
96+
def _upstream_stream_field(*, ccr_handle_responses: bool) -> object:
97+
"""Drive one streaming turn and report the `stream` value sent upstream."""
98+
marker = get_compression_store().store(
99+
original=json.dumps({"earlier": "tool output"}),
100+
compressed="{}",
101+
original_item_count=400,
102+
)
103+
config = ProxyConfig(
104+
optimize=False,
105+
cache_enabled=False,
106+
rate_limit_enabled=False,
107+
memory_enabled=False,
108+
# `--no-ccr` posture: nothing injected by us.
109+
ccr_inject_tool=False,
110+
ccr_inject_marker=False,
111+
ccr_handle_responses=ccr_handle_responses,
112+
ccr_context_tracking=False,
113+
image_optimize=False,
114+
)
115+
seen: dict[str, object] = {}
116+
app = create_app(config)
117+
with TestClient(app) as client:
118+
proxy = client.app.state.proxy
119+
120+
async def _fake_retry(method, url, headers, body, stream=False, **kwargs): # noqa: ANN001
121+
sent = json.loads(body) if isinstance(body, (str, bytes)) else body
122+
seen["stream"] = sent.get("stream")
123+
return httpx.Response(
124+
200,
125+
json={
126+
"id": "msg_1",
127+
"type": "message",
128+
"role": "assistant",
129+
"model": "claude-sonnet-4-6",
130+
"content": [{"type": "text", "text": "ok"}],
131+
"stop_reason": "end_turn",
132+
"usage": {
133+
"input_tokens": 10,
134+
"output_tokens": 5,
135+
"cache_read_input_tokens": 0,
136+
"cache_creation_input_tokens": 0,
137+
},
138+
},
139+
)
140+
141+
proxy._retry_request = _fake_retry # type: ignore[assignment]
142+
client.post(
143+
"/v1/messages",
144+
json={
145+
"model": "claude-sonnet-4-6",
146+
"max_tokens": 64,
147+
"stream": True,
148+
# The client advertises the tool itself, as the OpenCode plugin does.
149+
"tools": [create_ccr_tool_definition("anthropic")],
150+
"messages": [{"role": "user", "content": f"go <<ccr:{marker}>>"}],
151+
},
152+
headers={"x-api-key": "test-key", "anthropic-version": "2023-06-01"},
153+
)
154+
return seen.get("stream")
155+
156+
157+
def test_client_offered_tool_still_buffers_when_handling_is_on(_store) -> None: # noqa: ANN001
158+
"""The behaviour being switched off — pinned so the switch is meaningful."""
159+
assert _upstream_stream_field(ccr_handle_responses=True) is False
160+
161+
162+
def test_client_offered_tool_does_not_buffer_under_no_ccr(_store) -> None: # noqa: ANN001
163+
"""With the switch honoured, the turn keeps streaming despite the tool."""
164+
assert _upstream_stream_field(ccr_handle_responses=False) is not False

0 commit comments

Comments
 (0)