fix/gitpython-3158
2630 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Tejas Chopra
|
2130f150c7 |
fix(deps): raise the GitPython floor to 3.1.58 to clear 9 open advisories
The constraint floor was set at 3.1.50 for an older advisory batch and resolved to 3.1.54. Nine advisories published since still cover that version: GHSA-hmq2-w58f-27jc high <= 3.1.57 -> 3.1.58 GHSA-jm78-9fvv-mhgr high <= 3.1.57 -> 3.1.58 GHSA-wvpp-8hx9-p66j high <= 3.1.57 -> 3.1.58 GHSA-hh9p-6wh2-4mfc medium <= 3.1.57 -> 3.1.58 GHSA-9rj7-rf2p-w77r high <= 3.1.57 -> 3.1.58 GHSA-4gmw-gg2m-w46p high <= 3.1.57 -> 3.1.58 GHSA-3f7w-8rr8-f37f high <= 3.1.56 -> 3.1.57 GHSA-539m-9xh6-q6rr medium <= 3.1.56 -> 3.1.57 GHSA-p538-c434-8v24 medium <= 3.1.55 -> 3.1.56 3.1.58 is the highest fixed version across every GitPython advisory published to date, so the new floor clears all of them; the lock resolves to 3.1.59. GitPython is transitive (via agno) and is imported nowhere in headroom, so this is a supply-chain floor bump rather than a fix to reachable code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Tejas Chopra
|
b77d612913 |
fix(copilot): send VS Code inline completions to the host that serves them (#3112)
## Description #3077 stopped Copilot's inline completions being forwarded to `api.openai.com` (the corporate-blocked host in the original report) — but sent them to the **CAPI host**, which does not serve that endpoint. Copilot has two surfaces on two different hosts, and GitHub's own client library keeps them apart: ```js _getCAPIUrl(t) -> t?.endpoints.api || "https://api.githubcopilot.com" _getProxyUrl(t) -> t?.endpoints.proxy || DEFAULT_PROXY_BASE_URL DEFAULT_PROXY_BASE_URL = "https://copilot-proxy.githubusercontent.com" ``` building completions as `${proxyBaseURL}/v1/engines/<engine>/completions` (`@vscode/copilot-api` 0.5.2). Probed unauthenticated against the live hosts: | host | `POST /v1/engines/<e>/completions` | |---|---| | `copilot-proxy.githubusercontent.com` | **401** — exists, needs auth | | `proxy.individual.githubcopilot.com` | **401** — CNAME to the above | | `api.githubcopilot.com` | **404** — does not serve this path | So the destination #3077 chose could not have worked. Three separate defects were in the way, each sufficient on its own to keep completions broken. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `copilot_auth.py`: added `DEFAULT_COMPLETIONS_PROXY_URL` and made it the default in `copilot_completions_base_url()`, replacing the CAPI host. - `copilot_auth.py`: the "custom deployment keeps its own host" rule now excludes public Copilot hosts. Without this, `headroom wrap vscode` — the common setup, and the one that exports `GITHUB_COPILOT_API_URL=<resolved subscription URL>` — resolved straight back to the 404 host. **This was a bug in my own first cut of the fix, found by testing the real `wrap vscode` environment rather than just the routing table.** - `copilot_auth.py`: added `is_copilot_completions_host()` and `is_copilot_upstream_url()` (chat ∪ completions). The completions host was recognised as Copilot **nowhere**, so `apply_copilot_api_auth` attached no credentials (401 — routing correctly to a host we then failed to authenticate against) and `build_copilot_upstream_url` skipped `mark_request_routed_to_copilot()`, mislabelling the provider in telemetry. - The union is applied at exactly those two call sites. `is_copilot_api_url` is left alone, so validation of a token payload's `endpoints.api` and the Responses-API preference check keep their strict chat-only meaning. All six call sites were read before choosing this. - `proxy_targets.py`: the "already a Copilot host" guard now keys on the *completions* host. A CAPI host is not a completions host, so it must still be redirected; a genuine per-SKU completions host or operator override is still left untouched. - `providers/copilot/vscode.py`, `cli/wrap.py`, `docs/…/vscode-copilot.mdx`: stop writing/printing `github.copilot.advanced.debug.overrideAuthType`. No such setting exists in the modern Copilot Chat extension — the only one left after `GitHub.copilot` was deprecated in early 2026. Its full `advanced.*` surface is `authPermissions`, `authProvider`, `debug.overrideCapiUrl`, `debug.overrideProxyUrl`, `debug.use*Fetcher`. It is still *recognised* so a stale hand-written copy is detected, just never emitted. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text tests/test_copilot_vscode_completions_routing.py 59 passed Copilot-related suites 293 passed, 8 skipped Full suite: 3 failed, 11250 passed, 581 skipped in 342.50s ``` The 3 failures are pre-existing and environmental, identical to a plain-`main` baseline on this machine: no `cargo` (`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` @ `139c7cbd`, `HEADROOM_SKIP_UPSTREAM_CHECK=1` - Exact command / steps: (1) composed the real request path — `select_passthrough_base_url(proxy, headers, path)` → `build_copilot_upstream_url` → `apply_copilot_api_auth` — across 7 deployment shapes (no config, `wrap vscode`, advertised `endpoints.proxy`, operator override, GHE `.ghe.com`, GHE custom domain, target already a completions host); (2) probed the three candidate hosts unauthenticated with `curl -X POST /v1/engines/gpt-4o-copilot/completions`; (3) round-tripped `settings.json` through empty / one-setting / comments+array / CRLF shapes asserting valid JSON, idempotency and clean removal. - Observed result: before — `api.githubcopilot.com/...` (404 host), and with `GITHUB_COPILOT_API_URL` set as `wrap vscode` sets it, `api.business.githubcopilot.com` (also 404); no `Authorization` header on the completions host. After — `copilot-proxy.githubusercontent.com/v1/engines/gpt-41-copilot/completions` with credentials attached in every public-Copilot shape, `endpoints.proxy` and the operator override still winning, and a GHE tenant staying on its own host. `settings.json` stays valid JSON in all four shapes with the dead key gone; the two `restored=False` cases are pre-existing whitespace/CRLF normalisation, identical on `main`. Reverting the source fails 14 of the new tests, including the credential test on the completions host. - Not tested: no live VS Code session and no authenticated completion — the 401 proves the endpoint exists, not that GitHub accepts our forwarded request, which needs a real Copilot token. Confirmation from @rganesh-msys is still wanted. **Enterprise remains unresolved by default**: a GHE tenant stays on its own CAPI host, which is likely still the wrong surface for completions, but staying in-tenant beats forwarding keystrokes to a public GitHub host — `GITHUB_COPILOT_PROXY_URL` is the exact fix and now takes precedence over everything. ## Runtime Rollout Safety - Rollout-managed feature(s): None — no rollout channel gates this. - Minimum rollout channel: n/a - Stable/default behavior changed: Yes, and deliberately — the completions destination moves from a host that answers 404 to the one GitHub's own client defaults to. Only `/v1/engines/<engine>/completions` is affected; every other path keeps its upstream, pinned by tests. Copilot credentials now also reach the completions host, which is the point. - Kill switch / disable path: `GITHUB_COPILOT_PROXY_URL` pins the destination explicitly and beats all inference. - Unsafe override required: No. - Qualification impact: None. - Rollback path: Revert this commit; completions return to the CAPI host (404) and the settings block regains the inert `overrideAuthType`. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Additional Notes Two things found while reading the extension source, **not changed here**: 1. `advanced.debug.overrideProxyUrl` is **not** deprecated — the report that Copilot 0.60.0 stopped honouring it does not hold. The current canonical key is `github.copilot.internal.completionsUrl`, and `advanced.debug.overrideProxyUrl` is checked as its explicit legacy fallback (`getEndpointOverrideUrl` in `completions-core/lib/src/networkConfiguration.ts`), so what we write still works. Worth migrating to the `internal.*` keys eventually, since they take precedence. 2. `endpoints.proxy` is still only recorded during a token exchange, which is opt-in via `GITHUB_COPILOT_USE_TOKEN_EXCHANGE`, and the base URL is chosen before auth runs. With the default now correct this is a refinement for per-SKU hosts rather than a correctness requirement, so it is left as-is. Closes #3076 --------- Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Tejas Chopra
|
139c7cbdde |
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> |
||
|
Tejas Chopra
|
131b119c05 |
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> |
||
|
Parideboy
|
7ef736fb1a |
fix(ccr): make StreamingCCRHandler work on OpenAI streams (#3069)
## Description `StreamingCCRHandler` (`headroom/ccr/response_handler.py`) was written against the Anthropic wire format. Constructed with `provider="openai"` it does not work: it silently drops the response, reports the wrong `finish_reason`, and emits a stream shape no OpenAI client can read. This PR fixes all three. **Reachability, stated up front:** `StreamingCCRHandler` is exported from `headroom/ccr/__init__.py` but no proxy handler instantiates it today. Every live CCR path (`handlers/openai.py:4276`, `handlers/openai.py:5936`, `handlers/anthropic.py`, `handlers/gemini.py`) calls `CCRResponseHandler.handle_response` on a non-streaming body instead. So these defects are not currently hit by proxy traffic. They bite anyone importing the public `headroom.ccr.StreamingCCRHandler` export, and they would bite the moment streaming CCR gets wired up. I would rather fix them while they are cheap than have them surface as a mysterious truncation bug later. **This PR does not fix #1026.** I found these while investigating that issue and they turned out to be unrelated to it. #1026 needs information from the reporter before anyone can say whether Headroom is even in the request path; I have asked for it there. ### The three defects **1. The whole OpenAI response was dropped.** `StreamingCCRBuffer.add_chunk` detected a tool call by scanning the accumulated bytes for the literal `"type":"tool_use"`. That is Anthropic-only. An OpenAI-compatible stream carries tool calls as a `tool_calls` array inside `choices[].delta` and never emits that marker, so `detected_ccr` could never become `True`. Independently, `process_stream` decided the stream had ended by scanning for `"stop_reason"`, another Anthropic-only field. An OpenAI stream has no such field; it terminates with the `[DONE]` sentinel. With neither marker ever matching, and nothing flushing the buffer once the source iterator ran out, the outcome was: - OpenAI stream under 10 000 bytes: **nothing at all was yielded**. The client got an empty response. - OpenAI stream over 10 000 bytes: chunks flushed in ~10 KB batches, and the final sub-threshold batch was never flushed. The response visibly stopped mid-sentence. **2. `finish_reason` was hardcoded.** `_reconstruct_openai_response` always returned `"finish_reason": "stop"`, even when it had just finished reconstructing a non-empty `tool_calls` array, where the OpenAI API requires `"tool_calls"`. A client that drives its agent loop off `finish_reason` reads `stop`, concludes the turn is over, and never executes the tool calls. The Anthropic sibling `_reconstruct_anthropic_response` does this correctly, carrying `stop_reason` through from `message_delta`. It also discarded `id`, `object`, `created`, `model`, and `usage`, returning a bare `choices` list that is not a valid `chat.completion`. **3. `_response_to_sse` emitted the wrong shape.** The OpenAI branch serialised the reconstructed **non-streaming** body into a single SSE frame. A streaming client parses `choices[].delta`; this frame has `choices[].message`. Both the text and the tool calls were invisible to it. ### Why CI did not catch it `tests/test_ccr_response_handler_extra.py` exercised `_reconstruct_openai_response` but never asserted `finish_reason`, and the one `process_stream` test that passed `provider="openai"` fed it Anthropic-shaped bytes (`"type":"tool_use"` plus `"stop_reason"`). No test had ever run a real OpenAI stream through this class. That test now uses the real OpenAI wire shape, so it actually covers the path it claims to. ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update - [ ] Refactor / internal change ## Changes Made All in `headroom/ccr/response_handler.py`: - `StreamingCCRBuffer` gained a `provider` field (defaults to `"anthropic"`, so existing construction is unchanged) and picks its tool-call marker from it: `"type":"tool_use"` for Anthropic, `"tool_calls"` for everything else. `StreamingCCRHandler.__init__` now passes its own provider down. - `process_stream` selects the end-of-stream marker by provider (`"stop_reason"` for Anthropic, `data: [DONE]` for OpenAI), and **always flushes whatever is still buffered once the source iterator is exhausted**. That second part is deliberately unconditional on the marker: upstream can truncate, a gateway can omit the sentinel, and a future stream shape may not be recognised. Buffered bytes at that point are real response data, so they get flushed rather than dropped. - Removed the dead re-iteration block that followed the detection loop. Its guard was `not detection_complete and not self.buffer.detected_ccr`, and the only `break` out of the loop above required `detected_ccr` to be `True`, so it could only ever be reached with an already-exhausted iterator. The new flush takes its place. - `_reconstruct_openai_response` derives `finish_reason`: `"tool_calls"` when the message carries tool calls, otherwise the last non-null upstream value (so a truncated turn stays reported as `"length"`), defaulting to `"stop"`. It carries `id` / `created` / `model` / `system_fingerprint` / `usage` through from the chunk envelope and stamps `"object": "chat.completion"`. It also tolerates `"delta": null` on a terminal chunk, which some OpenAI-compatible providers send instead of `{}`, in the same spirit as #2467. - New `_openai_response_to_chunks` splits a non-streaming `chat.completion` body into proper `chat.completion.chunk` frames (a role delta, a content delta, one delta per tool call, then a terminal frame carrying `finish_reason`). `_response_to_sse` uses it and then emits `[DONE]`. The Anthropic branch still delegates to `StreamingMixin._response_to_sse` and is untouched. Tests in `tests/test_ccr_response_handler_extra.py`: - Seven new tests: OpenAI CCR detection on a `tool_calls` delta (plus a non-CCR negative case), a short OpenAI stream passing through byte for byte, a stream past the 10 000-byte flush threshold keeping its tail, a stream with no `[DONE]` sentinel still flushing, `finish_reason` becoming `"tool_calls"` with the envelope preserved, the upstream `finish_reason` being kept when there are no tool calls, and `_response_to_sse` emitting parseable chunk frames. - `test_streaming_handler_falls_back_to_buffer_on_processing_error` now feeds genuine OpenAI SSE bytes instead of Anthropic ones, so it exercises the OpenAI detection path it was always meant to. - `test_response_to_sse_formats` asserts the new chunk-frame shape for OpenAI. The Anthropic half is unchanged. No behaviour change for `provider="anthropic"` beyond the end-of-iterator flush, which can only add data that was previously discarded. ## Testing - [x] Unit tests added/updated - [x] Existing tests pass - [ ] Manual testing performed - [ ] Integration tests added Each of the seven new tests was confirmed to fail against the unmodified source (`git stash` on `response_handler.py` alone, tests untouched), so they are genuine regression tests rather than assertions written to match current behaviour: ``` $ git stash push -- headroom/ccr/response_handler.py $ python -m pytest tests/test_ccr_response_handler_extra.py -q -k openai FAILED tests/test_ccr_response_handler_extra.py::test_streaming_buffer_detects_ccr_in_openai_tool_calls_delta FAILED tests/test_ccr_response_handler_extra.py::test_openai_stream_without_ccr_yields_every_chunk FAILED tests/test_ccr_response_handler_extra.py::test_openai_stream_past_flush_threshold_keeps_the_tail FAILED tests/test_ccr_response_handler_extra.py::test_openai_stream_without_done_sentinel_still_flushes FAILED tests/test_ccr_response_handler_extra.py::test_reconstruct_openai_response_marks_tool_calls_finish_reason FAILED tests/test_ccr_response_handler_extra.py::test_reconstruct_openai_response_keeps_upstream_finish_reason FAILED tests/test_ccr_response_handler_extra.py::test_response_to_sse_emits_openai_chunk_frames 7 failed, 2 passed, 13 deselected in 0.79s ``` With the fix applied, the full CCR response-handler suite passes: ``` $ python -m pytest tests/test_ccr_response_handler_extra.py tests/test_ccr_response_handler.py -q collected 57 items tests\test_ccr_response_handler_extra.py ...................... [ 38%] tests\test_ccr_response_handler.py ................................... [100%] ============================= 57 passed in 1.74s ============================== ``` Wider CCR and streaming surface: ``` $ python -m pytest tests/ -k "ccr or streaming" -q 4 failed, 696 passed, 73 skipped, 10949 deselected, 2 warnings in 175.80s (0:02:55) ``` The 4 failures are pre-existing on a clean `upstream/main` and unrelated to this change (verified by stashing both changed files and re-running exactly those four): `test_ccr_mcp_http.py::test_streamable_http_initialize_and_list_tools`, `test_cli_proxy_env.py::TestCLICompressionOnlyFlags::test_ccr_defaults_on`, and two in `test_transforms/test_smart_crusher_ccr_roundtrip.py`. Lint and types: ``` $ python -m ruff check . All checks passed! $ python -m ruff format --check . 1505 files already formatted $ python -m mypy headroom --ignore-missing-imports Found 12 errors in 3 files (checked 521 source files) ``` Zero mypy errors in `headroom/ccr/response_handler.py`. The 12 are pre-existing, in `ccr/mcp_server.py`, `memory/mcp_server.py`, and `release_version.py`, none of which this PR touches (they come from a locally installed `mcp` whose stubs differ from CI's). ## Real Behavior Proof - Environment: Windows 11, Python 3.13.11, pytest 9.1.1, ruff and mypy from the repo's pinned config, branch `fix/ccr-streaming-openai-path` off `upstream/main` at `cbb950a4`. - Exact command / steps: `python -m pytest tests/test_ccr_response_handler_extra.py tests/test_ccr_response_handler.py -q`; then `git stash push -- headroom/ccr/response_handler.py` and `python -m pytest tests/test_ccr_response_handler_extra.py -q -k openai` to confirm the new tests fail without the source fix; then `python -m pytest tests/ -k "ccr or streaming" -q`; then `python -m ruff check .`, `python -m ruff format --check .`, `python -m mypy headroom --ignore-missing-imports`. - Observed result: 57/57 pass in the CCR response-handler suites with the fix; all 7 new tests fail without it. The wider run is 696 passed with 4 failures that reproduce identically on an unmodified tree. Ruff clean, mypy clean on the changed file. In `test_openai_stream_without_ccr_yields_every_chunk` the handler now returns every input chunk byte for byte, where before it returned an empty list. - Not tested: no end-to-end run against a live OpenAI-compatible backend, because no proxy handler instantiates `StreamingCCRHandler` today, so there is no wired path to drive. Coverage is at the class level using recorded-shape SSE frames. The Anthropic path is covered only by the existing tests, which still pass unchanged. ## Runtime Rollout Safety - Rollout-managed feature(s): none. `StreamingCCRHandler` is not gated by a rollout feature and is not reachable from any proxy handler. - Minimum rollout channel: not applicable; no rollout gate is involved. - Stable/default behavior changed: no. For `provider="anthropic"` the only behavioural difference is that bytes left buffered when the source iterator ends are now flushed instead of discarded, which can only add data the client previously lost. For `provider="openai"` the class was non-functional, so there is no prior behaviour to preserve. - Kill switch / disable path: not applicable; no new configuration, env var, or feature flag is introduced. - Unsafe override required: no. - Qualification impact: none. No qualification-gated surface is touched. - Rollback path: revert this commit. It is self-contained in `headroom/ccr/response_handler.py` and `tests/test_ccr_response_handler_extra.py`, with no schema, config, or persisted-state changes. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review Two judgement calls worth a reviewer's attention: 1. **Removing the dead re-iteration block** in `process_stream`. I am confident it was unreachable (the only `break` above it requires `detected_ccr`, which its own guard excludes), but it is the one deletion in this diff rather than an addition, so it is worth a second pair of eyes. 2. **The unconditional end-of-iterator flush.** I chose to flush regardless of whether an end marker matched, rather than only fixing the OpenAI marker. That makes the truncation bug unreachable even if a future provider uses a shape neither marker recognises. The cost is that a stream whose trailing bytes are genuinely not meant for the client would now be forwarded. Given the buffer only ever holds upstream response bytes, forwarding is the safer default, but flag it if you disagree. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Radhakrishnan Pachyappan
|
eeb038bc0c |
fix(opencode): send x-headroom-project header on all proxied requests (#2868)
## Description The OpenCode transport plugin set `HEADROOM_PROJECT` as a shell env var for child processes but never forwarded it as `x-headroom-project` on the actual proxied HTTP requests. The proxy's `classify_project` only attributes traffic via `x-headroom-project` header or `/p/<name>` URL prefix — without the header, every OpenCode request was unattributed and the Per-Project Savings dashboard showed `0 project(s)` permanently. Fixes #2847. ## Root cause `installHeadroomTransport` was called with only `{ proxyUrl, debug }`. The `project` value was computed and used only in the `shell.env` hook (for subprocess env injection), never threaded through to `mergeFetchHeaders` or `headersForNodeRequest`. ## Changes Made 1. Add `project?: string` to `InstallOptions` and `TransportState`. 2. Resolve the project value once at plugin init (`pluginOptions.project → input.project.id → input.directory`) and pass it to `installHeadroomTransport`. 3. Both header-building seams now set `x-headroom-project` when a project is present: - `mergeFetchHeaders` (wrapped `fetch` path) - `headersForNodeRequest` (wrapped `http.request` / `https.request` path) 4. Reuse the resolved `project` in the `shell.env` hook (removes the duplicate resolution that was there before). ## Changes - `plugins/opencode/src/transport.ts` — `InstallOptions.project`, `TransportState.project`; `mergeFetchHeaders`, `headersForNodeRequest`, `routedNodeOptions`, `withRoutedFetchInput`, `installHeadroomTransport` updated - `plugins/opencode/src/plugin.ts` — resolve `project` once, pass it to transport; reuse in `shell.env` - `plugins/opencode/src/transport.test.ts` — 3 new tests: project header on fetch, project header on https.request, no header when project unset - `headroom/providers/opencode/_dist/entry.opencode.js` — rebuilt with `npm run build:standalone` to match source ## Testing - [x] Unit tests pass - [x] TypeScript typecheck passes - [x] New regression tests added ### Test Output ``` cd plugins/opencode && npm test # 17 passed (14 existing + 3 new) ``` TypeScript build also passes: `npm run typecheck` (no errors). ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature - [ ] Breaking change - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring ## Real Behavior Proof - Environment: OpenCode transport plugin test environment on the current PR head. - Exact command / steps: ran the plugin test suite and TypeScript typecheck after rebuilding the standalone bundle. - Observed result: all 17 tests passed, including project-header coverage for fetch and Node HTTPS paths plus the unset-project control; typechecking passed. - Not tested: a live OpenCode session against a deployed Headroom proxy. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review --------- Signed-off-by: Radhakrishnan P <gingeekrishna@gmail.com> Signed-off-by: Radhakrishnan Pachyappan <gingeekrishna@gmail.com> |
||
|
Abhay Singh
|
2cae0f8eaf |
fix(proxy/cache): strip cache_control from messages in the semantic cache key (#3086)
## Description
The proxy semantic response-cache key (`compute_semantic_cache_key`)
strips `cache_control` from the response-shaping fields (`system`,
`tools`, ...) so that a moved prompt-cache breakpoint does not fragment
the key:
```python
{
"model": model,
"messages": messages, # hashed verbatim
**{k: strip_cache_control(v) for k, v in key_fields.items()}, # stripped
}
```
But `messages` was hashed **verbatim**. Messages are the primary key
component, and on the Anthropic path they are the most common place a
client (e.g. Claude Code) places and *moves* a `cache_control`
breakpoint between turns (on the last user turn / a `tool_result`
block). So two otherwise-identical requests that differed only in a
message-level breakpoint produced different keys and missed the semantic
cache — the exact fragmentation the `strip_cache_control` helper exists
to prevent, applied to everything except the field that matters most.
The existing tests pin the strip for `system`
(`test_cache_control_breakpoint_move_same_key`) and `tools`
(`test_tools_cache_control_ignored`), but never covered a message-level
breakpoint, so the gap went unnoticed.
## Fix
Apply `strip_cache_control` to `messages` as well. `cache_control` is a
prompt-caching directive for the upstream provider that never changes
the generated completion, so removing the annotation before hashing is
sound: message *content* still differentiates the key, and two requests
that differ only in a `cache_control` breakpoint now share the cache
entry (whose stored response body is identical either way).
The proxy carries two in-sync copies of this pure policy
(`semantic_cache_key_policy.py`, imported by the runtime
`SemanticCache`, and `semantic_cache_key.py`, imported by the policy
test); both are updated identically so they do not diverge.
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/proxy/semantic_cache_key_policy.py` and
`headroom/proxy/semantic_cache_key.py`: hash
`strip_cache_control(messages)` instead of `messages`, with a docstring
explaining why message-level breakpoints must not fragment the key.
- `tests/test_proxy_semantic_cache_key.py`: added
`test_message_cache_control_breakpoint_move_same_key` (behavioral,
through `SemanticCache._compute_key`) and
`test_message_content_change_still_distinct_key` (guards that stripping
does not collapse genuinely different messages).
- `tests/test_proxy_semantic_cache_key_policy.py`: added
`test_semantic_cache_key_ignores_moved_message_cache_control` at the
pure-policy level.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] Type checking passes (`mypy`)
- [x] New tests added
### Test Output
```text
tests/test_proxy_semantic_cache_key.py + tests/test_proxy_semantic_cache_key_policy.py 33 passed
# uvx ruff@0.15.22 check -> All checks passed!
# uvx mypy@1.20.2 (both policy modules) -> Success: no issues found in 2 source files
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1,
ruff 0.15.22 and mypy 1.20.2 via uvx.
- Exact command / steps: reverted the two policy modules and ran the new
tests to capture the bug (`python -m pytest
tests/test_proxy_semantic_cache_key.py::test_message_cache_control_breakpoint_move_same_key
tests/test_proxy_semantic_cache_key_policy.py::test_semantic_cache_key_ignores_moved_message_cache_control`
-> both failed with two distinct SHA-256 keys for messages that differ
only in a `cache_control` breakpoint); restored the fix; re-ran both key
suites (`python -m pytest tests/test_proxy_semantic_cache_key.py
tests/test_proxy_semantic_cache_key_policy.py` -> 33 passed); ran the
wider `tests/test_cache/` suite and confirmed the only failures
(`test_client_integration.py`) reproduce identically on clean `main` and
are unrelated to this change; then `uvx ruff@0.15.22 format`, `uvx
ruff@0.15.22 check`, and `uvx mypy@1.20.2` on both modules.
- Observed result: before the fix, a request whose last message carries
`cache_control: {type: ephemeral}` hashes to a different key than the
same request without it; after the fix they hash identically (a cache
hit), while messages with different text still hash differently.
- Not tested: a live multi-turn proxy session measuring the hit-rate
improvement (the key contract is verified directly through
`SemanticCache._compute_key` and the pure policy, which is what the
runtime calls).
## Runtime Rollout Safety
- Rollout-managed feature(s): none. This is the pure semantic-cache key
policy behind `SemanticCache`, not a rollout-channel-gated runtime
feature.
- Minimum rollout channel: N/A (no rollout-managed behavior).
- Stable/default behavior changed: yes, as a bug fix. Requests that
differ only in a message-level `cache_control` breakpoint now share a
semantic-cache key (a hit) instead of missing. No request that differs
in message content, model, or any shaping field changes key. Because the
cache key changes shape, any entries stored under the old (un-stripped)
keys are simply not reused and age out under the existing TTL/LRU — a
one-time cold start for the affected entries, never a wrong response.
- Kill switch / disable path: the semantic cache itself is already gated
by the existing cache-enable configuration; disabling it bypasses this
path entirely.
- Unsafe override required: no.
- Qualification impact: higher semantic-cache hit rate on the Anthropic
path where clients move `cache_control` breakpoints between turns; no
change to which distinct requests are considered equal beyond ignoring
the caching directive.
- Rollback path: revert this PR; the key returns to hashing messages
verbatim.
## 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
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective
- [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
## Additional Notes
Same class as the `system`/`tools` breakpoint handling already in place
(issue #327 kept the strip from fragmenting the key on a hit); this
extends it to messages, the primary key component. The two in-sync
policy copies are updated together to avoid divergence; consolidating
them into one module is left out of scope for this bug fix.
|
||
|
Abhay Singh
|
9ca5a16bde |
fix(proxy/anthropic): coerce present-null usage counters on the buffered backend path (#3084)
## Description
The buffered (non-streaming) Anthropic backend branch in
`handle_anthropic_messages` (`headroom/proxy/handlers/anthropic.py`) —
the path taken by Bedrock / Vertex / LiteLLM(anthropic) traffic — read
the response usage counters with a bare default:
```python
output_tokens = usage.get("output_tokens", 0)
...
cr_tokens = usage.get("cache_read_input_tokens", 0)
cw_tokens = usage.get("cache_creation_input_tokens", 0)
```
A backend can report these counters as JSON `null` (key **present**,
value null) rather than omitting them. For a present-null key
`dict.get(key, 0)` returns `None`, not the default `0`. That `None` then
flowed into:
```python
provider_input_tokens=(uncached_input_tokens + cr_tokens + cw_tokens)
```
raising `TypeError: unsupported operand type(s) for +: 'NoneType' and
'NoneType'`, which the outer handler converted into a failed turn (HTTP
500 `api_error`) instead of a normal 200 with zeroed counters.
The direct-Anthropic-API branch a few hundred lines down already guards
this exact case with `int(usage.get(key, 0) or 0)`, and the surrounding
code even comments that a backend may "send null" for `input_tokens`
(and None-guards that field). The buffered branch was simply left
behind, so the two parallel paths disagreed on null handling.
## Fix
Coerce the three counters on the buffered path with `int(usage.get(key,
0) or 0)`, exactly matching the direct-API idiom, so a present-null
value becomes `0` instead of `None`. The already-present `input_tokens
is not None` guard is unaffected, and its fallback subtraction now
operates on coerced ints.
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/proxy/handlers/anthropic.py` (buffered backend branch of
`handle_anthropic_messages`): coerce `output_tokens`,
`cache_read_input_tokens` and `cache_creation_input_tokens` with
`int(usage.get(key, 0) or 0)` so a present-null value is treated as `0`,
matching the direct-Anthropic path.
- `tests/test_backend_nonstreaming_cache_metrics.py`: added
`test_anthropic_backend_nonstreaming_present_null_cache_counters_do_not_crash`,
driving the buffered backend path with present-null `output_tokens` /
`cache_read_input_tokens` / `cache_creation_input_tokens` and asserting
a 200 with a recorded `RequestOutcome` whose counters are `0` and whose
uncached input comes from the present `input_tokens`.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] Type checking passes (`mypy`)
- [x] New tests added
### Test Output
```text
tests/test_backend_nonstreaming_cache_metrics.py 7 passed
# uvx ruff@0.15.22 check -> All checks passed!
# uvx mypy@1.20.2 headroom/proxy/handlers/anthropic.py -> Success: no issues found in 1 source file
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1,
ruff 0.15.22 and mypy 1.20.2 via uvx.
- Exact command / steps: ran the new regression against the unpatched
handler and captured the crash (`python -m pytest
tests/test_backend_nonstreaming_cache_metrics.py::test_anthropic_backend_nonstreaming_present_null_cache_counters_do_not_crash
-x -q` -> `assert 500 == 200` with body
`{"type":"error","error":{"type":"api_error","message":"unsupported
operand type(s) for +: 'NoneType' and 'NoneType'"}}`); applied the
`int(... or 0)` coercion; re-ran the whole file (`python -m pytest
tests/test_backend_nonstreaming_cache_metrics.py -q` -> 7 passed); then
`uvx ruff@0.15.22 format`, `uvx ruff@0.15.22 check`, and `uvx
mypy@1.20.2 headroom/proxy/handlers/anthropic.py`.
- Observed result: before the fix a backend response whose usage carries
`cache_read_input_tokens: null` (or a null `output_tokens` /
`cache_creation_input_tokens`) returned HTTP 500 and recorded no
outcome; after the fix the same response returns 200, the counters
coerce to `0`, and the `PERF` line reports `cache_read=0 cache_write=0`.
- Not tested: a live Bedrock/Vertex session emitting a real null-counter
usage block (the null-usage shape is reproduced directly through the
mocked backend that the existing suite already uses for this path).
## Runtime Rollout Safety
- Rollout-managed feature(s): none. This is the buffered Anthropic
response-accounting path behind `handle_anthropic_messages`, not a
rollout-channel-gated runtime feature.
- Minimum rollout channel: N/A (no rollout-managed behavior).
- Stable/default behavior changed: yes, as a bug fix. A backend response
with present-null usage counters now completes with a 200 and zeroed
counters instead of failing the turn with a 500. Responses with numeric
counters are unaffected.
- Kill switch / disable path: N/A. There is no behavioral toggle; the
change only hardens numeric coercion on the accounting path and does not
alter routing, compression, or request forwarding.
- Unsafe override required: no.
- Qualification impact: Bedrock / Vertex / LiteLLM(anthropic)
non-streaming turns that report a null cache/output counter stop 500-ing
and are recorded with zeroed counters, matching the direct-Anthropic
path.
- Rollback path: revert this PR; the buffered path returns to the bare
`usage.get(key, 0)` reads.
## 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
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective
- [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
## Additional Notes
This mirrors the recently fixed Gemini CCR-continuation present-null
usage bug: the same `dict.get(key, default)` present-null trap, on the
parallel Anthropic backend path. Only the buffered (non-streaming)
backend branch was affected; the direct-Anthropic and streaming paths
already coerce with `or 0`.
|
||
|
Parideboy
|
c3c921f2f7 |
test(install/windows): verify the PATH guard against the real HKCU registry (#3068)
## Description Follow-up requested in review of #2972, on top of the merged fix for #2970 (#2985). Test-only; no production code is touched and the `HEADROOM_INSTALL_PATH_SCOPE` mechanism is unchanged. `test_powershell_installer_does_not_leak_into_user_path` currently guards the fix by comparing the entry count of `[Environment]::GetEnvironmentVariable('Path','User')` across an installer run. That infers success from the environment variable rather than verifying it, and it leaves three gaps: - The .NET getter expands `%USERPROFILE%`-style references, so it cannot observe a change of the registry value kind (`REG_EXPAND_SZ` vs `REG_SZ`) at all. - A count comparison passes when an entry is replaced or reordered rather than appended. - There is no restore path. If the guard regresses, the test reports the leak and then leaves the polluted value behind in the contributor's registry, which is precisely the damage #2970 described: the test that detects the pollution also causes it. This PR reads `HKCU\Environment` directly instead, so the assertion verifies the guard rather than assuming it. ## Type of Change - [ ] 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 - [x] Code refactoring (no functional changes) ## Changes Made - `tests/test_install/test_native_installers.py`: new `_read_user_path_entry` helper returning the raw `HKCU\Environment` `Path` value together with its registry kind (or `None` when the value is absent), and `_restore_user_path_entry` writing that exact value and kind back. Both import `winreg` inside the function body, so the module still imports on non-Windows hosts. - `tests/test_install/test_native_installers.py`: `test_powershell_installer_does_not_leak_into_user_path` now records the raw value before the run and asserts both that the throwaway install dir is absent from the value afterwards (naming the #2970 symptom in the failure message) and that value and kind are byte-identical. The PowerShell subprocess that counted PATH entries is gone, so the test also spawns one process fewer. - `tests/test_install/test_native_installers.py`: the test now runs under `try/finally`. The `finally` cleans up the fake docker state, which this test was missing relative to its sibling `test_powershell_native_installer_supports_persistent_docker_lifecycle`, and restores the recorded registry value only when it actually changed, so a passing run performs zero registry writes and a regressed run cannot leave the contributor's PATH polluted. The scope allow-list tests added by #2985 (`_ENSURE_PATH_SCOPE_HARNESS`, `test_path_scope_accepts_process_case_insensitively`, `test_path_scope_rejects_machine_and_invalid_values`) are untouched. ## 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 ### Test Output ```text $ uv run pytest tests/test_install/test_native_installers.py -q platform win32 -- Python 3.13.11, pytest-9.0.3, pluggy-1.6.0 collected 5 items tests\test_install\test_native_installers.py s.... [100%] ======================== 4 passed, 1 skipped in 23.59s ======================== $ uv run ruff check . All checks passed! $ uv run ruff format --check tests/test_install/test_native_installers.py 1 file already formatted $ uv run mypy headroom --ignore-missing-imports Success: no issues found in 521 source files ``` The strengthened assertion was proven to detect a regression by temporarily neutralising the scope override in `scripts/install.ps1` (`if ($false -and $env:HEADROOM_INSTALL_PATH_SCOPE)`), so `Ensure-PathEntry` writes the `User` scope unconditionally again: ```text $ uv run pytest tests/test_install/test_native_installers.py -q -k does_not_leak_into_user_path tests\test_install\test_native_installers.py:638: in test_powershell_installer_does_not_leak_into_user_path assert str(home) not in (after[0] if after else ""), ( E AssertionError: installer leaked the throwaway install dir into the real User PATH: E C:\Users\<user>\AppData\Local\Temp\pytest-of-<user>\pytest-154\test_powershell_installer_does0\home ======================= 1 failed, 4 deselected in 2.72s ======================= ``` ## Real Behavior Proof - Environment: Windows 11 Pro 10.0.26200, PowerShell 7, Python 3.13.11, pytest 9.0.3, headroom at `main` (`a6ab359a`), provider Anthropic - Exact command / steps: recorded the raw `HKCU\Environment` `Path` value with `python -c "import winreg; ...QueryValueEx(k,'Path')"`, capturing its registry kind, entry count and a SHA-256 of the value; ran the full installer test file on the patched tree; re-read the registry; then neutralised the scope override in `scripts/install.ps1` as shown above, re-ran the single leak test, and re-read the registry a third time to confirm the failure path restored it. - Observed result: baseline `kind 1 entries 21 sha256 683ee646a95b8a28`. After the passing run the value was identical (`kind 1 entries 21 sha256 683ee646a95b8a28`), so a passing run writes nothing. With the override neutralised the test failed as quoted above and the registry read afterwards was again byte-identical to the recorded backup (compared as an exact `{value, kind}` match, `True`), confirming the `finally` restore. After reverting `scripts/install.ps1`, the full file is back to 4 passed, 1 skipped with the registry still unchanged. - Not tested: non-Windows hosts (the changed test is Windows-only and already skipped elsewhere; `scripts/install.sh` is untouched), elevated/admin installs, and the `Machine` scope, which `Ensure-PathEntry` rejects outright. One open question this change is positioned to catch but does not resolve: on this host the `HKCU\Environment` `Path` value is `REG_SZ` (kind `1`), not `REG_EXPAND_SZ`. A real install persists through `[Environment]::SetEnvironmentVariable(..., 'User')`, which is the API class known to rewrite that value, so it is possible that a production install silently downgrades an expandable PATH and freezes `%USERPROFILE%`-style entries. I have not verified whether headroom's installer caused it on this machine or whether the value was always `REG_SZ`, and this PR deliberately does not chase it. Happy to open a separate issue if that is worth investigating. ## Runtime Rollout Safety - Rollout-managed feature(s): none (test-only change) - Minimum rollout channel: n/a - Stable/default behavior changed: no; no production code path is modified - Kill switch / disable path: n/a - Unsafe override required: no - Qualification impact: none - Rollback path: revert this commit; the test returns to the entry-count comparison ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
gglucass
|
6c9f41e08c |
perf(perf): skip rotated logs outside the requested window (#3081)
## Description
`parse_log_files(last_n_hours=N)` reads every `proxy.log*` file in full
— line by line, applying the PERF / STAGE_TIMINGS / ROUTER regexes to
each — and only then filters records against the cutoff. The cost of a
windowed query is O(retained log history), not O(window).
`/stats` is the hot caller. `_build_stats_payload` recomputes throughput
over `last_n_hours=1.0` behind a 10s cache TTL, so anything polling the
endpoint re-reads and re-regexes the entire rotated set every 10 seconds
for an answer that lives in the tail of the newest file or two.
Rotation caps the log directory at 10 MB × 5 backups
(`proxy/helpers.py`), so this is a bounded ~60 MB rather than an
unbounded leak. But it is a fixed tax that ramps up as a user's logs
fill toward that ceiling and then stays there — on a machine that has
reached the cap it is ~0.43s of pure waste on every stats rebuild.
The fix: skip any file whose mtime predates the cutoff. The logs are
append-only, so a file untouched since before the window cannot contain
a record inside it. `--hours 0` ("all data") still reads everything.
## Type of Change
- [x] Performance improvement
## Changes Made
- `parse_log_files` prunes rotated files by mtime before opening them;
files are `stat`'d once and the value reused for the ordering
(previously `stat`'d once per file anyway, as the sort key).
- A file that rotates away between `glob` and `stat` is skipped instead
of raising `OSError`.
- New `PerfReport.log_files_skipped` so coverage reporting stays honest
— `log_files_read` on its own would silently understate how much log
exists on disk. Defaulted, so existing callers are unaffected.
## 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
Both new tests were confirmed to fail against unpatched `main`. The
windowed one fails on behavior (`total_lines_parsed`: `assert 2 == 1`),
not merely on the new field — the assertion order is deliberate, since a
read-then-filter implementation produces the same records and only
differs in work done.
### Test Output
```text
$ uv run --frozen --extra dev pytest tests/test_cli_perf_format.py \
tests/test_proxy_dashboard_stats_cache.py tests/test_agent_savings.py -q
59 passed, 1 skipped, 1 warning in 3.36s
$ uvx ruff check headroom/perf/analyzer.py tests/test_cli_perf_format.py
All checks passed!
$ uvx ruff format --check headroom/perf/analyzer.py tests/test_cli_perf_format.py
2 files already formatted
$ uv run --frozen --extra dev mypy headroom/perf/analyzer.py
Success: no issues found in 1 source file
```
## Real Behavior Proof
- Environment: macOS 15 (arm64), Python 3.10.18, headroom-ai at
|
||
|
gglucass
|
c5563d3a7d |
fix(learn): include stdout in CLI failure messages, not just stderr (#3080)
## Description `headroom learn` reports CLI backend failures using **stderr only**. `claude -p --output-format stream-json --verbose` writes *nothing* to stderr when the run fails at the API layer, so the failure a user actually sees is a message that stops at the colon: ```text LLM analysis failed: `claude -p --output-format stream-json --verbose` failed (exit 1): ``` The reason is not missing, it is discarded. Claude Code still emits a final `result` event on stdout whose `result` field is the human-readable cause, and the streaming path has already parsed it into `final_result` one line above the `raise`. This makes a whole class of failures undiagnosable for users and maintainers alike: a usage limit, an unreachable local proxy, and an expired login all render identically as an empty message. Reported by a desktop user who could only tell us "sometimes i have this LLM analysis failed" with nothing after the colon. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - Add `_failure_detail(stderr, stdout, *, result_text=None)` in `headroom/learn/analyzer.py`. Prefers the already-parsed `result` text, falls back to the **tail** of stdout (CLI backends emit the error last, after their whole event log), keeps stderr when present, and returns `"(no output captured)"` so the message is never a dangling colon. - Use it in `_call_claude_cli_streaming` (streaming claude-cli path) and in `_call_cli_llm` (the `subprocess.run` backends, gemini-cli / codex-cli), so the same blind spot is closed for every CLI backend rather than only the one that was reported. - Existing truncation behaviour is unchanged: each stream is still capped at `_MAX_SNIPPET_LEN`. Complements #3016, which makes an analysis failure propagate instead of being swallowed as success; that PR fixes *whether* the user learns a failure happened, this one fixes *what* the failure says. No overlapping lines. ## 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 ### Test Output ```text $ uv run --frozen --extra dev pytest tests/test_learn/ -q 247 passed, 4 skipped in 27.08s $ uvx ruff check headroom/learn/analyzer.py tests/test_learn/test_analyzer.py All checks passed! $ uvx ruff format --check headroom/learn/analyzer.py tests/test_learn/test_analyzer.py 2 files already formatted $ uv run --frozen --extra dev mypy headroom/learn/analyzer.py Success: no issues found in 1 source file ``` New tests: `test_claude_cli_nonzero_exit_includes_api_error_from_stdout`, `test_claude_cli_nonzero_exit_with_no_output_says_so`, `test_claude_cli_nonzero_exit_keeps_stderr_when_present`, `test_codex_nonzero_exit_includes_stdout_when_stderr_empty`. ## Real Behavior Proof - Environment: macOS 15.6 (Darwin 24.6.0), Claude Code 2.1.228, Python 3.10.18, headroom on this branch. - Exact command / steps: forced an API-layer failure in the exact command the analyzer runs, capturing the streams separately: `echo "say hi" | claude -p --output-format stream-json --verbose --settings '{"env":{"ANTHROPIC_BASE_URL":"http://127.0.0.1:9"}}' > out.txt 2> err.txt; echo "EXIT=$?"; wc -c err.txt; tail -c 400 out.txt` - Observed result: `EXIT=1`, `err.txt` is **0 bytes**, and the reason appears only in the last stdout line: `"terminal_reason":"api_error", ..., "result":"API Error: Connection refused — a firewall or proxy may be blocking it (ConnectionRefused)"`. A second run with `--bare` produced the same shape with `"result":"Not logged in · Please run /login"`. Before this change both surface as `failed (exit 1):` with nothing after the colon; after it, the `result` text is in the message. The unit tests encode this exact stream shape (stdout `result` event, empty stderr, exit 1). - Not tested: real usage-limit and 429 responses, which I cannot provoke on demand. They travel the same code path as the reproduced `api_error` case (final `result` event on stdout, empty stderr), so they are covered by construction rather than by observation. Windows and the gemini-cli backend were not exercised manually; the shared helper is covered by unit tests for both the streaming and `subprocess.run` paths. ## Runtime Rollout Safety - Rollout-managed feature(s): None. This touches only the error text raised by `headroom learn`'s CLI backends; no rollout-gated feature, flag, or runtime component is involved. - Minimum rollout channel: N/A, not rollout-gated. Ships with the package like any other library fix. - Stable/default behavior changed: Yes, narrowly. The message text of an existing `RuntimeError` on a non-zero CLI exit now includes the stdout/`result` reason alongside stderr. No control flow, exit code, public API, or return value changes: the same exception is raised in the same cases. - Kill switch / disable path: None needed. Nothing is enabled or newly executed, so there is nothing to switch off; the only behavioral surface is the string inside an exception that was already being raised. - Unsafe override required: No. - Qualification impact: None. No qualification-gated path, model, or provider behavior is touched. Callers that pattern-match this message on `"failed (exit N)"` still match, since that prefix is unchanged. - Rollback path: Revert this commit. The previous stderr-only message returns with no migration, state, or config to undo. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Abhay Singh
|
58f28dc7a6 |
fix(install): honor HEADROOM_PORT in install apply and deploy (#3085)
## Description
`headroom install apply --preset persistent-service` and `headroom
deploy` ignored an explicit `HEADROOM_PORT` and always configured port
8787, even though `headroom proxy --port` honors `HEADROOM_PORT`. Anyone
running a second instance, or avoiding a port conflict, got a silently
wrong configuration, and the failure is especially confusing because the
override *appears* supported on the direct proxy path.
Root cause: the `--port` options on the `install apply` and `deploy`
commands were declared with a hardcoded `default=8787` and **no**
`envvar` binding:
```python
@click.option("--port", "-p", default=8787, type=int, show_default=True, help="Persistent proxy port.")
```
The proxy command's `--port` already carries `envvar="HEADROOM_PORT"`,
so the two paths disagreed. `build_manifest` /
`_build_deployment_manifest` already thread the `port` argument all the
way through to the generated `HEADROOM_PORT` base-env and the health
URL, so the value was simply never resolved from the environment at the
CLI boundary.
## Fix
Bind both `--port` options to `envvar="HEADROOM_PORT"`, matching the
proxy command. Click resolves the value from the environment when
`--port` is not passed, and an explicit `--port` still wins over the env
var (standard Click precedence: explicit CLI argument over `envvar` over
`default`).
## Scope
This addresses **bug 1** of #3072. Bug 2 (`install status` reporting
`Status: stopped` alongside `Healthy: yes`, disagreeing with `doctor`)
is an unrelated status-reporting concern that the reporter offered a
live repro for; it is left for a separate follow-up rather than bundled
here.
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/cli/install.py`: add `envvar="HEADROOM_PORT"` to the
`--port` option on both `install apply` and `deploy` (and note the env
var in each help string), matching `headroom proxy --port`.
- `tests/test_cli/test_install_cli.py`: added
`test_install_apply_honors_headroom_port_env`,
`test_install_apply_explicit_port_overrides_env`, and
`test_deploy_honors_headroom_port_env`, capturing the `port` that
reaches the manifest builder.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] Type checking passes (`mypy`)
- [x] New tests added
### Test Output
```text
tests/test_cli/test_install_cli.py 40 passed
# uvx ruff@0.15.22 check -> All checks passed!
# uvx mypy@1.20.2 headroom/cli/install.py -> Success: no issues found in 1 source file
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1,
ruff 0.15.22 and mypy 1.20.2 via uvx.
- Exact command / steps: reverted the source fix and ran the two new
env-var tests to capture the bug (`python -m pytest
tests/test_cli/test_install_cli.py::test_install_apply_honors_headroom_port_env
tests/test_cli/test_install_cli.py::test_deploy_honors_headroom_port_env`
-> both failed with `assert 8787 == 8788`, proving `HEADROOM_PORT=8788`
was dropped); restored the fix; re-ran the full file (`python -m pytest
tests/test_cli/test_install_cli.py` -> 40 passed); then `uvx
ruff@0.15.22 format`, `uvx ruff@0.15.22 check`, and `uvx mypy@1.20.2
headroom/cli/install.py`.
- Observed result: with the fix, `HEADROOM_PORT=8788 headroom install
apply` (and `deploy`) resolves `port=8788` into `build_manifest`, so the
generated service config and `HEADROOM_PORT` base-env use 8788; passing
`--port 9999` alongside the env var still yields 9999.
- Not tested: an end-to-end persistent-service install on a machine with
a running supervisor (the CLI-to-manifest port resolution is verified
through the manifest builder, which already owns the downstream wiring
covered by the existing planner tests).
## Runtime Rollout Safety
- Rollout-managed feature(s): none. This is a CLI option-binding fix on
the install/deploy commands, not a rollout-channel-gated runtime
feature.
- Minimum rollout channel: N/A (no rollout-managed behavior).
- Stable/default behavior changed: only when `HEADROOM_PORT` is set in
the environment. Previously it was ignored (config wired to 8787); now
the install/deploy path honors it, matching `headroom proxy`. With no
`HEADROOM_PORT` set and no `--port`, the default is still 8787, so
existing installs are unaffected.
- Kill switch / disable path: unset `HEADROOM_PORT` (or pass `--port
8787`) to keep the prior port.
- Unsafe override required: no.
- Qualification impact: `install apply` / `deploy` now provision the
proxy on the operator's requested port instead of always 8787, so a
second instance or a port-conflict workaround configures correctly.
- Rollback path: revert this PR; the `--port` options return to ignoring
`HEADROOM_PORT`.
## 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
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective
- [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
## Additional Notes
Reported by @vsg-prog (split out of #3040 into #3072). The `--port`
option already carried the correct `type`/range validation and threaded
through the manifest builder; the only gap was the missing `envvar`
binding at the CLI boundary.
|
||
|
Abhay Singh
|
3ed8f76019 |
fix(providers): don't crash on a non-object HEADROOM_MODEL_LIMITS / models.json (#3089)
## Description
`_load_custom_model_config` in both `headroom/providers/anthropic.py`
and `headroom/providers/openai.py` loads the operator's custom model
configuration from `HEADROOM_MODEL_LIMITS` (a JSON string or a file
path) and `~/.headroom/models.json`, then reads it with
`loaded.get(...)`:
```python
loaded = json.loads(env_config) # or json.load(f)
anthropic_config = loaded.get("anthropic", loaded)
```
The `try` guards only `except (json.JSONDecodeError, OSError)`. When the
value is **valid JSON but not an object** (a JSON array, number, string,
bool, or `null`), `json.loads` succeeds and returns a non-dict, so
`loaded.get(...)` raises `AttributeError` — which is *not* one of the
caught types. Instead of the intended warn-and-fall-back-to-defaults, a
misconfigured `HEADROOM_MODEL_LIMITS` (e.g.
`HEADROOM_MODEL_LIMITS='[1,2,3]'` or `'"gpt-4"'`) crashes provider
initialization. The same gap exists in the `models.json` branch of both
providers.
## Fix
After each load, validate `isinstance(loaded, dict)` and raise
`ValueError` with a clear message, and broaden the handler from `except
(json.JSONDecodeError, OSError)` to `except (ValueError, OSError)`.
`json.JSONDecodeError` is a subclass of `ValueError`, so this strictly
supersets the previous handling: every previously-caught malformed value
still warns and falls back, and a valid-JSON-but-non-object value now
does too, instead of crashing.
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/providers/anthropic.py` and `headroom/providers/openai.py`
(`_load_custom_model_config`): add an `isinstance(loaded, dict)` guard
(raising `ValueError`) after the env-var load and after the
`models.json` load, and change both `except` clauses to `(ValueError,
OSError)`.
- `tests/test_provider_model_fallback.py`: added parametrized
`test_non_object_env_var_falls_back_to_defaults` (array / string /
number / bool / null) for both providers, and
`test_non_object_config_file_falls_back_to_defaults` for a non-object
`models.json`.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] Type checking passes (`mypy`)
- [x] New tests added
### Test Output
```text
tests/test_provider_model_fallback.py 44 passed
# uvx ruff@0.15.22 check -> All checks passed!
# uvx mypy@1.20.2 headroom/providers/anthropic.py headroom/providers/openai.py -> Success: no issues found in 2 source files
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1,
ruff 0.15.22 and mypy 1.20.2 via uvx.
- Exact command / steps: reverted both providers and ran the new
regressions to capture the bug (`python -m pytest
tests/test_provider_model_fallback.py::TestAnthropicConfigLoading::test_non_object_env_var_falls_back_to_defaults
tests/test_provider_model_fallback.py::TestOpenAIConfigLoading::test_non_object_env_var_falls_back_to_defaults
tests/test_provider_model_fallback.py::TestAnthropicConfigLoading::test_non_object_config_file_falls_back_to_defaults`
-> 11 failed with `AttributeError` on `loaded.get` across the
array/string/number/bool/null shapes); restored the fix; re-ran the full
file (`python -m pytest tests/test_provider_model_fallback.py` -> 44
passed); then `uvx ruff@0.15.22 format`, `uvx ruff@0.15.22 check`, and
`uvx mypy@1.20.2` on both providers.
- Observed result: before the fix, `HEADROOM_MODEL_LIMITS='[1,2,3]'` (or
`'"gpt-4"'`, `'42'`, `'true'`, `'null'`) raised `AttributeError` out of
`_load_custom_model_config`; after the fix the same values log a warning
and the loader returns the default `{"context_limits": {}, "pricing":
{}[, "encodings": {}]}`, and a well-formed object config is unchanged.
- Not tested: a live proxy boot with a corrupt `HEADROOM_MODEL_LIMITS`
(the loader is exercised directly, which is the exact function provider
init calls).
## Runtime Rollout Safety
- Rollout-managed feature(s): none. This is defensive parsing in the
provider model-config loader, not a rollout-channel-gated runtime
feature.
- Minimum rollout channel: N/A (no rollout-managed behavior).
- Stable/default behavior changed: only for a previously-crashing input.
A non-object `HEADROOM_MODEL_LIMITS` / `models.json` now warns and uses
built-in defaults instead of raising. Well-formed object configs are
parsed exactly as before.
- Kill switch / disable path: N/A — remove or correct the malformed
config value to load custom limits.
- Unsafe override required: no.
- Qualification impact: a corrupt or mistyped model-limits value
degrades to built-in defaults with a warning rather than failing
provider init.
- Rollback path: revert this PR; the loader returns to catching only
`json.JSONDecodeError`/`OSError`.
## 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
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective
- [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
## Additional Notes
Both providers carry the same loader shape, so the guard and the widened
`except` are applied identically to keep them in sync. The message names
the offending source (`HEADROOM_MODEL_LIMITS` vs the resolved
config-file path) so the warning is actionable.
|
||
|
Tejas Chopra
|
0ec73faa28 |
fix(ccr): relay a successful upstream turn when post-processing fails (#3094)
## Description Closes #3088 The buffered CCR path flips a streaming turn to `stream: false` so a `headroom_retrieve` call can be resolved server-side. Everything it does *after* the provider answers — retrieval, memory tool calls, turn hooks, usage accounting, caching, SSE resynthesis — is post-processing layered on a turn that already succeeded and was already billed. When any of that raised, the entire turn surfaced to the client as a generic `api_error`. In the reported capture the provider returned a complete **69,351-byte** answer in 1.9s and the client received **1,841 bytes**: keepalives, then a synthesized failure. A paid-for response was discarded because a bookkeeping step downstream of it broke. **On the reporter's stated root cause:** the "≈30s compression timeout" inference does not hold. Their own log says *"[12 seconds later]"*, which matches 49 pings × the 0.25s post-commit interval, not 30s. And `COMPRESSION_TIMEOUT_SECONDS` guards `_count_offloaded`, which **fails open** to estimation and cannot propagate. So that correlation is a coincidence. **What actually raises is still unidentified**, and that is the second half of this report. The handler logged `f"Request failed: {type(e).__name__}: {e}"` with no `exc_info`, which is exactly why the reporter found "no visible traceback" — and why reading the entire post-upstream path (memory tool calls, `run_response_hooks`, CCR handling all catch internally) does not reveal it either. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - Capture the upstream response the moment it parses as a 200, before any post-processing can touch it. - Wrap the buffered operation so an unexpected raise relays that captured response as SSE instead of a synthesized error. - Log the exception with `exc_info=True`. Salvaging **without** this would paper over the defect permanently; the goal is to stop losing user turns while making the real bug diagnosable. - Refuse to salvage a response the client cannot safely consume. A reply still carrying an unresolved `headroom_retrieve` call is exactly the case the handler already fails closed on — relaying it would hand the client a tool call it is not expected to service and a marker nobody expanded. The check reuses the existing `residual_ccr_status` / `has_ccr_tool_calls` signals rather than inventing a second notion of "safe". **This is containment, not root cause.** It converts a hard failure on a successful turn into a degraded success, and makes the underlying raise visible so it can be fixed properly. I have said so in the commit message too, so this is not mistaken for a full diagnosis later. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] New tests added for new functionality - [x] Manual testing performed New `tests/test_buffered_ccr_salvage.py` covers the reported shape (thinking + text, and the captured `bash` tool_use turn with no retrieve call), that the healthy path is untouched, and that an unresolved retrieve call is never relayed. ### Test Output ```text # BEFORE (main) — the same test file reproduces the report exactly: E AssertionError: {"type": "error", "error": {"type": "api_error", "message": "An error occurred while processing your request. Please try again."}} E assert 502 == 200 # AFTER (this branch): $ pytest tests/test_buffered_ccr_salvage.py -q 8 passed, 1 warning in 2.94s $ pytest tests/ -q 3 failed, 11168 passed, 581 skipped in 421.26s (0:07:01) Same 3 failures as a clean-main baseline run on this machine: tests/test_graceful_shutdown.py::test_run_server_installs_cancelled_error_filter tests/test_learn/test_integration.py::TestCodexIntegration::test_full_pipeline tests/test_release_workflows.py::test_no_native_tls_in_wheel_build_tree $ ruff check . && ruff format --check . All checks passed! ``` ## Real Behavior Proof - Environment: this branch driven through the real FastAPI app with a stubbed upstream returning a complete 200 turn; macOS arm64, Python 3.12. - Exact command / steps: posted a buffered CCR turn, then forced a post-upstream step to raise (`_record_request_outcome`), standing in for whatever breaks in the field; ran the identical test file against `main` and against this branch. - Observed result: on `main` the client gets HTTP 502 with the report's literal `api_error` string; on this branch the client gets HTTP 200 `text/event-stream` carrying the provider's own content (`message_start`, thinking, text / `toolu_bash`) and no invented error. - Not tested: the field defect itself. What raises in the reporter's environment is still unknown — that is what the added traceback logging exists to surface. A follow-up will need their logs on a build carrying this change. ## Runtime Rollout Safety - Rollout-managed feature(s): none — this guards an existing code path and is not behind a rollout channel. - Minimum rollout channel: n/a (ships to stable with the fix). - Stable/default behavior changed: yes, and only in the failure case. A buffered turn whose post-processing raises now returns the upstream's answer instead of a 502 `api_error`. Successful turns are byte-identical. - Kill switch / disable path: no new switch. The guard only engages on an exception that previously produced a hard failure, so disabling it would restore the bug. - Unsafe override required: no. - Qualification impact: none — no qualification-gated surface is touched. - Rollback path: revert this commit; the previous behavior (synthesized `api_error`) returns. ## 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 - [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` ## Additional Notes Documentation update is marked N/A: no user-facing flag or endpoint changes. Type checking (`mypy headroom`) was not run separately; `ruff` is the gate this repo's CI enforces. Same family, still open: #3078, #3082, #3017, #2857, #2825. The added traceback is the fastest route to whether they share this root cause. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Rod Boev
|
c16be9bbbe |
fix(proxy/anthropic): don't replay recorded prefix over live history (#3026) (#3052)
## Description A Claude Code session that reads a large tool result through `headroom proxy` can fail on turn 2 with Anthropic's `400 prompt_too_long`. The reporter's controlled comparison completed eight turns through 0.33.0 with 187,986 input tokens, while 0.35.0 failed after five requests with 753,077 input tokens. The local regression uses an actual prior optimized request to populate tracker state, then a decision-false bypass turn with Claude-shaped tool-result content. The old unconditional replay path substitutes the compressed prefix; the eligibility gate preserves the client's outbound body without claiming a live provider reproduction. The Anthropic `/v1/messages` route computes whether a request should be compressed, but cached-prefix replay currently runs outside that decision. The replay helper also derives its prefix length from the original message list and applies that index to the optimized list without proving the two lists still align. A stale forwarded prefix can therefore be grafted onto the wrong positions and enlarge later requests. This change limits replay to requests whose existing compression decision permits it and whose pre-upstream backpressure path is inactive. It also makes `overlay_cached_prefix()` decline misaligned or inflating candidates while preserving normal append-only replay. Reported by @itsumonotakumi, whose controlled comparison isolated the failure from compression, headers, one-request serialization, memory, code graph, and CCR. Closes #3026 ## 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 - Gate Anthropic cached-prefix replay on the existing `CompressionDecision.should_compress` result and the existing pre-upstream backpressure state. - Require positional alignment between optimized and original message arrays before replay. - Reject replay candidates that would serialize larger than the current optimized messages. - Add focused handler coverage for the decision-false tool-result regression, bypass and backpressure paths, and outbound optimize-on preservation. - Add direct unit coverage for positional mismatch, no-inflation, and JSON sizing-failure bailouts. - Update the moved-cache-control and pure-block-append regression fixtures to keep the no-inflation contract explicit. - Run the unchanged OpenAI cache-stability preservation proof; no OpenAI production code was edited. ## Testing - [x] Unit tests pass (153 focused proxy, helper, cache-control, block-append, cross-turn, byte-faithful, Anthropic, OpenAI, and backpressure tests) - [x] Linting passes (Ruff check and format validation on the seven changed repository files) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality when applicable - [x] Manual testing performed with the in-process proxy and local stub upstream ### Test Output ```text python -m pytest tests/test_proxy/test_anthropic_no_optimize_history_passthrough.py tests/test_cache_prefix_overlay.py tests/test_cross_turn_cache_safety.py tests/test_cache_control_move_bust.py tests/test_proxy_byte_faithful_forwarding.py tests/test_proxy_anthropic_cache_stability.py tests/test_anthropic_pre_upstream_backpressure.py -q python -m pytest tests/test_proxy_openai_cache_stability.py -q python -m pytest tests/test_issue_2671_block_growth_cache.py::test_pure_append_replays_forwarded_blocks_and_advances_breakpoint -q 153 passed across focused invocations, exit code 0 optimize_off turn2_message_count=3 marker_count=1 outbound_compact_utf8_bytes=2293 client_compact_utf8_bytes=2293 optimize_on turn2_message_count=3 client_message_count=3 marker_count=0 outbound_compact_utf8_bytes=171 client_compact_utf8_bytes=182 python -m ruff check headroom/proxy/handlers/anthropic.py headroom/cache/prefix_tracker.py tests/test_proxy/test_anthropic_no_optimize_history_passthrough.py tests/test_cache_prefix_overlay.py tests/test_cache_control_move_bust.py tests/test_issue_2671_block_growth_cache.py tests/test_proxy_openai_cache_stability.py All checks passed!, exit code 0 python -m ruff format headroom/proxy/handlers/anthropic.py headroom/cache/prefix_tracker.py tests/test_proxy/test_anthropic_no_optimize_history_passthrough.py tests/test_cache_prefix_overlay.py tests/test_cache_control_move_bust.py tests/test_issue_2671_block_growth_cache.py tests/test_proxy_openai_cache_stability.py --check 7 files already formatted, exit code 0 git diff --check clean, exit code 0 ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12 via `uv`, real Headroom proxy app with a local stub Anthropic upstream - Exact command / steps: send an actual optimize-on first request through the in-process proxy with a deterministic production-pipeline seam, then send a decision-false bypass turn containing a large Claude-shaped `tool_result` with moved `cache_control`; separately send an aligned optimize-on turn with a new suffix - Observed result: the exact base checkout fails with `AssertionError: assert 'compressed-tool-result' == 'large-tool-result-marker ...'`; the guarded path passes with the client marker present once and outbound compact JSON no larger than the client body. The optimize-on preservation run records `optimize_on turn2_message_count=3 client_message_count=3 marker_count=0 outbound_compact_utf8_bytes=171 client_compact_utf8_bytes=182`, proving the actual compressed prefix is outbound before the new suffix without turn-2 growth. - Not tested: live Claude Code session against api.anthropic.com on this host ## Runtime Rollout Safety - Rollout-managed feature(s): none. - Minimum rollout channel: N/A. - Stable/default behavior changed: cached-prefix replay now follows the existing compression and backpressure decision and rejects misaligned or inflating candidates. - Kill switch / disable path: no new switch; the existing optimize and bypass controls remain available. - Unsafe override required: none. - Qualification impact: none. - Rollback path: revert the implementation commit. ## 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 - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] 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 ## Additional Notes `CHANGELOG.md` is not modified because Headroom's release automation generates it from conventional commits. This change does not add a context-limit guard or alter compression, streaming tracker provenance, outbound-body selection, OpenAI behavior, or provider limits. Local tests prove request-body ownership and replay bounds. The reporter's live Claude Code completion and Anthropic token acceptance remain external to this local proof. |
||
|
Tejas Chopra
|
c502087db7 |
fix(ccr): only buffer a stream when a marker is actually redeemable (#3092)
## Description Closes #3071 `headroom_retrieve` is injected once and kept resident for the session so the tools array stays byte-stable and the prompt cache survives. The buffered-CCR path keyed on that tool merely being **present**, so once a session went sticky, *every* later streaming turn was silently converted to `stream: false`, buffered whole, and resynthesized as SSE: ``` CCR: stream:true request has headroom_retrieve available; using buffered stream:false upstream request ``` Buffering leaves time-to-last-byte roughly unchanged but makes **time-to-first-byte the entire generation**. The reporter measured 8s average and up to 100s across 234 requests in one day — turns that would have streamed a first token in ~1s instead delivered nothing until done. Retrieval can only expand a `<<ccr:...>>` marker present in the outgoing body, so a turn carrying none cannot benefit from the buffered path at all. Gate on that instead of on the tool. This is also the root cause #3082 traced independently from the OpenCode side — its plugin registers `headroom_retrieve` unconditionally, so *every* turn buffered and neither `--no-ccr` nor `HEADROOM_NO_CCR` stopped it. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - New `_outgoing_body_has_redeemable_marker()` scans the body **about to go on the wire** and verifies ownership against the compression store — the same `exists()` check the retrieve endpoint performs, so a same-shaped marker from another context tool is not adopted (#2836). Unexpected shapes answer `True`, keeping the long-standing behavior. - The buffered-stream decision site gates on it, and logs at INFO when it skips buffering. - The correctness detail worth reviewing: the check reads `body`, **not** the earlier `scan_for_markers(optimized_messages)` result. `optimized_messages` is reassigned five times after that scan (memory hooks, pre-send extensions, tool-search repair, CCR repair), so reusing it would have been stale. - Two existing test files encoded the very coupling this removes and had to be repaired — see Testing. Scope: this narrows *when* buffering happens; it does not make buffered turns stream. A turn that genuinely carries a marker still loses incremental delivery — restoring streaming there means wiring `StreamingCCRHandler`, which is #3069's scope. It does not fix #3088 either, whose requests do carry markers. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] New tests added for new functionality - [x] Manual testing performed Two existing files built a request with `headroom_retrieve` and **no** marker, relying on the tool alone to trigger buffering: - `tests/test_proxy/test_anthropic_streaming_ccr_retrieve.py` (11 tests) fell through to the live streaming path, where only `_retry_request` is stubbed — so the requests reached the network and the file **hung indefinitely** rather than failing. Seeded real markers; now passes in ~5s. - `tests/test_proxy_response_cache_replay.py::test_buffered_ccr_turn_does_not_write_the_response_cache` asserts its own premise (*"the conversion really happened — otherwise this test proves nothing"*), so it failed loudly instead of passing vacuously. Seeded a marker. New `test_buffering_is_gated_on_a_redeemable_marker` pins all three directions: owned marker → buffered, no marker → streaming, foreign marker → streaming. ### Test Output ```text $ pytest tests/test_ccr_buffered_stream_signed_thinking.py -q 8 passed, 1 warning in 4.45s $ pytest tests/test_proxy/test_anthropic_streaming_ccr_retrieve.py -q 11 passed, 1 warning in 4.94s # was: hung indefinitely $ pytest tests/test_proxy_response_cache_replay.py -q 9 passed, 1 warning in 1.69s $ pytest tests/ -q 3 failed, 11151 passed, 581 skipped in 398.28s (0:06:38) Same 3 failures as a clean-main baseline run on this machine: tests/test_graceful_shutdown.py::test_run_server_installs_cancelled_error_filter tests/test_learn/test_integration.py::TestCodexIntegration::test_full_pipeline tests/test_release_workflows.py::test_no_native_tls_in_wheel_build_tree $ ruff check . && ruff format --check . All checks passed! ``` ## Real Behavior Proof - Environment: this branch driven through the real FastAPI app with the outbound HTTP client captured; macOS arm64, Python 3.12. - Exact command / steps: posted a `stream: true` `/v1/messages` request carrying a resident `headroom_retrieve` tool in three variants — no marker, a marker seeded into the compression store, and a correctly-shaped marker the store does not own — recording whether `_retry_request` saw a `stream: false` body. - Observed result: no marker → **streams**, `_retry_request` never sees a flipped body; owned marker → **buffers**, exactly as before; foreign marker → streams, honoring #2836 rather than adopting another tool's hash. - Not tested: the latency improvement against live client traffic. The mechanism is verified (the buffered conversion no longer occurs), but the reported 8s → ~1s TTFB needs the reporter's traffic to confirm. ## Runtime Rollout Safety - Rollout-managed feature(s): none — this narrows an existing code path and is not behind a rollout channel. - Minimum rollout channel: n/a (ships to stable with the fix). - Stable/default behavior changed: yes. A streaming turn whose body carries no redeemable marker now stays streaming instead of being buffered. Turns carrying a marker are unchanged. - Kill switch / disable path: no new switch. Existing CCR controls still apply — disabling the CCR response handler bypasses this decision site entirely, and the helper fails open (returns `True`, i.e. the old behavior) on any unexpected message shape. - Unsafe override required: no. - Qualification impact: none — no qualification-gated surface is touched. - Rollback path: revert this commit. Note it also carries two test repairs; reverting the production change alone would leave those tests passing but vacuous. ## 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 - [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` ## Additional Notes Documentation update is marked N/A: no user-facing flag or endpoint changes. Type checking (`mypy headroom`) was not run separately; `ruff` is the gate this repo's CI enforces. Related: #2836 (marker ownership), #3069 (streaming CCR handler), #3082, #3088. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Tejas Chopra
|
a29d2015e5 |
fix(proxy): restore the buffered-CCR heartbeat behind a grace window (#3091)
## Description Closes #3079 #2997 removed the buffered-CCR keepalive preamble from both provider handlers. That preamble was added by #2479 to close #2465, so `main` is back to the condition #2465 described while #2465 stays closed. Confirmed against the tags: ``` v0.35.0 (released): keepalive_deadline = loop.time() + 1.0 + b'event: ping...' main (-> 0.36.0): neither ``` Since #2997 is queued in #3067, 0.36.0 would ship this. The justification left in the code does not hold. It reads *"clients budget minutes for a turn (Claude Code sends `x-stainless-timeout: 600`), so waiting is free"* — but `x-stainless-timeout` is the **total request** budget, and #2465 was about the **stream idle** watchdog, a separate timer. Total-budget headroom says nothing about idle-budget headroom, and not every client sends 600. The reporter's buffered turns routinely run 15-25s, all of it silent. The status-ordering half of #2997 is correct and is kept. What was wrong was treating the two properties as a trade. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - New `headroom/proxy/buffered_ccr_response.py` holding one implementation of the buffered-CCR ASGI wrapper. Both handlers previously carried ~100 duplicated lines each, which is how the OpenAI twin drifted from the Anthropic one; now only the error wire format differs. - A buffered turn holds out for `buffered_ccr_grace_seconds` before committing. Inside the window nothing is sent and the result is relayed untouched, so a fast 4xx — or a 429/529 that resolves once `_retry_request` has honored `Retry-After` — keeps its real status and headers. That is #2997's property. - Past the window the response is committed as SSE and a heartbeat starts, so a first byte always precedes the client's idle watchdog. That is #2479's property. - A failure landing after the commit can no longer carry an HTTP status, so it is translated into the provider's own **typed** SSE error (`rate_limit_error`, `overloaded_error`, ...) carrying the upstream's own message where there is one, rather than a generic `api_error`. **This is the part worth reviewing.** #2997 was right that early commits broke client backoff — but that was a consequence of degrading every post-commit failure to a bare `api_error`, not of committing itself. - New `buffered_ccr_grace_seconds` on `ProxyConfig`, default 5s, env `HEADROOM_BUFFERED_CCR_GRACE_SECONDS`. Setting it to `0` restores current `main` behavior exactly. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] New tests added for new functionality - [x] Manual testing performed **#2997's own tests pass unchanged.** `test_buffered_ccr_preserves_late_failure_status_and_headers` and `test_buffered_ccr_withholds_output_until_delayed_upstream_resolves` resolve at 1.1s, comfortably inside the 5s window, so everything #2997 bought for the cases it tested is intact. New `tests/test_buffered_ccr_grace_window.py` pins both constraints @JerrettDavis asked for on #2959 — a late failure keeping its real status and headers, and a slow success producing a first byte before the ceiling — plus the typed-error mapping, the zero-grace escape hatch, and the OpenAI wire format. ### Test Output ```text $ pytest tests/test_buffered_ccr_grace_window.py -q 9 passed in 2.01s $ pytest tests/test_proxy/test_anthropic_streaming_ccr_retrieve.py tests/test_ccr_buffered_stream_signed_thinking.py -q 16 passed, 1 warning in 8.19s $ pytest tests/ -q 3 failed, 11157 passed, 581 skipped in 334.97s (0:05:34) Same 3 failures as a clean-main baseline run on this machine: tests/test_graceful_shutdown.py::test_run_server_installs_cancelled_error_filter tests/test_learn/test_integration.py::TestCodexIntegration::test_full_pipeline tests/test_release_workflows.py::test_no_native_tls_in_wheel_build_tree (missing local `cargo` toolchain; a stale tool-name fixture; a logging test that loses to global-state pollution in a full run — all present on main.) $ ruff check . && ruff format --check . All checks passed! ``` ## Real Behavior Proof - Environment: this branch, the wrapper driven directly over ASGI with a stubbed buffered operation standing in for upstream; macOS arm64, Python 3.12. - Exact command / steps: drove three scenarios — a 429 resolving at 0.05s with a 5s window; a success released only after the window with a 0.1s window; a 429 resolving at 0.3s with a 0.05s window — recording every ASGI message sent. - Observed result: (1) client receives **HTTP 429** with `retry-after: 30` and zero bytes beforehand; (2) first byte (`200 text/event-stream` + ping) arrives **before** the upstream resolves, body follows intact; (3) committed 200, then `event: error` typed `rate_limit_error` carrying the upstream's own message rather than a generic one. - Not tested: a live client idle-timeout against a real 15-25s turn. #3079 notes this depends on whether the client's idle timer starts at request send or at first response byte — the grace window makes Headroom correct under either reading, but confirming the original symptom is gone needs the reporter's fleet. ## Runtime Rollout Safety - Rollout-managed feature(s): none — this is a fix to an existing code path, not behind a rollout channel. - Minimum rollout channel: n/a (ships to stable with the fix). - Stable/default behavior changed: yes. A buffered-CCR turn slower than 5s now emits SSE headers plus keepalives instead of staying silent. Turns resolving under 5s are byte-identical to current `main`. - Kill switch / disable path: `HEADROOM_BUFFERED_CCR_GRACE_SECONDS=0` restores current `main` behavior exactly (never commit early, no heartbeat). Covered by `test_a_zero_grace_window_never_commits_early`. - Unsafe override required: no. - Qualification impact: none — no qualification-gated surface is touched. - Rollback path: revert this commit, or set the env var to `0` without a redeploy of code. ## 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 - [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` ## Additional Notes Documentation update is marked N/A: the new env var is documented in the module docstring and the `ProxyConfig` field comment, matching how `HEADROOM_ANTHROPIC_BUFFERED_REQUEST_TIMEOUT_SECONDS` is handled. Type checking (`mypy headroom`) was not run separately; `ruff` is the gate this repo's CI enforces. Related: #2465, #2479, #2959, #2968, #2997, #3067. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Tejas Chopra
|
204e751d2f |
fix(copilot): route VS Code inline completions to Copilot, not OpenAI (#3077)
## Description Fixes #3076. When `github.copilot.advanced.debug.overrideProxyUrl` points at Headroom, the VS Code Copilot extension sends its inline ("ghost text") completions to `/v1/engines/<engine>/completions`. No route matches that path, so it falls into the catch-all passthrough — and `select_passthrough_base_url()` resolves an upstream from the **auth headers alone**, never looking at the path. Copilot sends none of the headers the earlier branches key on, so the request reached the final line (default to OpenAI) and Headroom forwarded editor keystrokes to: ``` https://api.openai.com/v1/engines/gpt-41-copilot/completions ``` Wrong under every configuration — OpenAI removed the Engines API years ago — and blocked outright on corporate networks that permit GitHub Copilot but not OpenAI, which is how it was reported. Inline completions stopped working for every user behind such a policy. The Copilot **CLI** was unaffected: it speaks the CAPI shape (`/chat/completions`), which already resolved correctly. That is the exact asymmetry in the report. ## Type of Change - [x] Bug fix ## Changes Made **Routing.** `select_passthrough_base_url()` now takes the request path and sends this one path to Copilot. The shape identifies Copilot on its own, so the redirect is unambiguous. It is scoped to the OpenAI fall-through — the branch that is wrong here — because every other branch reflects an upstream the caller chose with its own auth headers. **The destination is not hardcoded.** GitHub's token exchange advertises the completions host in `endpoints.proxy`, alongside the `endpoints.api` chat host Headroom already reads. It is now recorded at the single chokepoint every exchange passes through, and preferred. Resolution order: 1. `GITHUB_COPILOT_PROXY_URL` — operator override 2. `endpoints.proxy` from the last token exchange — GitHub's own answer 3. The Copilot API URL No I/O on the request path, and GHE deployments keep their host. This matters: it means the destination is not an assumption about which host serves completions, and if it is wrong for a given network it is an env var rather than a release. **Path preservation.** `build_copilot_upstream_url()` strips `/v1` when the upstream is a Copilot host, because Copilot serves its OpenAI-compatible surface unprefixed (`/chat/completions`, `/models`). But the extension built `/v1/engines/<engine>/completions` itself, so that path is already exactly what Copilot serves — stripping the prefix rewrites a working request into a 404. Preserved, the same carve-out `/v1/messages` needed in #2409. The rule: strip only for clients speaking generic-OpenAI at Copilot, never for Copilot's own paths. ## Testing - [x] New suite: `tests/test_copilot_vscode_completions_routing.py` (30 tests) — path recognition and its near-misses, upstream selection, the `endpoints.proxy` resolution order, and URL construction in both directions - [x] 286 passed across the Copilot, provider-routing and passthrough suites - [x] Ruff check and format pass ### Real Behavior Proof Environment: this branch, a `POST /v1/engines/gpt-41-copilot/completions` driven through the real app with `OPENAI_API_URL=https://api.openai.com` and the outbound HTTP client captured. ``` BEFORE (main): https://api.openai.com/v1/engines/gpt-41-copilot/completions AFTER (this): https://api.githubcopilot.com/v1/engines/gpt-41-copilot/completions ``` The "before" line reproduces the reported URL exactly. **Not tested:** a live VS Code Copilot session confirming GitHub accepts the forwarded request. That needs a real Copilot account and editor. If the completions host turns out to differ, the `endpoints.proxy` lookup or `GITHUB_COPILOT_PROXY_URL` covers it without a code change. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Tejas Chopra
|
6d2254dfb5 |
fix(anthropic): honor the [1m] 1M-context tier, and price it correctly (#3073)
Two coupled defects on Anthropic's 1M-context tier: Headroom **under-budgeted** those sessions and **under-priced** them by ~2x. The second gets worse once the first is fixed, so they ship together. --- # Part 1 — `[1m]` was lost before the context budget was sized `sanitize_anthropic_model_id()` strips a trailing `[1m]`, which is correct for the wire — upstream Anthropic rejects the suffix, and #2027 added the strip for exactly that reason. But `[1m]` is not only an ANSI artifact. Claude Code appends it to a model id to request the **1M context tier**, and only sends the `context-1m` beta header when it is present (#1158 — what `headroom wrap claude --1m` sets up). `get_context_limit()` sanitized *before* resolving, so the tier was gone by lookup time: ```python provider.get_context_limit("claude-sonnet-4-5[1m]") # 200_000 ← real window is 1M ``` The request still reached Anthropic correctly and still got a 1M window — the beta header goes through untouched. What broke is our **budget**: Headroom sized a 1M session at 200K and began compacting at a fifth of the available room. Models whose base is already 1M (`claude-opus-5`, `claude-sonnet-5`) resolved to 1M either way, which is why this went unnoticed. It bites the Sonnet 4 / 4.5 family — the models `[1m]` exists for. **Fix:** read the tier off the id *before* sanitizing; raise the resolved limit to at least 1M. `max()` rather than assignment, so a base wider than 1M keeps its own window. Detection is deliberately narrower than the sanitizer — only a literal `[1m]`; `[0m]`, `[1;32m]` and real `ESC[` sequences still strip without promoting. | model id | wire id (unchanged) | limit before | limit after | |---|---|---|---| | `claude-sonnet-4-5` | `claude-sonnet-4-5` | 200K | 200K | | `claude-sonnet-4-5[1m]` | `claude-sonnet-4-5` | **200K** | **1M** | | `claude-opus-5[1m]` | `claude-opus-5` | 1M | 1M | | `claude-sonnet-4-5[0m]` | `claude-sonnet-4-5` | 200K | 200K | | `ESC[1m claude-sonnet-4-5 ESC[0m` | `claude-sonnet-4-5` | 200K | 200K | The wire id is unchanged in every case, so #2027 holds — guarded by a regression test. --- # Part 2 — the pricing that reports those sessions was wrong ### 2a. The LiteLLM cost path was dead in every provider `litellm.completion_cost()` no longer accepts `prompt_tokens` / `completion_tokens`. Every call raised `TypeError`: ``` TypeError: completion_cost() got an unexpected keyword argument 'prompt_tokens' ``` All five providers — `anthropic`, `openai`, `google`, `cohere`, `litellm` — caught it with a bare `except` and silently fell through to their hand-maintained tables. The "up-to-date pricing from LiteLLM" the docstrings promise **has not run at all**. Anthropic additionally passed `input_tokens - cached_tokens`, the wrong convention (LiteLLM expects the cache-inclusive total), which would also have suppressed the long-context threshold even had the call worked. Replaced with `litellm.cost_per_token()` behind one shared helper, `pricing.litellm_pricing.estimate_cost_from_tokens()`, which reuses the existing gateway-alias candidate chain and returns `None` (not an exception) when LiteLLM can't price a model. ### 2b. Neither path applied Anthropic's long-context premium On the Sonnet 4 / 4.5 family a prompt over 200K re-prices the **whole** request — input 2×, output 1.5×, cache 2× — not just the tokens past the threshold. Rates confirmed from LiteLLM's `*_above_200k_tokens` fields. | request (`claude-sonnet-4-5`) | reported before | true | error | |---|---|---|---| | 100K in / 5K out | $0.3750 | $0.3750 | — | | 300K in / 5K out | $0.9750 | **$1.9125** | −49% | | 300K in (150K cached) / 5K out | $0.5700 | **$1.1025** | −48% | LiteLLM applies this itself once the call works. The manual fallback needed `_apply_long_context_premium()` — the LiteLLM dependency is gated `python_version < '3.14'`, so on 3.14 the fallback is the *only* path. **Both paths now agree to four decimal places on every case under test.** --- ## What I checked and did *not* change The fork report that prompted this claimed the Anthropic tables were materially stale ("Opus 4.x priced wrong"). **That does not hold.** I audited every entry against LiteLLM's vendored table: - **Anthropic** — every model LiteLLM knows matches exactly, Opus 4.x included. - **OpenAI** — all 17 entries match; the two that don't resolve are retired models. The defect was the mechanism, not the numbers, so the rate cards are untouched. One thing the repaired path fixes for free: OpenAI's cached-input discount is **50% on gpt-4o, 75% on gpt-4.1, 90% on gpt-5**, but the manual path applies a flat 50% estimate. With LiteLLM live, real per-model rates are used. The flat estimate remains only as the offline fallback. ## Scope **No Rust change needed.** `crates/headroom-proxy/src/compression/model_limits.rs` resolves context windows but has **no in-tree callers**; the Rust `[1m]` handling is wire-body sanitization only, correct as-is, and its integration tests assert behavior this PR does not touch. **Judgment call worth a reviewer's eye:** the `[1m]` marker is honored for *any* model, including ones with no 1M tier (`claude-haiku-4-5-20251001[1m]` → 1M). Gating on an allowlist would be more precise but reintroduces a hand-maintained table that rots — the failure mode `model_limits.rs` already documents against. Since `[1m]` is set by our own wrapper and Claude Code's opt-in, honoring it seemed the better default. Happy to tighten. ## Tests - `TestContext1MSuffix` — detection, the 200K→1M promotion, the `max()` floor, ANSI non-promotion, and the wire-id guard for #2027. - `TestLongContextPricing` — the premium on both paths (parametrized), threshold boundary (200,000 vs 200,001), an untiered model charged no premium, and the two halves meeting: a `[1m]` request gets both the 1M window and the premium rate. - `TestLiteLLMCostHelper` — unknown model returns `None`, a known model prices correctly, and `input_tokens` is cache-inclusive. Two existing tests were updated, both pinned to the broken behavior: - `test_estimate_cost_basic` probed a "per 1M" rate by sending exactly 1M tokens, which now crosses the 200K threshold. Re-probed at 100K. (Worth knowing: `claude-3-5-sonnet-20241022` is retired and no longer in LiteLLM, so the alias chain resolves it to `claude-sonnet-4-20250514` and it inherits that model's tier. Harmless — a 200K-window model can't exceed 200K in reality — but it explains the number.) - `test_litellm_provider_info_and_cost_fallbacks` monkeypatched `litellm.completion_cost`; repointed at the new helper seam. ``` ruff check / ruff format / mypy — clean across all six changed source files ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
JD Davis
|
b3f443636d |
fix(proxy): align signed-thinking wire accounting (#3015)
## Description Signed-thinking histories force byte-faithful passthrough because re-serializing signed Anthropic blocks can invalidate their signatures. Headroom correctly forwarded the original client bytes, but continued reporting mutations, transforms, savings, response headers, and prefix state from a different body that never reached the provider. Separately, the final Anthropic guard hoisted every `role: system` message into the top-level prompt, including valid mid-conversation system sections, changing their semantics and destroying the cached prefix if that mutation ever shipped. This coupled fix makes downstream accounting use the actual wire body whenever the signed-thinking lock discards edits, and narrows system relocation to the current Anthropic model and placement contract. Closes #2990 Closes #2991 ## 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 - [x] Performance improvement - [x] Code refactoring (no functional changes) ## Changes Made - Detects signed thinking in the original request as well as the mutated body, so a transform cannot remove the block and accidentally bypass the byte lock. - Keeps the original-body signature probe best-effort under malformed, recursive, and `MemoryError` conditions. - Carries discarded mutation reasons through the streaming forwarder and emits the existing structured warning on HTTP streaming paths too. - When signed passthrough wins, resets message savings, tool-schema savings, attribution ledgers, transform labels, response headers, and prefix tracking to the original client wire body. - Adds bounded public diagnostic tags naming/counting discarded mutation reasons without exposing body content. - Preserves valid mid-conversation system sections on currently supported Claude models and official Anthropic, Bedrock, and parsed `*.googleapis.com` routes; hostname-boundary validation rejects lookalike and userinfo URLs. - Preserves consecutive system sections and enforces documented predecessor/successor placement rules. - Continues relocating initial, invalidly placed, unsupported-model, and conservative third-party-gateway system messages to avoid upstream 400s. - Includes current `main`, including #2996, #2997, #2971, #3009, #3012, and the MCP dependency cap. ## 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 ### Test Output ```text uv run pytest -q <wire/cache/savings/system focused suite> 379 passed uv run pytest -q tests/test_proxy/test_anthropic_recount_and_reparse_safety.py tests/test_proxy_byte_faithful_forwarding.py tests/test_proxy_handler_helpers.py 99 passed pytest tests scripts/tests --splits 4 --group N --tb=short -q All four CI-shaped fresh-process groups passed locally after correcting the MemoryError regression; each completed in roughly 75-83 seconds. Post-CodeQL correction: 91 focused tests passed; all four exact-head CI-shaped shards passed in roughly 82-95 seconds. uv run ruff format --check . 1411 files already formatted uv run ruff check . All checks passed uv run mypy headroom Success: no issues found in 520 source files ``` ## Real Behavior Proof - Environment: macOS arm64, Python 3.13, branch rebased onto current `main`. - Exact command / steps: sent a signed-thinking request whose tool schema is measurably compacted inside the handler, captured the exact upstream bytes, wrapped the real outcome funnel, and inspected response headers, aggregate metrics, attribution tags, transforms, and prefix-tracker state. Exercised valid, consecutive, invalid, initial, supported-model, and unsupported-model system placements. - Observed result: upstream bytes remain byte-identical to the client; discarded edits contribute zero tokens, zero tool savings, no transform header, and no attribution while the prefix tracker stores the actual wire messages. Valid mid-conversation system sections remain in place; only out-of-contract sections relocate. - Not tested: live paid Anthropic traffic with production credentials. The placement/model contract was verified against the current official documentation and wire behavior is covered with a byte-capturing transport. ## Runtime Rollout Safety - Rollout-managed feature(s): signed-thinking wire-truth accounting and Anthropic mid-conversation system preservation. - Minimum rollout channel: normal patch release after exact-head CI is entirely green. - Stable/default behavior changed: discarded mutations no longer inflate savings; supported valid system sections are no longer hoisted into the top-level prompt. - Kill switch / disable path: no unsafe runtime override; human revert restores the previous conservative relocation/accounting behavior. - Unsafe override required: none. - Qualification impact: all Python shards, byte-forwarding, cache-prefix, outcome/savings, signed-thinking, Anthropic handler, static, Docker, and security checks must remain green. - Rollback path: fix forward through a human-reviewed corrective PR; no persisted data or configuration migration is involved. ## 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 - [x] I have made corresponding changes to the documentation — inline wire-contract documentation; no separate guide is required - [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) ## Screenshots (if applicable) Not applicable; proxy wire behavior and accounting only. ## Additional Notes Human review only. No merge or auto-merge is configured. Current provider contract reference: https://platform.claude.com/docs/en/build-with-claude/mid-conversation-system-messages |
||
|
Tejas Chopra
|
942af56f11 |
fix(ccr): re-inject headroom_retrieve when history references it on the sessionless path
Changelog-only correction. The work shipped in commit |
||
|
Tejas Chopra
|
1b0b0b89a4 |
feat(proxy): unify savings attribution across stats, perf, metrics, and dashboard
Changelog-only correction. The work shipped in commit |
||
|
Tejas Chopra
|
cbb950a441 |
ci(governance): require a Conventional Commit PR title (#3063)
## Description The repo squash-merges, so the PR title — not the commits inside the PR — becomes the commit subject on `main`. Nothing validated it. `commitlint` (`ci.yml:429`) lints a PR's *commits* and therefore cannot catch this by construction: a PR with clean conventional commits and a prose title passes CI and then lands a prose subject on `main`. That is how `31452426` landed: ``` Unify savings attribution across stats, perf, metrics, and dashboard (#2976) ``` release-please cannot parse it — `unexpected token ' ' at 1:6`, because `Unify` is five characters and position six is a space where the parser needs `(`, `!` or `:`. The change is silently dropped from the changelog. ## 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 ## Changes Made - Added `COMMIT_TYPES` and `TITLE_RE` to `scripts/pr-governance.py`, matching `.commitlintrc.json`'s `type-enum`. - Added a title check to `validate_pull_request`, reported through the existing governance comment. - Added a test asserting `COMMIT_TYPES` equals `.commitlintrc.json`'s `type-enum`, so the two gates cannot drift apart. - Gave the `_event` test helper the `title` field a real `pull_request` payload always carries. ## Testing - [x] Unit tests pass - [x] Linting passes (ruff check + format) - [ ] Type checking passes — N/A (script + test only) - [x] New tests added for new functionality ### Test Output ```text $ .venv/bin/python -m pytest scripts/tests/test_pr_governance.py -q 12 passed in 0.02s ``` Against the parent commit: ```text FAILED test_validate_pull_request_rejects_non_conventional_title FAILED test_validate_pull_request_rejects_empty_and_typeless_titles FAILED test_commit_types_match_commitlint_config 3 failed, 9 passed ``` ## Real Behavior Proof - Environment: macOS 15 (darwin 25.4.0), Python 3.12.13, `scripts/pr-governance.py` loaded directly. - Exact command / steps: ran `TITLE_RE` against the titles of **all 117 pull requests opened in this repository between 2026-08-10 and 2026-08-16**, pulled with `gh pr list --json title`. - Observed result: exactly one title is flagged — `#2976`, `Unify savings attribution across stats, perf, metrics, and dashboard`, the one that jammed the release. Zero false positives across the other 116, including every Dependabot `deps: bump ...` title, `chore: release main`, and scoped forms like `fix(proxy/anthropic): ...`. - Not tested: the check running inside a live `pull_request_target` event on a GitHub runner. ## Runtime Rollout Safety - Rollout-managed feature(s): none. - Minimum rollout channel: N/A. - Stable/default behavior changed: yes — a PR with a non-conventional title now gets the `status: needs author action` label and a governance comment. - Kill switch / disable path: revert; the check is not independently configurable. - Unsafe override required: none. - Qualification impact: none. - Rollback path: revert this commit. ## 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 - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] New and existing unit tests pass locally with my changes ## Additional Notes The check lives in `pr-governance.py` rather than `ci.yml` for two reasons: 1. `ci.yml`'s `pull_request` trigger has no `edited` type, so a corrected title would never be re-checked. 2. Its `paths-ignore` skips docs-only PRs, which still squash-merge a subject onto `main`. `pr-health.yml` already triggers on `edited` and reports through the same governance comment the author is reading anyway. Bot PRs keep their existing exemption — Dependabot and release-please titles are already conventional, and the early return for `is_bot_pr` is untouched. Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> |
||
|
Tejas Chopra
|
ac8646aa3c |
fix(ci): scope the release credential and stop persisting it to disk (#3062)
## Description `RELEASE_PLEASE_TOKEN` is currently a maintainer's personal PAT. It bypasses branch and tag protection on `main` (`release-please.yml` says so in its own comment), and forging a tag with it fires `release.yml` and `docker.yml` on `release: published`, which publish to PyPI, npm and GHCR. If it is a classic token with `repo` scope it is also valid against every other repository that account can reach. `release-metadata-sync.yml` made that credential readable on the runner. `actions/checkout` defaults to `persist-credentials: true`, writing the token into `.git/config`, and the very next step runs `scripts/version-sync.py` **from the checked-out branch**. The trigger is a push to the glob `release-please--branches--**`, which is not a protected namespace, so a principal with push access could land a modified `version-sync.py` and read it. Closes #2955. ## 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 ## Changes Made - Both release workflows now prefer a GitHub App installation token — scoped to this repository, expiring in an hour — over the PAT, via `actions/create-github-app-token@v3`. - The minting step is gated on `vars.RELEASE_APP_ID` and marked `continue-on-error`, so an unconfigured app falls through to the existing `PAT -> GITHUB_TOKEN` chain and nothing breaks today. - `release-metadata-sync.yml`'s checkout no longer persists credentials, and no longer receives a token at all. - The final push supplies the credential through the step's own `env` and an explicit remote URL, so it is never on disk while branch-supplied code runs. ## Testing - [x] Unit tests pass - [x] Linting passes (ruff check + format on the test file) - [ ] Type checking passes — N/A (YAML + test only) - [x] New tests added for new functionality ### Test Output ```text $ .venv/bin/python -m pytest tests/test_release_workflows.py -q 1 failed, 44 passed, 1 skipped in 0.23s ``` The single failure is `test_no_native_tls_in_wheel_build_tree`, which shells out to `cargo`. It reproduces identically on unmodified `main` on this machine (no Rust toolchain installed) and is unrelated to this change. New tests only: ```text $ .venv/bin/python -m pytest tests/test_release_workflows.py -q -k "persist_credentials or scoped_app_token" 3 passed, 46 deselected in 0.18s ``` Against the parent commit: ```text FAILED test_metadata_sync_does_not_persist_credentials_for_branch_supplied_code FAILED test_release_workflows_prefer_scoped_app_token[release-please.yml-release-please] FAILED test_release_workflows_prefer_scoped_app_token[release-metadata-sync.yml-sync] 3 failed, 46 deselected ``` ## Real Behavior Proof - Environment: macOS 15 (darwin 25.4.0), Python 3.12.13; workflows parsed with PyYAML, not executed on a runner. - Exact command / steps: parse both workflow files and assert (a) every `actions/checkout` step sets `persist-credentials: false` and receives no `token`, (b) exactly one gated `create-github-app-token` step exists per workflow, and (c) every credential consumer places `steps.app-token.outputs.token` ahead of `secrets.RELEASE_PLEASE_TOKEN` in its fallback chain. - Observed result: all three assertions pass on this branch and fail on the parent commit. Both files remain valid YAML. - **Not tested — important:** none of this has executed on a GitHub runner. I have not minted a real installation token, not confirmed the app-token step's `continue-on-error` fallback behaves as expected when `vars.RELEASE_APP_ID` is unset, and not performed a real push with the explicit-remote-URL form. The first live release run is the real test. ## Runtime Rollout Safety - Rollout-managed feature(s): none. - Minimum rollout channel: N/A. - Stable/default behavior changed: no, unless `vars.RELEASE_APP_ID` is set — without it both workflows resolve to exactly today's credential chain. - Kill switch / disable path: unset `vars.RELEASE_APP_ID` to fall back to the PAT. - Unsafe override required: none. - Qualification impact: none. - Rollback path: revert this commit. ## 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 - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] New and existing unit tests pass locally with my changes ## Additional Notes **This narrows blast radius; it does not make the trigger safe on its own.** For an `on: push` workflow GitHub reads the workflow file from the pushed ref, so a principal with push access can still edit this file on their branch. The durable fix is the scoped app token *plus revoking the personal PAT* — the revocation is a console action and is deliberately not in this commit. **Two repo settings are required to actually complete #2955**, and neither can land in git: ``` vars.RELEASE_APP_ID (repository variable) secrets.RELEASE_APP_PRIVATE_KEY (repository secret) ``` Until those exist this PR is a no-op on behavior and a defense-in-depth improvement on the `persist-credentials` path only. Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> |
||
|
Tejas Chopra
|
481e0b83d5 |
fix(docker): publish compose ports on loopback only (#3061)
## Description `docker compose up -d` published every service on `0.0.0.0`, and none of the three authenticates an inbound caller by default: | port | service | default auth | |---|---|---| | 8787 | proxy | `/v1/*` data plane open unless `HEADROOM_PROXY_TOKEN` is set | | 6333/6334 | Qdrant | **none at all** — holds embeddings derived from prompts | | 7474/7687 | Neo4j | `NEO4J_AUTH` falls back to `neo4j/devpassword`, published in this file | So the shipped default handed any peer on the surrounding network a relay through the proxy plus direct read/write on the vector and graph stores built from the operator's own prompt content. The proxy already warns about exactly this shape at `headroom/proxy/server.py:3289` — the compose file just never took its own advice. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [x] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Changes Made - Pinned all five published ports to `127.0.0.1`. - Documented in the file header how to expose the proxy deliberately, pairing the port override with `HEADROOM_PROXY_TOKEN` rather than leaving that implicit. - Added a commented `HEADROOM_PROXY_TOKEN` entry to the proxy service environment. - Added a regression test asserting every published port names a loopback host IP. ## Testing - [x] Unit tests pass - [x] Linting passes (ruff check + format) - [ ] Type checking passes — N/A (YAML + test only) - [x] New tests added for new functionality ### Test Output ```text $ .venv/bin/python -m pytest tests/test_docker_compose_persistence.py -q 3 passed in 0.14s $ docker compose -f docker-compose.yml config # validates headroom-proxy host_ip=127.0.0.1 published=8787 -> 8787 neo4j host_ip=127.0.0.1 published=7474 -> 7474 neo4j host_ip=127.0.0.1 published=7687 -> 7687 qdrant host_ip=127.0.0.1 published=6333 -> 6333 qdrant host_ip=127.0.0.1 published=6334 -> 6334 ``` Against the parent commit: ```text FAILED test_top_level_compose_publishes_only_to_loopback E AssertionError: headroom-proxy: port '8787:8787' publishes on all interfaces ``` ## Real Behavior Proof - Environment: macOS 15 (darwin 25.4.0), Docker Compose v2 available locally. - Exact command / steps: `docker compose -f docker-compose.yml config --format json` before and after, comparing the resolved `host_ip` on every published port. - Observed result: before, no port carried a `host_ip` (Docker binds `0.0.0.0`); after, all five resolve to `host_ip=127.0.0.1`. The compose file still validates. - Not tested: bringing the stack up and probing the ports from a second machine on the LAN — the assertion is made against Docker's own resolved configuration rather than a live two-host network. ## Runtime Rollout Safety - Rollout-managed feature(s): none. - Minimum rollout channel: N/A. - Stable/default behavior changed: yes — the compose stack is no longer reachable from other machines by default. - Kill switch / disable path: override `ports:` in a `docker-compose.override.yml`; the header documents this and pairs it with `HEADROOM_PROXY_TOKEN`. - Unsafe override required: none. - Qualification impact: none. - Rollback path: revert this commit. ## 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 - [x] I have made corresponding changes to the documentation (the compose header) - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] New and existing unit tests pass locally with my changes ## Additional Notes **This is a deliberate breaking change for one workflow**: anyone reaching the compose proxy from another machine will need to override `ports:`. That is exactly the configuration that was unsafe, so it should break loudly rather than silently. `http://localhost:8787` from the host is unchanged, the container still listens on `0.0.0.0` internally, and service-to-service traffic on the compose network is unaffected. Scope note: I fixed all three services rather than only the proxy. Closing 8787 while leaving an unauthenticated Qdrant and a default-password Neo4j published on `0.0.0.0` would not have improved the security posture. Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> |
||
|
Tejas Chopra
|
a6ab359a5d |
fix(proxy): guard feedback endpoints and add CSRF checks to loopback writes (#3060)
## Description
`#2927` brought eight telemetry/TOIN routes under `require_loopback`.
Two structurally identical siblings 60 lines above them were missed:
```
GET /v1/feedback
GET /v1/feedback/{tool_name}
```
Neither is an aggregate-counter endpoint. Their `common_queries` /
`queried_fields` keys are built verbatim from agent search text —
`event.query.lower()` at `headroom/cache/compression_feedback.py:311` —
and up to 100 queries are retained per tool, keyed by real tool name.
Under the shipped Docker default (`--host 0.0.0.0`) a LAN peer gets a
404 from `/v1/toin/patterns` and the query corpus from `/v1/feedback`.
Separately, five mutating loopback-only routes had no CSRF guard.
`require_loopback` cannot stop that attack: a remote page POSTing to a
known `127.0.0.1` URL with `Content-Type: text/plain` is a CORS *simple*
request, so there is no preflight, and the browser still sends the real
loopback `Host` header — both of the guard's gates pass. Only `Origin`
betrays the caller, and only `require_same_origin` inspects it. That
guard already existed at `headroom/proxy/loopback_guard.py:219` and was
applied solely to `/settings`.
Closes #2927 (completes it — the original eight routes were already
done).
## 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
## Changes Made
- Added `Depends(_require_loopback)` to `/v1/feedback` and
`/v1/feedback/{tool_name}`.
- Stripped `common_queries` / `queried_fields` from both response bodies
even on the guarded path, matching the whitelist discipline #2930
applied at `server.py:4909-4916`.
- Added `_feedback_stats_without_query_text()` so the scrub happens at
the HTTP boundary; `get_stats()` is unchanged and in-process compression
decisions are untouched.
- Added `Depends(_require_same_origin)` to `POST /stats/reset`,
`/cache/clear`, `/v1/retrieve`, `/v1/telemetry/import`,
`/admin/runtime-env`.
## Testing
- [x] Unit tests pass
- [x] Linting passes (ruff check + format)
- [ ] Type checking passes (`uv run mypy headroom`) — not run
- [x] New tests added for new functionality
### Test Output
```text
$ .venv/bin/python -m pytest tests/test_proxy_loopback_gating.py -q
99 passed, 1 warning in 4.18s
$ .venv/bin/python -m pytest tests/test_proxy_settings_endpoints.py tests/test_telemetry.py \
tests/test_proxy_cache_telemetry.py tests/test_proxy_telemetry_env.py tests/test_telemetry_context.py -q
101 passed, 1 warning in 3.67s
$ .venv/bin/python -m pytest tests/test_critical_fixes.py tests/test_compression_store.py \
tests/test_toin_full_integration.py tests/test_ccr_feedback.py tests/test_critical_gaps.py \
tests/test_proxy_ccr.py tests/test_proxy_dashboard_stats_cache.py -q
168 passed, 4 skipped, 3 warnings in 13.18s
$ .venv/bin/python -m ruff check headroom/proxy/server.py tests/test_proxy_loopback_gating.py
All checks passed!
```
Against the parent commit (`git stash` of `server.py` only), all 14 new
tests fail:
```text
FAILED test_non_loopback_caller_gets_404[get-/v1/feedback]
FAILED test_non_loopback_caller_gets_404[get-/v1/feedback/example]
FAILED test_cross_origin_post_rejected[/stats/reset]
FAILED test_cross_origin_post_rejected[/cache/clear]
FAILED test_cross_origin_post_rejected[/v1/retrieve]
FAILED test_cross_origin_post_rejected[/v1/telemetry/import]
FAILED test_cross_origin_post_rejected[/admin/runtime-env]
FAILED test_sandboxed_null_origin_post_rejected[...] (5 cases)
FAILED test_feedback_stats_exclude_agent_query_text
FAILED test_feedback_tool_detail_excludes_agent_query_text
14 failed, 85 passed
```
## Real Behavior Proof
- Environment: macOS 15 (darwin 25.4.0), Python 3.12.13, this branch,
FastAPI `TestClient` against the real `create_app` proxy.
- Exact command / steps: drive `/v1/feedback` with a feedback singleton
whose `common_queries` contains `"find the customer api key rotation
runbook"`, once from a non-loopback peer and once from a loopback peer;
POST each of the five mutating routes with `Origin:
https://attacker.example` and `Content-Type: text/plain`.
- Observed result: non-loopback callers now receive 404 where they
previously received 200 with the query corpus; on the loopback path the
response no longer contains `common_queries`, `queried_fields`, or the
substring `customer api key rotation`, while `retrieval_rate` still
resolves to `0.25`. All five cross-origin POSTs return 403; the same
requests with no `Origin`, or with `Origin: http://127.0.0.1`, are
unaffected.
- Not tested: a real browser issuing the cross-origin POST (the CORS
simple-request shape is reproduced at the header level, not in a
browser), and a live non-loopback deployment.
## Runtime Rollout Safety
- Rollout-managed feature(s): none.
- Minimum rollout channel: N/A.
- Stable/default behavior changed: yes — `/v1/feedback*` now 404 for
non-loopback callers and no longer return query text; five POST routes
reject cross-origin browser callers.
- Kill switch / disable path: none; these are security guards and are
deliberately not configurable.
- Unsafe override required: none.
- Qualification impact: none.
- Rollback path: revert this commit.
## 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
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective
- [x] New and existing unit tests pass locally with my changes
## Additional Notes
`/stats` also calls `feedback.get_stats()` (`server.py:3896`) but only
reads aggregate counters at `:4303-4311` and never emits query text —
verified, and the reason the scrub is applied at the HTTP boundary
rather than inside `get_stats()`.
The five POST routes are strictly loopback-gated, so the
trusted-dashboard wrapper `/settings` uses is unnecessary here; for a
loopback caller that wrapper falls through to the same raw guard. No
dashboard asset calls them, and the TypeScript SDK
(`sdk/typescript/src/client.ts:322,443`) sends no `Origin` header, which
the guard passes through unchanged.
Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local>
|
||
|
Tejas Chopra
|
96c25f5181 |
fix(cli): stop the macOS malloc re-exec replacing an embedder's process (#3064)
## Description **`main` cannot currently run its own test suite on macOS.** `pytest tests/` dies at roughly 2% with exit code 2 — no traceback, no summary, no failing test named. The pytest process is simply gone. Two independent defects, both landed today, both invisible to CI. ### 1. The macOS malloc re-exec replaces the calling process `headroom proxy` re-execs itself once on Darwin to apply two libmalloc knobs that libmalloc only reads before `main()` (#2820, PR #2879): ```python os.execv(sys.executable, [sys.executable, "-m", "headroom.cli", *sys.argv[1:]]) ``` That reconstruction is only faithful when the process really *is* the Headroom CLI. Ten-plus test files invoke the `proxy` command in-process through Click's `CliRunner`. There, `os.execv` replaces **pytest** with a Headroom process holding pytest's argv. Run with `-s`, the mechanism is visible: ``` tests/test_agent_savings.py Usage: python -m headroom.cli [OPTIONS] COMMAND [ARGS]... Error: No such command 'tests/test_agent_savings.py::test_proxy_cli_reads_agent_90_profile_env'. ``` Everything after the first such test — roughly 98% of the suite — never runs. The same hazard applies to any application embedding the CLI. **The documented kill switch does not help.** `tests/conftest.py:41` scrubs every `HEADROOM_*` variable for hermeticity, so `HEADROOM_MALLOC_TUNING` is deleted before the guard reads it. Only the private `_HEADROOM_MALLOC_TUNED` survives, because it starts with an underscore. **CI could not have caught this.** The tuning is Darwin-only, and while the repo *does* have macOS jobs (`macos-native-wrapper`, `wrap-native (macos-latest)`), neither runs the Python test suite — the `test` shards are `ubuntu-latest` only. So `sys.platform != "darwin"` returns first everywhere pytest actually runs. #2879 merged with 37 green checks. ### 2. A semantic merge conflict between two green PRs #3051 added `bind_scope(tags, request.scope)` at `gemini.py:325` and updated the three Gemini fakes it knew about. #3035 branched earlier and added a fourth `_FakeRequest` without `.scope`. Each was green against its own base; together they fail: ``` AttributeError: '_FakeRequest' object has no attribute 'scope' ``` Git merged both cleanly. Only running the suite on merged `main` surfaces it. ## 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 ## Changes Made - Added `_process_is_headroom_cli_entrypoint()`: the re-exec now verifies its own precondition — `argv[0]` must be the `headroom` console script or `headroom/cli/__main__.py`. - The embedded path returns **before** stamping `_HEADROOM_MALLOC_TUNED`, so a genuine CLI child inheriting the environment can still apply the tuning. - Gave the Gemini `_FakeRequest` the `.scope` every real Starlette `Request` carries. - `test_reexec_skips_when_operator_already_set_vars` now sets a realistic `argv[0]`, matching its sibling exec test. - New `tests/test_cli_proxy_malloc_reexec_guard.py` asserting the guard's logic on **every** platform, since no CI runner is macOS. ## Testing - [x] Unit tests pass - [x] Linting passes (ruff check + format) - [ ] Type checking passes (`uv run mypy headroom`) — not run - [x] New tests added for new functionality ### Test Output Before, on `main`: ```text $ .venv/bin/python -m pytest tests/ -q collected 11622 items / 8 skipped ... tests/test_agent_savings.py ............................ $ echo $? 2 ``` No summary line — the run does not end, it is replaced. After, on this branch: ```text $ .venv/bin/python -m pytest tests/ -q 3 failed, 11055 passed, 581 skipped, 6034 warnings in 303.69s (0:05:03) ``` All three remaining failures reproduce at `f9807fd6`, before today's merges, and are unrelated: | test | cause | |---|---| | `test_graceful_shutdown::test_run_server_installs_cancelled_error_filter` | full-suite ordering; passes in isolation (11 passed) | | `test_learn/test_integration::TestCodexIntegration::test_full_pipeline` | pre-existing | | `test_release_workflows::test_no_native_tls_in_wheel_build_tree` | requires `cargo`, absent on this host | ## Real Behavior Proof - Environment: macOS 15 (darwin 25.4.0), Python 3.12.13, arm64, real checkout of `main` at `ef7e07e0`. - Exact command / steps: bisected the crash to a single test, then to a single commit — `be5b26d8` (parent) exits 0, `6d87825f` (#2879) exits 2. Confirmed causation by temporarily replacing the `os.execv` line with `return`, which makes the test pass. Recovered the mechanism by running the crashing test with `-s`, which prints the Headroom CLI rejecting pytest's own argv. - Observed result: on `main` the suite cannot reach a summary; on this branch it completes with 11,055 passing. The two-file reproduction (`test_agent_savings.py` + `test_anthropic_beta_session_sticky.py`) goes from exit 2 to 62 passed. - Not tested: a real `headroom proxy` launch on macOS confirming libmalloc still receives the knobs after re-exec. The guard is covered by unit tests asserting `execv` is still called with `["-m", "headroom.cli", "proxy", "--port", "8787"]` for a console-script `argv[0]`, but I have not watched `vmmap` on a live proxy. **A macOS maintainer should confirm #2820's RSS fix still works end to end before this ships.** ## Runtime Rollout Safety - Rollout-managed feature(s): none. - Minimum rollout channel: N/A. - Stable/default behavior changed: no for a real CLI launch; the re-exec no longer fires when the CLI is invoked in-process, which was never intended to work. - Kill switch / disable path: `HEADROOM_MALLOC_TUNING=0` still disables the tuning outright. - Unsafe override required: none. - Qualification impact: none. - Rollback path: revert this commit — but that restores a `main` whose test suite cannot run on macOS. ## 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 - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] New and existing unit tests pass locally with my changes ## Additional Notes **This is my fault and worth recording.** I merged both #2879 and #3035 earlier today on the rule "approved + green CI". Both were genuinely approved and genuinely green. Neither was rebased onto current `main` first, and CI has no macOS runner, so green meant less than it appeared to. Two process gaps this exposes, neither of which this PR fixes: 1. **The Python test suite never runs on macOS.** The repo has macOS jobs (`macos-native-wrapper`, `wrap-native (macos-latest)`), but the `test` shards are `ubuntu-latest` only, so Darwin-only code paths — the allocator tuning is one, `wrap` has others — are unreachable by pytest in CI. Even a reduced macOS shard would have caught this. 2. **Nothing requires a PR to be current with `main` before merging.** Both defects here are cross-PR interactions that no per-PR check can see. Enabling "require branches to be up to date before merging" on `main` would have forced a rebase and surfaced the Gemini fake. I would suggest an issue for each rather than folding them in here. Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> |
||
|
AxelRay
|
ef7e07e0f5 |
fix(policy): price net-cost mutations with the 1h cache-write tier (#2780)
## Description This fixes the net-cost mutation gate for requests using Anthropic's 1-hour prompt-cache TTL. The gate previously hardcoded the 5-minute cache-write multiplier of 1.25x. A 1-hour cache write costs 2.0x, so the old calculation understated the true write penalty and could incorrectly recommend mutation for 1-hour clients. Closes #2773 ## 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 - Added TTL-aware cache-write multiplier selection for 5-minute and 1-hour tiers. - Threaded the resolved TTL through the content router and compression policy helpers. - Preserved the existing 5-minute behavior as the default. - Added Python and Rust regression coverage for the 1-hour tier. - Retuned the netcost gate fixtures so the 1-hour write tier flips the decision in the full ContentRouter path. - Did not edit CHANGELOG.md. ## Testing - [x] Unit tests pass (pytest) - [x] Linting passes (ruff check .) - [ ] Type checking passes (mypy headroom) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text pytest tests/test_compression_policy.py -q 20 passed cargo test -p headroom-core --lib compression_policy -- --nocapture 14 passed pytest tests/test_netcost_gate.py -q 27 passed Ruff checks and formatting passed. git diff --check passed. ``` ## Real Behavior Proof - Environment: Linux x86_64 contributor checkout with Python and Rust test environments. - Exact command / steps: - Ran the Python compression policy test suite. - Ran the Rust compression policy unit tests. - Ran the netcost gate suite, including the 1-hour env and request-marker cases. - Exercised the new 1-hour TTL golden case alongside the existing 5-minute cases. - Observed result: The 1-hour case uses the 2.0x write multiplier and skips the same candidate that still mutates under 5-minute pricing. Existing 5-minute behavior remains covered and passing. - Not tested: A live Anthropic request through the proxy and production traffic. ## 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 - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] 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) ## Screenshots (if applicable) Not applicable for this backend policy fix. ## Additional Notes Ready for review. CI is green on the current tip. |
||
|
Abhay Singh
|
2a8472525d |
feat(wrap/claude): make the --1m fallback model configurable via HEADROOM_1M_MODEL (#2983)
## Description
The model `headroom wrap claude --1m` falls back to (when no model is
otherwise selected) was a hardcoded constant `claude-opus-4-8`, with no
env var or config key to override it. So it goes stale with every new
Opus release, and the only workaround is pinning `ANTHROPIC_MODEL`
globally -- which also changes every non-`--1m` session and overrides
Claude Code's own `/model` picker. The knob the user actually wants
("what should `--1m` default to") did not exist (#2937).
## Fix
Add a `HEADROOM_1M_MODEL` env override that `_resolve_1m_model` consults
for its fallback default, and bump the built-in default to
`claude-opus-5` (Opus 5 has shipped):
```python
_1M_MODEL_ENV = "HEADROOM_1M_MODEL"
_DEFAULT_1M_MODEL = "claude-opus-5"
def _resolve_1m_model(current: str | None) -> str:
fallback = (os.environ.get(_1M_MODEL_ENV) or "").strip() or _DEFAULT_1M_MODEL
base = (current or "").strip() or fallback
return base if base.endswith(_CONTEXT_1M_SUFFIX) else f"{base}{_CONTEXT_1M_SUFFIX}"
```
Precedence is unchanged: an explicit `ANTHROPIC_MODEL` (or a
pass-through `--model`, via the existing `_apply_1m_to_claude_args`)
still wins. `HEADROOM_1M_MODEL` only supplies the fallback when nothing
else is selected. The `[1m]` suffixing and idempotency are unchanged.
Fixes #2937
## Type of Change
- [ ] Bug fix (non-breaking change that fixes an issue)
- [x] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [x] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `docs/content/docs/configuration.mdx`: document `HEADROOM_1M_MODEL`
(new "Claude 1M context window" subsection covering `--1m` resolution
order and `[1m]` acceptance) and register it in the Environment
Variables catalog with its current default.
- `tests/test_cli/test_wrap_helpers.py`: assert the knob stays
documented and the documented default tracks `_DEFAULT_1M_MODEL`, so it
cannot silently drift.
- `headroom/cli/wrap.py`: add `HEADROOM_1M_MODEL` env override in
`_resolve_1m_model`; bump `_DEFAULT_1M_MODEL` to `claude-opus-5`.
- `tests/test_cli/test_wrap_helpers.py`: env override wins the fallback;
an explicit current model still wins over the env; blank env falls back
to the built-in; env value is idempotent for an already-`[1m]` value.
Updated the existing "falls back to default" test to assert against the
constant (robust to future bumps) and to clear the env var.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] Type checking passes (`mypy`)
- [x] New tests added
### Test Output
```text
tests/test_cli/test_wrap_helpers.py -k "resolve_1m or apply_1m" 11 passed
tests/test_cli/test_wrap_claude_vertex_proxy_env.py -k 1m 4 passed
# uvx ruff@0.15.22 check -> All checks passed!
# uvx mypy@1.20.2 headroom/cli/wrap.py -> Success: no issues found in 1 source file
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1,
ruff 0.15.22 and mypy 1.20.2 via uvx.
- Exact command / steps: exercised `_resolve_1m_model` directly with the
env var set/unset. With `HEADROOM_1M_MODEL=claude-opus-9` and no
`ANTHROPIC_MODEL`, `--1m` resolves to `claude-opus-9[1m]`; with the env
var unset it resolves to `claude-opus-5[1m]`; a set `ANTHROPIC_MODEL`
(e.g. `claude-sonnet-5`) still wins as `claude-sonnet-5[1m]`.
- Observed result: operators can point `--1m` at the current Opus
without a code change and without pinning `ANTHROPIC_MODEL` globally,
and a fresh install no longer silently opts `--1m` into the previous
generation.
- Not tested: a live Claude Code 1M session (no entitled account here).
The resolution is verified at the helper the launch path uses.
## Runtime Rollout Safety
- Rollout-managed feature(s): none. `wrap claude --1m` model resolution
is a launch-time CLI helper, not a rollout-channel-gated runtime
feature.
- Minimum rollout channel: N/A (no rollout-managed behavior).
- Stable/default behavior changed: yes, narrowly. The built-in `--1m`
fallback default moves from `claude-opus-4-8` to `claude-opus-5` only
when neither `HEADROOM_1M_MODEL` nor `ANTHROPIC_MODEL` is set; any
explicit selection is unaffected.
- Kill switch / disable path: set `HEADROOM_1M_MODEL` (or
`ANTHROPIC_MODEL`) to pin any model; both override the default.
- Unsafe override required: no.
- Qualification impact: none. No proxy request path, routing, or token
accounting is touched.
- Rollback path: revert this PR, or set
`HEADROOM_1M_MODEL=claude-opus-4-8` to restore the prior default without
a code change.
## 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
- [x] 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
- [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
## Additional Notes
The default bump (`claude-opus-4-8` -> `claude-opus-5`) is the second
half of the issue's request. If you would rather keep the constant and
ship only the env override, I can drop that one line; the override alone
already lets operators avoid the stale default.
---------
Co-authored-by: JD Davis <mxjerrett@gmail.com>
|
||
|
Abhay Singh
|
ddd9f76729 |
fix(install): stop the PowerShell installer leaking temp dirs into the real user PATH (#2985)
## Description
`scripts/install.ps1` persists the install directory to the user's PATH
through `Ensure-PathEntry`, which calls
`[Environment]::SetEnvironmentVariable('Path', ..., 'User')`. That value
lives in the `HKCU\Environment` registry key, so it is **not** scoped by
a `HOME` / `USERPROFILE` override.
`tests/test_install/test_native_installers.py::test_powershell_native_installer_supports_persistent_docker_lifecycle`
runs that real installer against a `tmp_path` fake home. Every run
therefore prepended the test's throwaway shim directory to the
developer's actual, persistent user PATH -- and it stayed there after
the test finished. The entries accumulate one per run, ahead of the real
install dir; and since the installer also drops
`headroom.ps1`/`headroom.cmd` into that dir, `headroom` in a fresh shell
could then resolve to a leftover wrapper from a deleted temp directory
(#2970).
## Fix
Make the persistence scope configurable via
`HEADROOM_INSTALL_PATH_SCOPE`, defaulting to `'User'` so production
behavior is unchanged:
```powershell
$scope = if ($env:HEADROOM_INSTALL_PATH_SCOPE) { $env:HEADROOM_INSTALL_PATH_SCOPE } else { 'User' }
$currentPath = [Environment]::GetEnvironmentVariable('Path', $scope)
...
[Environment]::SetEnvironmentVariable('Path', ($newPath -join ';'), $scope)
```
The installer tests (`_build_env`) set
`HEADROOM_INSTALL_PATH_SCOPE=Process`, so the PATH update stays in the
spawned PowerShell process (discarded when it exits) instead of writing
to the registry.
Fixes #2970
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `scripts/install.ps1` (`Ensure-PathEntry`): read/write the PATH via
`$env:HEADROOM_INSTALL_PATH_SCOPE` (default `'User'`).
- `tests/test_install/test_native_installers.py`: `_build_env` sets
`HEADROOM_INSTALL_PATH_SCOPE=Process` for every installer invocation;
add a Windows-only
`test_powershell_installer_does_not_leak_into_user_path` asserting the
real User PATH entry count is unchanged across an installer run.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] New test added
### Test Output
```text
tests/test_install/test_native_installers.py -k does_not_leak_into_user_path 1 passed
# uvx ruff@0.15.22 check tests/test_install/test_native_installers.py -> All checks passed!
```
## Real Behavior Proof
- Environment: Windows 11, Windows PowerShell 5.1, Python 3.12.11,
project venv, pytest 9.1.1, ruff 0.15.22 via uvx.
- Exact command / steps: recorded the real user PATH entry count
(`([Environment]::GetEnvironmentVariable('Path','User') -split
';').Count` = 27), ran the PowerShell installer test with the fix, then
re-read the count: still 27 -- no leak. The new
`test_powershell_installer_does_not_leak_into_user_path` formalizes this
(before == after).
- Observed result: running the installer test suite no longer mutates
the developer's persistent user PATH; production installs still persist
to `'User'` as before.
- Not tested: the sibling
`test_powershell_native_installer_supports_persistent_docker_lifecycle`
fails on my Windows host on an unrelated `trusted_cidrs`
dashboard-gateway assertion (it fails identically on `main` without this
change, and the whole PowerShell suite is skipped on the Linux CI
runners). This PR does not touch that path.
## Runtime Rollout Safety
- Rollout-managed feature(s): none. This is the native PowerShell
installer script, not a rollout-channel-gated runtime feature.
- Minimum rollout channel: N/A (no rollout-managed behavior).
- Stable/default behavior changed: no. Production installs still persist
PATH to the `User` scope exactly as before; the new
`HEADROOM_INSTALL_PATH_SCOPE` override defaults to `User` and is used
only by the test suite to avoid mutating the developer's persistent
PATH.
- Kill switch / disable path: leave `HEADROOM_INSTALL_PATH_SCOPE` unset
(the default) for the normal `User` behavior.
- Unsafe override required: no.
- Qualification impact: none. Installer-only; no proxy runtime path is
touched.
- Rollback path: revert this PR; the installer returns to writing the
`User` PATH unconditionally.
## 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
- [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
## Additional Notes
The scope override defaults to `'User'`, so nothing changes for real
installs. It doubles as an escape hatch for any environment (CI images,
ephemeral containers) that must not touch the persistent user PATH.
---------
Co-authored-by: JD Davis <mxjerrett@gmail.com>
|
||
|
Nestor G Pestelos Jr
|
c8310819a4 |
fix(wrap): set xAI upstream for grok-build proxy (#2772)
## Description `headroom wrap grok-build` injected the client hop into `~/.grok/config.toml` but started the local proxy **without** setting the OpenAI-compatible upstream to xAI. The proxy defaulted to `api.openai.com`, so Grok session auth returned **401** on every chat completion even though compression still ran. `wrap grok` already passes `openai_api_url` → xAI. This PR aligns `wrap grok-build` and the Grok-only persistent `install` path on the shared `DEFAULT_API_URL` (`https://api.x.ai`). Closes # (none — discovered in live Grok Build pilot) ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which 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 - Pass `openai_api_url=_GROK_DEFAULT_API_URL` into `_run_proxy_only_watcher` from `wrap grok-build` - Use shared `DEFAULT_API_URL` from `wrap grok` (no hard-coded string drift) - Print proxy upstream in Grok Build setup lines - Persistent install: when targets are Grok-only, set `OPENAI_TARGET_API_URL` + `--openai-api-url` (skip when Codex/Copilot/Aider/OpenCode share the proxy; explicit env still wins) - Regression tests for wrap kwargs, setup lines, and install planner ## Testing - [x] Unit tests pass (`pytest` targeted suite) - [ ] Linting passes (`ruff check .`) — not run in this environment (no native editable build) - [ ] Type checking passes (`mypy headroom`) — not run - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ PYTHONPATH=$PWD python -m pytest \ tests/test_cli/test_wrap_bridge.py::test_wrap_grok_build_passes_xai_openai_api_url \ tests/test_cli/test_wrap_bridge.py::test_wrap_grok_build_uses_actual_proxy_port \ tests/test_install/test_planner.py::test_build_manifest_grok_build_only_sets_xai_upstream \ tests/test_install/test_planner.py::test_build_manifest_grok_with_codex_does_not_force_xai \ tests/test_install/test_planner.py::test_build_manifest_extra_env_wins_over_grok_xai_default \ tests/test_provider_grok_build.py::test_grok_build_setup_lines_include_proxy_url -q ...... 6 passed in 0.33s ``` ## Real Behavior Proof - Environment: macOS (darwin), Headroom 0.33.0 via `uv tool install "headroom-ai[proxy,mcp,code]==0.33.0"`, Grok Build CLI, models `grok-build` and `grok-4.5`, proxy on `127.0.0.1:8787`, upstream must be xAI - Exact command / steps: (1) Before: stock `headroom wrap grok-build` then `grok -m grok-build` one-shot prompt. (2) After: same wrap path with this branch (`openai_api_url=DEFAULT_API_URL` into `_run_proxy_only_watcher`) then `grok -m grok-build -p '…HEADROOM_XAI_OK…'`. Also exercised `grok-4.5` via `[model."grok-4.5"] base_url` → same proxy. - Observed result: Before — proxy log outbound `api.openai.com` → HTTP 401; client failed while local compression still ran. After — setup line prints Proxy upstream `https://api.x.ai`; proxy log `POST https://api.x.ai/v1/chat/completions` (and `/v1/responses` for grok-4.5) → status=200; dashboard shows 0 failed requests and accumulating token savings on live traffic. - Not tested: full `uv run` editable/maturin native build on this host; multi-tool install matrix beyond planner unit tests; Windows; ruff/mypy full tree ## 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 commented my code, particularly in hard-to-understand areas - [ ] I made corresponding changes to the documentation (CLI help text / setup lines only) - [x] My changes generate no new warnings - [x] I 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` — generated by release-please from Conventional Commit PR title (a CI guard enforces this) ## Additional Notes - Intentional non-goal: changing default model, savings %, or Grok Build context-tool defaults - Mixed-target install (e.g. `grok_build` + `codex`) does **not** force xAI — operator must set upstream explicitly if they share one proxy - Related live routing: manual `[model."grok-4.5"] base_url` through the same proxy works once upstream is xAI (`/v1/responses`) --------- Co-authored-by: Grok 4.5 <noreply@x.ai> Co-authored-by: Nestor G Pestelos Jr <ngpestelos@me.com> Co-authored-by: JD Davis <jd@jds-macbook-air.tail2a279.ts.net> |
||
|
Abhay Singh
|
6d87825f62 |
fix(proxy): tune macOS libmalloc and trim allocator pages so long-lived RSS stays bounded (#2879)
## Summary Fixes #2820. Prevents long-lived macOS proxies from retaining every largest transient request-body allocation in libmalloc. The reporter’s production A/B isolated the allocator behavior and verified the two pre-main libmalloc knobs; this PR applies them through a one-time Darwin-only re-exec and adds periodic per-worker pressure relief. - `MallocAggressiveMadvise=1` returns freed pages eagerly. - `MallocLargeCache=0` disables the large-allocation death-row cache. - Operator-set allocator variables are preserved; `HEADROOM_MALLOC_TUNING=0` is the kill switch. - Periodic trim defaults on only for macOS, runs off the event loop, performs no forced Python GC, validates its interval, and is retained/cancelled through the app lifecycle. - Non-Darwin behavior remains unchanged unless explicitly enabled. - Semantically rebased onto current `main`, retaining startup dependency validation, MCP SDK v1 compatibility, and all newer proxy behavior. ## Verification - 147 proxy CLI/config/malloc/MCP-contract tests pass; 1 platform skip. - Ruff check and formatting clean; `git diff --check` clean. - The reporter’s macOS A/B reduced dirty empty malloc regions to zero and lowered steady/startup RSS; the control flow and shutdown lifecycle are covered locally. ## Safety The re-exec is Darwin-only, PID-preserving, loop-guarded, and opt-out. The trim task is per worker because allocator state is per process, and shutdown cancels it explicitly. |
||
|
Abhay Singh
|
be5b26d807 |
fix(doctor): surface that Claude Desktop agent sessions bypass the proxy (#2987)
## Description `headroom doctor` reports the `claude` check as a pass whenever `~/.claude/settings.json` carries an `ANTHROPIC_BASE_URL` pointing at the proxy. That is correct for the terminal Claude Code CLI. But Claude Desktop (`com.anthropic.claudefordesktop`) unconditionally overwrites that variable when spawning agent sessions (#869), so on a Desktop-primary machine `doctor` asserts routing that is in fact discarded, and nothing in the output hints that Desktop sessions are unrouted (#2925). ## Fix Add a per-surface `claude desktop` check that warns about the bypass when Claude Desktop's config directory is detected, pointing at #869. Following the issue's suggestion, it models per-surface reporting like the existing `wrap_marker` / `shell env` rows: it is a separate row emitted only when Desktop is present, so it never contradicts a genuinely routed CLI, and the existing `claude` check is left unchanged. Detection uses Claude Desktop's per-user config directory (distinct from the CLI's `~/.claude`): - macOS: `~/Library/Application Support/Claude` - Windows: `%APPDATA%\Claude` - Linux: `$XDG_CONFIG_HOME/Claude` (or `~/.config/Claude`) Fixes #2925 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature - [ ] Breaking change - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/cli/doctor.py`: add `claude_desktop_config_dir()` (cross-platform) and `check_claude_desktop()` (WARN when the dir exists, `None` otherwise); append it to the `doctor()` check list when present. - `tests/test_cli_doctor.py`: `TestClaudeDesktop` -- no row when absent; WARN naming the bypass and #869 when present; the `doctor --json` entrypoint appends the row only when Desktop is detected. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check`) - [x] Type checking passes (`mypy`) - [x] New tests added ### Test Output ```text tests/test_cli_doctor.py 78 passed # uvx ruff@0.15.22 check -> All checks passed! # uvx mypy@1.20.2 headroom/cli/doctor.py -> Success: no issues found in 1 source file ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1, ruff 0.15.22 and mypy 1.20.2 via uvx. - Exact command / steps: `uvx ruff@0.15.22 check headroom/cli/doctor.py tests/test_cli_doctor.py`; `uvx mypy@1.20.2 headroom/cli/doctor.py`; `python -m pytest tests/test_cli_doctor.py -q`; then drove the check directly and through the `doctor --json` entrypoint with `claude_desktop_config_dir` pointed at a tmp dir (created the dir, ran `doctor --json`, then removed it and reran). - Observed result: with the dir present, a `claude desktop` row appears with status `warn` and a `#869` hint; with the dir absent, no such row is emitted and the rest of the report is unchanged. A Desktop-primary machine now gets an explicit warning that Desktop agent sessions bypass the proxy, instead of a bare `claude: pass` that reads as though all Claude routing is live. - Not tested: a live Claude Desktop install (detection is directory-existence, exercised against a tmp dir). ## Runtime Rollout Safety - Rollout-managed feature(s): none. This adds a read-only diagnostic row to `headroom doctor`; it is not behind any rollout channel or feature flag. - Minimum rollout channel: N/A (no rollout-managed behavior). - Stable/default behavior changed: no. The existing `claude` check and all other rows are unchanged; the new `claude desktop` row is additive and only appears when Claude Desktop's config directory is detected. - Kill switch / disable path: N/A. The row self-suppresses (returns `None`) on any machine without the Desktop config directory. - Unsafe override required: no. - Qualification impact: none. No proxy request path, routing, or token accounting is touched; the change is confined to the doctor diagnostic surface. - Rollback path: revert this PR; the doctor output returns to its prior set of rows with no state or migration to undo. ## 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 - [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 ## Additional Notes Scope: this warns whenever Claude Desktop is present, which is accurate (Desktop agent sessions always bypass per #869) and matches the precedent for doctor-accuracy fixes (#2618/#2614 Codex, #2566 ollama). The issue's stronger refinement -- suppress the warning when a `client=claude-code` request has recently reached the proxy -- would need per-client traffic observation the doctor does not have today; I left that as a follow-up rather than build new traffic-tracking infra into this fix. Happy to add it if you'd prefer the conditional form. Rebased onto current `main` to resolve an overlap with the newly merged `check_claude_auth_conflict` in `doctor.py`; both checks now coexist. Co-authored-by: JD Davis <mxjerrett@gmail.com> |
||
|
Abhay Singh
|
536c949a69 |
fix(proxy/openai): propagate provider usage on the Responses WS->HTTP fallback (#2988)
## Description
When Codex uses the OpenAI Responses WebSocket endpoint through Headroom
and the upstream WebSocket is rejected, Headroom falls back to HTTPS
POST/SSE. On that fallback the dashboard reported zero or tiny input
tokens for a large request, and invalid savings:
```json
{ "input_tokens_original": 3, "input_tokens_optimized": 0,
"output_tokens": 246, "tokens_saved": 31052, "savings_percent": 33233.33 }
```
## Root cause
`_ws_http_fallback` (openai.py) relays the SSE `data:` events to the
client but never parses the terminal `response.completed` event for
usage. The non-fallback WS path accumulates
`_extract_responses_usage(event)` into the session totals on every
`response.completed` frame (openai.py ~8182); the fallback path did not.
So `ws_input_tokens_total` stayed at the small local count, and the
session-end RequestLog computed `optimized_tokens =
residual_input_tokens = 0`, leaving `tokens_saved >
input_tokens_original` and `savings_percent` far above 100%.
## Fix
`_ws_http_fallback` now parses each relayed `response.completed` line
with the existing `_extract_responses_usage` and returns the accumulated
`(input, output, cache_read, cache_write, uncached)` provider usage. The
caller folds it into the WS session totals, so the session-end outcome
uses the authoritative provider wire-token count -- bringing the
fallback to parity with the non-fallback WS path. SSE relay behaviour is
otherwise unchanged.
Fixes #2957
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/proxy/handlers/openai.py` (`_ws_http_fallback`): accumulate
usage from `response.completed` SSE lines (both the main relay loop and
the buffer flush) and return the `(input, output, cache_read,
cache_write, uncached)` tuple from every exit path; the WS handler
caller adds it to `ws_input_tokens_total` / `ws_output_tokens_total` /
cache / uncached totals before the session-end RequestLog.
- `tests/test_ws_http_fallback.py`: the fallback returns the provider
usage from a `response.completed` event
(input/output/cache_read/uncached), and returns all-zeros when no
completed event arrives.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] Type checking passes (`mypy`)
- [x] New tests added
### Test Output
```text
tests/test_ws_http_fallback.py 13 passed (11 existing + 2 new)
# uvx ruff@0.15.22 check -> All checks passed!
# uvx mypy@1.20.2 headroom/proxy/handlers/openai.py -> Success: no issues found in 1 source file
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1,
ruff 0.15.22 and mypy 1.20.2 via uvx.
- Exact command / steps: drove `_ws_http_fallback` with the existing
WS/stream mocks, feeding an SSE `response.completed` carrying
`usage.input_tokens=31055`, `output_tokens=246`,
`input_tokens_details.cached_tokens=20000`. The method now returns
`(31055, 246, 20000, ..., 11055)`; a stream with no completed event
returns all zeros. The existing 11 relay/routing/retry tests are
unchanged (they ignore the new return value).
- Observed result: the fallback surfaces the provider's real input
usage, so the WS session-end outcome records the actual input tokens
instead of 0, and savings percentages stay within a meaningful range.
- Not tested: a live Codex WS session that triggers the upstream-WS
rejection and HTTP fallback end to end (needs a real upstream refusing
the WS). The usage-propagation contract is verified at the fallback
boundary with the same mocks the existing fallback tests use.
## Runtime Rollout Safety
- Rollout-managed feature(s): none. The OpenAI Responses WS-to-HTTP
fallback is always-on transport behavior, not rollout-channel-gated.
- Minimum rollout channel: N/A (no rollout-managed behavior).
- Stable/default behavior changed: yes, as a bug fix. On the WS-to-HTTP
fallback the session-end outcome now records the provider's real
input/output/cache usage from `response.completed` instead of leaving
`ws_input_tokens_total` at 0 (which produced >100% savings). SSE relay
to the client is unchanged.
- Kill switch / disable path: N/A. This corrects accounting only; there
is no behavioral toggle and no user-facing surface beyond the recorded
outcome numbers.
- Unsafe override required: no.
- Qualification impact: fallback-path token accounting now matches the
non-fallback WS path and the HTTP Responses path (all three use
`_extract_responses_usage`); savings percentages return to a valid
range.
- Rollback path: revert this PR; the fallback returns to reporting zero
input usage on this path.
## 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
- [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
## Additional Notes
The fix reuses the already-present `_extract_responses_usage` (same
parser the non-fallback WS path and HTTP Responses path use), so
cache-read/write and uncached accounting stay consistent across all
three transports.
Co-authored-by: JD Davis <mxjerrett@gmail.com>
|
||
|
Gil Korzen
|
a06a51eca6 |
fix(proxy): preserve Codex WebSocket model attribution (#3029)
## Description Codex can switch models during a multi-turn Responses WebSocket conversation. Headroom was not consistently attributing each completed turn to the model that handled it, which made per-model usage and savings reporting inaccurate. Closes #3027 ## 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 - Attribute each completed WebSocket response to its reported model. - Keep session-end metrics consistent with the response that completed. - Add a regression test covering two different models on one WebSocket session. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added new functionality - [x] Manual testing performed ### Test Output ```text uv run pytest -q tests/test_openai_codex_ws_lifecycle.py -k session_metrics_track_model_per_response_create 1 passed, 51 deselected in 2.09s Full Codex WebSocket lifecycle module: 52 passed Adjacent Codex WebSocket suites: 77 passed, 1 skipped uv run ruff check . All checks passed uv run ruff format --check . 1411 files already formatted uv run mypy headroom Success: no issues found in 520 source files ``` ## Real Behavior Proof - Environment: Windows, Python 3.13.3, OpenAI Codex Responses WebSocket. - Exact command / steps: From the repository root, run `uv sync --extra dev --extra proxy`, then run `uv run headroom wrap codex`; in one live Codex conversation complete one turn with model A, switch to model B, complete a second turn, and inspect the proxy dashboard or `http://localhost:8787/stats` recent requests. - Observed result: Both completed turns appeared under the models that handled them, in order. - Not tested: Production deployment and non-Codex transports. ## Runtime Rollout Safety - Rollout-managed feature(s): None. - Minimum rollout channel: Stable/default. - Stable/default behavior changed: Corrects telemetry attribution only; no public API or routing changes. - Kill switch / disable path: Revert the change or use the previous release. - Unsafe override required: No. - Qualification impact: None. - Rollback path: Revert commit `d5d8d7ca`. ## 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 - [ ] 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) ## Screenshots (if applicable) ### Pre change In a single session, started with `5.6-sol` and then switched to `5.6-luna`. The dashboard did not reflect the model change. <img width="1274" height="207" alt="image" src="https://github.com/user-attachments/assets/697803e4-d33d-4660-b8dd-f1a8d6404517" /> ### After change Repeated the same steps: started with `5.6-sol` and switched to `5.6-luna`. The dashboard now correctly reflects the model change. <img width="1264" height="202" alt="image" src="https://github.com/user-attachments/assets/722e5fab-ad1b-4e62-9344-5f9dd312d614" /> ## Additional Notes |
||
|
Abhay Singh
|
a01897c791 |
fix(proxy/gemini): guard CCR continuation usage against present-null counts (#3035)
## Description
On the Gemini native `generateContent` path, a successful (200) response
that triggers a CCR retrieval continuation is masked as a synthetic 502
when the continuation response carries a present-null usage count.
`handle_gemini_generate_content` reads `usageMetadata` at three sites.
The initial-response site and the non-CCR site both guard against Gemini
returning a present-null count (a key present with a JSON `null`, which
`.get(key, default)` returns as `None` rather than the default). The
CCR-continuation site read the continuation's `usageMetadata` with a
bare `.get(key, prior)`:
```python
total_input_tokens = usage.get("promptTokenCount", total_input_tokens)
output_tokens = usage.get("candidatesTokenCount", output_tokens)
cache_read_tokens = usage.get("cachedContentTokenCount", cache_read_tokens)
```
When the continuation turn reports `"promptTokenCount": null`,
`total_input_tokens` becomes `None`, and the following
`uncached_input_tokens = max(0, total_input_tokens - cache_read_tokens)`
and the `total_input_tokens > 0` baseline guard raise `TypeError`. The
method's outer `except Exception` then returns a 502 JSONResponse and
records a provider failure, so a genuinely successful upstream turn is
reported to the client as a 502.
## Fix
Read the continuation usage through the same `_usage_int` guard the two
sibling sites use, keeping the pre-continuation count as the fallback
(`_usage_int(value, default)` returns `default` when `value is None`).
Behavior is otherwise unchanged: a present, valid count is still used,
and an absent count still falls back to the pre-continuation value.
Fixes #3034
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/proxy/handlers/gemini.py` (`handle_gemini_generate_content`,
CCR-continuation branch): read `promptTokenCount` /
`candidatesTokenCount` / `cachedContentTokenCount` through
`_usage_int(..., prior)` instead of a bare `.get(key, prior)`.
- `tests/test_gemini_ccr_continuation_usage.py`: drive the handler
through a CCR continuation whose `usageMetadata` counts are
present-null; assert the client gets 200 (not 502), no provider failure
is recorded, and the pre-continuation count survives as the fallback.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] Type checking passes (`mypy`)
- [x] New tests added
### Test Output
```text
tests/test_gemini_ccr_continuation_usage.py 1 passed
tests/test_gemini_nonjson_status.py tests/test_gemini_compression_offload.py tests/test_proxy_gemini_native_integration.py (all pass; platform-skipped cases skipped)
# uvx ruff@0.15.22 check -> All checks passed!
# uvx mypy@1.20.2 headroom/proxy/handlers/gemini.py -> Success: no issues found in 1 source file
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1,
ruff 0.15.22 and mypy 1.20.2 via uvx.
- Exact command / steps: ran `python -m pytest
tests/test_gemini_ccr_continuation_usage.py -q` (pass-after); proved
fail-before by `git stash`-ing only the `gemini.py` change and
re-running (the test failed with `assert 502 == 200` and the captured
log `TypeError: unsupported operand type(s) for -: 'NoneType' and
'NoneType'` at `gemini.py`), then restored the fix and re-ran green; ran
the surrounding Gemini suite (`test_gemini_nonjson_status.py`,
`test_gemini_compression_offload.py`,
`test_proxy_gemini_native_integration.py`); then `uvx ruff@0.15.22
check` and `uvx mypy@1.20.2 headroom/proxy/handlers/gemini.py`.
- Observed result: with the fix a CCR continuation carrying a
present-null `promptTokenCount` returns 200 to the client and records
the outcome with the pre-continuation count (100) instead of raising
`TypeError` and returning a synthetic 502.
- Not tested: a live Gemini session that both triggers a CCR retrieval
continuation and receives a present-null continuation usage payload
(needs a real safety-blocked continuation). The contract is verified at
the handler with the same stub pattern the existing
`test_gemini_nonjson_status.py` uses.
## Runtime Rollout Safety
- Rollout-managed feature(s): none. This is the always-on Gemini native
`generateContent` request path, not a rollout-channel-gated feature.
- Minimum rollout channel: N/A (no rollout-managed behavior).
- Stable/default behavior changed: yes, as a bug fix. A CCR continuation
with a present-null usage count now returns the real 200 instead of a
synthetic 502; all other cases (present valid count, absent count) are
unchanged.
- Kill switch / disable path: N/A. There is no behavioral toggle; the
change only makes the existing continuation path null-safe.
- Unsafe override required: no.
- Qualification impact: brings the CCR-continuation usage extraction to
parity with the two sibling sites that already guard present-null
counts; no routing, compression, or pricing change.
- Rollback path: revert this PR; the continuation site returns to the
bare `.get(key, prior)` read.
## 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
- [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
## Additional Notes
The unguarded site was introduced in #2253 (native CCR retrieval); the
present-null guard on the sibling sites landed separately and did not
extend to it. The fix reuses the existing `_usage_int` helper so all
three Gemini usage-extraction sites now handle present-null identically.
|
||
|
Parideboy
|
9d370592b0 |
fix(proxy): stop cached responses replaying the producing turn's wire framing (#3024)
## Description Closes #3019 A response-cache hit could hand the client an HTTP 200 that the client could not read, and nothing in the logs marked the turn as anything other than normal. Two separate problems combine to produce the reported failure. **The unreadable 200.** A cache entry stores the producing upstream's response headers verbatim. When the entry is replayed, the Anthropic handler removed only `content-encoding`, `content-length` and `content-type` before handing those headers to a brand-new `Response`. Anything else describing how that *other* connection framed its body rode along — most damagingly `transfer-encoding: chunked`. RFC 9112 §6.1 makes `Transfer-Encoding` override `Content-Length`, so the client is told to parse a plain JSON body as chunked frames, finds no valid chunk-size line, and reads an empty body out of a 200. Every other response-forwarding site in the Python proxy already strips that header; the two cache-hit sites were the only ones that did not. **How a CCR turn could put a foreign response in the cache.** On the Anthropic path, `cache.get` is gated on `not stream` but `cache.set` was not, and the cache key has no `stream` component. A CCR buffered-stream conversion takes a request the client sent with `stream: true`, forces `stream: false` upstream, and — unlike every other streaming turn, which returns via `_stream_response` and never touches the cache — falls through to the store site. The stored reply was shaped by that forced flip plus CCR tool injection, and the key cannot distinguish it from an ordinary non-streaming reply, so a later non-streaming caller could be served a response built for a request it never made. This is why the reporters saw the failures pair with CCR activity and stop under `--lossless` / `--no-ccr`. **Why it was invisible.** The cache-hit block emitted no log line at all, and the `PERF` line rendered no field for `RequestOutcome.from_response_cache`. A cache-served turn contacts no upstream, so it has no `outbound_request` line, no upstream stage timings, and all-zero token counters — byte-for-byte what a turn that died would look like. That is why `headroom doctor` reported zero failures while turns were dying. ### Scope note The header fix also lands on the OpenAI cache-hit site, which additionally never received the `content-type` fix from #2952. The `not stream` gate is added to the OpenAI store site too, where it is currently redundant — a streaming chat request returns via `_stream_response` long before that point — purely to state the invariant, since the Anthropic handler had exactly that shape until a buffered-CCR branch began falling through to it. Because the strip list now lives in one shared helper, the OpenAI handler's other five forwarding sites strip the three added headers as well. That is a widening, so it is worth being explicit about: each of those sites builds a fresh fixed-length `Response` (or, at `openai.py:6122`, synthesises SSE) from `response.content`, so replaying the upstream's framing there was the same latent bug, just without a cache to make it outlive the request that produced it. The precedent is already in the file — `openai.py:9865` passes `"transfer-encoding", "connection"` as extra names by hand, which is exactly the gap this PR closes centrally. That call site keeps its now-redundant arguments; removing them is a cleanup for another PR. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - Added `sanitize_forwarded_response_headers` to `headroom/proxy/helpers.py`, promoting the private helper that already lived in `headroom/proxy/handlers/openai.py` and extending it with the remaining wire-framing headers (`transfer-encoding`, `connection`, `keep-alive`). Matching is now case-insensitive; surviving headers keep their original casing. `openai.py`'s `_sanitize_forwarded_response_headers` is now a thin alias so its six call sites and the Anthropic handler strip an identical set. - `headroom/proxy/handlers/anthropic.py`: the response-cache hit now sanitises through that helper (passing `content-type` as an extra name, preserving #2952) instead of three hand-rolled `pop` calls. - `headroom/proxy/handlers/openai.py`: the response-cache hit sanitises the same way, gains the `content-type` handling it was missing, and sets `media_type="application/json"` explicitly. - `headroom/proxy/handlers/anthropic.py`: `cache.set` is now gated on `not stream`, mirroring the read gate. `stream` still holds the client's original flag at that point — the buffered-CCR conversion flips `body["stream"]`, never the local variable. - `headroom/proxy/handlers/openai.py`: the same `not stream` gate on its store site, as an invariant guard. - Both cache-hit sites now log `RESPONSE-CACHE-HIT: model=… bytes=… age_s=… hits=…`, following the existing `CACHE-MISS-ATTRIBUTION` line style. - `headroom/proxy/outcome.py`: the `PERF` line appends `cached=1` on a response-cache hit. It is appended only on a hit, so every other PERF line is byte-identical to before and existing parsers are unaffected. - `headroom/perf/analyzer.py`: `PerfRecord.from_response_cache` reads that field, so `headroom perf` can tell a cache-served turn from a dead one. It defaults to `False`, so older logs still parse. `PERF_RECORD_FIELDS` gains the name at the end of the list, which is what `headroom perf --format csv --raw` uses as its column set; appending keeps every existing column at its current position. `--format json --raw` gains the key too. - `tests/test_anthropic_pre_upstream_backpressure.py`: its cache-hit double was a partial hand-rolled stand-in for `CacheEntry` carrying only a body and headers, so it broke once the hit path started reading the entry's age and hit count. It now constructs a real `CacheEntry`, which is what the cache actually returns. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality ### Test Output ```text $ python -m pytest tests/test_proxy_response_cache_replay.py -q tests\test_proxy_response_cache_replay.py ......... [100%] ============================== 9 passed in 4.22s ============================== # Everything that mentions PERF, the sanitiser, cache.set, PerfRecord or # response_headers, plus the whole proxy suite. $ python -m pytest tests/test_proxy/ tests/test_proxy_compression_headers.py \ tests/test_agent_savings.py tests/test_anthropic_pre_upstream_backpressure.py \ tests/test_backend_nonstreaming_cache_metrics.py tests/test_backend_streaming_cache_metrics.py \ tests/test_ccr_buffered_stream_signed_thinking.py tests/test_cli_perf_format.py \ tests/test_codex_ws_compression_scheduler.py tests/test_handler_outcome_tag_invariant.py \ tests/test_openai_codex_ws_lifecycle.py tests/test_provider_codex_images.py \ tests/test_proxy_handlers_batch.py tests/test_proxy_passthrough_transient_retry.py \ tests/test_proxy_response_cache_replay.py tests/test_proxy_semantic_cache_key.py \ tests/test_proxy_streaming_request_logger.py tests/test_request_outcome.py \ tests/test_savings_tool_search_aggregation.py -q ================== 555 passed, 1 skipped in 88.60s (0:01:28) ================== # Full suite, 16 workers. See "Real Behavior Proof" below for how every # failure here was traced to a pre-existing failure or a parallelism flake. $ python -m pytest tests scripts/tests -n 16 -q -p no:randomly --timeout=300 83 failed, 10493 passed, 657 skipped, 80 errors in 437.00s (0:07:17) $ ruff check . All checks passed! $ ruff format --check <the 7 changed files> 7 files already formatted $ python -m mypy headroom --ignore-missing-imports --python-version 3.13 Found 12 errors in 3 files (checked 520 source files) # All 12 are pre-existing MCP-SDK/tomllib drift in release_version.py, # ccr/mcp_server.py and memory/mcp_server.py; identical count before and # after this change, none in the files it touches. ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.13.11, pytest 9.1.1, ruff 0.16.2, branch based on `upstream/main` at `2d88e31a`. - Exact command / steps: Two experiments. (1) Revert-and-rerun: I reverted both fixes in place (dropped the three framing headers from `FRAMING_RESPONSE_HEADERS`, restored `cache.set` to `if self.cache and response.status_code == 200 and resp_json is not None:`), ran `python -m pytest tests/test_proxy_response_cache_replay.py -q`, then restored the fixes and re-ran. (2) Regression sweep: ran the full suite on this branch, then checked out `upstream/main` into a second worktree and re-ran, in that worktree, exactly the tests that failed here and not there. - Observed result: With the fixes reverted, 5 of 9 new tests fail and reproduce both halves of the bug. `test_buffered_ccr_turn_does_not_write_the_response_cache` fails with `AssertionError: Expected mock to not have been awaited. Awaited 1 times.` — a turn the client sent as `stream: true` really does reach `cache.set` through the buffered-CCR branch. `test_cache_hit_replays_a_body_the_client_can_actually_read` fails with `AssertionError: assert 'transfer-encoding' not in {'transfer-encoding': 'chunked', 'connection': 'keep-alive', 'request-id': ..., 'content-length': '228', ...}` — the replayed 200 carries the producing turn's chunked framing alongside a fresh `content-length`, which is the exact framing conflict a client cannot parse. With the fixes restored, all 9 pass, the replayed body arrives intact as `application/json`, and the run logs both `RESPONSE-CACHE-HIT` and a `PERF … cached=1` line. The full suite on this branch gives `83 failed, 10493 passed, 657 skipped, 80 errors`; 33 of those failures were not in my baseline list, so I ran those 33 in the `upstream/main` worktree and 20 failed there identically (Windows-specific: `sqlite:///C:\…` path handling, private-directory permissions, fsync, ONNX thread caps, serena config discovery). Re-running the remaining 13 serially on this branch gave `1 failed, 25 passed` — the other 12 were xdist parallelism flakes, including all four `tests/test_proxy/test_anthropic_ccr_deferred_injection.py` tests, which are the only ones in this change's blast radius and which pass serially. The one real serial failure, `tests/test_savings_ledger_offload.py::test_concurrent_requests_all_land_their_events` (`AssertionError: a concurrent append was lost / assert 23 == 24`), fails the same way on `upstream/main` run serially. The 80 errors are dashboard-template collection errors unrelated to the proxy. Net: no failure attributable to this change. - Not tested: I could not reproduce against live upstream traffic, so I have not confirmed which upstream in the reporters' setups emits `transfer-encoding: chunked`. Anthropic direct is HTTP/2, where the header is forbidden, but any HTTP/1.1 hop (corporate proxy, third-party gateway, local relay) reintroduces it. I have also not measured whether the `not stream` gate reduces the cache hit rate in practice; by construction it can only drop entries that were unsafe to serve. A reporter running unmodified 0.35.0 with `headroom proxy --no-cache` would confirm the cache path is the one involved, and that flag is a lighter workaround than `--lossless` or `--no-ccr` because it keeps CCR and compression enabled. ## Runtime Rollout Safety - Rollout-managed feature(s): none — this is a correctness fix on the always-on response-cache path (`cache_enabled` defaults to `True`). - Minimum rollout channel: stable. - Stable/default behavior changed: yes, in four ways. Replayed cached responses no longer carry the producing upstream's framing headers (or `server`, on the Anthropic side). Forwarded responses on the OpenAI handler's other five sanitiser call sites no longer carry `transfer-encoding`, `connection` or `keep-alive` either, since the strip list is now shared; all five build a fixed-length response from `response.content`, so none of them could legitimately replay that framing. A turn whose client asked for `stream: true` no longer writes the response cache on the Anthropic path. `PERF` lines gain a trailing `cached=1` on a response-cache hit only; all other PERF lines are unchanged. - Kill switch / disable path: `headroom proxy --no-cache` disables the response cache entirely and bypasses every path this PR touches. - Unsafe override required: no. - Qualification impact: low. No public API, config key, CLI flag or wire format changes. Two additive output changes: the `cached=1` PERF field, which `_parse_kv` already handles the same way it handles the existing trailing `client=` field, and a `from_response_cache` column appended to `headroom perf --format csv --raw` (plus the matching key in `--format json --raw`). Anything consuming that CSV positionally keeps working because the column is last; anything reading it by name is unaffected. - Rollback path: revert this commit. It is self-contained with no migration, no persisted state and no schema change; cache entries written before or after behave identically on read. ## 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 is marked N/A: no user-facing surface changes, and the new `cached=` PERF field is additive and self-describing. Relationship to nearby open PRs, since several touch adjacent code: - **#2953** (already merged, unreleased) added the `resp_json is not None` guard at the same Anthropic store site. That stops an SSE *body* being stored; it does not stop a JSON-bodied response storing chunked framing headers, and it does not add the `stream` gate. The two changes are complementary. - **#2959** and **#2968** both touch the buffered-CCR response path but address when and how the status is committed. Neither reaches the cache-hit replay. - **#3013** rewrites CCR into event-level stream splicing and keeps `buffered_stream_ccr` as a fallback, so the store site this PR gates remains reachable. If #3013 lands first I am happy to rebase. `mypy headroom --ignore-missing-imports` reports 12 pre-existing errors in `headroom/release_version.py`, `headroom/ccr/mcp_server.py` and `headroom/memory/mcp_server.py` from MCP SDK version drift in my local environment. None are in the files this PR touches, and the count is identical before and after the change. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: JD Davis <mxjerrett@gmail.com> |
||
|
Tejas Chopra
|
f9807fd69e |
feat(proxy): let extensions report cost savings and their own latency (#3051)
## What
Two changes that let a proxy extension report **what it saved** and
**what it cost**, so both show up under `/stats`, the dashboard, and
Prometheus.
`record_scope_savings` already existed and already accepted `usd` — the
one channel in the proxy that can express savings *without* tokens. Two
things stopped it working end to end.
### 1. Savings were silently dropped on Gemini traffic (bug)
`bind_scope` shares one attribution ledger between ASGI middleware and
the request handler. Anthropic and OpenAI call it; **Gemini never did**,
so anything an extension recorded into the request scope was discarded
for Gemini traffic only — silently, because an empty ledger and an
unbound one are indistinguishable at the outcome funnel. Now bound at
all four Gemini tag sites.
### 2. An extension's own latency was invisible (gap)
`overhead_ms` is measured *inside* the handler, and an ASGI extension
**wraps** that handler — so every millisecond it spends reaches the
client while every timing surface stays flat. An extension that halves
the bill and adds 200 ms per request is a trade the operator has to see
both halves of, and only one half was reaching the dashboard.
`record_scope_timing(scope, stage, ms)` is the symmetric counterpart to
`record_scope_savings`, carried on the same bound ledger and merged into
`RequestOutcome.pipeline_timing` at the outcome funnel — one place, so
every provider picks it up at once.
## API surface
```python
from headroom.proxy.savings_attribution import record_scope_savings, record_scope_timing
record_scope_savings(scope, "my_extension", tokens=0, usd=0.004) # money without tokens
record_scope_timing(scope, "my_extension", elapsed_ms)
```
Both take the ASGI `scope`, because middleware has no other way in.
Documented in `extensions.py` — the module extension authors actually
read, and the stability contract for this interface.
- Savings → `/stats` `savings.by_source`, dashboard card,
`headroom_savings_attributed_usd_total{source=...}`
- Timing → `/stats` `pipeline_timing`, dashboard Performance panel,
`headroom_transform_timing_ms_*`
**Attribution only.** These rows explain the headline total; they are
never added to it.
## Changes to existing behavior
- `public_tags` now strips `_headroom_stage_timing` as well as
`_headroom_savings_attribution`. Both ride on `tags` because that is the
one dict reaching the outcome funnel from every handler, and a list and
a dict must not land in a string-keyed label store.
- `pipeline_timing` passed to `metrics.record_request` is merged rather
than passed through **only when an extension contributed timings**; with
no extension the handler's own dict is passed through unchanged
(asserted by identity in the tests).
- Stage names are extension-supplied, so they are capped at 16 and
namespaced `ext:` — `deep_copy` reported by a plugin must never
accumulate into the same series as `deep_copy` measured by the pipeline.
A handler's own timing wins a collision (unreachable while the prefix
stands; the safe way round if it ever goes).
## Failure modes
Both calls are bounded (32 sources, 16 stages), never raise, and never
change a response — telemetry from a plugin must not be able to break
the request it is describing. Non-positive and non-numeric durations are
ignored: a zero is a clock artifact, not an observation, and averaging
it in would drag the mean down exactly where the stage is cheapest to
skip. `timings_from_tags` tolerates junk on the tag.
## Test-double fix
Three Gemini test fakes (`FakeRequest`, `_FakeRequest`,
`_VertexGeminiImageRequest`) had no `.scope`, which every real Starlette
`Request` has. They now do. This is a double that had drifted from the
type it stands in for; the alternative was weakening the handler to
tolerate a request shape that cannot occur in production.
---
## Real behavior proof
**Setup:** macOS 15.4 (darwin 25.4.0), Python 3.12.13, this branch at
`c814b950`, real `create_app` proxy with `respx`-mocked Anthropic
upstream, a demo ASGI extension added via `app.add_middleware`.
**The extension** — written as a third party would, reporting `tokens=0`
because it re-routed `claude-opus-5` → `claude-haiku-4-5`: same tokens,
cheaper model. That is precisely the case no existing Headroom savings
channel can express, since all of them compute `saved = before - after`.
```python
class DemoRouter:
def __init__(self, app): self.app = app
async def __call__(self, scope, receive, send):
if scope.get("type") != "http":
return await self.app(scope, receive, send)
started = time.perf_counter()
record_scope_savings(scope, "routemegood", tokens=0, usd=0.173)
record_scope_timing(scope, "routemegood", (time.perf_counter() - started) * 1000)
await self.app(scope, receive, send)
```
**Ran:** three POSTs to `/v1/messages`, then `GET /stats` and `GET
/metrics`.
**Observed:**
```
upstream call -> 200
upstream call -> 200
upstream call -> 200
=== /stats savings.by_source (what the dashboard renders) ===
[
{
"source": "routemegood",
"realized": true,
"events": 3,
"tokens": 0,
"usd": 0.519
}
]
=== /stats pipeline_timing (dashboard Performance panel) ===
{
"ext:routemegood": {
"average_ms": 0.01,
"max_ms": 0.02,
"count": 3
}
}
=== /metrics ===
# HELP headroom_savings_attributed_tokens_total Tokens attributed to a savings source
# TYPE headroom_savings_attributed_tokens_total counter
headroom_savings_attributed_tokens_total{realized="true",source="routemegood"} 0
# HELP headroom_savings_attributed_usd_total Cost savings attributed to a source; may be negative
# TYPE headroom_savings_attributed_usd_total gauge
headroom_savings_attributed_usd_total{realized="true",source="routemegood"} 0.519
headroom_transform_timing_ms_sum{transform="ext:routemegood"} 0.03
```
`$0.519 = 3 × $0.173` — three requests, correctly accumulated, with
`tokens: 0` throughout.
**Also have (not a substitute for the above):** 22 new unit tests in
`tests/test_extension_attribution.py`, including four that drive the
real `_record_request_outcome` funnel via the same descriptor-binding
harness `test_request_outcome.py` uses.
Full suite on this branch: **10,989 passed, 578 skipped**. Three
failures —
`test_graceful_shutdown.py::test_run_server_installs_cancelled_error_filter`
(full-suite ordering; passes in isolation),
`test_learn/test_integration.py::TestCodexIntegration::test_full_pipeline`,
and `test_release_workflows.py::test_no_native_tls_in_wheel_build_tree`
(needs `cargo`) — **reproduce identically on clean `main`** (`2f4d001c`,
10,967 passed, same 3 failed). Verified by stashing this branch and
re-running the full suite on main in the same tree.
**What I did not test:** a live provider (upstream is `respx`-mocked);
the Gemini `bind_scope` fix against real Google traffic (covered by the
existing 114 Gemini tests, which all pass); the dashboard rendered in a
browser — I verified the JSON shape its templates bind to
(`stats.savings?.by_source`, `stats.pipeline_timing`) rather than the
pixels.
---
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Joseph Benno
|
2f4d001c9f |
fix(proxy): keep prefixed core tools resident (#3046)
## Description Headroom's Tool Search deferral lowercased core tool names but did not account for client namespace prefixes. Oh My Pi sends built-ins such as `_read`, `_edit`, `_write`, and `_bash`, so those core tools were incorrectly marked `defer_loading=True`. This change centralizes resident-name normalization for both the Anthropic and OpenAI paths. It lowercases names and removes only leading underscores, preserving internal separators such as `mcp__server__read` so unrelated tools do not become resident. Closes #3031 ## 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 - Added a shared resident-tool name normalizer in `headroom/proxy/helpers.py`. - Applied the same normalization to Anthropic and OpenAI Tool Search deferral. - Added a regression test for Oh My Pi's exact 12-tool surface at the deferral threshold. - Added OpenAI coverage for prefixed resident tools and negative namespace cases. ## Testing <!-- Check what you actually ran, then paste the real command output below. --> - [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 ### Test Output ```text $ uv run --no-sync pytest --noconftest -q tests/test_openai_tool_search_deferral.py tests/test_issue_746_tool_search.py -k 'not normalize_tool_search_mode and not configure_' 72 passed, 23 deselected in 0.25s $ uv run --no-sync ruff check . All checks passed! $ uv run --no-sync ruff format --check . 1499 files already formatted $ UV_CACHE_DIR=/tmp/headroom-uv-cache uv run --no-sync mypy headroom Success: no issues found in 520 source files ``` ## Real Behavior Proof - Environment: Linux x86_64 sandbox; Python 3.12.13; uv 0.11.33; no provider credentials. - Exact command / steps: Exercised the exact 12-tool Oh My Pi fixture through the Anthropic deferral helper and prefixed resident plus negative names through the OpenAI helper. - Observed result: Anthropic kept `_edit`, `_task`, `_read`, `_bash`, `_glob`, `_grep`, `_write`, `computer`, and `web_search` resident while deferring `_hub`, `_todo`, and `_eval`. OpenAI kept prefixed core tools resident while `mcp__server__read` and `terminal_helper` remained deferred. - Not tested: Live Oh My Pi traffic against Anthropic, provider E2E tests, and the full native-backed pytest suite. ## Runtime Rollout Safety - Rollout-managed feature(s): Existing server-side Tool Search deferral for Anthropic and OpenAI. - Minimum rollout channel: N/A; targeted bug fix to existing behavior. - Stable/default behavior changed: Yes. Leading-underscore names that normalize to known resident names now remain resident. - Kill switch / disable path: Set `HEADROOM_TOOL_SEARCH=0`. - Unsafe override required: No. - Qualification impact: Prefixed core tools remain immediately available; non-core and MCP namespace behavior is unchanged. - Rollback path: Revert this commit or disable Tool Search with `HEADROOM_TOOL_SEARCH=0`. ## 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 - [ ] 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) ## Screenshots (if applicable) N/A ## Additional Notes |
||
|
dependabot[bot]
|
322425c43b |
deps: bump sha2 from 0.10.9 to 0.11.0 (#2288)
Bumps [sha2](https://github.com/RustCrypto/hashes) from 0.10.9 to 0.11.0. <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/RustCrypto/hashes/commit/ffe093984c004769747e998f77da8ff7c0e7a765"><code>ffe0939</code></a> Release sha2 0.11.0 (<a href="https://redirect.github.com/RustCrypto/hashes/issues/806">#806</a>)</li> <li><a href="https://github.com/RustCrypto/hashes/commit/8991b65fe400c31c4cc189510f86ae642c470cd9"><code>8991b65</code></a> Use the standard order of the <code>[package]</code> section fields (<a href="https://redirect.github.com/RustCrypto/hashes/issues/807">#807</a>)</li> <li><a href="https://github.com/RustCrypto/hashes/commit/3d2bc57db40fd6aeb25d6c6da98d67e2784c2985"><code>3d2bc57</code></a> sha2: refactor backends (<a href="https://redirect.github.com/RustCrypto/hashes/issues/802">#802</a>)</li> <li><a href="https://github.com/RustCrypto/hashes/commit/faa55fb83697c8f3113636d88070e5f5edc8c335"><code>faa55fb</code></a> sha3: bump <code>keccak</code> to v0.2 (<a href="https://redirect.github.com/RustCrypto/hashes/issues/803">#803</a>)</li> <li><a href="https://github.com/RustCrypto/hashes/commit/d3e6489e56f8486d4a93ceb7a8abf4924af1de7b"><code>d3e6489</code></a> sha3 v0.11.0-rc.9 (<a href="https://redirect.github.com/RustCrypto/hashes/issues/801">#801</a>)</li> <li><a href="https://github.com/RustCrypto/hashes/commit/bbf6f51ff97f81ab15e6e5f6cf878bfbcb1f47c8"><code>bbf6f51</code></a> sha2: tweak backend docs (<a href="https://redirect.github.com/RustCrypto/hashes/issues/800">#800</a>)</li> <li><a href="https://github.com/RustCrypto/hashes/commit/155dbbf2959dbec0ec75948a82590ddaede2d3bc"><code>155dbbf</code></a> sha3: add default value for the <code>DS</code> generic parameter on <code>TurboShake128/256</code>...</li> <li><a href="https://github.com/RustCrypto/hashes/commit/ed514f2b34526683b3b7c41670f1887982c3df64"><code>ed514f2</code></a> Use published version of <code>keccak</code> v0.2 (<a href="https://redirect.github.com/RustCrypto/hashes/issues/799">#799</a>)</li> <li><a href="https://github.com/RustCrypto/hashes/commit/702bcd83735a49c928c0fc24506924f5c0aa22af"><code>702bcd8</code></a> Migrate to closure-based <code>keccak</code> (<a href="https://redirect.github.com/RustCrypto/hashes/issues/796">#796</a>)</li> <li><a href="https://github.com/RustCrypto/hashes/commit/827c043f82d57666a0b146d156e91c39535c1305"><code>827c043</code></a> sha3 v0.11.0-rc.8 (<a href="https://redirect.github.com/RustCrypto/hashes/issues/794">#794</a>)</li> <li>Additional commits viewable in <a href="https://github.com/RustCrypto/hashes/compare/sha2-v0.10.9...sha2-v0.11.0">compare view</a></li> </ul> </details> <br /> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
dependabot[bot]
|
5731be7e68 |
deps: bump axum from 0.7.9 to 0.8.9 (#2966)
Bumps [axum](https://github.com/tokio-rs/axum) from 0.7.9 to 0.8.9. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/tokio-rs/axum/releases">axum's releases</a>.</em></p> <blockquote> <h2>axum-v0.8.9</h2> <ul> <li><strong>added:</strong> <code>WebSocketUpgrade::{requested_protocols, set_selected_protocol}</code> for more flexible subprotocol selection (<a href="https://redirect.github.com/tokio-rs/axum/issues/3597">#3597</a>)</li> <li><strong>changed:</strong> Update minimum rust version to 1.80 (<a href="https://redirect.github.com/tokio-rs/axum/issues/3620">#3620</a>)</li> <li><strong>fixed:</strong> Set connect endpoint on correct field in MethodRouter (<a href="https://redirect.github.com/tokio-rs/axum/issues/3656">#3656</a>)</li> <li><strong>fixed:</strong> Return specific error message when multipart body limit is exceeded (<a href="https://redirect.github.com/tokio-rs/axum/issues/3611">#3611</a>)</li> </ul> <p><a href="https://redirect.github.com/tokio-rs/axum/issues/3597">#3597</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3597">tokio-rs/axum#3597</a> <a href="https://redirect.github.com/tokio-rs/axum/issues/3620">#3620</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3620">tokio-rs/axum#3620</a> <a href="https://redirect.github.com/tokio-rs/axum/issues/3656">#3656</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3656">tokio-rs/axum#3656</a> <a href="https://redirect.github.com/tokio-rs/axum/issues/3611">#3611</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3611">tokio-rs/axum#3611</a></p> <h2>axum v0.8.8</h2> <ul> <li>Clarify documentation for <code>Router::route_layer</code> (<a href="https://redirect.github.com/tokio-rs/axum/issues/3567">#3567</a>)</li> </ul> <p><a href="https://redirect.github.com/tokio-rs/axum/issues/3567">#3567</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3567">tokio-rs/axum#3567</a></p> <h2>axum v0.8.7</h2> <ul> <li>Relax implicit <code>Send</code> / <code>Sync</code> bounds on <code>RouterAsService</code>, <code>RouterIntoService</code> (<a href="https://redirect.github.com/tokio-rs/axum/issues/3555">#3555</a>)</li> <li>Make it easier to visually scan for default features (<a href="https://redirect.github.com/tokio-rs/axum/issues/3550">#3550</a>)</li> <li>Fix some documentation typos</li> </ul> <p><a href="https://redirect.github.com/tokio-rs/axum/issues/3550">#3550</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3550">tokio-rs/axum#3550</a> <a href="https://redirect.github.com/tokio-rs/axum/issues/3555">#3555</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3555">tokio-rs/axum#3555</a></p> <h2>axum v0.8.5</h2> <ul> <li><strong>fixed:</strong> Reject JSON request bodies with trailing characters after the JSON document (<a href="https://redirect.github.com/tokio-rs/axum/issues/3453">#3453</a>)</li> <li><strong>added:</strong> Implement <code>OptionalFromRequest</code> for <code>Multipart</code> (<a href="https://redirect.github.com/tokio-rs/axum/issues/3220">#3220</a>)</li> <li><strong>added:</strong> Getter methods <code>Location::{status_code, location}</code></li> <li><strong>added:</strong> Support for writing arbitrary binary data into server-sent events (<a href="https://redirect.github.com/tokio-rs/axum/issues/3425">#3425</a>)]</li> <li><strong>added:</strong> <code>middleware::ResponseAxumBodyLayer</code> for mapping response body to <code>axum::body::Body</code> (<a href="https://redirect.github.com/tokio-rs/axum/issues/3469">#3469</a>)</li> <li><strong>added:</strong> <code>impl FusedStream for WebSocket</code> (<a href="https://redirect.github.com/tokio-rs/axum/issues/3443">#3443</a>)</li> <li><strong>changed:</strong> The <code>sse</code> module and <code>Sse</code> type no longer depend on the <code>tokio</code> feature (<a href="https://redirect.github.com/tokio-rs/axum/issues/3154">#3154</a>)</li> <li><strong>changed:</strong> If the location given to one of <code>Redirect</code>s constructors is not a valid header value, instead of panicking on construction, the <code>IntoResponse</code> impl now returns an HTTP 500, just like <code>Json</code> does when serialization fails (<a href="https://redirect.github.com/tokio-rs/axum/issues/3377">#3377</a>)</li> <li><strong>changed:</strong> Update minimum rust version to 1.78 (<a href="https://redirect.github.com/tokio-rs/axum/issues/3412">#3412</a>)</li> </ul> <p><a href="https://redirect.github.com/tokio-rs/axum/issues/3154">#3154</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3154">tokio-rs/axum#3154</a> <a href="https://redirect.github.com/tokio-rs/axum/issues/3220">#3220</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3220">tokio-rs/axum#3220</a> <a href="https://redirect.github.com/tokio-rs/axum/issues/3377">#3377</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3377">tokio-rs/axum#3377</a> <a href="https://redirect.github.com/tokio-rs/axum/issues/3412">#3412</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3412">tokio-rs/axum#3412</a> <a href="https://redirect.github.com/tokio-rs/axum/issues/3425">#3425</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3425">tokio-rs/axum#3425</a> <a href="https://redirect.github.com/tokio-rs/axum/issues/3443">#3443</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3443">tokio-rs/axum#3443</a> <a href="https://redirect.github.com/tokio-rs/axum/issues/3453">#3453</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3453">tokio-rs/axum#3453</a> <a href="https://redirect.github.com/tokio-rs/axum/issues/3469">#3469</a>: <a href="https://redirect.github.com/tokio-rs/axum/pull/3469">tokio-rs/axum#3469</a></p> <h2>axum v0.8.4</h2> <ul> <li><strong>added:</strong> <code>Router::reset_fallback</code> (<a href="https://redirect.github.com/tokio-rs/axum/issues/3320">#3320</a>)</li> <li><strong>added:</strong> <code>WebSocketUpgrade::selected_protocol</code> (<a href="https://redirect.github.com/tokio-rs/axum/issues/3248">#3248</a>)</li> <li><strong>fixed:</strong> Panic location for overlapping method routes (<a href="https://redirect.github.com/tokio-rs/axum/issues/3319">#3319</a>)</li> <li><strong>fixed:</strong> Don't leak a tokio task when using <code>serve</code> without graceful shutdown (<a href="https://redirect.github.com/tokio-rs/axum/issues/3129">#3129</a>)</li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/tokio-rs/axum/commit/c59208c86fded335cd85e388030ad59347b0e5ae"><code>c59208c</code></a> revert axum-core changelog changes</li> <li><a href="https://github.com/tokio-rs/axum/commit/99068f5a4b309d0966777eb6e5a8ce924f204e6d"><code>99068f5</code></a> Revert "Fix <code>IntoResponse</code> for tuples overriding error response codes (<a href="https://redirect.github.com/tokio-rs/axum/issues/3603">#3603</a>)"</li> <li><a href="https://github.com/tokio-rs/axum/commit/23d7098691871ccec71ca17ea31d1d40b036c0d0"><code>23d7098</code></a> Revert "axum-core 0.5.6"</li> <li><a href="https://github.com/tokio-rs/axum/commit/e8a39ad416d1ee4f61249904309691909db2db09"><code>e8a39ad</code></a> axum-macros 0.5.1</li> <li><a href="https://github.com/tokio-rs/axum/commit/6e9a249a4fa45507b1157e570f9b6ec58d71cb86"><code>6e9a249</code></a> axum-extra 0.12.6</li> <li><a href="https://github.com/tokio-rs/axum/commit/0ec9041a1b903778a91a23558e064a83b43674c1"><code>0ec9041</code></a> axum 0.8.9</li> <li><a href="https://github.com/tokio-rs/axum/commit/c3fcebb38f356ccf96da158199d4e920aa8cfda3"><code>c3fcebb</code></a> axum-core 0.5.6</li> <li><a href="https://github.com/tokio-rs/axum/commit/a8790fc29b0db5708cdbcae70597d37c5afe1143"><code>a8790fc</code></a> update release notes</li> <li><a href="https://github.com/tokio-rs/axum/commit/26ba7bb6f21cf8996493481a5275c01152f0aaf9"><code>26ba7bb</code></a> docs: consolidate state management docs in crate root (<a href="https://redirect.github.com/tokio-rs/axum/issues/3683">#3683</a>)</li> <li><a href="https://github.com/tokio-rs/axum/commit/9fc59efc1fa9a11f4157cff1f2d22355f01d7bc0"><code>9fc59ef</code></a> Update to tokio-tungstenite 0.29 (<a href="https://redirect.github.com/tokio-rs/axum/issues/3689">#3689</a>)</li> <li>Additional commits viewable in <a href="https://github.com/tokio-rs/axum/compare/axum-v0.7.9...axum-v0.8.9">compare view</a></li> </ul> </details> <br /> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
dependabot[bot]
|
ff17961cd7 |
deps: bump ruff from 0.15.22 to 0.16.2 in the pip-minor-patch group across 1 directory (#2962)
Bumps the pip-minor-patch group with 1 update in the / directory: [ruff](https://github.com/astral-sh/ruff). Updates `ruff` from 0.15.22 to 0.16.2 <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/astral-sh/ruff/releases">ruff's releases</a>.</em></p> <blockquote> <h2>0.16.2</h2> <h2>Release Notes</h2> <p>Released on 2026-08-06.</p> <h3>Bug fixes</h3> <ul> <li>[<code>flake8-pyi</code>] Avoid false positives on <code>singledispatch</code> functions (<code>PYI041</code>) (<a href="https://redirect.github.com/astral-sh/ruff/pull/27335">#27335</a>)</li> </ul> <h3>Server</h3> <ul> <li>Register formatting capabilities dynamically to exclude TOML files (<a href="https://redirect.github.com/astral-sh/ruff/pull/27332">#27332</a>)</li> </ul> <h3>Contributors</h3> <ul> <li><a href="https://github.com/MeGaGiGaGon"><code>@MeGaGiGaGon</code></a></li> <li><a href="https://github.com/charliermarsh"><code>@charliermarsh</code></a></li> <li><a href="https://github.com/epage"><code>@epage</code></a></li> <li><a href="https://github.com/sharkdp"><code>@sharkdp</code></a></li> <li><a href="https://github.com/ntBre"><code>@ntBre</code></a></li> </ul> <h2>Install ruff 0.16.2</h2> <h3>Install prebuilt binaries via shell script</h3> <pre lang="sh"><code>curl --proto '=https' --tlsv1.2 -LsSf https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-installer.sh | sh </code></pre> <h3>Install prebuilt binaries via powershell script</h3> <pre lang="sh"><code>powershell -ExecutionPolicy Bypass -c "irm https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-installer.ps1 | iex" </code></pre> <h2>Download ruff 0.16.2</h2> <table> <thead> <tr> <th>File</th> <th>Platform</th> <th>Checksum</th> </tr> </thead> <tbody> <tr> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-aarch64-apple-darwin.tar.gz">ruff-aarch64-apple-darwin.tar.gz</a></td> <td>Apple Silicon macOS</td> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-aarch64-apple-darwin.tar.gz.sha256">checksum</a></td> </tr> <tr> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-x86_64-apple-darwin.tar.gz">ruff-x86_64-apple-darwin.tar.gz</a></td> <td>Intel macOS</td> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-x86_64-apple-darwin.tar.gz.sha256">checksum</a></td> </tr> <tr> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-aarch64-pc-windows-msvc.zip">ruff-aarch64-pc-windows-msvc.zip</a></td> <td>ARM64 Windows</td> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-aarch64-pc-windows-msvc.zip.sha256">checksum</a></td> </tr> <tr> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-i686-pc-windows-msvc.zip">ruff-i686-pc-windows-msvc.zip</a></td> <td>x86 Windows</td> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-i686-pc-windows-msvc.zip.sha256">checksum</a></td> </tr> <tr> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-x86_64-pc-windows-msvc.zip">ruff-x86_64-pc-windows-msvc.zip</a></td> <td>x64 Windows</td> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-x86_64-pc-windows-msvc.zip.sha256">checksum</a></td> </tr> <tr> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-aarch64-unknown-linux-gnu.tar.gz">ruff-aarch64-unknown-linux-gnu.tar.gz</a></td> <td>ARM64 Linux</td> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-aarch64-unknown-linux-gnu.tar.gz.sha256">checksum</a></td> </tr> <tr> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-i686-unknown-linux-gnu.tar.gz">ruff-i686-unknown-linux-gnu.tar.gz</a></td> <td>x86 Linux</td> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-i686-unknown-linux-gnu.tar.gz.sha256">checksum</a></td> </tr> <tr> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-powerpc64-unknown-linux-gnu.tar.gz">ruff-powerpc64-unknown-linux-gnu.tar.gz</a></td> <td>PPC64 Linux</td> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-powerpc64-unknown-linux-gnu.tar.gz.sha256">checksum</a></td> </tr> <tr> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-powerpc64le-unknown-linux-gnu.tar.gz">ruff-powerpc64le-unknown-linux-gnu.tar.gz</a></td> <td>PPC64LE Linux</td> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-powerpc64le-unknown-linux-gnu.tar.gz.sha256">checksum</a></td> </tr> <tr> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-riscv64gc-unknown-linux-gnu.tar.gz">ruff-riscv64gc-unknown-linux-gnu.tar.gz</a></td> <td>RISCV Linux</td> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-riscv64gc-unknown-linux-gnu.tar.gz.sha256">checksum</a></td> </tr> <tr> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-s390x-unknown-linux-gnu.tar.gz">ruff-s390x-unknown-linux-gnu.tar.gz</a></td> <td>S390x Linux</td> <td><a href="https://releases.astral.sh/github/ruff/releases/download/0.16.2/ruff-s390x-unknown-linux-gnu.tar.gz.sha256">checksum</a></td> </tr> </tbody> </table> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md">ruff's changelog</a>.</em></p> <blockquote> <h2>0.16.2</h2> <p>Released on 2026-08-06.</p> <h3>Bug fixes</h3> <ul> <li>[<code>flake8-pyi</code>] Avoid false positives on <code>singledispatch</code> functions (<code>PYI041</code>) (<a href="https://redirect.github.com/astral-sh/ruff/pull/27335">#27335</a>)</li> </ul> <h3>Server</h3> <ul> <li>Register formatting capabilities dynamically to exclude TOML files (<a href="https://redirect.github.com/astral-sh/ruff/pull/27332">#27332</a>)</li> </ul> <h3>Contributors</h3> <ul> <li><a href="https://github.com/MeGaGiGaGon"><code>@MeGaGiGaGon</code></a></li> <li><a href="https://github.com/charliermarsh"><code>@charliermarsh</code></a></li> <li><a href="https://github.com/epage"><code>@epage</code></a></li> <li><a href="https://github.com/sharkdp"><code>@sharkdp</code></a></li> <li><a href="https://github.com/ntBre"><code>@ntBre</code></a></li> </ul> <h2>0.16.1</h2> <p>Released on 2026-07-30.</p> <h3>Preview features</h3> <ul> <li>Add an option to opt out of human-readable names (<a href="https://redirect.github.com/astral-sh/ruff/pull/27160">#27160</a>)</li> <li>[<code>flake8-pytest-style</code>] Make fixes safe by default and unsafe only when comments are present (<code>PT018</code>) (<a href="https://redirect.github.com/astral-sh/ruff/pull/27201">#27201</a>)</li> <li>[<code>pyupgrade</code>] Skip fix when a defaulted <code>TypeVar</code> precedes a non-defaulted one (<code>UP040</code>, <code>UP046</code>, <code>UP047</code>) (<a href="https://redirect.github.com/astral-sh/ruff/pull/27133">#27133</a>)</li> <li>[<code>ruff</code>] Fix false positive with unpacked arguments (<code>RUF065</code>) (<a href="https://redirect.github.com/astral-sh/ruff/pull/26959">#26959</a>)</li> </ul> <h3>Bug fixes</h3> <ul> <li>Bump <code>gen-lsp-types</code> to gracefully handle unknown enumeration values in LSP messages (<a href="https://redirect.github.com/astral-sh/ruff/pull/27230">#27230</a>)</li> <li>[<code>flake8-bugbear</code>] Mark <code>range</code> as immutable (<code>B008</code>) (<a href="https://redirect.github.com/astral-sh/ruff/pull/27247">#27247</a>)</li> <li>[<code>flake8-comprehensions</code>] NFKC-normalize keyword names in <code>C408</code> fix (<a href="https://redirect.github.com/astral-sh/ruff/pull/26813">#26813</a>)</li> <li>[<code>flake8-return</code>] Fix false positive when variable is read in <code>finally</code> clause (<code>RET504</code>) (<a href="https://redirect.github.com/astral-sh/ruff/pull/25441">#25441</a>)</li> <li>[<code>pydocstyle</code>] Skip section detection inside RST directive bodies (<code>D214</code>, <code>D405</code>, <code>D413</code>) (<a href="https://redirect.github.com/astral-sh/ruff/pull/23635">#23635</a>)</li> <li>[<code>refurb</code>] Parenthesize <code>yield</code> arguments in the <code>FURB192</code> fix (<a href="https://redirect.github.com/astral-sh/ruff/pull/27192">#27192</a>)</li> </ul> <h3>Rule changes</h3> <ul> <li>[<code>flake8-pytest-style</code>] Mark <code>PT022</code> fixes as unsafe (<a href="https://redirect.github.com/astral-sh/ruff/pull/26440">#26440</a>)</li> <li>[<code>refurb</code>] Mark fixes that remove unknown separators as unsafe (<code>FURB105</code>) (<a href="https://redirect.github.com/astral-sh/ruff/pull/27200">#27200</a>)</li> </ul> <h3>Server</h3> <ul> <li>Fix indexing of excluded nested Ruff workspaces (<a href="https://redirect.github.com/astral-sh/ruff/pull/27303">#27303</a>)</li> <li>Lint TOML files in the LSP (<a href="https://redirect.github.com/astral-sh/ruff/pull/26862">#26862</a>)</li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/astral-sh/ruff/commit/5b48a040974781ba90b47c8df628f8fd9b6c95dd"><code>5b48a04</code></a> Bump 0.16.2 (<a href="https://redirect.github.com/astral-sh/ruff/issues/27555">#27555</a>)</li> <li><a href="https://github.com/astral-sh/ruff/commit/1b9e5fc483b95a01fe02ff104820280b1b32e8ae"><code>1b9e5fc</code></a> Update Swatinem/rust-cache action to v2.9.2 (<a href="https://redirect.github.com/astral-sh/ruff/issues/27568">#27568</a>)</li> <li><a href="https://github.com/astral-sh/ruff/commit/c4e86fc0394c92a9334ba2eb026c77c21db403be"><code>c4e86fc</code></a> [ty] Add helper extension methods for half-range and equality constraints (<a href="https://redirect.github.com/astral-sh/ruff/issues/2">#2</a>...</li> <li><a href="https://github.com/astral-sh/ruff/commit/17a00de2e298612201a8fe30790e9399204af1b9"><code>17a00de</code></a> [ty] Reuse primer commands in memory reports (<a href="https://redirect.github.com/astral-sh/ruff/issues/27553">#27553</a>)</li> <li><a href="https://github.com/astral-sh/ruff/commit/6ea296b96923e142eb13af2bc6ad261c280d8eb1"><code>6ea296b</code></a> [ty] Normalize type labels in structured docstrings (<a href="https://redirect.github.com/astral-sh/ruff/issues/26923">#26923</a>)</li> <li><a href="https://github.com/astral-sh/ruff/commit/2fc445f0053f4ec27c717fae0de3671d73c103be"><code>2fc445f</code></a> [ty] Diagnose invalid <strong>getattr</strong> calls (<a href="https://redirect.github.com/astral-sh/ruff/issues/27502">#27502</a>)</li> <li><a href="https://github.com/astral-sh/ruff/commit/22c7823c4e8bffcca97688d8438c9b567d6817d8"><code>22c7823</code></a> [ty] Enable (but downrank) auto-import completion suggestions from stub-only ...</li> <li><a href="https://github.com/astral-sh/ruff/commit/05160d507f05345a72db9c28ab4edf7c92334819"><code>05160d5</code></a> [ty] Diagnose invalid descriptor <code>__get__</code> calls (<a href="https://redirect.github.com/astral-sh/ruff/issues/27400">#27400</a>)</li> <li><a href="https://github.com/astral-sh/ruff/commit/baea3d0dcec6d6f6d1659321940f3725771c5f45"><code>baea3d0</code></a> [ty] Expose strict analysis options in the playground (<a href="https://redirect.github.com/astral-sh/ruff/issues/27543">#27543</a>)</li> <li><a href="https://github.com/astral-sh/ruff/commit/c88946ebeb92be6d276087f0d528cd6471df4ead"><code>c88946e</code></a> [ty] Bump ecosystem-analyzer for strict project settings (<a href="https://redirect.github.com/astral-sh/ruff/issues/27542">#27542</a>)</li> <li>Additional commits viewable in <a href="https://github.com/astral-sh/ruff/compare/0.15.22...0.16.2">compare view</a></li> </ul> </details> <br /> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
dependabot[bot]
|
bbe901319d |
deps: bump tokio-tungstenite from 0.24.0 to 0.30.0 (#2967)
Bumps [tokio-tungstenite](https://github.com/snapview/tokio-tungstenite) from 0.24.0 to 0.30.0. <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/snapview/tokio-tungstenite/blob/master/CHANGELOG.md">tokio-tungstenite's changelog</a>.</em></p> <blockquote> <h1>0.30.0</h1> <ul> <li>Update <code>tungstenite</code> to <code>0.30.0</code>. See <a href="https://github.com/snapview/tungstenite-rs/blob/master/CHANGELOG.md"><code>tungstenite</code> release</a>.</li> </ul> <h1>0.29.0</h1> <ul> <li>Update <code>tungstenite</code> to <code>0.29.0</code>. See <a href="https://github.com/snapview/tungstenite-rs/blob/master/CHANGELOG.md"><code>tungstenite</code> release</a>.</li> </ul> <h1>0.28.0</h1> <ul> <li>Update <code>tungstenite</code> to <code>0.28.0</code>. See <a href="https://github.com/snapview/tungstenite-rs/blob/master/CHANGELOG.md"><code>tungstenite</code> release</a>.</li> </ul> <h1>0.27.0</h1> <ul> <li>See <a href="https://github.com/snapview/tungstenite-rs/blob/master/CHANGELOG.md#0270">performance updates in <code>tungstenite-rs</code></a>.</li> </ul> <h1>0.26.2</h1> <ul> <li>Update <code>tungstenite</code>, see <a href="https://github.com/snapview/tungstenite-rs/blob/master/CHANGELOG.md#0262">changes here</a>.</li> </ul> <h1>0.26.1</h1> <ul> <li>Update <code>tungstenite</code> to address an issue that might cause UB in certain cases.</li> </ul> <h1>0.26.0</h1> <ul> <li>Update <code>tungstenite</code> to <code>0.26.0</code> (<a href="https://github.com/snapview/tungstenite-rs/blob/master/CHANGELOG.md#0260">breaking changes</a>).</li> </ul> <h1>0.25.0</h1> <ul> <li>Update <code>tungstenite</code> to <code>0.25.0</code> (<a href="https://github.com/snapview/tungstenite-rs/blob/master/CHANGELOG.md#0250">important updates!</a>).</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/snapview/tokio-tungstenite/commit/4994a078037a18a9347b8cf49291a5031d63d9e0"><code>4994a07</code></a> Bump version</li> <li><a href="https://github.com/snapview/tokio-tungstenite/commit/753ca72690919485a1aa1f0f69a336b1152fb0ae"><code>753ca72</code></a> Document cancel safety of reading from WebSocketStream (<a href="https://redirect.github.com/snapview/tokio-tungstenite/issues/378">#378</a>)</li> <li><a href="https://github.com/snapview/tokio-tungstenite/commit/751d7e2bc26e5de302f4a79907b6949bf00e0043"><code>751d7e2</code></a> Update version number listed in Readme (<a href="https://redirect.github.com/snapview/tokio-tungstenite/issues/375">#375</a>)</li> <li><a href="https://github.com/snapview/tokio-tungstenite/commit/57fc3d0276564efaf11efe8c1499b26b44dbe9a4"><code>57fc3d0</code></a> docs(CHANGELOG.md): fix <code>tungstenite</code> versions (<a href="https://redirect.github.com/snapview/tokio-tungstenite/issues/374">#374</a>)</li> <li><a href="https://github.com/snapview/tokio-tungstenite/commit/7930ff2f825a69cad44b928b19b6fb81bffc3f7a"><code>7930ff2</code></a> Bump version</li> <li><a href="https://github.com/snapview/tokio-tungstenite/commit/38d04656fe28be0000920201d6a49bf5ec3d537b"><code>38d0465</code></a> Update Readme (<a href="https://redirect.github.com/snapview/tokio-tungstenite/issues/369">#369</a>)</li> <li><a href="https://github.com/snapview/tokio-tungstenite/commit/35d110c24c9d030d1608ec964d70c789dfb27452"><code>35d110c</code></a> Implement into_inner to get the underlying stream (<a href="https://redirect.github.com/snapview/tokio-tungstenite/issues/367">#367</a>)</li> <li><a href="https://github.com/snapview/tokio-tungstenite/commit/f3ae75d1de14a4d25869b5ffa771ea3da012904b"><code>f3ae75d</code></a> Update <code>tungstenite</code> version and fix bugs</li> <li><a href="https://github.com/snapview/tokio-tungstenite/commit/25b544e43fe979bca951f085ee1b66e9c1cc3113"><code>25b544e</code></a> Allow getting a reference to the shared inner stream (<a href="https://redirect.github.com/snapview/tokio-tungstenite/issues/363">#363</a>)</li> <li><a href="https://github.com/snapview/tokio-tungstenite/commit/e855f9eb8c88daf230a9ddc6db35603e2b601e8b"><code>e855f9e</code></a> Fix errors in the examples caused by <code>Utf8Error</code></li> <li>Additional commits viewable in <a href="https://github.com/snapview/tokio-tungstenite/compare/v0.24.0...v0.30.0">compare view</a></li> </ul> </details> <br /> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
dependabot[bot]
|
888a9f4e14 |
deps: bump the cargo-minor-patch group across 1 directory with 4 updates (#2964)
Bumps the cargo-minor-patch group with 4 updates in the / directory: [aws-config](https://github.com/smithy-lang/smithy-rs), [rusqlite](https://github.com/rusqlite/rusqlite), [async-trait](https://github.com/dtolnay/async-trait) and [cc](https://github.com/rust-lang/cc-rs). Updates `aws-config` from 1.10.0 to 1.10.1 <details> <summary>Commits</summary> <ul> <li>See full diff in <a href="https://github.com/smithy-lang/smithy-rs/commits">compare view</a></li> </ul> </details> <br /> Updates `rusqlite` from 0.40.1 to 0.40.2 <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/rusqlite/rusqlite/releases">rusqlite's releases</a>.</em></p> <blockquote> <h2>0.40.2</h2> <h2>What's Changed</h2> <ul> <li>Lower MSRV to 1.88.0</li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/rusqlite/rusqlite/compare/v0.40.1...v0.40.2">https://github.com/rusqlite/rusqlite/compare/v0.40.1...v0.40.2</a></p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/rusqlite/rusqlite/commit/e88f112bef7899234a497baed5cc3c3d553deeb8"><code>e88f112</code></a> Prepare release</li> <li><a href="https://github.com/rusqlite/rusqlite/commit/d11c76e7d7e20eb8e22ede9250407187bd3f22e3"><code>d11c76e</code></a> Update main.yml</li> <li><a href="https://github.com/rusqlite/rusqlite/commit/c922ca5b716b5a226df6eba9eea84c6320c60311"><code>c922ca5</code></a> Lower MSRV to 1.88.0</li> <li>See full diff in <a href="https://github.com/rusqlite/rusqlite/compare/v0.40.1...v0.40.2">compare view</a></li> </ul> </details> <br /> Updates `async-trait` from 0.1.91 to 0.1.92 <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/dtolnay/async-trait/releases">async-trait's releases</a>.</em></p> <blockquote> <h2>0.1.92</h2> <ul> <li>Resolve double_must_use clippy lint in generated code (<a href="https://redirect.github.com/dtolnay/async-trait/issues/303">#303</a>)</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/dtolnay/async-trait/commit/82e7e9edd60f622294373a23c0ce9c0077ad0263"><code>82e7e9e</code></a> Release 0.1.92</li> <li><a href="https://github.com/dtolnay/async-trait/commit/9a35cb87f9366cd992bbc00d430e1b5fe1aa0cdd"><code>9a35cb8</code></a> Merge pull request <a href="https://redirect.github.com/dtolnay/async-trait/issues/303">#303</a> from dtolnay/mustuse</li> <li><a href="https://github.com/dtolnay/async-trait/commit/875ceecb100bab2cf369178633b4791336d92b75"><code>875ceec</code></a> Resolve double_must_use clippy lint</li> <li><a href="https://github.com/dtolnay/async-trait/commit/62993a57bc6a8d5bd3de23fbae48cede333cb925"><code>62993a5</code></a> Raise minimum tested compiler to rust 1.88</li> <li>See full diff in <a href="https://github.com/dtolnay/async-trait/compare/0.1.91...0.1.92">compare view</a></li> </ul> </details> <br /> Updates `cc` from 1.4.1 to 1.4.2 <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/rust-lang/cc-rs/releases">cc's releases</a>.</em></p> <blockquote> <h2>cc-v1.4.2</h2> <h3>Fixed</h3> <ul> <li>Infer NEON, not VFPv4, from <code>neon</code> in the target name (<a href="https://redirect.github.com/rust-lang/cc-rs/pull/1843">#1843</a>)</li> <li>do not emit <code>-mno-omit-leaf-frame-pointer</code> if unsupported (<a href="https://redirect.github.com/rust-lang/cc-rs/pull/1845">#1845</a>)</li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/rust-lang/cc-rs/blob/main/CHANGELOG.md">cc's changelog</a>.</em></p> <blockquote> <h2><a href="https://github.com/rust-lang/cc-rs/compare/cc-v1.4.1...cc-v1.4.2">1.4.2</a> - 2026-08-08</h2> <h3>Fixed</h3> <ul> <li>Infer NEON, not VFPv4, from <code>neon</code> in the target name (<a href="https://redirect.github.com/rust-lang/cc-rs/pull/1843">#1843</a>)</li> <li>do not emit <code>-mno-omit-leaf-frame-pointer</code> if unsupported (<a href="https://redirect.github.com/rust-lang/cc-rs/pull/1845">#1845</a>)</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/rust-lang/cc-rs/commit/a91e05ec40f26d4637d4bff9e9764221d0a59dd8"><code>a91e05e</code></a> chore(cc): release v1.4.2 (<a href="https://redirect.github.com/rust-lang/cc-rs/issues/1846">#1846</a>)</li> <li><a href="https://github.com/rust-lang/cc-rs/commit/0e91755354dbfe073fbe5f88aee427b75cc1c06f"><code>0e91755</code></a> fix: Infer NEON, not VFPv4, from <code>neon</code> in the target name (<a href="https://redirect.github.com/rust-lang/cc-rs/issues/1843">#1843</a>)</li> <li><a href="https://github.com/rust-lang/cc-rs/commit/c20feddb4f25567c488478cf12e1d95cbb7eef87"><code>c20fedd</code></a> do not emit -mno-omit-leaf-frame-pointer if unsupported (<a href="https://redirect.github.com/rust-lang/cc-rs/issues/1845">#1845</a>)</li> <li>See full diff in <a href="https://github.com/rust-lang/cc-rs/compare/cc-v1.4.1...cc-v1.4.2">compare view</a></li> </ul> </details> <br /> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
JD Davis
|
2d88e31a40 |
fix(claude): reject conflicting auth before proxy startup (#2993)
## Description Fixes #1443. Claude Code rejects an effective configuration containing both ANTHROPIC_API_KEY and ANTHROPIC_AUTH_TOKEN before any request reaches Headroom. The existing wrapper started the proxy and mutated project settings before Claude surfaced its generic Invalid API key message, leaving users to guess which credential came from their shell, global settings, or project settings. Headroom does not own either credential, and both represent legitimate but different auth/billing modes, so automatically deleting one would be destructive. This PR detects the contradiction before any proxy/config mutation and tells the user which source contains each key without exposing credential values. ## Changes Made - Add a pure Claude auth-conflict classifier with explicit settings-layer precedence. - Cover user settings, project .claude/settings.json, project .claude/settings.local.json, and shell environment. - Treat higher-precedence empty values as clearing inherited credentials. - Abort wrap claude before proxy registration/startup when both keys remain effective. - Add a headroom doctor failure with the same source-aware, value-redacted remediation. - Preserve both user credentials and require an explicit choice between API-key billing and token/gateway auth. ## 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) ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text 151 Claude runtime, wrap, doctor, Remote Control, and MCP dependency-contract tests passed ruff check and format checks passed git diff --check passed ``` Branch contains current main, including the MCP v1 cap and the five just-merged blocker PRs. ## Real Behavior Proof - Environment: isolated local worktree on current `main` with Claude wrapper and doctor fixtures. - Exact command / steps: exercised conflicting and non-conflicting shell, user, project, and local-project credential layers through the focused wrap and doctor test suites. - Observed result: conflicting effective credentials fail before proxy startup or settings mutation, report only credential sources, and never expose values. - Not tested: a live Claude Code login with production credentials; credential precedence and side-effect boundaries are covered by fixtures. ## Runtime Rollout Safety - Rollout-managed feature(s): Claude authentication-conflict preflight. - Minimum rollout channel: normal patch release. - Stable/default behavior changed: only configurations with both effective credentials now stop early with actionable diagnostics. - Kill switch / disable path: remove or clear either conflicting credential in its reported source. - Unsafe override required: none; Headroom deliberately does not choose or delete a user credential. - Qualification impact: Claude wrap, doctor, Remote Control, and MCP dependency-contract tests must remain green. - Rollback path: human revert restores the previous late Claude Code rejection; no persisted migration is involved. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Safety No credential value is returned by the classifier, printed by wrap, or emitted in doctor JSON. The preflight runs before _register_proxy_client, proxy startup, MCP registration, or settings writes. |
||
|
JD Davis
|
aa811fa91f |
ci: allow generated dependency commit bodies (#3012)
## Description Disable commitlint's per-line body length limit because Dependabot generates grouped-update commit bodies with dependency/link lines whose length varies with group contents. PR #2964 currently fails only because one generated line is 274 characters long. Closes # N/A ## 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 - Disabled `body-max-line-length` in `.commitlintrc.json`. - Kept Conventional Commit type, subject, and all other configured validation rules enforced. ## Testing - [ ] Unit tests pass (`pytest`) - [ ] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [ ] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text PR #2964 Dependabot commit (49 lines; maximum line length 274) exit code: 0 bogus: should fail type must be one of [build, chore, ci, docs, deps, feat, fix, parity, perf, refactor, revert, style, test] [type-enum] exit code: 1 fix: subject may not be empty [subject-empty] exit code: 1 git diff --check exit code: 0 ``` ## Real Behavior Proof - Environment: Windows PowerShell; Node.js 22; `@commitlint/cli` and `@commitlint/config-conventional` 19.8.1. - Exact command / steps: Fetched the current PR #2964 commit message through the GitHub API and piped the complete message into commitlint using this branch's `.commitlintrc.json`; then ran negative type and subject cases. - Observed result: The exact grouped Dependabot commit passed; an unapproved type and empty subject remained rejected. - Not tested: Python/Rust unit tests and runtime behavior; this change only modifies commit-message validation configuration. ## Runtime Rollout Safety - Rollout-managed feature(s): N/A; CI configuration only. - Minimum rollout channel: N/A. - Stable/default behavior changed: Commit bodies may contain lines of any length; all other commitlint rules remain active. - Kill switch / disable path: Revert this commit or restore a numeric `body-max-line-length` limit. - Unsafe override required: No. - Qualification impact: Generated Dependabot group descriptions no longer fail CI due solely to a long dependency/link line. - Rollback path: Revert this commit. ## 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 - [ ] 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 - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] 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) ## Screenshots (if applicable) N/A; this change has no user interface. ## Additional Notes - The comment and documentation checklist items are not applicable to this one-line commitlint configuration change. - No automated test file was added; the exact positive and negative commitlint cases were run manually as shown above. - Full application tests were not run because no application code or runtime behavior changed. - Keep this PR unmerged pending maintainer review. |
||
|
JD Davis
|
7e3128057c |
ci: allow Dependabot deps commits (#3009)
## Description Allow the `deps:` Conventional Commit type emitted by Dependabot. Dependabot PRs currently fail the CI `commitlint` job because `deps` is not included in the repository's configured `type-enum`. Closes # N/A ## 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 - Added `deps` to the allowed commit types in `.commitlintrc.json`. - Existing and future Dependabot commits using `deps: ...` can pass the commit-message policy. ## Testing - [ ] Unit tests pass (`pytest`) - [ ] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [ ] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text deps: bump ruff from 0.15.22 to 0.16.2 exit code: 0 bogus: should fail type must be one of [build, chore, ci, docs, deps, feat, fix, parity, perf, refactor, revert, style, test] [type-enum] exit code: 1 git diff --check exit code: 0 ``` ## Real Behavior Proof - Environment: Windows PowerShell; Node.js 22; `@commitlint/cli` and `@commitlint/config-conventional` 19.8.1. - Exact command / steps: Ran commitlint with `.commitlintrc.json` against a real failing Dependabot subject, then against an unapproved `bogus:` type. - Observed result: The `deps:` subject passed; the unapproved type remained rejected by `type-enum`. - Not tested: Python/Rust unit tests and runtime behavior; this change only modifies commit-message validation configuration. ## Runtime Rollout Safety - Rollout-managed feature(s): N/A; CI configuration only. - Minimum rollout channel: N/A. - Stable/default behavior changed: Commitlint now accepts the `deps` type. - Kill switch / disable path: Revert this commit or remove `deps` from `type-enum`. - Unsafe override required: No. - Qualification impact: Dependabot PR commit messages no longer fail solely because their type is `deps`. - Rollback path: Revert this commit. ## 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 - [ ] 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 - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] 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) ## Screenshots (if applicable) N/A; this change has no user interface. ## Additional Notes - The comment and documentation checklist items are not applicable to this one-line commitlint configuration change. - No automated test file was added; the exact positive and negative commitlint cases were run manually as shown above. - Full application tests were not run because no application code or runtime behavior changed. - Keep this PR unmerged pending maintainer review. |
||
|
Copilot
|
8ea87e7804 |
fix: tool_search_tool_regex deferred and falsely resolved on direct-Anthropic path (#2971)
## Description Direct Anthropic users could receive `400 Tool reference 'tool_search_tool_regex' not found in available tools` when Claude Code sent a typeless `tool_search_tool_regex` entry. Headroom treated it as an ordinary deferrable tool, injected a typed search tool with the same name, and later mistook that typed search mechanism for a valid target of the stale `tool_reference`. This change prevents the duplicate injection and repairs already-poisoned transcripts without stripping valid references to ordinary deferred tools. It addresses the first-party Anthropic regression reported in [PR #2539's follow-up](https://github.com/headroomlabs-ai/headroom/pull/2539#issuecomment-5280259642) and complements the history repair from #2805. ## 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 - recognize typeless, case-insensitive `tool_search_tool_*` names as an existing client tool-search surface and skip Headroom's duplicate injection - exclude typed Anthropic search mechanisms from the set of valid `tool_reference` targets - preserve valid regular deferred-tool references and the normal deferral path for similar non-reserved names - add a first-party Anthropic handler regression that proves the outbound tools remain unchanged and stale search bookkeeping is removed - rebase onto #2996, which prevents the native detector from hanging the full CI test shard ## 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 ### Test Output ```text .venv/Scripts/python.exe -m pytest \ tests/test_issue_746_tool_search.py \ tests/test_anthropic_stage_timings.py \ tests/test_cache_control_ttl_order.py \ tests/test_cache_ttl_preserved.py \ tests/test_proxy/test_tool_search_repair_after_turn_hooks.py \ tests/test_transforms/test_detect_fallback_1123.py \ tests/test_transforms_content_detection.py \ tests/test_transforms_content_router.py \ -q --disable-warnings --maxfail=1 170 passed, 1 warning in 10.27s .venv/Scripts/ruff.exe check . All checks passed! .venv/Scripts/ruff.exe format --check . 1411 files already formatted pre-commit run mypy --all-files Success: no issues found in 519 source files git diff --check (no output) ``` ## Real Behavior Proof - Environment: Windows, Python 3.13.3, first-party Anthropic handler test with `HEADROOM_TOOL_SEARCH` at its default enabled setting - Exact command / steps: run the same helper-level payload against the pre-fix base and this branch, then run `test_anthropic_direct_path_repairs_typeless_tool_search_regression` through `handle_anthropic_messages()` with 20 ordinary tools, one typeless `tool_search_tool_regex`, and a stale self-reference - Observed result: before the fix, Headroom injected a second typed search tool, deferred the typeless client tool, and removed 0 stale blocks; on this branch, it skips duplicate injection, preserves the client tools array, and removes the paired `server_tool_use` and `tool_search_tool_result` blocks before forwarding - Not tested: a live request against a paid Anthropic account; the production handler's outbound body is captured before the network boundary instead ## Runtime Rollout Safety - Rollout-managed feature(s): Anthropic server-side tool-search deferral (`HEADROOM_TOOL_SEARCH`) - Minimum rollout channel: standard CI; narrow corrective change to an existing default-on path - Stable/default behavior changed: yes; reserved typeless client search tools now suppress duplicate injection, and typed search mechanisms no longer satisfy deferred-tool references - Kill switch / disable path: set `HEADROOM_TOOL_SEARCH=0` to disable new injection; history repair remains unconditional so existing poisoned sessions can recover - Unsafe override required: no - Qualification impact: no new rollout surface or configuration; focused handler, helper, cache-control, and hook-order regressions cover the affected path - Rollback path: revert this PR; operators can set `HEADROOM_TOOL_SEARCH=0` while rolling back ## 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) ## Screenshots (if applicable) N/A — proxy request transformation only. ## Additional Notes - Documentation is not changed because this fixes internal request classification and transcript repair without adding a user-facing option or workflow. - Anthropic documents `tool_search_tool_regex` / `tool_search_tool_bm25` as server search mechanisms; deferred definitions, rather than the search mechanism itself, are the valid `tool_reference` targets. - Rebased onto #2996, which fixes the unrelated native-detector hang that timed out shard 4 on the prior merge commit. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
JD Davis
|
8a1d38bc5d |
fix(proxy): complete stateless Responses and buffered CCR lifecycle (#2997)
## Description Consolidates the related OpenAI Responses ZDR/stateless continuation and buffered CCR response-lifecycle corrections on current main. It preserves client storage policy, makes Headroom-owned continuations stateless across HTTP and WebSocket, and prevents buffered streaming paths from committing a false HTTP 200 before the real upstream outcome is known. Closes #2675 ## 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 - Preserves explicit and omitted Responses `store` policy instead of forcing provider storage or disabling memory tools. - Replays normalized input, replayable outputs, encrypted reasoning content, and Headroom function outputs without `previous_response_id`. - Applies the same stateless continuation policy to HTTP and WebSocket. - Prevents transparent memory execution after client-visible WebSocket output. - Delays buffered CCR ASGI status/headers until the operation resolves for Anthropic Messages and OpenAI Responses. - Preserves real 429/5xx status and retry headers. - Converts malformed non-JSON/non-SSE upstream 200 replies to a sanitized 502 protocol error. - Preserves valid JSON-to-SSE synthesis and existing SSE adaptation. - Removes unreachable task cleanup left behind after replacing the old keepalive polling loop with a direct awaited operation. ## 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 ### Test Output ```text 118 passed across the changed HTTP/WS ZDR, lifecycle, and both-provider CCR suites 11526 tests collected with no collection errors ruff check .: All checks passed ruff format --check .: 1411 files already formatted mypy headroom/proxy/handlers/anthropic.py headroom/proxy/handlers/openai.py: Success: no issues found in 2 source files ``` Exact-head CI is entirely green on `cbc2739c0c633ea477727ec3eb2f8a3862fa08f3`. ## Real Behavior Proof - Environment: macOS arm64/Python 3.13 locally; GitHub-hosted Ubuntu matrix pending. - Exact command / steps: exercise `store=false` Responses memory calls over HTTP and WebSocket; exercise buffered Anthropic and Responses requests returning successful JSON/SSE, delayed 429 responses, exceptions, and malformed successful bodies; invoke returned ASGI responses and inspect emitted status, headers, and body order. - Observed result: stateless continuations omit provider response IDs and retain `store=false`; no ASGI start event is emitted before the buffered outcome; real failures preserve status/headers; malformed 200 responses become sanitized 502 errors. - Not tested: live ZDR tenant and live Anthropic/OpenAI upstream credentials are unavailable in repository CI; wire contracts are exercised through deterministic upstream doubles. ## Runtime Rollout Safety - Rollout-managed feature(s): Responses memory continuation and buffered CCR handling. - Minimum rollout channel: normal patch release after full CI qualification. - Stable/default behavior changed: memory continuation no longer requires provider storage; buffered CCR waits before committing response status. - Kill switch / disable path: disable memory/CCR using existing proxy configuration (`--no-ccr` for CCR); ordinary non-buffered paths are unchanged. - Unsafe override required: none. - Qualification impact: full Python matrix plus focused HTTP/WS lifecycle suites must pass; patch coverage must not rely on unreachable cleanup. - Rollback path: human revert of this PR restores prior continuation/buffering behavior; no persisted data migration is introduced. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review — exact-head CI is entirely green ## 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 - [x] I have made corresponding changes to the documentation — inline protocol/lifecycle documentation; no separate user guide required - [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) ## Screenshots (if applicable) Not applicable; proxy protocol behavior. ## Additional Notes Human review only. No merge or auto-merge is configured. This supersedes narrower #2995 and incorporates the complete intent of #2705, #2959, and #2968 without falsely closing those PRs. It does not claim the broader event-level streaming-splice guarantees requested by #1877. Refreshed from main after #2996; the MCP cap `mcp>=1.28.1,<2.0.0` is preserved. |
||
|
JD Davis
|
a708c0571e |
fix(ci): prevent native detector from hanging test shards (#2996)
## Description CI shard 4 was not merely slow: after thousands of fast tests it parked indefinitely inside `headroom._core.detect_content_type` at 0% CPU. The router watchdogged only the first native call and then permanently trusted direct calls via `_detect_native_verified`. Earlier suite activity can change ORT/native state after that first success, making a later call deadlock until GitHub cancels the job. This keeps every native call bounded by the existing watchdog, activates the process-wide pure-Python circuit breaker after a timeout, restores the test-job ceiling to 30 minutes, and removes a separate wall-clock scheduler assertion that generated false shard-1 failures despite the structural regression guards passing. No issue is auto-closed by this infrastructure repair. ## 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 - Removed the unsafe process-lifetime `_detect_native_verified` fast path. - Kept every native detection call behind the existing bounded watchdog. - Preserved the process-wide fallback circuit breaker so only the first wedged call consumes the watchdog budget. - Added a success-then-hang regression test. - Isolated native circuit-breaker state in fallback exception tests. - Restored the CI test timeout from the temporary 90-minute diagnostic ceiling to 30 minutes. - Replaced the Codex scheduler's noise-sensitive p99/p50 assertion with its meaningful absolute regression ceiling while retaining source-level guards against the removed semaphore and nested executor. - Corrected import order and formatting defects inherited from current main so the synthetic merge commit passes repository-wide lint. ## 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 ### Test Output ```text Exact local shard-4 command with coverage: 2723 passed, 172 skipped, 8661 deselected in 108.40s Focused detector/router suite: 62 passed Codex scheduler suite: 3 passed, 1 skipped ruff check . All checks passed! ruff format --check . 1411 files already formatted mypy headroom/transforms/content_router.py Success: no issues found in 1 source file ``` Exact-head GitHub CI on `28f284c7a156d5be2fac2d21ba904a19d3389e6d` is entirely green. Test jobs 1–4, test-extras, test-agno, build, wheel, lint, CodeQL, dependency audit, secret scan, smoke, governance, and conflict checks all passed. Remaining skips are path-filtered jobs not applicable to this diff. ## Real Behavior Proof - Environment: macOS arm64/Python 3.13 locally; GitHub-hosted Ubuntu/Python 3.12 using the production CI workflow and prebuilt wheel. - Exact command / steps: reproduced `pytest tests scripts/tests --splits 4 --group 4 ...` hanging in native detection; sampled the parked process; reran with `pytest-timeout` to locate `_rust_detect`; applied the correction; reran the exact shard locally and all four CI shards remotely. - Observed result: local shard 4 completed in 1:48. GitHub shard 4's pytest step completed in 5:45 and its full job in 8:06 under the restored 30-minute ceiling. All four shards passed on the same head. - Not tested: deliberately wedging a real production ORT runtime outside the deterministic mocked regression; the watchdog behavior is covered with a native-call fake that succeeds once and then never returns. ## Runtime Rollout Safety - Rollout-managed feature(s): native content detection watchdog and fallback only. - Minimum rollout channel: normal patch release; no staged feature flag required. - Stable/default behavior changed: every native detection call remains watchdog-bounded instead of only the first successful call. - Kill switch / disable path: `HEADROOM_DETECT_BACKEND=python` bypasses native detection; `HEADROOM_DETECT_TIMEOUT_SECS` controls the watchdog budget. - Unsafe override required: none. - Qualification impact: full Python CI matrix must remain green; exact shard-4 completion is the primary qualification evidence. - Rollback path: human revert of this PR if bounded calls cause an unexpected regression; setting the Python backend provides an immediate operational fallback without code rollback. ## 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 - [x] I have made corresponding changes to the documentation — inline lifecycle documentation and PR operational notes; no user-facing docs change is needed - [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) ## Screenshots (if applicable) Not applicable; no UI change. ## Additional Notes Human review only. No merge or auto-merge action has been configured. The branch includes current main and preserves the MCP SDK compatibility cap `mcp>=1.28.1,<2.0.0`. |