mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(copilot): route mixed-model requests per model (#1785)
## Description Copilot subscription sessions can mix a chat-completions main model with a Responses-only internal bootstrap model. The wrapper still seeds one `COPILOT_PROVIDER_WIRE_API` value for launch-time compatibility, but the proxy now chooses the Copilot upstream path per request model inside OpenAI chat dispatch. That keeps `gpt-5.4-mini` on `/responses` while `claude-sonnet-5` stays on `/chat/completions`. Closes #1745 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Added a narrow OpenAI chat-handler path resolver that reuses the existing Copilot model heuristic and only switches Copilot-hosted requests to `/responses` when the model already prefers Responses. - Threaded that resolved path through the OpenAI chat handler so request logs, cache hits, and upstream dispatch all reflect the actual per-request route. - Added a regression test that captures the upstream URL for `gpt-5.4-mini`, preserves the `claude-sonnet-5` control case, and keeps the non-Copilot control case on chat completions through the same path resolver composition used by the handler. - Left the Copilot launch wrapper behavior intact, so the existing subscription env defaults still serialize the same way at launch. - Preserved the existing invalid/custom upstream base URL fallback behavior while applying the Copilot-only per-model route switch. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_proxy_copilot_auth_hooks.py tests/test_cli/test_wrap_copilot.py tests/test_proxy/test_openai_transport_path_prefix.py tests/test_proxy/test_openai_upstream_header.py -q`) - [x] Linting passes (`uv run ruff check headroom/proxy/handlers/openai.py headroom/cli/wrap.py tests/test_proxy_copilot_auth_hooks.py tests/test_cli/test_wrap_copilot.py tests/test_proxy/test_openai_transport_path_prefix.py tests/test_proxy/test_openai_upstream_header.py`; `uv run ruff format --check headroom/proxy/handlers/openai.py tests/test_proxy_copilot_auth_hooks.py`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text collected 44 items tests\test_proxy_copilot_auth_hooks.py ... [ 6%] tests\test_cli\test_wrap_copilot.py .............................. [ 75%] tests\test_proxy\test_openai_transport_path_prefix.py ....... [ 90%] tests\test_proxy\test_openai_upstream_header.py .... [100%] ======================== 44 passed, 1 warning in 1.38s ======================== All checks passed! 2 files already formatted ``` ## Real Behavior Proof - Environment: Windows, Python 3.12.13, focused local proxy tests. - Exact command / steps: `uv run pytest tests/test_proxy_copilot_auth_hooks.py tests/test_cli/test_wrap_copilot.py tests/test_proxy/test_openai_transport_path_prefix.py tests/test_proxy/test_openai_upstream_header.py -q`, then `uv run ruff check headroom/proxy/handlers/openai.py headroom/cli/wrap.py tests/test_proxy_copilot_auth_hooks.py tests/test_cli/test_wrap_copilot.py tests/test_proxy/test_openai_transport_path_prefix.py tests/test_proxy/test_openai_upstream_header.py`, then `uv run ruff format --check headroom/proxy/handlers/openai.py tests/test_proxy_copilot_auth_hooks.py`. - Observed result: the new proxy regression test saw `https://api.githubcopilot.com/responses` for `gpt-5.4-mini` and `https://api.githubcopilot.com/chat/completions` for `claude-sonnet-5`; the non-Copilot control stayed on `/v1/chat/completions`, invalid base URL fallbacks kept the configured OpenAI `/v1` route, and the wrap regression tests still passed unchanged. - Not tested: live GitHub Copilot subscription traffic and the rest of the suite. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable ## Additional Notes `CHANGELOG.md` stays unchecked because Headroom generates release notes from conventional commits, not manual edits, and this patch preserves the existing launch flags while changing only the runtime route decision. Co-authored-by: JD Davis <mxjerrett@gmail.com>
This commit is contained in:
parent
7de2c1e4c2
commit
5af5e22862
2 changed files with 75 additions and 12 deletions
|
|
@ -44,8 +44,13 @@ if TYPE_CHECKING:
|
|||
import httpx
|
||||
|
||||
from headroom.agent_savings import proxy_pipeline_kwargs
|
||||
from headroom.copilot_auth import apply_copilot_api_auth, build_copilot_upstream_url
|
||||
from headroom.copilot_auth import (
|
||||
apply_copilot_api_auth,
|
||||
build_copilot_upstream_url,
|
||||
is_copilot_api_url,
|
||||
)
|
||||
from headroom.pipeline import PipelineStage, summarize_routing_markers
|
||||
from headroom.providers.copilot import model_prefers_responses_api
|
||||
from headroom.proxy.auth_mode import (
|
||||
classify_auth_mode,
|
||||
classify_client,
|
||||
|
|
@ -168,6 +173,14 @@ def _resolve_openai_upstream_base(request_headers: dict[str, str]) -> str | None
|
|||
return normalized
|
||||
|
||||
|
||||
def _resolve_openai_chat_handler_path(base_url: str, model: str | None) -> str:
|
||||
"""Return the upstream path suffix for an OpenAI chat-completions request."""
|
||||
|
||||
if is_copilot_api_url(base_url) and model_prefers_responses_api(model):
|
||||
return _OPENAI_RESPONSES_PATH
|
||||
return _OPENAI_CHAT_COMPLETIONS_PATH
|
||||
|
||||
|
||||
def _append_request_query(url: str, query: str) -> str:
|
||||
if not query:
|
||||
return url
|
||||
|
|
@ -965,8 +978,7 @@ class OpenAIHandlerMixin:
|
|||
not just the generic passthrough route that already honors it. Falls
|
||||
back to the configured ``OPENAI_API_URL`` (``OPENAI_TARGET_API_URL``).
|
||||
"""
|
||||
custom = request.headers.get("x-headroom-base-url", "").strip()
|
||||
return custom or self.OPENAI_API_URL
|
||||
return _resolve_openai_upstream_base(request.headers) or self.OPENAI_API_URL
|
||||
|
||||
@staticmethod
|
||||
def _strict_previous_turn_frozen_count(
|
||||
|
|
@ -1896,6 +1908,17 @@ class OpenAIHandlerMixin:
|
|||
model = body.get("model", "unknown")
|
||||
messages = body.get("messages", [])
|
||||
original_client_messages = copy.deepcopy(messages)
|
||||
custom_upstream_base_url = _resolve_openai_upstream_base(request.headers)
|
||||
upstream_base_url = self._resolve_openai_upstream(request)
|
||||
handler_path_suffix = _resolve_openai_chat_handler_path(
|
||||
upstream_base_url,
|
||||
model,
|
||||
)
|
||||
handler_path = (
|
||||
_resolve_openai_handler_path(request.headers, handler_path=handler_path_suffix)
|
||||
if custom_upstream_base_url is not None
|
||||
else f"/v1{handler_path_suffix}"
|
||||
)
|
||||
input_event = self.pipeline_extensions.emit(
|
||||
PipelineStage.INPUT_RECEIVED,
|
||||
operation="proxy.request",
|
||||
|
|
@ -1904,7 +1927,7 @@ class OpenAIHandlerMixin:
|
|||
model=model,
|
||||
messages=messages,
|
||||
tools=body.get("tools"),
|
||||
metadata={"path": "/v1/chat/completions", "stream": body.get("stream", False)},
|
||||
metadata={"path": handler_path, "stream": body.get("stream", False)},
|
||||
)
|
||||
if input_event.messages is not None:
|
||||
messages = input_event.messages
|
||||
|
|
@ -2112,7 +2135,7 @@ class OpenAIHandlerMixin:
|
|||
provider="openai",
|
||||
model=model,
|
||||
messages=messages,
|
||||
metadata={"cache_hit": True, "path": "/v1/chat/completions"},
|
||||
metadata={"cache_hit": True, "path": handler_path},
|
||||
)
|
||||
# Response-cache hit: same pattern as the anthropic
|
||||
# cache-hit site. ``from_response_cache=True`` is the
|
||||
|
|
@ -2598,7 +2621,7 @@ class OpenAIHandlerMixin:
|
|||
messages=optimized_messages,
|
||||
tools=tools,
|
||||
headers=headers,
|
||||
metadata={"path": "/v1/chat/completions", "stream": stream},
|
||||
metadata={"path": handler_path, "stream": stream},
|
||||
)
|
||||
if presend_event.messages is not None:
|
||||
optimized_messages = presend_event.messages
|
||||
|
|
@ -2632,7 +2655,7 @@ class OpenAIHandlerMixin:
|
|||
model=model,
|
||||
messages=body["messages"],
|
||||
tools=tools,
|
||||
metadata={"path": "/v1/chat/completions", "stream": True},
|
||||
metadata={"path": handler_path, "stream": True},
|
||||
)
|
||||
# Streaming: use stream_openai_message() → SSE events
|
||||
return await self._stream_openai_via_backend(
|
||||
|
|
@ -2667,7 +2690,7 @@ class OpenAIHandlerMixin:
|
|||
tools=tools,
|
||||
response=backend_response.body,
|
||||
metadata={
|
||||
"path": "/v1/chat/completions",
|
||||
"path": handler_path,
|
||||
"stream": False,
|
||||
"status_code": backend_response.status_code,
|
||||
},
|
||||
|
|
@ -2680,7 +2703,7 @@ class OpenAIHandlerMixin:
|
|||
model=model,
|
||||
response=backend_response.body,
|
||||
metadata={
|
||||
"path": "/v1/chat/completions",
|
||||
"path": handler_path,
|
||||
"stream": False,
|
||||
"status_code": backend_response.status_code,
|
||||
},
|
||||
|
|
@ -2883,7 +2906,7 @@ class OpenAIHandlerMixin:
|
|||
model=model,
|
||||
messages=body["messages"],
|
||||
tools=tools,
|
||||
metadata={"path": "/v1/chat/completions", "stream": True},
|
||||
metadata={"path": handler_path, "stream": True},
|
||||
)
|
||||
return await self._stream_response(
|
||||
url,
|
||||
|
|
@ -2915,7 +2938,7 @@ class OpenAIHandlerMixin:
|
|||
tools=tools,
|
||||
response=response,
|
||||
metadata={
|
||||
"path": "/v1/chat/completions",
|
||||
"path": handler_path,
|
||||
"stream": False,
|
||||
"status_code": response.status_code,
|
||||
},
|
||||
|
|
@ -2928,7 +2951,7 @@ class OpenAIHandlerMixin:
|
|||
model=model,
|
||||
response=response,
|
||||
metadata={
|
||||
"path": "/v1/chat/completions",
|
||||
"path": handler_path,
|
||||
"stream": False,
|
||||
"status_code": response.status_code,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ def _load_handler_module(monkeypatch: pytest.MonkeyPatch, module_name: str, rela
|
|||
httpx_mod.ConnectError = type("ConnectError", (Exception,), {})
|
||||
httpx_mod.ConnectTimeout = type("ConnectTimeout", (Exception,), {})
|
||||
httpx_mod.PoolTimeout = type("PoolTimeout", (Exception,), {})
|
||||
httpx_mod.ReadTimeout = type("ReadTimeout", (Exception,), {})
|
||||
monkeypatch.setitem(sys.modules, "httpx", httpx_mod)
|
||||
|
||||
responses_mod = types.ModuleType("fastapi.responses")
|
||||
|
|
@ -227,3 +228,42 @@ def test_streaming_response_applies_copilot_auth(monkeypatch: pytest.MonkeyPatch
|
|||
assert sent_headers["Authorization"] == "Bearer upstream-token"
|
||||
assert sent_headers["content-type"] == "application/json"
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
def test_openai_chat_routes_copilot_requests_per_model(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
openai_mod = _load_handler_module(
|
||||
monkeypatch,
|
||||
"tests.headroom_proxy_handlers_openai",
|
||||
"headroom/proxy/handlers/openai.py",
|
||||
)
|
||||
|
||||
copilot_base = "https://api.githubcopilot.com"
|
||||
gpt54_mini_url = openai_mod.build_copilot_upstream_url(
|
||||
copilot_base,
|
||||
openai_mod._resolve_openai_handler_path(
|
||||
{},
|
||||
handler_path=openai_mod._resolve_openai_chat_handler_path(copilot_base, "gpt-5.4-mini"),
|
||||
),
|
||||
)
|
||||
claude_url = openai_mod.build_copilot_upstream_url(
|
||||
copilot_base,
|
||||
openai_mod._resolve_openai_handler_path(
|
||||
{},
|
||||
handler_path=openai_mod._resolve_openai_chat_handler_path(
|
||||
copilot_base, "claude-sonnet-5"
|
||||
),
|
||||
),
|
||||
)
|
||||
openai_url = openai_mod.build_copilot_upstream_url(
|
||||
"https://api.openai.com",
|
||||
openai_mod._resolve_openai_handler_path(
|
||||
{},
|
||||
handler_path=openai_mod._resolve_openai_chat_handler_path(
|
||||
"https://api.openai.com", "gpt-5.4-mini"
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
assert gpt54_mini_url == "https://api.githubcopilot.com/responses"
|
||||
assert claude_url == "https://api.githubcopilot.com/chat/completions"
|
||||
assert openai_url == "https://api.openai.com/v1/chat/completions"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue