fix(kompress): accept ccr_original on the remote compressor - #3162
Merged
Conversation
Reported from a field log (Copilot Chat on Windows, 0.36.x). On EVERY request:
WARNING Kompress failed: RemoteKompressCompressor.compress() got an
unexpected keyword argument 'ccr_original'
INFO [router] route_counts={...} compressed=0 frozen=1 msgs=2
INFO PERF ... tok_before=1623 tok_after=1623 tok_saved=0 savings=none
`RemoteKompressCompressor` promises in its module docstring to mirror
`KompressCompressor`'s public surface "so it is a drop-in at the ContentRouter
seam". That promise lapsed: the local `compress` gained a `ccr_original`
keyword and the remote one did not.
ContentRouter passes `ccr_original` whenever custom tags are protected. Its
comment there reads "Only set it when tags were protected so callers/compressors
that don't accept the kwarg are unaffected on the common path" — but the remote
compressor IS affected, and catastrophically: the call raises TypeError, which
ContentRouter catches under a broad `except Exception` and logs as
`Kompress failed: ...` at WARNING.
The blast radius is the whole deployment, not one request. `_get_kompress`
returns the remote compressor ahead of every local path, so on any install with
HEADROOM_KOMPRESS_ENDPOINT set — precisely the sandboxed/enterprise deployment
the class exists to serve — ML compression was silently disabled while the proxy
kept reporting success and the dashboards read "working, 0 saved".
Two parts, because fixing only the crash would leave the bug `ccr_original`
exists to prevent:
* accept the keyword, so the seam contract holds;
* honor it — store the pre-protection text in CCR rather than the placeholder
intermediate, so a later full retrieval returns the real block instead of
`{{HEADROOM_TAG_N}}`. The endpoint's own `original_tokens` describes
`content`, so when an override is supplied the stored text is counted
locally. Unchanged on the common path where no override is passed.
Adds a signature-compatibility test over the two `compress` methods, so this
drift cannot recur silently. It compares public keywords only —
`_deadline_started_at` is underscore-prefixed and only ever passed by
kompress_compressor to itself on its recursive batch path, never across the
seam.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
chopratejas
requested review from
DevanshiVyas and
JerrettDavis
as code owners
August 21, 2026 04:09
Contributor
PR governanceThis PR does not yet satisfy the required template fields:
Please update the PR body, or move the PR back to draft while it is still in progress. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
14 tasks
chopratejas
added a commit
that referenced
this pull request
Aug 21, 2026
…3164) ## Description Reported from a Copilot CLI session: ``` [CopilotCLISession] Failed to fetch models: Error: 401 "unauthorized: unable to validate HMAC for the given Copilot-Integration-ID" [CopilotCLISession] Proxy URL configured (authType=hmac), skipping client-side token validation ``` GitHub **binds a Copilot API token to the `Copilot-Integration-Id` it was minted under** and verifies the pairing with an HMAC. Present a token minted for integration A alongside a header naming integration B, and you get exactly this error. `apply_copilot_api_auth` applied the integration ID with *set-default* semantics — `_set_header_default` returns early when the header is already present — **before** deciding whose token to use: ```python for name, value in _copilot_chat_header_defaults().items(): _set_header_default(resolved, name, value) # ← never overwrites ... if incoming_auth and _is_forwardable_copilot_bearer_token(...): return resolved # client's token kept ... token = await get_copilot_token_provider().get_api_token() # ← REPLACED ``` The client always sends an ID, so when Headroom replaced the token — the common case, logged as `incoming token not suitable (kind=unknown), will replace` — the request left carrying **the client's integration ID next to Headroom's token**, minted under `vscode-chat` via `_copilot_token_exchange_headers`. A Copilot CLI session does not identify as `vscode-chat`. The second log line is why nothing caught it sooner: seeing a proxy URL, the Copilot client reports `authType=hmac` and **skips its own token validation**, deferring to the proxy. Nobody validates the pairing until GitHub rejects it. **Why this matters beyond one 401:** the failing call is *model discovery*. When it fails the client falls back to its built-in model list — which is why a user's selected model never appeared in telemetry and all traffic surfaced as `gpt-4o-mini`. Closes # ## 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 Restores one invariant: **the credential and the integration ID leave together.** - **Mint under the client's ID** rather than the proxy's default, so GitHub's usage attribution keeps pointing at the surface that actually made the call. - **Overwrite the forwarded header to match what we minted** — but only on the replace path. The pass-through branch returns earlier and keeps the client's own ID beside the client's own token, which is equally a matched pair. - **Key the token cache by integration ID.** A single slot would hand a `vscode-chat` token to a CLI session and reproduce the same 401 straight from cache. Two existing contracts deliberately preserved: - Resolution order is **client header > `GITHUB_COPILOT_INTEGRATION_ID` > built-in default**. The env var configures the *default* this proxy sends; it does not override a client that stated its own identity. Pinned by the existing `test_apply_copilot_api_auth_preserves_existing_copilot_headers` (whose fixture literally names the value `should-not-override`). - The overwrite writes through the client's **existing key**, so a lowercase `copilot-integration-id` does not gain a second capitalised variant beside it — pinned by the existing `..._preserves_existing_headers_case_insensitively`. Existing test stubs for `get_api_token` gained the new keyword — the same signature-drift hazard this repo just hit in `RemoteKompressCompressor` (#3162). ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ pytest tests/ -q -k copilot 338 passed, 8 skipped $ pytest tests/ -q # this branch 6 failed, 11386 passed, 587 skipped in 425.40s All 6 also fail on clean origin/main, same machine — pre-existing, not regressions: test_graceful_shutdown.py::test_run_server_installs_cancelled_error_filter test_learn/test_integration.py::TestCodexIntegration::test_full_pipeline test_release_workflows.py::test_no_native_tls_in_wheel_build_tree test_providers/test_deepseek.py::... (3 litellm pricing tests) $ ruff check headroom/ All checks passed! $ mypy headroom/copilot_auth.py 0 errors ``` 12 new tests: the mint/forward pairing, the pass-through branch keeping the client's pair untouched, no duplicate case-variant header, resolution order in both directions, blank/absent client values, non-Copilot upstreams untouched, and per-integration cache isolation. ## Real Behavior Proof - **Environment:** macOS, Python 3.12.13, branch on `origin/main` @ `a3821378`. - **Exact command / steps:** drove `apply_copilot_api_auth` with the reported shape — an unusable client bearer plus `Copilot-Integration-Id: copilot-cli-chat` against `api.githubcopilot.com` — and compared the ID the token would be **minted under** (via `_copilot_token_exchange_headers`) against the ID actually **forwarded**. Run against the same script before and after the change, with `PYTHONPATH` pinned to the worktree. - **Observed result:** ``` ########## PRE-FIX ########## token minted under : vscode-chat header forwarded : copilot-cli-chat -> GitHub would REJECT (401 HMAC) ########## POST-FIX ########## token minted under : copilot-cli-chat header forwarded : copilot-cli-chat -> GitHub would ACCEPT ``` - **Not tested:** no live call to GitHub's CAPI — the HMAC is validated server-side by GitHub and cannot be exercised offline. The claim verified here is that the two halves now agree; that GitHub accepts a correctly-paired credential is inferred from its error message, not observed. **Worth one live Copilot CLI run before shipping to a reporter.** The `GITHUB_COPILOT_API_TOKEN` path is also unchanged: an externally-supplied token was minted under an integration this proxy cannot know, so it is passed through as before. ## Runtime Rollout Safety - **Rollout-managed feature(s):** none. - **Minimum rollout channel:** n/a. - **Stable/default behavior changed:** requests where Headroom replaces the token now forward the integration ID the replacement was minted under. For a client sending `vscode-chat` (VS Code, the previous default) nothing changes at all — the resolved value is identical. - **Kill switch / disable path:** setting `GITHUB_COPILOT_INTEGRATION_ID` pins the value used for clients that send none; clients that send one are unaffected either way. - **Unsafe override required:** none. - **Qualification impact:** model discovery should stop 401ing for non-VS-Code Copilot surfaces, which restores the real model list. - **Rollback path:** revert the commit; behavior returns to minting under `vscode-chat` regardless of caller. ## 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 Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Merged
chopratejas
pushed a commit
that referenced
this pull request
Aug 21, 2026
🤖 I have created a release *beep* *boop* --- ## [0.36.2](v0.36.1...v0.36.2) (2026-08-21) ### Bug Fixes * **copilot:** bind the minted token to the integration ID we forward ([#3164](#3164)) ([397803a](397803a)) * **kompress:** accept ccr_original on the remote compressor ([#3162](#3162)) ([45cb1b9](45cb1b9)) * **proxy:** count output tokens from the stream's text, not its wire size ([#3163](#3163)) ([4006964](4006964)) ### Dependencies * bump ai from 6.0.138 to 7.0.59 in /sdk/typescript ([#2281](#2281)) ([0891062](0891062)) * bump ai from 6.0.149 to 7.0.59 in /docs ([#2277](#2277)) ([f7e5d37](f7e5d37)) * bump md-5 from 0.10.6 to 0.11.0 ([#3146](#3146)) ([c6dd823](c6dd823)) * bump ruff from 0.16.2 to 0.16.3 in the pip-minor-patch group ([#3143](#3143)) ([c8db13d](c8db13d)) * bump the cargo-minor-patch group with 8 updates ([#3145](#3145)) ([9c14e3a](9c14e3a)) * bump tiktoken-rs from 0.11.0 to 0.12.0 ([#3147](#3147)) ([a307c11](a307c11)) * bump tokenizers from 0.22.2 to 0.23.1 ([#3149](#3149)) ([6e2e10f](6e2e10f)) * bump typescript from 5.9.3 to 7.0.2 in /plugins/openclaw ([#2279](#2279)) ([85774fc](85774fc)) * bump typescript from 5.9.3 to 7.0.2 in /plugins/opencode ([#2280](#2280)) ([a382137](a382137)) * update mcp requirement from <2.0.0,>=1.28.1 to >=1.28.1,<3.0.0 ([#3144](#3144)) ([6928d19](6928d19)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
From a user's proxy log (Copilot Chat 0.61.0 on Windows, VS Code 1.133.0, Headroom 0.36.x). This appears on every single request:
RemoteKompressCompressor's module docstring promises the class "mirrorsKompressCompressor's public surface (is_ready/preload/ensure_background_load/compress), so it is a drop-in at the ContentRouter seam". That promise lapsed — the localcompressgained accr_originalkeyword and the remote one did not.ContentRouter._try_ml_compressorpassesccr_originalwhenever custom tags are protected. The comment there reads:That assumption is wrong. The remote compressor is affected: the call raises
TypeError, which the surrounding broadexcept Exceptioncatches and downgrades tologger.warning("Kompress failed: %s", e). The request then forwards uncompressed and the proxy reports success.The blast radius is the entire deployment, not one request.
_get_kompressreturns the remote compressor ahead of every local path, so on any install withHEADROOM_KOMPRESS_ENDPOINTset — precisely the sandboxed/enterprise deployment this class exists to serve — ML compression was silently disabled while every dashboard read "working, 0 tokens saved".Closes #
Type of Change
Changes Made
Two parts, because fixing only the crash would leave the bug
ccr_originalexists to prevent:RemoteKompressCompressor.compress, so the seam contract actually holds.{{HEADROOM_TAG_N}}. The endpoint's ownoriginal_tokensdescribescontent, so when an override is supplied the stored text is counted locally; the common path (no override) keeps the endpoint's count exactly as before.compressmethods, so this drift cannot recur silently. It compares public keywords only —_deadline_started_atis underscore-prefixed and only ever passed bykompress_compressorto itself on its recursive batch path, never across the seam.Testing
pytest)ruff check .)mypy headroom)Test Output
Real Behavior Proof
origin/main@a3821378.RemoteKompressCompressor.compresswith the exact kwargsContentRouter._try_ml_compressorbuilds whenprotectedis truthy (context,question,target_ratio,allow_download,ccr_original), against a stubbed HTTP client.TypeError: ... unexpected keyword argument 'ccr_original'— byte-identical to the user's log line. Post-fix it returns aKompressResult, and CCR receives the pre-protection text ("HEADROOM_TAG" not in stored) with a token count matching what was stored.HEADROOM_KOMPRESS_ENDPOINThas not been exercised here.Runtime Rollout Safety
HEADROOM_KOMPRESS_ENDPOINTset.HEADROOM_KOMPRESS_ENDPOINTunset, orkompress_model="disabled").Review Readiness
Checklist