mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(wrap): preserve custom Codex provider base_url during proxy injection (#1894)
## Description Refs #1614 (Bug 2 only; Bug 1's config-mutation ordering is covered by a separate PR). `headroom wrap codex` unconditionally pointed the proxy's upstream OpenAI route at `api.openai.com`, even when the user's Codex config already declared a custom OpenAI-compatible provider such as `freemodel.dev`, LiteLLM, or vLLM under `[model_providers.<name>]`. The proxy then silently rerouted traffic to OpenAI, which rejected the user's gateway API key, and Codex interpreted the resulting auth failures as an invalid session. ## Type of Change - [x] Bug fix ## Changes Made - `_detect_custom_codex_upstream_base_url` and `_codex_custom_provider_base_urls` in `headroom/cli/wrap.py` scan the existing `config.toml` for a user-declared custom `[model_providers.*]` table, excluding Codex built-ins and Headroom's own table, and return its `base_url` when the selection is unambiguous: either the top-level `model_provider` names it directly, or a prior wrap left the original provider in the `# was: <original>` comment from `_redirect_existing_top_level_keys`. - The detector falls back to the sole custom provider when exactly one candidate exists and no matching top-level selection is present, which covers the issue repro where the custom table exists without a static top-level provider pin. - `_inject_codex_provider_config` now detects that custom upstream before building the injected provider block. When found, it adds `X-Headroom-Base-Url` to `env_http_headers`, mapped to `HEADROOM_CODEX_UPSTREAM_BASE_URL`, matching Codex's env-var-based header contract. - `codex()` exports the detected value into `HEADROOM_CODEX_UPSTREAM_BASE_URL` for the launched Codex process unless the user already set it. The proxy's OpenAI HTTP handlers already honor `X-Headroom-Base-Url`, so HTTP `/v1/chat/completions` and `/v1/responses` requests forward to the preserved gateway instead of the default OpenAI upstream. This is scoped to the HTTP request path. Codex's WebSocket transport for `/v1/responses` resolves its upstream from a separate header-independent path and keeps the existing behavior. ## Testing - [x] Focused Codex wrap tests passed locally before PR review: `pytest tests/test_cli/test_wrap_codex.py -q` - [x] Broader Codex CLI test selection passed locally before PR review: `pytest tests/test_cli/ -k codex -q` - [x] CI lint, format, and type checks passed on PR head `fb769349`. - [x] CI build, test shards, native wrapper, Docker init, Docker wrap, security, merge-conflict, and governance jobs passed on PR head `fb769349`. - [ ] Live `headroom wrap codex` against a real `freemodel.dev` account was not run for this PR. ## Test Output Previously reported local focused test output: ```text pytest tests/test_cli/test_wrap_codex.py -q 83 passed pytest tests/test_cli/ -k codex -q 113 passed, 426 deselected ``` Previously reported local static checks: ```text ruff check ruff format --check mypy ``` CI evidence on PR head `fb769349`: GitHub Actions run `28987811482` completed successfully for CI, including `lint` with `ruff check`, `ruff format --check`, and `mypy`; `build`; `build-wheel`; four test shards; `test-agno`; `test-extras`; `test-dashboard-ui`; `windows-native-wrapper`; `macos-native-wrapper`; and `docker-native-e2e`. PR Governance run `28987811437` completed successfully. ## Real Behavior Proof - Environment: Unit-level Codex config injection using pytest tmp home, PR head `fb769349`, and GitHub Actions CI run `28987811482`. - Exact command / steps: With a Codex config containing `[model_providers.freemodel]`, `base_url = "https://api.freemodel.dev"`, and `wire_api = "responses"`, call `_inject_codex_provider_config(8787)`. - Observed result: The injector returns `https://api.freemodel.dev`; the injected `[model_providers.headroom]` table contains `env_http_headers = { "X-Headroom-Project" = "HEADROOM_PROJECT", "X-Headroom-Base-Url" = "HEADROOM_CODEX_UPSTREAM_BASE_URL" }`; the user's `[model_providers.freemodel]` table remains unchanged; and re-running `_inject_codex_provider_config(9999)` preserves the same upstream while updating the proxy port. - Not tested: Live external traffic through a real `freemodel.dev` key, WebSocket custom-upstream routing, and Bug 1's dependency-check-before-config-mutation path. ## Review Readiness - [x] Scope is limited to #1614 Bug 2, custom provider `base_url` preservation for Codex wrap. - [x] Bug 1 remains out of scope and is called out separately. - [x] The changed code uses the existing proxy `X-Headroom-Base-Url` contract instead of adding a new proxy route. - [x] Ambiguous multiple custom providers keep prior fallback behavior instead of guessing. - [x] A collaborator reviewed and approved the current head after running the focused Codex wrap tests locally. - [x] This PR is ready for human review. ## Checklist - [x] I have performed a self-review. - [x] Focused tests added and passing. - [x] Lint, format, and type-check clean on CI. - [x] No unrelated files changed. - [x] This PR is ready for human review.
This commit is contained in:
parent
662b7bc00e
commit
372d6c8cd4
2 changed files with 292 additions and 6 deletions
|
|
@ -673,6 +673,157 @@ class TestSubscriptionRouting:
|
|||
assert "env_key" not in content
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Custom upstream preservation (#1614): wrap must not silently reroute a
|
||||
# pre-existing custom [model_providers.*] base_url to api.openai.com.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestDetectCustomCodexUpstreamBaseUrl:
|
||||
"""Unit tests for the detection helper used by ``_inject_codex_provider_config``."""
|
||||
|
||||
def test_no_config_returns_none(self) -> None:
|
||||
assert wrap_mod._detect_custom_codex_upstream_base_url("") is None
|
||||
|
||||
def test_no_custom_provider_returns_none(self) -> None:
|
||||
content = (
|
||||
'model_provider = "openai"\n\n'
|
||||
"[model_providers.openai]\n"
|
||||
'base_url = "https://api.openai.com/v1"\n'
|
||||
)
|
||||
assert wrap_mod._detect_custom_codex_upstream_base_url(content) is None
|
||||
|
||||
def test_sole_candidate_used_without_explicit_selection(self) -> None:
|
||||
"""Matches the #1614 repro: a custom table with no static top-level pin."""
|
||||
content = (
|
||||
"[model_providers.freemodel]\n"
|
||||
'base_url = "https://api.freemodel.dev"\n'
|
||||
'wire_api = "responses"\n'
|
||||
)
|
||||
assert (
|
||||
wrap_mod._detect_custom_codex_upstream_base_url(content) == "https://api.freemodel.dev"
|
||||
)
|
||||
|
||||
def test_explicit_top_level_selection_wins(self) -> None:
|
||||
content = (
|
||||
'model_provider = "freemodel"\n\n'
|
||||
"[model_providers.freemodel]\n"
|
||||
'base_url = "https://api.freemodel.dev"\n\n'
|
||||
"[model_providers.other]\n"
|
||||
'base_url = "https://api.other.example"\n'
|
||||
)
|
||||
assert (
|
||||
wrap_mod._detect_custom_codex_upstream_base_url(content) == "https://api.freemodel.dev"
|
||||
)
|
||||
|
||||
def test_was_comment_recovers_selection_on_rewrap(self) -> None:
|
||||
"""After a prior wrap, model_provider reads 'headroom # was: freemodel'."""
|
||||
content = (
|
||||
'model_provider = "headroom" # was: freemodel\n\n'
|
||||
"[model_providers.freemodel]\n"
|
||||
'base_url = "https://api.freemodel.dev"\n'
|
||||
)
|
||||
assert (
|
||||
wrap_mod._detect_custom_codex_upstream_base_url(content) == "https://api.freemodel.dev"
|
||||
)
|
||||
|
||||
def test_ambiguous_multiple_candidates_returns_none(self) -> None:
|
||||
content = (
|
||||
"[model_providers.freemodel]\n"
|
||||
'base_url = "https://api.freemodel.dev"\n\n'
|
||||
"[model_providers.other]\n"
|
||||
'base_url = "https://api.other.example"\n'
|
||||
)
|
||||
assert wrap_mod._detect_custom_codex_upstream_base_url(content) is None
|
||||
|
||||
def test_builtin_provider_tables_excluded(self) -> None:
|
||||
content = (
|
||||
'model_provider = "openai"\n\n'
|
||||
"[model_providers.openai]\n"
|
||||
'base_url = "https://api.openai.com/v1"\n\n'
|
||||
"[model_providers.anthropic]\n"
|
||||
'base_url = "https://api.anthropic.com/v1"\n'
|
||||
)
|
||||
assert wrap_mod._detect_custom_codex_upstream_base_url(content) is None
|
||||
|
||||
def test_own_headroom_table_excluded(self) -> None:
|
||||
content = (
|
||||
'model_provider = "headroom"\n\n'
|
||||
"[model_providers.headroom]\n"
|
||||
'base_url = "http://127.0.0.1:8787/v1"\n'
|
||||
)
|
||||
assert wrap_mod._detect_custom_codex_upstream_base_url(content) is None
|
||||
|
||||
|
||||
class TestInjectPreservesCustomUpstreamBaseUrl:
|
||||
"""``_inject_codex_provider_config`` must preserve a pre-existing custom
|
||||
provider's ``base_url`` instead of silently rerouting to api.openai.com."""
|
||||
|
||||
def test_inject_returns_and_carries_custom_base_url(
|
||||
self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
_set_test_home(monkeypatch, tmp_path)
|
||||
config_dir = tmp_path / ".codex"
|
||||
config_dir.mkdir()
|
||||
config_file = config_dir / "config.toml"
|
||||
config_file.write_text(
|
||||
"[model_providers.freemodel]\n"
|
||||
'base_url = "https://api.freemodel.dev"\n'
|
||||
'wire_api = "responses"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
result = wrap_mod._inject_codex_provider_config(8787)
|
||||
|
||||
assert result == "https://api.freemodel.dev"
|
||||
content = config_file.read_text(encoding="utf-8")
|
||||
parsed = tomllib.loads(content)
|
||||
headers = parsed["model_providers"]["headroom"]["env_http_headers"]
|
||||
assert (
|
||||
headers[wrap_mod._UPSTREAM_BASE_URL_HEADER_NAME] == wrap_mod._UPSTREAM_BASE_URL_ENV_VAR
|
||||
)
|
||||
# The user's own table is left untouched — only headroom's own is managed.
|
||||
assert parsed["model_providers"]["freemodel"]["base_url"] == "https://api.freemodel.dev"
|
||||
|
||||
def test_inject_without_custom_provider_returns_none(
|
||||
self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
_set_test_home(monkeypatch, tmp_path)
|
||||
|
||||
result = wrap_mod._inject_codex_provider_config(8787)
|
||||
|
||||
assert result is None
|
||||
content = (tmp_path / ".codex" / "config.toml").read_text(encoding="utf-8")
|
||||
assert wrap_mod._UPSTREAM_BASE_URL_HEADER_NAME not in content
|
||||
|
||||
def test_preserved_upstream_survives_rewrap_and_port_change(
|
||||
self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
|
||||
) -> None:
|
||||
_set_test_home(monkeypatch, tmp_path)
|
||||
config_dir = tmp_path / ".codex"
|
||||
config_dir.mkdir()
|
||||
config_file = config_dir / "config.toml"
|
||||
config_file.write_text(
|
||||
'model_provider = "freemodel"\n\n'
|
||||
"[model_providers.freemodel]\n"
|
||||
'base_url = "https://api.freemodel.dev"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
first = wrap_mod._inject_codex_provider_config(8787)
|
||||
second = wrap_mod._inject_codex_provider_config(9999) # port change / re-wrap
|
||||
|
||||
assert first == "https://api.freemodel.dev"
|
||||
assert second == "https://api.freemodel.dev"
|
||||
content = config_file.read_text(encoding="utf-8")
|
||||
parsed = tomllib.loads(content)
|
||||
assert parsed["model_providers"]["headroom"]["base_url"] == "http://127.0.0.1:9999/v1"
|
||||
headers = parsed["model_providers"]["headroom"]["env_http_headers"]
|
||||
assert (
|
||||
headers[wrap_mod._UPSTREAM_BASE_URL_HEADER_NAME] == wrap_mod._UPSTREAM_BASE_URL_ENV_VAR
|
||||
)
|
||||
|
||||
|
||||
class TestInjectAvoidsDuplicateTopLevelKeys:
|
||||
"""Wrap must not produce a TOML-validity-breaking duplicate-key error.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue