refactor(agent/background_review,anthropic_adapter): drop dead is_background_review_enabled (tests repointed to load_background_review_settings), share _new_sdk_client, table call-detail defaults

agent/anthropic_adapter.py

@@ -39,12 +39,7 @@ from agent.anthropic_credentials import (  # noqa: F401
     run_hermes_oauth_login_pure, run_oauth_setup_token,
 )
 
-try:
-    import hermes_cli as _hermes_cli
-
-    _HERMES_VERSION = str(_hermes_cli.__version__)
-except Exception:
-    _HERMES_VERSION = "0.0.0"
+from hermes_cli import __version__ as _HERMES_VERSION
 
 
 # ``import anthropic`` is deliberately NOT at module top: the SDK costs ~220 ms of imports and
@@ -287,7 +282,7 @@ def _detect_claude_code_version() -> str:
 
 
 def _get_claude_code_version() -> str:
-    """Lazily detect the installed Claude Code version when OAuth headers need it."""
+    """Detect lazily (only OAuth headers need it) and cache for the process."""
     global _claude_code_version_cache
     if _claude_code_version_cache is None:
         _claude_code_version_cache = _detect_claude_code_version()
@@ -406,13 +401,19 @@ def _build_anthropic_client_with_bearer_hook(
     kwargs["http_client"] = build_bearer_http_client(token_provider, timeout=kwargs["timeout"])
     kwargs["auth_token"] = "entra-id-bearer-via-http-hook"
     headers = _beta_header(_common_betas_for_base_url(normalized_base_url, drop_context_1m_beta=drop_context_1m_beta))
+    return _new_sdk_client(sdk, kwargs, headers)
+
+
+def _new_sdk_client(sdk, kwargs: Dict[str, Any], headers: Dict[str, str]):
+    """``sdk.Anthropic(**kwargs)`` with ``headers`` attached. Bearer-only construction leaves
+    ``api_key`` unset, so the SDK fills it from ANTHROPIC_API_KEY (loaded from ~/.hermes/.env) and
+    sends dual auth — X-Api-Key *and* Authorization: Bearer — on every Portal/MiniMax/OAuth/Entra
+    request; clear it whenever we intentionally authenticated via auth_token."""
     if headers:
         kwargs["default_headers"] = headers
-
     client = sdk.Anthropic(**kwargs)
-    # Same env-inference trap as build_anthropic_client: auth_token-only construction would
-    # otherwise also send ANTHROPIC_API_KEY as X-Api-Key.
-    client.api_key = None
+    if "auth_token" in kwargs and "api_key" not in kwargs:
+        client.api_key = None
     return client
 
 
@@ -475,16 +476,7 @@ def build_anthropic_client(api_key, base_url: str = None, timeout: float = None,
         # get these from profile.default_headers, but this route never sees the profile.
         for k, v in _attribution_headers().items():
             headers.setdefault(k, v)
-    if headers:
-        kwargs["default_headers"] = headers
-
-    client = sdk.Anthropic(**kwargs)
-    # Bearer-only construction leaves ``api_key`` unset, so the SDK fills it from ANTHROPIC_API_KEY
-    # (loaded from ~/.hermes/.env) and sends dual auth — X-Api-Key *and* Authorization: Bearer —
-    # on every Portal/MiniMax/OAuth request. Clear it whenever we authenticated via auth_token.
-    if "auth_token" in kwargs and "api_key" not in kwargs:
-        client.api_key = None
-    return client
+    return _new_sdk_client(sdk, kwargs, headers)
 
 
 def build_anthropic_bedrock_client(region: str):

agent/background_review.py

@@ -213,25 +213,6 @@ def load_background_review_settings() -> tuple[bool, Dict[str, Any]]:
         return True, {}
 
 
-def is_background_review_enabled(task_cfg: Optional[Dict[str, Any]] = None) -> bool:
-    """Whether automatic post-turn review may spawn (``enabled``, default true). Explicit
-    ``/refine`` (``focus`` set) bypasses this gate; prefer :func:`load_background_review_settings`
-    at the spawn site so the block is not re-read on the same turn."""
-    if task_cfg is None:
-        return load_background_review_settings()[0]
-    try:
-        from utils import is_truthy_value
-
-        return is_truthy_value(task_cfg.get("enabled"), default=True)
-    except Exception:
-        logger.warning(
-            "Failed to interpret background_review.enabled; leaving "
-            "automatic review enabled (fail-open)",
-            exc_info=True,
-        )
-        return True
-
-
 def _resolve_review_runtime(agent: Any, task_cfg: Optional[Dict[str, Any]] = None) -> Dict[str, Any]:
     """Resolve provider/model/credentials for the review fork.
 
@@ -666,6 +647,13 @@ def _verbose_memory_lines(label: str, detail: Dict) -> List[str]:
     return [line or f"{label} updated"]
 
 
+# Tool-call argument fields surfaced in action summaries, with their defaults.
+_CALL_DETAIL_DEFAULTS = (
+    ("action", "?"), ("target", "memory"), ("content", ""), ("old_text", ""), ("name", ""),
+    ("old_string", ""), ("new_string", ""),
+)
+
+
 def _collect_review_call_details(review_messages: List[Dict]) -> Tuple[set, dict]:
     """Map review-agent tool_call ids -> parsed call arguments for notify tools.
 
@@ -695,15 +683,8 @@ def _collect_review_call_details(review_messages: List[Dict]) -> Tuple[set, dict
                 args = {}
             if tcid:
                 call_details[tcid] = {
-                    "tool": fn_name,
-                    "action": args.get("action", "?"),
-                    "target": args.get("target", "memory"),
-                    "content": args.get("content", ""),
-                    "old_text": args.get("old_text", ""),
-                    "operations": args.get("operations") or [],
-                    "name": args.get("name", ""),
-                    "old_string": args.get("old_string", ""),
-                    "new_string": args.get("new_string", ""),
+                    "tool": fn_name, "operations": args.get("operations") or [],
+                    **{k: args.get(k, default) for k, default in _CALL_DETAIL_DEFAULTS},
                 }
     return all_tool_call_ids, call_details
 
@@ -1332,7 +1313,6 @@ __all__ = [
     "_MEMORY_REVIEW_PROMPT",
     "_SKILL_REVIEW_PROMPT",
     "_COMBINED_REVIEW_PROMPT",
-    "is_background_review_enabled",
     "load_background_review_settings",
     "spawn_background_review_thread",
     "summarize_background_review_actions",

tests/agent/test_background_review_usage.py

@@ -160,7 +160,7 @@ def test_enabled_config_failure_logs_warning(caplog):
         "hermes_cli.config.load_config_readonly",
         side_effect=RuntimeError("boom"),
     ), caplog.at_level(logging.WARNING, logger="agent.background_review"):
-        assert background_review.is_background_review_enabled() is True
+        assert background_review.load_background_review_settings()[0] is True
     assert any(
         "fail-open" in r.message.lower() or "leaving automatic" in r.message.lower()
         for r in caplog.records

tests/run_agent/test_background_review_cost_controls.py

@@ -166,10 +166,10 @@ def test_digest_records_tool_names_in_arc():
 
 def test_enabled_defaults_true():
     with patch("hermes_cli.config.load_config_readonly", return_value={}):
-        assert br.is_background_review_enabled() is True
+        assert br.load_background_review_settings()[0] is True
 
 
 def test_enabled_false_disables_automatic_review():
     cfg = {"auxiliary": {"background_review": {"enabled": False}}}
     with patch("hermes_cli.config.load_config_readonly", return_value=cfg):
-        assert br.is_background_review_enabled() is False
+        assert br.load_background_review_settings()[0] is False