mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(install): repair macOS launchd restart/start lifecycle (#1290)
## Description Fixes `headroom install restart` and `headroom install start` for macOS launchd `persistent-service` deployments — both currently leave the proxy **stopped**. `restart = stop + start`, but the two halves used incompatible `launchctl` verbs: `stop` runs `launchctl bootout` (which **unregisters** the job from the domain), while `start` only ran `launchctl kickstart -k` (which requires the job to **still be registered**). After `bootout` removes the job, `kickstart` can never find it again (`exit 113`), and nothing ever called `launchctl bootstrap` — so neither a post-`bootout` restart nor a cold `start` could (re)register it. `stop` also used `check=True`, so booting out an already-absent job (`exit 3`) raised and aborted `restart` before it could start again. Closes #1289 ## 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_supervisor` (darwin): try `launchctl kickstart -k` first (fast path when the job is already bootstrapped, e.g. right after `install apply` or on a running service); on failure, `launchctl bootstrap` the plist fresh — which also starts it via `RunAtLoad`. - Retry the `bootstrap` for ~15s. launchd returns EIO (`Bootstrap failed: 5: Input/output error`) from `bootstrap` for several seconds after a `bootout` while it releases the label; on exhaustion a `click.ClickException` surfaces the last launchctl error instead of a raw traceback. Tunables: `_MACOS_BOOTSTRAP_RETRIES` / `_MACOS_BOOTSTRAP_RETRY_DELAY`. - `stop_supervisor` (darwin): run `bootout` with `check=False` so an already-absent job (`exit 3`) is treated as already-stopped rather than aborting `restart`. - Tests: 5 new cases in `tests/test_install/test_supervisors.py` (warm `kickstart` success, `bootstrap` fallback when not registered, EIO retry, raise-after-exhaustion, tolerant stop); `time.sleep` is monkeypatched so they stay fast. - `CHANGELOG.md`: entry under Unreleased → Bug Fixes. ## 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_install/ 77 passed, 1 skipped, 1 warning in 5.35s $ pytest tests/test_install/test_supervisors.py -q 19 passed, 1 warning in 0.10s $ ruff check headroom/install/supervisors.py tests/test_install/test_supervisors.py All checks passed! $ ruff format --check headroom/install/supervisors.py tests/test_install/test_supervisors.py 2 files already formatted $ mypy --python-version 3.10 headroom/install/supervisors.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - Environment: macOS 26.5.1 (arm64), launchd 7.0.0, headroom installed via pipx; profile `default`, preset `persistent-service`, scope `user`, port 8787. - Exact command / steps: patched the installed `supervisors.py` to this exact code, then exercised the live deployment — `headroom install restart --profile default` (warm restart), `headroom install stop --profile default`, then `headroom install start --profile default` (cold start, post-bootout); health checked via `curl http://127.0.0.1:8787/readyz` and `headroom install status` after each. - Observed result: every transition lands healthy with no traceback (before this PR they failed). `install restart` on a running service → healthy (was: `bootout` exit 3 → abort, proxy down); `install start` cold/post-bootout → healthy in ~8s (was: exit 113 / EIO); `install stop` → down; `install start` from stopped → healthy; 3× rapid `install restart` → all healthy. The EIO settle window was measured directly: `bootstrap` failed with error 5 for ~5s (10 attempts) then succeeded on attempt 11 — which is what the retry loop rides out. - Not tested: system-scope (`/Library/LaunchDaemons`) deployments and the Linux/Windows branches were not exercised on hardware (unchanged by this PR); covered by unit tests only. ## 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 — CLI lifecycle change. ## Additional Notes - Docs checkbox left unchecked: no user-facing docs describe the launchd lifecycle internals; happy to add a note if you point me at the right place. - **Tradeoff:** because the correct post-`bootout` recovery has to wait out launchd's ~5s EIO window, `restart` and cold `start` take several seconds. The `kickstart`-first fast path keeps the common already-bootstrapped case instant; only the post-`bootout` path pays the settle. Open to a different shape if you'd prefer (e.g. having `restart` avoid the full `bootout`). - CI-only checks (commitlint, pre-commit `ci-precheck`) were not run locally; the commit header follows conventional commits (`fix(install): …`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
This commit is contained in:
parent
5e0bb69725
commit
da1a3973ed
3 changed files with 177 additions and 5 deletions
|
|
@ -40,7 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
* **proxy:** make `force_kompress` skip ContentRouter auto-detection during compression and pass savings-profile kwargs through Anthropic batch requests.
|
||||
* **proxy:** add native Bedrock `/model/{id}/converse-stream` route and forward it through the existing streaming EventStream/SSE pipeline.
|
||||
* **wrap (codex):** fix `headroom wrap codex` producing a `config.toml` with duplicate top-level `model_provider` / `openai_base_url` keys (TOML-spec error) when the user had already configured their own provider. The injector now rewrites pre-existing top-level `model_provider` and `openai_base_url` lines in place — the previous value is kept in a `# was: …` trailing comment — instead of unconditionally prepending a duplicate, so `codex` can start against the proxy. The pre-wrap snapshot mechanism continues to byte-for-byte restore the original file on `headroom unwrap codex`.
|
||||
|
||||
* **install (macOS):** fix `headroom install restart` / `install start` for launchd `persistent-service` deployments. `stop` `bootout`s the job but `start` only ran `launchctl kickstart`, which cannot recover the un-bootstrapped state `stop`/`restart` leave behind (launchctl error 113), so the proxy was left stopped. `start` now tries `kickstart` (fast path for an already-bootstrapped job) and, on failure, `bootstrap`s the plist fresh — retrying for ~15s to ride out the transient `bootstrap` EIO (error 5) window while launchd releases the label after a `bootout`. `stop` tolerates only the already-absent case (`bootout` ESRCH / error 3) and still raises on any other `bootout` failure ([#1289](https://github.com/headroomlabs-ai/headroom/issues/1289)).
|
||||
* **wrap:** isolate wrapped proxy subprocess stdout/stderr into `proxy-stdio.log`, so `proxy.log` remains the canonical rotating runtime log and Windows rollover failures from `RotatingFileHandler` are no longer blocked by wrapper stdio handles ([#1184](https://github.com/chopratejas/headroom/issues/1184)).
|
||||
* **langchain:** fix `HeadroomChatModel.ainvoke()` crashing with `AttributeError: 'AsyncStream' object has no attribute 'model_dump'` when the wrapped model has `streaming=True`. `_agenerate()` now uses a per-call non-streaming copy of the wrapped model instead of mutating shared state across an `await` ([#1285](https://github.com/headroomlabs-ai/headroom/issues/1285)).
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import re
|
|||
import shlex
|
||||
import subprocess
|
||||
import sys
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
import click
|
||||
|
|
@ -22,6 +23,16 @@ from .paths import (
|
|||
)
|
||||
from .runtime import resolve_headroom_command
|
||||
|
||||
# After `launchctl bootout`, a follow-up `bootstrap` of the same label can
|
||||
# return EIO (error 5) for several seconds while launchd releases it. Retry the
|
||||
# bootstrap up to ~15s (30 attempts x 0.5s) to ride out that settle window.
|
||||
_MACOS_BOOTSTRAP_RETRIES = 30
|
||||
_MACOS_BOOTSTRAP_RETRY_DELAY = 0.5
|
||||
|
||||
# `launchctl bootout` of an already-absent job exits with ESRCH ("No such
|
||||
# process"). That single code is the only failure we treat as already-stopped.
|
||||
_LAUNCHCTL_ESRCH = 3
|
||||
|
||||
|
||||
def _is_windows() -> bool:
|
||||
return sys.platform.startswith("win")
|
||||
|
|
@ -310,8 +321,44 @@ def start_supervisor(manifest: DeploymentManifest) -> None:
|
|||
and manifest.supervisor_kind == SupervisorKind.SERVICE.value
|
||||
else f"gui/{os.getuid()}"
|
||||
)
|
||||
subprocess.run(["launchctl", "kickstart", "-k", f"{domain}/{label}"], check=True)
|
||||
return
|
||||
# Fast path: when the job is already bootstrapped (e.g. `start` right
|
||||
# after `install apply`, or `start` on a running service), `kickstart`
|
||||
# restarts it in place.
|
||||
kick = subprocess.run(
|
||||
["launchctl", "kickstart", "-k", f"{domain}/{label}"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if kick.returncode == 0:
|
||||
return
|
||||
# Otherwise the job is not registered in the domain. This is the state
|
||||
# `stop`/`restart` leave behind, since they `bootout` the job, and
|
||||
# `kickstart` cannot recover it (launchctl error 113). Bootstrap fresh
|
||||
# instead — a successful bootstrap also starts the job via RunAtLoad.
|
||||
# launchd can return EIO (error 5) from bootstrap for several seconds
|
||||
# after a bootout while it releases the label, so retry for ~15s.
|
||||
plist_dir = (
|
||||
Path("/Library/LaunchDaemons")
|
||||
if manifest.scope == "system"
|
||||
and manifest.supervisor_kind == SupervisorKind.SERVICE.value
|
||||
else Path.home() / "Library" / "LaunchAgents"
|
||||
)
|
||||
plist_path = plist_dir / f"{label}.plist"
|
||||
last = kick
|
||||
for _ in range(_MACOS_BOOTSTRAP_RETRIES):
|
||||
boot = subprocess.run(
|
||||
["launchctl", "bootstrap", domain, str(plist_path)],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if boot.returncode == 0:
|
||||
return
|
||||
last = boot
|
||||
time.sleep(_MACOS_BOOTSTRAP_RETRY_DELAY)
|
||||
detail = (last.stderr or last.stdout or "").strip()
|
||||
raise click.ClickException(
|
||||
f"launchctl could not start {domain}/{label}: {detail or 'unknown error'}"
|
||||
)
|
||||
if _is_windows() and manifest.supervisor_kind == SupervisorKind.SERVICE.value:
|
||||
subprocess.run(["sc.exe", "start", manifest.service_name], check=True)
|
||||
|
||||
|
|
@ -333,7 +380,21 @@ def stop_supervisor(manifest: DeploymentManifest) -> None:
|
|||
and manifest.supervisor_kind == SupervisorKind.SERVICE.value
|
||||
else f"gui/{os.getuid()}"
|
||||
)
|
||||
subprocess.run(["launchctl", "bootout", f"{domain}/{label}"], check=True)
|
||||
# `bootout` exits with ESRCH ("No such process") when the job is already
|
||||
# absent — tolerate only that, so `restart` can proceed to start again.
|
||||
# Any other non-zero result is a real failure (permissions, malformed
|
||||
# domain, launchd error) and must surface; otherwise `restart` could
|
||||
# report success while a stale job is still running.
|
||||
result = subprocess.run(
|
||||
["launchctl", "bootout", f"{domain}/{label}"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if result.returncode not in (0, _LAUNCHCTL_ESRCH):
|
||||
detail = (result.stderr or result.stdout or "").strip()
|
||||
raise click.ClickException(
|
||||
f"launchctl bootout failed for {domain}/{label}: {detail or 'unknown error'}"
|
||||
)
|
||||
return
|
||||
if _is_windows() and manifest.supervisor_kind == SupervisorKind.SERVICE.value:
|
||||
subprocess.run(["sc.exe", "stop", manifest.service_name], check=True)
|
||||
|
|
|
|||
|
|
@ -335,11 +335,18 @@ def test_install_supervisor_darwin_windows_and_unsupported(monkeypatch, tmp_path
|
|||
install_supervisor(_manifest(supervisor=SupervisorKind.SERVICE.value))
|
||||
|
||||
|
||||
class _LaunchctlResult:
|
||||
def __init__(self, returncode: int = 0, stderr: str = "", stdout: str = "") -> None:
|
||||
self.returncode = returncode
|
||||
self.stderr = stderr
|
||||
self.stdout = stdout
|
||||
|
||||
|
||||
def test_start_and_stop_supervisor_darwin_windows_and_none(monkeypatch) -> None:
|
||||
calls: list[list[str]] = []
|
||||
monkeypatch.setattr(
|
||||
"headroom.install.supervisors.subprocess.run",
|
||||
lambda command, **kwargs: calls.append(command),
|
||||
lambda command, **kwargs: calls.append(command) or _LaunchctlResult(0),
|
||||
)
|
||||
monkeypatch.setattr("headroom.install.supervisors.os.getuid", lambda: 77, raising=False)
|
||||
|
||||
|
|
@ -349,6 +356,8 @@ def test_start_and_stop_supervisor_darwin_windows_and_none(monkeypatch) -> None:
|
|||
|
||||
monkeypatch.setattr("headroom.install.supervisors.sys.platform", "darwin")
|
||||
monkeypatch.setattr("headroom.install.supervisors.sys.platform", "darwin")
|
||||
# Warm path: kickstart succeeds (job already bootstrapped), so start does
|
||||
# not fall through to bootstrap.
|
||||
start_supervisor(_manifest(supervisor=SupervisorKind.SERVICE.value))
|
||||
stop_supervisor(_manifest(supervisor=SupervisorKind.SERVICE.value))
|
||||
assert calls == [
|
||||
|
|
@ -367,6 +376,108 @@ def test_start_and_stop_supervisor_darwin_windows_and_none(monkeypatch) -> None:
|
|||
]
|
||||
|
||||
|
||||
def test_macos_start_bootstraps_when_job_not_registered(monkeypatch, tmp_path: Path) -> None:
|
||||
# Post-`stop`/`restart` state: the job was booted out, so `kickstart` fails
|
||||
# (launchctl 113) and start must bootstrap the plist instead.
|
||||
monkeypatch.setattr("headroom.install.supervisors.sys.platform", "darwin")
|
||||
monkeypatch.setattr("headroom.install.supervisors.os.getuid", lambda: 77, raising=False)
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
calls: list[list[str]] = []
|
||||
|
||||
def fake_run(command, **kwargs):
|
||||
calls.append(command)
|
||||
if command[1] == "kickstart":
|
||||
return _LaunchctlResult(113, stderr="Could not find service")
|
||||
return _LaunchctlResult(0)
|
||||
|
||||
monkeypatch.setattr("headroom.install.supervisors.subprocess.run", fake_run)
|
||||
|
||||
start_supervisor(_manifest(supervisor=SupervisorKind.SERVICE.value))
|
||||
|
||||
plist_path = tmp_path / "Library" / "LaunchAgents" / "com.headroom.default.plist"
|
||||
assert calls == [
|
||||
["launchctl", "kickstart", "-k", "gui/77/com.headroom.default"],
|
||||
["launchctl", "bootstrap", "gui/77", str(plist_path)],
|
||||
]
|
||||
|
||||
|
||||
def test_macos_start_retries_bootstrap_until_launchd_settles(monkeypatch, tmp_path: Path) -> None:
|
||||
# launchd returns EIO (error 5) from bootstrap for a while after a bootout;
|
||||
# start should retry until it succeeds.
|
||||
monkeypatch.setattr("headroom.install.supervisors.sys.platform", "darwin")
|
||||
monkeypatch.setattr("headroom.install.supervisors.os.getuid", lambda: 77, raising=False)
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
monkeypatch.setattr("headroom.install.supervisors.time.sleep", lambda _s: None)
|
||||
bootstrap_attempts = 0
|
||||
|
||||
def fake_run(command, **kwargs):
|
||||
nonlocal bootstrap_attempts
|
||||
if command[1] == "kickstart":
|
||||
return _LaunchctlResult(113)
|
||||
bootstrap_attempts += 1
|
||||
if bootstrap_attempts < 3:
|
||||
return _LaunchctlResult(5, stderr="Bootstrap failed: 5: Input/output error")
|
||||
return _LaunchctlResult(0)
|
||||
|
||||
monkeypatch.setattr("headroom.install.supervisors.subprocess.run", fake_run)
|
||||
|
||||
start_supervisor(_manifest(supervisor=SupervisorKind.SERVICE.value))
|
||||
assert bootstrap_attempts == 3
|
||||
|
||||
|
||||
def test_macos_start_raises_after_bootstrap_keeps_failing(monkeypatch, tmp_path: Path) -> None:
|
||||
monkeypatch.setattr("headroom.install.supervisors.sys.platform", "darwin")
|
||||
monkeypatch.setattr("headroom.install.supervisors.os.getuid", lambda: 77, raising=False)
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
monkeypatch.setattr("headroom.install.supervisors.time.sleep", lambda _s: None)
|
||||
monkeypatch.setattr("headroom.install.supervisors._MACOS_BOOTSTRAP_RETRIES", 3)
|
||||
|
||||
def fake_run(command, **kwargs):
|
||||
if command[1] == "kickstart":
|
||||
return _LaunchctlResult(113)
|
||||
return _LaunchctlResult(5, stderr="Bootstrap failed: 5: Input/output error")
|
||||
|
||||
monkeypatch.setattr("headroom.install.supervisors.subprocess.run", fake_run)
|
||||
|
||||
with pytest.raises(click.ClickException, match="could not start"):
|
||||
start_supervisor(_manifest(supervisor=SupervisorKind.SERVICE.value))
|
||||
|
||||
|
||||
def test_macos_stop_tolerates_missing_job(monkeypatch) -> None:
|
||||
# `bootout` of an absent job exits with ESRCH (3); stop must not raise so
|
||||
# that `restart` can proceed to start again.
|
||||
monkeypatch.setattr("headroom.install.supervisors.sys.platform", "darwin")
|
||||
monkeypatch.setattr("headroom.install.supervisors.os.getuid", lambda: 77, raising=False)
|
||||
calls: list[list[str]] = []
|
||||
|
||||
def fake_run(command, **kwargs):
|
||||
calls.append(command)
|
||||
assert kwargs.get("check") is not True
|
||||
return _LaunchctlResult(3, stderr="Boot-out failed: 3: No such process")
|
||||
|
||||
monkeypatch.setattr("headroom.install.supervisors.subprocess.run", fake_run)
|
||||
|
||||
stop_supervisor(_manifest(supervisor=SupervisorKind.SERVICE.value))
|
||||
assert calls == [["launchctl", "bootout", "gui/77/com.headroom.default"]]
|
||||
|
||||
|
||||
def test_macos_stop_raises_on_non_esrch_failure(monkeypatch) -> None:
|
||||
# A non-3 `bootout` failure (e.g. permissions) is a real error and must
|
||||
# surface — otherwise `restart` could report success with a stale job still
|
||||
# running.
|
||||
monkeypatch.setattr("headroom.install.supervisors.sys.platform", "darwin")
|
||||
monkeypatch.setattr("headroom.install.supervisors.os.getuid", lambda: 77, raising=False)
|
||||
monkeypatch.setattr(
|
||||
"headroom.install.supervisors.subprocess.run",
|
||||
lambda command, **kwargs: _LaunchctlResult(
|
||||
9, stderr="Boot-out failed: 9: Operation not permitted"
|
||||
),
|
||||
)
|
||||
|
||||
with pytest.raises(click.ClickException, match="bootout failed"):
|
||||
stop_supervisor(_manifest(supervisor=SupervisorKind.SERVICE.value))
|
||||
|
||||
|
||||
def test_remove_supervisor_removes_user_crontab_block(monkeypatch) -> None:
|
||||
calls: list[tuple[list[str], str | None]] = []
|
||||
monkeypatch.setattr("headroom.install.supervisors.sys.platform", "linux")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue