From 131b119c053e66fe825dabb3c242f6dc5c6049d7 Mon Sep 17 00:00:00 2001 From: Tejas Chopra Date: Tue, 18 Aug 2026 07:15:55 -0700 Subject: [PATCH] fix(ccr): make --no-ccr disable server-side response handling too (#3101) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 `<>` 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 Co-authored-by: Claude Opus 5 --- headroom/cli/proxy.py | 18 +- .../test_no_ccr_disables_response_handling.py | 164 ++++++++++++++++++ 2 files changed, 178 insertions(+), 4 deletions(-) create mode 100644 tests/test_no_ccr_disables_response_handling.py diff --git a/headroom/cli/proxy.py b/headroom/cli/proxy.py index bbdba0fed..e802bd0d6 100644 --- a/headroom/cli/proxy.py +++ b/headroom/cli/proxy.py @@ -1330,12 +1330,22 @@ def proxy( protect_recent=_get_env_int_optional("HEADROOM_PROTECT_RECENT"), protect_analysis_context=_get_env_bool_optional("HEADROOM_PROTECT_ANALYSIS_CONTEXT"), accuracy_guard=os.environ.get("HEADROOM_ACCURACY_GUARD") or None, - # CCR opt-out: --no-ccr disables both halves at once (markers in content - # AND the injected retrieve tool). Markers without a tool — or a tool - # without markers — are useless, so it is a single switch. Default keeps - # CCR fully on. + # CCR opt-out: --no-ccr disables every half at once — markers in + # content, the injected retrieve tool, AND server-side response + # handling. Markers without a tool, or a tool without markers, are + # useless, so it is a single switch. Default keeps CCR fully on. + # + # Response handling has to be part of it. The buffered stream:false + # path keys off ``headroom_retrieve`` being present in the *request's* + # tools, and a client can advertise that tool on its own — the bundled + # OpenCode plugin registers it unconditionally. So with response + # handling left on, `--no-ccr` silently kept flipping streaming turns + # to buffered whenever history still held a redeemable marker, and the + # documented escape hatch for the CCR buffered-stream bugs did nothing + # for exactly the clients told to use it (#3082). ccr_inject_tool=not no_ccr, ccr_inject_marker=not no_ccr, + ccr_handle_responses=not no_ccr, ccr_resolve_markers_inline=ccr_inline_resolve, lossless=lossless, ccr_proactive_expansion=not no_ccr_proactive_expansion, diff --git a/tests/test_no_ccr_disables_response_handling.py b/tests/test_no_ccr_disables_response_handling.py new file mode 100644 index 000000000..5a1947b8a --- /dev/null +++ b/tests/test_no_ccr_disables_response_handling.py @@ -0,0 +1,164 @@ +"""`--no-ccr` has to disable server-side CCR handling too (#3082). + +The flag advertises "Disable CCR entirely", and names the case it exists for: +streaming / non-MCP clients that cannot resolve an injected tool. It mapped onto +only two of the three CCR subsystems, though — markers and tool injection — +leaving ``ccr_handle_responses`` on, which has no flag and no env var of its own. + +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 the tool +unconditionally. So `--no-ccr` left the buffered path fully armed for exactly +the clients it was recommended to, and a turn whose history still held a +redeemable marker kept being flipped to buffered. +""" + +from __future__ import annotations + +import json +from unittest.mock import patch + +import pytest + +click = pytest.importorskip("click") +pytest.importorskip("fastapi") +httpx = pytest.importorskip("httpx") + +from click.testing import CliRunner # noqa: E402 +from fastapi.testclient import TestClient # noqa: E402 + +from headroom.cache.backends import InMemoryBackend # noqa: E402 +from headroom.cache.compression_store import ( # noqa: E402 + get_compression_store, + reset_compression_store, +) +from headroom.ccr.tool_injection import create_ccr_tool_definition # noqa: E402 +from headroom.cli.main import main # noqa: E402 +from headroom.proxy.server import ProxyConfig, create_app # noqa: E402 + + +def _config_for(args: list[str], env: dict[str, str] | None = None) -> ProxyConfig: + """Run `headroom proxy ...` far enough to capture the ProxyConfig it builds.""" + captured: dict[str, ProxyConfig] = {} + + def mock_run_server(config, **kwargs): # noqa: ANN001, ANN003 + captured["config"] = config + + with patch("headroom.proxy.server.run_server", mock_run_server): + result = CliRunner().invoke(main, args, env=env or {}, catch_exceptions=False) + + assert result.exit_code == 0, result.output + return captured["config"] + + +# --------------------------------------------------------------------------- # +# The flag -> config mapping +# --------------------------------------------------------------------------- # +def test_no_ccr_flag_disables_response_handling() -> None: + config = _config_for(["proxy", "--no-ccr"]) + + assert config.ccr_inject_tool is False + assert config.ccr_inject_marker is False + # The half that used to survive the switch. + assert config.ccr_handle_responses is False + + +def test_no_ccr_env_var_disables_response_handling() -> None: + config = _config_for(["proxy"], env={"HEADROOM_NO_CCR": "1"}) + + assert config.ccr_inject_tool is False + assert config.ccr_inject_marker is False + assert config.ccr_handle_responses is False + + +def test_default_keeps_ccr_fully_on() -> None: + """The switch must not leak into the default posture.""" + config = _config_for(["proxy"]) + + assert config.ccr_inject_tool is True + assert config.ccr_inject_marker is True + assert config.ccr_handle_responses is True + + +# --------------------------------------------------------------------------- # +# What that mapping buys: no buffered flip, even when the client offers the tool +# --------------------------------------------------------------------------- # +@pytest.fixture +def _store(): + reset_compression_store() + get_compression_store(backend=InMemoryBackend()) + try: + yield + finally: + reset_compression_store() + + +def _upstream_stream_field(*, ccr_handle_responses: bool) -> object: + """Drive one streaming turn and report the `stream` value sent upstream.""" + marker = get_compression_store().store( + original=json.dumps({"earlier": "tool output"}), + compressed="{}", + original_item_count=400, + ) + config = ProxyConfig( + optimize=False, + cache_enabled=False, + rate_limit_enabled=False, + memory_enabled=False, + # `--no-ccr` posture: nothing injected by us. + ccr_inject_tool=False, + ccr_inject_marker=False, + ccr_handle_responses=ccr_handle_responses, + ccr_context_tracking=False, + image_optimize=False, + ) + seen: dict[str, object] = {} + app = create_app(config) + with TestClient(app) as client: + proxy = client.app.state.proxy + + async def _fake_retry(method, url, headers, body, stream=False, **kwargs): # noqa: ANN001 + sent = json.loads(body) if isinstance(body, (str, bytes)) else body + seen["stream"] = sent.get("stream") + return httpx.Response( + 200, + json={ + "id": "msg_1", + "type": "message", + "role": "assistant", + "model": "claude-sonnet-4-6", + "content": [{"type": "text", "text": "ok"}], + "stop_reason": "end_turn", + "usage": { + "input_tokens": 10, + "output_tokens": 5, + "cache_read_input_tokens": 0, + "cache_creation_input_tokens": 0, + }, + }, + ) + + proxy._retry_request = _fake_retry # type: ignore[assignment] + client.post( + "/v1/messages", + json={ + "model": "claude-sonnet-4-6", + "max_tokens": 64, + "stream": True, + # The client advertises the tool itself, as the OpenCode plugin does. + "tools": [create_ccr_tool_definition("anthropic")], + "messages": [{"role": "user", "content": f"go <>"}], + }, + headers={"x-api-key": "test-key", "anthropic-version": "2023-06-01"}, + ) + return seen.get("stream") + + +def test_client_offered_tool_still_buffers_when_handling_is_on(_store) -> None: # noqa: ANN001 + """The behaviour being switched off — pinned so the switch is meaningful.""" + assert _upstream_stream_field(ccr_handle_responses=True) is False + + +def test_client_offered_tool_does_not_buffer_under_no_ccr(_store) -> None: # noqa: ANN001 + """With the switch honoured, the turn keeps streaming despite the tool.""" + assert _upstream_stream_field(ccr_handle_responses=False) is not False