From 8f3e33a00ea377403497b600841c03344d0c1cd8 Mon Sep 17 00:00:00 2001 From: Tejas Chopra Date: Sat, 22 Aug 2026 15:25:47 -0700 Subject: [PATCH] fix(doctor): report project-scoped Claude routing instead of a false negative (#3213) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Co-authored-by: Claude Opus 5 --- headroom/cli/doctor.py | 81 ++++++++++++++++++++++++++++------------ tests/test_cli_doctor.py | 79 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 24 deletions(-) diff --git a/headroom/cli/doctor.py b/headroom/cli/doctor.py index cd5d4839a..03d0ce58e 100644 --- a/headroom/cli/doctor.py +++ b/headroom/cli/doctor.py @@ -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), diff --git a/tests/test_cli_doctor.py b/tests/test_cli_doctor.py index 8695af976..7f68cd4e6 100644 --- a/tests/test_cli_doctor.py +++ b/tests/test_cli_doctor.py @@ -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