From 8a903fa5fb760f1b00a56340465a90f0e0c9afad Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:41:48 -0700 Subject: [PATCH] =?UTF-8?q?refactor(hermes=5Fcli):=20doctor=20=E2=80=94=20?= =?UTF-8?q?every=20check=20body=20on=20@doctor=5Fcheck=20(propagating=20by?= =?UTF-8?q?=20default);=20join=20FDA=20hint=20literal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/doctor.py | 12 +++++----- hermes_cli/doctor_config.py | 15 +++++-------- hermes_cli/doctor_platform.py | 42 ++++++++++++++--------------------- hermes_cli/doctor_report.py | 12 +++++++--- hermes_cli/doctor_state.py | 24 +++++++++----------- hermes_cli/doctor_tools.py | 20 +++++++---------- 6 files changed, 55 insertions(+), 70 deletions(-) diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index 6b3609b9c0..b442998191 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -51,7 +51,7 @@ from hermes_cli.doctor_state import ( # noqa: F401 (re-exported; tests use her _check_skills_hub, _check_state_db, _honcho_is_configured_for_doctor, _render_state_db_stats, ) from hermes_cli.doctor_report import ( # noqa: F401 (re-exported for doctor_live and tests) - Finding, _fail_and_issue, _section, check_bool, check_fail, check_info, check_ok, check_warn, + Finding, _fail_and_issue, _section, check_bool, check_fail, check_info, check_ok, check_warn, doctor_check, ) from hermes_cli.doctor_connectivity import ( # noqa: F401 (re-exported; tests import from hermes_cli.doctor) _build_apikey_providers_list, _has_healthy_oauth_fallback_for_apikey_provider, build_probes, @@ -72,9 +72,9 @@ _PROVIDER_ENV_HINTS = ( from hermes_cli.sizefmt import format_bytes as _human_bytes # noqa: F401 (tests import doctor._human_bytes) -def _check_auth_providers(should_fix: bool) -> Finding: +@doctor_check() +def _check_auth_providers(should_fix: bool, f: Finding) -> None: """Refresh-free OAuth status snapshot (doctor must never trigger a token refresh).""" - f = Finding() try: from hermes_cli.auth import get_nous_auth_status_local, get_codex_auth_status, get_minimax_oauth_auth_status # Read-only display: refresh-free snapshot — doctor must never trigger an OAuth refresh. @@ -93,7 +93,6 @@ def _check_auth_providers(should_fix: bool) -> Finding: _login_row("xAI OAuth", get_xai_oauth_auth_status() or {}, show_error=True) except Exception: pass - return f def _login_row(label: str, status: dict, ok_detail: str = "(logged in)", show_error: bool = False) -> bool: @@ -104,9 +103,9 @@ def _login_row(label: str, status: dict, ok_detail: str = "(logged in)", show_er return logged_in -def _check_api_connectivity(should_fix: bool) -> Finding: +@doctor_check() +def _check_api_connectivity(should_fix: bool, f: Finding) -> None: """Parallel HTTP/SDK probes for every configured provider; results printed in submission order.""" - f = Finding() probes = build_probes() # Single status line so users see something happening; ``\r`` clears it once results land. print(f" {color(f'Running {len(probes)} connectivity checks in parallel…', Colors.DIM)}", end="", flush=True) @@ -117,7 +116,6 @@ def _check_api_connectivity(should_fix: bool) -> Finding: 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 # Ordered (section title, check). None title = check prints its own header (or none); order is user-visible. diff --git a/hermes_cli/doctor_config.py b/hermes_cli/doctor_config.py index 09efb1e134..93c10aa6da 100644 --- a/hermes_cli/doctor_config.py +++ b/hermes_cli/doctor_config.py @@ -137,10 +137,10 @@ def _check_mcp_security(should_fix: bool, f: Finding) -> None: check_ok("No suspicious MCP stdio commands") -def _check_env_file(should_fix: bool) -> Finding: +@doctor_check() +def _check_env_file(should_fix: bool, f: Finding) -> None: """Managed scope plus ~/.hermes/.env presence and provider credentials.""" from hermes_cli.doctor import HERMES_HOME, PROJECT_ROOT, _DHH - f = Finding() managed_scope_check() env_path = HERMES_HOME / '.env' if env_path.exists(): @@ -170,7 +170,6 @@ def _check_env_file(should_fix: bool) -> Finding: else: check_info("Run 'hermes setup' to create one") f.issues.append("Run 'hermes setup' to create .env") - return f def _known_provider_ids(cfg: dict) -> tuple[set, list, object, object, object]: @@ -291,10 +290,10 @@ def _validate_model_config(config_path, issues: list) -> None: pass -def _check_config_file(should_fix: bool) -> Finding: +@doctor_check() +def _check_config_file(should_fix: bool, f: Finding) -> None: """config.yaml presence (project cli-config.yaml as fallback); model/provider validation.""" from hermes_cli.doctor import HERMES_HOME, PROJECT_ROOT, _DHH - f = Finding() config_path = HERMES_HOME / 'config.yaml' if config_path.exists(): check_ok(f"{_DHH}/config.yaml exists") @@ -316,7 +315,6 @@ def _check_config_file(should_fix: bool) -> Finding: f.fixed += 1 else: check_warn("config.yaml not found", "(using defaults)") - return f def _drift_config_version(f: Finding, should_fix: bool, config_path) -> None: @@ -427,13 +425,13 @@ _CONFIG_DRIFT_STEPS = ( ) -def _check_config_drift(should_fix: bool) -> Finding: +@doctor_check() +def _check_config_drift(should_fix: bool, f: Finding) -> None: """Config version, stale root keys, HERMES_MAX_ITERATIONS ghost, deprecations, structure. Each step is independent and best-effort: a failure in one never hides the next. """ from hermes_cli.doctor import HERMES_HOME - f = Finding() config_path = HERMES_HOME / 'config.yaml' if not config_path.exists(): config_path = None @@ -442,7 +440,6 @@ def _check_config_drift(should_fix: bool) -> Finding: step(f, should_fix, config_path) except Exception: pass - return f @doctor_check("xAI retirement check skipped", "({e})") diff --git a/hermes_cli/doctor_platform.py b/hermes_cli/doctor_platform.py index 6bf7322b3b..cc3cf397c4 100644 --- a/hermes_cli/doctor_platform.py +++ b/hermes_cli/doctor_platform.py @@ -339,17 +339,12 @@ def check_macos_full_disk_access() -> None: except OSError as e: if not isinstance(e, PermissionError): return - check_info( - "One switch silences all macOS folder prompts: grant your terminal " - "app Full Disk Access and Hermes will never trip per-folder dialogs " - "(Desktop/Downloads/Documents/...) again. Open: System Settings → " - "Privacy & Security → Full Disk Access — or run:\n" - " open \"x-apple.systempreferences:com.apple.preference" - ".security?Privacy_AllFiles\"\n" - " then enable your terminal (and Hermes.app if you use Desktop), " - "and restart them once. With Hermes' stable signing identities the " - "grant survives every update." - ) + check_info("One switch silences all macOS folder prompts: grant your terminal app Full Disk Access and Hermes " + "will never trip per-folder dialogs (Desktop/Downloads/Documents/...) again. Open: System Settings → " + "Privacy & Security → Full Disk Access — or run:\n" + " open \"x-apple.systempreferences:com.apple.preference.security?Privacy_AllFiles\"\n" + " then enable your terminal (and Hermes.app if you use Desktop), and restart them once. " + "With Hermes' stable signing identities the grant survives every update.") else: check_ok("macOS Full Disk Access granted", "(no per-folder permission prompts will occur)") @@ -376,9 +371,9 @@ def _check_security_advisories(should_fix: bool, f: Finding) -> None: check_warn(f"{h.package}=={h.installed_version} still installed (advisory {h.advisory.id} acknowledged)") -def _check_python_environment(should_fix: bool) -> Finding: +@doctor_check() +def _check_python_environment(should_fix: bool, f: Finding) -> None: """Interpreter, linked SQLite, venv, macOS TCC anchors/FDA/grants, version-file drift.""" - f = Finding() v = sys.version_info label = f"Python {v.major}.{v.minor}.{v.micro}" if v >= (3, 10): @@ -410,7 +405,6 @@ def _check_python_environment(should_fix: bool) -> Finding: check_macos_full_disk_access() _check_version_consistency(f.issues) check_macos_tcc_grants() - return f @doctor_check() @@ -426,8 +420,8 @@ _PACKAGES = ( ) -def _check_required_packages(should_fix: bool) -> Finding: - f = Finding() +@doctor_check() +def _check_required_packages(should_fix: bool, f: Finding) -> None: for module, name, optional in _PACKAGES: try: __import__(module) @@ -437,7 +431,6 @@ def _check_required_packages(should_fix: bool) -> Finding: check_warn(name, "(optional, not installed)") else: _fail_and_issue(name, "(missing)", f"Install {name}: {_python_install_cmd()} {module}", f.issues) - return f @doctor_check() @@ -446,12 +439,12 @@ def _check_gateway_supervision(should_fix: bool, f: Finding) -> None: _check_s6_supervision(f.issues) -def _check_command_installation(should_fix: bool) -> Finding: +@doctor_check() +def _check_command_installation(should_fix: bool, f: Finding) -> None: """Venv entry point and the ~/.local/bin (or $PREFIX/bin) symlink; skipped on Windows.""" from hermes_cli.doctor import PROJECT_ROOT - f = Finding() if sys.platform == "win32": - return f + return _section("Command Installation") venv_bin = next((c for c in (PROJECT_ROOT / n / "bin" / "hermes" for n in ("venv", ".venv")) if c.exists()), None) # Expected command link directory (mirrors install.sh logic). @@ -464,17 +457,17 @@ def _check_command_installation(should_fix: bool) -> Finding: if venv_bin is None: check_warn("Venv entry point not found", "(hermes not in venv/bin/ or .venv/bin/ — reinstall with pip install -e '.[all]')") f.manual_issues.append(f"Reinstall entry point: cd {PROJECT_ROOT} && source venv/bin/activate && pip install -e '.[all]'") - return f + return check_ok(f"Venv entry point exists ({venv_bin.relative_to(PROJECT_ROOT)})") if link.is_symlink(): target, expected = link.resolve(), venv_bin.resolve() if target == expected: check_ok(f"{display}/hermes → correct target") - return f + return check_warn(f"{display}/hermes points to wrong target", f"(→ {target}, expected → {expected})") if not should_fix: f.issues.append(f"Broken symlink at {display}/hermes — run 'hermes doctor --fix'") - return f + return link.unlink() _link_venv(f, link, venv_bin, f"Fixed symlink: {display}/hermes → {venv_bin}") elif link.exists(): # regular file (wrapper script), not a symlink @@ -483,13 +476,12 @@ def _check_command_installation(should_fix: bool) -> Finding: check_fail(f"{display}/hermes not found", "(hermes command may not work outside the venv)") if not should_fix: f.issues.append(f"Missing {display}/hermes symlink — run 'hermes doctor --fix'") - return f + return link_dir.mkdir(parents=True, exist_ok=True) _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: diff --git a/hermes_cli/doctor_report.py b/hermes_cli/doctor_report.py index 6a5190dbca..efa5350052 100644 --- a/hermes_cli/doctor_report.py +++ b/hermes_cli/doctor_report.py @@ -57,16 +57,22 @@ class Finding: def doctor_check(on_error: str | None = None, detail: str = ""): """Turn ``fn(should_fix, f: Finding)`` into a ``(should_fix) -> Finding`` doctor check. - If *fn* raises, prints ``check_warn(on_error.format(e=e), detail.format(e=e))`` (nothing when *on_error* - is None); the partial Finding is still returned, so issues recorded before the crash survive.""" + + *on_error* None: exceptions propagate (as they always did for that check). Otherwise the check is + best-effort: a crash prints ``check_warn(on_error.format(e=e), detail.format(e=e))`` (nothing for + ``""``) and the partial Finding is still returned, so issues recorded before the crash survive. + """ def deco(fn): @functools.wraps(fn) def check(should_fix: bool) -> Finding: f = Finding() + if on_error is None: + fn(should_fix, f) + return f try: fn(should_fix, f) except Exception as e: - if on_error is not None: + if on_error: check_warn(on_error.format(e=e), detail.format(e=e)) return f return check diff --git a/hermes_cli/doctor_state.py b/hermes_cli/doctor_state.py index 97c1eaa933..ad2c1c380c 100644 --- a/hermes_cli/doctor_state.py +++ b/hermes_cli/doctor_state.py @@ -100,10 +100,10 @@ def _memory_store_flags(hermes_home: Path) -> tuple: return get_builtin_memory_store_flags({"memory": _doctor_memory_config(hermes_home)}) -def _check_directory_structure(should_fix: bool) -> Finding: +@doctor_check() +def _check_directory_structure(should_fix: bool, f: Finding) -> None: """HERMES_HOME, expected subdirs, SOUL.md, and the enabled built-in memory files.""" from hermes_cli.doctor import HERMES_HOME, _DHH - f = Finding() hermes_home = HERMES_HOME ensure_dir(f, should_fix, hermes_home, f"{_DHH} directory exists", f"Created {_DHH} directory", f"{_DHH} not found") _memory_enabled, _user_profile_enabled = _memory_store_flags(hermes_home) @@ -148,7 +148,6 @@ def _check_directory_structure(should_fix: bool) -> Finding: check_ok(f"{fname} exists ({len(mem_file.read_text(encoding='utf-8').strip())} chars)") elif enabled: check_info(f"{fname} not created yet (will be created when the agent first writes a memory)") - return f def _session_count(state_db_path: Path): @@ -258,10 +257,10 @@ def _state_db_wal(f: Finding, should_fix: bool, state_db_path: Path) -> None: pass -def _check_state_db(should_fix: bool) -> Finding: +@doctor_check() +def _check_state_db(should_fix: bool, f: Finding) -> None: """state.db session count, FTS write health, schema repair, stats snapshot, WAL size.""" from hermes_cli.doctor import HERMES_HOME, _DHH - f = Finding() state_db_path = HERMES_HOME / "state.db" if state_db_path.exists(): _state_db_health(f, should_fix, state_db_path, _DHH) @@ -269,7 +268,6 @@ def _check_state_db(should_fix: bool) -> Finding: else: check_info(f"{_DHH}/state.db not created yet (will be created on first session)") _state_db_wal(f, should_fix, state_db_path) - return f def _gh_authenticated() -> bool: @@ -281,9 +279,9 @@ def _gh_authenticated() -> bool: return False -def _check_skills_hub(should_fix: bool) -> Finding: +@doctor_check() +def _check_skills_hub(should_fix: bool, f: Finding) -> None: from hermes_cli.doctor import HERMES_HOME, _DHH - f = Finding() hub_dir = HERMES_HOME / "skills" / ".hub" if not hub_dir.exists(): check_warn("Skills Hub directory not initialized", "(run: hermes skills list)") @@ -307,7 +305,6 @@ def _check_skills_hub(should_fix: bool) -> Finding: else: 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 def _memory_provider_honcho(issues: list) -> None: @@ -367,13 +364,13 @@ def _memory_provider_generic(name: str) -> None: check_warn(f"{name} plugin not found", "run: hermes memory setup") -def _check_memory_provider(should_fix: bool) -> Finding: +@doctor_check() +def _check_memory_provider(should_fix: bool, f: Finding) -> None: from hermes_cli.doctor import HERMES_HOME - f = Finding() name = _doctor_memory_config(HERMES_HOME).get("provider", "") if not name: check_ok("Built-in memory active", "(no external provider configured — this is fine)") - return f + return checker, missing_row, missing_issue, label = _MEMORY_PROVIDER_CHECKS.get(name, (None, None, None, name)) try: checker(f.issues) if checker else _memory_provider_generic(name) @@ -384,10 +381,9 @@ def _check_memory_provider(should_fix: bool) -> Finding: _fail_and_issue(*missing_row, missing_issue, f.issues) except Exception as _e: check_warn(f"{label} check failed", str(_e)) - return f -@doctor_check() +@doctor_check("") # best-effort: profile enumeration must never break doctor def _check_profiles(should_fix: bool, f: Finding) -> None: from hermes_cli.profiles import list_profiles, _get_wrapper_dir, profile_exists import re as _re diff --git a/hermes_cli/doctor_tools.py b/hermes_cli/doctor_tools.py index 930758225a..a00a67e983 100644 --- a/hermes_cli/doctor_tools.py +++ b/hermes_cli/doctor_tools.py @@ -124,13 +124,12 @@ def _missing_api_key_toolsets_for_summary(unavailable: list[dict]) -> list[dict] return [item for item in api_key_unavailable if str(item.get("name") or "") in enabled_toolsets] -def _check_git_and_rg(should_fix: bool) -> Finding: - f = Finding() +@doctor_check() +def _check_git_and_rg(should_fix: bool, f: Finding) -> None: check_bool(_safe_which("git"), "git", ("git not found", "(optional)")) if not check_bool(_safe_which("rg"), ("ripgrep (rg)", "(faster file search)"), ("ripgrep (rg) not found", "(file search uses grep fallback)")): check_info(f"Install for faster search: {_system_package_install_cmd('ripgrep')}") - return f _BUILTIN_TERMINAL_BACKENDS = {"local", "docker", "singularity", "modal", "managed_modal", "daytona", "vercel_sandbox", "ssh"} @@ -226,9 +225,9 @@ def _check_plugin_backend(terminal_env: str, issues: list[str]) -> None: _BACKEND_CHECKS = {"ssh": _check_ssh_backend, "daytona": _check_daytona_backend, "vercel_sandbox": _check_vercel_backend} -def _check_terminal_backend(should_fix: bool) -> Finding: +@doctor_check() +def _check_terminal_backend(should_fix: bool, f: Finding) -> None: """Docker/SSH/Daytona/Vercel/plugin terminal backends, gated on TERMINAL_ENV.""" - f = Finding() terminal_env = os.getenv("TERMINAL_ENV", "local") try: from hermes_constants import is_container as _is_container @@ -245,7 +244,6 @@ def _check_terminal_backend(should_fix: bool) -> Finding: _BACKEND_CHECKS[terminal_env](f.issues) elif terminal_env not in _BUILTIN_TERMINAL_BACKENDS: _check_plugin_backend(terminal_env, f.issues) - return f def _check_agent_browser(should_fix: bool) -> bool: @@ -331,9 +329,9 @@ def _check_lightpanda() -> None: check_info(LIGHTPANDA_INSTALL_HINT) -def _check_node_and_browser(should_fix: bool) -> Finding: +@doctor_check() +def _check_node_and_browser(should_fix: bool, f: Finding) -> None: """Node.js, agent-browser resolution, Playwright Chromium, Lightpanda engine.""" - f = Finding() if _safe_which("node"): check_ok("Node.js") if _check_agent_browser(should_fix) and not _is_termux(): # Chromium check is not a tested Termux path @@ -344,7 +342,6 @@ def _check_node_and_browser(should_fix: bool) -> Finding: else: check_warn("Node.js not found", "(optional, needed for browser tools)") _check_lightpanda() - return f def _plural(n: int) -> str: @@ -384,7 +381,8 @@ def _audit_one(npm_bin: str, npm_dir, label: str, audit_extra: list[str], issues pass -def _check_npm_audit(should_fix: bool) -> Finding: +@doctor_check() +def _check_npm_audit(should_fix: bool, f: Finding) -> None: """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 @@ -392,7 +390,6 @@ def _check_npm_audit(should_fix: bool) -> Finding: 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() npm_bin = _safe_which("npm") if npm_bin: try: @@ -413,7 +410,6 @@ def _check_npm_audit(should_fix: bool) -> Finding: check_info("Termux compatibility fallbacks:") for note in _TERMUX_INSTALL_ALL_FALLBACK_NOTES: check_info(note) - return f @doctor_check("Could not check tool availability", "({e})")