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>
This commit is contained in:
Tejas Chopra
+14
-4
@@ -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,
|
||||
|
||||
@@ -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 <<ccr:{marker}>>"}],
|
||||
},
|
||||
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
|
||||
Reference in New Issue
Block a user