From 3ec549288aed46dc46067b6dc55f287a8b19d218 Mon Sep 17 00:00:00 2001 From: chopratejas Date: Fri, 15 May 2026 17:54:24 -0700 Subject: [PATCH] fix(proxy): thread tags into 13 outcome sites + synth /v1/models + free-fn _extract_tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .claude-plugin/marketplace.json | 4 +- .github/plugin/marketplace.json | 4 +- headroom/providers/proxy_routes.py | 86 +++++++++++++ headroom/proxy/handlers/anthropic.py | 10 +- headroom/proxy/handlers/batch.py | 11 ++ headroom/proxy/handlers/gemini.py | 10 +- headroom/proxy/handlers/openai.py | 20 +-- headroom/proxy/helpers.py | 20 +++ headroom/proxy/server.py | 15 +-- .../.claude-plugin/plugin.json | 2 +- .../.github/plugin/plugin.json | 2 +- tests/test_handler_outcome_tag_invariant.py | 118 ++++++++++++++++++ tests/test_provider_proxy_routes.py | 81 ++++++++++++ tests/test_proxy_codex_route_aliases.py | 74 +++++++---- tests/test_proxy_copilot_auth_hooks.py | 10 ++ tests/test_proxy_handlers_batch.py | 9 ++ 16 files changed, 426 insertions(+), 50 deletions(-) create mode 100644 tests/test_handler_outcome_tag_invariant.py diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index d321c0a7c..68995e1de 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -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" diff --git a/.github/plugin/marketplace.json b/.github/plugin/marketplace.json index d321c0a7c..68995e1de 100644 --- a/.github/plugin/marketplace.json +++ b/.github/plugin/marketplace.json @@ -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" diff --git a/headroom/providers/proxy_routes.py b/headroom/providers/proxy_routes.py index 785e6def9..96a006db6 100644 --- a/headroom/providers/proxy_routes.py +++ b/headroom/providers/proxy_routes.py @@ -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}" diff --git a/headroom/proxy/handlers/anthropic.py b/headroom/proxy/handlers/anthropic.py index 7262186c7..4d04f9d3a 100644 --- a/headroom/proxy/handlers/anthropic.py +++ b/headroom/proxy/handlers/anthropic.py @@ -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, ) ) diff --git a/headroom/proxy/handlers/batch.py b/headroom/proxy/handlers/batch.py index 4009783f3..febd8cba9 100644 --- a/headroom/proxy/handlers/batch.py +++ b/headroom/proxy/handlers/batch.py @@ -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, ) ) diff --git a/headroom/proxy/handlers/gemini.py b/headroom/proxy/handlers/gemini.py index ef847a5f5..e105e9bfd 100644 --- a/headroom/proxy/handlers/gemini.py +++ b/headroom/proxy/handlers/gemini.py @@ -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, diff --git a/headroom/proxy/handlers/openai.py b/headroom/proxy/handlers/openai.py index ffb4df40a..04f27cc6a 100644 --- a/headroom/proxy/handlers/openai.py +++ b/headroom/proxy/handlers/openai.py @@ -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, ) ) diff --git a/headroom/proxy/helpers.py b/headroom/proxy/helpers.py index f99170446..622dc9170 100644 --- a/headroom/proxy/helpers.py +++ b/headroom/proxy/helpers.py @@ -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. diff --git a/headroom/proxy/server.py b/headroom/proxy/server.py index 41001a7ad..843555f8b 100644 --- a/headroom/proxy/server.py +++ b/headroom/proxy/server.py @@ -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, diff --git a/plugins/headroom-agent-hooks/.claude-plugin/plugin.json b/plugins/headroom-agent-hooks/.claude-plugin/plugin.json index 218caf453..f25139f31 100644 --- a/plugins/headroom-agent-hooks/.claude-plugin/plugin.json +++ b/plugins/headroom-agent-hooks/.claude-plugin/plugin.json @@ -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", diff --git a/plugins/headroom-agent-hooks/.github/plugin/plugin.json b/plugins/headroom-agent-hooks/.github/plugin/plugin.json index 0116ff27b..b94fc5de5 100644 --- a/plugins/headroom-agent-hooks/.github/plugin/plugin.json +++ b/plugins/headroom-agent-hooks/.github/plugin/plugin.json @@ -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", diff --git a/tests/test_handler_outcome_tag_invariant.py b/tests/test_handler_outcome_tag_invariant.py new file mode 100644 index 000000000..33c1a0517 --- /dev/null +++ b/tests/test_handler_outcome_tag_invariant.py @@ -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." + ) diff --git a/tests/test_provider_proxy_routes.py b/tests/test_provider_proxy_routes.py index d75bd80aa..23e2c0460 100644 --- a/tests/test_provider_proxy_routes.py +++ b/tests/test_provider_proxy_routes.py @@ -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" diff --git a/tests/test_proxy_codex_route_aliases.py b/tests/test_proxy_codex_route_aliases.py index 909a21129..d3d38dd7f 100644 --- a/tests/test_proxy_codex_route_aliases.py +++ b/tests/test_proxy_codex_route_aliases.py @@ -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 diff --git a/tests/test_proxy_copilot_auth_hooks.py b/tests/test_proxy_copilot_auth_hooks.py index e2d7714fd..99761c08b 100644 --- a/tests/test_proxy_copilot_auth_hooks.py +++ b/tests/test_proxy_copilot_auth_hooks.py @@ -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) diff --git a/tests/test_proxy_handlers_batch.py b/tests/test_proxy_handlers_batch.py index 700222346..bfe6788d6 100644 --- a/tests/test_proxy_handlers_batch.py +++ b/tests/test_proxy_handlers_batch.py @@ -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}