From d7a8cdbee1c500be35b87c9da8395087a37ff8b9 Mon Sep 17 00:00:00 2001 From: Serge ARADJ Date: Sat, 18 Jul 2026 18:54:06 +0200 Subject: [PATCH] =?UTF-8?q?feat(proxy):=20label=20GitHub=20Copilot=20traff?= =?UTF-8?q?ic=20as=20"copilot"=20in=20the=20outcome=E2=80=A6=20(#2377)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Requests routed to the GitHub Copilot API travel on the OpenAI or Anthropic wire, so the proxy handlers stamp the *wire* provider (`openai` / `anthropic`) on the outcome. As a result, Copilot traffic is attributed to OpenAI/Claude in the dashboard's per-request provider stats, hiding the real upstream. (This is distinct from the existing **Copilot Quota** panel, which is separate from per-request provider attribution.) This labels Copilot traffic as `copilot` in the single outcome funnel. `build_copilot_upstream_url()` is already the one routing chokepoint every Copilot surface goes through (OpenAI `/chat/completions` + `/responses` and the Anthropic `/v1/messages` route all build their upstream URL there), so it flags the request via a task-local `ContextVar`; `emit_request_outcome()` reads the flag and relabels the provider. The relabel runs before the `>= 500` failed guard, so a failed Copilot request is attributed to `copilot` too. Non-Copilot traffic never sets the flag and is untouched. ## Type of Change - [x] New feature (non-breaking change that adds functionality) ## Changes Made - `headroom/copilot_auth.py`: add a task-local `_request_routed_to_copilot` `ContextVar` with `mark_request_routed_to_copilot()` / `request_routed_to_copilot()` helpers; set the flag in `build_copilot_upstream_url()` whenever the base is a Copilot API URL (the existing `is_copilot_api_url` check). `/v1` path normalization is unchanged. - `headroom/proxy/outcome.py`: in `emit_request_outcome()`, when the request was routed to Copilot and the wire provider is `openai`/`anthropic`, relabel the outcome provider to `copilot` (before the 5xx guard). - `tests/test_copilot_provider_label.py`: new tests for the chokepoint marking and the outcome relabel. ## Testing - [x] Unit tests pass (`pytest`) - [ ] Linting passes (`ruff check .`) — ran on the changed files only (clean) - [ ] Type checking passes (`mypy headroom`) — ran on the changed files only (clean) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ python -m pytest tests/test_copilot_provider_label.py tests/test_outcome_records_5xx_as_failed.py -q tests/test_copilot_provider_label.py ..... [ 71%] tests/test_outcome_records_5xx_as_failed.py .. [100%] 7 passed $ python -m pytest tests/test_copilot_auth.py -k "build_copilot_upstream_url or copilot_api_url" -q 8 passed, 58 deselected # existing /v1-stripping behavior preserved $ python -m ruff check headroom/copilot_auth.py headroom/proxy/outcome.py tests/test_copilot_provider_label.py All checks passed! $ python -m mypy headroom/copilot_auth.py headroom/proxy/outcome.py Success: no issues found in 2 source files ``` ## Real Behavior Proof - Environment: Python 3.11, headroom installed with the `proxy` extra. - Exact command / steps: the unit tests above drive `build_copilot_upstream_url()` followed by `emit_request_outcome()` in an isolated context and assert the recorded provider. - Observed result: an `anthropic`/`openai` outcome for a request routed to `https://api.githubcopilot.com` is recorded as provider `copilot`; a request not routed to Copilot is recorded under its wire provider unchanged. - Not tested: end-to-end against a live Copilot subscription (no live seat in the test environment). ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Additional Notes - The flag is a `ContextVar` (task-local), so it cannot bleed across concurrent requests; each request that is not routed to Copilot simply reads the `False` default. - No `CHANGELOG.md` edits (release-please generates it from the Conventional Commit PR title). --------- Co-authored-by: Claude Co-authored-by: JerrettDavis --- headroom/copilot_auth.py | 56 ++++++++++- headroom/proxy/outcome.py | 14 +++ tests/conftest.py | 14 +++ tests/test_copilot_provider_label.py | 136 +++++++++++++++++++++++++++ 4 files changed, 218 insertions(+), 2 deletions(-) create mode 100644 tests/test_copilot_provider_label.py diff --git a/headroom/copilot_auth.py b/headroom/copilot_auth.py index cd29f7a84..d8c779e44 100644 --- a/headroom/copilot_auth.py +++ b/headroom/copilot_auth.py @@ -10,6 +10,7 @@ import logging import math import os import time +from contextvars import ContextVar from ctypes import wintypes from dataclasses import dataclass from datetime import datetime @@ -952,13 +953,64 @@ def _is_ghe_copilot_api_host(host: str) -> bool: ) +# Per-request flag: set when a request is routed to the GitHub Copilot API so +# the single outcome funnel can label the provider "copilot" regardless of the +# wire shape (OpenAI or Anthropic) the request travelled on. A ContextVar is +# task-local, so it never bleeds across concurrent requests. A ContextVar value +# nonetheless persists until overwritten within a single execution context, so +# the outcome funnel consumes it (read-and-clear) rather than just reading it — +# otherwise a later non-Copilot outcome in the same context (e.g. successive +# messages on one WebSocket task) would be mislabeled. +_request_routed_to_copilot: ContextVar[bool] = ContextVar( + "_request_routed_to_copilot", default=False +) + + +def mark_request_routed_to_copilot() -> None: + """Flag the current request as routed to the GitHub Copilot API.""" + _request_routed_to_copilot.set(True) + + +def request_routed_to_copilot() -> bool: + """Return True when the current request was routed to the Copilot API. + + Read-only; does not clear the flag. Prefer :func:`consume_request_routed_to_copilot` + at the point the label is applied so the flag cannot leak to a later outcome. + """ + return _request_routed_to_copilot.get() + + +def consume_request_routed_to_copilot() -> bool: + """Return whether the current request was routed to the Copilot API, and + clear the flag so a subsequent outcome emitted in the same execution context + is not mislabeled.""" + routed = _request_routed_to_copilot.get() + if routed: + reset_request_routed_to_copilot() + return routed + + +def reset_request_routed_to_copilot() -> None: + """Clear the Copilot routing flag. For test isolation and any explicit + request-boundary reset (build_copilot_upstream_url sets it as a side effect, + so callers outside a request task should reset it to avoid leaking state).""" + _request_routed_to_copilot.set(False) + + def build_copilot_upstream_url(base_url: str, path: str) -> str: """Build an upstream URL, normalizing GitHub Copilot's non-/v1 path layout.""" normalized_base = base_url.rstrip("/") normalized_path = path if path.startswith("/") else f"/{path}" - if is_copilot_api_url(normalized_base) and normalized_path.startswith("/v1/"): - normalized_path = normalized_path[3:] + if is_copilot_api_url(normalized_base): + # Single routing chokepoint for every Copilot surface (OpenAI + # chat/responses and Anthropic messages all build their upstream URL + # here), so mark the request for provider relabeling downstream. + mark_request_routed_to_copilot() + if normalized_path.startswith("/v1/"): + normalized_path = normalized_path[3:] + else: + reset_request_routed_to_copilot() return f"{normalized_base}{normalized_path}" diff --git a/headroom/proxy/outcome.py b/headroom/proxy/outcome.py index 311a365c9..e61be614e 100644 --- a/headroom/proxy/outcome.py +++ b/headroom/proxy/outcome.py @@ -339,10 +339,24 @@ async def emit_request_outcome(handler: Any, outcome: RequestOutcome) -> None: and is awaitable-compatible. We could lift this to a typing.Protocol if/when another contract surface emerges, but YAGNI. """ + from headroom.copilot_auth import consume_request_routed_to_copilot from headroom.proxy.cost import _summarize_transforms from headroom.proxy.models import RequestLog from headroom.proxy.project_context import get_current_project + # GitHub Copilot: requests routed to the Copilot API travel on the OpenAI or + # Anthropic wire, so the handlers stamp the wire provider. Relabel to + # "copilot" here — the single outcome funnel — so the dashboard shows the + # real upstream instead of "openai"/"anthropic". Keyed on the per-request + # flag set in build_copilot_upstream_url; never touches non-Copilot traffic. + # Done before the 5xx guard so a failed Copilot request is attributed too. + # consume_* reads AND clears the flag (called unconditionally via short-circuit + # order) so it cannot leak onto a later outcome in the same execution context. + if consume_request_routed_to_copilot() and outcome.provider in ("openai", "anthropic"): + import dataclasses + + outcome = dataclasses.replace(outcome, provider="copilot") + # Upstream failure (>= 500, e.g. a 529 Overloaded surfaced after retry # exhaustion) must not feed the savings/cost/log success stats; that would # let a failed request inflate the save-rate. Record it as failed and stop, diff --git a/tests/conftest.py b/tests/conftest.py index 5cffdb1fc..93f18d541 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -30,6 +30,20 @@ def _scrub_developer_headroom_env(monkeypatch): monkeypatch.delenv("ANTHROPIC_CUSTOM_HEADERS", raising=False) +# The Copilot "routed to Copilot" flag is a module-global ContextVar that +# build_copilot_upstream_url() sets as a side effect. Unit tests that call that +# builder directly (or otherwise run in the shared root context) would leave it +# set and mislabel a later test's request outcome as "copilot". Reset it around +# every test so build-time side effects can't leak between tests. +@pytest.fixture(autouse=True) +def _reset_copilot_routing_flag(): + from headroom.copilot_auth import reset_request_routed_to_copilot + + reset_request_routed_to_copilot() + yield + reset_request_routed_to_copilot() + + # ============================================================================= # Global test hooks # ============================================================================= diff --git a/tests/test_copilot_provider_label.py b/tests/test_copilot_provider_label.py new file mode 100644 index 000000000..5aca8f4c0 --- /dev/null +++ b/tests/test_copilot_provider_label.py @@ -0,0 +1,136 @@ +"""GitHub Copilot traffic must be labeled provider "copilot" in the outcome +funnel, even though it travels on the OpenAI/Anthropic wire. + +``build_copilot_upstream_url`` is the single routing chokepoint for every +Copilot surface (OpenAI chat/responses and Anthropic messages all build their +upstream URL there), so it flags the request; ``emit_request_outcome`` reads +the flag and relabels the provider. The flag is a task-local ContextVar, so it +never bleeds across concurrent requests. +""" + +import asyncio +import contextvars + +from headroom import copilot_auth +from headroom.proxy.outcome import RequestOutcome, emit_request_outcome + +COPILOT = "https://api.githubcopilot.com" + + +def _run_isolated(fn): + """Run ``fn`` in a fresh context so the per-request + ContextVar set by one test never leaks into the next.""" + return contextvars.Context().run(fn) + + +# --- chokepoint marking ----------------------------------------------------- + + +def test_build_url_marks_request_routed_to_copilot() -> None: + def scenario() -> bool: + assert copilot_auth.request_routed_to_copilot() is False + copilot_auth.build_copilot_upstream_url(COPILOT, "/v1/chat/completions") + return copilot_auth.request_routed_to_copilot() + + assert _run_isolated(scenario) is True + + +def test_build_url_does_not_mark_non_copilot_hosts() -> None: + def scenario() -> bool: + copilot_auth.build_copilot_upstream_url("https://api.openai.com", "/v1/chat/completions") + return copilot_auth.request_routed_to_copilot() + + assert _run_isolated(scenario) is False + + +def test_build_url_clears_stale_flag_for_non_copilot_hosts() -> None: + def scenario() -> bool: + copilot_auth.build_copilot_upstream_url(COPILOT, "/v1/messages") + assert copilot_auth.request_routed_to_copilot() is True + copilot_auth.build_copilot_upstream_url("https://api.openai.com", "/v1/chat/completions") + return copilot_auth.request_routed_to_copilot() + + assert _run_isolated(scenario) is False + + +# --- outcome relabeling ----------------------------------------------------- + + +class _Metrics: + def __init__(self) -> None: + self.failed: list[str] = [] + + async def record_failed(self, provider: str) -> None: + self.failed.append(provider) + + +class _Handler: + # Exposes ONLY .metrics: a >=500 outcome must relabel and then hit the + # failed-request guard without touching the success funnel. + def __init__(self) -> None: + self.metrics = _Metrics() + + +def _outcome(provider: str, status_code: int) -> RequestOutcome: + return RequestOutcome( + request_id="req-1", + provider=provider, + model="claude-opus-4-8", + original_tokens=0, + optimized_tokens=0, + output_tokens=0, + tokens_saved=0, + attempted_input_tokens=0, + status_code=status_code, + ) + + +def test_relabels_anthropic_to_copilot_when_routed() -> None: + def scenario() -> _Handler: + copilot_auth.build_copilot_upstream_url(COPILOT, "/v1/messages") + handler = _Handler() + asyncio.run(emit_request_outcome(handler, _outcome("anthropic", 503))) + return handler + + handler = _run_isolated(scenario) + # Relabeled before the 5xx guard, so even a failed Copilot request is + # attributed to "copilot" rather than the wire provider. + assert handler.metrics.failed == ["copilot"] + + +def test_relabels_openai_to_copilot_when_routed() -> None: + def scenario() -> _Handler: + copilot_auth.build_copilot_upstream_url(COPILOT, "/v1/chat/completions") + handler = _Handler() + asyncio.run(emit_request_outcome(handler, _outcome("openai", 503))) + return handler + + handler = _run_isolated(scenario) + assert handler.metrics.failed == ["copilot"] + + +def test_no_relabel_when_not_routed_to_copilot() -> None: + def scenario() -> _Handler: + handler = _Handler() + asyncio.run(emit_request_outcome(handler, _outcome("anthropic", 503))) + return handler + + handler = _run_isolated(scenario) + assert handler.metrics.failed == ["anthropic"] + + +def test_flag_does_not_leak_to_a_later_outcome_in_the_same_context() -> None: + # Regression: the Copilot flag is a ContextVar whose value persists until + # overwritten. Within one execution context (e.g. successive messages on a + # single long-lived WebSocket task), a Copilot request followed by a + # non-Copilot one must NOT relabel the second. emit_request_outcome consumes + # (reads AND clears) the flag, so only the first outcome is labeled copilot. + async def scenario() -> _Handler: + copilot_auth.build_copilot_upstream_url(COPILOT, "/v1/messages") + handler = _Handler() + await emit_request_outcome(handler, _outcome("anthropic", 503)) # routed + await emit_request_outcome(handler, _outcome("openai", 503)) # not routed + return handler + + handler = asyncio.run(scenario()) + assert handler.metrics.failed == ["copilot", "openai"]