Skip to content

Commit 84509a4

Browse files
Parideboyclaude
andauthored
fix: detect and clear stale ANTHROPIC_BASE_URL from crashed wrap sessions (headroomlabs-ai#1768) (headroomlabs-ai#1837)
## Description `headroom wrap claude` writes `env.ANTHROPIC_BASE_URL` (or the foundry/vertex variant) into a project's `.claude/settings.local.json` so daemon-spawned Claude Code workers route through the local Headroom proxy. Removal only happened in the wrap process's `finally:` block. An unclean exit — `SIGKILL`, OOM, reboot, or terminal/tmux close (`SIGHUP`, which was not caught; only `SIGINT`/`SIGTERM` were) — skipped that cleanup, so the entry persisted indefinitely. Every subsequent bare `claude` in that project then routed to the dead port and hung indefinitely retrying it. Closes headroomlabs-ai#1768 ## 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 - `_write_claude_wrap_base_url` now optionally stamps a sidecar marker (`.claude/.headroom_wrap_marker.json`) recording the writer's pid/identity, the port, and the true prior value — kept out of `settings.local.json` itself so Headroom bookkeeping never shows up as a stray key in a file Claude Code's own config loader parses. - A shared `_identity_mismatch` helper (factored out of the existing `_marker_pid_reused` proxy-client-refcounting logic) lets a marker be judged stale: missing/invalid pid, dead pid, or a live pid whose identity doesn't match the recorded one (PID reuse after a crash). - `claude()` now checks for — and self-heals — a stale marker immediately before writing a fresh entry, restoring the recorded prior value instead of trusting a leftover from a dead session. - `claude()` now also registers a `SIGHUP` handler (guarded via `hasattr`, since Windows has none) alongside the existing `SIGTERM` handler, so terminal-close triggers the same cleanup/restore path. - `headroom unwrap claude` now reads the marker's recorded prior value before restoring, instead of unconditionally deleting the key — so a user's own pre-existing `ANTHROPIC_BASE_URL` (set before ever running `wrap`) isn't blindly wiped. - `headroom doctor` gained a new check (`check_wrap_marker_staleness`) that flags a stale project-local marker and points at `headroom unwrap claude` to clean it up — separate from the existing global-settings `check_claude_routing` check. - (Unrelated, pre-existing on `main`) reformatted `headroom/proxy/handlers/openai.py`, `tests/test_openai_codex_ws_lifecycle.py`, `tests/test_output_shaper.py` — whitespace/indentation only, no logic change — since they were already failing `ruff format --check .` on `main` before this branch touched anything, and the repo-wide lint gate blocks on it. Out of scope: `wrap --worktree` — no such flag or multi-worktree `.claude` handling exists anywhere in `wrap.py` today; not adding new surface for an aspirational scenario the issue mentions but that isn't implemented. ## 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_claude_base_url.py tests/test_cli/test_unwrap_claude.py tests/test_cli/test_wrap_stale_marker.py -q 42 passed $ pytest tests/test_cli -q 512 passed, 1 failed (test_wrap_codex_prepare_only_registers_serena_when_uvx_exists — confirmed to fail identically on a clean checkout of main with no changes applied; test-order flake, unrelated to this PR) $ ruff check . All checks passed! $ ruff format --check . 1047 files already formatted $ mypy headroom/cli/wrap.py headroom/cli/doctor.py Success: no issues found in 2 source files ``` ## Real Behavior Proof - Environment: local checkout, Python 3.13, Windows. - Exact command / steps: wrote a base_url entry + marker via `_write_claude_wrap_base_url(..., port=8787)`, then overwrote the marker's recorded pid with a value guaranteed not to be a live process (simulating the crash from the issue's own repro: `headroom wrap claude -- -p ok & ; kill -9 <wrap-pid>`). Ran `headroom.cli.doctor.check_wrap_marker_staleness()` against that path, then called `_check_and_clear_stale_wrap_marker()` (the same check `claude()` now runs before writing a fresh entry). - Observed result: `doctor`'s check correctly reports `WARN` naming the dead pid/port and pointing at `headroom unwrap claude`. The stale-check call then self-heals: in the "nothing existed before wrap" case the leaked entry is removed; in a second run seeded with a real pre-existing `ANTHROPIC_BASE_URL` (set before `wrap` ever ran), that original value is recovered instead of being deleted. In both cases the marker file is cleared afterward. - Not tested: actual OS-level signal delivery (`kill -HUP` against a real running `headroom wrap claude` subprocess) — the SIGHUP registration is exercised via a source-inspection test instead of a live signal, since spawning/killing the real CLI subprocess isn't practical in this environment; verified E2E via CI's `wrap-native` jobs (Ubuntu/macOS) which passed. ## 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/backend fix, no UI surface. ## Additional Notes - Documentation checklist item left unchecked: no user-facing docs currently describe wrap's settings.local.json write/cleanup behavior in enough detail to need updating; happy to add a troubleshooting note if maintainers want one. - `wrap --worktree` handling is out of scope (see Changes Made) — flagging in case maintainers want it tracked as a separate follow-up issue. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 5e29c06 commit 84509a4

6 files changed

Lines changed: 347 additions & 20 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99
## Unreleased
1010

1111
### Fixed
12+
- `headroom wrap claude` no longer leaves a dead `ANTHROPIC_BASE_URL` in a
13+
project's `.claude/settings.local.json` after an unclean exit (`SIGKILL`,
14+
OOM, reboot, or terminal/tmux close via `SIGHUP`, which was not caught).
15+
`_write_claude_wrap_base_url`/`_restore_claude_wrap_base_url` only removed
16+
or restored the entry from the wrap process's own `finally` block, so a
17+
crash skipped it and every later bare `claude` invocation in that project
18+
inherited the stale proxy URL and hung indefinitely retrying a dead port.
19+
A wrap session now stamps a sidecar marker (pid, port, prior value); the
20+
next `wrap`, `unwrap`, or `headroom doctor` run detects a marker whose pid
21+
is dead or reused and restores the recorded prior value automatically.
22+
`claude()` also now catches `SIGHUP` alongside the existing `SIGTERM`
23+
handler ([#1768](https://github.com/headroomlabs-ai/headroom/issues/1768)).
1224
- Non-finite values (`NaN`, `Infinity`) in `proxy_savings.json` or in upstream
1325
cost/token metadata no longer crash the proxy or corrupt the savings
1426
dashboard. `SavingsTracker`'s numeric coercion caught only `TypeError` and

‎headroom/cli/doctor.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
)
3434

3535
from .main import get_version, main
36+
from .wrap import _read_wrap_marker, _wrap_marker_is_stale
3637

3738
PASS = "pass"
3839
WARN = "warn"
@@ -188,6 +189,34 @@ def check_claude_remote_control_gate(
188189
return None
189190

190191

192+
def check_wrap_marker_staleness(settings_path: Path) -> CheckResult:
193+
"""Flag a project-local ANTHROPIC_BASE_URL left by a crashed wrap session.
194+
195+
A crashed ``headroom wrap claude`` (SIGKILL, OOM, reboot) can leave
196+
``.claude/settings.local.json`` pointing at a dead proxy port, hanging
197+
every subsequent bare ``claude`` invocation in the project (issue #1768).
198+
This checks the project-local settings file — separate from the global
199+
``~/.claude/settings.json`` :func:`check_claude_routing` inspects.
200+
"""
201+
name = "wrap_marker"
202+
marker = _read_wrap_marker(settings_path)
203+
if marker is None:
204+
return CheckResult(name=name, status=SKIP, summary="no wrap marker found")
205+
if not _wrap_marker_is_stale(marker):
206+
return CheckResult(
207+
name=name, status=PASS, summary=f"live wrap session (pid {marker.get('pid')})"
208+
)
209+
return CheckResult(
210+
name=name,
211+
status=WARN,
212+
summary=(
213+
f"stale ANTHROPIC_BASE_URL from crashed wrap session "
214+
f"(pid {marker.get('pid')}, port {marker.get('port')}) — "
215+
"run `headroom unwrap claude` to clean it up"
216+
),
217+
)
218+
219+
191220
def check_codex_routing(config_path: Path, port: int) -> CheckResult:
192221
"""Is Codex configured to route through the proxy?
193222
@@ -419,6 +448,7 @@ def doctor(port: int, emit_json: bool) -> None:
419448
check_proxy_liveness(livez, base_url),
420449
check_version_drift(livez, installed),
421450
check_claude_routing(claude_settings_path(), port),
451+
check_wrap_marker_staleness(Path.cwd() / ".claude" / "settings.local.json"),
422452
check_codex_routing(codex_config_path(), port),
423453
check_shell_env(os.environ, port),
424454
check_savings(stats, savings_path()),

‎headroom/cli/wrap.py‎

Lines changed: 143 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -846,12 +846,94 @@ def _claude_wrap_base_url_env_key(*, foundry_mode: bool = False, vertex_mode: bo
846846
return "ANTHROPIC_BASE_URL"
847847

848848

849+
def _wrap_marker_path(settings_path: Path) -> Path:
850+
"""Sidecar marker path for a given settings.local.json path.
851+
852+
Kept out of settings.local.json itself so Headroom's own bookkeeping never
853+
shows up as a stray key inside a file Claude Code's config loader parses.
854+
"""
855+
return settings_path.parent / ".headroom_wrap_marker.json"
856+
857+
858+
def _write_wrap_marker(settings_path: Path, *, port: int, key: str, previous: str | None) -> None:
859+
"""Best-effort record of which (pid, port, key) wrote the base_url entry.
860+
861+
Lets a later wrap/doctor/unwrap invocation tell a stale leftover (writer
862+
process is dead or its PID was recycled) from a still-live wrap session,
863+
and recover the true prior value (issue #1768) instead of guessing.
864+
"""
865+
try:
866+
ident = _proc_identity(os.getpid())
867+
payload = {
868+
"pid": os.getpid(),
869+
"start_src": ident[0] if ident else None,
870+
"start_time": ident[1] if ident else None,
871+
"port": port,
872+
"key": key,
873+
"previous": previous,
874+
}
875+
_write_text(_wrap_marker_path(settings_path), json.dumps(payload))
876+
except OSError:
877+
pass
878+
879+
880+
def _read_wrap_marker(settings_path: Path) -> dict[str, Any] | None:
881+
marker = _wrap_marker_path(settings_path)
882+
try:
883+
rec = json.loads(_read_text(marker))
884+
except (OSError, ValueError):
885+
return None
886+
return rec if isinstance(rec, dict) else None
887+
888+
889+
def _wrap_marker_is_stale(marker: dict[str, Any]) -> bool:
890+
"""True if ``marker`` describes a writer that is provably gone.
891+
892+
Missing/invalid pid, a dead pid, or a live pid whose recorded identity no
893+
longer matches (PID reuse) all count as stale — the entry it describes was
894+
left behind by a wrap session that no longer exists.
895+
"""
896+
pid = marker.get("pid")
897+
if not isinstance(pid, int):
898+
return True
899+
if not _pid_alive(pid):
900+
return True
901+
return _identity_mismatch(marker.get("start_src"), marker.get("start_time"), pid)
902+
903+
904+
def _clear_wrap_marker(settings_path: Path, *, key: str) -> None:
905+
marker = _read_wrap_marker(settings_path)
906+
if marker is not None and marker.get("key") == key:
907+
_wrap_marker_path(settings_path).unlink(missing_ok=True)
908+
909+
910+
def _check_and_clear_stale_wrap_marker(settings_path: Path, *, key: str) -> str | None:
911+
"""If a stale wrap marker for ``key`` exists, restore its recorded prior
912+
value and clear the marker. Returns the restored value, or None if there
913+
was nothing stale to clean up.
914+
915+
Called before writing a fresh base_url entry so a crashed wrap session's
916+
leftover doesn't get treated as this session's own state to restore later.
917+
"""
918+
marker = _read_wrap_marker(settings_path)
919+
if marker is None or marker.get("key") != key or not _wrap_marker_is_stale(marker):
920+
return None
921+
previous = marker.get("previous")
922+
click.echo(
923+
f"headroom: clearing stale {key} left by crashed wrap session (pid {marker.get('pid')})",
924+
err=True,
925+
)
926+
_restore_claude_wrap_base_url(previous, settings_path=settings_path, _key_override=key)
927+
return previous
928+
929+
849930
def _write_claude_wrap_base_url(
850931
proxy_url: str,
851932
*,
852933
foundry_mode: bool = False,
853934
vertex_mode: bool = False,
854935
settings_path: Path | None = None,
936+
port: int | None = None,
855937
) -> str | None:
856938
"""Persist proxy URL into project-local settings env key for daemon child inheritance.
857939
@@ -863,6 +945,10 @@ def _write_claude_wrap_base_url(
863945
initial launch — routes through the Headroom proxy without touching the
864946
global user settings file or affecting sessions in other projects. Returns
865947
the previous value so the caller can restore it on exit (issue #951).
948+
949+
When ``port`` is given, also stamps a sidecar marker recording this
950+
process's identity and the previous value, so a later crash can be
951+
detected and self-healed (issue #1768).
866952
"""
867953
path = settings_path or (Path.cwd() / ".claude" / "settings.local.json")
868954
payload: dict[str, Any] = {}
@@ -880,6 +966,8 @@ def _write_claude_wrap_base_url(
880966
payload["env"] = env_map
881967
path.parent.mkdir(parents=True, exist_ok=True)
882968
_write_text(path, json.dumps(payload, indent=2) + "\n")
969+
if port is not None:
970+
_write_wrap_marker(path, port=port, key=key, previous=previous)
883971
return previous
884972

885973

@@ -889,16 +977,22 @@ def _restore_claude_wrap_base_url(
889977
foundry_mode: bool = False,
890978
vertex_mode: bool = False,
891979
settings_path: Path | None = None,
980+
_key_override: str | None = None,
892981
) -> None:
893982
"""Restore (or remove) the env key written by _write_claude_wrap_base_url.
894983
895984
Called in both the wrap-session finally block and unwrap_claude so the
896985
project-local settings entry is never left pointing at a dead proxy. When
897986
``previous`` is None the key is removed; when it has a value it is
898-
restored — preserving any URL the project already had set.
987+
restored — preserving any URL the project already had set. Also clears
988+
this key's sidecar wrap marker, if any (issue #1768).
899989
"""
900990
path = settings_path or (Path.cwd() / ".claude" / "settings.local.json")
991+
key = _key_override or _claude_wrap_base_url_env_key(
992+
foundry_mode=foundry_mode, vertex_mode=vertex_mode
993+
)
901994
if not path.exists():
995+
_clear_wrap_marker(path, key=key)
902996
return
903997
try:
904998
payload = json.loads(_read_text(path))
@@ -909,9 +1003,9 @@ def _restore_claude_wrap_base_url(
9091003
env_map = payload.get("env")
9101004
if not isinstance(env_map, dict):
9111005
return
912-
key = _claude_wrap_base_url_env_key(foundry_mode=foundry_mode, vertex_mode=vertex_mode)
9131006
if previous is None:
9141007
if key not in env_map:
1008+
_clear_wrap_marker(path, key=key)
9151009
return
9161010
del env_map[key]
9171011
if env_map:
@@ -925,6 +1019,7 @@ def _restore_claude_wrap_base_url(
9251019
_write_text(path, json.dumps(payload, indent=2) + "\n")
9261020
else:
9271021
path.unlink(missing_ok=True)
1022+
_clear_wrap_marker(path, key=key)
9281023

9291024

9301025
def _setup_headroom_mcp(
@@ -3072,26 +3167,33 @@ def _pid_alive(pid: int) -> bool:
30723167
return pid_alive(pid)
30733168

30743169

3170+
def _identity_mismatch(src: Any, recorded: Any, pid: int) -> bool:
3171+
"""True only if ``pid``'s current identity *provably* differs from the
3172+
recorded ``(src, recorded)`` identity (i.e. the PID was recycled).
3173+
3174+
Conservative by design: any uncertainty (unknown/legacy identity, unknown
3175+
start time, mismatched source) returns ``False`` — never claim a mismatch
3176+
without proof, since the caller uses this to decide whether to trust or
3177+
discard state tied to a live PID.
3178+
"""
3179+
if not isinstance(src, str) or not isinstance(recorded, int | float):
3180+
return False # legacy / identity-less record — can't tell
3181+
ident = _proc_identity(pid)
3182+
if ident is None or ident[0] != src:
3183+
return False # can't compare like-for-like — don't claim mismatch
3184+
# Start times are stable per process; >1s apart means a different process.
3185+
return abs(ident[1] - float(recorded)) > 1.0
3186+
3187+
30753188
def _marker_pid_reused(marker: Path, pid: int) -> bool:
30763189
"""True only if the live ``pid`` is *provably* a different process than the
30773190
one that wrote ``marker`` (i.e. the PID was recycled after a crash).
3078-
3079-
Conservative by design: any uncertainty (legacy marker, unknown start time,
3080-
mismatched source) returns ``False`` so a real client is never pruned.
30813191
"""
30823192
try:
30833193
rec = json.loads(_read_text(marker))
30843194
except (OSError, ValueError):
30853195
return False
3086-
src = rec.get("start_src")
3087-
recorded = rec.get("start_time")
3088-
if not isinstance(src, str) or not isinstance(recorded, int | float):
3089-
return False # legacy / identity-less marker — can't tell
3090-
ident = _proc_identity(pid)
3091-
if ident is None or ident[0] != src:
3092-
return False # can't compare like-for-like — don't prune
3093-
# Start times are stable per process; >1s apart means a different process.
3094-
return abs(ident[1] - float(recorded)) > 1.0
3196+
return _identity_mismatch(rec.get("start_src"), rec.get("start_time"), pid)
30953197

30963198

30973199
def _live_proxy_clients(port: int, *, exclude_self: bool = True) -> list[int]:
@@ -3600,6 +3702,10 @@ def claude(
36003702
_register_proxy_client(port)
36013703
signal.signal(signal.SIGINT, _ignore_child_sigint)
36023704
signal.signal(signal.SIGTERM, cleanup)
3705+
if hasattr(signal, "SIGHUP"):
3706+
# Terminal close / tmux kill-session sends SIGHUP, not SIGTERM — without
3707+
# this, the finally block's base_url restore never runs (issue #1768).
3708+
signal.signal(signal.SIGHUP, cleanup)
36033709

36043710
# Memory sync BEFORE proxy startup — sync headroom DB ↔ Claude's files
36053711
if memory:
@@ -3764,6 +3870,13 @@ def claude(
37643870
# daemon's environment) also route through Headroom.
37653871
_settings_vertex[0] = bool(use_vertex)
37663872
_settings_foundry[0] = bool(foundry_upstream) and not _settings_vertex[0]
3873+
_wrap_settings_path = Path.cwd() / ".claude" / "settings.local.json"
3874+
_check_and_clear_stale_wrap_marker(
3875+
_wrap_settings_path,
3876+
key=_claude_wrap_base_url_env_key(
3877+
foundry_mode=_settings_foundry[0], vertex_mode=_settings_vertex[0]
3878+
),
3879+
)
37673880
_saved_base_url[0] = _write_claude_wrap_base_url(
37683881
(
37693882
_foundry_proxy_url(proxy_url)
@@ -3774,6 +3887,8 @@ def claude(
37743887
),
37753888
foundry_mode=_settings_foundry[0],
37763889
vertex_mode=_settings_vertex[0],
3890+
settings_path=_wrap_settings_path,
3891+
port=port,
37773892
)
37783893

37793894
# Per-project savings attribution: tag every request with the launch
@@ -3817,6 +3932,7 @@ def claude(
38173932
_saved_base_url[0],
38183933
foundry_mode=_settings_foundry[0],
38193934
vertex_mode=_settings_vertex[0],
3935+
settings_path=_wrap_settings_path,
38203936
)
38213937
cleanup()
38223938

@@ -3884,9 +4000,19 @@ def unwrap_claude(
38844000
else:
38854001
click.echo(" Kept rtk Claude hooks (--keep-rtk).")
38864002

3887-
_restore_claude_wrap_base_url(None)
3888-
_restore_claude_wrap_base_url(None, foundry_mode=True)
3889-
_restore_claude_wrap_base_url(None, vertex_mode=True)
4003+
_unwrap_settings_path = Path.cwd() / ".claude" / "settings.local.json"
4004+
for _foundry, _vertex in ((False, False), (True, False), (False, True)):
4005+
_key = _claude_wrap_base_url_env_key(foundry_mode=_foundry, vertex_mode=_vertex)
4006+
_marker = _read_wrap_marker(_unwrap_settings_path)
4007+
_prior = (
4008+
_marker.get("previous") if _marker is not None and _marker.get("key") == _key else None
4009+
)
4010+
_restore_claude_wrap_base_url(
4011+
_prior,
4012+
foundry_mode=_foundry,
4013+
vertex_mode=_vertex,
4014+
settings_path=_unwrap_settings_path,
4015+
)
38904016

38914017
click.echo()
38924018
click.echo("✓ Claude is no longer durably wrapped by Headroom.")

‎tests/test_cli/test_unwrap_claude.py‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,26 @@ def restore_base_url(previous: str | None, **kwargs: object) -> None:
223223
)
224224

225225
assert result.exit_code == 0, result.output
226+
settings_path = Path.cwd() / ".claude" / "settings.local.json"
226227
assert restore_calls == [
227-
{"previous": None},
228-
{"previous": None, "foundry_mode": True},
229-
{"previous": None, "vertex_mode": True},
228+
{
229+
"previous": None,
230+
"foundry_mode": False,
231+
"vertex_mode": False,
232+
"settings_path": settings_path,
233+
},
234+
{
235+
"previous": None,
236+
"foundry_mode": True,
237+
"vertex_mode": False,
238+
"settings_path": settings_path,
239+
},
240+
{
241+
"previous": None,
242+
"foundry_mode": False,
243+
"vertex_mode": True,
244+
"settings_path": settings_path,
245+
},
230246
]
231247

232248

0 commit comments

Comments
 (0)