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`.
This commit is contained in:
JD Davis 2026-06-22 19:06:04 -05:00 committed by GitHub
parent c10969873b
commit b829ceba84
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 127 additions and 21 deletions

View file

@ -143,7 +143,6 @@ _WRAP_PROXY_TIMEOUT_ML_MODULES = ("torch", "sentence_transformers", "spacy")
_TOOL_SEARCH_ENV = TOOL_SEARCH_ENV _TOOL_SEARCH_ENV = TOOL_SEARCH_ENV
_TOOL_SEARCH_DEFAULT = TOOL_SEARCH_DEFAULT _TOOL_SEARCH_DEFAULT = TOOL_SEARCH_DEFAULT
_AGENT_SAVINGS_WRAP_AGENTS = {"claude", "codex", "cursor"} _AGENT_SAVINGS_WRAP_AGENTS = {"claude", "codex", "cursor"}
_DEFAULT_AGENT_SAVINGS_PROFILE = "agent-90"
def _normalize_tool_search_mode(value: str) -> str: 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: if agent_type not in _AGENT_SAVINGS_WRAP_AGENTS:
return None 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: def _default_wrap_proxy_timeout_seconds() -> int:
@ -399,9 +398,6 @@ def _start_proxy(
# Ensure proxy subprocess uses UTF-8 (Windows defaults to cp1252) # Ensure proxy subprocess uses UTF-8 (Windows defaults to cp1252)
proxy_env = os.environ.copy() proxy_env = os.environ.copy()
proxy_env["PYTHONIOENCODING"] = "utf-8" 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) # Tell the proxy which agent is being wrapped (for traffic learning output)
if agent_type != "unknown": if agent_type != "unknown":
proxy_env["HEADROOM_AGENT_TYPE"] = agent_type 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: if agent_type not in _AGENT_SAVINGS_TARGET_AGENTS:
return [] return []
if _wrap_agent_savings_profile(agent_type) is None:
return []
desired_env = os.environ.copy() desired_env = os.environ.copy()
apply_agent_savings_env_defaults(desired_env) apply_agent_savings_env_defaults(desired_env)
checks: tuple[tuple[str, str, str, str], ...] = ( checks: tuple[tuple[str, str, str, str], ...] = (

View file

@ -192,7 +192,8 @@ def acquire_runtime_start_lock(profile: str) -> Iterator[bool]:
import fcntl import fcntl
try: 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 acquired = True
except BlockingIOError: except BlockingIOError:
yield False yield False
@ -217,7 +218,8 @@ def acquire_runtime_start_lock(profile: str) -> Iterator[bool]:
else: else:
import fcntl 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: def run_foreground(manifest: DeploymentManifest) -> int:

View file

@ -283,9 +283,10 @@ def write_github_outputs(info: ReleaseVersionInfo, output_path: str) -> None:
def main() -> None: def main() -> None:
root = Path.cwd() root = Path.cwd()
manual_version = os.environ.get("MANUAL_VER", "").strip() 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( manual_match = re.fullmatch(
r"v?(\d+\.\d+\.\d+(?:[abrc]\d+)?)", 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: if manual_match:
version = manual_match.group(1) version = manual_match.group(1)

View file

@ -29,7 +29,7 @@ import threading
import time import time
from datetime import timedelta from datetime import timedelta
from pathlib import Path from pathlib import Path
from typing import Any from typing import Any, cast
from headroom import paths as _paths from headroom import paths as _paths
from headroom.subscription.base import QuotaTracker from headroom.subscription.base import QuotaTracker
@ -487,7 +487,8 @@ class SubscriptionTracker(QuotaTracker):
try: try:
lock_path.parent.mkdir(parents=True, exist_ok=True) lock_path.parent.mkdir(parents=True, exist_ok=True)
fd = open(lock_path, "w") # noqa: SIM115 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.write(str(os.getpid()))
fd.flush() fd.flush()
self._rtk_poll_lock_fd = fd self._rtk_poll_lock_fd = fd
@ -517,7 +518,8 @@ class SubscriptionTracker(QuotaTracker):
try: try:
import fcntl import fcntl
fcntl.flock(fd, fcntl.LOCK_UN) fcntl_any = cast(Any, fcntl)
fcntl_any.flock(fd, fcntl_any.LOCK_UN)
except Exception: except Exception:
pass pass
try: try:

View file

@ -174,7 +174,74 @@ def test_compress_savings_profile_does_not_mutate_supplied_config(monkeypatch) -
assert config.min_tokens_to_compress == 999 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) profile = get_agent_savings_profile(AGENT_90_PROFILE)
running_config = { running_config = {
"savings_profile": profile.name, "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") == [] 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) profile = get_agent_savings_profile(AGENT_90_PROFILE)
running_config = { running_config = {
"savings_profile": profile.name, "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") == [] 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 = { running_config = {
"savings_profile": None, "savings_profile": None,
"target_ratio": "not-a-float", "target_ratio": "not-a-float",

View file

@ -761,10 +761,10 @@ def test_start_proxy_uses_separate_session_for_signal_isolation(
@pytest.mark.parametrize("agent_type", ["claude", "codex", "cursor"]) @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 monkeypatch: pytest.MonkeyPatch, tmp_path: Path, agent_type: str
) -> None: ) -> 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] = {} popen_kwargs: dict[str, object] = {}
class FakeProc: class FakeProc:
@ -785,10 +785,10 @@ def test_start_proxy_applies_agent_90_defaults(
env = popen_kwargs["env"] env = popen_kwargs["env"]
assert isinstance(env, dict) assert isinstance(env, dict)
assert env["HEADROOM_SAVINGS_PROFILE"] == "agent-90" assert "HEADROOM_SAVINGS_PROFILE" not in env
assert env["HEADROOM_TARGET_RATIO"] == "0.10" assert "HEADROOM_TARGET_RATIO" not in env
assert env["HEADROOM_MAX_ITEMS"] == "8" assert "HEADROOM_MAX_ITEMS" not in env
assert env["HEADROOM_SMART_CRUSHER_COMPACTION"] == "0" assert "HEADROOM_SMART_CRUSHER_COMPACTION" not in env
def test_start_proxy_preserves_explicit_savings_overrides( def test_start_proxy_preserves_explicit_savings_overrides(

View file

@ -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" 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] = [] calls: list[object] = []
health = { health = {
"version": wrap_cli._HEADROOM_VERSION, "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}, "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, "_find_persistent_manifest", lambda port: None)
monkeypatch.setattr(wrap_cli, "_check_proxy", lambda port: len(calls) == 0) monkeypatch.setattr(wrap_cli, "_check_proxy", lambda port: len(calls) == 0)
monkeypatch.setattr(wrap_cli, "_query_proxy_health", lambda port: health) 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, "_find_persistent_manifest", lambda port: None)
monkeypatch.setattr(wrap_cli, "_check_proxy", lambda port: True) monkeypatch.setattr(wrap_cli, "_check_proxy", lambda port: True)
monkeypatch.setattr(wrap_cli, "_query_proxy_health", lambda port: health) monkeypatch.setattr(wrap_cli, "_query_proxy_health", lambda port: health)