mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix: wire HEADROOM_EXCLUDE_TOOLS / HEADROOM_TOOL_PROFILES into Click proxy entrypoint (#943)
## Description The Click-based `headroom proxy` entrypoint (`headroom/cli/proxy.py`) constructed `ProxyConfig` without calling `_parse_exclude_tools` or `_parse_tool_profiles`, so `HEADROOM_EXCLUDE_TOOLS` and `HEADROOM_TOOL_PROFILES` were silently ignored for any service launched via `headroom proxy`. The argparse path in `headroom/proxy/server.py` already handled these correctly. This PR imports both helpers into the Click entrypoint and wires their output into `ProxyConfig`. Closes #825 ## 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 - `headroom/cli/proxy.py`: import `_parse_exclude_tools` and `_parse_tool_profiles` alongside `ProxyConfig`/`run_server`; pass their output into the `ProxyConfig(...)` construction (`or None` guard collapses empty set/dict to `None` so unset vars leave `DEFAULT_EXCLUDE_TOOLS` unchanged) - `tests/test_cli_proxy_env.py`: new `TestCLIProxyExcludeToolsEnvVar` class with 5 regression tests ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ### Paste relevant command output or artifact links here ```text ============================= test session starts ============================== platform darwin -- Python 3.13.12, pytest-9.0.3 collected 43 items tests/test_cli_proxy_env.py::TestCLIProxyExcludeToolsEnvVar::test_exclude_tools_single_name_from_env PASSED tests/test_cli_proxy_env.py::TestCLIProxyExcludeToolsEnvVar::test_exclude_tools_multi_name_from_env PASSED tests/test_cli_proxy_env.py::TestCLIProxyExcludeToolsEnvVar::test_exclude_tools_unset_leaves_none PASSED tests/test_cli_proxy_env.py::TestCLIProxyExcludeToolsEnvVar::test_tool_profiles_from_env PASSED tests/test_cli_proxy_env.py::TestCLIProxyExcludeToolsEnvVar::test_tool_profiles_unset_leaves_none PASSED ============================== 43 passed in 8.95s ============================== ruff check headroom/cli/proxy.py tests/test_cli_proxy_env.py All checks passed! ``` ## Real Behavior Proof - Environment: Python 3.13.12, headroom-ai dev install - Exact command / steps: `HEADROOM_EXCLUDE_TOOLS=WebSearch headroom proxy` before fix silently built `ProxyConfig(exclude_tools=None)` despite the env var being set - Observed result: After fix, `ProxyConfig.exclude_tools` contains `{"WebSearch", "websearch"}` as verified by the new unit tests - Not tested: end-to-end proxy run with a live Anthropic endpoint ## 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 - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A ## Additional Notes The fix mirrors the exact pattern already used in the argparse path (`_main()` in `headroom/proxy/server.py` lines 3920-3922). The `or None` guard is intentional: `_parse_exclude_tools(None)` returns `set()` when the env var is unset, and `ProxyConfig.exclude_tools=None` means "use `DEFAULT_EXCLUDE_TOOLS` unchanged" — passing an empty set would instead replace the defaults with nothing. Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Matt Van Horn
GitHub
parent
e0a9fdb62c
commit
9b7b436b04
2 changed files with 125 additions and 1 deletions
|
|
@ -672,7 +672,12 @@ def proxy(
|
|||
"""
|
||||
# Import here to avoid slow startup
|
||||
try:
|
||||
from headroom.proxy.server import ProxyConfig, run_server
|
||||
from headroom.proxy.server import (
|
||||
ProxyConfig,
|
||||
_parse_exclude_tools,
|
||||
_parse_tool_profiles,
|
||||
run_server,
|
||||
)
|
||||
except ImportError as e:
|
||||
click.secho(
|
||||
"Error: Proxy dependencies not installed. Run: pip install headroom-ai[proxy]",
|
||||
|
|
@ -786,6 +791,8 @@ def proxy(
|
|||
compress_user_messages=_get_env_bool("HEADROOM_COMPRESS_USER_MESSAGES", False),
|
||||
min_tokens_to_crush=_get_env_int_optional("HEADROOM_MIN_TOKENS") or 500,
|
||||
max_items_after_crush=_get_env_int_optional("HEADROOM_MAX_ITEMS") or 50,
|
||||
exclude_tools=_parse_exclude_tools(None) or None,
|
||||
tool_profiles=_parse_tool_profiles([]) or None,
|
||||
smart_crusher_with_compaction=_get_env_bool_optional("HEADROOM_SMART_CRUSHER_COMPACTION"),
|
||||
savings_profile=os.environ.get("HEADROOM_SAVINGS_PROFILE") or None,
|
||||
target_ratio=_get_env_float_optional("HEADROOM_TARGET_RATIO"),
|
||||
|
|
|
|||
|
|
@ -785,3 +785,120 @@ class TestArgparseBackendValidation:
|
|||
config = _proxy_config_from_env()
|
||||
|
||||
assert config.disable_kompress is True
|
||||
|
||||
|
||||
class TestCLIProxyExcludeToolsEnvVar:
|
||||
"""HEADROOM_EXCLUDE_TOOLS and HEADROOM_TOOL_PROFILES must reach ProxyConfig via the Click path.
|
||||
|
||||
Regression coverage for issue #825: the Click entrypoint (headroom/cli/proxy.py)
|
||||
previously built ProxyConfig without calling _parse_exclude_tools or
|
||||
_parse_tool_profiles, so those env vars were silently ignored for all
|
||||
shared/deployed services that launch via `headroom proxy`.
|
||||
"""
|
||||
|
||||
def test_exclude_tools_single_name_from_env(self, runner):
|
||||
"""HEADROOM_EXCLUDE_TOOLS=WebSearch propagates to ProxyConfig.exclude_tools."""
|
||||
captured_config = {}
|
||||
|
||||
def mock_run_server(config, **kwargs):
|
||||
captured_config["config"] = config
|
||||
|
||||
with patch("headroom.proxy.server.run_server", mock_run_server):
|
||||
result = runner.invoke(
|
||||
main,
|
||||
["proxy"],
|
||||
env={"HEADROOM_EXCLUDE_TOOLS": "WebSearch"},
|
||||
catch_exceptions=False,
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
cfg = captured_config["config"]
|
||||
assert cfg.exclude_tools is not None
|
||||
assert "WebSearch" in cfg.exclude_tools
|
||||
|
||||
def test_exclude_tools_multi_name_from_env(self, runner):
|
||||
"""HEADROOM_EXCLUDE_TOOLS=WebSearch,WebFetch yields both names (and lowercased) in result."""
|
||||
captured_config = {}
|
||||
|
||||
def mock_run_server(config, **kwargs):
|
||||
captured_config["config"] = config
|
||||
|
||||
with patch("headroom.proxy.server.run_server", mock_run_server):
|
||||
result = runner.invoke(
|
||||
main,
|
||||
["proxy"],
|
||||
env={"HEADROOM_EXCLUDE_TOOLS": "WebSearch,WebFetch"},
|
||||
catch_exceptions=False,
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
cfg = captured_config["config"]
|
||||
assert cfg.exclude_tools is not None
|
||||
assert "WebSearch" in cfg.exclude_tools
|
||||
assert "WebFetch" in cfg.exclude_tools
|
||||
assert "websearch" in cfg.exclude_tools
|
||||
assert "webfetch" in cfg.exclude_tools
|
||||
|
||||
def test_exclude_tools_unset_leaves_none(self, runner):
|
||||
"""Without HEADROOM_EXCLUDE_TOOLS, exclude_tools stays None (DEFAULT_EXCLUDE_TOOLS used)."""
|
||||
captured_config = {}
|
||||
|
||||
def mock_run_server(config, **kwargs):
|
||||
captured_config["config"] = config
|
||||
|
||||
env = {k: v for k, v in os.environ.items() if k != "HEADROOM_EXCLUDE_TOOLS"}
|
||||
|
||||
with (
|
||||
patch("headroom.proxy.server.run_server", mock_run_server),
|
||||
patch.dict(os.environ, env, clear=True),
|
||||
):
|
||||
result = runner.invoke(
|
||||
main,
|
||||
["proxy"],
|
||||
catch_exceptions=False,
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert captured_config["config"].exclude_tools is None
|
||||
|
||||
def test_tool_profiles_from_env(self, runner):
|
||||
"""HEADROOM_TOOL_PROFILES=Grep:conservative propagates to ProxyConfig.tool_profiles."""
|
||||
captured_config = {}
|
||||
|
||||
def mock_run_server(config, **kwargs):
|
||||
captured_config["config"] = config
|
||||
|
||||
with patch("headroom.proxy.server.run_server", mock_run_server):
|
||||
result = runner.invoke(
|
||||
main,
|
||||
["proxy"],
|
||||
env={"HEADROOM_TOOL_PROFILES": "Grep:conservative"},
|
||||
catch_exceptions=False,
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
cfg = captured_config["config"]
|
||||
assert cfg.tool_profiles is not None
|
||||
assert "Grep" in cfg.tool_profiles
|
||||
|
||||
def test_tool_profiles_unset_leaves_none(self, runner):
|
||||
"""Without HEADROOM_TOOL_PROFILES, tool_profiles stays None."""
|
||||
captured_config = {}
|
||||
|
||||
def mock_run_server(config, **kwargs):
|
||||
captured_config["config"] = config
|
||||
|
||||
env = {k: v for k, v in os.environ.items() if k != "HEADROOM_TOOL_PROFILES"}
|
||||
|
||||
with (
|
||||
patch("headroom.proxy.server.run_server", mock_run_server),
|
||||
patch.dict(os.environ, env, clear=True),
|
||||
):
|
||||
result = runner.invoke(
|
||||
main,
|
||||
["proxy"],
|
||||
catch_exceptions=False,
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert captured_config["config"].tool_profiles is None
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue