Commit graph

1 commit

Author SHA1 Message Date
quentinmaisonneuve
6cba4419d0
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 13:49:28 -05:00