fix(wrap): verify proxy deps before mutating Codex config (#1628)

## Description

\`headroom wrap codex\` now verifies that optional proxy dependencies
(\`headroom-ai[proxy]\`) are installed before mutating Codex
\`config.toml\`. If the check fails, the command exits with the same
error message as \`headroom proxy\` and leaves Codex config untouched.

Fixes #1614 (Bug 1: config mutated before proxy dependency check).

## 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

- Extract \`ensure_proxy_dependencies()\` in \`headroom/cli/proxy.py\`
(shared with \`headroom proxy\`)
- Call it at the start of \`wrap codex\` when \`not no_proxy\`, before
config snapshot/injection
- Add regression tests for prepare-only abort, \`--no-proxy\` skip, and
import failure messaging

## Testing

- [x] Unit tests pass (\`pytest\`)
- [x] Linting passes (\`ruff check .\`)
- [ ] Type checking passes (\`mypy headroom\`)
- [x] New tests added for new functionality
- [x] Manual testing performed

### Test Output

\`\`\`bash
pytest
tests/test_cli/test_wrap_codex.py::test_wrap_codex_aborts_before_mutating_config_when_proxy_deps_missing
\

tests/test_cli/test_wrap_codex.py::test_wrap_codex_skips_proxy_dependency_check_with_no_proxy
\

tests/test_cli/test_wrap_codex.py::test_ensure_proxy_dependencies_exits_when_server_import_fails
-q
# 3 passed
ruff check headroom/cli/wrap.py headroom/cli/proxy.py
tests/test_cli/test_wrap_codex.py
ruff format --check headroom/cli/wrap.py headroom/cli/proxy.py
tests/test_cli/test_wrap_codex.py
\`\`\`

## Real Behavior Proof

Environment: Linux (Ubuntu), Python 3.12, local checkout with
\`PYTHONPATH\` pointed at patched sources.

Exact command / steps:
1. Created a temp \`~/.codex/config.toml\` with \`model_provider =
"openai"\`.
2. Patched \`headroom.cli.wrap.ensure_proxy_dependencies\` to raise
\`SystemExit(1)\` (simulating missing \`[proxy]\` extra).
3. Ran \`headroom wrap codex --prepare-only --no-serena --port 8787\`.

Observed result: exit code 1; \`config.toml\` unchanged; no
\`config.toml.headroom-backup\` created; no \`[mcp_servers.headroom]\`
block written.

Also verified: \`headroom wrap codex --prepare-only --no-proxy ...\`
does not invoke the dependency check.

Not tested: Windows-specific proxy selector behavior (covered separately
in #1655).

## 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
- [x] 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
- [x] I did not edit CHANGELOG.md; release notes are generated
automatically

---------

Co-authored-by: syf2211 <syf2211@users.noreply.github.com>
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
Co-authored-by: JD Davis <jd@jds-macbook-air.tail2a279.ts.net>
This commit is contained in:
石岳峰
2026-08-13 09:52:22 -07:00
committed by GitHub
parent 9fde127534
commit b7f342c153
8 changed files with 189 additions and 38 deletions
+42 -16
View File
@@ -4,6 +4,7 @@ import logging
import os
import sys
import warnings
from importlib import import_module
from typing import Any, Literal, cast
import click
@@ -18,6 +19,38 @@ from headroom.proxy.modes import PROXY_MODE_CACHE, normalize_proxy_mode
from .main import main
def ensure_proxy_dependencies() -> None:
"""Verify optional proxy extras are installed before starting or wrapping."""
required_modules: list[str] = [
"fastapi",
"uvicorn",
"httpx",
"openai",
"mcp",
"magika",
"zstandard",
"websockets",
"onnxruntime",
"transformers",
"watchdog",
]
if sys.implementation.name != "pypy":
required_modules.append("orjson")
try:
for module in required_modules:
import_module(module)
except ImportError as e:
click.secho(
"Error: Proxy dependencies not installed. Run: pip install headroom-ai[proxy]",
fg="red",
err=True,
)
click.secho(f"Details: {e}", fg="red", err=True)
raise SystemExit(1) from None
# ---------------------------------------------------------------------------
# Startup log suppression.
#
@@ -1032,23 +1065,16 @@ def proxy(
Usage with OpenAI-compatible clients:
OPENAI_BASE_URL=http://localhost:8787/v1 your-app
"""
ensure_proxy_dependencies()
# Import here to avoid slow startup
try:
from headroom.proxy.server import (
ProxyConfig,
_parse_csv_tools,
_parse_exclude_tools,
_parse_tool_profiles,
run_server,
)
except ImportError as e:
click.secho(
"Error: Proxy dependencies not installed. Run: pip install headroom-ai[proxy]",
fg="red",
err=True,
)
click.secho(f"Details: {e}", fg="red", err=True)
raise SystemExit(1) from None
from headroom.proxy.server import (
ProxyConfig,
_parse_csv_tools,
_parse_exclude_tools,
_parse_tool_profiles,
run_server,
)
# Warn if --learn and --no-learn are both set (--no-learn wins, per docstring)
if learn and no_learn:
+4
View File
@@ -59,6 +59,7 @@ from headroom._version import normalize_release_version as _normalize_release_ve
from headroom.agent_savings import (
apply_agent_savings_env_defaults,
)
from headroom.cli.proxy import ensure_proxy_dependencies
from headroom.copilot_auth import (
_API_TOKEN_ENV_VARS,
_API_TOKEN_EXPIRES_AT_ENV_VAR,
@@ -5717,6 +5718,9 @@ def _run_codex_wrap(
codex_args: tuple,
) -> None:
"""Execute the Codex wrap flow against the durable Codex home."""
if not no_proxy:
ensure_proxy_dependencies()
if prepare_only:
_prepare_codex_wrap_state(
port=port,
+1
View File
@@ -495,6 +495,7 @@ markers = [
"slow: slow tests (model loads, large fixtures)",
"real_llm: tests that hit real LLM APIs; skipped unless explicitly enabled",
"live: opt-in multi-turn tests that hit real upstream APIs; require provider keys",
"proxy_dependency_gate: exercises ensure_proxy_dependencies() without mocking",
]
[tool.coverage.run]
+16
View File
@@ -22,6 +22,22 @@ from tests._skip_helpers import external_model_skip_reason
# those up inside CliRunner, so assertions would see the developer's proxy
# config instead of the test's. Scrub them so local runs match CI; tests
# that need a value set it explicitly via monkeypatch or CliRunner env.
@pytest.fixture(autouse=True)
def _skip_proxy_dependency_gate_unless_exercised(
request: pytest.FixtureRequest, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Most CLI tests run without headroom-ai[proxy] extras installed."""
if request.node.get_closest_marker("proxy_dependency_gate") is not None:
return
try:
from headroom.cli import proxy
except ModuleNotFoundError:
# Native-wrapper jobs intentionally install only pytest and exercise the
# installer scripts without importing Headroom's runtime dependencies.
return
monkeypatch.setattr(proxy, "ensure_proxy_dependencies", lambda: None)
@pytest.fixture(autouse=True)
def _scrub_developer_headroom_env(monkeypatch):
for key in list(os.environ):
+2 -1
View File
@@ -42,7 +42,8 @@ def test_wrap_codex_prepare_only_updates_config(monkeypatch, tmp_path: Path) ->
_set_test_home(monkeypatch, tmp_path)
runner = CliRunner()
result = runner.invoke(main, ["wrap", "codex", "--prepare-only", "--port", "8787"])
with patch("headroom.cli.wrap.ensure_proxy_dependencies", return_value=None):
result = runner.invoke(main, ["wrap", "codex", "--prepare-only", "--port", "8787"])
assert result.exit_code == 0, result.output
config_file = tmp_path / ".codex" / "config.toml"
+67
View File
@@ -40,6 +40,24 @@ def runner() -> CliRunner:
return CliRunner()
_PROXY_DEP_TESTS = frozenset(
{
"test_wrap_codex_aborts_before_mutating_config_when_proxy_deps_missing",
"test_wrap_codex_skips_proxy_dependency_check_with_no_proxy",
}
)
@pytest.fixture(autouse=True)
def _skip_wrap_proxy_dependency_gate_unless_exercised(
request: pytest.FixtureRequest, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Wrap-codex integration tests run in the base CI env without [proxy] extras."""
if request.node.name in _PROXY_DEP_TESTS:
return
monkeypatch.setattr("headroom.cli.wrap.ensure_proxy_dependencies", lambda: None)
# ---------------------------------------------------------------------------
# Unit tests: helpers operating on ~/.codex/config.toml
# ---------------------------------------------------------------------------
@@ -990,6 +1008,55 @@ def test_wrap_codex_prepare_only_creates_backup_and_config(
assert backup.read_text(encoding="utf-8") == original
def test_wrap_codex_aborts_before_mutating_config_when_proxy_deps_missing(
runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
_set_test_home(monkeypatch, tmp_path)
config_file = tmp_path / ".codex" / "config.toml"
config_file.parent.mkdir(parents=True)
original = 'model_provider = "openai"\n'
config_file.write_text(original, encoding="utf-8")
with patch("headroom.cli.wrap.ensure_proxy_dependencies", side_effect=SystemExit(1)):
result = runner.invoke(
main,
["wrap", "codex", "--prepare-only", "--no-serena", "--port", "8787"],
)
assert result.exit_code == 1, result.output
assert config_file.read_text(encoding="utf-8") == original
assert "[mcp_servers.headroom]" not in config_file.read_text(encoding="utf-8")
assert not (tmp_path / ".codex" / "config.toml.headroom-backup").exists()
def test_wrap_codex_skips_proxy_dependency_check_with_no_proxy(
runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
_set_test_home(monkeypatch, tmp_path)
config_file = tmp_path / ".codex" / "config.toml"
config_file.parent.mkdir(parents=True)
config_file.write_text('model_provider = "openai"\n', encoding="utf-8")
with patch(
"headroom.cli.wrap.ensure_proxy_dependencies",
side_effect=AssertionError("should not run with --no-proxy"),
):
result = runner.invoke(
main,
[
"wrap",
"codex",
"--prepare-only",
"--no-proxy",
"--no-serena",
"--port",
"8787",
],
)
assert result.exit_code == 0, result.output
def test_wrap_codex_registers_mcp_when_codex_home_does_not_exist_yet(
runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
+10
View File
@@ -20,6 +20,16 @@ def _no_retired_context_tool_env(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.delenv("HEADROOM_CONTEXT_TOOL", raising=False)
@pytest.fixture(autouse=True)
def _mock_ensure_proxy(monkeypatch: pytest.MonkeyPatch) -> None:
"""Wrap-opencode tests should not spawn a real proxy subprocess in CI."""
def fake_ensure_proxy(port: int, no_proxy: bool, **kwargs): # noqa: ANN002, ANN003
return None, port
monkeypatch.setattr(wrap_mod, "_ensure_proxy", fake_ensure_proxy)
@pytest.fixture
def runner() -> CliRunner:
return CliRunner()
+47 -21
View File
@@ -299,32 +299,58 @@ class TestMemoryTopKValidation:
class TestMissingProxyDepsError:
"""When proxy dependencies are absent the CLI should print an actionable error and exit 1."""
def test_import_error_exits_nonzero(self, runner: CliRunner) -> None:
with patch.dict(
"sys.modules",
{"headroom.proxy.server": None},
@pytest.mark.proxy_dependency_gate
def test_proxy_command_exits_when_mcp_missing(
self, runner: CliRunner, monkeypatch: pytest.MonkeyPatch
) -> None:
import builtins
real_import = builtins.__import__
def fake_import(
name: str,
globals: dict | None = None,
locals: dict | None = None,
fromlist: tuple = (),
level: int = 0,
):
result = runner.invoke(main, ["proxy"])
# Click CliRunner may raise SystemExit or catch it; exit code must be non-zero
assert result.exit_code != 0
if name == "mcp":
raise ImportError("No module named 'mcp'")
return real_import(name, globals, locals, fromlist, level)
def test_import_error_message_is_actionable(self, runner: CliRunner) -> None:
"""The error message should tell the user how to fix the problem."""
original_import = (
__builtins__.__import__ if hasattr(__builtins__, "__import__") else __import__
)
monkeypatch.setattr(builtins, "__import__", fake_import)
result = runner.invoke(main, ["proxy"])
assert result.exit_code == 1, result.output
assert "pip install headroom-ai[proxy]" in result.output
assert "No module named 'mcp'" in result.output
def patched_import(name, *args, **kwargs):
if name == "headroom.proxy.server":
raise ImportError("No module named 'headroom.proxy.server'")
return original_import(name, *args, **kwargs)
@pytest.mark.proxy_dependency_gate
def test_ensure_proxy_dependencies_exits_when_fastapi_missing(
self, monkeypatch: pytest.MonkeyPatch
) -> None:
import builtins
with patch("builtins.__import__", side_effect=patched_import):
result = runner.invoke(main, ["proxy"])
from headroom.cli.proxy import ensure_proxy_dependencies
# Either exit code 1 or output with actionable guidance
# (some test environments may shadow the import differently)
assert result.exit_code != 0 or "proxy" in result.output.lower()
real_import = builtins.__import__
def fake_import(
name: str,
globals: dict | None = None,
locals: dict | None = None,
fromlist: tuple = (),
level: int = 0,
):
if name == "fastapi":
raise ImportError("No module named 'fastapi'")
return real_import(name, globals, locals, fromlist, level)
monkeypatch.setattr(builtins, "__import__", fake_import)
with pytest.raises(SystemExit) as exc_info:
ensure_proxy_dependencies()
assert exc_info.value.code == 1
class TestKeyboardInterruptExitCode: