From 2684412ed5f00a9d6ec00d3f5b129bc4c658f419 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:51:36 -0700 Subject: [PATCH] refactor(hermes_cli): doctor status tables (_OPENROUTER_STATUS, _PACKAGES, _KEYED_PROBES), _require helper, compact docstrings --- hermes_cli/doctor.py | 2 +- hermes_cli/doctor_config.py | 113 ++++++++----------------- hermes_cli/doctor_connectivity.py | 99 +++++++++------------- hermes_cli/doctor_live.py | 45 ++++------ hermes_cli/doctor_platform.py | 134 +++++++++++++----------------- hermes_cli/doctor_state.py | 42 ++++------ hermes_cli/doctor_tools.py | 134 ++++++++++++------------------ hermes_cli/dump.py | 45 +++++----- 8 files changed, 241 insertions(+), 373 deletions(-) diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index 73ff8a8596..9f561c84f5 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -124,7 +124,7 @@ def _check_api_connectivity(should_fix: bool) -> Finding: print("\r" + " " * 70 + "\r", end="") for r in results: for glyph, label, detail in r.lines: - print(f" {glyph} {label} {detail}" if detail else f" {glyph} {label}") + print(f" {glyph} {label}" + (f" {detail}" if detail else "")) if r.issues and not _has_healthy_oauth_fallback_for_apikey_provider(r.label): f.issues.extend(r.issues) return f diff --git a/hermes_cli/doctor_config.py b/hermes_cli/doctor_config.py index eb1cd3174f..3b795e8602 100644 --- a/hermes_cli/doctor_config.py +++ b/hermes_cli/doctor_config.py @@ -9,7 +9,7 @@ from __future__ import annotations import os import shutil from hermes_cli.doctor_report import ( - Finding, _fail_and_issue, _section, check_fail, check_info, check_ok, check_warn, doctor_check, + Finding, _fail_and_issue, _section, check_bool, check_fail, check_info, check_ok, check_warn, doctor_check, ) @@ -46,10 +46,7 @@ _DEPRECATED_ENV_VARS: tuple[tuple[str, str], ...] = ( def collect_deprecated_config_keys(raw_config: dict | None) -> list[tuple[str, str]]: - """Return ``(legacy_path, replacement)`` for deprecated keys present in the on-disk YAML. - - Empty containers still count — presence of the legacy key is the signal to migrate. - """ + """``(legacy_path, replacement)`` for deprecated keys in the on-disk YAML (empty containers still count).""" if not isinstance(raw_config, dict): return [] return [ @@ -60,11 +57,8 @@ def collect_deprecated_config_keys(raw_config: dict | None) -> list[tuple[str, s def collect_deprecated_env_vars(env_map: dict | None) -> list[tuple[str, str]]: - """Return ``(legacy_env, replacement)`` for deprecated vars present in *env_map*. - - *env_map* should come from the on-disk ``.env`` (e.g. ``load_env()``), not - ``os.environ``, so bridged runtime vars do not trigger false positives. - """ + """``(legacy_env, replacement)`` for deprecated vars in *env_map* (the on-disk ``.env``, not ``os.environ``, + so bridged runtime vars do not false-positive).""" if not isinstance(env_map, dict): return [] return [(name, replacement) for name, replacement in _DEPRECATED_ENV_VARS @@ -101,11 +95,8 @@ def collect_relay_plugin_cutover_findings(raw_config: dict | None, env_map: dict def report_deprecated_config_and_env(raw_config: dict | None = None, env_map: dict | None = None) -> list[tuple[str, str]]: - """Emit non-failing doctor warnings for deprecated config keys and env vars. - - Returns the ``(legacy, replacement)`` findings reported (empty when nothing deprecated - is present). Does not mutate config/env and does not append to the blocking ``issues`` list. - """ + """Emit non-failing doctor warnings for deprecated config keys and env vars; returns the findings reported. + Does not mutate config/env and does not append to the blocking ``issues`` list.""" deprecated = collect_deprecated_config_keys(raw_config) + collect_deprecated_env_vars(env_map) relay_cutover = collect_relay_plugin_cutover_findings(raw_config, env_map) findings = deprecated + relay_cutover @@ -123,11 +114,8 @@ def report_deprecated_config_and_env(raw_config: dict | None = None, env_map: di def managed_scope_check() -> None: - """Report the active managed scope (resolved dir + pinned key counts); silent when none. - - A managed dir resolved from the HERMES_MANAGED_DIR override (rather than the system default) is - surfaced too — a redirected scope is the documented foot-gun (docs/design/managed-scope.md §7). - """ + """Report the active managed scope (resolved dir + pinned key counts); silent when none. A HERMES_MANAGED_DIR + override is surfaced too — a redirected scope is the documented foot-gun (docs/design/managed-scope.md §7).""" try: from hermes_cli import managed_scope managed_dir = managed_scope.get_managed_dir() @@ -178,10 +166,7 @@ def _check_env_file(should_fix: bool) -> Finding: content = env_path.read_text(encoding="utf-8") except UnicodeDecodeError: content = env_path.read_text(encoding="latin-1") - if _has_provider_env_config(content): - check_ok("API key or custom endpoint configured") - else: - check_warn(f"No API key found in {_DHH}/.env") + if not check_bool(_has_provider_env_config(content), "API key or custom endpoint configured", f"No API key found in {_DHH}/.env"): f.issues.append("Run 'hermes setup' to configure API keys") elif (PROJECT_ROOT / '.env').exists(): # project root as fallback check_ok(".env file exists (in project directory)") @@ -205,20 +190,16 @@ def _check_env_file(should_fix: bool) -> Finding: def _known_provider_ids(cfg: dict) -> tuple[set, list, object, object, object]: - """Return (known ids, custom providers, resolve_auth, normalize, resolve_full). - - Registry lookups are best-effort: any import failure leaves the matching resolver as None so - validation degrades to "unavailable" rather than crashing doctor. - """ + """Return (known ids, custom providers, resolve_auth, normalize, resolve_full); any import failure leaves + the matching resolver as None so validation degrades to "unavailable" rather than crashing doctor.""" known: set = set() - resolve_auth = normalize = resolve_full = None + resolve_auth = normalize = resolve_full = aliases = None + custom_providers: list = [] try: from hermes_cli.auth import PROVIDER_REGISTRY, resolve_provider as resolve_auth known = set(PROVIDER_REGISTRY.keys()) | {"openrouter", "custom", "auto", "moa"} except Exception: pass - custom_providers: list = [] - aliases = None try: from hermes_cli.config import get_compatible_custom_providers from hermes_cli.providers import ( @@ -236,11 +217,8 @@ def _known_provider_ids(cfg: dict) -> tuple[set, list, object, object, object]: user_providers = cfg.get("providers") if isinstance(user_providers, dict): from hermes_cli.config import is_provider_enabled - known.update( - str(name).strip().lower() - for name, prov_cfg in user_providers.items() - if str(name).strip() and is_provider_enabled(prov_cfg) - ) + known.update(str(name).strip().lower() for name, prov_cfg in user_providers.items() + if str(name).strip() and is_provider_enabled(prov_cfg)) if aliases is not None: for entry in custom_providers: name = str(entry.get("name") or "").strip() if isinstance(entry, dict) else "" @@ -258,9 +236,8 @@ _VENDOR_SLUG_PROVIDERS = { def _provider_has_credentials(runtime_provider: str) -> bool: - """Only API-key providers in PROVIDER_REGISTRY are checked — OAuth/SDK/custom providers have their - own env-var checks elsewhere in doctor, and get_auth_status() returns a bare {logged_in: False} - for anything it doesn't dispatch, which would false-positive.""" + """Only API-key providers in PROVIDER_REGISTRY are checked — OAuth/SDK/custom providers have their own + checks elsewhere, and get_auth_status() returns a bare {logged_in: False} for anything it doesn't dispatch.""" if runtime_provider == "openrouter": from hermes_cli.config import get_env_value @@ -308,18 +285,12 @@ def _validate_model_config(config_path, issues: list) -> None: if catalog_provider is not None: accept.add(catalog_provider) - if provider and provider != "auto" and ( - catalog_provider is None or (known_providers and not (accept & valid_provider_ids)) - ): + if provider and provider != "auto" and (catalog_provider is None or (known_providers and not (accept & valid_provider_ids))): known_list = ", ".join(sorted(known_providers)) if known_providers else "(unavailable)" _fail_and_issue( - f"model.provider '{provider_raw}' is not a recognised provider", - f"(known: {known_list})", - ( - f"model.provider '{provider_raw}' is unknown. " - f"Valid providers: {known_list}. " - f"Fix: run 'hermes config set model.provider '" - ), + f"model.provider '{provider_raw}' is not a recognised provider", f"(known: {known_list})", + f"model.provider '{provider_raw}' is unknown. Valid providers: {known_list}. " + f"Fix: run 'hermes config set model.provider '", issues, ) @@ -342,11 +313,9 @@ def _validate_model_config(config_path, issues: list) -> None: _fail_and_issue( f"model.provider '{runtime_provider}' is set but no API key is configured", "(check ~/.hermes/.env or run 'hermes setup')", - ( - f"No credentials found for provider '{runtime_provider}'. " - f"Run 'hermes setup' or set the provider's API key in {_DHH}/.env, " - f"or switch providers with 'hermes config set model.provider '" - ), + f"No credentials found for provider '{runtime_provider}'. " + f"Run 'hermes setup' or set the provider's API key in {_DHH}/.env, " + f"or switch providers with 'hermes config set model.provider '", issues, ) except Exception: @@ -385,10 +354,9 @@ def _check_config_file(should_fix: bool) -> Finding: def _drift_config_version(f: Finding, should_fix: bool, config_path) -> None: from hermes_cli.config import check_config_version, migrate_config current_ver, latest_ver = check_config_version() - if current_ver >= latest_ver: - check_ok(f"Config version up to date (v{current_ver})") + if check_bool(current_ver >= latest_ver, f"Config version up to date (v{current_ver})", + (f"Config version outdated (v{current_ver} → v{latest_ver})", "(new settings available)")): return - check_warn(f"Config version outdated (v{current_ver} → v{latest_ver})", "(new settings available)") if not should_fix: f.issues.append("Run 'hermes doctor --fix' or 'hermes setup' to migrate config") return @@ -412,8 +380,7 @@ def _drift_stale_root_keys(f: Finding, should_fix: bool, config_path) -> None: if not should_fix: f.issues.append("Stale root-level provider/base_url in config.yaml — run 'hermes doctor --fix'") return - # Coerce scalar/None ``model:`` into a dict before mutation — setdefault - # would hand back an existing scalar and item-assignment would TypeError. + # Coerce scalar/None ``model:`` into a dict before mutation (setdefault would hand back a scalar). raw_model = raw_config.get("model") if isinstance(raw_model, dict): model_section = raw_model @@ -432,10 +399,9 @@ def _drift_stale_root_keys(f: Finding, should_fix: bool, config_path) -> None: def _drift_max_iterations_ghost(f: Finding, should_fix: bool, config_path) -> None: """A stale HERMES_MAX_ITERATIONS in .env shadows agent.max_turns in config.yaml. - The setup wizard used to dual-write the budget to both stores. The gateway bridge normally derives - HERMES_MAX_ITERATIONS from agent.max_turns, but if that bridge bails on an earlier config-parse - error the .env value silently wins. Read the .env FILE (load_env), not get_env_value/os.environ, - which the bridge may have overridden already. + The setup wizard used to dual-write the budget. The gateway bridge derives HERMES_MAX_ITERATIONS from + agent.max_turns, but if it bails on an earlier config-parse error the .env value silently wins. Read the + .env FILE (load_env), not get_env_value/os.environ, which the bridge may have overridden already. """ from hermes_cli.doctor import _DHH from hermes_cli.config import load_env, read_user_config_raw, remove_env_value @@ -447,29 +413,20 @@ def _drift_max_iterations_ghost(f: Finding, should_fix: bool, config_path) -> No env_ghost = load_env().get("HERMES_MAX_ITERATIONS") if cfg_max_turns is None or env_ghost is None or str(cfg_max_turns).strip() == str(env_ghost).strip(): return - check_warn( - f"HERMES_MAX_ITERATIONS={env_ghost} in .env shadows agent.max_turns={cfg_max_turns} in config.yaml", - "(stale ghost from an earlier `hermes setup` run)", - ) + check_warn(f"HERMES_MAX_ITERATIONS={env_ghost} in .env shadows agent.max_turns={cfg_max_turns} in config.yaml", + "(stale ghost from an earlier `hermes setup` run)") if not should_fix: f.issues.append("Stale HERMES_MAX_ITERATIONS in .env shadows config.yaml — run 'hermes doctor --fix'") elif remove_env_value("HERMES_MAX_ITERATIONS"): - check_ok( - "Removed stale HERMES_MAX_ITERATIONS from .env " - f"(config.yaml agent.max_turns={cfg_max_turns} is now authoritative)" - ) + check_ok(f"Removed stale HERMES_MAX_ITERATIONS from .env (config.yaml agent.max_turns={cfg_max_turns} is now authoritative)") f.fixed += 1 else: check_warn("Could not remove HERMES_MAX_ITERATIONS from .env") - f.manual_issues.append( - "Manually delete the HERMES_MAX_ITERATIONS line from " - f"{_DHH}/.env — config.yaml agent.max_turns is authoritative." - ) + f.manual_issues.append(f"Manually delete the HERMES_MAX_ITERATIONS line from {_DHH}/.env — config.yaml agent.max_turns is authoritative.") def _drift_deprecations(f: Finding, should_fix: bool, config_path) -> None: - """Warn-only deprecation sweep over the raw file + on-disk .env (bridged - process env like TERMINAL_CWD would false-positive).""" + """Warn-only deprecation sweep over the raw file + on-disk .env (process env would false-positive).""" from hermes_cli.config import load_env, read_user_config_raw raw = read_user_config_raw(config_path) if config_path is not None else {} try: diff --git a/hermes_cli/doctor_connectivity.py b/hermes_cli/doctor_connectivity.py index 774bdaac49..c04eb2634d 100644 --- a/hermes_cli/doctor_connectivity.py +++ b/hermes_cli/doctor_connectivity.py @@ -1,13 +1,13 @@ """API connectivity probes for ``hermes doctor`` (split out of ``doctor.py``). -Every probe is a pure function: takes its inputs, makes one HTTP/SDK call and -returns a ``ProbeResult`` carrying the row(s) to print and issue strings to -append. No printing inside workers — the caller prints in submission order. +Every probe is a pure function: one HTTP/SDK call returning a ``ProbeResult`` with the row(s) to +print and issue strings to append. No printing inside workers — the caller prints in submission order. """ from __future__ import annotations import concurrent.futures +import functools import os import sys from typing import NamedTuple @@ -40,9 +40,8 @@ def _skip(name: str) -> ProbeResult: def _has_healthy_oauth_fallback_for_apikey_provider(provider_label: str) -> bool: - """True when a failed direct API-key probe is non-blocking because the same - provider family's OAuth runtime path is already healthy: the failed row is - still shown, but not promoted into the final blocking summary.""" + """True when a failed direct API-key probe is non-blocking because the same provider family's OAuth + runtime path is already healthy: the failed row is still shown, but not promoted into the summary.""" normalized = (provider_label or "").strip().lower() getter = {"minimax": "get_minimax_oauth_auth_status", "xai": "get_xai_oauth_auth_status"}.get(normalized) if not getter: @@ -84,18 +83,14 @@ def _build_apikey_providers_list() -> list: ("OpenCode Go", ("OPENCODE_GO_API_KEY",), None, "OPENCODE_GO_BASE_URL", False), ] _known_names = {t[0] for t in _static} - # Canonical profile names of the static rows, so profiles without a - # display_name don't duplicate providers already listed above. - _known_canonical = { - "zai", "kimi-coding", "stepfun", "kimi-coding-cn", "arcee", "gmi", "deepseek", - "huggingface", "nvidia", "alibaba", "minimax", "minimax-cn", "ai-gateway", - "kilocode", "opencode-zen", "opencode-go", - } - # Providers with a dedicated health check (custom headers/auth). Skip their - # pluggable profiles so the generic Bearer-auth loop doesn't run a duplicate, - # broken check (e.g. Anthropic native API requires x-api-key, not Bearer). + # Providers with a dedicated health check (custom headers/auth): skip their pluggable profiles so + # the generic Bearer loop doesn't run a duplicate, broken check (Anthropic needs x-api-key). _dedicated_canonical = {"anthropic", "openrouter", "bedrock"} - _known_canonical.update(_dedicated_canonical) + # Canonical profile names of the static rows, so profiles without a display_name don't duplicate. + _known_canonical = { + "zai", "kimi-coding", "stepfun", "kimi-coding-cn", "arcee", "gmi", "deepseek", "huggingface", "nvidia", + "alibaba", "minimax", "minimax-cn", "ai-gateway", "kilocode", "opencode-zen", "opencode-go", + } | _dedicated_canonical try: from providers import list_providers from providers.base import ProviderProfile as _PP @@ -126,6 +121,18 @@ def _build_apikey_providers_list() -> list: return _static +# HTTP status -> (detail, issue) for the OpenRouter probe; anything else is a generic HTTP failure. +_OPENROUTER_STATUS = { + 401: ("(invalid API key)", "Check OPENROUTER_API_KEY in .env"), + 402: ("(out of credits — payment required)", + "OpenRouter account has insufficient credits. " + "Fix: run 'hermes config set model.provider ' " + "to switch providers, or fund your OpenRouter account " + "at https://openrouter.ai/settings/credits"), + 429: ("(rate limited)", "OpenRouter rate limit hit — consider switching to a different provider or waiting"), +} + + def _probe_openrouter() -> ProbeResult: name = "OpenRouter API" key = os.getenv("OPENROUTER_API_KEY") @@ -138,18 +145,8 @@ def _probe_openrouter() -> ProbeResult: return _row(name, "fail", f"({e})", ["Check network connectivity"]) if r.status_code == 200: return _row(name, "ok") - if r.status_code == 401: - return _row(name, "fail", "(invalid API key)", ["Check OPENROUTER_API_KEY in .env"]) - if r.status_code == 402: - return _row(name, "fail", "(out of credits — payment required)", [ - "OpenRouter account has insufficient credits. " - "Fix: run 'hermes config set model.provider ' " - "to switch providers, or fund your OpenRouter account " - "at https://openrouter.ai/settings/credits"]) - if r.status_code == 429: - return _row(name, "fail", "(rate limited)", [ - "OpenRouter rate limit hit — consider switching to a different provider or waiting"]) - return _row(name, "fail", f"(HTTP {r.status_code})") + detail, issue = _OPENROUTER_STATUS.get(r.status_code, (f"(HTTP {r.status_code})", None)) + return _row(name, "fail", detail, [issue] if issue else None) def _probe_anthropic() -> ProbeResult: @@ -180,9 +177,7 @@ def _probe_anthropic() -> ProbeResult: return _row(name, "warn", f"({e})") if r.status_code == 200: return _row(name, "ok") - if r.status_code == 401: - return _row(name, "fail", "(invalid API key)") - return _row(name, "warn", "(couldn't verify)") + return _row(name, "fail", "(invalid API key)") if r.status_code == 401 else _row(name, "warn", "(couldn't verify)") def _probe_apikey_provider(pname, env_vars, default_url, base_env, supports_health_check) -> ProbeResult: @@ -195,13 +190,11 @@ def _probe_apikey_provider(pname, env_vars, default_url, base_env, supports_heal try: import httpx base = os.getenv(base_env, "") if base_env else "" - # Auto-detect Kimi Code keys (sk-kimi-) → api.kimi.com/coding/v1 - # (OpenAI-compat surface, which exposes /models for health check). + # Kimi Code keys (sk-kimi-) → api.kimi.com/coding/v1 (OpenAI-compat surface exposing /models). if not base and key.startswith("sk-kimi-"): base = "https://api.kimi.com/coding/v1" - # Anthropic-compat endpoints (/anthropic, api.kimi.com/coding - # with no /v1) don't support /models. Rewrite to OpenAI-compat - # /v1 surface for health checks. + # Anthropic-compat endpoints (/anthropic, api.kimi.com/coding with no /v1) don't support + # /models — rewrite to the OpenAI-compat /v1 surface for health checks. if base and base.rstrip("/").endswith("/anthropic"): from agent.auxiliary_client import _to_openai_base_url base = _to_openai_base_url(base) @@ -211,9 +204,8 @@ def _probe_apikey_provider(pname, env_vars, default_url, base_env, supports_heal headers = {"Authorization": f"Bearer {key}", "User-Agent": _HERMES_USER_AGENT} if base_url_host_matches(base, "api.kimi.com"): headers["User-Agent"] = "claude-code/0.1.0" - # Google's Generative Language API rejects ``Authorization: Bearer - # `` with 401 ``ACCESS_TOKEN_TYPE_UNSUPPORTED`` — that header is - # reserved for OAuth 2 access tokens; plain keys use ``x-goog-api-key``. + # Google's Generative Language API rejects ``Authorization: Bearer `` with 401 + # ACCESS_TOKEN_TYPE_UNSUPPORTED (reserved for OAuth 2 tokens); plain keys use ``x-goog-api-key``. if url and base_url_host_matches(url, "generativelanguage.googleapis.com"): headers.pop("Authorization", None) headers["x-goog-api-key"] = key @@ -254,8 +246,7 @@ def _probe_bedrock() -> ProbeResult: return _row(name, "warn", f"(boto3 not installed — {pip})", [f"Install boto3 for Bedrock: {pip}"], label=label) except Exception as e: err_name = type(e).__name__ - return _row(name, "warn", f"({err_name}: {e})", - [f"AWS Bedrock: {err_name} — check IAM permissions for bedrock:ListFoundationModels"], label=label) + return _row(name, "warn", f"({err_name}: {e})", [f"AWS Bedrock: {err_name} — check IAM permissions for bedrock:ListFoundationModels"], label=label) def _probe_azure_entra() -> ProbeResult: @@ -281,10 +272,7 @@ def _probe_azure_entra() -> ProbeResult: try: from agent.azure_identity_adapter import ( - EntraIdentityConfig, - SCOPE_AI_AZURE_DEFAULT, - describe_active_credential, - has_azure_identity_installed, + EntraIdentityConfig, SCOPE_AI_AZURE_DEFAULT, describe_active_credential, has_azure_identity_installed, ) except Exception as exc: return _row(name, "warn", f"(adapter import failed: {exc})", [f"Azure Foundry adapter import failed: {exc}"], label=label) @@ -301,10 +289,7 @@ def _probe_azure_entra() -> ProbeResult: tag = ", ".join(env_sources) if env_sources else "default credential chain" return _row(name, "ok", f"({tag}, scope={scope})", label=label) err = info.get("error") or "credential chain exhausted" - hint = info.get("hint") or ( - "Run `az login`, set AZURE_TENANT_ID/AZURE_CLIENT_ID/" - "AZURE_CLIENT_SECRET, or attach a managed identity to this VM." - ) + hint = info.get("hint") or "Run `az login`, set AZURE_TENANT_ID/AZURE_CLIENT_ID/AZURE_CLIENT_SECRET, or attach a managed identity to this VM." return _row(name, "warn", f"({err})", [f"Azure Foundry Entra: {err}. {hint}"], label=label) @@ -313,14 +298,12 @@ def build_probes() -> list: global _APIKEY_PROVIDERS_CACHE if _APIKEY_PROVIDERS_CACHE is None: _APIKEY_PROVIDERS_CACHE = _build_apikey_providers_list() - probes = [("OpenRouter API", _probe_openrouter), ("Anthropic API", _probe_anthropic)] - for _pname, _env_vars, _default_url, _base_env, _supports in _APIKEY_PROVIDERS_CACHE: - # Bind loop vars via default args so every closure keeps its own provider. - probes.append((_pname, lambda p=_pname, e=_env_vars, u=_default_url, b=_base_env, s=_supports: - _probe_apikey_provider(p, e, u, b, s))) - probes.append(("AWS Bedrock", _probe_bedrock)) - probes.append(("Azure Foundry (Entra ID)", _probe_azure_entra)) - return probes + return [ + ("OpenRouter API", _probe_openrouter), ("Anthropic API", _probe_anthropic), + # functools.partial binds each row's args so every callable keeps its own provider. + *((row[0], functools.partial(_probe_apikey_provider, *row)) for row in _APIKEY_PROVIDERS_CACHE), + ("AWS Bedrock", _probe_bedrock), ("Azure Foundry (Entra ID)", _probe_azure_entra), + ] def run_probes(probes: list) -> list: diff --git a/hermes_cli/doctor_live.py b/hermes_cli/doctor_live.py index 9b2807ff88..330fb9648f 100644 --- a/hermes_cli/doctor_live.py +++ b/hermes_cli/doctor_live.py @@ -14,19 +14,17 @@ from hermes_cli.doctor import _section, check_fail, check_info, check_ok, check_ DEFAULT_PROBE_TIMEOUT = 10.0 -# Metadata-only endpoints. None of these spend generation credits. -FIRECRAWL_HEALTH_URL = "https://api.firecrawl.dev/v2/team/credit-usage" -FAL_MODELS_URL = "https://fal.ai/api/models?page=1" -OPENAI_MODELS_URL = "https://api.openai.com/v1/models" -GROQ_MODELS_URL = "https://api.groq.com/openai/v1/models" -ELEVENLABS_VOICES_URL = "https://api.elevenlabs.io/v1/voices" - +# Metadata-only endpoints (none spend generation credits): name -> (url, env var, auth scheme). +_KEYED_PROBES = { + "Firecrawl": ("https://api.firecrawl.dev/v2/team/credit-usage", "FIRECRAWL_API_KEY", "Bearer"), + "FAL": ("https://fal.ai/api/models?page=1", "FAL_KEY", "Key"), +} # TTS/STT providers that never touch the network (nothing to probe). _LOCAL_AUDIO_PROVIDERS = {"", "local", "edge", "neutts", "kittentts", "piper"} _AUDIO_PROBES = { - "openai": (OPENAI_MODELS_URL, "OPENAI_API_KEY", "Bearer"), - "groq": (GROQ_MODELS_URL, "GROQ_API_KEY", "Bearer"), - "elevenlabs": (ELEVENLABS_VOICES_URL, "ELEVENLABS_API_KEY", "xi"), + "openai": ("https://api.openai.com/v1/models", "OPENAI_API_KEY", "Bearer"), + "groq": ("https://api.groq.com/openai/v1/models", "GROQ_API_KEY", "Bearer"), + "elevenlabs": ("https://api.elevenlabs.io/v1/voices", "ELEVENLABS_API_KEY", "xi"), } @@ -89,11 +87,8 @@ def _browser_available() -> bool: def _launch_browser_probe(timeout: float) -> tuple: - """Launch a browser, open about:blank, close. Returns (ok, detail). - - Uses Playwright directly (what agent-browser drives underneath) so the probe owns the full - lifecycle and always cleans up. - """ + """Launch a browser, open about:blank, close. Returns (ok, detail). Uses Playwright directly (what + agent-browser drives underneath) so the probe owns the full lifecycle and always cleans up.""" try: from playwright.sync_api import sync_playwright except ImportError: @@ -191,11 +186,8 @@ def _run_one(name: str, fn: Callable[[], ProbeResult], issues: List[str]) -> Pro def run_live_checks(issues: List[str]) -> List[ProbeResult]: - """Run one bounded, read-only probe per configured tool backend. - - Sequential by design (bounded, predictable output ordering). Appends a remediation line to - ``issues`` for each failed probe. Skipped backends never fail and never append issues. - """ + """Run one bounded, read-only probe per configured tool backend — sequential by design (predictable output + ordering). Appends a remediation line to ``issues`` per failed probe; skipped backends never append.""" config = _load_config() try: timeout = float((config.get("doctor") or {}).get("live_probe_timeout", DEFAULT_PROBE_TIMEOUT)) @@ -205,10 +197,10 @@ def run_live_checks(issues: List[str]) -> List[ProbeResult]: _section("Live Backend Probes (opt-in, real calls)") results: List[ProbeResult] = [ - _run_one("Firecrawl", lambda: _keyed_probe("Firecrawl", FIRECRAWL_HEALTH_URL, "FIRECRAWL_API_KEY", "Bearer", timeout), issues), - _run_one("FAL", lambda: _keyed_probe("FAL", FAL_MODELS_URL, "FAL_KEY", "Key", timeout), issues), - _run_one("Browser", lambda: _probe_browser(timeout), issues), + _run_one(name, lambda n=name, spec=spec: _keyed_probe(n, *spec, timeout), issues) + for name, spec in _KEYED_PROBES.items() ] + results.append(_run_one("Browser", lambda: _probe_browser(timeout), issues)) servers = config.get("mcp_servers") or {} if isinstance(servers, dict) and servers: @@ -232,11 +224,8 @@ def run_live_checks(issues: List[str]) -> List[ProbeResult]: def maybe_run_live_checks(args, issues: List[str]): - """Entry point called from ``run_doctor`` after the static checks. - - No-ops (returns None) unless the user explicitly passed ``--live``. A crash anywhere in the live - subsystem must never break doctor. - """ + """Called from ``run_doctor`` after the static checks; no-op (None) unless ``--live`` was passed. + A crash anywhere in the live subsystem must never break doctor.""" if not getattr(args, "live", False): return None try: diff --git a/hermes_cli/doctor_platform.py b/hermes_cli/doctor_platform.py index 085ea23cc0..62e42f77dd 100644 --- a/hermes_cli/doctor_platform.py +++ b/hermes_cli/doctor_platform.py @@ -60,8 +60,7 @@ def _unreadable_reason(db_path: Path) -> str: """Explain why a database file could not be read, without opening it. ``read_header_bytes_preopen`` collapses every ``OSError`` into ``None``, but doctor must say *which* - problem it hit. ``stat()`` and ``access()`` answer that from directory metadata alone — neither takes - a file descriptor, so neither can cancel the file's POSIX advisory locks. + problem it hit. ``stat()``/``access()`` answer from directory metadata alone — no descriptor, no lock loss. """ try: db_path.stat() @@ -74,10 +73,9 @@ def _read_journal_mode(db_path: Path) -> tuple[str | None, str | None]: """Return (journal mode, error) from header byte 18 (2=WAL, 1=rollback) without opening the database. Opening through SQLite — even read-only — creates -wal/-shm sidecars, which a diagnostic must not do. - The read goes through ``read_header_bytes_preopen`` rather than a bare ``open()``: closing *any* - descriptor cancels this process's POSIX advisory locks (see ``hermes_cli.sqlite_safe_read``), and the - dashboard console runs ``run_doctor`` in-process with live ``SessionDB`` connections — the helper - refuses then and the mode is reported as unreadable. + ``read_header_bytes_preopen`` rather than a bare ``open()``: closing *any* descriptor cancels this + process's POSIX advisory locks (see ``hermes_cli.sqlite_safe_read``), and the dashboard console runs + ``run_doctor`` in-process with live ``SessionDB`` connections — the helper refuses then (unreadable). """ from hermes_cli.sqlite_safe_read import has_live_connection, read_header_bytes_preopen @@ -154,10 +152,8 @@ def _read_pyproject_version() -> str | None: def _check_version_consistency(issues: list[str]) -> None: - """Detect pyproject.toml vs hermes_cli.__version__ drift (a git conflict resolution can revert one but not the other). - - Silent no-op for installed wheels (no pyproject). - """ + """Detect pyproject.toml vs hermes_cli.__version__ drift (a conflict resolution can revert one but not the + other). Silent no-op for installed wheels (no pyproject).""" try: from hermes_cli import __version__ as init_version except Exception: @@ -168,20 +164,15 @@ def _check_version_consistency(issues: list[str]) -> None: if pyproject_version == init_version: check_ok("Version files consistent", f"({init_version})") else: - _fail_and_issue( - "Version mismatch between source files", - f"(pyproject.toml {pyproject_version} != hermes_cli/__init__.py {init_version})", - "Re-sync version files (e.g. run 'hermes update', or set " - "hermes_cli/__init__.py __version__ to match pyproject.toml)", - issues, - ) + _fail_and_issue("Version mismatch between source files", + f"(pyproject.toml {pyproject_version} != hermes_cli/__init__.py {init_version})", + "Re-sync version files (e.g. run 'hermes update', or set " + "hermes_cli/__init__.py __version__ to match pyproject.toml)", issues) def _check_s6_supervision(issues: list[str]) -> None: - """Inside a container under our s6 /init, report static services and per-profile gateway slots that are ``up``. - - Counterpart to :func:`_check_gateway_service_linger` (systemd-on-host); no-op outside the s6 container. - """ + """Under our s6 /init, report static services and per-profile gateway slots that are ``up``; no-op elsewhere. + Counterpart to :func:`_check_gateway_service_linger` (systemd-on-host).""" try: from hermes_cli.service_manager import S6ServiceManager, detect_service_manager except Exception: @@ -192,10 +183,8 @@ def _check_s6_supervision(issues: list[str]) -> None: _section("s6 Supervision") mgr = S6ServiceManager() for static in ("main-hermes", "dashboard"): # s6-rc symlinks under /run/service/, same s6-svstat probe - if mgr.is_running(static): - check_ok(f"{static}: up") - else: - check_info(f"{static}: down (expected if not enabled via env)") + (check_ok if mgr.is_running(static) else check_info)( + f"{static}: up" if mgr.is_running(static) else f"{static}: down (expected if not enabled via env)") profiles = mgr.list_profile_gateways() if not profiles: @@ -209,8 +198,8 @@ def _check_s6_supervision(issues: list[str]) -> None: def check_certificates(should_fix: bool = False, issues: "list | None" = None) -> None: """Verify the certifi CA bundle is loadable before the first HTTPS call tracebacks. - ``--fix`` repairs a broken bundle (e.g. a brew Python upgrade rebuilt the venv) by - force-reinstalling certifi into THIS interpreter's environment and re-verifying. + ``--fix`` repairs a broken bundle (e.g. a brew Python upgrade rebuilt the venv) by force-reinstalling + certifi into THIS interpreter's environment and re-verifying. """ try: from agent.ssl_guard import verify_ca_bundle_with_fallback @@ -271,10 +260,7 @@ def check_certificates(should_fix: bool = False, issues: "list | None" = None) - def _check_gateway_service_linger(issues: list[str]) -> None: - """Warn when a systemd user gateway service will stop after logout. - - Skipped under s6 (no systemd, no logout, no linger concept; ``_check_s6_supervision`` reports that state). - """ + """Warn when a systemd user gateway service will stop after logout (skipped under s6: no linger concept).""" try: from hermes_cli.gateway import get_systemd_linger_status, get_systemd_unit_path, is_linux from hermes_cli.service_manager import detect_service_manager @@ -297,13 +283,12 @@ def _check_gateway_service_linger(issues: list[str]) -> None: def check_macos_tcc_grants() -> None: - """Check macOS TCC grant persistence for a locally-built desktop bundle. + """Check macOS TCC grant persistence for a locally-built desktop bundle; silent on non-macOS / no bundle. TCC keys grants to the app's designated requirement (DR). A cdhash-pinned ad-hoc DR changes on every - rebuild, so grants silently stop matching while the Settings toggle stays ON and macOS re-prompts; - identifier-pinned builds survive rebuilds, but grants made to older binaries stay stale until re-granted - once. TCC.db needs Full Disk Access, so the DR string is the only readable signal — a cdhash anchor is a - proxy for the signing class, not a contract on DR wording. Silent on non-macOS / no bundle. + rebuild, so grants silently stop matching while the Settings toggle stays ON; identifier-pinned builds + survive rebuilds, but grants made to older binaries stay stale until re-granted once. TCC.db needs Full + Disk Access, so the DR string is the only readable signal (a proxy for the signing class, not DR wording). """ from hermes_cli.doctor import _desktop_app_bundle, _macos_desktop_dr if sys.platform != "darwin": @@ -324,14 +309,10 @@ def check_macos_tcc_grants() -> None: "identity, then re-grant permissions once.", ) return - if "certificate" in dr.lower(): # --setup-tcc-identity or notarized build: strongest anchor - check_ok("macOS TCC signing identity is stable", "(certificate-anchored DR; grants survive rebuilds)") - else: - check_ok( - "macOS TCC signing identity is stable", - "(identifier-pinned DR; grants survive rebuilds — for the strongest " - "anchor, see `hermes desktop --setup-tcc-identity`)", - ) + check_ok("macOS TCC signing identity is stable", + # --setup-tcc-identity or notarized build: strongest anchor + "(certificate-anchored DR; grants survive rebuilds)" if "certificate" in dr.lower() else + "(identifier-pinned DR; grants survive rebuilds — for the strongest anchor, see `hermes desktop --setup-tcc-identity`)") check_info( "If macOS still re-prompts for permissions (toggle shows ON): the stored " "grant is stale — run `tccutil reset ScreenCapture com.nousresearch.hermes` " @@ -341,10 +322,10 @@ def check_macos_tcc_grants() -> None: def _desktop_app_bundle() -> Path | None: - """Locate the locally-built desktop bundle (``apps/desktop/release/mac-/Hermes.app``), newest arch tree first. + """Locate the locally-built desktop bundle (``apps/desktop/release/mac-/Hermes.app``), newest first. - That is the only layout whose ad-hoc re-signed bundle can invalidate TCC grants. ``/Applications/Hermes.app`` - is deliberately not probed: it is the separately-signed, certificate-anchored Hermes-Setup launcher. + The only layout whose ad-hoc re-signed bundle can invalidate TCC grants. ``/Applications/Hermes.app`` is + deliberately not probed: it is the separately-signed, certificate-anchored Hermes-Setup launcher. """ release_dir = Path(__file__).resolve().parents[1] / "apps" / "desktop" / "release" candidates = [p for p in release_dir.glob("mac*/Hermes.app") if p.is_dir()] @@ -365,9 +346,7 @@ def _macos_desktop_dr(app: Path) -> str | None: def check_macos_tcc_anchor(should_fix: bool = False) -> None: """Report (and with --fix install) the dylib-complete TCC anchor; silent on non-macOS / non-uv interpreters. - - Never raises. Install is gated by the module's pre-install boot probe, so ``--fix`` cannot brick the CLI. - """ + Never raises. Install is gated by the module's pre-install boot probe, so ``--fix`` cannot brick the CLI.""" try: from hermes_cli import macos_tcc_anchor as tcc @@ -391,8 +370,7 @@ def check_macos_full_disk_access() -> None: """One-grant guidance: Full Disk Access silences every per-folder TCC prompt. Silent on non-macOS. Probe: listdir of ``~/Library/Application Support/com.apple.TCC`` — FDA-gated, and probing it does NOT - trigger a prompt (prompts fire for protected-CATEGORY paths like Desktop; the TCC dir just returns EPERM). - A missing dir / other error is indeterminate, so stay silent rather than nag. + trigger a prompt (the TCC dir just returns EPERM). A missing dir / other error is indeterminate: stay silent. """ if sys.platform != "darwin": return @@ -448,26 +426,24 @@ def _check_python_environment(should_fix: bool) -> Finding: f = Finding() v = sys.version_info label = f"Python {v.major}.{v.minor}.{v.micro}" - if v >= (3, 11): + if v >= (3, 10): check_ok(label) - elif v >= (3, 10): - check_ok(label) - check_warn("Python 3.11+ recommended for RL Training tools (tinker requires >= 3.11)") + if v < (3, 11): + check_warn("Python 3.11+ recommended for RL Training tools (tinker requires >= 3.11)") elif v >= (3, 8): check_warn(label, "(3.10+ recommended)") else: _fail_and_issue(label, "(3.10+ required)", "Upgrade Python to 3.10+", f.issues) - # Linked SQLite: version + source id matter independently of the Python minor - # (uv's python-build-standalone can keep a vulnerable SQLite across upgrades). + # Linked SQLite: version + source id matter independently of the Python minor (uv's + # python-build-standalone can keep a vulnerable SQLite across upgrades). try: import sqlite3 from hermes_state import is_sqlite_wal_reset_vulnerable, sqlite_source_id src = sqlite_source_id() if is_sqlite_wal_reset_vulnerable(): - # Warn-only: Hermes already refuses to enable WAL on fresh DBs, and - # runtime repair is best-effort, so this never goes into ``issues``. + # Warn-only: Hermes already refuses WAL on fresh DBs and runtime repair is best-effort. check_warn(f"SQLite {sqlite3.sqlite_version} (WAL-reset bug)", _sqlite_upgrade_hint()) else: check_ok(f"SQLite {sqlite3.sqlite_version}") @@ -491,21 +467,25 @@ def _check_certificates(should_fix: bool) -> Finding: return f +# (import name, display name, optional) +_PACKAGES = ( + ("openai", "OpenAI SDK", False), ("rich", "Rich (terminal UI)", False), ("dotenv", "python-dotenv", False), + ("yaml", "PyYAML", False), ("httpx", "HTTPX", False), + ("croniter", "Croniter (cron expressions)", True), ("telegram", "python-telegram-bot", True), ("discord", "discord.py", True), +) + + def _check_required_packages(should_fix: bool) -> Finding: f = Finding() - for module, name in (("openai", "OpenAI SDK"), ("rich", "Rich (terminal UI)"), ("dotenv", "python-dotenv"), - ("yaml", "PyYAML"), ("httpx", "HTTPX")): + for module, name, optional in _PACKAGES: try: __import__(module) - check_ok(name) + check_ok(name, "(optional)" if optional else "") except ImportError: - _fail_and_issue(name, "(missing)", f"Install {name}: {_python_install_cmd()} {module}", f.issues) - for module, name in (("croniter", "Croniter (cron expressions)"), ("telegram", "python-telegram-bot"), ("discord", "discord.py")): - try: - __import__(module) - check_ok(name, "(optional)") - except ImportError: - check_warn(name, "(optional, not installed)") + if optional: + check_warn(name, "(optional, not installed)") + else: + _fail_and_issue(name, "(missing)", f"Install {name}: {_python_install_cmd()} {module}", f.issues) return f @@ -548,9 +528,7 @@ def _check_command_installation(should_fix: bool) -> Finding: f.issues.append(f"Broken symlink at {display}/hermes — run 'hermes doctor --fix'") return f link.unlink() - link.symlink_to(venv_bin) - check_ok(f"Fixed symlink: {display}/hermes → {venv_bin}") - f.fixed += 1 + _link_venv(f, link, venv_bin, f"Fixed symlink: {display}/hermes → {venv_bin}") elif link.exists(): # regular file (wrapper script), not a symlink check_ok(f"{display}/hermes exists (non-symlink)") else: @@ -559,10 +537,14 @@ def _check_command_installation(should_fix: bool) -> Finding: f.issues.append(f"Missing {display}/hermes symlink — run 'hermes doctor --fix'") return f link_dir.mkdir(parents=True, exist_ok=True) - link.symlink_to(venv_bin) - check_ok(f"Created symlink: {display}/hermes → {venv_bin}") - f.fixed += 1 + _link_venv(f, link, venv_bin, f"Created symlink: {display}/hermes → {venv_bin}") if str(link_dir) not in os.environ.get("PATH", "").split(os.pathsep): check_warn(f"{display} is not on your PATH", "(add it to your shell config: export PATH=\"$HOME/.local/bin:$PATH\")") f.manual_issues.append(f"Add {display} to your PATH") return f + + +def _link_venv(f: Finding, link: Path, venv_bin: Path, msg: str) -> None: + link.symlink_to(venv_bin) + check_ok(msg) + f.fixed += 1 diff --git a/hermes_cli/doctor_state.py b/hermes_cli/doctor_state.py index c79f3aa6ac..54e7922480 100644 --- a/hermes_cli/doctor_state.py +++ b/hermes_cli/doctor_state.py @@ -9,7 +9,7 @@ from __future__ import annotations import subprocess from pathlib import Path from hermes_cli.doctor_report import ( - Finding, _fail_and_issue, _section, check_info, check_ok, check_warn, doctor_check, ensure_dir, + Finding, _fail_and_issue, _section, check_bool, check_info, check_ok, check_warn, doctor_check, ensure_dir, ) from hermes_cli.sizefmt import format_bytes as _human_bytes @@ -28,11 +28,10 @@ def _honcho_is_configured_for_doctor() -> bool: def _doctor_memory_config(hermes_home: Path | None = None) -> dict: """Return the effective memory section used by doctor diagnostics.""" from hermes_cli.doctor import HERMES_HOME - home = hermes_home if hermes_home is not None else HERMES_HOME try: from hermes_cli.config import _expand_env_vars, read_user_config_raw - config_path = home / "config.yaml" + config_path = (hermes_home if hermes_home is not None else HERMES_HOME) / "config.yaml" if not config_path.exists(): return {} config = _expand_env_vars(read_user_config_raw(config_path)) @@ -173,11 +172,9 @@ def _check_directory_structure(should_fix: bool) -> Finding: check_ok(f"{_DHH}/memories/ directory exists") for enabled, fname in ((_memory_enabled, "MEMORY.md"), (_user_profile_enabled, "USER.md")): mem_file = memories_dir / fname - if not enabled: - continue - if mem_file.exists(): + if enabled and mem_file.exists(): check_ok(f"{fname} exists ({len(mem_file.read_text(encoding='utf-8').strip())} chars)") - else: + elif enabled: check_info(f"{fname} not created yet (will be created when the agent first writes a memory)") else: check_warn(f"{_DHH}/memories/ not found", "(will be created on first use)") @@ -214,10 +211,9 @@ def _repair_state_db(f: Finding, should_fix: bool, state_db_path: Path, *, return if callable(ok_label): try: - count = _session_count(state_db_path) + ok_label = ok_label(_session_count(state_db_path)) except Exception: - count = "?" - ok_label = ok_label(count) + ok_label = ok_label("?") backup_name = Path(report["backup_path"]).name if report.get("backup_path") else "n/a" check_ok(ok_label, f"(strategy: {report.get('strategy')}; backup: {backup_name})") f.fixed += 1 @@ -337,7 +333,9 @@ def _check_skills_hub(should_fix: bool) -> Finding: from hermes_cli.doctor import HERMES_HOME, _DHH f = Finding() hub_dir = HERMES_HOME / "skills" / ".hub" - if hub_dir.exists(): + if not hub_dir.exists(): + check_warn("Skills Hub directory not initialized", "(run: hermes skills list)") + else: check_ok("Skills Hub directory exists") lock_file = hub_dir / "lock.json" if lock_file.exists(): @@ -351,17 +349,14 @@ def _check_skills_hub(should_fix: bool) -> Finding: q_count = sum(1 for d in quarantine.iterdir() if d.is_dir()) if quarantine.exists() else 0 if q_count > 0: check_warn(f"{q_count} skill(s) in quarantine", "(pending review)") - else: - check_warn("Skills Hub directory not initialized", "(run: hermes skills list)") from hermes_cli.config import get_env_value if get_env_value("GITHUB_TOKEN") or get_env_value("GH_TOKEN"): check_ok("GitHub token configured (authenticated API access)") - elif _gh_authenticated(): - check_ok("GitHub authenticated via gh CLI", "(full API access — no GITHUB_TOKEN needed)") else: - check_warn("No GITHUB_TOKEN", f"(60 req/hr rate limit — set in {_DHH}/.env for better rates)") + check_bool(_gh_authenticated(), ("GitHub authenticated via gh CLI", "(full API access — no GITHUB_TOKEN needed)"), + ("No GITHUB_TOKEN", f"(60 req/hr rate limit — set in {_DHH}/.env for better rates)")) return f @@ -371,11 +366,9 @@ def _memory_provider_honcho(issues: list) -> None: cfg_path = resolve_config_path() if not cfg_path.exists(): # Config file missing — env-var fallback may still have resolved it. - if hcfg.api_key or hcfg.base_url: - check_ok("Honcho configured via environment variables", - f"config file {cfg_path} not found, using HONCHO_API_KEY env var") - else: - check_warn("Honcho config not found", "run: hermes memory setup") + check_bool(hcfg.api_key or hcfg.base_url, + ("Honcho configured via environment variables", f"config file {cfg_path} not found, using HONCHO_API_KEY env var"), + ("Honcho config not found", "run: hermes memory setup")) elif not hcfg.enabled: check_info(f"Honcho disabled (set enabled: true in {cfg_path} to activate)") elif not (hcfg.api_key or hcfg.base_url): @@ -460,10 +453,9 @@ def _check_profiles(should_fix: bool, f: Finding) -> None: parts.append("gateway running") if p.model: parts.append(p.model[:30]) - if not (p.path / "config.yaml").exists(): - parts.append("⚠ missing config") - if not (p.path / ".env").exists(): - parts.append("no .env") + for missing, text in (("config.yaml", "⚠ missing config"), (".env", "no .env")): + if not (p.path / missing).exists(): + parts.append(text) if not (wrapper_dir / p.name).exists(): parts.append("no alias") check_ok(f" {p.name}: {', '.join(parts) if parts else 'configured'}") diff --git a/hermes_cli/doctor_tools.py b/hermes_cli/doctor_tools.py index 14433a1289..5734daacde 100644 --- a/hermes_cli/doctor_tools.py +++ b/hermes_cli/doctor_tools.py @@ -72,8 +72,7 @@ def _doctor_web_capability_rows() -> list[tuple[str, str, str]]: from agent.web_search_registry import get_active_extract_provider, get_active_search_provider from tools.web_tools import _ensure_web_plugins_loaded, _provider_is_ready - # Doctor is a fresh process: bundled web providers only register during plugin - # discovery, which nothing has triggered yet (idempotent, cheap on rerun). + # Fresh process: bundled web providers only register during plugin discovery (idempotent, cheap). _ensure_web_plugins_loaded() except Exception: return rows @@ -87,10 +86,8 @@ def _doctor_web_capability_rows() -> list[tuple[str, str, str]]: rows.append(("warn", capability, "(no provider selected or registered)")) continue name = getattr(provider, "name", None) or type(provider).__name__ - if _provider_is_ready(provider): - rows.append(("ok", capability, f"({name})")) - else: - rows.append(("warn", capability, f"({name} selected; provider not configured)")) + rows.append(("ok", capability, f"({name})") if _provider_is_ready(provider) + else ("warn", capability, f"({name} selected; provider not configured)")) return rows @@ -149,10 +146,9 @@ def _check_docker_backend(terminal_env: str, running_in_container: bool, issues: if terminal_env == "docker": if not _safe_which("docker"): _fail_and_issue("docker not found", "(required for TERMINAL_ENV=docker)", "Install Docker or change TERMINAL_ENV", issues) - elif _run_ok(["docker", "info"], timeout=10): - check_ok("docker", "(daemon running)") else: - _fail_and_issue("docker daemon not running", "", "Start Docker daemon", issues) + _require(_run_ok(["docker", "info"], timeout=10), ("docker", "(daemon running)"), ("docker daemon not running", ""), + "Start Docker daemon", issues) elif _safe_which("docker"): check_ok("docker", "(optional)") elif _is_termux(): @@ -173,17 +169,19 @@ def _check_ssh_backend(issues: list[str]) -> None: if ssh_key: cmd += ["-i", os.path.expanduser(ssh_key)] cmd += [f"{ssh_user}@{ssh_host}" if ssh_user else ssh_host, "echo ok"] - if _run_ok(cmd, timeout=15, text=True, encoding='utf-8', errors='replace'): - check_ok(f"SSH connection to {ssh_host}") - else: - _fail_and_issue(f"SSH connection to {ssh_host}", "", f"Check SSH configuration for {ssh_host}", issues) + _require(_run_ok(cmd, timeout=15, text=True, encoding='utf-8', errors='replace'), + f"SSH connection to {ssh_host}", (f"SSH connection to {ssh_host}", ""), f"Check SSH configuration for {ssh_host}", issues) + + +def _require(cond, ok, bad, issue: str, issues: list[str]) -> None: + """``check_ok(*ok)`` when *cond*, else ``check_fail(*bad)`` and record *issue*.""" + if not check_bool(cond, ok, bad, fail=True): + issues.append(issue) def _check_daytona_backend(issues: list[str]) -> None: - if os.getenv("DAYTONA_API_KEY"): - check_ok("Daytona API key", "(configured)") - else: - _fail_and_issue("DAYTONA_API_KEY not set", "(required for TERMINAL_ENV=daytona)", "Set DAYTONA_API_KEY environment variable", issues) + _require(os.getenv("DAYTONA_API_KEY"), ("Daytona API key", "(configured)"), + ("DAYTONA_API_KEY not set", "(required for TERMINAL_ENV=daytona)"), "Set DAYTONA_API_KEY environment variable", issues) try: from daytona import Daytona # noqa: F401 — SDK presence check check_ok("daytona SDK", "(installed)") @@ -194,23 +192,15 @@ def _check_daytona_backend(issues: list[str]) -> None: def _check_vercel_backend(issues: list[str]) -> None: from tools.terminal_tool import _SUPPORTED_VERCEL_RUNTIMES runtime = os.getenv("TERMINAL_VERCEL_RUNTIME", "node24").strip() or "node24" - if runtime in _SUPPORTED_VERCEL_RUNTIMES: - check_ok("Vercel runtime", f"({runtime})") - else: - supported = ", ".join(_SUPPORTED_VERCEL_RUNTIMES) - _fail_and_issue("Vercel runtime unsupported", f"({runtime}; use {supported})", f"Set TERMINAL_VERCEL_RUNTIME to one of: {supported}", issues) - - if os.getenv("TERMINAL_CONTAINER_DISK", "51200").strip() in {"", "0", "51200"}: - check_ok("Vercel disk setting", "(uses platform default)") - else: - _fail_and_issue("Vercel custom disk unsupported", "(reset terminal.container_disk to 51200)", - "Vercel Sandbox does not support custom container_disk; use the shared default 51200", issues) - - if importlib.util.find_spec("vercel") is not None: - check_ok("vercel SDK", "(installed)") - else: - _fail_and_issue("vercel SDK not installed", "(pip install 'hermes-agent[vercel]')", - "Install the Vercel optional dependency: pip install 'hermes-agent[vercel]'", issues) + supported = ", ".join(_SUPPORTED_VERCEL_RUNTIMES) + _require(runtime in _SUPPORTED_VERCEL_RUNTIMES, ("Vercel runtime", f"({runtime})"), + ("Vercel runtime unsupported", f"({runtime}; use {supported})"), f"Set TERMINAL_VERCEL_RUNTIME to one of: {supported}", issues) + _require(os.getenv("TERMINAL_CONTAINER_DISK", "51200").strip() in {"", "0", "51200"}, + ("Vercel disk setting", "(uses platform default)"), ("Vercel custom disk unsupported", "(reset terminal.container_disk to 51200)"), + "Vercel Sandbox does not support custom container_disk; use the shared default 51200", issues) + _require(importlib.util.find_spec("vercel") is not None, ("vercel SDK", "(installed)"), + ("vercel SDK not installed", "(pip install 'hermes-agent[vercel]')"), + "Install the Vercel optional dependency: pip install 'hermes-agent[vercel]'", issues) auth_status = describe_vercel_auth() if auth_status.ok: @@ -222,11 +212,9 @@ def _check_vercel_backend(issues: list[str]) -> None: "Configure Vercel Sandbox auth with VERCEL_TOKEN, VERCEL_PROJECT_ID, and VERCEL_TEAM_ID", issues) for line in auth_status.detail_lines: check_info(f"Vercel auth {line}") - - if os.getenv("TERMINAL_CONTAINER_PERSISTENT", "true").lower() in {"1", "true", "yes", "on"}: - check_info("Vercel persistence: snapshot filesystem only; live processes do not survive sandbox recreation") - else: - check_info("Vercel persistence: ephemeral filesystem") + persistent = os.getenv("TERMINAL_CONTAINER_PERSISTENT", "true").lower() in {"1", "true", "yes", "on"} + check_info("Vercel persistence: snapshot filesystem only; live processes do not survive sandbox recreation" + if persistent else "Vercel persistence: ephemeral filesystem") def _check_plugin_backend(terminal_env: str, issues: list[str]) -> None: @@ -244,10 +232,7 @@ def _check_plugin_backend(terminal_env: str, issues: list[str]) -> None: "Fix terminal.backend in config.yaml, or install/enable the plugin that provides it", issues) return for ok, label, detail in provider.doctor_checks(): - if ok: - check_ok(label, detail) - else: - _fail_and_issue(label, detail, detail.strip("()"), issues) + _require(ok, (label, detail), (label, detail), detail.strip("()"), issues) _BACKEND_CHECKS = {"ssh": _check_ssh_backend, "daytona": _check_daytona_backend, "vercel_sandbox": _check_vercel_backend} @@ -263,9 +248,8 @@ def _check_terminal_backend(should_fix: bool) -> Finding: except Exception: running_in_container = False - # Inside our container docker-in-docker isn't set up, so the local backend is the - # intended one: skip the noisy "docker not found" warning. An explicit - # TERMINAL_ENV=docker (user likely mounted docker.sock) still gets the normal check. + # Inside our container docker-in-docker isn't set up, so the local backend is the intended one: skip the + # noisy "docker not found" warning. An explicit TERMINAL_ENV=docker (mounted docker.sock) still gets checked. if running_in_container and terminal_env != "docker": check_info("Running inside a container — using local terminal backend (docker-in-docker is not configured by default)") terminal_env = "local" @@ -280,9 +264,8 @@ def _check_terminal_backend(should_fix: bool) -> Finding: def _check_agent_browser(should_fix: bool) -> bool: """agent-browser resolution; returns True when browser tools will find a usable install. - Mirrors ``tools.browser_tool._find_agent_browser``'s own cascade (it resolves lazily via - npx or a global/Hermes-managed install) so doctor can't diverge from what the tools find; - validate=False keeps this a cheap existence check with no subprocess or install side effects. + Mirrors ``tools.browser_tool._find_agent_browser``'s own cascade (lazy npx or a global/Hermes-managed + install) so doctor can't diverge from the tools; validate=False keeps it a cheap, side-effect-free check. """ try: from tools.browser_tool import _find_agent_browser, _is_npx_agent_browser_sentinel @@ -293,20 +276,16 @@ def _check_agent_browser(should_fix: bool) -> bool: if resolved and _is_npx_agent_browser_sentinel(resolved): check_ok("agent-browser", "(resolves via npx on first use)") if should_fix: - # Can't tell from here whether npx's cache is warm — fire the same warm-up - # `hermes update` does so the first browser call doesn't pay the registry fetch. + # Can't tell whether npx's cache is warm — fire the same warm-up `hermes update` does. from tools.browser_tool import warm_agent_browser_npx_cache - if warm_agent_browser_npx_cache(): - check_info(" Warmed npx cache for agent-browser") - else: - check_info(" Could not warm npx cache (offline or npx unavailable)") + check_info(" Warmed npx cache for agent-browser" if warm_agent_browser_npx_cache() + else " Could not warm npx cache (offline or npx unavailable)") return True if resolved and agent_browser_runnable(resolved): check_ok("agent-browser", "(browser automation)") return True if resolved: - # Almost always a dangling global symlink left by agent-browser's npm - # postinstall after `hermes update` wiped node_modules. + # Almost always a dangling global symlink left by npm postinstall after `hermes update` wiped node_modules. check_warn("agent-browser found but not runnable", f"(broken symlink at {resolved}? run: npx agent-browser --version)") elif _is_termux(): _termux_browser_hints( @@ -330,9 +309,8 @@ def _termux_browser_hints(*lines: str, node_installed: bool) -> None: def _check_chromium() -> None: """Playwright Chromium presence, using the exact predicate browser_tool uses to hide browser_* tools. - Lazy import: browser_tool is a ~150KB module; if it can't import that is a separate - bug surfaced elsewhere. Camofox, a CDP override, a cloud provider, or Lightpanda all - bypass the local Chromium requirement, so no warning is emitted for them. + Lazy import: browser_tool is ~150KB; an import failure is a separate bug surfaced elsewhere. Camofox, a + CDP override, a cloud provider, or Lightpanda all bypass the local Chromium requirement (no warning). """ from hermes_cli.doctor import PROJECT_ROOT try: @@ -342,12 +320,10 @@ def _check_chromium() -> None: return if _is_camofox_mode() or bool(_get_cdp_override_raw()) or _get_cloud_provider() is not None or _using_lightpanda_engine(): return - if _chromium_installed(): - check_ok("Playwright Chromium", "(browser engine)") - return - check_warn("Playwright Chromium not installed", "(browser_* tools will be hidden from the agent)") - with_deps = "" if sys.platform == "win32" else "--with-deps " - check_info(f"Install with: cd {PROJECT_ROOT} && npx playwright install {with_deps}chromium") + if not check_bool(_chromium_installed(), ("Playwright Chromium", "(browser engine)"), + ("Playwright Chromium not installed", "(browser_* tools will be hidden from the agent)")): + with_deps = "" if sys.platform == "win32" else "--with-deps " + check_info(f"Install with: cd {PROJECT_ROOT} && npx playwright install {with_deps}chromium") def _check_lightpanda() -> None: @@ -367,10 +343,8 @@ def _check_lightpanda() -> None: if not used: check_warn("browser.engine=lightpanda is shadowed", f"({reason})") check_info("Fix: pick Lightpanda in `hermes tools` → Browser Automation, or set browser.engine: auto") - elif find_lightpanda_binary(): - check_ok("Lightpanda", f"({reason})") - else: - check_warn("Lightpanda selected but binary not found", "(browser tools will fail until it is installed)") + elif not check_bool(find_lightpanda_binary(), ("Lightpanda", f"({reason})"), + ("Lightpanda selected but binary not found", "(browser tools will fail until it is installed)")): check_info(LIGHTPANDA_INSTALL_HINT) @@ -400,10 +374,9 @@ def _plural(n: int) -> str: def _audit_one(npm_bin: str, npm_dir, label: str, audit_extra: list[str], issues: list[str]) -> None: """Run one `npm audit --json` and report; any failure is silently skipped. - Workspace-scoped (`--workspace `) advisories are build-time tooling (esbuild/vite), - not runtime code. `npm audit fix --workspace` crashes on current npm (arborist "edgesOut"), - and the root-level fix can crash on the same tree ("isDescendantOf"), so no manual fix - command is offered for those — they clear via a lockfile bump. + Workspace-scoped (`--workspace `) advisories are build-time tooling (esbuild/vite), not runtime + code. `npm audit fix --workspace` crashes on current npm (arborist "edgesOut") and the root-level fix can + crash on the same tree ("isDescendantOf"), so no manual fix command is offered — they clear via a lockfile bump. """ import json try: @@ -437,10 +410,9 @@ def _audit_one(npm_bin: str, npm_dir, label: str, audit_extra: list[str], issues def _check_npm_audit(should_fix: bool) -> Finding: """npm audit per Node package tree (root, web/ui-tui workspaces, WhatsApp bridge). - PROJECT_ROOT is audited with --workspaces=false so the apps/* glob (Electron, node-pty, ...) - is never resolved for a routine security check; web and ui-tui are audited via --workspace. - The WhatsApp bridge may live under a writable HERMES_HOME mirror rather than the (possibly - read-only) install tree in Docker, so it is resolved through the shared helper. + PROJECT_ROOT is audited with --workspaces=false so the apps/* glob (Electron, node-pty, ...) is never + resolved for a routine check; web and ui-tui via --workspace. The WhatsApp bridge may live under a writable + HERMES_HOME mirror rather than the (possibly read-only) Docker install tree, hence the shared resolver. """ from hermes_cli.doctor import PROJECT_ROOT f = Finding() @@ -457,10 +429,8 @@ def _check_npm_audit(should_fix: bool) -> Finding: (PROJECT_ROOT, "ui-tui workspace", ["--workspace", "ui-tui"]), (whatsapp_bridge_dir, "WhatsApp bridge", []), ): - # Workspace-scoped audits run from PROJECT_ROOT check the root node_modules; - # standalone dirs (whatsapp-bridge) check their own. - check_dir = PROJECT_ROOT if audit_extra else npm_dir - if (check_dir / "node_modules").exists(): + # Workspace-scoped audits check the root node_modules; standalone dirs check their own. + if ((PROJECT_ROOT if audit_extra else npm_dir) / "node_modules").exists(): _audit_one(npm_bin, npm_dir, label, audit_extra, f.issues) if _is_termux(): diff --git a/hermes_cli/dump.py b/hermes_cli/dump.py index d0ea246ddb..2070e507e7 100644 --- a/hermes_cli/dump.py +++ b/hermes_cli/dump.py @@ -14,11 +14,8 @@ from agent.skill_utils import is_excluded_skill_path def _dotenv_key_names() -> set[str]: - """Return the set of env-var names assigned a non-empty value in ~/.hermes/.env. - - The managed backends (launchd / systemd / the desktop-spawned ``serve`` process) load credentials - from this file — NOT from an interactive shell's exports, which ``os.getenv`` reflects here. - """ + """Env-var names assigned a non-empty value in ~/.hermes/.env — what the managed backends (launchd / + systemd / desktop ``serve``) load, as opposed to the shell exports ``os.getenv`` reflects here.""" try: text = get_env_path().read_text(encoding="utf-8", errors="ignore") except (OSError, UnicodeError): @@ -52,11 +49,8 @@ def _git_output(project_root: Path, *args: str) -> str: def _get_git_commit(project_root: Path) -> str: - """Return short git commit hash, or '(unknown)'. - - Source installs resolve live via ``git rev-parse``. The published Docker image excludes ``.git``, - so fall back to the build SHA baked into ``/.hermes_build_sha`` by the Dockerfile. - """ + """Short git commit hash, or '(unknown)'. Docker images exclude ``.git``, so fall back to the build SHA + the Dockerfile bakes into ``/.hermes_build_sha``.""" value = _git_output(project_root, "rev-parse", "--short=8", "HEAD") if value: return value @@ -185,8 +179,8 @@ _API_KEYS = [ def _version_line(project_root: Path) -> str: - """`` [] ()`` — the commit date is the real "as-of" date; - __release_date__ is intentionally NOT shown (reads like a wall-clock timestamp, confuses triage).""" + """`` [] ()`` — the commit date is the real "as-of" date; __release_date__ + is intentionally NOT shown (reads like a wall-clock timestamp, confuses triage).""" try: from hermes_cli import __version__ except ImportError: @@ -197,9 +191,8 @@ def _version_line(project_root: Path) -> str: def _effective_terminal_backend(config: dict) -> str: - """The EFFECTIVE backend, not just config.yaml: a TERMINAL_ENV set directly in .env / the shell - overrides ``terminal.backend`` and is what terminal_tool actually uses. run_dump() has already - loaded .env, so os.environ reflects the real override here.""" + """The EFFECTIVE backend: a TERMINAL_ENV set directly in .env / the shell overrides ``terminal.backend`` and + is what terminal_tool uses. run_dump() has already loaded .env, so os.environ reflects the override.""" config_backend = config.get("terminal", {}).get("backend", "local") env_backend = (os.environ.get("TERMINAL_ENV") or "").strip().lower() if env_backend and env_backend != str(config_backend).strip().lower(): @@ -213,13 +206,11 @@ def _api_key_lines(show_keys: bool) -> list[str]: for env_var, label in _API_KEYS: val = os.getenv(env_var, "") display = _redact(val) if show_keys and val else ("set" if val else "not set") - # Set in this (shell) process but absent from ~/.hermes/.env: a managed backend - # (launchd/systemd/desktop `serve`) loads .env, not the login shell, so it likely can't - # see this key — flag it so support doesn't chase a phantom "key is configured". + # Set in this shell but absent from ~/.hermes/.env: a managed backend loads .env, not the login + # shell, so it likely can't see this key — flag it so support doesn't chase a phantom "configured". if val and env_var not in dotenv_keys: display += " (shell only — not in .env; managed/desktop backend may not see it)" - # A credential added via `hermes auth add openrouter` lives in the credential pool, not as an - # env var — surface it so the dump doesn't read "not set" while `hermes auth list` shows it. + # `hermes auth add openrouter` credentials live in the pool, not env — don't read "not set". if not val and label == "openrouter": try: from agent.credential_pool import load_pool as _load_pool @@ -232,6 +223,14 @@ def _api_key_lines(show_keys: bool) -> list[str]: return lines +def _openai_version() -> str: + try: + import openai + return openai.__version__ + except ImportError: + return "not installed" + + def run_dump(args): """Output a compact, copy-pasteable setup summary.""" show_keys = getattr(args, "show_keys", False) @@ -250,11 +249,7 @@ def run_dump(args): profile = get_active_profile_name() or "(default)" except Exception: profile = "(default)" - try: - import openai - openai_ver = openai.__version__ - except ImportError: - openai_ver = "not installed" + openai_ver = _openai_version() lines = [ "--- hermes dump ---",