2 commits

Author	SHA1	Message	Date
guyoron1	f42ce4a239	fix: harden fd lifecycle and SystemError handling in runtime and proxy kill (#1556) ... ## Description Make `pid_alive()` safe on Windows even when `psutil` is not installed, and harden `_kill_proxy_by_pid` exception handling for stale PIDs. ### Problem `headroom._subprocess.pid_alive()` falls back to `os.kill(pid, 0)` when `psutil` cannot be imported. On Windows, CPython routes `os.kill(pid, 0)` through `TerminateProcess` — a destructive call that **kills the target process**. Since `psutil` is not a declared runtime dependency in `pyproject.toml`, a normal lightweight install can hit that fallback, meaning `runtime_status()` can silently terminate a live proxy. ### Fix - **`headroom/_subprocess.py`**: On `win32`, bypass `os.kill` entirely and probe via `kernel32.OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION)`. If `ctypes` also fails, return `True` conservatively (assume alive) to prevent false-negative liveness from causing callers to kill a running process. - **`headroom/cli/wrap.py`**: Widen `_kill_proxy_by_pid` exception handlers on both SIGTERM and SIGKILL paths to catch `OSError` and `SystemError` (Windows `WinError 87`), preventing crashes from stale/invalid PIDs. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom/_subprocess.py`) ### New Tests - `test_pid_alive_win32_no_psutil_never_calls_os_kill` — simulates `win32` + broken `psutil`, asserts `os.kill` is never called and the `kernel32.OpenProcess` path is used instead - `test_pid_alive_win32_no_psutil_no_ctypes_returns_conservative` — simulates `win32` + broken `psutil` + broken `ctypes`, asserts `os.kill` is never called and `True` is returned conservatively	2026-07-16 13:51:03 -07:00
Parideboy	6b227b9c90	fix(install): use Windows-safe PID liveness probe in runtime_status (#1544) (#1560) ... ## Description `headroom install status` crashed with `OSError: [WinError 87] The parameter is incorrect` on Windows and, worse, tore down the live proxy it was only meant to inspect. `runtime_status()` probed liveness with a bare `os.kill(pid, 0)` guarded only by `except OSError`. Against a detached Windows agent (`DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP`), that call raises WinError 87, which CPython surfaces as a `SystemError` — not an `OSError` — so it escaped the handler, crashed status, and left the deployment dead (PID file removed, port 8787 freed). This mirrors the `os.kill`/`SystemError` fix PR #1315 applied to `cli/wrap.py`. Closes #1544 ## 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 - Added a shared Windows-safe `headroom._subprocess.pid_alive()` helper: rejects non-positive PIDs, prefers `psutil.pid_exists()`, and treats `SystemError` (WinError 87) as "not alive". - `install/runtime.py` `runtime_status()` now delegates to `pid_alive()` instead of an unguarded `os.kill(pid, 0)`. - `install/runtime.py` `stop_runtime()` now also catches `SystemError` to avoid the same crash class on shutdown. - `cli/wrap.py` `_pid_alive()` now delegates to the shared helper, so the marker-cleanup path and the install/runtime status path share one liveness probe (the shared helper the issue asked for). - Added regression tests for the helper and `runtime_status`. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ ruff check . All checks passed! $ ruff format --check headroom/_subprocess.py headroom/install/runtime.py headroom/cli/wrap.py tests/test_install/test_runtime.py tests/test_pid_alive.py 5 files already formatted $ mypy headroom --ignore-missing-imports (exit 0) $ pytest tests/test_pid_alive.py tests/test_install/test_runtime.py tests/test_cli/test_wrap_helpers.py tests/test_cli/test_wrap_persistent.py \ --deselect "tests/test_install/test_runtime.py::test_runtime_start_lock_blocks_another_process" 89 passed ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.13, ruff 0.15.17 / mypy 1.20.2 / pytest 9.1.0, psutil 7.2.2, branch `fix/1544-windows-pid-liveness`. - Exact command / steps: ran the four checks above; the new `tests/test_pid_alive.py` injects a `SystemError` (simulated WinError 87) and a stubbed `psutil` to drive both code paths, and `test_runtime_status_*` exercise `runtime_status()` end to end with a PID file present. - Observed result: `runtime_status` returns `"running"` for a live PID without sending any signal (asserted), returns `"stopped"` instead of crashing when the probe raises `SystemError`, and the helper only ever passes signal `0`. All 89 targeted tests pass; ruff/format/mypy clean. - Not tested: the full `headroom install apply --preset persistent-task` detached-agent reproduction against a live proxy was not run end to end; it is instead covered by the deterministic `SystemError`/WinError-87 injection regression tests. ## 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 - [ ] I have updated the CHANGELOG.md if applicable ## Additional Notes - One pre-existing test, `test_runtime_start_lock_blocks_another_process`, fails on my local Windows checkout **before** these changes too (it asserts cross-process file-lock blocking and depends on `HOME` semantics that differ on Windows). It is unrelated to this fix and is deselected above; it passes on the Linux CI runners. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-01 17:13:11 -05:00