headroom/tests/test_cli/test_wrap_proxy_detach.py
quentinmaisonneuve 6cba4419d0
fix(wrap): detach the shared proxy on Windows so it survives an ungraceful agent close (#1464)
## Description

Closing one `headroom wrap <agent>` instance on Windows could kill the
**shared proxy** out from under every other running instance, so their
requests started failing.

`_start_proxy` launched the proxy as a child of whichever agent started
it first, without detaching it from that agent's console and Job object.
The wrapper already reference-counts clients via per-PID markers and
`_make_cleanup` leaves the proxy running while other clients exist — but
that only runs on a *graceful* exit. On an *ungraceful* close (closing
the terminal window, `taskkill`, a crash) Windows tree-kills the whole
process group/Job and the proxy dies directly, bypassing the reference
counting. Every other instance's `ANTHROPIC_BASE_URL` then points at a
dead `127.0.0.1:8787`, so all of its API traffic fails.

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

- `_start_proxy` creates the proxy with `DETACHED_PROCESS |
CREATE_NEW_PROCESS_GROUP | CREATE_BREAKAWAY_FROM_JOB` on Windows, so an
ungraceful close of the launching agent can no longer reach it; only the
ref-counted `_make_cleanup` ends the proxy.
- Falls back without `CREATE_BREAKAWAY_FROM_JOB` (catching `OSError`)
when the launcher's Job forbids breakaway; `DETACHED_PROCESS` still
spares the proxy from console-close events.
- Platform guard is `sys.platform == "win32"` (not `os.name == "nt"`) so
mypy narrows the platform and resolves the Windows-only `subprocess`
constants.
- POSIX path unchanged: `creationflags=0`, detachment still via
`start_new_session` (`setsid`).
- Added `tests/test_cli/test_wrap_proxy_detach.py` and a CHANGELOG Bug
Fixes entry.

## 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
$ pytest tests/test_cli/test_wrap_proxy_detach.py -q
..                                                                       [100%]
2 passed, 2 warnings in 1.50s

$ ruff check headroom/cli/wrap.py tests/test_cli/test_wrap_proxy_detach.py
All checks passed!

$ mypy --follow-imports=silent headroom/cli/wrap.py tests/test_cli/test_wrap_proxy_detach.py
Success: no issues found
```

## Real Behavior Proof

- Environment: Windows 11, Python 3.11.9, headroom-ai (pipx). Two
concurrent `headroom wrap claude` instances sharing proxy
`127.0.0.1:8787`. `_start_proxy` was also exercised directly on this
host with `subprocess.Popen` stubbed.
- Exact command / steps: (1) start two `headroom wrap claude` instances;
(2) close the terminal window of the one that started the proxy
(ungraceful — not `/exit`); (3) issue a request from the surviving
instance. Separately: call `_start_proxy(8787)` with `subprocess.Popen`
stubbed and read back the creation flags.
- Observed result: before the fix the proxy died with the closed window
and the surviving instance failed (`ANTHROPIC_BASE_URL` → dead `:8787`),
because the OS tree-killed the child before the ref-count path could
spare it. After the fix the detached proxy survives the close and the
surviving instance keeps working; the stub harness reports
`creationflags=0x1000208` (`DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP
| CREATE_BREAKAWAY_FROM_JOB`) on win32 and `0` when forced off-Windows.
- Not tested: real breakaway behavior under an actual restrictive Job
object on this host (the OS-level effect). The `OSError` fallback path
itself now has a dedicated unit test
(`test_start_proxy_retries_without_breakaway_when_job_forbids_it`).

## 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
- [x] I have updated the CHANGELOG.md if applicable

## Screenshots (if applicable)

N/A — no UI changes.

## Additional Notes

- Documentation checklist item is N/A: this is a behavioral bug fix with
no user-facing doc surface.
- Scope is the single `subprocess.Popen` call in `_start_proxy`; the
marker-based reference counting in `_make_cleanup` is unchanged and
remains the only thing that intentionally stops the proxy.
2026-06-30 13:49:28 -05:00

146 lines
5.9 KiB
Python

"""Detachment of the shared proxy subprocess in ``headroom wrap``.
``_start_proxy`` launches the proxy that every wrapped agent on a port shares.
It must outlive an *ungraceful* close of the agent that happened to start it
(closing the terminal window, taskkill, a crash) -- otherwise the OS tree-kills
the proxy and breaks the other live clients, bypassing the marker-based
reference counting in ``_make_cleanup``.
On Windows that means detaching from the launcher's console and Job object via
creation flags; on POSIX ``start_new_session`` already detaches via setsid().
"""
from __future__ import annotations
from pathlib import Path
from typing import Any
import pytest
from headroom.cli import wrap as wrap_cli
# Windows-only creation flag; not always exported by the host's ``subprocess``.
_CREATE_BREAKAWAY_FROM_JOB = 0x01000000
class _FakeProc:
"""Stand-in for a live proxy process (``poll() is None``)."""
returncode = 0
def poll(self) -> None:
return None
def _capture_popen_kwargs(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> dict[str, Any]:
"""Invoke ``_start_proxy`` with all I/O stubbed; return the Popen kwargs."""
captured: dict[str, Any] = {}
def _fake_popen(cmd: Any, **kwargs: Any) -> _FakeProc:
captured.clear()
captured.update(kwargs)
return _FakeProc()
monkeypatch.setattr(wrap_cli.subprocess, "Popen", _fake_popen)
monkeypatch.setattr(wrap_cli, "_check_proxy", lambda port: True)
monkeypatch.setattr(wrap_cli.time, "sleep", lambda _seconds: None)
monkeypatch.setattr(wrap_cli, "_get_log_path", lambda: tmp_path / "proxy.log")
monkeypatch.setattr(wrap_cli, "_resolve_wrap_proxy_timeout_seconds", lambda: 1)
wrap_cli._start_proxy(8787)
return captured
def test_start_proxy_detaches_on_windows(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
# Force the Windows branch regardless of the host OS, and supply the
# Windows-only ``subprocess`` constants the host lacks on POSIX.
monkeypatch.setattr(wrap_cli.sys, "platform", "win32")
monkeypatch.setattr(wrap_cli.subprocess, "CREATE_NO_WINDOW", 0x08000000, raising=False)
monkeypatch.setattr(wrap_cli.subprocess, "CREATE_NEW_PROCESS_GROUP", 0x200, raising=False)
flags = _capture_popen_kwargs(monkeypatch, tmp_path)["creationflags"]
assert flags & 0x08000000 # CREATE_NO_WINDOW: own hidden console
assert flags & 0x200 # CREATE_NEW_PROCESS_GROUP: ignore the parent's Ctrl-C
assert flags & _CREATE_BREAKAWAY_FROM_JOB # survive Job kill-on-close
def test_start_proxy_keeps_creationflags_zero_off_windows(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
monkeypatch.setattr(wrap_cli.sys, "platform", "linux")
kwargs = _capture_popen_kwargs(monkeypatch, tmp_path)
# POSIX detaches via setsid(); no Windows creation flags are applied.
assert kwargs["creationflags"] == 0
assert isinstance(kwargs["start_new_session"], bool)
def test_start_proxy_retries_without_breakaway_when_job_forbids_it(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
# Force the Windows branch and supply its constants, as above.
monkeypatch.setattr(wrap_cli.sys, "platform", "win32")
monkeypatch.setattr(wrap_cli.subprocess, "CREATE_NO_WINDOW", 0x08000000, raising=False)
monkeypatch.setattr(wrap_cli.subprocess, "CREATE_NEW_PROCESS_GROUP", 0x200, raising=False)
# First spawn raises OSError (the launcher's Job forbids breakaway); the
# second must succeed without CREATE_BREAKAWAY_FROM_JOB.
seen: list[int] = []
def _flaky_popen(cmd: Any, **kwargs: Any) -> _FakeProc:
seen.append(kwargs["creationflags"])
if len(seen) == 1:
raise OSError("a process in the job cannot break away")
return _FakeProc()
monkeypatch.setattr(wrap_cli.subprocess, "Popen", _flaky_popen)
monkeypatch.setattr(wrap_cli, "_check_proxy", lambda port: True)
monkeypatch.setattr(wrap_cli.time, "sleep", lambda _seconds: None)
monkeypatch.setattr(wrap_cli, "_get_log_path", lambda: tmp_path / "proxy.log")
monkeypatch.setattr(wrap_cli, "_resolve_wrap_proxy_timeout_seconds", lambda: 1)
wrap_cli._start_proxy(8787)
assert len(seen) == 2 # the call was retried exactly once
assert seen[0] & _CREATE_BREAKAWAY_FROM_JOB # first attempt requests breakaway
assert not seen[1] & _CREATE_BREAKAWAY_FROM_JOB # retry drops it
assert seen[1] & 0x08000000 # but still gets its own hidden console
assert seen[1] & 0x200 # and still gets its own process group
class _TrackingFile:
"""Records whether the parent closed its copy of the stdio log handle."""
def __init__(self) -> None:
self.closed = False
def close(self) -> None:
self.closed = True
def test_start_proxy_closes_log_when_both_spawns_fail(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
# Both spawn attempts raise (e.g. the Job forbids breakaway AND the retry
# still fails). The parent must not leak the stdio log file handle.
monkeypatch.setattr(wrap_cli.sys, "platform", "win32")
monkeypatch.setattr(wrap_cli.subprocess, "CREATE_NO_WINDOW", 0x08000000, raising=False)
monkeypatch.setattr(wrap_cli.subprocess, "CREATE_NEW_PROCESS_GROUP", 0x200, raising=False)
log_file = _TrackingFile()
monkeypatch.setattr(wrap_cli, "open", lambda *a, **k: log_file, raising=False)
def _always_fails(cmd: Any, **kwargs: Any) -> _FakeProc:
raise OSError("spawn failed on both attempts")
monkeypatch.setattr(wrap_cli.subprocess, "Popen", _always_fails)
monkeypatch.setattr(wrap_cli.time, "sleep", lambda _seconds: None)
monkeypatch.setattr(wrap_cli, "_get_log_path", lambda: tmp_path / "proxy.log")
monkeypatch.setattr(wrap_cli, "_resolve_wrap_proxy_timeout_seconds", lambda: 1)
with pytest.raises(OSError):
wrap_cli._start_proxy(8787)
assert log_file.closed # finally closed the handle despite the failure