diff --git a/CHANGELOG.md b/CHANGELOG.md index a0317a9dd..bafc2f98e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * **learn (verbosity):** `--verbosity --apply --all` now aggregates the savings baseline across every project instead of overwriting it per project (last-project-wins), which previously left the output shaper with a tiny, unrepresentative baseline. The applied verbosity level comes from the project with the most samples ([#1288](https://github.com/headroomlabs-ai/headroom/pull/1288)). * **proxy/anthropic:** restore token-mode compression on continued Claude Code turns with a frozen prefix and deferred CCR tool injection. Token mode now runs request-side compression even when the client did not pre-register `headroom_retrieve`, relying on the existing marker-triggered injection override to keep emitted CCR markers redeemable ([#1487](https://github.com/headroomlabs-ai/headroom/issues/1487)). * **subscription:** stop zeroing the 5-hour headroom contribution counters on every poll. The rollover check compared `five_hour.resets_at` with a bare `!=`, but the usage API reports that timestamp with second-level jitter (observed flapping between `01:59:59Z` and `02:00:00Z` on consecutive polls within the same window), so a spurious "5h window rolled over" reset fired every poll interval (~5 min) and the dashboard's per-window savings stuck near 0%. Only a forward jump larger than `_ROLLOVER_MIN_ADVANCE` (1 minute) now counts as a real rollover. +* **wrap:** keep the shared proxy alive when the agent that launched it closes *ungracefully* on Windows. `_start_proxy` spawned the proxy without detaching it, so it stayed in the launcher's console and Job object; closing that terminal window (or `taskkill`/a crash) tree-killed the proxy, bypassing the marker-based reference counting in `_make_cleanup` and breaking every other `headroom wrap` instance routed through the same port. The proxy is now created with `CREATE_NO_WINDOW | CREATE_NEW_PROCESS_GROUP | CREATE_BREAKAWAY_FROM_JOB` (with a graceful fallback when the launcher's Job forbids breakaway); POSIX behavior is unchanged. `CREATE_NO_WINDOW` (rather than `DETACHED_PROCESS`) gives the proxy its own *hidden* console: `DETACHED_PROCESS` leaves a console-subsystem exe (`python.exe`) consoleless, so Windows surfaces a visible console window whose close button kills the proxy. * **transforms/content_router:** stop replacing `role="tool"` output with a lossy-unrecoverable summary on the live compression path (refs [#1307](https://github.com/chopratejas/headroom/issues/1307)). `ContentRouter.apply()` routed OpenAI-style `role="tool"` string messages — `Bash`/`grep`/`ls`/`cat` output — through the ML/word-drop summarizers; when the result carried no CCR retrieve marker (CCR off, ratio >= 0.8, or the size-gate fallback) the original was unrecoverable and the agent acted on a fabricated summary. Tool-role string content is now kept verbatim unless the compressed form is CCR-recoverable. Assistant/user text is unaffected, and structurally-lossless passes (SmartCrusher/Log/Search) still apply. The Anthropic `tool_result` block path is tracked separately. * **rtk:** stop `rtk` hook registration from spuriously timing out during `headroom wrap`. Output is captured to a temp file instead of pipes, and `stdin` is closed, so a background process forked by `rtk init` can no longer hold the pipe open and block `subprocess.run` past its 10s timeout after the hooks were already registered. * **ccr:** stop re-compressing `headroom_retrieve` output, which created an infinite retrieval loop, and stop emitting retrieval markers when the `headroom_retrieve` tool is not injected, which silently dropped data ([#1077](https://github.com/chopratejas/headroom/issues/1077), [#1006](https://github.com/chopratejas/headroom/issues/1006)). diff --git a/headroom/cli/wrap.py b/headroom/cli/wrap.py index c6f8f8d0c..1c40b0154 100644 --- a/headroom/cli/wrap.py +++ b/headroom/cli/wrap.py @@ -462,39 +462,83 @@ def _start_proxy( if openai_api_url: proxy_env["GITHUB_COPILOT_API_URL"] = openai_api_url - proc = subprocess.Popen( - cmd, - stdout=stdio_log_file, - stderr=stdio_log_file, - env=proxy_env, - start_new_session=os.name == "posix", - ) + # Detach the proxy from the launching console on Windows so an ungraceful + # close of the owning agent (closing the terminal window, taskkill, or a + # crash) cannot tree-kill the shared proxy out from under other live + # clients. Without this the proxy stays in the owner's console + Job + # object; closing that window terminates the whole tree, bypassing the + # marker-based reference counting in ``_make_cleanup`` and breaking every + # other ``headroom wrap`` instance routed through the same port. + # CREATE_NO_WINDOW — give the proxy its OWN, invisible console. + # A separate console means the parent's + # CTRL_CLOSE_EVENT never reaches it, and no + # stray console window pops up. DETACHED_PROCESS + # also isolates the console, but for a console + # subsystem exe (python.exe) it leaves the proxy + # consoleless and Windows surfaces a visible + # console window — closing that window killed + # the proxy, defeating the whole point. + # CREATE_NEW_PROCESS_GROUP — isolate from the parent's Ctrl-C + # CREATE_BREAKAWAY_FROM_JOB— survive Job kill-on-close (Windows Terminal, + # VS Code integrated terminal, conhost) + # CREATE_NO_WINDOW / DETACHED_PROCESS / CREATE_NEW_CONSOLE are mutually + # exclusive — pick exactly one. On POSIX, ``start_new_session`` already + # detaches via setsid(). ``sys.platform == "win32"`` (not ``os.name == + # "nt"``) so mypy narrows the platform and resolves the Windows-only + # ``subprocess`` constants below. + _CREATE_BREAKAWAY_FROM_JOB = 0x01000000 + creationflags = 0 + if sys.platform == "win32": + creationflags = ( + subprocess.CREATE_NO_WINDOW + | subprocess.CREATE_NEW_PROCESS_GROUP + | _CREATE_BREAKAWAY_FROM_JOB + ) - # Wait for proxy to be ready. - # ML components (Kompress, Magika, Tree-sitter) load synchronously before - # uvicorn binds the port. On slower machines this can take 20-30 seconds. - for _i in range(timeout_seconds): - time.sleep(1) - if _check_proxy(port): - click.echo(f" Logs: {log_path}") - stdio_log_file.close() - return proc - # Check if process died - if proc.poll() is not None: - stdio_log_file.close() - # Read last few lines of log for error context - try: - tail = _read_text(stdio_log_path)[-500:] - except Exception: - tail = "(no log output)" - raise RuntimeError(f"Proxy exited with code {proc.returncode}: {tail}") + popen_kwargs: dict[str, Any] = { + "stdout": stdio_log_file, + "stderr": stdio_log_file, + "env": proxy_env, + "start_new_session": os.name == "posix", + "creationflags": creationflags, + } + # Close the parent's copy of the stdio log handle on every exit path, + # including when BOTH spawn attempts raise. The child keeps its own + # inherited duplicate, so closing here never starves the proxy's logging. + try: + try: + proc = subprocess.Popen(cmd, **popen_kwargs) + except OSError: + # The launcher's Job object forbids breakaway. Retry without that flag; + # CREATE_NO_WINDOW still spares the proxy from console-close events. + if sys.platform == "win32": + popen_kwargs["creationflags"] = creationflags & ~_CREATE_BREAKAWAY_FROM_JOB + proc = subprocess.Popen(cmd, **popen_kwargs) - proc.kill() - stdio_log_file.close() - raise RuntimeError( - f"Proxy failed to start on port {port} within {timeout_seconds} seconds. " - f"Set {_WRAP_PROXY_TIMEOUT_ENV} to a larger number of seconds for slow startup." - ) + # Wait for proxy to be ready. + # ML components (Kompress, Magika, Tree-sitter) load synchronously before + # uvicorn binds the port. On slower machines this can take 20-30 seconds. + for _i in range(timeout_seconds): + time.sleep(1) + if _check_proxy(port): + click.echo(f" Logs: {log_path}") + return proc + # Check if process died + if proc.poll() is not None: + # Read last few lines of log for error context + try: + tail = _read_text(stdio_log_path)[-500:] + except Exception: + tail = "(no log output)" + raise RuntimeError(f"Proxy exited with code {proc.returncode}: {tail}") + + proc.kill() + raise RuntimeError( + f"Proxy failed to start on port {port} within {timeout_seconds} seconds. " + f"Set {_WRAP_PROXY_TIMEOUT_ENV} to a larger number of seconds for slow startup." + ) + finally: + stdio_log_file.close() def _setup_rtk(verbose: bool = False) -> Path | None: diff --git a/tests/test_cli/test_wrap_proxy_detach.py b/tests/test_cli/test_wrap_proxy_detach.py new file mode 100644 index 000000000..283a22a01 --- /dev/null +++ b/tests/test_cli/test_wrap_proxy_detach.py @@ -0,0 +1,146 @@ +"""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