From b829ceba84ce058dadb4e70f6766af13806a4385 Mon Sep 17 00:00:00 2001 From: JD Davis Date: Mon, 22 Jun 2026 19:06:04 -0500 Subject: [PATCH] fix(wrap): keep agent savings opt-in (#1294) ## Description Fixes a regression from #830 where `headroom wrap codex` / `claude` / `cursor` treated `agent-90` as required even when the user started a normal proxy without `HEADROOM_SAVINGS_PROFILE`. A plain `headroom proxy` on port 8787 followed by `headroom wrap codex` currently reports `Proxy on port 8787 is missing: --savings-profile` and tries to restart the already-running proxy. The `agent-90` profile was documented as opt-in, so wrap should only require or inject it when `HEADROOM_SAVINGS_PROFILE` is explicitly set. Closes #1293 ## 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 - Stop defaulting agent wrappers to `agent-90` when `HEADROOM_SAVINGS_PROFILE` is unset. - Stop reporting agent-savings config mismatches unless an agent savings profile was explicitly requested. - Add regression tests for default wrap startup, explicit profile forwarding, and reuse/restart behavior around existing proxies. - Fix repo-wide pre-commit issues found during amend: Windows-safe `fcntl` typing, an optional env typing issue, OpenCode JSON parser return typing, and ruff import/format drift. ## Testing - [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 > maturin develop -m crates/headroom-py/Cargo.toml Finished `dev` profile [unoptimized + debuginfo] target(s) in 27.41s Built wheel for abi3 Python >= 3.10 Installed headroom-ai-0.27.0 > C:\git\headroom\.venv\Scripts\python.exe -c "from headroom._core import hello; print(hello())" headroom-core > ruff check . All checks passed! > ruff format --check . 913 files already formatted > C:\git\headroom\.venv\Scripts\python.exe -m mypy headroom Success: no issues found in 388 source files > C:\git\headroom\.venv\Scripts\python.exe -m pytest tests/test_agent_savings.py tests/test_cli/test_wrap_persistent.py tests/test_proxy_healthchecks.py tests/test_transforms/test_content_router.py -q collected 112 items 112 passed, 1 warning in 16.04s > git diff --check # no output > git commit --amend --no-edit Sync plugin versions.....................................................Passed ruff.....................................................................Passed ruff-format..............................................................Passed mypy.....................................................................Passed ``` ## Real Behavior Proof - Environment: Windows dev checkout, Python 3.13.3 via `C:\git\headroom\.venv\Scripts\python.exe`, Rust extension built with `maturin develop -m crates/headroom-py/Cargo.toml`. - Exact command / steps: reproduced the code path from #830 by exercising `_ensure_proxy(8787, False, agent_type="codex")` with a running proxy health payload that has no `savings_profile`, and by exercising `_start_proxy(8787, agent_type="codex")` with `HEADROOM_SAVINGS_PROFILE` unset and set. - Observed result: without `HEADROOM_SAVINGS_PROFILE`, wrap reuses the running proxy and `_start_proxy` does not inject `HEADROOM_SAVINGS_PROFILE` or `HEADROOM_TARGET_RATIO`; with `HEADROOM_SAVINGS_PROFILE=agent-90`, wrap still requires/forwards the profile and restarts an incompatible proxy. - Not tested: full end-to-end CLI launch against a live Codex binary. The focused proxy/wrap tests cover the failing restart/config decision directly. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A ## Additional Notes The pytest run emits an existing Windows `cp1252` background-thread warning while reading subprocess output; the tests still pass. No documentation or changelog update is included because this restores the already-documented opt-in behavior for `agent-90`. --- headroom/cli/wrap.py | 9 ++-- headroom/install/runtime.py | 6 ++- headroom/release_version.py | 3 +- headroom/subscription/tracker.py | 8 +-- tests/test_agent_savings.py | 75 ++++++++++++++++++++++++-- tests/test_cli/test_wrap_codex.py | 12 ++--- tests/test_cli/test_wrap_persistent.py | 35 +++++++++++- 7 files changed, 127 insertions(+), 21 deletions(-) diff --git a/headroom/cli/wrap.py b/headroom/cli/wrap.py index 35cdc0478..846f38a58 100644 --- a/headroom/cli/wrap.py +++ b/headroom/cli/wrap.py @@ -143,7 +143,6 @@ _WRAP_PROXY_TIMEOUT_ML_MODULES = ("torch", "sentence_transformers", "spacy") _TOOL_SEARCH_ENV = TOOL_SEARCH_ENV _TOOL_SEARCH_DEFAULT = TOOL_SEARCH_DEFAULT _AGENT_SAVINGS_WRAP_AGENTS = {"claude", "codex", "cursor"} -_DEFAULT_AGENT_SAVINGS_PROFILE = "agent-90" def _normalize_tool_search_mode(value: str) -> str: @@ -238,7 +237,7 @@ def _wrap_agent_savings_profile(agent_type: str) -> str | None: if agent_type not in _AGENT_SAVINGS_WRAP_AGENTS: return None - return os.environ.get("HEADROOM_SAVINGS_PROFILE") or _DEFAULT_AGENT_SAVINGS_PROFILE + return os.environ.get("HEADROOM_SAVINGS_PROFILE") or None def _default_wrap_proxy_timeout_seconds() -> int: @@ -399,9 +398,6 @@ def _start_proxy( # Ensure proxy subprocess uses UTF-8 (Windows defaults to cp1252) proxy_env = os.environ.copy() proxy_env["PYTHONIOENCODING"] = "utf-8" - if agent_type in {"claude", "codex", "cursor"}: - apply_agent_savings_env_defaults(proxy_env) - # Tell the proxy which agent is being wrapped (for traffic learning output) if agent_type != "unknown": proxy_env["HEADROOM_AGENT_TYPE"] = agent_type @@ -1936,6 +1932,9 @@ def _agent_savings_config_mismatches( if agent_type not in _AGENT_SAVINGS_TARGET_AGENTS: return [] + if _wrap_agent_savings_profile(agent_type) is None: + return [] + desired_env = os.environ.copy() apply_agent_savings_env_defaults(desired_env) checks: tuple[tuple[str, str, str, str], ...] = ( diff --git a/headroom/install/runtime.py b/headroom/install/runtime.py index feaa815e9..badd5394c 100644 --- a/headroom/install/runtime.py +++ b/headroom/install/runtime.py @@ -192,7 +192,8 @@ def acquire_runtime_start_lock(profile: str) -> Iterator[bool]: import fcntl try: - fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + fcntl_any = cast(Any, fcntl) + fcntl_any.flock(lock_file.fileno(), fcntl_any.LOCK_EX | fcntl_any.LOCK_NB) acquired = True except BlockingIOError: yield False @@ -217,7 +218,8 @@ def acquire_runtime_start_lock(profile: str) -> Iterator[bool]: else: import fcntl - fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN) + fcntl_any = cast(Any, fcntl) + fcntl_any.flock(lock_file.fileno(), fcntl_any.LOCK_UN) def run_foreground(manifest: DeploymentManifest) -> int: diff --git a/headroom/release_version.py b/headroom/release_version.py index 75561cfad..395f1f9ea 100644 --- a/headroom/release_version.py +++ b/headroom/release_version.py @@ -283,9 +283,10 @@ def write_github_outputs(info: ReleaseVersionInfo, output_path: str) -> None: def main() -> None: root = Path.cwd() manual_version = os.environ.get("MANUAL_VER", "").strip() + manual_raw = os.environ.get("MANUAL_VER") or os.environ.get("LEVEL") or "patch" manual_match = re.fullmatch( r"v?(\d+\.\d+\.\d+(?:[abrc]\d+)?)", - (os.environ.get("MANUAL_VER") or os.environ.get("LEVEL", "patch")).strip(), + manual_raw.strip(), ) if manual_match: version = manual_match.group(1) diff --git a/headroom/subscription/tracker.py b/headroom/subscription/tracker.py index a2b3c1d78..9b6547978 100644 --- a/headroom/subscription/tracker.py +++ b/headroom/subscription/tracker.py @@ -29,7 +29,7 @@ import threading import time from datetime import timedelta from pathlib import Path -from typing import Any +from typing import Any, cast from headroom import paths as _paths from headroom.subscription.base import QuotaTracker @@ -487,7 +487,8 @@ class SubscriptionTracker(QuotaTracker): try: lock_path.parent.mkdir(parents=True, exist_ok=True) fd = open(lock_path, "w") # noqa: SIM115 - fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + fcntl_any = cast(Any, fcntl) + fcntl_any.flock(fd, fcntl_any.LOCK_EX | fcntl_any.LOCK_NB) fd.write(str(os.getpid())) fd.flush() self._rtk_poll_lock_fd = fd @@ -517,7 +518,8 @@ class SubscriptionTracker(QuotaTracker): try: import fcntl - fcntl.flock(fd, fcntl.LOCK_UN) + fcntl_any = cast(Any, fcntl) + fcntl_any.flock(fd, fcntl_any.LOCK_UN) except Exception: pass try: diff --git a/tests/test_agent_savings.py b/tests/test_agent_savings.py index 21451bab7..38dbe2b4c 100644 --- a/tests/test_agent_savings.py +++ b/tests/test_agent_savings.py @@ -174,7 +174,74 @@ def test_compress_savings_profile_does_not_mutate_supplied_config(monkeypatch) - assert config.min_tokens_to_compress == 999 -def test_agent_savings_config_mismatches_returns_specific_labels() -> None: +def test_wrap_agent_savings_profile_is_opt_in(monkeypatch) -> None: + monkeypatch.delenv("HEADROOM_SAVINGS_PROFILE", raising=False) + + assert wrap_module._wrap_agent_savings_profile("codex") is None + + monkeypatch.setenv("HEADROOM_SAVINGS_PROFILE", AGENT_90_PROFILE) + + assert wrap_module._wrap_agent_savings_profile("codex") == AGENT_90_PROFILE + + +def test_agent_savings_config_mismatches_requires_explicit_profile(monkeypatch) -> None: + monkeypatch.delenv("HEADROOM_SAVINGS_PROFILE", raising=False) + + assert wrap_module._agent_savings_config_mismatches({}, "claude") == [] + + +def test_start_proxy_does_not_inject_agent_savings_by_default(monkeypatch, tmp_path) -> None: + captured_env: dict[str, str] = {} + + class Proc: + returncode = None + + def poll(self) -> None: + return None + + def popen(cmd, **kwargs): # noqa: ANN001 + captured_env.update(kwargs["env"]) + return Proc() + + monkeypatch.delenv("HEADROOM_SAVINGS_PROFILE", raising=False) + monkeypatch.setattr(wrap_module.subprocess, "Popen", popen) + monkeypatch.setattr(wrap_module.time, "sleep", lambda seconds: None) + monkeypatch.setattr(wrap_module, "_check_proxy", lambda port: True) + monkeypatch.setattr(wrap_module, "_get_log_path", lambda: tmp_path / "proxy.log") + + wrap_module._start_proxy(8787, agent_type="codex") + + assert "HEADROOM_SAVINGS_PROFILE" not in captured_env + assert "HEADROOM_TARGET_RATIO" not in captured_env + + +def test_start_proxy_injects_explicit_agent_savings_profile(monkeypatch, tmp_path) -> None: + captured_env: dict[str, str] = {} + + class Proc: + returncode = None + + def poll(self) -> None: + return None + + def popen(cmd, **kwargs): # noqa: ANN001 + captured_env.update(kwargs["env"]) + return Proc() + + monkeypatch.setenv("HEADROOM_SAVINGS_PROFILE", AGENT_90_PROFILE) + monkeypatch.setattr(wrap_module.subprocess, "Popen", popen) + monkeypatch.setattr(wrap_module.time, "sleep", lambda seconds: None) + monkeypatch.setattr(wrap_module, "_check_proxy", lambda port: True) + monkeypatch.setattr(wrap_module, "_get_log_path", lambda: tmp_path / "proxy.log") + + wrap_module._start_proxy(8787, agent_type="codex") + + assert captured_env["HEADROOM_SAVINGS_PROFILE"] == AGENT_90_PROFILE + assert captured_env["HEADROOM_TARGET_RATIO"] == "0.10" + + +def test_agent_savings_config_mismatches_returns_specific_labels(monkeypatch) -> None: + monkeypatch.setenv("HEADROOM_SAVINGS_PROFILE", AGENT_90_PROFILE) profile = get_agent_savings_profile(AGENT_90_PROFILE) running_config = { "savings_profile": profile.name, @@ -196,7 +263,8 @@ def test_agent_savings_config_mismatches_ignores_non_target_agents() -> None: assert wrap_module._agent_savings_config_mismatches({}, "openhands") == [] -def test_agent_savings_config_mismatches_accepts_matching_runtime_config() -> None: +def test_agent_savings_config_mismatches_accepts_matching_runtime_config(monkeypatch) -> None: + monkeypatch.setenv("HEADROOM_SAVINGS_PROFILE", AGENT_90_PROFILE) profile = get_agent_savings_profile(AGENT_90_PROFILE) running_config = { "savings_profile": profile.name, @@ -214,7 +282,8 @@ def test_agent_savings_config_mismatches_accepts_matching_runtime_config() -> No assert wrap_module._agent_savings_config_mismatches(running_config, "cursor") == [] -def test_agent_savings_config_mismatches_reports_unparseable_values() -> None: +def test_agent_savings_config_mismatches_reports_unparseable_values(monkeypatch) -> None: + monkeypatch.setenv("HEADROOM_SAVINGS_PROFILE", AGENT_90_PROFILE) running_config = { "savings_profile": None, "target_ratio": "not-a-float", diff --git a/tests/test_cli/test_wrap_codex.py b/tests/test_cli/test_wrap_codex.py index 1fff7eb67..1aaa0173f 100644 --- a/tests/test_cli/test_wrap_codex.py +++ b/tests/test_cli/test_wrap_codex.py @@ -761,10 +761,10 @@ def test_start_proxy_uses_separate_session_for_signal_isolation( @pytest.mark.parametrize("agent_type", ["claude", "codex", "cursor"]) -def test_start_proxy_applies_agent_90_defaults( +def test_start_proxy_does_not_apply_agent_90_defaults( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, agent_type: str ) -> None: - """Wrapped coding agents should start the proxy with high-savings defaults.""" + """Wrapped coding agents keep agent-savings opt-in by default.""" popen_kwargs: dict[str, object] = {} class FakeProc: @@ -785,10 +785,10 @@ def test_start_proxy_applies_agent_90_defaults( env = popen_kwargs["env"] assert isinstance(env, dict) - assert env["HEADROOM_SAVINGS_PROFILE"] == "agent-90" - assert env["HEADROOM_TARGET_RATIO"] == "0.10" - assert env["HEADROOM_MAX_ITEMS"] == "8" - assert env["HEADROOM_SMART_CRUSHER_COMPACTION"] == "0" + assert "HEADROOM_SAVINGS_PROFILE" not in env + assert "HEADROOM_TARGET_RATIO" not in env + assert "HEADROOM_MAX_ITEMS" not in env + assert "HEADROOM_SMART_CRUSHER_COMPACTION" not in env def test_start_proxy_preserves_explicit_savings_overrides( diff --git a/tests/test_cli/test_wrap_persistent.py b/tests/test_cli/test_wrap_persistent.py index 7bdfb5654..e440dc8db 100644 --- a/tests/test_cli/test_wrap_persistent.py +++ b/tests/test_cli/test_wrap_persistent.py @@ -321,7 +321,38 @@ def test_ensure_proxy_restarts_ephemeral_proxy_for_openai_api_url_mismatch(monke assert calls[1][2]["openai_api_url"] == "https://api.individual.githubcopilot.com" -def test_ensure_proxy_restarts_agent_proxy_without_savings_profile(monkeypatch) -> None: +def test_ensure_proxy_reuses_agent_proxy_without_savings_profile(monkeypatch) -> None: + health = { + "version": wrap_cli._HEADROOM_VERSION, + "runtime": {"websocket_sessions": {"active_sessions": 0, "active_relay_tasks": 0}}, + "config": {"pid": "12345", "memory": False, "learn": False, "code_graph": False}, + } + + monkeypatch.delenv("HEADROOM_SAVINGS_PROFILE", raising=False) + monkeypatch.setattr(wrap_cli, "_find_persistent_manifest", lambda port: None) + monkeypatch.setattr(wrap_cli, "_check_proxy", lambda port: True) + monkeypatch.setattr(wrap_cli, "_query_proxy_health", lambda port: health) + monkeypatch.setattr( + wrap_cli, + "_kill_proxy_by_pid", + lambda *args, **kwargs: (_ for _ in ()).throw( + AssertionError("default agent proxy should not restart for savings profile") + ), + ) + monkeypatch.setattr( + wrap_cli, + "_start_proxy", + lambda *args, **kwargs: (_ for _ in ()).throw( + AssertionError("replacement proxy should not start") + ), + ) + + result = wrap_cli._ensure_proxy(8787, False, agent_type="codex") + + assert result is None + + +def test_ensure_proxy_restarts_for_explicit_agent_savings_profile(monkeypatch) -> None: calls: list[object] = [] health = { "version": wrap_cli._HEADROOM_VERSION, @@ -329,6 +360,7 @@ def test_ensure_proxy_restarts_agent_proxy_without_savings_profile(monkeypatch) "config": {"pid": "12345", "memory": False, "learn": False, "code_graph": False}, } + monkeypatch.setenv("HEADROOM_SAVINGS_PROFILE", "agent-90") monkeypatch.setattr(wrap_cli, "_find_persistent_manifest", lambda port: None) monkeypatch.setattr(wrap_cli, "_check_proxy", lambda port: len(calls) == 0) monkeypatch.setattr(wrap_cli, "_query_proxy_health", lambda port: health) @@ -373,6 +405,7 @@ def test_ensure_proxy_reuses_agent_proxy_with_savings_profile(monkeypatch) -> No }, } + monkeypatch.setenv("HEADROOM_SAVINGS_PROFILE", "agent-90") monkeypatch.setattr(wrap_cli, "_find_persistent_manifest", lambda port: None) monkeypatch.setattr(wrap_cli, "_check_proxy", lambda port: True) monkeypatch.setattr(wrap_cli, "_query_proxy_health", lambda port: health)