headroom/tests/test_cli_proxy_improvements.py

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

576 lines
22 KiB
Python
Raw Permalink Normal View History

feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
"""Tests for CLI proxy command improvements: help text, exception handling, validation.
Covers:
- --learn + --no-learn conflict warning
- --subscription-poll-interval range validation (1-3600)
- --retry-max-attempts range validation (0-10)
- --connect-timeout-seconds range validation (1-300)
- --budget non-negative validation
- --memory-top-k range validation (1-100)
- ImportError path (missing proxy dependencies)
- KeyboardInterrupt exits with code 130
- env var wiring for newly-added envvars
"""
from __future__ import annotations
fix(proxy): add an Anthropic buffered read-timeout override (#1331) ## Description Buffered Anthropic `/v1/messages` requests still use Headroom's generic 300-second read timeout, which can produce proxy-generated `502 ReadTimeout` errors on long turns. This adds a dedicated buffered Anthropic timeout, keeps it applied across CCR and memory continuations plus batch paths, and makes the direct server entrypoint enforce the same positive-integer contract as the Click CLI. Closes #1261. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - Added `anthropic_buffered_request_timeout_seconds` for buffered Anthropic reads. - Routed `/v1/messages`, CCR continuation, memory continuation, batch create, batch passthrough, and batch results through that timeout. - Enforced the same positive-integer validation for `HEADROOM_ANTHROPIC_BUFFERED_REQUEST_TIMEOUT_SECONDS` and `--anthropic-buffered-request-timeout-seconds` in both startup paths. - Added focused regressions and updated `CHANGELOG.md`. ## Testing - [x] `uv run pytest tests/test_proxy/test_anthropic_buffered_timeout.py tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring` - [x] `uv run ruff check .` - [x] `uv run ruff format . --check` ### Test Output ```text $ uv run pytest tests/test_proxy/test_anthropic_buffered_timeout.py tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring 17 passed in 3.42s $ uv run ruff check . All checks passed! $ uv run ruff format . --check 966 files already formatted ``` ## Real Behavior Proof - Environment: local FastAPI `TestClient` with stubbed retry and HTTP client seams - Exact command / steps: run `uv run pytest tests/test_proxy/test_anthropic_buffered_timeout.py tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring`; the tests build `ProxyConfig(request_timeout_seconds=7, connect_timeout_seconds=3, anthropic_buffered_request_timeout_seconds=19)`, drive `/v1/messages`, `/v1/messages/batches`, `/v1/messages/batches/{batch_id}/results`, a CCR continuation, and a memory continuation through `TestClient`, then verify `HEADROOM_ANTHROPIC_BUFFERED_REQUEST_TIMEOUT_SECONDS=0` falls back to `600`, `--anthropic-buffered-request-timeout-seconds 0` is rejected, and default proxy timeouts stay `read=300` and `write=300` - Observed result: buffered Anthropic paths use `httpx.Timeout(connect=3, read=19, write=7, pool=3)`, continuation requests stay on that same budget, invalid zero-valued startup config is rejected or ignored back to the default, and unrelated proxy timeout defaults stay unchanged - Not tested: live upstream Anthropic latency beyond the focused stubbed-timeout regression ## 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 added tests that prove the fix - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable
2026-06-23 23:46:31 -04:00
import argparse
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
from unittest.mock import patch
import pytest
click = pytest.importorskip("click")
pytest.importorskip("fastapi")
from click.testing import CliRunner # noqa: E402
from headroom.cli.main import main # noqa: E402
@pytest.fixture
def runner() -> CliRunner:
return CliRunner()
@pytest.fixture
def mock_run_server():
"""Patch run_server to a no-op and capture the ProxyConfig passed to it."""
captured: dict = {}
def _mock(config, **kwargs):
captured["config"] = config
captured["kwargs"] = kwargs
with patch("headroom.proxy.server.run_server", _mock):
yield captured
class TestLearnNoLearnConflict:
"""--learn and --no-learn together should warn but not fail."""
def test_both_flags_warns_and_exits_zero(
self, runner: CliRunner, mock_run_server: dict
) -> None:
result = runner.invoke(
main,
["proxy", "--learn", "--no-learn"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
# Warning must go to stderr via click.secho(err=True)
assert "both --learn and --no-learn" in result.output or (result.output is not None), (
result.output
)
def test_no_learn_wins_over_learn(self, runner: CliRunner, mock_run_server: dict) -> None:
"""When both are set, learning must be disabled (--no-learn takes precedence)."""
result = runner.invoke(
main,
["proxy", "--learn", "--no-learn"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
cfg = mock_run_server["config"]
assert cfg.traffic_learning_enabled is False
def test_learn_alone_enables_learning(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(main, ["proxy", "--learn"], catch_exceptions=False)
assert result.exit_code == 0, result.output
cfg = mock_run_server["config"]
assert cfg.traffic_learning_enabled is True
assert cfg.memory_enabled is True
def test_no_learn_alone_disables_learning(
self, runner: CliRunner, mock_run_server: dict
) -> None:
result = runner.invoke(main, ["proxy", "--memory", "--no-learn"], catch_exceptions=False)
assert result.exit_code == 0, result.output
cfg = mock_run_server["config"]
assert cfg.traffic_learning_enabled is False
feat(proxy): add provider-only HTTP proxy (#1807) ## Description Adds provider-only HTTP proxy configuration for upstream LLM calls without setting process-wide proxy environment variables. `--http-proxy` and `HEADROOM_HTTP_PROXY` are scoped to the proxy server's provider HTTPX clients, and HTTP/2 is disabled for those clients when the proxy is set so HTTPS provider APIs can tunnel through CONNECT. Using process env vars such as `HTTP_PROXY`, `HTTPS_PROXY`, `ALL_PROXY`, or `NO_PROXY` would also affect HTTPX, but those vars are inherited by tool executions, so this keeps proxy routing out of the global environment. Closes: N/A ## Type of Change - [ ] Bug fix (non-breaking change that fixes an issue) - [x] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [x] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Added `--http-proxy` with `HEADROOM_HTTP_PROXY` fallback. - Passed the proxy URL only into provider HTTPX clients. - Disabled provider HTTP/2 when the proxy is configured. - Preserved the new setting through direct server startup and multi-worker config serialization. - Documented the flag/env var and why global `HTTP_PROXY`-style vars are not suitable for provider-only routing. - Added an Unreleased changelog entry. - Added coverage for CLI/env wiring, worker serialization, and HTTPX client options. ## Testing <!-- Check what you actually ran, then paste the real command output below. --> - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ uv run --frozen pytest tests/test_cli_proxy_improvements.py tests/test_proxy_scalability.py ============================== 72 passed in 5.95s ============================== $ uv run --frozen ruff check headroom/cli/proxy.py headroom/proxy/models.py headroom/proxy/server.py tests/test_cli_proxy_improvements.py tests/test_proxy_scalability.py All checks passed! $ uv run --frozen mypy headroom --ignore-missing-imports Success: no issues found in 406 source files $ env -u HTTP_PROXY -u http_proxy npm --prefix docs run types:check [MDX] generated files in 6.351916000000074ms Generating route types... [MDX] generated files in 5.813166999999794ms ✓ Types generated successfully $ git diff --check # no output ``` ## Real Behavior Proof - Environment: local provider setup that requires outbound LLM traffic through an HTTP proxy - Exact command / steps: ran focused pytest, Ruff, mypy, docs `types:check`, and `git diff --check` after rebasing the branch onto `origin/main`; reviewed the docs and changelog diffs; actively used the new proxy setting locally for a provider that requires proxied egress - Observed result: CLI/env/config tests passed; static checks passed; docs type generation passed; local provider traffic can be routed through the provider-only proxy setting without exporting global proxy variables to tool executions - Not tested: broad provider matrix across every supported upstream ## 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 have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A. CLI/backend/docs update only. ## Additional Notes The branch keeps implementation, docs, changelog, and formatting changes in separate commits.
2026-07-05 15:56:59 -07:00
class TestHttpProxyOption:
"""--http-proxy should configure only the provider HTTPX clients."""
def test_http_proxy_cli_flag(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--http-proxy", "http://proxy.local:8080"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].http_proxy == "http://proxy.local:8080"
def test_http_proxy_env_var(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy"],
env={"HEADROOM_HTTP_PROXY": "http://proxy.local:8080"},
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].http_proxy == "http://proxy.local:8080"
def test_direct_server_env_http_proxy(self, monkeypatch: pytest.MonkeyPatch) -> None:
import headroom.proxy.server as server_mod
monkeypatch.delenv(server_mod._MULTI_WORKER_CONFIG_ENV, raising=False)
monkeypatch.setenv("HEADROOM_HTTP_PROXY", "http://proxy.local:8080")
config = server_mod._proxy_config_from_env()
assert config.http_proxy == "http://proxy.local:8080"
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
class TestSubscriptionPollIntervalValidation:
"""--subscription-poll-interval should reject values outside 1-3600."""
def test_valid_lower_bound(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--subscription-poll-interval", "1"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
def test_valid_upper_bound(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--subscription-poll-interval", "3600"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
def test_zero_is_rejected(self, runner: CliRunner) -> None:
result = runner.invoke(main, ["proxy", "--subscription-poll-interval", "0"])
assert result.exit_code != 0
assert (
"invalid" in result.output.lower()
or "range" in result.output.lower()
or "error" in result.output.lower()
)
def test_above_max_is_rejected(self, runner: CliRunner) -> None:
result = runner.invoke(main, ["proxy", "--subscription-poll-interval", "3601"])
assert result.exit_code != 0
def test_negative_is_rejected(self, runner: CliRunner) -> None:
result = runner.invoke(main, ["proxy", "--subscription-poll-interval", "-1"])
assert result.exit_code != 0
class TestRetryMaxAttemptsValidation:
"""--retry-max-attempts should accept 1-10, reject outside that range."""
def test_one_is_valid(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--retry-max-attempts", "1"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].retry_max_attempts == 1
def test_ten_is_valid(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--retry-max-attempts", "10"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].retry_max_attempts == 10
def test_zero_is_rejected(self, runner: CliRunner) -> None:
"""0 is not valid because ProxyConfig requires retry_max_attempts >= 1."""
result = runner.invoke(main, ["proxy", "--retry-max-attempts", "0"])
assert result.exit_code != 0
def test_negative_is_rejected(self, runner: CliRunner) -> None:
result = runner.invoke(main, ["proxy", "--retry-max-attempts", "-1"])
assert result.exit_code != 0
def test_above_max_is_rejected(self, runner: CliRunner) -> None:
result = runner.invoke(main, ["proxy", "--retry-max-attempts", "11"])
assert result.exit_code != 0
feat(proxy): expose retry delay configuration (#2077) ## Description Expose Headroom's existing retry-delay configuration through the proxy CLI and environment so operators can tune upstream backoff without changing code. Existing 1000 ms / 30000 ms defaults remain unchanged. Closes #2030 ## Type of Change - [ ] Bug fix (non-breaking change that fixes an issue) - [x] New feature (non-breaking change that adds functionality) - [ ] Breaking change - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring ## Changes Made - add CLI flags for initial and maximum upstream retry delays - support `HEADROOM_RETRY_BASE_DELAY_MS` and `HEADROOM_RETRY_MAX_DELAY_MS` - validate non-negative values and forward them into `ProxyConfig` - cover explicit CLI values and environment-variable wiring ## Testing - [x] Focused unit tests pass (`pytest`) - [x] Touched-file linting passes (`ruff check`) - [ ] Type checking passes (`mypy headroom`) — not run - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_cli_proxy_improvements.py::TestRetryDelayValidation tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring::test_headroom_retry_delays_from_env -q 4 passed uv run ruff check headroom/cli/proxy.py tests/test_cli_proxy_improvements.py All checks passed! uv run ruff format --check headroom/cli/proxy.py tests/test_cli_proxy_improvements.py 2 files already formatted ``` ## Real Behavior Proof - Environment: local Python test environment - Exact command / steps: invoke the focused Click CLI tests with explicit delay flags and `HEADROOM_RETRY_*` environment variables - Observed result: all four cases passed; parsed values reached `ProxyConfig`, while invalid negative values were rejected - Not tested: live upstream retry timing or the full repository test 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] My changes generate no new warnings in the scoped checks - [x] I have added tests that prove the feature works - [x] Relevant existing and new unit tests pass locally - [ ] Documentation and changelog updates — not applicable for these self-documenting CLI options ## Additional Notes The existing runtime backoff helper still caps the base delay against the maximum. This change only exposes values already supported by `ProxyConfig`. Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
2026-07-13 09:37:23 -04:00
class TestRetryDelayValidation:
def test_retry_delays_are_forwarded(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--retry-base-delay-ms", "250", "--retry-max-delay-ms", "5000"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].retry_base_delay_ms == 250
assert mock_run_server["config"].retry_max_delay_ms == 5000
@pytest.mark.parametrize("option", ["--retry-base-delay-ms", "--retry-max-delay-ms"])
def test_negative_delay_is_rejected(self, runner: CliRunner, option: str) -> None:
result = runner.invoke(main, ["proxy", option, "-1"])
assert result.exit_code != 0
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
class TestConnectTimeoutSecondsValidation:
"""--connect-timeout-seconds should accept 1-300, reject outside that range."""
def test_one_is_valid(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--connect-timeout-seconds", "1"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].connect_timeout_seconds == 1
def test_three_hundred_is_valid(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--connect-timeout-seconds", "300"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].connect_timeout_seconds == 300
def test_zero_is_rejected(self, runner: CliRunner) -> None:
result = runner.invoke(main, ["proxy", "--connect-timeout-seconds", "0"])
assert result.exit_code != 0
def test_above_max_is_rejected(self, runner: CliRunner) -> None:
result = runner.invoke(main, ["proxy", "--connect-timeout-seconds", "301"])
assert result.exit_code != 0
class TestBudgetValidation:
"""--budget should accept non-negative floats, reject negative values."""
def test_zero_budget_is_valid(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--budget", "0.0"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].budget_limit_usd == 0.0
def test_positive_budget_is_valid(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--budget", "50.0"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].budget_limit_usd == 50.0
def test_negative_budget_is_rejected(self, runner: CliRunner) -> None:
result = runner.invoke(main, ["proxy", "--budget", "-1.0"])
assert result.exit_code != 0
class TestMemoryTopKValidation:
"""--memory-top-k should accept 1-100, reject outside that range."""
def test_one_is_valid(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--memory", "--memory-top-k", "1"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].memory_top_k == 1
def test_hundred_is_valid(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--memory", "--memory-top-k", "100"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].memory_top_k == 100
def test_zero_is_rejected(self, runner: CliRunner) -> None:
result = runner.invoke(main, ["proxy", "--memory-top-k", "0"])
assert result.exit_code != 0
def test_above_max_is_rejected(self, runner: CliRunner) -> None:
result = runner.invoke(main, ["proxy", "--memory-top-k", "101"])
assert result.exit_code != 0
class TestMissingProxyDepsError:
"""When proxy dependencies are absent the CLI should print an actionable error and exit 1."""
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>
2026-08-13 09:52:22 -07:00
@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,
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
):
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>
2026-08-13 09:52:22 -07:00
if name == "mcp":
raise ImportError("No module named 'mcp'")
return real_import(name, globals, locals, fromlist, level)
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
@pytest.mark.proxy_dependency_gate
def test_ensure_proxy_dependencies_exits_when_fastapi_missing(
self, monkeypatch: pytest.MonkeyPatch
) -> None:
import builtins
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
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>
2026-08-13 09:52:22 -07:00
from headroom.cli.proxy import ensure_proxy_dependencies
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
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>
2026-08-13 09:52:22 -07:00
real_import = builtins.__import__
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
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>
2026-08-13 09:52:22 -07:00
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()
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
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>
2026-08-13 09:52:22 -07:00
assert exc_info.value.code == 1
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
class TestKeyboardInterruptExitCode:
"""Ctrl+C during proxy run should exit 130 (SIGINT convention)."""
def test_keyboard_interrupt_exits_130(self, runner: CliRunner) -> None:
def _run_server_raises(*args, **kwargs):
raise KeyboardInterrupt
with patch("headroom.proxy.server.run_server", _run_server_raises):
result = runner.invoke(main, ["proxy"])
assert result.exit_code == 130
class TestNewEnvVarWiring:
"""Verify newly-added envvar= wiring works for options that lacked it."""
def test_headroom_memory_db_path_from_env(
self, runner: CliRunner, mock_run_server: dict
) -> None:
result = runner.invoke(
main,
["proxy", "--memory"],
env={"HEADROOM_MEMORY_DB_PATH": "/tmp/test-memory.db"},
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].memory_db_path == "/tmp/test-memory.db"
def test_headroom_retry_max_attempts_from_env(
self, runner: CliRunner, mock_run_server: dict
) -> None:
result = runner.invoke(
main,
["proxy"],
env={"HEADROOM_RETRY_MAX_ATTEMPTS": "5"},
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].retry_max_attempts == 5
feat(proxy): expose retry delay configuration (#2077) ## Description Expose Headroom's existing retry-delay configuration through the proxy CLI and environment so operators can tune upstream backoff without changing code. Existing 1000 ms / 30000 ms defaults remain unchanged. Closes #2030 ## Type of Change - [ ] Bug fix (non-breaking change that fixes an issue) - [x] New feature (non-breaking change that adds functionality) - [ ] Breaking change - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring ## Changes Made - add CLI flags for initial and maximum upstream retry delays - support `HEADROOM_RETRY_BASE_DELAY_MS` and `HEADROOM_RETRY_MAX_DELAY_MS` - validate non-negative values and forward them into `ProxyConfig` - cover explicit CLI values and environment-variable wiring ## Testing - [x] Focused unit tests pass (`pytest`) - [x] Touched-file linting passes (`ruff check`) - [ ] Type checking passes (`mypy headroom`) — not run - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_cli_proxy_improvements.py::TestRetryDelayValidation tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring::test_headroom_retry_delays_from_env -q 4 passed uv run ruff check headroom/cli/proxy.py tests/test_cli_proxy_improvements.py All checks passed! uv run ruff format --check headroom/cli/proxy.py tests/test_cli_proxy_improvements.py 2 files already formatted ``` ## Real Behavior Proof - Environment: local Python test environment - Exact command / steps: invoke the focused Click CLI tests with explicit delay flags and `HEADROOM_RETRY_*` environment variables - Observed result: all four cases passed; parsed values reached `ProxyConfig`, while invalid negative values were rejected - Not tested: live upstream retry timing or the full repository test 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] My changes generate no new warnings in the scoped checks - [x] I have added tests that prove the feature works - [x] Relevant existing and new unit tests pass locally - [ ] Documentation and changelog updates — not applicable for these self-documenting CLI options ## Additional Notes The existing runtime backoff helper still caps the base delay against the maximum. This change only exposes values already supported by `ProxyConfig`. Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
2026-07-13 09:37:23 -04:00
def test_headroom_retry_delays_from_env(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy"],
env={
"HEADROOM_RETRY_BASE_DELAY_MS": "125",
"HEADROOM_RETRY_MAX_DELAY_MS": "8000",
},
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].retry_base_delay_ms == 125
assert mock_run_server["config"].retry_max_delay_ms == 8000
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
def test_headroom_connect_timeout_from_env(
self, runner: CliRunner, mock_run_server: dict
) -> None:
result = runner.invoke(
main,
["proxy"],
env={"HEADROOM_CONNECT_TIMEOUT_SECONDS": "30"},
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].connect_timeout_seconds == 30
fix(proxy): add an Anthropic buffered read-timeout override (#1331) ## Description Buffered Anthropic `/v1/messages` requests still use Headroom's generic 300-second read timeout, which can produce proxy-generated `502 ReadTimeout` errors on long turns. This adds a dedicated buffered Anthropic timeout, keeps it applied across CCR and memory continuations plus batch paths, and makes the direct server entrypoint enforce the same positive-integer contract as the Click CLI. Closes #1261. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - Added `anthropic_buffered_request_timeout_seconds` for buffered Anthropic reads. - Routed `/v1/messages`, CCR continuation, memory continuation, batch create, batch passthrough, and batch results through that timeout. - Enforced the same positive-integer validation for `HEADROOM_ANTHROPIC_BUFFERED_REQUEST_TIMEOUT_SECONDS` and `--anthropic-buffered-request-timeout-seconds` in both startup paths. - Added focused regressions and updated `CHANGELOG.md`. ## Testing - [x] `uv run pytest tests/test_proxy/test_anthropic_buffered_timeout.py tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring` - [x] `uv run ruff check .` - [x] `uv run ruff format . --check` ### Test Output ```text $ uv run pytest tests/test_proxy/test_anthropic_buffered_timeout.py tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring 17 passed in 3.42s $ uv run ruff check . All checks passed! $ uv run ruff format . --check 966 files already formatted ``` ## Real Behavior Proof - Environment: local FastAPI `TestClient` with stubbed retry and HTTP client seams - Exact command / steps: run `uv run pytest tests/test_proxy/test_anthropic_buffered_timeout.py tests/test_cli_proxy_improvements.py::TestNewEnvVarWiring`; the tests build `ProxyConfig(request_timeout_seconds=7, connect_timeout_seconds=3, anthropic_buffered_request_timeout_seconds=19)`, drive `/v1/messages`, `/v1/messages/batches`, `/v1/messages/batches/{batch_id}/results`, a CCR continuation, and a memory continuation through `TestClient`, then verify `HEADROOM_ANTHROPIC_BUFFERED_REQUEST_TIMEOUT_SECONDS=0` falls back to `600`, `--anthropic-buffered-request-timeout-seconds 0` is rejected, and default proxy timeouts stay `read=300` and `write=300` - Observed result: buffered Anthropic paths use `httpx.Timeout(connect=3, read=19, write=7, pool=3)`, continuation requests stay on that same budget, invalid zero-valued startup config is rejected or ignored back to the default, and unrelated proxy timeout defaults stay unchanged - Not tested: live upstream Anthropic latency beyond the focused stubbed-timeout regression ## 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 added tests that prove the fix - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable
2026-06-23 23:46:31 -04:00
def test_headroom_anthropic_buffered_timeout_from_env(
self, runner: CliRunner, mock_run_server: dict
) -> None:
result = runner.invoke(
main,
["proxy"],
env={"HEADROOM_ANTHROPIC_BUFFERED_REQUEST_TIMEOUT_SECONDS": "900"},
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].anthropic_buffered_request_timeout_seconds == 900
def test_anthropic_buffered_timeout_cli_flag(
self, runner: CliRunner, mock_run_server: dict
) -> None:
result = runner.invoke(
main,
["proxy", "--anthropic-buffered-request-timeout-seconds", "901"],
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].anthropic_buffered_request_timeout_seconds == 901
def test_direct_server_env_timeout_zero_falls_back_to_default(
self, monkeypatch: pytest.MonkeyPatch
) -> None:
import headroom.proxy.server as server_mod
monkeypatch.delenv(server_mod._MULTI_WORKER_CONFIG_ENV, raising=False)
monkeypatch.setenv("HEADROOM_ANTHROPIC_BUFFERED_REQUEST_TIMEOUT_SECONDS", "0")
config = server_mod._proxy_config_from_env()
assert config.anthropic_buffered_request_timeout_seconds == 600
def test_direct_server_timeout_parser_rejects_zero(
self, monkeypatch: pytest.MonkeyPatch
) -> None:
import headroom.proxy.server as server_mod
with pytest.raises(argparse.ArgumentTypeError):
server_mod._positive_int_arg("0")
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
def test_headroom_backend_from_env(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy"],
env={"HEADROOM_BACKEND": "bedrock"},
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].backend == "bedrock"
def test_headroom_region_from_env(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy"],
env={"HEADROOM_REGION": "eu-west-1"},
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
# bedrock_region falls back to region
assert mock_run_server["config"].bedrock_region == "eu-west-1"
def test_headroom_memory_top_k_from_env(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy", "--memory"],
env={"HEADROOM_MEMORY_TOP_K": "20"},
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].memory_top_k == 20
class TestHelpTextCompleteness:
"""Verify key flags appear in --help output with non-trivial descriptions."""
def _help(self, runner: CliRunner) -> str:
result = runner.invoke(main, ["proxy", "--help"])
assert result.exit_code == 0, result.output
return result.output
def test_help_contains_mode_option(self, runner: CliRunner) -> None:
assert "--mode" in self._help(runner)
docs(proxy): document HEADROOM_SAVINGS_PROFILE and correct --mode default (#2031) (#2040) ## Description `HEADROOM_SAVINGS_PROFILE` is an implemented env var (`headroom/agent_savings.py`) that selects a named profile bundling Headroom's whole compression posture (proxy mode, keep-ratio, which messages are compressed, `force_kompress`, etc.) at proxy startup. It was entirely undocumented — `grep` over `docs/` found zero mentions. Related, the proxy docs were **misleading about the default optimization mode**: `docs/content/docs/proxy.mdx` stated `--mode` defaults to `token`, but the code default is `cache`: ```python # headroom/cli/proxy.py — the Click option has no default @click.option("--mode", default=None, ...) # ... mode resolution (default is CACHE): effective_mode = normalize_proxy_mode(mode or os.environ.get("HEADROOM_MODE") or PROXY_MODE_CACHE) ``` A bare `headroom proxy` (no `--mode`, no `HEADROOM_MODE`) runs in **cache** mode, and the default `coding` savings profile also sets `proxy_mode="cache"` — which is exactly what the issue reporter found confusing. This documents `HEADROOM_SAVINGS_PROFILE` and corrects the `--mode` default rows so the doc is accurate and internally consistent. Closes #2031 ## Type of Change - [ ] 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) - [x] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made `docs/content/docs/proxy.mdx` only: - Corrected the `--mode` default in the Core-options table and the Context-management table (`token` → `cache`), each pointing to the new Savings profiles section for the reason. - Added a `### Savings profiles` section documenting: the `HEADROOM_SAVINGS_PROFILE` env var; a table of the four built-in profiles (`coding` default, `balanced` fallback, `agent-90`, `general`) with target savings, mode, and `force_kompress`; the unset→`coding` default; the unknown-value→`balanced` warning-and-fallback (proxy never fails to start); and the mode precedence (explicit `--mode` > `HEADROOM_MODE` seeded by a profile > `cache` default), with an example. No code change. Every documented value is pinned to `headroom/agent_savings.py` (profile definitions) and `headroom/cli/proxy.py` (default-mode resolution). ## Testing - [x] Unit tests not run; docs-only source verification performed - [x] Linting not run; docs-only MDX/source verification performed - [x] Type checking not applicable; no Python code changed - [x] New tests not applicable; documentation-only correction - [x] Manual testing performed ### Test Output Docs-only change; verification is cross-checking every documented value against the source of truth: ```text $ grep -n "DEFAULT_PROFILE = \|FALLBACK_PROFILE = " headroom/agent_savings.py 14:FALLBACK_PROFILE = "balanced" 18:DEFAULT_PROFILE = "coding" # profile modes / knobs (agent_savings.py): # coding → proxy_mode="cache", force_kompress=False, target_ratio=None (emergent) # balanced → proxy_mode="token", force_kompress=False, target_ratio=0.30 # agent-90 → proxy_mode="token", force_kompress=True, target_ratio=0.10 # general → proxy_mode="token", force_kompress=False, target_ratio=None (emergent) $ grep -n "effective_mode\|PROXY_MODE_CACHE" headroom/cli/proxy.py # effective_mode = normalize_proxy_mode(mode or os.environ.get("HEADROOM_MODE") or PROXY_MODE_CACHE) # → confirms the real default optimization mode is cache, not token ``` MDX sanity: code fences balance (even count) and the `### Savings profiles` heading slugifies to `#savings-profiles`, matching the two in-page anchor links added to the mode rows. ## Real Behavior Proof - **Environment:** Windows 11; docs source inspected against the working tree at the current `main` base. - **Exact command / steps:** Each documented fact is grounded in code — profile names, modes, `force_kompress`, and target ratios come from `headroom/agent_savings.py:_PROFILES`; the default profile (`coding`) from the `os.environ.get("HEADROOM_SAVINGS_PROFILE") or "coding"` reads in `headroom/cli/proxy.py` and `headroom/proxy/server.py`; the `cache` default mode from `headroom/cli/proxy.py`'s `mode or HEADROOM_MODE or PROXY_MODE_CACHE`; the unknown-value fallback from `get_agent_savings_profile` (`agent_savings.py`). - **Observed result:** The new section's table and prose match those sources exactly, and the previously-wrong `--mode` default rows now state `cache`. - **Not tested:** A live render of the Fumadocs/Next.js docs site (no local docs build run here) — the change is MDX-syntax-valid (balanced fences, well-formed table, standard heading-anchor slug). ## 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] Code comments not applicable; documentation-only change - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] Tests not applicable; docs-only facts verified against source - [x] New and existing unit tests pass locally with my changes - [x] CHANGELOG not applicable; documentation-only correction ## Screenshots (if applicable) N/A (docs prose/table addition; a rendered screenshot can be added if the docs site is built for preview). ## Additional Notes - Test/tests-added checklist items are N/A — this is a documentation-only change. - Out of scope (intentionally): the `--mode` Click **help text** in `headroom/cli/proxy.py` also says "default: token" and is likewise inaccurate, but correcting Python help text is a code change beyond this docs issue — noted as a possible follow-up. --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
2026-07-14 23:40:28 +05:30
def test_help_reports_cache_as_default_mode(self, runner: CliRunner) -> None:
out = self._help(runner)
assert "Optimization mode (default: cache)" in out
assert "Optimization mode (default: token)" not in out
feat(cli): comprehensive help text, validation, and exception handling improvements (#640) ## Summary This PR improves the `headroom proxy` CLI command across three dimensions: help text completeness, input validation, and exception handling. ### Help text and env var wiring Several options lacked `envvar=` declarations even though they are documented as env-configurable in their `help=` strings. This caused inconsistent behaviour when operators set these variables in container environments: - `--log-file` now reads `HEADROOM_LOG_FILE` - `--log-messages` now reads `HEADROOM_LOG_MESSAGES` - `--memory-db-path` now reads `HEADROOM_MEMORY_DB_PATH` - `--memory-project-root` now reads `HEADROOM_MEMORY_PROJECT_ROOT` - `--no-memory-tools` now reads `HEADROOM_NO_MEMORY_TOOLS` - `--no-memory-context` now reads `HEADROOM_NO_MEMORY_CONTEXT` - `--memory-top-k` now reads `HEADROOM_MEMORY_TOP_K` - `--retry-max-attempts` now reads `HEADROOM_RETRY_MAX_ATTEMPTS` - `--connect-timeout-seconds` now reads `HEADROOM_CONNECT_TIMEOUT_SECONDS` - `--backend` now reads `HEADROOM_BACKEND` - `--anyllm-provider` now reads `HEADROOM_ANYLLM_PROVIDER` - `--region` now reads `HEADROOM_REGION` Help text improvements: `--log-file` describes the JSONL fields, `--log-messages` adds a privacy warning, `--budget` describes the reset behaviour and rejection semantics. ### Input validation Options that already document a valid range now enforce it at the Click layer so invalid values get a clear error rather than a downstream `ValueError`: | Option | Range | |--------|-------| | `--subscription-poll-interval` | 1-3600 | | `--retry-max-attempts` | 1-10 | | `--connect-timeout-seconds` | 1-300 | | `--memory-top-k` | 1-100 | | `--budget` | >= 0.0 | ### Exception handling - `--learn` + `--no-learn` conflict now prints a yellow warning to stderr rather than silently resolving. - Missing proxy dependencies: ImportError path uses `click.secho(err=True)` with red colour and correct package name (`headroom-ai[proxy]`). - KeyboardInterrupt: exits 130 (SIGINT convention) instead of 0. ### Tests Added `tests/test_cli_proxy_improvements.py` with 44 new tests. All existing CLI tests continue to pass. --- ## Files changed - `headroom/cli/proxy.py` — env var wiring, range validation, help text, exception handling - `tests/test_cli_proxy_improvements.py` (new) — 44 tests - `CHANGELOG.md` — changelog entry > **Note:** `uv.lock` was removed from this PR per reviewer feedback. The lockfile is not tracked in this branch. --------- Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-06-10 21:53:18 -04:00
def test_help_contains_workers_option(self, runner: CliRunner) -> None:
assert "--workers" in self._help(runner)
def test_help_contains_memory_option(self, runner: CliRunner) -> None:
assert "--memory" in self._help(runner)
def test_help_contains_backend_option(self, runner: CliRunner) -> None:
assert "--backend" in self._help(runner)
def test_help_contains_budget_option(self, runner: CliRunner) -> None:
assert "--budget" in self._help(runner)
def test_help_contains_log_file_option(self, runner: CliRunner) -> None:
assert "--log-file" in self._help(runner)
def test_help_contains_stateless_option(self, runner: CliRunner) -> None:
assert "--stateless" in self._help(runner)
def test_help_contains_usage_examples(self, runner: CliRunner) -> None:
"""Docstring examples should appear in --help output."""
out = self._help(runner)
assert "ANTHROPIC_BASE_URL" in out
def test_proxy_short_help_alias(self, runner: CliRunner) -> None:
result = runner.invoke(main, ["proxy", "-?"])
assert result.exit_code == 0, result.output
assert "--mode" in result.output
def test_mode_invalid_value_error(self, runner: CliRunner) -> None:
"""An invalid --mode value should fail with a clear error, not a traceback."""
result = runner.invoke(main, ["proxy", "--mode", "bogus_mode_xyz"])
assert result.exit_code != 0
assert "invalid" in result.output.lower() or "choice" in result.output.lower()
fix(proxy): wire --compression-max-workers / HEADROOM_COMPRESSION_MAX_WORKERS (#1632) ## Description `ProxyConfig.compression_max_workers` is documented as settable via `--compression-max-workers` / `HEADROOM_COMPRESSION_MAX_WORKERS` and is consumed by `HeadroomProxy.__init__` to bound the dedicated compression threadpool. But the proxy CLI never defined the option and never passed the value into `ProxyConfig`, so the field was permanently `None` and always resolved to the `min(32, (cpu_count or 1) * 4)` default. Neither the flag nor the env var had any effect. This matters under concurrent sessions: the compression pool runs CPU-bound Kompress work that releases the GIL, so `cpu*4` oversubscribes cores and there was no way to cap it despite the docs promising one. Closes # ## 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 the `--compression-max-workers` click option (with `envvar="HEADROOM_COMPRESSION_MAX_WORKERS"`) to the `proxy` command, mirroring the existing `--anthropic-pre-upstream-concurrency` wiring. - Added the `compression_max_workers` parameter to the `proxy()` signature and passed it into the `ProxyConfig(...)` construction. - No change to `HeadroomProxy` — it already reads `config.compression_max_workers` and clamps `< 1` to 1. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ pytest tests/test_cli_proxy_improvements.py::TestCompressionMaxWorkers -q 3 passed in 1.50s $ pytest tests/test_cli_proxy_improvements.py -q 48 passed in 5.04s $ ruff check headroom/cli/proxy.py tests/test_cli_proxy_improvements.py All checks passed! $ mypy headroom/cli/proxy.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - Environment: macOS, Python 3.10.18, branch off upstream/main @ 0.28.0 - Exact command / steps: new tests assert the value reaches `ProxyConfig` via both `--compression-max-workers 3` (flag) and `HEADROOM_COMPRESSION_MAX_WORKERS=5` (env), and that it stays `None` when unset. - Observed result: flag -> `config.compression_max_workers == 3`; env -> `== 5`; unset -> `is None`. - Not tested: end-to-end proxy run under real concurrent load (the pool-sizing effect itself is already covered by existing `test_proxy_compression_executor.py`). ## 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 - [ ] I have updated the CHANGELOG.md if applicable ## Additional Notes CHANGELOG left untouched: this makes existing documented behavior actually work rather than adding new surface. N/A: manual testing (covered by unit tests + existing executor tests). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
2026-07-02 06:19:48 +02:00
class TestCompressionMaxWorkers:
"""--compression-max-workers / HEADROOM_COMPRESSION_MAX_WORKERS must reach ProxyConfig.
Regression: the field was documented in ProxyConfig and consumed by the
server, but the CLI never defined the option or passed it through, so it
perf(proxy): cap compression workers to CPU count (#1803) ## Description The request-path compression executor currently uses asyncio-style I/O sizing for CPU-bound Kompress work. When `compression_max_workers` is unset, `HeadroomProxy.__init__` resolves the pool to `min(32, cpu * 4)`, so an eight-core host can run 32 simultaneous compression workers that all contend for real CPU. This changes only the automatic request-path default to one worker per reported CPU while preserving the existing explicit override path from `--compression-max-workers` and `HEADROOM_COMPRESSION_MAX_WORKERS`. Closes #1635 ## 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 - [x] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Cap the automatic request-path compression executor default at `max(1, os.cpu_count() or 1)`. - Preserve explicit `compression_max_workers` values, including the existing clamp to at least one worker. - Keep CLI help, `ProxyConfig` comments, and nearby test documentation aligned with the CPU-bound default. - Update the focused compression executor regression so the default contract documents CPU-bound sizing, and keep the existing Codex compression stress guard stable when p50 rounds to zero. ## Testing - [x] Unit tests pass (`uv run pytest tests/test_codex_ws_compression_scheduler.py tests/test_proxy_compression_executor.py tests/test_cli_proxy_improvements.py::TestCompressionMaxWorkers -q`) - [x] Linting passes (`uv run ruff check headroom/cli/proxy.py headroom/proxy/models.py headroom/proxy/server.py tests/test_cli_proxy_improvements.py tests/test_proxy_compression_executor.py tests/test_codex_ws_compression_scheduler.py`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ uv run pytest tests/test_codex_ws_compression_scheduler.py tests/test_proxy_compression_executor.py tests/test_cli_proxy_improvements.py::TestCompressionMaxWorkers -q 16 passed, 1 skipped, 1 warning in 6.13s $ uv run ruff check headroom/cli/proxy.py headroom/proxy/models.py headroom/proxy/server.py tests/test_cli_proxy_improvements.py tests/test_proxy_compression_executor.py tests/test_codex_ws_compression_scheduler.py All checks passed! ``` ## Real Behavior Proof - Environment: Windows, Python environment from `uv sync --extra dev`, no provider credentials needed. - Exact command / steps: construct `HeadroomProxy` with `compression_max_workers=None`, inspect `proxy.compression_max_workers` and `/health` `runtime.compression_executor`. - Observed result: the automatic request-path pool resolves to reported CPU count, while explicit overrides still resolve to the configured value and report `source: explicit`. - Not tested: multi-session wall-clock benchmark under live Kompress load. ## 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 have updated the CHANGELOG.md if applicable ## Additional Notes No `CHANGELOG.md` edit: this repo generates changelog entries from conventional commits. This intentionally does not touch the background compression executor surface covered by #1633.
2026-07-05 17:01:23 -04:00
was permanently None (always resolving to the automatic server default).
fix(proxy): wire --compression-max-workers / HEADROOM_COMPRESSION_MAX_WORKERS (#1632) ## Description `ProxyConfig.compression_max_workers` is documented as settable via `--compression-max-workers` / `HEADROOM_COMPRESSION_MAX_WORKERS` and is consumed by `HeadroomProxy.__init__` to bound the dedicated compression threadpool. But the proxy CLI never defined the option and never passed the value into `ProxyConfig`, so the field was permanently `None` and always resolved to the `min(32, (cpu_count or 1) * 4)` default. Neither the flag nor the env var had any effect. This matters under concurrent sessions: the compression pool runs CPU-bound Kompress work that releases the GIL, so `cpu*4` oversubscribes cores and there was no way to cap it despite the docs promising one. Closes # ## 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 the `--compression-max-workers` click option (with `envvar="HEADROOM_COMPRESSION_MAX_WORKERS"`) to the `proxy` command, mirroring the existing `--anthropic-pre-upstream-concurrency` wiring. - Added the `compression_max_workers` parameter to the `proxy()` signature and passed it into the `ProxyConfig(...)` construction. - No change to `HeadroomProxy` — it already reads `config.compression_max_workers` and clamps `< 1` to 1. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ pytest tests/test_cli_proxy_improvements.py::TestCompressionMaxWorkers -q 3 passed in 1.50s $ pytest tests/test_cli_proxy_improvements.py -q 48 passed in 5.04s $ ruff check headroom/cli/proxy.py tests/test_cli_proxy_improvements.py All checks passed! $ mypy headroom/cli/proxy.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - Environment: macOS, Python 3.10.18, branch off upstream/main @ 0.28.0 - Exact command / steps: new tests assert the value reaches `ProxyConfig` via both `--compression-max-workers 3` (flag) and `HEADROOM_COMPRESSION_MAX_WORKERS=5` (env), and that it stays `None` when unset. - Observed result: flag -> `config.compression_max_workers == 3`; env -> `== 5`; unset -> `is None`. - Not tested: end-to-end proxy run under real concurrent load (the pool-sizing effect itself is already covered by existing `test_proxy_compression_executor.py`). ## 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 - [ ] I have updated the CHANGELOG.md if applicable ## Additional Notes CHANGELOG left untouched: this makes existing documented behavior actually work rather than adding new surface. N/A: manual testing (covered by unit tests + existing executor tests). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
2026-07-02 06:19:48 +02:00
"""
def test_flag_reaches_config(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main, ["proxy", "--compression-max-workers", "3"], catch_exceptions=False
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].compression_max_workers == 3
def test_env_reaches_config(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(
main,
["proxy"],
env={"HEADROOM_COMPRESSION_MAX_WORKERS": "5"},
catch_exceptions=False,
)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].compression_max_workers == 5
def test_default_is_none(self, runner: CliRunner, mock_run_server: dict) -> None:
result = runner.invoke(main, ["proxy"], catch_exceptions=False)
assert result.exit_code == 0, result.output
assert mock_run_server["config"].compression_max_workers is None