mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(proxy): thread tags into 13 outcome sites + synth /v1/models + free-fn _extract_tags
Three fixes bundled; all in admin / cache-hit paths where tests didn't catch the regression. ## (A) 13 RequestOutcome sites missing tags= An AST audit found that 13 of 21 ``RequestOutcome(...)`` construction sites across the four handler files emitted outcomes without threading ``tags=``. Affected paths: * ``handle_anthropic_messages`` — the ``from_response_cache=True`` early-return outcome (Claude Code cache-hit turns dashboard-blind) * ``handle_openai_chat`` — same cache-hit early-return (Codex + Cursor + Continue cache-hit turns dashboard-blind) * ``handle_openai_responses_ws`` — the per-turn outcome inside the Codex WS session. The stale comment that said "ws_session_tags is not yet bound" was wrong — ``ws_tags`` was already extracted at handler entry * ``handle_anthropic_batch_create / batch_passthrough / batch_results`` * ``handle_passthrough`` (OpenAI Models / Files / List-Batches) * ``handle_google_batch_create / batch_passthrough / batch_results`` * ``_google_batch_passthrough`` (internal helper) * ``handle_batch_create`` (OpenAI batch entry) * ``handle_gemini_count_tokens`` (also fixed in #479; identical) Pattern of the fix is uniform: pull tags from headers and thread them into the ``RequestOutcome`` construction. New contract test ``test_handler_outcome_tag_invariant.py`` walks each handler file's AST and asserts every ``RequestOutcome`` site inside any ``handle_*`` or ``*_passthrough`` method passes both ``tags=`` and ``client=``. Future handlers get a clear test failure with file + line + method name if they regress. ## (B) Issue #478 — /v1/models 403 under Codex ChatGPT auth Codex Desktop with ChatGPT-subscription OAuth polls ``/v1/models`` to populate its model picker. Forwarding to ``chatgpt.com/backend-api/ models`` returned 403 to OAuth tokens. Fix: synthesize an OpenAI- compatible payload locally from a known-supported model set (``gpt-5.5`` through ``gpt-5``). All other ChatGPT-auth paths still forward as before — only model-metadata gets the local response. ## (C) Move _extract_tags to free function (mixin-isolation test compat) Handlers called ``self._extract_tags(headers)``. That worked in production where ``HeadroomProxy`` composes every mixin and defines the method, but broke tests that instantiate a single mixin via ``object.__new__(OpenAIHandlerMixin)``. The free-function form removes that coupling — handlers import ``extract_tags`` from ``headroom.proxy.helpers`` and call directly. ``HeadroomProxy. _extract_tags`` is kept as a thin wrapper for any external caller still using the method form. 17 call sites migrated. ## Zero behavior change for existing users Claude Code, Codex, Cursor, Continue, Aider, Gemini-routed harnesses all hit handlers that already extracted tags. Their wire bytes to upstream LLMs are byte-identical. Only the dashboard view gains tags on previously-blind paths. Closes #478.
This commit is contained in:
parent
88983ad34e
commit
3ec549288a
16 changed files with 426 additions and 50 deletions
|
|
@ -5,14 +5,14 @@
|
|||
},
|
||||
"metadata": {
|
||||
"description": "Headroom marketplace for Claude Code and GitHub Copilot CLI plugins.",
|
||||
"version": "0.22.0"
|
||||
"version": "0.21.39"
|
||||
},
|
||||
"plugins": [
|
||||
{
|
||||
"name": "headroom",
|
||||
"source": "./plugins/headroom-agent-hooks",
|
||||
"description": "Headroom startup hooks for Claude Code and GitHub Copilot CLI.",
|
||||
"version": "0.22.0",
|
||||
"version": "0.21.39",
|
||||
"author": {
|
||||
"name": "Headroom Contributors",
|
||||
"url": "https://github.com/chopratejas/headroom"
|
||||
|
|
|
|||
4
.github/plugin/marketplace.json
vendored
4
.github/plugin/marketplace.json
vendored
|
|
@ -5,14 +5,14 @@
|
|||
},
|
||||
"metadata": {
|
||||
"description": "Headroom marketplace for Claude Code and GitHub Copilot CLI plugins.",
|
||||
"version": "0.22.0"
|
||||
"version": "0.21.39"
|
||||
},
|
||||
"plugins": [
|
||||
{
|
||||
"name": "headroom",
|
||||
"source": "./plugins/headroom-agent-hooks",
|
||||
"description": "Headroom startup hooks for Claude Code and GitHub Copilot CLI.",
|
||||
"version": "0.22.0",
|
||||
"version": "0.21.39",
|
||||
"author": {
|
||||
"name": "Headroom Contributors",
|
||||
"url": "https://github.com/chopratejas/headroom"
|
||||
|
|
|
|||
|
|
@ -46,6 +46,83 @@ def _select_passthrough_base_url(proxy: Any, headers: dict[str, str]) -> str:
|
|||
return _api_target(proxy, provider_name)
|
||||
|
||||
|
||||
# Codex ChatGPT-subscription auth doesn't have access to
|
||||
# `chatgpt.com/backend-api/models` — that endpoint returns 403 to OAuth
|
||||
# bearer tokens (issue #478). Codex polls `/v1/models` every few seconds
|
||||
# to populate its model-picker UI, so the 403 storm is noisy and breaks
|
||||
# refresh. The fix: when Codex hits `/v1/models` under ChatGPT auth,
|
||||
# synthesize an OpenAI-compatible response from the known-supported
|
||||
# Codex/ChatGPT model set instead of forwarding to a 403.
|
||||
#
|
||||
# The list mirrors what Codex itself ships in its built-in model
|
||||
# registry (the same models its provider config exposes); it's the
|
||||
# safe-by-construction set since these are what `/v1/responses` actually
|
||||
# accepts under ChatGPT auth.
|
||||
_CHATGPT_AUTH_CODEX_MODELS: tuple[str, ...] = (
|
||||
"gpt-5.5",
|
||||
"gpt-5.4",
|
||||
"gpt-5.3",
|
||||
"gpt-5.2",
|
||||
"gpt-5.1",
|
||||
"gpt-5",
|
||||
)
|
||||
|
||||
|
||||
def _synthetic_models_list_response() -> Response:
|
||||
"""OpenAI-compatible `/v1/models` payload for Codex ChatGPT auth."""
|
||||
import json
|
||||
|
||||
payload = {
|
||||
"object": "list",
|
||||
"data": [
|
||||
{
|
||||
"id": model_id,
|
||||
"object": "model",
|
||||
"created": 0,
|
||||
"owned_by": "openai",
|
||||
}
|
||||
for model_id in _CHATGPT_AUTH_CODEX_MODELS
|
||||
],
|
||||
}
|
||||
return Response(
|
||||
content=json.dumps(payload),
|
||||
status_code=200,
|
||||
headers={"content-type": "application/json"},
|
||||
)
|
||||
|
||||
|
||||
def _synthetic_model_get_response(model_id: str) -> Response:
|
||||
"""OpenAI-compatible `/v1/models/{id}` payload."""
|
||||
import json
|
||||
|
||||
if model_id not in _CHATGPT_AUTH_CODEX_MODELS:
|
||||
return Response(
|
||||
content=json.dumps(
|
||||
{
|
||||
"error": {
|
||||
"message": f"Model {model_id!r} not available under ChatGPT auth",
|
||||
"type": "invalid_request_error",
|
||||
"code": "model_not_found",
|
||||
}
|
||||
}
|
||||
),
|
||||
status_code=404,
|
||||
headers={"content-type": "application/json"},
|
||||
)
|
||||
return Response(
|
||||
content=json.dumps(
|
||||
{
|
||||
"id": model_id,
|
||||
"object": "model",
|
||||
"created": 0,
|
||||
"owned_by": "openai",
|
||||
}
|
||||
),
|
||||
status_code=200,
|
||||
headers={"content-type": "application/json"},
|
||||
)
|
||||
|
||||
|
||||
async def _handle_chatgpt_model_metadata(
|
||||
proxy: Any,
|
||||
request: Request,
|
||||
|
|
@ -57,6 +134,15 @@ async def _handle_chatgpt_model_metadata(
|
|||
if not is_chatgpt_auth:
|
||||
return None
|
||||
|
||||
# Short-circuit `/backend-api/models[/{id}]` — chatgpt.com returns
|
||||
# 403 here for OAuth tokens; synthesize a Codex-compatible response
|
||||
# locally so the model-picker refresh succeeds (issue #478).
|
||||
if upstream_path == "/backend-api/models":
|
||||
return _synthetic_models_list_response()
|
||||
if upstream_path.startswith("/backend-api/models/"):
|
||||
model_id = upstream_path[len("/backend-api/models/") :]
|
||||
return _synthetic_model_get_response(model_id)
|
||||
|
||||
url = f"https://chatgpt.com{upstream_path}"
|
||||
if request.url.query:
|
||||
url = f"{url}?{request.url.query}"
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ import httpx
|
|||
from headroom.pipeline import PipelineStage, summarize_routing_markers
|
||||
from headroom.proxy.auth_mode import classify_auth_mode, classify_client
|
||||
from headroom.proxy.compression_decision import CompressionDecision
|
||||
from headroom.proxy.helpers import extract_tags
|
||||
from headroom.proxy.outcome import RequestOutcome
|
||||
|
||||
logger = logging.getLogger("headroom.proxy")
|
||||
|
|
@ -589,7 +590,7 @@ class AnthropicHandlerMixin:
|
|||
# the upstream can honor; if httpx lacks brotli support the response
|
||||
# body is undecipherable → 502.
|
||||
headers.pop("accept-encoding", None)
|
||||
tags = self._extract_tags(headers)
|
||||
tags = extract_tags(headers)
|
||||
# Identify the harness (codex / claude-code / aider / etc.)
|
||||
# from User-Agent or X-Client. Surfaced via the funnel into
|
||||
# PERF logs and RequestLog.tags — see RequestOutcome.client.
|
||||
|
|
@ -725,6 +726,7 @@ class AnthropicHandlerMixin:
|
|||
total_latency_ms=optimization_latency,
|
||||
overhead_ms=optimization_latency,
|
||||
num_messages=len(messages),
|
||||
tags=tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
@ -2349,6 +2351,7 @@ class AnthropicHandlerMixin:
|
|||
headers.pop("host", None)
|
||||
headers.pop("content-length", None)
|
||||
client = classify_client(headers)
|
||||
tags = extract_tags(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* before forwarding upstream.
|
||||
from headroom.proxy.helpers import _strip_internal_headers, log_outbound_headers
|
||||
|
||||
|
|
@ -2516,6 +2519,7 @@ class AnthropicHandlerMixin:
|
|||
overhead_ms=optimization_latency,
|
||||
pipeline_timing=pipeline_timing,
|
||||
num_messages=len(compressed_requests),
|
||||
tags=tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
@ -2599,6 +2603,7 @@ class AnthropicHandlerMixin:
|
|||
headers = dict(request.headers.items())
|
||||
headers.pop("host", None)
|
||||
client = classify_client(headers)
|
||||
tags = extract_tags(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* before forwarding upstream.
|
||||
from headroom.proxy.helpers import _strip_internal_headers, log_outbound_headers
|
||||
|
||||
|
|
@ -2634,6 +2639,7 @@ class AnthropicHandlerMixin:
|
|||
tokens_saved=0,
|
||||
attempted_input_tokens=0,
|
||||
total_latency_ms=latency_ms,
|
||||
tags=tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
@ -2720,6 +2726,7 @@ class AnthropicHandlerMixin:
|
|||
headers = dict(request.headers.items())
|
||||
headers.pop("host", None)
|
||||
client = classify_client(headers)
|
||||
tags = extract_tags(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* before forwarding upstream.
|
||||
from headroom.proxy.helpers import _strip_internal_headers, log_outbound_headers
|
||||
|
||||
|
|
@ -2811,6 +2818,7 @@ class AnthropicHandlerMixin:
|
|||
tokens_saved=0,
|
||||
attempted_input_tokens=0,
|
||||
total_latency_ms=latency_ms,
|
||||
tags=tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ if TYPE_CHECKING:
|
|||
from fastapi.responses import Response
|
||||
|
||||
from headroom.proxy.auth_mode import classify_client
|
||||
from headroom.proxy.helpers import extract_tags
|
||||
from headroom.proxy.outcome import RequestOutcome
|
||||
|
||||
logger = logging.getLogger("headroom.proxy")
|
||||
|
|
@ -103,6 +104,7 @@ class BatchHandlerMixin:
|
|||
headers.pop("host", None)
|
||||
headers.pop("content-length", None)
|
||||
client = classify_client(headers)
|
||||
tags = extract_tags(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* before forwarding upstream.
|
||||
from headroom.proxy.helpers import _strip_internal_headers, log_outbound_headers
|
||||
|
||||
|
|
@ -282,6 +284,7 @@ class BatchHandlerMixin:
|
|||
total_latency_ms=optimization_latency,
|
||||
overhead_ms=optimization_latency,
|
||||
pipeline_timing=pipeline_timing,
|
||||
tags=tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
@ -353,6 +356,7 @@ class BatchHandlerMixin:
|
|||
headers.pop("host", None)
|
||||
headers.pop("content-length", None)
|
||||
client = classify_client(headers)
|
||||
tags = extract_tags(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* before forwarding upstream.
|
||||
from headroom.proxy.helpers import _strip_internal_headers, log_outbound_headers
|
||||
|
||||
|
|
@ -432,6 +436,7 @@ class BatchHandlerMixin:
|
|||
tokens_saved=0,
|
||||
attempted_input_tokens=0,
|
||||
total_latency_ms=latency_ms,
|
||||
tags=tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
@ -471,6 +476,7 @@ class BatchHandlerMixin:
|
|||
headers = dict(request.headers.items())
|
||||
headers.pop("host", None)
|
||||
client = classify_client(headers)
|
||||
tags = extract_tags(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* before forwarding upstream.
|
||||
from headroom.proxy.helpers import _strip_internal_headers, log_outbound_headers
|
||||
|
||||
|
|
@ -515,6 +521,7 @@ class BatchHandlerMixin:
|
|||
tokens_saved=0,
|
||||
attempted_input_tokens=0,
|
||||
total_latency_ms=latency_ms,
|
||||
tags=tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
@ -612,6 +619,7 @@ class BatchHandlerMixin:
|
|||
headers = dict(request.headers.items())
|
||||
headers.pop("host", None)
|
||||
client = classify_client(headers)
|
||||
tags = extract_tags(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* before forwarding upstream.
|
||||
from headroom.proxy.helpers import _strip_internal_headers, log_outbound_headers
|
||||
|
||||
|
|
@ -733,6 +741,7 @@ class BatchHandlerMixin:
|
|||
tokens_saved=0,
|
||||
attempted_input_tokens=0,
|
||||
total_latency_ms=latency_ms,
|
||||
tags=tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
@ -811,6 +820,7 @@ class BatchHandlerMixin:
|
|||
headers.pop("host", None)
|
||||
headers.pop("content-length", None)
|
||||
client = classify_client(headers)
|
||||
tags = extract_tags(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* before forwarding upstream.
|
||||
from headroom.proxy.helpers import _strip_internal_headers, log_outbound_headers
|
||||
|
||||
|
|
@ -954,6 +964,7 @@ class BatchHandlerMixin:
|
|||
attempted_input_tokens=stats["total_compressed_tokens"]
|
||||
+ stats["total_tokens_saved"],
|
||||
total_latency_ms=total_latency,
|
||||
tags=tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ if TYPE_CHECKING:
|
|||
|
||||
from headroom.proxy.auth_mode import classify_client
|
||||
from headroom.proxy.compression_decision import CompressionDecision
|
||||
from headroom.proxy.helpers import extract_tags
|
||||
from headroom.proxy.outcome import RequestOutcome
|
||||
|
||||
logger = logging.getLogger("headroom.proxy")
|
||||
|
|
@ -194,7 +195,7 @@ class GeminiHandlerMixin:
|
|||
headers = dict(request.headers.items())
|
||||
headers.pop("host", None)
|
||||
headers.pop("content-length", None)
|
||||
tags = self._extract_tags(headers)
|
||||
tags = extract_tags(headers)
|
||||
client = classify_client(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* from upstream-bound
|
||||
# headers AFTER `_extract_tags` reads them. Memory user-id reads
|
||||
|
|
@ -604,7 +605,7 @@ class GeminiHandlerMixin:
|
|||
headers.pop("host", None)
|
||||
headers.pop("content-length", None)
|
||||
headers.pop("accept-encoding", None)
|
||||
tags = self._extract_tags(headers)
|
||||
tags = extract_tags(headers)
|
||||
# Note: streaming handlers delegate to _stream_response, which
|
||||
# does its own classify_client. No need to compute here.
|
||||
is_antigravity = self._is_cloudcode_antigravity_request(body, headers)
|
||||
|
|
@ -743,7 +744,7 @@ class GeminiHandlerMixin:
|
|||
headers = dict(request.headers.items())
|
||||
headers.pop("host", None)
|
||||
headers.pop("content-length", None)
|
||||
tags = self._extract_tags(headers)
|
||||
tags = extract_tags(headers)
|
||||
# Streaming variant — delegates to _stream_response which
|
||||
# classifies the client itself from headers.
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* before forwarding upstream.
|
||||
|
|
@ -831,6 +832,7 @@ class GeminiHandlerMixin:
|
|||
headers.pop("host", None)
|
||||
headers.pop("content-length", None)
|
||||
client = classify_client(headers)
|
||||
tags = extract_tags(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* before forwarding upstream.
|
||||
from headroom.proxy.helpers import _strip_internal_headers, log_outbound_headers
|
||||
|
||||
|
|
@ -882,7 +884,7 @@ class GeminiHandlerMixin:
|
|||
# out of headers; sibling handlers do and thread them into the
|
||||
# outcome. Extract here so apply_to_tags below has a dict to
|
||||
# mutate and the outcome at end-of-call inherits the tag.
|
||||
tags = self._extract_tags(request.headers)
|
||||
tags = extract_tags(request.headers)
|
||||
_decision = CompressionDecision.decide(
|
||||
headers=request.headers,
|
||||
config=self.config,
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ from typing import TYPE_CHECKING, Any
|
|||
from headroom.proxy.helpers import (
|
||||
COMPRESSION_TIMEOUT_SECONDS,
|
||||
_headroom_bypass_enabled,
|
||||
extract_tags,
|
||||
jitter_delay_ms,
|
||||
)
|
||||
from headroom.proxy.stage_timer import StageTimer, emit_stage_timings_log
|
||||
|
|
@ -1226,7 +1227,7 @@ class OpenAIHandlerMixin:
|
|||
# Cloudflare Workers forward "br, zstd" which OpenAI may honor;
|
||||
# if httpx lacks brotli support the response body is undecipherable → 502.
|
||||
headers.pop("accept-encoding", None)
|
||||
tags = self._extract_tags(headers)
|
||||
tags = extract_tags(headers)
|
||||
client = classify_client(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* from upstream-bound
|
||||
# headers AFTER `_extract_tags` reads them. Inbound bypass gating
|
||||
|
|
@ -1313,6 +1314,7 @@ class OpenAIHandlerMixin:
|
|||
from_response_cache=True,
|
||||
total_latency_ms=_cache_hit_latency,
|
||||
num_messages=len(messages),
|
||||
tags=tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
@ -2313,7 +2315,7 @@ class OpenAIHandlerMixin:
|
|||
# Cloudflare Workers forward "br, zstd" which OpenAI may honor;
|
||||
# if httpx lacks brotli support the response body is undecipherable → 502.
|
||||
headers.pop("accept-encoding", None)
|
||||
tags = self._extract_tags(headers)
|
||||
tags = extract_tags(headers)
|
||||
client = classify_client(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* from upstream-bound
|
||||
# headers AFTER `_extract_tags` reads them. Memory user-id reads
|
||||
|
|
@ -4239,11 +4241,12 @@ class OpenAIHandlerMixin:
|
|||
# traffic was invisible to
|
||||
# ``headroom perf`` and the recent-
|
||||
# requests feed. Funnel restores all
|
||||
# four effects uniformly per turn.
|
||||
# ``ws_session_tags`` is not yet bound
|
||||
# at per-turn time (set at session-end
|
||||
# below); pass empty so the funnel
|
||||
# gets a real dict.
|
||||
# four effects uniformly per turn. Per-
|
||||
# turn outcomes carry ``ws_tags`` (the
|
||||
# `x-headroom-tag-*` headers extracted
|
||||
# at the WS upgrade) so dashboards can
|
||||
# slice WS turns by tag — same surface
|
||||
# as HTTP turns.
|
||||
await self._record_request_outcome(
|
||||
RequestOutcome(
|
||||
request_id=request_id,
|
||||
|
|
@ -4267,6 +4270,7 @@ class OpenAIHandlerMixin:
|
|||
)
|
||||
if isinstance(body, dict)
|
||||
else 0,
|
||||
tags=ws_tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
@ -5332,6 +5336,7 @@ class OpenAIHandlerMixin:
|
|||
headers.pop("host", None)
|
||||
headers.pop("accept-encoding", None)
|
||||
client = classify_client(headers)
|
||||
tags = extract_tags(headers)
|
||||
# PR-A5 (P5-49): strip internal x-headroom-* before forwarding upstream.
|
||||
from headroom.proxy.helpers import _strip_internal_headers, log_outbound_headers
|
||||
|
||||
|
|
@ -5397,6 +5402,7 @@ class OpenAIHandlerMixin:
|
|||
tokens_saved=0,
|
||||
attempted_input_tokens=0,
|
||||
total_latency_ms=latency_ms,
|
||||
tags=tags,
|
||||
client=client,
|
||||
)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -292,6 +292,26 @@ def get_python_forwarder_mode() -> PythonForwarderMode:
|
|||
)
|
||||
|
||||
|
||||
def extract_tags(headers: Any) -> dict[str, str]:
|
||||
"""Extract ``x-headroom-*`` tags from inbound headers.
|
||||
|
||||
Pure function (no I/O, no state). Used by every handler at request
|
||||
entry to capture operator slicing tags into the per-request
|
||||
``RequestOutcome.tags``. Free function rather than a mixin method so
|
||||
handler mixins instantiated in isolation (tests using
|
||||
``object.__new__(OpenAIHandlerMixin)``) don't need a shim
|
||||
implementation.
|
||||
|
||||
Header name match is case-insensitive; the returned key has the
|
||||
``x-headroom-`` prefix stripped.
|
||||
"""
|
||||
return {
|
||||
k.lower().replace("x-headroom-", ""): v
|
||||
for k, v in headers.items()
|
||||
if k.lower().startswith("x-headroom-")
|
||||
}
|
||||
|
||||
|
||||
def _headroom_bypass_enabled(headers: Any) -> bool:
|
||||
"""Return True when inbound headers request full Headroom passthrough.
|
||||
|
||||
|
|
|
|||
|
|
@ -1174,13 +1174,14 @@ class HeadroomProxy(
|
|||
return f"hr_{int(time.time())}_{self._request_counter:06d}"
|
||||
|
||||
def _extract_tags(self, headers: dict) -> dict[str, str]:
|
||||
"""Extract Headroom tags from headers."""
|
||||
tags = {}
|
||||
for key, value in headers.items():
|
||||
if key.lower().startswith("x-headroom-"):
|
||||
tag_name = key.lower().replace("x-headroom-", "")
|
||||
tags[tag_name] = value
|
||||
return tags
|
||||
"""Backwards-compat wrapper around :func:`extract_tags`.
|
||||
|
||||
Handlers call ``extract_tags(headers)`` directly. Kept here for
|
||||
any external caller still using ``proxy._extract_tags(headers)``.
|
||||
"""
|
||||
from headroom.proxy.helpers import extract_tags
|
||||
|
||||
return extract_tags(headers)
|
||||
|
||||
async def _retry_request(
|
||||
self,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "headroom",
|
||||
"version": "0.22.0",
|
||||
"version": "0.21.39",
|
||||
"description": "Headroom startup hooks for Claude Code and GitHub Copilot CLI.",
|
||||
"author": {
|
||||
"name": "Headroom Contributors",
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "headroom",
|
||||
"version": "0.22.0",
|
||||
"version": "0.21.39",
|
||||
"description": "Headroom startup hooks for Claude Code and GitHub Copilot CLI.",
|
||||
"author": {
|
||||
"name": "Headroom Contributors",
|
||||
|
|
|
|||
118
tests/test_handler_outcome_tag_invariant.py
Normal file
118
tests/test_handler_outcome_tag_invariant.py
Normal file
|
|
@ -0,0 +1,118 @@
|
|||
"""Contract test: every handler that emits a RequestOutcome must thread tags.
|
||||
|
||||
Prior to PR #480, 12 RequestOutcome construction sites across four
|
||||
handler files emitted outcomes without passing ``tags=`` — so any
|
||||
request hitting those handlers reached the dashboard / RequestLog feed
|
||||
with an empty tag dict, invisible to per-harness / per-tag filtering.
|
||||
Affected paths included:
|
||||
|
||||
* The `from_response_cache=True` early-return paths in
|
||||
``handle_anthropic_messages`` and ``handle_openai_chat`` (so Claude
|
||||
Code's + Codex's cache-hit turns were dashboard-blind)
|
||||
* The Codex WS per-turn outcome in ``handle_openai_responses_ws``
|
||||
* All four Anthropic batch handlers, all four Google batch handlers,
|
||||
the OpenAI batch handler, and the OpenAI passthrough handler
|
||||
|
||||
This test introspects the four handler modules' ASTs and asserts that
|
||||
every ``RequestOutcome(...)`` keyword-call inside any handler-shaped
|
||||
method passes a ``tags=`` kwarg. The check is static; no handler is
|
||||
invoked. ``from_stream`` classmethod construction is allowed (it
|
||||
takes ``tags`` as a required kwarg) and the test verifies that too.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
HANDLER_FILES = [
|
||||
Path("headroom/proxy/handlers/anthropic.py"),
|
||||
Path("headroom/proxy/handlers/openai.py"),
|
||||
Path("headroom/proxy/handlers/gemini.py"),
|
||||
Path("headroom/proxy/handlers/batch.py"),
|
||||
]
|
||||
|
||||
|
||||
def _collect_outcome_call_sites() -> list[tuple[Path, str, int, set[str]]]:
|
||||
"""Walk each handler module's AST; for every
|
||||
``RequestOutcome(...)`` or ``RequestOutcome.from_stream(...)`` call
|
||||
inside any ``async def handle_*`` or ``async def _*_passthrough``
|
||||
method, record (file, method_name, lineno, kwarg_keys).
|
||||
"""
|
||||
sites: list[tuple[Path, str, int, set[str]]] = []
|
||||
for file_path in HANDLER_FILES:
|
||||
source = file_path.read_text(encoding="utf-8")
|
||||
tree = ast.parse(source, filename=str(file_path))
|
||||
for module_node in ast.walk(tree):
|
||||
if not isinstance(module_node, ast.ClassDef):
|
||||
continue
|
||||
for class_node in module_node.body:
|
||||
if not isinstance(class_node, ast.AsyncFunctionDef):
|
||||
continue
|
||||
method_name = class_node.name
|
||||
# Only audit methods that look like request entry points
|
||||
# or batch-passthrough helpers. ``_record_request_outcome``
|
||||
# itself is a helper, not a handler — skip it.
|
||||
if not (method_name.startswith("handle_") or method_name.endswith("_passthrough")):
|
||||
continue
|
||||
for sub_node in ast.walk(class_node):
|
||||
if not isinstance(sub_node, ast.Call):
|
||||
continue
|
||||
# Match RequestOutcome(...) or RequestOutcome.from_stream(...)
|
||||
is_outcome = False
|
||||
if isinstance(sub_node.func, ast.Name) and sub_node.func.id == "RequestOutcome":
|
||||
is_outcome = True
|
||||
elif (
|
||||
isinstance(sub_node.func, ast.Attribute)
|
||||
and isinstance(sub_node.func.value, ast.Name)
|
||||
and sub_node.func.value.id == "RequestOutcome"
|
||||
and sub_node.func.attr == "from_stream"
|
||||
):
|
||||
is_outcome = True
|
||||
if not is_outcome:
|
||||
continue
|
||||
kwarg_keys = {kw.arg for kw in sub_node.keywords if kw.arg is not None}
|
||||
sites.append(
|
||||
(file_path, method_name, sub_node.lineno, kwarg_keys),
|
||||
)
|
||||
return sites
|
||||
|
||||
|
||||
def test_outcome_call_sites_pass_tags_kwarg() -> None:
|
||||
"""Every RequestOutcome construction inside a handler method MUST
|
||||
pass ``tags=``. Otherwise the dashboard's tag-based slicing is
|
||||
silently bypassed for that traffic path.
|
||||
|
||||
If this test fails on a new handler you just wrote, add
|
||||
``tags = self._extract_tags(headers)`` near the top of your handler
|
||||
and thread ``tags=tags`` into the RequestOutcome construction.
|
||||
"""
|
||||
sites = _collect_outcome_call_sites()
|
||||
assert sites, "AST walk found zero RequestOutcome sites — handler files moved?"
|
||||
missing = [(f, m, ln) for f, m, ln, kws in sites if "tags" not in kws]
|
||||
if missing:
|
||||
formatted = "\n".join(f" {f.name}:{ln} {m}" for f, m, ln in missing)
|
||||
pytest.fail(
|
||||
f"{len(missing)} RequestOutcome sites miss `tags=`:\n{formatted}\n\n"
|
||||
"Each handler MUST extract tags from headers and thread them "
|
||||
"into the outcome construction. See PR #480 for the pattern."
|
||||
)
|
||||
|
||||
|
||||
def test_outcome_call_sites_pass_client_kwarg() -> None:
|
||||
"""Sibling invariant: every RequestOutcome from a handler also
|
||||
threads ``client=``. We have this everywhere today; this test
|
||||
locks it so future handlers can't regress."""
|
||||
sites = _collect_outcome_call_sites()
|
||||
assert sites
|
||||
missing = [(f, m, ln) for f, m, ln, kws in sites if "client" not in kws]
|
||||
if missing:
|
||||
formatted = "\n".join(f" {f.name}:{ln} {m}" for f, m, ln in missing)
|
||||
pytest.fail(
|
||||
f"{len(missing)} RequestOutcome sites miss `client=`:\n{formatted}\n\n"
|
||||
"Each handler MUST classify the harness via "
|
||||
"`client = classify_client(headers)` and thread `client=client` "
|
||||
"into the outcome construction. See PR #473 for the pattern."
|
||||
)
|
||||
|
|
@ -337,3 +337,84 @@ def test_gemini_batch_embed_contents_passthrough_uses_gemini_target(monkeypatch)
|
|||
assert calls == [
|
||||
("/v1beta/models/demo:batchEmbedContents", "https://api.gemini.test", "batchEmbedContents")
|
||||
]
|
||||
|
||||
|
||||
def test_v1_models_returns_synthetic_list_under_chatgpt_auth() -> None:
|
||||
"""Issue #478: under Codex ChatGPT-subscription OAuth, the proxy
|
||||
must NOT forward `/v1/models` to chatgpt.com/backend-api/models
|
||||
(which returns 403). Synthesize an OpenAI-compatible response with
|
||||
the known-supported Codex/ChatGPT model set instead, so Codex's
|
||||
model-picker refresh succeeds.
|
||||
"""
|
||||
with TestClient(_app()) as client:
|
||||
# ChatGPT auth detected via Bearer + ChatGPT account header
|
||||
# (mirrors what Codex Desktop sends).
|
||||
response = client.get(
|
||||
"/v1/models",
|
||||
headers={
|
||||
"authorization": "Bearer eyJ-chatgpt-oauth-token",
|
||||
"chatgpt-account-id": "test-account",
|
||||
"originator": "Codex Desktop",
|
||||
},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
payload = response.json()
|
||||
assert payload["object"] == "list"
|
||||
assert isinstance(payload["data"], list)
|
||||
assert len(payload["data"]) > 0
|
||||
model_ids = {entry["id"] for entry in payload["data"]}
|
||||
# Spot-check: the model from issue #478's repro log must be present.
|
||||
assert "gpt-5.5" in model_ids
|
||||
for entry in payload["data"]:
|
||||
assert entry["object"] == "model"
|
||||
assert entry["owned_by"] == "openai"
|
||||
|
||||
|
||||
def test_v1_models_get_single_synthetic_under_chatgpt_auth() -> None:
|
||||
"""The single-model variant (`/v1/models/{id}`) is also called by
|
||||
Codex for some flows. Must return a synthetic OpenAI-compatible
|
||||
payload for known models, 404 for unknown."""
|
||||
with TestClient(_app()) as client:
|
||||
ok = client.get(
|
||||
"/v1/models/gpt-5.5",
|
||||
headers={
|
||||
"authorization": "Bearer eyJ-chatgpt-oauth-token",
|
||||
"chatgpt-account-id": "test-account",
|
||||
},
|
||||
)
|
||||
unknown = client.get(
|
||||
"/v1/models/gpt-99-future",
|
||||
headers={
|
||||
"authorization": "Bearer eyJ-chatgpt-oauth-token",
|
||||
"chatgpt-account-id": "test-account",
|
||||
},
|
||||
)
|
||||
assert ok.status_code == 200
|
||||
assert ok.json() == {
|
||||
"id": "gpt-5.5",
|
||||
"object": "model",
|
||||
"created": 0,
|
||||
"owned_by": "openai",
|
||||
}
|
||||
assert unknown.status_code == 404
|
||||
|
||||
|
||||
def test_v1_models_still_forwards_under_non_chatgpt_auth() -> None:
|
||||
"""Non-ChatGPT auth (regular API key, Gemini, etc.) must still
|
||||
forward to the upstream provider — only the ChatGPT-OAuth path
|
||||
short-circuits to the synthetic response."""
|
||||
calls: list[tuple[str, str, str]] = []
|
||||
|
||||
async def fake_passthrough(self, request, base_url, sub_path="", provider_name=""): # type: ignore[no-untyped-def]
|
||||
calls.append((request.url.path, base_url, provider_name))
|
||||
return JSONResponse({"base_url": base_url, "provider": provider_name})
|
||||
|
||||
with patch.object(HeadroomProxy, "handle_passthrough", fake_passthrough):
|
||||
with TestClient(_app()) as client:
|
||||
response = client.get(
|
||||
"/v1/models",
|
||||
headers={"authorization": "Bearer sk-real-api-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
# Forwarded — not synthesized — because no chatgpt-account-id header.
|
||||
assert calls, "Non-ChatGPT-auth /v1/models must forward, not synthesize"
|
||||
|
|
|
|||
|
|
@ -159,23 +159,18 @@ def test_codex_responses_subpath_passthrough_derives_chatgpt_routing_from_jwt(pa
|
|||
assert headers["ChatGPT-Account-ID"] == "acct-from-jwt"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("path", "expected_url"),
|
||||
[
|
||||
(
|
||||
"/v1/models?client_version=0.130.0",
|
||||
"https://chatgpt.com/backend-api/models?client_version=0.130.0",
|
||||
),
|
||||
(
|
||||
"/v1/models/gpt-5.3-codex?client_version=0.130.0",
|
||||
"https://chatgpt.com/backend-api/models/gpt-5.3-codex?client_version=0.130.0",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_codex_model_metadata_routes_to_chatgpt_backend_for_subscription_auth(
|
||||
path,
|
||||
expected_url,
|
||||
):
|
||||
def test_codex_model_metadata_synthesises_response_for_chatgpt_auth():
|
||||
"""Issue #478: under Codex ChatGPT-subscription OAuth, the proxy
|
||||
must NOT forward `/v1/models[/{id}]` to chatgpt.com/backend-api —
|
||||
that endpoint returns 403 to OAuth tokens, breaking Codex's model
|
||||
refresh. Headroom synthesises an OpenAI-compatible payload locally
|
||||
so the picker UI populates correctly.
|
||||
|
||||
The earlier version of this test asserted the buggy forward
|
||||
behaviour; flipped here to lock the synthetic-response contract
|
||||
so the bug can't return.
|
||||
"""
|
||||
|
||||
class FakeAsyncClient:
|
||||
def __init__(self):
|
||||
self.calls: list[tuple[str, str, dict[str, str]]] = []
|
||||
|
|
@ -200,13 +195,42 @@ def test_codex_model_metadata_routes_to_chatgpt_backend_for_subscription_auth(
|
|||
client.app.state.proxy.http_client = fake_http_client
|
||||
client.app.state.proxy.OPENAI_API_URL = "https://api.openai.test"
|
||||
|
||||
response = client.get(path, headers={"Authorization": f"Bearer {token}"})
|
||||
list_response = client.get(
|
||||
"/v1/models?client_version=0.130.0",
|
||||
headers={"Authorization": f"Bearer {token}"},
|
||||
)
|
||||
known_response = client.get(
|
||||
"/v1/models/gpt-5.5",
|
||||
headers={"Authorization": f"Bearer {token}"},
|
||||
)
|
||||
unknown_response = client.get(
|
||||
"/v1/models/gpt-5.3-codex?client_version=0.130.0",
|
||||
headers={"Authorization": f"Bearer {token}"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert len(fake_http_client.calls) == 1
|
||||
# No upstream call must be made — the response is synthesised locally.
|
||||
assert fake_http_client.calls == [], (
|
||||
f"Expected zero upstream calls; got {fake_http_client.calls}"
|
||||
)
|
||||
|
||||
method, url, headers = fake_http_client.calls[0]
|
||||
assert method == "GET"
|
||||
assert url == expected_url
|
||||
assert headers["authorization"] == f"Bearer {token}"
|
||||
assert headers["ChatGPT-Account-ID"] == "acct-from-jwt"
|
||||
# List endpoint returns a non-empty OpenAI-compatible list.
|
||||
assert list_response.status_code == 200
|
||||
list_payload = list_response.json()
|
||||
assert list_payload["object"] == "list"
|
||||
assert isinstance(list_payload["data"], list)
|
||||
assert len(list_payload["data"]) > 0
|
||||
assert "gpt-5.5" in {entry["id"] for entry in list_payload["data"]}
|
||||
|
||||
# Single-model GET returns a model object when known.
|
||||
assert known_response.status_code == 200
|
||||
known_payload = known_response.json()
|
||||
assert known_payload == {
|
||||
"id": "gpt-5.5",
|
||||
"object": "model",
|
||||
"created": 0,
|
||||
"owned_by": "openai",
|
||||
}
|
||||
|
||||
# Unknown model variants 404 — the synthetic set is bounded to the
|
||||
# ChatGPT-auth-supported list.
|
||||
assert unknown_response.status_code == 404
|
||||
|
|
|
|||
|
|
@ -116,6 +116,16 @@ def test_openai_passthrough_applies_copilot_auth(monkeypatch: pytest.MonkeyPatch
|
|||
|
||||
await emit_request_outcome(self, outcome)
|
||||
|
||||
def _extract_tags(self, headers: dict) -> dict[str, str]:
|
||||
# Mirror of HeadroomProxy._extract_tags. The passthrough
|
||||
# handler now extracts tags at entry as part of the
|
||||
# outcome-tag invariant lock (PR #480).
|
||||
return {
|
||||
k.lower().replace("x-headroom-", ""): v
|
||||
for k, v in headers.items()
|
||||
if k.lower().startswith("x-headroom-")
|
||||
}
|
||||
|
||||
async def _request(self, **kwargs): # noqa: ANN003
|
||||
seen["request_kwargs"] = kwargs
|
||||
return SimpleNamespace(headers={}, content=b"{}", status_code=200)
|
||||
|
|
|
|||
|
|
@ -101,6 +101,15 @@ class DummyBatchHandler(batch_module.BatchHandlerMixin):
|
|||
|
||||
await emit_request_outcome(self, outcome)
|
||||
|
||||
def _extract_tags(self, headers: dict) -> dict[str, str]:
|
||||
# Mirror of HeadroomProxy._extract_tags. Handlers now call this
|
||||
# at entry to capture x-headroom-* slicing tags into the outcome.
|
||||
return {
|
||||
k.lower().replace("x-headroom-", ""): v
|
||||
for k, v in headers.items()
|
||||
if k.lower().startswith("x-headroom-")
|
||||
}
|
||||
|
||||
async def handle_passthrough(self, request, base_url): # noqa: ANN001, ANN201
|
||||
return {"request": request, "base_url": base_url}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue