mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
9 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Parideboy
|
1edaeb8b76
|
fix(install/windows): register persistent-task from S4U hidden XML (#2453) (#2459)
## Description Windows `persistent-task` created its startup and 5-minute health tasks via `schtasks` command-line flags, which register the task with an **interactive-token** principal. Every task run spawned a visible console window that briefly grabbed keyboard focus before vanishing — every 5 minutes, indefinitely (and at boot / proxy restart). Fixes #2453. This registers the tasks from Task Scheduler **XML** instead: user-scope tasks use an **S4U** principal (run whether the user is logged on or not, no stored password) with `<Hidden>true</Hidden>`, so runs execute in a non-interactive session and never draw a window. System-scope tasks keep the LocalSystem service account (which already has no desktop). ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) ## Changes Made - `headroom/install/supervisors.py`: add `_windows_task_xml()` (S4U/hidden for user scope, LocalSystem for system scope), `_windows_boot_trigger()`, `_windows_health_trigger()` (PT5M repetition), and `_register_windows_task()` (writes UTF-16 XML to a temp file and calls `schtasks /Create /TN <n> /XML <file> /F`). Rewrite the Windows TASK branch of `install_supervisor` to register both tasks from XML. - `tests/test_install/test_supervisors.py`: unit tests asserting the XML carries `S4U` + `Hidden` + `PT5M` for user scope and `S-1-5-18` / `ServiceAccount` for system scope; updated the install-flow assertion to expect `schtasks /XML` registration for the startup and health tasks. ## Testing - [x] Unit tests pass ``` $ python -m pytest tests/test_install/test_supervisors.py -q collected 29 items tests\test_install\test_supervisors.py ............................. [100%] ============================= 29 passed in 1.48s ============================== ``` ## Real Behavior Proof - Environment: Windows 11 Pro 10.0.26200, Python 3.13.11 - Exact command / steps: python -m pytest tests/test_install/test_supervisors.py -q; ruff check + ruff format --check; mypy headroom/install/supervisors.py --ignore-missing-imports - Observed result: 29 passed; ruff clean; mypy exit 0. Generated XML contains <LogonType>S4U</LogonType> and <Hidden>true</Hidden> for user scope. - Not tested: live end-to-end `headroom install apply --preset persistent-task` on a physical desktop confirming zero console flash over a >5-minute window (no interactive Windows session in CI). ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Ingmar Krusch
|
896454e978
|
feat(install): add apply flag parity, --env passthrough, and EIO retry (#2152)
## Description Three related gaps in `headroom install apply` and its supervisor lifecycle, found operating a real persistent deployment on this fork: 1. `install apply` only exposed a fixed subset of `headroom proxy`'s flags (`--backend`, `--region`, `--mode`, `--port`, `--memory`, `--telemetry`, `--no-http2`). Deployments that need code-aware compression, tool-result interception, per-tool lossy-compression protection, or a named AWS profile for Bedrock had no native way to configure them through `install apply` — the generated `manifest.json` would have to be hand-edited after the fact, which silently reverts on the next `install apply` and isn't tracked anywhere. 2. Supervised runners (macOS launchd, Linux systemd/cron, Windows services/tasks) all start their runner scripts with a bare environment and do not inherit the interactive shell's exports. In particular, a custom `HEADROOM_WORKSPACE_DIR` never reached the supervised process, so `headroom install agent run` looked for its manifest in the wrong location and failed outright with "No deployment profile named 'default' is installed" even though `install apply` itself had succeeded moments earlier. 3. `install_supervisor`'s macOS branch does an unconditional `launchctl bootout` followed by a bare `bootstrap` with no retry, unlike `start_supervisor` (already fixed by #1290), which rides out the ~15s EIO (error 5) window launchd exhibits for several seconds after a bootout. This left `install apply`'s own reinstall path exposed to the same race #1290 fixed elsewhere — requiring the exact manual recovery (bootout + remove the plist + reapply) #1290 was meant to eliminate. Closes # ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [x] 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 - `headroom/cli/install.py`: `install apply` gains `--code-aware/--no-code-aware`, `--intercept-tool-results`, `--protect-tool-results <tool1,tool2>`, and `--bedrock-profile <profile>`, mirroring the equivalent flags already on `headroom proxy` (same names, same help text style). Also gains `--env KEY=VALUE` (repeatable). - `headroom/install/planner.py`: `build_manifest()` threads all five new parameters into `proxy_args`/`base_env`, following the exact pattern already used for `--region`/`--no-http2`. `--env` entries are merged into `base_env` last, so they can override auto-derived defaults. - `headroom/install/supervisors.py`: - `_render_unix_runner`/`_render_windows_runner` emit `export`/`$env:` lines for `base_env` before the `exec`, so `run-headroom.sh`/`ensure-headroom.sh` (and Windows equivalents) carry the environment forward to both the outer `install agent run` process and the proxy subprocess it spawns. The Docker runtime path already threaded `base_env` into `docker run --env`; this closes the same gap for the process-based runtime. - New `_bootstrap_with_retry()` helper extracted from `start_supervisor`'s existing retry loop (from #1290), now shared by both `start_supervisor` and `install_supervisor`. - `tests/test_install/test_planner.py`: new tests for all five flags (default-omitted and persisted cases), following the existing `--no-http2` test pattern. - `tests/test_install/test_supervisors.py`: new tests for `--env` propagation into rendered runner scripts, and for `install_supervisor`'s retry-until-success and raise-after-exhausted-retries paths (mirroring the existing `start_supervisor` coverage). Also fixes a pre-existing test's mock that returned `None` from a `subprocess.run` stub — this only worked before because the old bare `bootstrap` call site never inspected the return value; the new `_bootstrap_with_retry()` call does. - `CHANGELOG.md`: added `### Features` and `### Fixed` entries under `Unreleased`. ## 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 $ uv run pytest tests/test_install/ tests/test_cli/test_wrap_persistent.py tests/test_cli/test_init_cli.py -q ============================= test session starts ============================== platform darwin -- Python 3.13.13, pytest-9.0.3, pluggy-1.6.0 collected 215 items tests/test_install/test_health.py ... [ 1%] tests/test_install/test_native_installers.py ss [ 2%] tests/test_install/test_paths.py ... [ 3%] tests/test_install/test_planner.py .................. [ 12%] tests/test_install/test_providers.py ................................... [ 28%] ...... [ 31%] tests/test_install/test_runtime.py .................... [ 40%] tests/test_install/test_state.py ..... [ 42%] tests/test_install/test_supervisors.py ......................... [ 54%] tests/test_cli/test_wrap_persistent.py ............................ [ 67%] tests/test_cli/test_init_cli.py ........................................ [ 86%] .............................. [100%] ======================== 213 passed, 2 skipped in 0.57s ======================== $ uv run ruff check headroom/cli/install.py headroom/install/planner.py headroom/install/supervisors.py tests/test_install/ All checks passed! $ uv run mypy headroom/cli/install.py headroom/install/planner.py headroom/install/supervisors.py Success: no issues found in 3 source files ``` ## Real Behavior Proof - Environment: personal fork deployed as a real proxy (macOS launchd service via `headroom install apply`), profile `default`, backend `bedrock` with a named AWS SSO profile. - Exact command / steps: (flags 1 & 2) ran `headroom install apply --backend bedrock --mode token --code-aware --protect-tool-results Bash --bedrock-profile sso-bedrock --env HEADROOM_WORKSPACE_DIR=/Users/<redacted>/.headroom-workspace --env AWS_PROFILE=sso-bedrock --env AWS_REGION=eu-west-1`, then inspected the generated `manifest.json`, the rendered `run-headroom.sh`, and the running launchd job. - Observed result: before this PR, none of `--code-aware`, `--protect-tool-results`, `--bedrock-profile`, or `--env` were accepted flags on `install apply` at all (`Error: No such option`). Reproduced the `--env` gap specifically by running the exact command a launchd job invokes with a stripped environment (no `HEADROOM_WORKSPACE_DIR`, no `AWS_PROFILE`) — it failed to find the manifest; with the interactive shell's env forwarded manually, it started fine. The generated plist had no `EnvironmentVariables` key and `run-headroom.sh` was a bare `exec`, confirming this wasn't a config mistake but a real gap between `install apply`'s flag surface and what a supervisor actually runs with. After this PR, `install apply` with all the flags above produces a launchd job that starts clean, reports healthy, and successfully proxies a real request to Bedrock (200, not just a green health check) using the named AWS profile with no `AWS_PROFILE` env var needed elsewhere. - Exact command / steps: (EIO retry, flag 3) triggered the same EIO race #1290 documents by running `headroom install apply` twice in quick succession against the same profile (the second run's `install_supervisor` bootout+bootstrap lands inside the first run's launchd settle window). - Observed result: before this PR, the second `install apply` occasionally failed outright with `CalledProcessError` from the bare `subprocess.run(..., check=True)` bootstrap call, requiring the manual bootout+`rm` plist+reapply recovery. After this PR (with `_bootstrap_with_retry` in place), the same back-to-back sequence completes successfully every time observed, riding out the EIO window instead of failing. - Not tested: Linux systemd/cron and Windows service/task supervisor paths for the `--env` propagation — verified via the new unit tests (which cover the runner-script rendering directly) but not against a live Linux or Windows machine, since this deployment is macOS-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/install logic change, no UI surface. ## Additional Notes - "I have made corresponding changes to the documentation" is unchecked: no file in `docs/`, `README.md`, or `CONTRIBUTING.md` documents `install apply`'s flag surface in detail (it's discoverable via `--help`), so there is no existing section to update for the new flags. - Re-derivation note: this PR's `install_supervisor` EIO-retry fix and its `_bootstrap_with_retry` extraction are written directly against current `upstream/main`'s post-#1290 shape of `start_supervisor` (inline retry loop with `_MACOS_BOOTSTRAP_RETRIES`/`_MACOS_BOOTSTRAP_RETRY_DELAY`), not cherry-picked from an older fork commit that predated #1290 — the diff here is intentionally different from what a naive cherry-pick would have produced. - No linked issue number: found via operating a real persistent deployment on a personal fork, not filed as a `headroomlabs-ai/headroom` issue first. Checked `gh pr list --search` for "install apply flags/env" and "bootstrap EIO"/"install_supervisor bootstrap retry" — no open or merged coverage found beyond #1290 (which fixes `start_supervisor` only, a different call site from the one this PR fixes). Co-authored-by: JerrettDavis <mxjerrett@gmail.com> |
||
|
Parideboy
|
d6e0710228
|
fix(install): pass sc.exe create as raw command line so binPath= quoting survives (#1654) (#1702)
## Description
`headroom install apply --preset persistent-service` fails on Windows
with `sc.exe` error 1639 ("invalid start= field"). The service install
built the `sc.exe create` invocation as an argv list whose `binPath=`
token embedded both spaces and inner double quotes (`cmd.exe /c
"…run-headroom.cmd"`). Python's `subprocess.list2cmdline` then wrapped
that whole token in outer quotes, so the command line `sc.exe` actually
received tokenized as `'binPath= cmd.exe /c "…"'` and `'start= auto'` —
single glued tokens — instead of the documented `binPath=` `<value>`
`start=` `<value>` separate-token pairs. `sc.exe` rejects that with
1639.
This PR builds the exact command line as a pre-quoted string and passes
it to `subprocess.run` directly; on Windows a string argument goes
verbatim to `CreateProcess`, bypassing `list2cmdline` entirely. The
`sc.exe failure` / `start` / `stop` / `delete` calls keep the argv-list
form since none of their tokens embed quotes.
Fixes #1654
## Type of Change
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Documentation update
- [ ] Refactoring (no functional changes)
## Changes Made
- `headroom/install/supervisors.py`: the Windows `SERVICE` branch of
`install_supervisor` now builds the `sc.exe create` command as a single
pre-quoted string — `sc.exe create <name> binPath= "cmd.exe /c
\"<run-headroom.cmd>\"" start= auto` — and passes it to `subprocess.run`
as a string instead of an argv list.
- `tests/test_install/test_supervisors.py`: updated the Windows-service
assertion to expect the new command-line string (regression test for
#1654), verifying the backslash-escaped inner quotes and `start= auto`
as a separate trailing pair.
## Testing
- [x] Unit tests pass (`tests/test_install/test_supervisors.py`)
- [x] Lint/type gates pass (`ruff check`, `ruff format --check`, `mypy`)
```
$ python -m pytest tests/test_install/ -q
94 passed, 1 failed, 1 skipped
# the 1 failure is tests/test_install/test_runtime.py::test_runtime_start_lock_blocks_another_process,
# which fails identically on a clean upstream/main checkout on this machine (pre-existing local env flake,
# unrelated to this change)
$ 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 headroom --ignore-missing-imports # exit 0, notes only
```
## Real Behavior Proof
- Environment: Windows 11 Pro 10.0.26200, Python 3.13, local checkout of
this branch.
- Exact command / steps: Tokenized both the old (argv-list →
`list2cmdline`) and new (pre-quoted string) command lines with
`shell32.CommandLineToArgvW` — the same parsing `sc.exe` applies to its
received command line — using the exact path from the issue report. Also
ran the new string form through `subprocess.run` against the real
`sc.exe` (non-elevated).
- Observed result: Old form tokenizes to `['sc.exe', 'create',
'headroom-default', 'binPath= cmd.exe /c
"C:\\Users\\Adron\\...\\run-headroom.cmd"', 'start= auto']` —
`binPath=`/`start=` glued to their values, which `sc.exe` rejects with
1639. New form tokenizes to `['sc.exe', 'create', 'headroom-default',
'binPath=', 'cmd.exe /c "C:\\Users\\Adron\\...\\run-headroom.cmd"',
'start=', 'auto']` — exactly the documented `sc create` token shape.
Running the new string against real `sc.exe` non-elevated proceeds past
argument parsing to `OpenSCManager FAILED 5: Access is denied` (the
expected no-admin outcome per the issue reporter's own non-admin run),
with no 1639 syntax error.
- Not tested: Full elevated end-to-end `headroom install apply --preset
persistent-service` service creation + service start on an Administrator
shell (no elevated session available in this environment); behavior on
non-English locales other than the tokenization-level verification
above.
## Review Readiness
- [x] I have performed a self-review
- [x] This PR is ready for human review
**Follow-up candidate (out of scope here)**: the issue also notes that a
failed install removes `~/.headroom/deploy/<profile>/` artifacts,
hampering post-mortem debugging — worth a separate issue/PR to preserve
or relocate failed-install artifacts.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Grant McNaught
|
da1a3973ed
|
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> |
||
|
Garm
|
efd2ac1ca4 |
chore: renormalize line endings to LF
`.gitattributes` declares `*.py text eol=lf` and `*.sh text eol=lf`, but 74 files (73 .py, 1 .sh) are stored in the index with CRLF line endings, violating that contract. Every macOS/Linux clone reports these files as "modified" on fresh checkout because git's diff engine sees the stored bytes don't match the attribute contract, even though the working tree and index match byte-for-byte. Running `git add --renormalize .` rewrites each affected blob so the stored form matches the attribute declaration. No semantic changes — every affected file's diff is "N insertions, N deletions" with inserts and deletes being the same lines modulo line endings. Follow-up commit adds `.git-blame-ignore-revs` so `git blame` / GitHub blame skip this mechanical commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
JerrettDavis
|
d4574f4ae2 |
test: avoid global platform leaks in install tests
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
JerrettDavis
|
38bf3e639c |
test: expand coverage across helper slices
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
JerrettDavis
|
bd242fc62d |
test: expand persistent install coverage
Add focused regression coverage for install, runtime, provider, state, health, supervisor, and persistent wrap flows so the new persistent deployment surfaces are exercised more thoroughly in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
JerrettDavis
|
b325a06aae |
feat: harden persistent install wrappers
Tighten Docker-native bash and PowerShell wrapper validation for wrap and proxy flows, pin the bash wrapper to the install-time interpreter, clean up failed persistent container starts, and extend docs, CI, e2e, and native installer coverage for persistent Docker installs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |