fix(doctor): report project-scoped Claude routing instead of a false negative (#3213)
Refs #3205 — **issue 2 of 2**. The `wrap`-session crashes reported in that issue are *not* addressed here; see the note at the bottom. ## The report A session routed via `headroom init claude` was reported by `headroom doctor` as **not routed**, while it demonstrably was: - `ps eww` on the live `claude` process showed `ANTHROPIC_BASE_URL=http://127.0.0.1:8787` - the `mcp__headroom__*` tools were present and firing - `headroom_stats` showed **164 of 174 requests compressed** on that very session The cost wasn't cosmetic. The team believed 3 of 4 sessions were unrouted on doctor's word, and hand-checked `ps eww` plus MCP tool presence on each one to find the real state. ## Root cause — a scope mismatch | | Path | |---|---| | `init claude` (non-global) **writes** | `./.claude/settings.local.json` | | `doctor` **read** | `~/.claude/settings.json` only | Claude Code layers project settings over user settings, so project-scoped routing — what `init` writes by default — was invisible to the check. ## Fix `check_claude_routing` now takes the project-scoped candidates and consults them in **Claude's own precedence order** (project-local, project, then user), reporting the first that carries `ANTHROPIC_BASE_URL`. The summary names the file that supplied it, so which scope is in effect is never ambiguous — that ambiguity is what made this expensive to diagnose. Reading more files must not turn a routed session into a crash or a silent skip: - a per-file parse failure is surfaced verbatim (`could not parse …`) rather than swallowed into the misleading "not routed" - the non-dict guard is preserved **per file** — a hand-edited settings file containing `[]` or `null` would otherwise raise `AttributeError` inside the very command run to diagnose it - a missing project file is skipped, not fatal The third argument is optional and defaults to the previous single-file behaviour, so existing callers and tests are unaffected. ## Not scraping `ps` The reporter suggested inspecting live `claude` process environments. That isn't needed and would be platform-specific — the routing is written to a file whose path we already know. The gap was that we read the wrong scope, so that's what this fixes. ## Testing 86 passing. Confirmed **discriminating** — all 7 new tests fail against unfixed `doctor.py` and pass after: | Test | Covers | |---|---| | project-local counts as routed | the reported bug | | project `settings.json` counts as routed | the other project file | | project takes precedence over user | Claude's layering | | falls back to user when project has no base URL | no false positive | | still warns when nothing routes | no blanket pass | | missing project file skipped | not fatal | | unparseable project file surfaces | not silently "not routed" | | no project paths → original behaviour | backward compatibility | 🤖 Generated with [Claude Code](https://claude.com/claude-code) 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
+57
-24
@@ -15,7 +15,7 @@ import json
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
from collections.abc import Callable, Mapping
|
||||
from collections.abc import Callable, Mapping, Sequence
|
||||
from dataclasses import asdict, dataclass
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
@@ -149,47 +149,76 @@ def check_version_drift(livez: dict[str, Any] | None, installed: str) -> CheckRe
|
||||
)
|
||||
|
||||
|
||||
def check_claude_routing(settings_path: Path, port: int) -> CheckResult:
|
||||
"""Is Claude Code configured to route through the proxy?"""
|
||||
def _claude_base_url_in(path: Path) -> tuple[str, CheckResult | None]:
|
||||
"""Read ``env.ANTHROPIC_BASE_URL`` from one Claude settings file.
|
||||
|
||||
Returns ``(base_url, error)``. A parse problem comes back as a WARN so the
|
||||
caller surfaces it verbatim instead of skipping the file and reporting the
|
||||
misleading "not routed".
|
||||
"""
|
||||
name = "claude"
|
||||
if not settings_path.exists():
|
||||
return CheckResult(
|
||||
name=name,
|
||||
status=WARN,
|
||||
summary="not routed (no ~/.claude/settings.json)",
|
||||
hint="wrap it: headroom wrap claude",
|
||||
)
|
||||
try:
|
||||
payload = json.loads(settings_path.read_text(encoding="utf-8"))
|
||||
payload = json.loads(path.read_text(encoding="utf-8"))
|
||||
except (OSError, ValueError) as exc:
|
||||
return CheckResult(
|
||||
name=name,
|
||||
status=WARN,
|
||||
summary=f"could not parse {settings_path}: {exc}",
|
||||
)
|
||||
return "", CheckResult(name=name, status=WARN, summary=f"could not parse {path}: {exc}")
|
||||
# `json.loads` succeeds on valid non-object JSON (e.g. `[]`, `null`, `42`),
|
||||
# which a hand-edited or reset settings file can contain. `.get` on a
|
||||
# non-dict raises AttributeError, and it is not one of the caught parse
|
||||
# errors above, so it would crash the very command run to diagnose the
|
||||
# broken config. Treat a non-object like an unparseable file.
|
||||
if not isinstance(payload, dict):
|
||||
return CheckResult(
|
||||
return "", CheckResult(
|
||||
name=name,
|
||||
status=WARN,
|
||||
summary=f"could not parse {settings_path}: not a JSON object",
|
||||
summary=f"could not parse {path}: not a JSON object",
|
||||
)
|
||||
base_url = ""
|
||||
env_block = payload.get("env")
|
||||
if isinstance(env_block, dict):
|
||||
base_url = str(env_block.get("ANTHROPIC_BASE_URL", "") or "")
|
||||
if not base_url:
|
||||
return str(env_block.get("ANTHROPIC_BASE_URL", "") or ""), None
|
||||
return "", None
|
||||
|
||||
|
||||
def check_claude_routing(
|
||||
settings_path: Path,
|
||||
port: int,
|
||||
project_settings_paths: Sequence[Path] | None = None,
|
||||
) -> CheckResult:
|
||||
"""Is Claude Code configured to route through the proxy?
|
||||
|
||||
Claude Code layers project settings over user settings, and `headroom init
|
||||
claude` without --global writes the project-scoped
|
||||
``.claude/settings.local.json``. Reading only ``~/.claude/settings.json``
|
||||
reported "not routed" for sessions that demonstrably were -- confirmed by
|
||||
`ps eww` on the live process and by active compression on it (#3205).
|
||||
Candidates are consulted in Claude's own precedence order, and the summary
|
||||
names the file that supplied the routing so the scope is never ambiguous.
|
||||
"""
|
||||
name = "claude"
|
||||
candidates = [*(project_settings_paths or []), settings_path]
|
||||
existing = [path for path in candidates if path.exists()]
|
||||
if not existing:
|
||||
return CheckResult(
|
||||
name=name,
|
||||
status=WARN,
|
||||
summary="not routed (no ANTHROPIC_BASE_URL in settings env)",
|
||||
summary="not routed (no ~/.claude/settings.json)",
|
||||
hint="wrap it: headroom wrap claude",
|
||||
)
|
||||
return _classify_routing_url(name, base_url, port, source=str(settings_path))
|
||||
first_error: CheckResult | None = None
|
||||
for candidate in existing:
|
||||
base_url, error = _claude_base_url_in(candidate)
|
||||
if error is not None:
|
||||
first_error = first_error or error
|
||||
continue
|
||||
if base_url:
|
||||
return _classify_routing_url(name, base_url, port, source=str(candidate))
|
||||
if first_error is not None:
|
||||
return first_error
|
||||
return CheckResult(
|
||||
name=name,
|
||||
status=WARN,
|
||||
summary="not routed (no ANTHROPIC_BASE_URL in settings env)",
|
||||
hint="wrap it: headroom wrap claude",
|
||||
)
|
||||
|
||||
|
||||
def check_claude_auth_conflict(
|
||||
@@ -653,7 +682,11 @@ def doctor(port: int, emit_json: bool) -> None:
|
||||
checks = [
|
||||
check_proxy_liveness(livez, base_url),
|
||||
check_version_drift(livez, installed),
|
||||
check_claude_routing(claude_settings_path(), port),
|
||||
check_claude_routing(
|
||||
claude_settings_path(),
|
||||
port,
|
||||
[project_local_claude_settings, project_claude_settings],
|
||||
),
|
||||
check_wrap_marker_staleness(project_local_claude_settings),
|
||||
check_codex_routing(codex_config_path(), port),
|
||||
check_shell_env(os.environ, port),
|
||||
|
||||
@@ -375,6 +375,85 @@ class TestClaudeRemoteControlGate:
|
||||
assert result.status == PASS
|
||||
|
||||
|
||||
class TestClaudeRoutingScope:
|
||||
"""Project-scoped routing must not read as "not routed" (#3205).
|
||||
|
||||
`headroom init claude` without --global writes
|
||||
`.claude/settings.local.json`. Reading only `~/.claude/settings.json`
|
||||
reported not-routed for sessions that were genuinely routed and actively
|
||||
compressing, which sent one team hand-checking `ps eww` on every session.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _settings(path, base_url): # noqa: ANN001, ANN205
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
body = {"env": {"ANTHROPIC_BASE_URL": base_url}} if base_url else {"env": {}}
|
||||
path.write_text(json.dumps(body), encoding="utf-8")
|
||||
return path
|
||||
|
||||
def test_project_local_settings_count_as_routed(self, tmp_path):
|
||||
user = tmp_path / "user" / "settings.json"
|
||||
project = self._settings(
|
||||
tmp_path / "proj" / ".claude" / "settings.local.json", "http://127.0.0.1:8787"
|
||||
)
|
||||
|
||||
result = check_claude_routing(user, 8787, [project])
|
||||
|
||||
assert result.status == PASS
|
||||
assert "settings.local.json" in result.summary or "settings.local.json" in str(result)
|
||||
|
||||
def test_project_settings_json_counts_as_routed(self, tmp_path):
|
||||
user = tmp_path / "user" / "settings.json"
|
||||
project = self._settings(
|
||||
tmp_path / "proj" / ".claude" / "settings.json", "http://127.0.0.1:8787"
|
||||
)
|
||||
|
||||
assert check_claude_routing(user, 8787, [project]).status == PASS
|
||||
|
||||
def test_project_scope_takes_precedence_over_user_scope(self, tmp_path):
|
||||
"""Claude layers project over user, so the reported port follows suit."""
|
||||
user = self._settings(tmp_path / "user" / "settings.json", "http://127.0.0.1:9999")
|
||||
project = self._settings(
|
||||
tmp_path / "proj" / ".claude" / "settings.local.json", "http://127.0.0.1:8787"
|
||||
)
|
||||
|
||||
assert check_claude_routing(user, 8787, [project]).status == PASS
|
||||
|
||||
def test_falls_back_to_user_scope_when_project_has_no_base_url(self, tmp_path):
|
||||
user = self._settings(tmp_path / "user" / "settings.json", "http://127.0.0.1:8787")
|
||||
project = self._settings(tmp_path / "proj" / ".claude" / "settings.local.json", "")
|
||||
|
||||
assert check_claude_routing(user, 8787, [project]).status == PASS
|
||||
|
||||
def test_still_warns_when_nothing_routes(self, tmp_path):
|
||||
user = self._settings(tmp_path / "user" / "settings.json", "")
|
||||
project = self._settings(tmp_path / "proj" / ".claude" / "settings.local.json", "")
|
||||
|
||||
assert check_claude_routing(user, 8787, [project]).status == WARN
|
||||
|
||||
def test_missing_project_file_is_skipped_not_fatal(self, tmp_path):
|
||||
user = self._settings(tmp_path / "user" / "settings.json", "http://127.0.0.1:8787")
|
||||
absent = tmp_path / "proj" / ".claude" / "settings.local.json"
|
||||
|
||||
assert check_claude_routing(user, 8787, [absent]).status == PASS
|
||||
|
||||
def test_unparseable_project_file_surfaces_rather_than_reporting_not_routed(self, tmp_path):
|
||||
project = tmp_path / "proj" / ".claude" / "settings.local.json"
|
||||
project.parent.mkdir(parents=True, exist_ok=True)
|
||||
project.write_text("{not json", encoding="utf-8")
|
||||
user = tmp_path / "user" / "settings.json"
|
||||
|
||||
result = check_claude_routing(user, 8787, [project])
|
||||
|
||||
assert result.status == WARN
|
||||
assert "could not parse" in result.summary
|
||||
|
||||
def test_no_project_paths_preserves_original_behaviour(self, tmp_path):
|
||||
user = self._settings(tmp_path / "user" / "settings.json", "http://127.0.0.1:8787")
|
||||
|
||||
assert check_claude_routing(user, 8787).status == PASS
|
||||
|
||||
|
||||
class TestCodexRouting:
|
||||
def test_missing_file_warns(self, tmp_path):
|
||||
assert check_codex_routing(tmp_path / "config.toml", 8787).status == WARN
|
||||
|
||||
Reference in New Issue
Block a user