mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(doctor): never install a moved plugin from a diagnostic; name the install instead
LC-3 (r4 review). `hermes doctor`'s auxiliary routing check resolves each routed provider through resolve_runtime_provider, whose left-core recovery installed the plugin over the network with dependency consent; its notice was queued for an agent that never starts in that process, and the check's splitlines()[0] cut the install hint off the error. left_core_migration.installs_held() holds every migration attempt in its scope (recover_at_startup returns before any work); run_doctor runs all its checks inside it. The failed auxiliary check's issue now carries provider_install_hint (or the old fix-the-config advice for other providers).
This commit is contained in:
1 parent
b3252c2ad5
commit
bef9b25814
4 files changed
+48
-11
No files matched your search
+10
-8
@@ -186,13 +186,15 @@ def run_doctor(args):
|
||||
"└─────────────────────────────────────────────────────────┘"):
|
||||
print(color(line, Colors.CYAN))
|
||||
total = Finding()
|
||||
for title, check in DOCTOR_CHECKS:
|
||||
if title:
|
||||
_section(title)
|
||||
total.merge(check(should_fix))
|
||||
# Opt-in live probes run AFTER all static checks (`--live`: real network calls; bounded + read-only).
|
||||
with warn_on_error(""):
|
||||
from hermes_cli.doctor_live import maybe_run_live_checks
|
||||
maybe_run_live_checks(args, total.manual_issues)
|
||||
from hermes_cli.left_core_migration import installs_held
|
||||
with installs_held(): # a diagnostic: provider checks resolve through the runtime, which would install
|
||||
for title, check in DOCTOR_CHECKS:
|
||||
if title:
|
||||
_section(title)
|
||||
total.merge(check(should_fix))
|
||||
# Opt-in live probes run AFTER all static checks (`--live`: real network calls; bounded + read-only).
|
||||
with warn_on_error(""):
|
||||
from hermes_cli.doctor_live import maybe_run_live_checks
|
||||
maybe_run_live_checks(args, total.manual_issues)
|
||||
_print_summary(should_fix, total)
|
||||
return int(bool(total.issues or total.manual_issues))
|
||||
@@ -309,6 +309,7 @@ def _validate_auxiliary_config(config_path, issues: list) -> None:
|
||||
"""Resolve every routed ``auxiliary.<task>`` block through the real entry point the tasks use and report
|
||||
the ones that fail — an unresolvable block otherwise silently runs the task on the main model (#116055)."""
|
||||
from hermes_cli.config import read_user_config_raw
|
||||
from hermes_cli.left_core_migration import provider_install_hint
|
||||
from hermes_cli.runtime_provider import resolve_runtime_provider
|
||||
from utils import base_url_hostname
|
||||
aux = read_user_config_raw(config_path).get("auxiliary")
|
||||
@@ -320,9 +321,11 @@ def _validate_auxiliary_config(config_path, issues: list) -> None:
|
||||
try:
|
||||
runtime = resolve_runtime_provider(requested=provider, target_model=model, explicit_api_key=api_key, explicit_base_url=base_url)
|
||||
except Exception as exc:
|
||||
_fail_and_issue(f"auxiliary.{task}.provider '{provider}' does not resolve", f"({str(exc).splitlines()[0]})",
|
||||
f"auxiliary.{task}.provider '{provider}' cannot be resolved ({str(exc).splitlines()[0]}); the task "
|
||||
f"silently runs on the main model. Fix the provider name/credentials in auxiliary.{task}.", issues)
|
||||
reason, moved = str(exc).splitlines()[0], provider_install_hint(provider or "")
|
||||
_fail_and_issue(f"auxiliary.{task}.provider '{provider}' does not resolve", f"({reason})",
|
||||
f"auxiliary.{task}.provider '{provider}' cannot be resolved ({reason}); the task silently runs "
|
||||
f"on the main model. " + (moved or f"Fix the provider name/credentials in auxiliary.{task}."),
|
||||
issues)
|
||||
continue
|
||||
if not runtime.get("api_key") and not runtime.get("command"):
|
||||
check_warn(f"auxiliary.{task}.provider '{provider}' resolved without credentials", f"({runtime.get('provider')} @ {runtime.get('base_url')})")
|
||||
|
||||
@@ -29,6 +29,8 @@ from __future__ import annotations
|
||||
import logging
|
||||
import os
|
||||
import sys
|
||||
from contextlib import contextmanager
|
||||
from contextvars import ContextVar
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Callable, Optional
|
||||
@@ -655,6 +657,18 @@ def moved_plugin_hint(name: str) -> str:
|
||||
|
||||
_attempted: set[str] = set()
|
||||
_undelivered: dict[str, list[str]] = {}
|
||||
_installs_held: ContextVar[bool] = ContextVar("left_core_installs_held", default=False)
|
||||
|
||||
|
||||
@contextmanager
|
||||
def installs_held():
|
||||
"""No migration attempt inside (a read-only diagnostic such as ``hermes doctor``, whose provider checks run
|
||||
the runtime resolver): the unknown-provider and host hints still name the install command."""
|
||||
token = _installs_held.set(True)
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
_installs_held.reset(token)
|
||||
|
||||
|
||||
def defer_startup_notice(message: str) -> None:
|
||||
@@ -986,6 +1000,8 @@ def recover_at_startup(*, say: Optional[Callable[[str], None]] = None) -> list[s
|
||||
logged and queued for the home's first agent.
|
||||
Returns installed plugin names."""
|
||||
from agent.safe_worker_policy import worker_config_snapshot
|
||||
if _installs_held.get():
|
||||
return []
|
||||
if worker_config_snapshot() is not None:
|
||||
# A frozen safe/ignore-user-config worker never reads the profile's config.yaml and never
|
||||
# installs customizations; the owner process runs this migration for the home.
|
||||
|
||||
@@ -100,3 +100,19 @@ def test_a_route_configured_outside_the_main_model_installs_when_resolved(tmp_pa
|
||||
_copilot_home(tmp_path, monkeypatch, config)
|
||||
_resolve(requested="copilot")
|
||||
assert installs == [("github-copilot", True)]
|
||||
|
||||
|
||||
def test_doctor_installs_nothing_and_names_the_install(tmp_path, monkeypatch, installs, capsys):
|
||||
"""`hermes doctor` is read-only: its auxiliary routing check resolves through the runtime resolver,
|
||||
which must not fetch the plugin there; the failed check names the install command instead."""
|
||||
from types import SimpleNamespace
|
||||
from hermes_cli import doctor
|
||||
from hermes_cli.doctor_config import _check_config_file
|
||||
home = _copilot_home(tmp_path, monkeypatch, "model:\n provider: nous\nauxiliary:\n vision:\n"
|
||||
" provider: copilot\n model: gpt-4.1\n")
|
||||
monkeypatch.setattr(doctor, "HERMES_HOME", home)
|
||||
monkeypatch.setattr(doctor, "DOCTOR_CHECKS", ((None, _check_config_file),))
|
||||
doctor.run_doctor(SimpleNamespace(fix=False, ack=None, live=False))
|
||||
assert installs == []
|
||||
assert not (home / "plugins" / "github-copilot").exists()
|
||||
assert "hermes plugins install github-copilot" in capsys.readouterr().out
|
||||
Reference in new issue
Block a user