mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(wrap): write env.ANTHROPIC_BASE_URL to settings.json so daemon-spawned conversations inherit proxy (#951) (#1078)
## Description Claude Code pre-forks conversation workers via spawn (not fork) on macOS. Those workers read settings files fresh on each new session rather than inheriting the daemon process's environment. `headroom wrap claude` was passing `ANTHROPIC_BASE_URL` only via `subprocess.run`'s `env` dict, which reaches the initial Claude Code process and the daemon — but not conversation workers spawned later from the daemon pool. New conversations silently bypassed the proxy and hit `api.anthropic.com` directly. ### Design decision: why project-local settings Three approaches were considered: **1. Global `~/.claude/settings.json`** — rejected. This file is shared across every Claude Code session on the machine. A user who runs `headroom wrap claude` in one terminal but opens an unwrapped session elsewhere would have their global settings rewritten to point at the Headroom proxy. If the proxy dies and cleanup does not run (SIGKILL, crash), the stale URL breaks all future sessions until the user manually edits the global file. **2. Kill cc-daemon before launch** — rejected. The issue itself suggests this, but killing the daemon is disruptive: it destroys the pre-forked worker pool shared by any other open Claude Code windows. Active conversations may lose their parent process. This is a hard-to-reverse side-effect of a command the user expects to be safe. **3. Project-local `<cwd>/.claude/settings.local.json`** — chosen. Claude Code applies `env` keys from project-local settings per its documented precedence order (Local > Project > User), and reloads them per-conversation. Scoping to the project means: other projects and unwrapped sessions are unaffected; the file is git-ignored by default so it won't be committed; and the worst-case stale URL (proxy crash without cleanup) affects only that one project's local settings and is trivially recoverable by re-running `headroom wrap claude` or deleting the file. Closes #951 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - Added `_write_claude_wrap_base_url(proxy_url, *, foundry_mode, settings_path)` in `headroom/cli/wrap.py`: merges `ANTHROPIC_BASE_URL` (or `ANTHROPIC_FOUNDRY_BASE_URL` in foundry mode) into `<cwd>/.claude/settings.local.json` under the `env` key. Returns the previous value for restore. - Added `_restore_claude_wrap_base_url(previous, *, foundry_mode, settings_path)`: called in the `wrap claude` `finally` block and in `unwrap_claude` to remove or restore the key so a stale proxy URL is never left behind. - `unwrap_claude` calls restore for both standard and foundry keys. - New test file `tests/test_cli/test_wrap_claude_base_url.py` (12 tests covering write, restore, roundtrip, foundry mode, sibling key preservation, and noop on absent file). ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text pytest tests/test_cli/test_wrap_claude_base_url.py -v 12 passed in 0.21s ruff check headroom/cli/wrap.py tests/test_cli/test_wrap_claude_base_url.py All checks passed! ``` ## Real Behavior Proof - Environment: macOS, branch `fix/daemon-base-url-inheritance`, Python 3.11.9. - Exact command / steps: Ran `pytest tests/test_cli/test_wrap_claude_base_url.py -v` and `ruff check` on the modified files from the PR branch. - Observed result: 12 new unit tests pass; ruff reports no issues. - Not tested: Live end-to-end verification (opening a second conversation via the daemon pool and confirming proxy receives traffic) — not safe to test inside the current wrapped session on port 8787. ## 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 The issue reporter tried `apiBaseUrl` in settings.json and found it ineffective. That key configures the API endpoint at the CC UI layer, not the process environment. `env.ANTHROPIC_BASE_URL` is the correct mechanism for propagating an environment variable to CC worker processes.
This commit is contained in:
Focused Instability
GitHub
parent
9f712ccbd7
commit
a554c3a0e6
2 changed files with 285 additions and 0 deletions
|
|
@ -576,6 +576,85 @@ def _remove_claude_rtk_hooks(settings_path: Path | None = None) -> bool:
|
|||
return True
|
||||
|
||||
|
||||
def _write_claude_wrap_base_url(
|
||||
proxy_url: str,
|
||||
*,
|
||||
foundry_mode: bool = False,
|
||||
settings_path: Path | None = None,
|
||||
) -> str | None:
|
||||
"""Persist proxy URL into project-local settings env key for daemon child inheritance.
|
||||
|
||||
Claude Code's cc-daemon pre-forks conversation workers using spawn (not
|
||||
fork), so those workers read settings.json fresh rather than inheriting
|
||||
the daemon's environment. Writing env.ANTHROPIC_BASE_URL into the
|
||||
project-local settings file (.claude/settings.local.json in cwd) ensures
|
||||
every new conversation — including those started after the initial launch —
|
||||
routes through the Headroom proxy without touching the global user settings
|
||||
file or affecting sessions in other projects. Returns the previous value
|
||||
so the caller can restore it on exit (issue #951).
|
||||
"""
|
||||
path = settings_path or (Path.cwd() / ".claude" / "settings.local.json")
|
||||
payload: dict[str, Any] = {}
|
||||
if path.exists():
|
||||
try:
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
except (OSError, json.JSONDecodeError):
|
||||
payload = {}
|
||||
if not isinstance(payload, dict):
|
||||
payload = {}
|
||||
env_map = dict(payload.get("env") or {}) if isinstance(payload.get("env"), dict) else {}
|
||||
key = "ANTHROPIC_FOUNDRY_BASE_URL" if foundry_mode else "ANTHROPIC_BASE_URL"
|
||||
previous = env_map.get(key)
|
||||
env_map[key] = proxy_url
|
||||
payload["env"] = env_map
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8")
|
||||
return previous
|
||||
|
||||
|
||||
def _restore_claude_wrap_base_url(
|
||||
previous: str | None,
|
||||
*,
|
||||
foundry_mode: bool = False,
|
||||
settings_path: Path | None = None,
|
||||
) -> None:
|
||||
"""Restore (or remove) the env key written by _write_claude_wrap_base_url.
|
||||
|
||||
Called in both the wrap-session finally block and unwrap_claude so the
|
||||
project-local settings entry is never left pointing at a dead proxy. When
|
||||
``previous`` is None the key is removed; when it has a value it is
|
||||
restored — preserving any URL the project already had set.
|
||||
"""
|
||||
path = settings_path or (Path.cwd() / ".claude" / "settings.local.json")
|
||||
if not path.exists():
|
||||
return
|
||||
try:
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
except (OSError, json.JSONDecodeError):
|
||||
return
|
||||
if not isinstance(payload, dict):
|
||||
return
|
||||
env_map = payload.get("env")
|
||||
if not isinstance(env_map, dict):
|
||||
return
|
||||
key = "ANTHROPIC_FOUNDRY_BASE_URL" if foundry_mode else "ANTHROPIC_BASE_URL"
|
||||
if previous is None:
|
||||
if key not in env_map:
|
||||
return
|
||||
del env_map[key]
|
||||
if env_map:
|
||||
payload["env"] = env_map
|
||||
else:
|
||||
payload.pop("env", None)
|
||||
else:
|
||||
env_map[key] = previous
|
||||
payload["env"] = env_map
|
||||
if payload:
|
||||
path.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8")
|
||||
else:
|
||||
path.unlink(missing_ok=True)
|
||||
|
||||
|
||||
def _setup_headroom_mcp(
|
||||
registrar: Any, port: int, *, verbose: bool = False, force: bool = False
|
||||
) -> None:
|
||||
|
|
@ -2885,6 +2964,8 @@ def claude(
|
|||
|
||||
# Setup rtk before launching (Claude-specific)
|
||||
proxy_holder: list[subprocess.Popen | None] = [None]
|
||||
_saved_base_url: list[str | None] = [None] # previous settings.json value for restore
|
||||
_settings_foundry: list[bool] = [False]
|
||||
cleanup = _make_cleanup(proxy_holder, port)
|
||||
_register_proxy_client(port)
|
||||
signal.signal(signal.SIGINT, _ignore_child_sigint)
|
||||
|
|
@ -3026,6 +3107,14 @@ def claude(
|
|||
else:
|
||||
env["ANTHROPIC_BASE_URL"] = proxy_url
|
||||
|
||||
# Issue #951: write to settings.json so daemon-spawned conversation
|
||||
# workers (which read settings.json fresh rather than inheriting the
|
||||
# daemon's environment) also route through Headroom.
|
||||
_settings_foundry[0] = bool(foundry_upstream)
|
||||
_saved_base_url[0] = _write_claude_wrap_base_url(
|
||||
proxy_url, foundry_mode=_settings_foundry[0]
|
||||
)
|
||||
|
||||
# Per-project savings attribution: tag every request with the launch
|
||||
# directory's name via X-Headroom-Project (user override wins).
|
||||
_apply_project_header_env(env)
|
||||
|
|
@ -3053,6 +3142,7 @@ def claude(
|
|||
click.echo(f" Error: {e}")
|
||||
raise SystemExit(1) from e
|
||||
finally:
|
||||
_restore_claude_wrap_base_url(_saved_base_url[0], foundry_mode=_settings_foundry[0])
|
||||
cleanup()
|
||||
|
||||
|
||||
|
|
@ -3110,6 +3200,9 @@ def unwrap_claude(
|
|||
else:
|
||||
click.echo(" Kept rtk Claude hooks (--keep-rtk).")
|
||||
|
||||
_restore_claude_wrap_base_url(None)
|
||||
_restore_claude_wrap_base_url(None, foundry_mode=True)
|
||||
|
||||
click.echo()
|
||||
click.echo("✓ Claude is no longer durably wrapped by Headroom.")
|
||||
if not no_stop_proxy:
|
||||
|
|
|
|||
192
tests/test_cli/test_wrap_claude_base_url.py
Normal file
192
tests/test_cli/test_wrap_claude_base_url.py
Normal file
|
|
@ -0,0 +1,192 @@
|
|||
"""Tests for _write_claude_wrap_base_url / _restore_claude_wrap_base_url (issue #951)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
from headroom.cli import wrap as wrap_cli
|
||||
|
||||
|
||||
def _settings(tmp_path: Path) -> Path:
|
||||
return tmp_path / ".claude" / "settings.json"
|
||||
|
||||
|
||||
def test_write_creates_env_key_in_fresh_file(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
prev = wrap_cli._write_claude_wrap_base_url("http://127.0.0.1:8787", settings_path=path)
|
||||
assert prev is None
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
assert payload["env"]["ANTHROPIC_BASE_URL"] == "http://127.0.0.1:8787"
|
||||
|
||||
|
||||
def test_write_preserves_other_env_keys(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text(json.dumps({"env": {"KEEP": "1", "ANOTHER": "2"}}), encoding="utf-8")
|
||||
wrap_cli._write_claude_wrap_base_url("http://127.0.0.1:8787", settings_path=path)
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
assert payload["env"]["KEEP"] == "1"
|
||||
assert payload["env"]["ANOTHER"] == "2"
|
||||
assert payload["env"]["ANTHROPIC_BASE_URL"] == "http://127.0.0.1:8787"
|
||||
|
||||
|
||||
def test_write_returns_none_when_key_absent(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
prev = wrap_cli._write_claude_wrap_base_url("http://127.0.0.1:8787", settings_path=path)
|
||||
assert prev is None
|
||||
|
||||
|
||||
def test_write_returns_previous_value_when_key_present(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text(
|
||||
json.dumps({"env": {"ANTHROPIC_BASE_URL": "http://old.proxy:9000"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
prev = wrap_cli._write_claude_wrap_base_url("http://127.0.0.1:8787", settings_path=path)
|
||||
assert prev == "http://old.proxy:9000"
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
assert payload["env"]["ANTHROPIC_BASE_URL"] == "http://127.0.0.1:8787"
|
||||
|
||||
|
||||
def test_write_foundry_mode_sets_foundry_key(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
wrap_cli._write_claude_wrap_base_url(
|
||||
"http://127.0.0.1:8787", foundry_mode=True, settings_path=path
|
||||
)
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
assert payload["env"]["ANTHROPIC_FOUNDRY_BASE_URL"] == "http://127.0.0.1:8787"
|
||||
assert "ANTHROPIC_BASE_URL" not in payload["env"]
|
||||
|
||||
|
||||
def test_restore_removes_key_when_previous_none(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text(
|
||||
json.dumps({"env": {"ANTHROPIC_BASE_URL": "http://127.0.0.1:8787"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
wrap_cli._restore_claude_wrap_base_url(None, settings_path=path)
|
||||
# file is deleted when payload becomes empty — key is gone
|
||||
assert not path.exists()
|
||||
|
||||
|
||||
def test_restore_removes_env_dict_when_empty(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text(
|
||||
json.dumps({"env": {"ANTHROPIC_BASE_URL": "http://127.0.0.1:8787"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
wrap_cli._restore_claude_wrap_base_url(None, settings_path=path)
|
||||
# entire payload was {"env": {...only our key...}} — file deleted rather than left as {}
|
||||
assert not path.exists()
|
||||
|
||||
|
||||
def test_restore_preserves_sibling_env_keys(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text(
|
||||
json.dumps({"env": {"ANTHROPIC_BASE_URL": "http://127.0.0.1:8787", "KEEP": "1"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
wrap_cli._restore_claude_wrap_base_url(None, settings_path=path)
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
assert "ANTHROPIC_BASE_URL" not in payload["env"]
|
||||
assert payload["env"]["KEEP"] == "1"
|
||||
|
||||
|
||||
def test_restore_sets_key_back_to_previous_value(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text(
|
||||
json.dumps({"env": {"ANTHROPIC_BASE_URL": "http://127.0.0.1:8787"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
wrap_cli._restore_claude_wrap_base_url("http://old.proxy:9000", settings_path=path)
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
assert payload["env"]["ANTHROPIC_BASE_URL"] == "http://old.proxy:9000"
|
||||
|
||||
|
||||
def test_restore_foundry_mode_removes_foundry_key(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text(
|
||||
json.dumps({"env": {"ANTHROPIC_FOUNDRY_BASE_URL": "http://127.0.0.1:8787"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
wrap_cli._restore_claude_wrap_base_url(None, foundry_mode=True, settings_path=path)
|
||||
# file deleted when payload empties
|
||||
assert not path.exists()
|
||||
|
||||
|
||||
def test_restore_noop_when_file_absent(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
wrap_cli._restore_claude_wrap_base_url(None, settings_path=path) # must not raise
|
||||
|
||||
|
||||
def test_restore_noop_when_key_not_present(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text(json.dumps({"env": {"OTHER": "1"}}), encoding="utf-8")
|
||||
wrap_cli._restore_claude_wrap_base_url(None, settings_path=path) # key absent — no-op
|
||||
assert json.loads(path.read_text())["env"]["OTHER"] == "1"
|
||||
|
||||
|
||||
def test_restore_noop_when_env_not_dict(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text(json.dumps({"env": "not-a-dict"}), encoding="utf-8")
|
||||
wrap_cli._restore_claude_wrap_base_url(None, settings_path=path) # must not raise
|
||||
|
||||
|
||||
def test_restore_noop_when_payload_not_dict(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text("[1, 2, 3]", encoding="utf-8") # valid JSON but not a dict
|
||||
wrap_cli._restore_claude_wrap_base_url(None, settings_path=path) # must not raise
|
||||
|
||||
|
||||
def test_restore_noop_when_file_corrupt(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text("not valid json {{{{", encoding="utf-8")
|
||||
wrap_cli._restore_claude_wrap_base_url(None, settings_path=path) # must not raise
|
||||
|
||||
|
||||
def test_write_recovers_from_corrupt_file(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text("not valid json {{{{", encoding="utf-8")
|
||||
prev = wrap_cli._write_claude_wrap_base_url("http://127.0.0.1:8787", settings_path=path)
|
||||
assert prev is None # treated as fresh
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
assert payload["env"]["ANTHROPIC_BASE_URL"] == "http://127.0.0.1:8787"
|
||||
|
||||
|
||||
def test_write_recovers_from_non_dict_payload(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text("[1, 2, 3]", encoding="utf-8") # valid JSON but not a dict
|
||||
prev = wrap_cli._write_claude_wrap_base_url("http://127.0.0.1:8787", settings_path=path)
|
||||
assert prev is None
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
assert payload["env"]["ANTHROPIC_BASE_URL"] == "http://127.0.0.1:8787"
|
||||
|
||||
|
||||
def test_write_restore_roundtrip(tmp_path: Path) -> None:
|
||||
path = _settings(tmp_path)
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_text(json.dumps({"model": "opus", "env": {"OTHER": "x"}}), encoding="utf-8")
|
||||
prev = wrap_cli._write_claude_wrap_base_url("http://127.0.0.1:8787", settings_path=path)
|
||||
assert prev is None
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
assert payload["env"]["ANTHROPIC_BASE_URL"] == "http://127.0.0.1:8787"
|
||||
assert payload["model"] == "opus"
|
||||
|
||||
wrap_cli._restore_claude_wrap_base_url(prev, settings_path=path)
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
assert "ANTHROPIC_BASE_URL" not in payload.get("env", {})
|
||||
assert payload["env"]["OTHER"] == "x"
|
||||
assert payload["model"] == "opus"
|
||||
Loading…
Add table
Add a link
Reference in a new issue