headroom/tests/test_cli/test_wrap_proxy_detach.py

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

147 lines
5.9 KiB
Python
Raw Permalink Normal View History

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 20:49:28 +02:00
"""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