1 commit

Author	SHA1	Message	Date
Parideboy	3752458022	fix(cache): enforce Anthropic's 1h-before-5m cache_control ordering before forwarding (#2941) ... ## Description Anthropic evaluates prompt-cache breakpoints in **one pass over the whole request** — `tools`, then `system`, then `messages` — and rejects the request outright when a `ttl='1h'` breakpoint appears after a 5-minute one. A bare `{"type": "ephemeral"}` marker counts as 5 minutes, so this is easy to trip without any `ttl` field being visibly wrong: ``` API Error: 400 messages.15.content.1.cache_control.ttl: a ttl='1h' cache_control block must not come after a ttl='5m' cache_control block. Note that blocks are processed in the following order: tools, system, messages. ``` Headroom rewrites `cache_control` markers in several independent places, each looking at one section, and nothing checked the invariant that spans them. The failure mode is a dead turn, not a silent cost regression. Two paths can leave the forwarded body illegal today: 1. **Replayed 1h marker in a 5m request.** Claude Code picks its TTL lane per request, not per session: the main loop asks for 1h and sends the `extended-cache-ttl` beta header, while a side question (`/btw` in the report) goes out in the 5m lane with bare markers and no beta header. Headroom replays part of the previous turn's forwarded bytes into `messages` to keep the prefix stable, and those bytes still carry `ttl: "1h"`. `tools`/`system` at 5m, `messages` at 1h — 400. 2. **Tools breakpoint downgraded.** `inject_tool_search_deferral` re-places the *last* marker it stripped, so a bare marker on a later deferred tool overwrites a `ttl='1h'` one. The tools prefix goes upstream at 5m while message breakpoints are still 1h — 400. Reported separately as #2767. The rule spans `tools`/`system`/`messages`, so no individual transform is in a position to check it. The fix is a guard at the last seam before the body goes on the wire, plus the one Python/Rust divergence that manufactures the violation upstream of it. Closes #2939 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/proxy/helpers.py`: new `enforce_cache_control_ttl_order`, plus `cache_control_ttl_lane` / `cache_control_ttl_lanes` / `walk_cache_control`. The walk visits markers in Anthropic's documented order and matches the traversal `count_cache_breakpoints` already performs, so the two cannot disagree about what counts as a breakpoint. TTL ranking is ported verbatim from the Rust `TtlOrderingWalk::observe`: absent or `"5m"` is short, `"1h"` is long, any other value is left alone rather than guessed at. Two repairs: - **Lane containment** — when the client sent no 1h marker of its own, strip `ttl` from any 1h marker that leaked in. That request never sent the `extended-cache-ttl` beta header, so it could not have written a 1h entry anyway; nothing is lost. Other marker fields (`scope`, …) are preserved. - **Ordering** — when the client did ask for 1h, promote every 5m marker preceding the last 1h one. Demoting would also make the request legal but would discard 1h caching the client is explicitly paying for, which is the regression #2375 / #2382 / #2651 were filed to stop. A violation seen on the way out means headroom downgraded or introduced a marker, so promoting restores what the client's own (legal) request asked for at that position. - Copy-on-write: a legal body is returned by identity, so the hot path pays only a walk over at most a handful of markers. Kill switch `HEADROOM_CACHE_CONTROL_TTL_GUARD=0`, matching the `HEADROOM_TOOL_SEARCH=0` convention. - `headroom/proxy/handlers/anthropic.py`: - Capture the client's TTL lane from the inbound snapshot, before any transform runs. This cannot be inferred from the session or from config — the lane is a per-request property of Claude Code, which is the whole reason the `/btw` case exists. - Call the guard immediately before `log_cache_breakpoints`, i.e. after every transform, the tool sort, the deferral, CCR injection and the pipeline extensions. Mark the body mutated when a repair fires, and log a WARNING carrying the repair kind, the counts and the offending sections — the diagnostic the next report of this class will need. - `_sort_tools_deterministically` now skips the sort when any tool carries `cache_control`, logging `event=tool_sort_skipped reason=marker_present`. A breakpoint on a tool means "cache through here", so reordering changes what is inside the cached prefix and can move a 1h-marked tool behind a 5m-marked one. The Rust proxy already refuses for exactly this reason (`any_tool_has_cache_control` in `crates/headroom-proxy/src/compression/live_zone_anthropic.rs:651`); the Python path never got the same guard. Putting the check in `_sort_tools_deterministically` rather than `_tools_for_forwarding` covers all call sites including the batch path. - `tests/test_cache_control_ttl_order.py`: new, 24 cases. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed The new tests validate against an independent reimplementation of Anthropic's rule rather than against the guard's own walk, so a bug in the walk cannot make the assertions pass. Coverage: lane classification (bare marker is 5m, unknown TTLs are `other`); containment of a replayed 1h marker including preservation of non-`ttl` fields; promotion across `tools`→`messages`, `system`→`messages` and within `messages`; markers nested in `tool_result` sub-blocks; only markers before the *last* 1h one are rewritten, so a legal 1h-then-5m ordering is left alone; legal bodies returned by `is` identity; unknown `ttl` untouched; kill switch; the tool sort skipping on marked tools and still sorting unmarked ones, with one test pinning that the sort *would* have created a violation without the guard; and an end-to-end regression running `inject_tool_search_deferral` then the guard on the #2767 shape. ### Test Output ```text $ pytest tests/test_cache_control_ttl_order.py -q 24 passed in 0.77s $ pytest tests/test_cache_ttl_preserved.py tests/test_cache_control_move_bust.py \ tests/test_cache_breakpoint_diagnostics.py tests/test_issue_746_tool_search.py -q 86 passed in 1.36s $ pytest tests/test_cache/ tests/test_proxy/ -q 16 failed, 489 passed, 2 skipped in 91.41s $ ruff check headroom/ tests/test_cache_control_ttl_order.py All checks passed! $ ruff format --check headroom/ tests/test_cache_control_ttl_order.py 1 file would be reformatted, 520 files already formatted $ mypy --python-version 3.13 headroom --ignore-missing-imports Found 12 errors in 3 files (checked 517 source files) ``` The three non-green results above are all pre-existing on a clean `upstream/main` in this environment, verified by stashing the changes and re-running: - The 16 failures are all in `tests/test_cache/test_client_integration.py` and are a Windows temp-path problem in this sandbox (`OSError: [WinError 123] ... '\\C:\\Users\\...\\Temp'`), not a code failure. They fail identically with the branch stashed. - `ruff format --check` flags `headroom/testing/README.md`, a docs code block untouched by this PR. - The mypy errors are in `headroom/memory/mcp_server.py` and `headroom/release_version.py`; none are in the files this PR changes. `--python-version 3.13` is needed locally because the pinned `python_version = "3.10"` makes mypy reject the installed numpy stubs before it checks anything. ## Real Behavior Proof - Environment: Windows 11, Python 3.13.11, headroom at this branch's head, run against the real `headroom.proxy` forwarding helpers. No Anthropic API key is available in this environment, so Anthropic's validator is reimplemented locally from its documented rule and its own error string; the request bodies are produced by the real code path (`_sort_tools_deterministically` then `inject_tool_search_deferral` then the guard), not hand-written. - Exact command / steps: build two request shapes — (A) a 5m-lane request whose `messages` carries a replayed `ttl:"1h"` marker, the `/btw` case; (B) 13 tools with markers on two deferred tools plus 1h message breakpoints, the #2767 case — push each through the forwarding helpers twice, once with `HEADROOM_CACHE_CONTROL_TTL_GUARD=0` and once with the guard at its default, and validate the resulting body. - Observed result: both scenarios are rejected with the issue's exact 400 when the guard is off, and both are legal with it on. Scenario A is repaired by lane containment (the leaked 1h ttl is stripped), scenario B by promotion (the downgraded tools breakpoint goes back to 1h). Full output: ```text ########## Scenario A: /btw side question replays a 1h marker into a 5m request ########## ===== BEFORE (HEADROOM_CACHE_CONTROL_TTL_GUARD=0) ===== tools.0 5m system.0 5m messages.1.content.0 1h RESULT: API Error: 400 messages.1.content.0.cache_control.ttl: a ttl='1h' cache_control block must not come after a ttl='5m' cache_control block. Note that blocks are processed in the following order: tools, system, messages. ===== AFTER (default) ===== WARNING event=cache_control_ttl_order request_id=repro-2939 repair=lane_containment demoted=1 leaked_from_section=messages; the client sent no 1h marker, so a replayed 1h breakpoint would have been rejected upstream tools.0 5m system.0 5m messages.1.content.0 5m RESULT: 200 OK (request satisfies the ordering rule) ########## Scenario B: tool-search deferral downgrades the tools breakpoint ########## ===== BEFORE (HEADROOM_CACHE_CONTROL_TTL_GUARD=0) ===== INFO event=tool_sort_skipped reason=marker_present tool_count=13 marked=2 tools.1 5m messages.1.content.0 1h RESULT: API Error: 400 messages.1.content.0.cache_control.ttl: a ttl='1h' cache_control block must not come after a ttl='5m' cache_control block. Note that blocks are processed in the following order: tools, system, messages. ===== AFTER (default) ===== INFO event=tool_sort_skipped reason=marker_present tool_count=13 marked=2 WARNING event=cache_control_ttl_order request_id=repro-2939 repair=promote_to_1h promoted=1 first_short_section=tools first_long_section=messages; a 5m breakpoint preceded a 1h one, which Anthropic rejects outright tools.1 1h messages.1.content.0 1h RESULT: 200 OK (request satisfies the ordering rule) ``` - Not tested: no live call to `api.anthropic.com` — no credentials in this environment — so the 400/200 above come from a local reimplementation of the rule, not from the API itself. The reporter's original `/btw` flow was not reproduced end to end through `headroom wrap claude`. Cache-hit-rate impact of promoting a 5m marker to 1h was not measured against real traffic; the reasoning for promoting over demoting is argued above, not benchmarked. Nothing on the Rust side was exercised. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I did **not** edit `CHANGELOG.md` — it is generated by release-please from my Conventional Commit PR title (a CI guard enforces this) ## Additional Notes Documentation was not updated: the new env var is a kill switch for an internal correctness guard with no user-facing behaviour when things are working, matching how `HEADROOM_KOMPRESS_BACKGROUND_WARM` is handled. Happy to add a line to the env-var reference if maintainers prefer. **One trade-off worth a maintainer's eye.** `affinity_tools` feeds a `segment_fingerprint` used for prefix-tracker affinity. Skipping the sort makes that fingerprint depend on the client's tool order. Clients that mark tools must already keep a stable order for their own prefix cache to work, so this should be safe, but it is stated rather than assumed. **Deliberately out of scope, flagged rather than dropped:** - The sibling hole in `inject_tool_search_deferral`: when `resident_has_cache_control` is already true at 5m, a dropped 1h marker is discarded outright. That is a cost regression rather than a 400, and it sits in the same handful of lines that the open PR #2771 rewrites, so touching it here would conflict. Better raised on #2767. - `TtlOrderingWalk` in `crates/headroom-core/src/cache_control.rs` is instantiated separately per field list, so it only ever sees violations *within* `messages`, `system` or `tools` — never across them — and it only warns. Its module doc justifies warn-only with "Anthropic itself accepts both orderings (just with potentially-suboptimal cache eviction)". #2939 and #2767 both show that premise is now stale. Changing Rust behaviour is a separate blast radius. - `cold_prefix._cache_control_ttls` never scans `tools[]`, so a client whose only 1h marker rides on `tools` is read as 300s. Separate bug, separate PR. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: JD Davis <mxjerrett@gmail.com>	2026-08-12 16:32:12 -05:00