fix(wrap): stop concurrent wrap sessions clobbering settings.local.json (#3232)

## Description

Several `headroom wrap` sessions in one project each write the proxy URL
into
`.claude/settings.local.json` and restore it on exit. That
read-modify-write was
unsynchronised. The write itself is atomic so the file never tears, but
the
updates were still lost against each other:

- **Live sessions were silently unrouted.** The first session to exit
deleted the
key while its siblings were still running. They kept working, but their
traffic
  stopped going through the proxy — no error, no warning, no savings.
- **A dead proxy was written back into the project.** A session that
started
second captured the *first* session's proxy URL as "the original", so
its exit
restored a URL pointing at a port that was already gone. Every later
session in
  that project then failed to connect.
- **SIGTERM/SIGHUP never ran the restore at all.** `cleanup` was
registered as the
handler, but a Python signal handler that returns normally does not
unwind the
stack — under PEP 475 the interrupted `waitpid` is simply retried. The
`finally`
block that restores `settings.local.json` never ran, while the handler
had
  already terminated the proxy underneath a child that was still alive.

Closes #3205

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)

## Changes Made

- **`_wrap_settings_lock`** — an exclusive OS lock (flock /
`msvcrt.locking`) held
across the settings read-modify-write. A workspace that cannot hold lock
state
  degrades to the previous behaviour rather than failing, matching
  `_proxy_start_lock`.
- **`.headroom_wrap_owners.json`** — a sidecar recording, per env key,
the true
pre-wrap `original` plus the live sessions holding it. The first writer
records
the original; later writers inherit it and are flagged `inherited`, so
no
session restores a value it did not observe first-hand. A session exits
without
restoring while a sibling still holds the key. Dead holders are pruned
with the
same conservative PID+identity liveness the proxy-client markers use, so
a
  SIGKILLed session cannot wedge the key.
- **`unwrap` passes `force=True`** — unwrap is the user explicitly
asking for
their settings back, so it drops every claim instead of deferring to a
live
sibling and silently printing success while leaving the proxy URL in the
file.
- **The #2221 self-heal passes `dead_ports`** — a wrapper process can
outlive its
proxy (proxy alone SIGKILLed). Its claim would otherwise veto the
self-heal and
  leave `ANTHROPIC_BASE_URL` pointing at a port just proven dead.
- **`_rehome_wrap_marker`** — the wrap marker has one slot, won by the
last
writer. When that writer exits while a sibling still owns the key, the
marker is
rewritten to describe the survivor (carrying the record's true
original), so the
survivor keeps its #2221 self-heal record instead of being left with a
marker
  describing a dead process.
- **`_exit_on_signal`** replaces `cleanup` as the SIGTERM/SIGHUP
handler. Raising
`SystemExit` unwinds, so the settings restore actually runs and cleanup
happens
  exactly once from `finally`.
- **`_proxy_start_lock` now shares `_locked_file`** with the new
settings lock
  rather than carrying a second verbatim copy of the platform branches.

## Testing

- [x] Unit tests pass (`pytest`) — full suite, 11518 passed / 588
skipped
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [x] Manual testing performed

`tests/test_wrap_concurrent_settings.py` (14 tests) covers: a sibling
exit leaving
survivors routed, the last session out restoring the true original, a
pre-existing
user URL surviving the whole cycle, three sessions in every exit order,
a crashed
session not wedging the key, forced unwrap past a live session, a holder
that
outlived its proxy not vetoing the self-heal, marker rehoming, and the
signal-handler unwind.

### Test Output

```text
$ uv run pytest tests/test_cli/test_unwrap_claude.py tests/test_cli/test_wrap_claude_base_url.py \
    tests/test_cli/test_wrap_claude_finally_unbound.py tests/test_cli/test_wrap_claude_vertex_proxy_env.py \
    tests/test_cli/test_wrap_claude.py tests/test_cli/test_wrap_dead_marker_selfheal.py \
    tests/test_cli/test_wrap_helpers.py tests/test_cli/test_wrap_stale_marker.py \
    tests/test_cli/test_wrap_persistent.py tests/test_wrap_concurrent_settings.py tests/test_cli_doctor.py -q

tests/test_wrap_concurrent_settings.py ..............                    [ 72%]
tests/test_cli_doctor.py ............................................... [ 89%]
...............................                                          [100%]

============================= 285 passed in 3.01s ==============================

$ uv run pytest tests/ -q
======== 11518 passed, 588 skipped, 6036 warnings in 1831.34s (0:30:31) ========

$ uv run ruff check .
All checks passed!

$ uv run mypy headroom
Success: no issues found in 527 source files
```

## Real Behavior Proof

- **Environment:** macOS 15 (Darwin 25.4.0), Python 3.12.13, repo venv,
Claude
  provider path (`ANTHROPIC_BASE_URL` in `.claude/settings.local.json`).
- **Exact command / steps:** a script spawning **two real OS processes**
— no
  mocks, real PIDs, real files — that call the same
`_write_claude_wrap_base_url` / `_restore_claude_wrap_base_url` helpers
`wrap claude` uses. The project starts with a real user gateway already
set.
Session A (port 8787) starts, session B (port 8788) starts 0.7s later, A
exits
while B is still running, then B exits. Run identically on `main` and on
this
  branch.

**Before (on `main`) — both bugs visible:**

```text
start                       : {"ANTHROPIC_BASE_URL": "https://my-gateway.example.com"}
  session port=8787 started, remembers previous='https://my-gateway.example.com'
  session port=8788 started, remembers previous='http://127.0.0.1:8787'
both sessions running       : {"ANTHROPIC_BASE_URL": "http://127.0.0.1:8788"}
  session port=8787 exited
after FIRST session exits   : {"ANTHROPIC_BASE_URL": "https://my-gateway.example.com"}
  session port=8788 exited
after LAST session exits    : {"ANTHROPIC_BASE_URL": "http://127.0.0.1:8787"}
```

Session B is still running, but after A exits the proxy URL is gone from
under it
— B is unrouted with no error. And the final state is
`http://127.0.0.1:8787`: a
dead proxy left permanently in the user's project, with their real
gateway lost.

**After (this branch):**

```text
start                       : {"ANTHROPIC_BASE_URL": "https://my-gateway.example.com"}
  session port=8787 started, remembers previous='https://my-gateway.example.com'
  session port=8788 started, remembers previous='http://127.0.0.1:8787'
both sessions running       : {"ANTHROPIC_BASE_URL": "http://127.0.0.1:8788"}
  session port=8787 exited
after FIRST session exits   : {"ANTHROPIC_BASE_URL": "http://127.0.0.1:8788"}
  session port=8788 exited
after LAST session exits    : {"ANTHROPIC_BASE_URL": "https://my-gateway.example.com"}
```

B stays routed after A exits, and the last session out restores the
user's real
gateway.

- **Observed result:** matches the intent on both counts — no unrouting,
no dead
  proxy residue, user's pre-existing URL preserved.
- **Not tested:** Windows (`msvcrt.locking`) — the lock and dead-holder
pruning
  are exercised on POSIX only; the Windows branch is the same code path
`_proxy_start_lock` has shipped with. No live end-to-end run against a
real
Anthropic endpoint with two concurrent `claude` CLIs; the proof above
drives the
same helpers out of two real processes instead. Foundry/Vertex key
variants are
covered by unit tests, not by a live run. Real SIGTERM/SIGHUP delivery
to a
running `wrap claude` was not exercised end to end — the handler's
unwind is
covered by a unit test, and full signal delivery would need a spawned
and
  killed subprocess, which the existing #1768 test also declined to do.

## Runtime Rollout Safety

- **Rollout-managed feature(s):** none — this is an unconditional
correctness fix
  on the wrap settings path.
- **Minimum rollout channel:** n/a.
- **Stable/default behavior changed:** yes, three ways. (1) A wrap
session exiting
while a sibling holds the key now leaves the key in place instead of
removing
  it. (2) SIGTERM/SIGHUP now unwinds, so the child CLI is terminated by
  `subprocess.run`'s cleanup rather than being left running against a
  torn-down proxy. (3) Two new sidecar files appear next to
`settings.local.json`: `.headroom_wrap_owners.json` (removed when the
last
holder exits) and `.headroom_wrap_settings.lock` (retained by design —
deleting
  a live lock file creates an inode-replacement race).
- **Kill switch / disable path:** none. A workspace where the lock file
cannot be
created degrades to the previous unsynchronised behaviour automatically.
- **Unsafe override required:** no.
- **Qualification impact:** none beyond the wrap settings path.
- **Rollback path:** revert the commit; the sidecar files are ignored by
older
  versions and can be deleted safely.

## 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
- [x] 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 did **not** edit `CHANGELOG.md`

## Additional Notes

- The ownership record is keyed per env key, so `ANTHROPIC_BASE_URL`,
the
Foundry/Vertex variants and the tool-search entry are tracked
independently.
- Documentation: the behaviour is documented in the helper docstrings
rather than
user-facing docs — the sidecar files are internal state a user never
configures.
- Follow-up worth considering: `.headroom_wrap_settings.lock` is
intentionally
never deleted (matching `_proxy_start_lock`'s retention rationale), so
it stays
in `.claude/` after `unwrap`. Removing it safely needs a separate think
about
  the inode-replacement race.

Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tejas Chopra 2026-08-23 22:36:17 -07:00 committed by GitHub
parent 701e4616d9
commit f27f235032
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 660 additions and 79 deletions

View file

@ -36,7 +36,7 @@ from collections.abc import Callable
from contextlib import contextmanager
from functools import wraps
from pathlib import Path
from typing import Any, cast
from typing import Any, NamedTuple, cast
from headroom._subprocess import pid_alive, run
@ -1192,6 +1192,235 @@ def _wrap_marker_path(settings_path: Path) -> Path:
return settings_path.parent / ".headroom_wrap_marker.json"
def _wrap_owners_path(settings_path: Path) -> Path:
"""Sidecar recording which live wrap sessions own each settings env key.
Separate from ``.headroom_wrap_marker.json`` on purpose: that marker
describes a single writer and is consumed by doctor, unwrap and the
staleness checks. Concurrency ownership is additive state, so it lives in
its own file rather than changing a shape those readers depend on.
"""
return settings_path.parent / ".headroom_wrap_owners.json"
def _wrap_settings_lock(settings_path: Path) -> Any:
"""Serialize settings read-modify-write across concurrent wrap sessions.
Writing the proxy URL into ``settings.local.json`` is a read-modify-write,
and several ``headroom wrap`` sessions in one project run it concurrently.
The write itself is atomic, so the file never tears -- but without this the
updates are still lost against each other (#3205).
"""
from contextlib import nullcontext
lock_path = settings_path.parent / ".headroom_wrap_settings.lock"
try:
lock_path.parent.mkdir(parents=True, exist_ok=True)
lock_file = open(lock_path, "a+b") # noqa: SIM115
except OSError:
# Matches _proxy_start_lock: a workspace that cannot hold lock state is
# degraded, not unusable.
return nullcontext()
return _locked_file(lock_file)
@contextmanager
def _locked_file(lock_file: Any) -> Any:
"""Hold an exclusive OS lock on an already-open file for the block.
Shared by ``_proxy_start_lock`` and ``_wrap_settings_lock`` -- the two
differ only in which file they lock, and an OS-lock dance duplicated per
call site is one place for the platform branches to drift apart.
"""
with lock_file:
if sys.platform == "win32":
import msvcrt
# msvcrt.locking operates on bytes from the current file position.
lock_file.seek(0)
if lock_file.read(1) == b"":
lock_file.seek(0)
lock_file.write(b"0")
lock_file.flush()
lock_file.seek(0)
# LK_LOCK has implementation-dependent retry limits, and a holder
# may legitimately take longer than that (a proxy loading ML
# components), so use the non-blocking primitive in a loop.
while True:
try:
msvcrt.locking(lock_file.fileno(), msvcrt.LK_NBLCK, 1)
break
except OSError:
time.sleep(0.05)
try:
yield
finally:
lock_file.seek(0)
msvcrt.locking(lock_file.fileno(), msvcrt.LK_UNLCK, 1)
else:
import fcntl
fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX)
try:
yield
finally:
fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)
def _read_wrap_owners(settings_path: Path) -> dict[str, Any]:
try:
rec = json.loads(_read_text(_wrap_owners_path(settings_path)))
except (OSError, ValueError):
return {}
return rec if isinstance(rec, dict) else {}
def _write_wrap_owners(settings_path: Path, owners: dict[str, Any]) -> None:
target = _wrap_owners_path(settings_path)
try:
if not owners:
target.unlink(missing_ok=True)
return
_write_text(target, json.dumps(owners, indent=2) + "\n")
except OSError:
pass
def _live_holders(entry: Any, *, dead_ports: frozenset[int] = frozenset()) -> list[dict[str, Any]]:
"""Holders in *entry* whose process is still provably alive.
Reuses the same conservative liveness the proxy-client markers use: a PID
that is gone, or that is now provably a different process, is dropped. Any
uncertainty keeps the holder, because dropping a live owner is what causes
a running session to be unrouted.
``dead_ports`` additionally drops holders whose proxy port the caller has
*proven* dead. A wrapper process outlives its proxy after a hard reboot or
SIGKILL of the proxy alone, and such a holder routes nothing; left in place
it would block the #2221 self-heal from clearing a base_url that now points
at nothing.
"""
if not isinstance(entry, dict):
return []
holders = entry.get("holders")
if not isinstance(holders, list):
return []
live: list[dict[str, Any]] = []
for holder in holders:
if not isinstance(holder, dict):
continue
pid = holder.get("pid")
if not isinstance(pid, int) or not _pid_alive(pid):
continue
if _identity_mismatch(holder.get("start_src"), holder.get("start_time"), pid):
continue
port = holder.get("port")
if isinstance(port, int) and port in dead_ports:
continue
live.append(holder)
return live
def _self_holder(port: int | None) -> dict[str, Any]:
ident = _proc_identity(os.getpid())
return {
"pid": os.getpid(),
"start_src": ident[0] if ident else None,
"start_time": ident[1] if ident else None,
"port": port,
}
def _claim_wrap_key(
settings_path: Path,
key: str,
current_value: str | None,
*,
port: int | None = None,
) -> None:
"""Register this process as an owner of *key*, recording the true original.
The first live owner records ``original``; later owners inherit it and are
flagged ``inherited`` so their exit knows the value they happened to
observe was not the pre-wrap one. Without that, a second wrap session
captures the *first session's* proxy URL as the value to restore, and puts
a dead proxy back into the file on exit (#3205).
"""
owners = _read_wrap_owners(settings_path)
entry = owners.get(key)
live = _live_holders(entry)
inherited = bool(live) and isinstance(entry, dict) and "original" in entry
original = entry.get("original") if inherited and isinstance(entry, dict) else current_value
me = _self_holder(port)
me["inherited"] = inherited
live = [h for h in live if h.get("pid") != me["pid"]]
live.append(me)
owners[key] = {"original": original, "holders": live}
_write_wrap_owners(settings_path, owners)
class _KeyRelease(NamedTuple):
"""Outcome of dropping this process's claim on a settings env key."""
should_restore: bool
original: str | None
trust_caller: bool
survivor: dict[str, Any] | None
def _release_wrap_key(
settings_path: Path,
key: str,
*,
force: bool = False,
dead_ports: frozenset[int] = frozenset(),
) -> _KeyRelease:
"""Drop this process's claim on *key*.
``should_restore`` is False while another live wrap session still owns the
key -- restoring then silently unroutes a running session. ``force`` is for
``unwrap``, where the user is explicitly asking for their settings back:
every claim is dropped and the restore happens regardless.
``trust_caller`` says whether the caller's remembered ``previous`` is its
own first-hand observation of the pre-wrap value. True when there is no
owner record at all (unwrap of a pre-upgrade session, and the legacy
callers that pass the value directly), and when this process founded the
record. False for an inheriting holder -- it remembers the *first
session's* proxy URL, so honouring it writes a dead proxy back, the exact
bug #3205 is about -- and false for a caller with no claim of its own,
whose marker-derived value is second-hand where the record is not.
``survivor`` is a still-live holder the caller can re-point the
single-slot wrap marker at, so an exiting session does not take the
surviving one's #2221 self-heal record with it.
"""
owners = _read_wrap_owners(settings_path)
entry = owners.get(key)
if not isinstance(entry, dict):
return _KeyRelease(True, None, True, None)
me = os.getpid()
remaining = [h for h in _live_holders(entry, dead_ports=dead_ports) if h.get("pid") != me]
original = entry.get("original")
# Look this process's own claim up in the raw holder list, never the
# liveness-filtered one: the caller is by definition running, and its claim
# is what says whether the value it remembers is first-hand.
raw = entry.get("holders")
mine = (
next((h for h in raw if isinstance(h, dict) and h.get("pid") == me), None)
if isinstance(raw, list)
else None
)
trust_caller = mine is not None and not mine.get("inherited")
if remaining and not force:
owners[key] = {"original": original, "holders": remaining}
_write_wrap_owners(settings_path, owners)
return _KeyRelease(False, original, trust_caller, remaining[0])
owners.pop(key, None)
_write_wrap_owners(settings_path, owners)
return _KeyRelease(True, original, trust_caller, None)
def _write_wrap_marker(settings_path: Path, *, port: int, key: str, previous: str | None) -> None:
"""Best-effort record of which (pid, port, key) wrote the base_url entry.
@ -1214,6 +1443,53 @@ def _write_wrap_marker(settings_path: Path, *, port: int, key: str, previous: st
pass
def _rehome_wrap_marker(
settings_path: Path,
*,
key: str,
survivor: dict[str, Any] | None,
original: str | None,
) -> None:
"""Hand this session's wrap marker to a session that is still running.
The marker has one slot and the last writer wins it. When that writer exits
while a sibling still owns the key, leaving the marker describes a dead
process, and deleting it strips the survivor of the #2221 dead-proxy
self-heal record. Rewrite it to describe the survivor instead, carrying the
owner record's ``original`` as the value to restore -- the marker's own
``previous`` may be an earlier session's proxy URL (#3205).
Only ever touches a marker this process wrote; a sibling's marker is
already accurate.
"""
marker_path = _wrap_marker_path(settings_path)
marker = _read_wrap_marker(settings_path)
if marker is None or marker.get("key") != key or marker.get("pid") != os.getpid():
return
port = survivor.get("port") if survivor is not None else None
try:
if survivor is None or not isinstance(port, int):
# No survivor to hand it to, or one whose port we never recorded:
# a marker without a usable port is worse than none.
marker_path.unlink(missing_ok=True)
return
_write_text(
marker_path,
json.dumps(
{
"pid": survivor.get("pid"),
"start_src": survivor.get("start_src"),
"start_time": survivor.get("start_time"),
"port": port,
"key": key,
"previous": original,
}
),
)
except OSError:
pass
def _read_wrap_marker(settings_path: Path) -> dict[str, Any] | None:
marker = _wrap_marker_path(settings_path)
try:
@ -1337,7 +1613,15 @@ def _check_and_clear_dead_wrap_marker(settings_path: Path, *, key: str) -> str |
f"running (issue #2221); restoring prior value",
err=True,
)
_restore_claude_wrap_base_url(previous, settings_path=settings_path, _key_override=key)
_restore_claude_wrap_base_url(
previous,
settings_path=settings_path,
_key_override=key,
# The wrapper process can outlive its proxy (the proxy alone was
# SIGKILLed). Its ownership claim would otherwise veto this restore and
# leave the base_url pointing at a port proven dead just above (#3205).
dead_ports=frozenset({port}) if isinstance(port, int) else frozenset(),
)
return previous
@ -1503,16 +1787,21 @@ def _write_claude_wrap_base_url(
detected and self-healed (issue #1768).
"""
path = settings_path or (Path.cwd() / ".claude" / "settings.local.json")
payload = _read_settings_for_write(path)
env_map = dict(payload.get("env") or {}) if isinstance(payload.get("env"), dict) else {}
key = _claude_wrap_base_url_env_key(foundry_mode=foundry_mode, vertex_mode=vertex_mode)
previous = env_map.get(key)
env_map[key] = proxy_url
payload["env"] = env_map
path.parent.mkdir(parents=True, exist_ok=True)
_write_text(path, json.dumps(payload, indent=2) + "\n")
if port is not None:
_write_wrap_marker(path, port=port, key=key, previous=previous)
with _wrap_settings_lock(path):
payload = _read_settings_for_write(path)
env_map = dict(payload.get("env") or {}) if isinstance(payload.get("env"), dict) else {}
previous = env_map.get(key)
# Claim before writing, so the recorded original is the value that was
# there before *any* wrap session touched it -- not the previous
# session's proxy URL (#3205).
_claim_wrap_key(path, key, previous, port=port)
env_map[key] = proxy_url
payload["env"] = env_map
_write_text(path, json.dumps(payload, indent=2) + "\n")
if port is not None:
_write_wrap_marker(path, port=port, key=key, previous=previous)
return previous
@ -1525,13 +1814,15 @@ def _write_claude_wrap_tool_search(value: str, *, settings_path: Path | None = N
process, and is restored transactionally when the wrap session exits.
"""
path = settings_path or (Path.cwd() / ".claude" / "settings.local.json")
payload = _read_settings_for_write(path)
env_map = dict(payload.get("env") or {}) if isinstance(payload.get("env"), dict) else {}
previous = env_map.get(_TOOL_SEARCH_ENV)
env_map[_TOOL_SEARCH_ENV] = value
payload["env"] = env_map
path.parent.mkdir(parents=True, exist_ok=True)
_write_text(path, json.dumps(payload, indent=2) + "\n")
with _wrap_settings_lock(path):
payload = _read_settings_for_write(path)
env_map = dict(payload.get("env") or {}) if isinstance(payload.get("env"), dict) else {}
previous = env_map.get(_TOOL_SEARCH_ENV)
_claim_wrap_key(path, _TOOL_SEARCH_ENV, previous)
env_map[_TOOL_SEARCH_ENV] = value
payload["env"] = env_map
_write_text(path, json.dumps(payload, indent=2) + "\n")
return previous
@ -1553,6 +1844,8 @@ def _restore_claude_wrap_base_url(
vertex_mode: bool = False,
settings_path: Path | None = None,
_key_override: str | None = None,
force: bool = False,
dead_ports: frozenset[int] = frozenset(),
) -> None:
"""Restore (or remove) the env key written by _write_claude_wrap_base_url.
@ -1561,40 +1854,63 @@ def _restore_claude_wrap_base_url(
``previous`` is None the key is removed; when it has a value it is
restored preserving any URL the project already had set. Also clears
this key's sidecar wrap marker, if any (issue #1768).
Concurrency (#3205): while another live wrap session still owns the key,
this is a no-op restoring underneath a running session unroutes it. Set
``force`` when the user has explicitly asked for their settings back
(``unwrap``), and ``dead_ports`` to name proxy ports already proven dead so
holders that outlived their proxy stop counting as live.
"""
path = settings_path or (Path.cwd() / ".claude" / "settings.local.json")
key = _key_override or _claude_wrap_base_url_env_key(
foundry_mode=foundry_mode, vertex_mode=vertex_mode
)
if not path.exists():
_clear_wrap_marker(path, key=key)
return
try:
payload = json.loads(_read_text(path))
except (OSError, json.JSONDecodeError):
return
if not isinstance(payload, dict):
return
env_map = payload.get("env")
if not isinstance(env_map, dict):
return
if previous is None:
if key not in env_map:
with _wrap_settings_lock(path):
# Another live wrap session in this project may still be using the key.
# Restoring underneath it silently unroutes a running session -- traffic
# bypasses the proxy with no error anywhere (#3205).
release = _release_wrap_key(path, key, force=force, dead_ports=dead_ports)
if not release.should_restore:
# The value stays, but this session's marker must not linger
# describing a process that is gone: hand the slot to a survivor.
_rehome_wrap_marker(path, key=key, survivor=release.survivor, original=release.original)
return
# The owner record holds the value from before *any* wrap session wrote.
# Prefer the caller's own value only when the caller observed it
# first-hand; a session that started second remembers the first
# session's (now dead) proxy URL, and so does the marker an unwrap or a
# self-heal reads it from.
restore_to = previous if release.trust_caller else release.original
if not path.exists():
_clear_wrap_marker(path, key=key)
return
del env_map[key]
if env_map:
payload["env"] = env_map
try:
payload = json.loads(_read_text(path))
except (OSError, json.JSONDecodeError):
return
if not isinstance(payload, dict):
return
env_map = payload.get("env")
if not isinstance(env_map, dict):
return
if restore_to is None:
if key not in env_map:
_clear_wrap_marker(path, key=key)
return
del env_map[key]
if env_map:
payload["env"] = env_map
else:
payload.pop("env", None)
else:
payload.pop("env", None)
else:
env_map[key] = previous
payload["env"] = env_map
if payload:
_write_text(path, json.dumps(payload, indent=2) + "\n")
else:
path.unlink(missing_ok=True)
_clear_wrap_marker(path, key=key)
env_map[key] = restore_to
payload["env"] = env_map
if payload:
_write_text(path, json.dumps(payload, indent=2) + "\n")
else:
path.unlink(missing_ok=True)
_clear_wrap_marker(path, key=key)
def _setup_headroom_mcp(
@ -4104,39 +4420,8 @@ def _proxy_start_lock(port: int) -> Any:
# environment.
yield
return
with lock_file:
if sys.platform == "win32":
import msvcrt
# msvcrt.locking operates on bytes from the current file position.
lock_file.seek(0)
if lock_file.read(1) == b"":
lock_file.seek(0)
lock_file.write(b"0")
lock_file.flush()
lock_file.seek(0)
# LK_LOCK has implementation-dependent retry limits. A proxy may
# legitimately take longer than that to load ML components, so
# use the non-blocking primitive in a loop instead.
while True:
try:
msvcrt.locking(lock_file.fileno(), msvcrt.LK_NBLCK, 1)
break
except OSError:
time.sleep(0.05)
try:
yield
finally:
lock_file.seek(0)
msvcrt.locking(lock_file.fileno(), msvcrt.LK_UNLCK, 1)
else:
import fcntl
fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX)
try:
yield
finally:
fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)
with _locked_file(lock_file):
yield
@wraps(_ensure_proxy_unlocked)
@ -4330,6 +4615,20 @@ def _ignore_child_sigint(signum: int | None = None, frame: Any = None) -> None:
return None
def _exit_on_signal(signum: int | None = None, frame: Any = None) -> None:
"""Unwind on SIGTERM/SIGHUP so the ``finally`` block actually runs.
Registering ``cleanup`` itself as the handler did not achieve what its call
site documented. A Python signal handler that returns normally does not
unwind the stack -- under PEP 475 the interrupted ``waitpid`` is simply
retried -- so the ``finally`` that restores ``settings.local.json`` never
ran, while the handler had already terminated the proxy underneath a child
that was still alive. Raising SystemExit reverses that: the settings are
restored and cleanup runs exactly once, from ``finally`` (#3205).
"""
raise SystemExit(128 + int(signum or 0))
def _launch_tool(
binary: str,
args: tuple,
@ -4361,7 +4660,7 @@ def _launch_tool(
port_holder: list[int] = [port]
cleanup = _make_cleanup(proxy_holder, port_holder)
signal.signal(signal.SIGINT, _ignore_child_sigint)
signal.signal(signal.SIGTERM, cleanup)
signal.signal(signal.SIGTERM, _exit_on_signal)
try:
click.echo()
@ -4833,11 +5132,11 @@ def claude(
)
cleanup = _make_cleanup(proxy_holder, port_holder)
signal.signal(signal.SIGINT, _ignore_child_sigint)
signal.signal(signal.SIGTERM, cleanup)
signal.signal(signal.SIGTERM, _exit_on_signal)
if hasattr(signal, "SIGHUP"):
# Terminal close / tmux kill-session sends SIGHUP, not SIGTERM — without
# this, the finally block's base_url restore never runs (issue #1768).
signal.signal(signal.SIGHUP, cleanup)
signal.signal(signal.SIGHUP, _exit_on_signal)
# Memory sync BEFORE proxy startup — sync headroom DB ↔ Claude's files
if memory:
@ -5222,6 +5521,10 @@ def unwrap_claude(
foundry_mode=_foundry,
vertex_mode=_vertex,
settings_path=_unwrap_settings_path,
# unwrap is the user asking for their settings back, so it drops
# every wrap session's claim rather than deferring to a live
# sibling and silently doing nothing (#3205).
force=True,
)
# Issue #2238: unwrap restores settings.local.json, but a proxy URL that was

View file

@ -243,18 +243,24 @@ def test_unwrap_claude_restores_all_base_url_modes(runner: CliRunner) -> None:
"foundry_mode": False,
"vertex_mode": False,
"settings_path": settings_path,
# unwrap is the user asking for their settings back, so it drops
# every wrap session's ownership claim instead of deferring to a
# live sibling and silently doing nothing (#3205).
"force": True,
},
{
"previous": None,
"foundry_mode": True,
"vertex_mode": False,
"settings_path": settings_path,
"force": True,
},
{
"previous": None,
"foundry_mode": False,
"vertex_mode": True,
"settings_path": settings_path,
"force": True,
},
]

View file

@ -1,8 +1,11 @@
from __future__ import annotations
import json
import signal
from pathlib import Path
import pytest
from headroom.cli import doctor as doctor_cli
from headroom.cli import wrap as wrap_cli
@ -49,4 +52,19 @@ def test_claude_command_registers_sighup_next_to_sigterm() -> None:
src = inspect.getsource(wrap_cli.claude.callback)
assert 'hasattr(signal, "SIGHUP")' in src
assert "signal.signal(signal.SIGHUP, cleanup)" in src
assert "signal.signal(signal.SIGHUP, _exit_on_signal)" in src
assert "signal.signal(signal.SIGTERM, _exit_on_signal)" in src
def test_signal_handler_unwinds_so_the_restore_can_run() -> None:
"""Registering `cleanup` directly never achieved what #1768 wanted.
A Python signal handler that returns normally does not unwind the stack --
under PEP 475 the interrupted `waitpid` is simply retried -- so the finally
block that restores settings.local.json never ran, while the handler had
already torn the proxy down under a live child. The handler must raise.
"""
with pytest.raises(SystemExit) as excinfo:
wrap_cli._exit_on_signal(signal.SIGHUP, None)
assert excinfo.value.code == 128 + signal.SIGHUP

View file

@ -0,0 +1,254 @@
"""Concurrent `headroom wrap` sessions sharing one project's settings (#3205).
`wrap claude` writes ANTHROPIC_BASE_URL into `.claude/settings.local.json` and
restores it on exit. Several sessions in one project run that read-modify-write
concurrently. The write is atomic so the file never tears, but the updates were
still lost against each other:
* the first session's exit deleted the key while the others were still
running -- they silently stopped routing through the proxy, kept working,
and lost every byte of compression with no error anywhere; and
* a session that started second remembered the *first* session's proxy URL as
"the original", so its exit wrote a dead proxy back into the file, which
every later session in that project then failed to connect to.
"""
from __future__ import annotations
import json
from pathlib import Path
from unittest import mock
import pytest
from headroom.cli import wrap as W
@pytest.fixture
def settings(tmp_path: Path) -> Path:
path = tmp_path / ".claude" / "settings.local.json"
path.parent.mkdir(parents=True)
path.write_text(json.dumps({"env": {"FOO": "bar"}}), encoding="utf-8")
return path
def _env(path: Path) -> dict:
return json.loads(path.read_text(encoding="utf-8")).get("env", {}) if path.exists() else {}
class _Sessions:
"""Drive several wrap sessions with distinct, controllable PIDs."""
def __init__(self, *pids: int) -> None:
self.live = set(pids)
def __enter__(self) -> _Sessions:
self._patches = [
mock.patch.object(W, "_pid_alive", lambda pid: pid in self.live),
mock.patch.object(W, "_identity_mismatch", lambda *a: False),
]
for p in self._patches:
p.start()
return self
def __exit__(self, *exc: object) -> None:
for p in self._patches:
p.stop()
def launch(self, pid: int, url: str, path: Path, port: int | None = None) -> str | None:
with mock.patch("os.getpid", lambda: pid):
return W._write_claude_wrap_base_url(url, settings_path=path, port=port)
def exit(self, pid: int, previous: str | None, path: Path) -> None:
self.live.discard(pid)
with mock.patch("os.getpid", lambda: pid):
W._restore_claude_wrap_base_url(previous, settings_path=path)
def crash(self, pid: int) -> None:
"""Vanish without running cleanup (SIGKILL, hard reboot)."""
self.live.discard(pid)
def test_first_session_exiting_leaves_the_others_routed(settings: Path) -> None:
"""The reported symptom: sessions silently stop routing when a sibling exits."""
with _Sessions(1001, 1002) as s:
a = s.launch(1001, "http://127.0.0.1:8787", settings)
s.launch(1002, "http://127.0.0.1:8788", settings)
s.exit(1001, a, settings)
assert "ANTHROPIC_BASE_URL" in _env(settings), "surviving session was unrouted"
def test_last_session_out_restores_the_true_original(settings: Path) -> None:
"""A later session must not restore an earlier session's dead proxy URL."""
with _Sessions(1001, 1002) as s:
a = s.launch(1001, "http://127.0.0.1:8787", settings)
b = s.launch(1002, "http://127.0.0.1:8788", settings)
s.exit(1001, a, settings)
s.exit(1002, b, settings)
assert _env(settings) == {"FOO": "bar"}, "stale proxy URL left behind"
def test_a_pre_existing_user_base_url_survives_the_whole_cycle(settings: Path) -> None:
"""A URL the project already had is restored, not deleted."""
settings.write_text(
json.dumps({"env": {"ANTHROPIC_BASE_URL": "http://user-proxy:1234"}}), encoding="utf-8"
)
with _Sessions(1001, 1002) as s:
a = s.launch(1001, "http://127.0.0.1:8787", settings)
b = s.launch(1002, "http://127.0.0.1:8788", settings)
s.exit(1001, a, settings)
s.exit(1002, b, settings)
assert _env(settings)["ANTHROPIC_BASE_URL"] == "http://user-proxy:1234"
def test_three_sessions_any_exit_order(settings: Path) -> None:
for order in ([1001, 1002, 1003], [1003, 1001, 1002], [1002, 1003, 1001]):
settings.write_text(json.dumps({"env": {"FOO": "bar"}}), encoding="utf-8")
with _Sessions(*order) as s:
prev = {
pid: s.launch(pid, f"http://127.0.0.1:{8787 + i}", settings)
for i, pid in enumerate(order)
}
for pid in order[:-1]:
s.exit(pid, prev[pid], settings)
assert "ANTHROPIC_BASE_URL" in _env(settings), f"unrouted early in {order}"
s.exit(order[-1], prev[order[-1]], settings)
assert _env(settings) == {"FOO": "bar"}, f"residue after {order}"
def test_a_crashed_session_does_not_wedge_the_key(settings: Path) -> None:
"""A SIGKILLed session never releases; its claim must be pruned as dead."""
with _Sessions(1001, 1002) as s:
s.launch(1001, "http://127.0.0.1:8787", settings)
b = s.launch(1002, "http://127.0.0.1:8788", settings)
s.crash(1001)
s.exit(1002, b, settings)
assert _env(settings) == {"FOO": "bar"}
assert not W._wrap_owners_path(settings).exists()
def test_single_session_behaviour_is_unchanged(settings: Path) -> None:
with _Sessions(1001) as s:
a = s.launch(1001, "http://127.0.0.1:8787", settings)
assert _env(settings)["ANTHROPIC_BASE_URL"] == "http://127.0.0.1:8787"
s.exit(1001, a, settings)
assert _env(settings) == {"FOO": "bar"}
def test_restore_without_an_owner_record_still_honours_the_caller(settings: Path) -> None:
"""unwrap and legacy sessions pass the previous value directly."""
settings.write_text(
json.dumps({"env": {"ANTHROPIC_BASE_URL": "http://127.0.0.1:8787"}}), encoding="utf-8"
)
assert not W._wrap_owners_path(settings).exists()
W._restore_claude_wrap_base_url("http://legacy:9999", settings_path=settings)
assert _env(settings)["ANTHROPIC_BASE_URL"] == "http://legacy:9999"
def test_tool_search_key_is_tracked_independently(settings: Path) -> None:
"""Ownership is per key -- the tool-search entry has the same race."""
with _Sessions(1001, 1002) as s:
with mock.patch("os.getpid", lambda: 1001):
a = W._write_claude_wrap_tool_search("auto", settings_path=settings)
with mock.patch("os.getpid", lambda: 1002):
W._write_claude_wrap_tool_search("auto", settings_path=settings)
s.live.discard(1001)
with mock.patch("os.getpid", lambda: 1001):
W._restore_claude_wrap_tool_search(a, settings_path=settings)
assert W._TOOL_SEARCH_ENV in _env(settings), "surviving session lost tool-search"
def test_exit_on_signal_unwinds_so_finally_can_run() -> None:
"""`cleanup` as the handler never unwound; the settings restore never ran."""
with pytest.raises(SystemExit) as excinfo:
W._exit_on_signal(15, None)
assert excinfo.value.code == 143
def test_unwrap_forces_the_restore_past_a_live_session(settings: Path) -> None:
"""`unwrap` is the user asking for their settings back -- it must not no-op.
Deferring to a live sibling is right for a session exiting on its own, but
unwrap deferring means the command prints success while leaving the proxy
URL in the file.
"""
with _Sessions(1001) as s:
s.launch(1001, "http://127.0.0.1:8787", settings)
with mock.patch("os.getpid", lambda: 2002):
W._restore_claude_wrap_base_url(None, settings_path=settings, force=True)
assert _env(settings) == {"FOO": "bar"}, "unwrap left the proxy URL behind"
assert not W._wrap_owners_path(settings).exists(), "unwrap left ownership state behind"
def test_unwrap_restores_the_true_original_not_the_marker_value(settings: Path) -> None:
"""A caller with no claim of its own trusts the record over its marker.
The single-slot marker is won by the *last* writer, whose `previous` is the
first session's proxy URL -- restoring that is the #3205 bug via unwrap.
"""
settings.write_text(
json.dumps({"env": {"ANTHROPIC_BASE_URL": "http://user-proxy:1234"}}), encoding="utf-8"
)
with _Sessions(1001, 1002) as s:
s.launch(1001, "http://127.0.0.1:8787", settings, port=8787)
s.launch(1002, "http://127.0.0.1:8788", settings, port=8788)
with mock.patch("os.getpid", lambda: 2002):
W._restore_claude_wrap_base_url(
"http://127.0.0.1:8787", settings_path=settings, force=True
)
assert _env(settings)["ANTHROPIC_BASE_URL"] == "http://user-proxy:1234"
def test_a_holder_that_outlived_its_proxy_cannot_veto_the_selfheal(settings: Path) -> None:
"""#2221: a wrapper PID can outlive its proxy; its claim must not block."""
with _Sessions(1001) as s:
s.launch(1001, "http://127.0.0.1:8787", settings, port=8787)
# PID 1001 is still alive, but port 8787 has been proven dead.
W._restore_claude_wrap_base_url(None, settings_path=settings, dead_ports=frozenset({8787}))
assert _env(settings) == {"FOO": "bar"}, "dead proxy URL survived the self-heal"
def test_exiting_session_hands_its_marker_to_a_survivor(settings: Path) -> None:
"""The marker has one slot; the leaver must not strand or hijack it."""
with _Sessions(1001, 1002) as s:
s.launch(1001, "http://127.0.0.1:8787", settings, port=8787)
b = s.launch(1002, "http://127.0.0.1:8788", settings, port=8788)
marker = W._read_wrap_marker(settings)
assert marker is not None and marker["pid"] == 1002, "last writer owns the marker"
s.exit(1002, b, settings)
marker = W._read_wrap_marker(settings)
assert marker is not None, "survivor lost its #2221 self-heal record"
assert marker["pid"] == 1001, "marker still describes the exited session"
assert marker["port"] == 8787
assert marker["previous"] is None, "marker must carry the true original"
def test_the_founding_session_still_honours_an_explicit_previous(settings: Path) -> None:
"""A sole writer observed the pre-wrap value first-hand; do not override it."""
with _Sessions(1001) as s:
s.launch(1001, "http://127.0.0.1:8787", settings)
s.exit(1001, "https://existing-gateway.example.com/v1", settings)
assert _env(settings)["ANTHROPIC_BASE_URL"] == "https://existing-gateway.example.com/v1"