From 39c0a391006c376d92512ae5108c71bf23eaf196 Mon Sep 17 00:00:00 2001 From: ethernet Date: Thu, 24 Sep 2026 23:37:27 -0400 Subject: [PATCH] fix(providers): report why a lazy SDK install did not land _get_anthropic_sdk() swallowed every ensure_import("anthropic") failure and _require_sdk() then told the user to "Install it with: hermes pm install --extra anthropic". PM often HAS installed it: sync_venv succeeds into a new dependency environment that only activates at process boot, and ensure_import raises "installed; restart Hermes". A lazy-install guard ("this process is not running from the install's dependency environment") was flattened the same way. Users were told to install something that was installed, or given a command that doesn't address the actual refusal. Keep the import as the decider, but remember the InstallError and put its text in the ImportError. bedrock_adapter._require_boto3 had the identical shape; azure_identity_adapter already propagates str(exc) and is the model. --- agent/anthropic_adapter.py | 15 +++++++++------ agent/bedrock_adapter.py | 11 ++++++----- tests/agent/test_anthropic_adapter.py | 22 ++++++++++++++++++++++ tests/agent/test_bedrock_adapter.py | 18 ++++++++++++++++++ 4 files changed, 55 insertions(+), 11 deletions(-) diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index 252338da41..d51aec1767 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -34,20 +34,20 @@ from hermes_cli.version_info import get_version_info # ``import anthropic`` is deliberately NOT at module top: the SDK costs ~220 ms of imports and # every usage site is a cold user-triggered path. ``...`` = not yet tried; None = tried, missing. _anthropic_sdk: Any = ... +# Why the lazy install did not make the SDK importable. A completed install that needs a restart +# (PM activates a new dependency environment only at boot) must not be reported as "install it". +_anthropic_install_error: Optional[Exception] = None def _get_anthropic_sdk(): """Return the ``anthropic`` SDK module, importing lazily. None if not installed.""" - global _anthropic_sdk + global _anthropic_sdk, _anthropic_install_error if _anthropic_sdk is ...: try: from pm import ensure_import ensure_import("anthropic") - except ImportError: - pass - except Exception: - # InstallError — fall through to ImportError handling below - pass + except Exception as exc: # the import below decides; exc explains a miss + _anthropic_install_error = exc try: import anthropic as _sdk _anthropic_sdk = _sdk @@ -59,6 +59,9 @@ def _get_anthropic_sdk(): def _require_sdk(purpose: str, verb: str = "Install it with"): """``_get_anthropic_sdk()`` or ImportError naming the feature that needs it.""" sdk = _get_anthropic_sdk() + if sdk is None and _anthropic_install_error is not None: + raise ImportError(f"The 'anthropic' package is required for {purpose}: " + f"{_anthropic_install_error}") from _anthropic_install_error if sdk is None: raise ImportError(f"The 'anthropic' package is required for {purpose}. {verb}: " f"{install_hint('anthropic')}") diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index fb8c5e59f4..358ee71210 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -87,6 +87,7 @@ _MIN_BOTO3_VERSION = (1, 34, 59) def _require_boto3(): """Import boto3; converse_stream() needs >= 1.34.59 (a system boto3 can shadow the venv pin).""" + install_error = None try: # boto3 left [all] (PRs #24220, #24515); PM installs the [bedrock] extra on first use. This # runs at the first client build, never at import: an import-time sync would rebuild the @@ -94,14 +95,14 @@ def _require_boto3(): try: from pm import ensure_import ensure_import("bedrock") - except Exception as exc: # the import below reports the real failure + except Exception as exc: # the import below decides; exc explains a miss logger.warning("boto3 lazy install did not complete: %s", exc) + install_error = exc import boto3 except ImportError: - raise ImportError( - "The 'boto3' package is required for the AWS Bedrock provider. " - f"Run: {install_hint('bedrock')}" - ) + # A completed install that needs a restart must not be reported as "install it". + reason = f": {install_error}" if install_error else f". Run: {install_hint('bedrock')}" + raise ImportError(f"The 'boto3' package is required for the AWS Bedrock provider{reason}") from install_error try: version = tuple(int(x) for x in boto3.__version__.split(".")[:3]) except (AttributeError, ValueError): diff --git a/tests/agent/test_anthropic_adapter.py b/tests/agent/test_anthropic_adapter.py index a0b4e563ad..929cd633b7 100644 --- a/tests/agent/test_anthropic_adapter.py +++ b/tests/agent/test_anthropic_adapter.py @@ -29,6 +29,28 @@ class TestIsOAuthToken: assert _is_oauth_token("sk-ant-api03-abcdef1234567890") is False +def test_missing_sdk_error_reports_why_the_lazy_install_did_not_land(monkeypatch): + """A completed install that needs a restart must not tell the user to install it again.""" + import pm + from agent import anthropic_adapter + from pm.package import InstallError + + restart = InstallError("venv", "anthropic installed; restart Hermes to activate the new dependency environment") + + def ensure_import(extra): + raise restart + + monkeypatch.setattr(pm, "ensure_import", ensure_import) + monkeypatch.setattr(anthropic_adapter, "_anthropic_sdk", ...) + monkeypatch.setattr(anthropic_adapter, "_anthropic_install_error", None) + monkeypatch.setitem(sys.modules, "anthropic", None) + + with pytest.raises(ImportError) as excinfo: + build_anthropic_client("sk-ant-api03-test") + assert str(restart) in str(excinfo.value) + assert pm.install_hint("anthropic") not in str(excinfo.value) + + class TestBuildAnthropicClient: diff --git a/tests/agent/test_bedrock_adapter.py b/tests/agent/test_bedrock_adapter.py index 1d725b856f..c710352a52 100644 --- a/tests/agent/test_bedrock_adapter.py +++ b/tests/agent/test_bedrock_adapter.py @@ -1437,6 +1437,24 @@ class TestRequireBoto3VersionCheck: with pytest.raises(RuntimeError, match="does not support converse_stream"): _require_boto3() + def test_missing_boto3_error_reports_why_the_lazy_install_did_not_land(self, monkeypatch): + """A completed install that needs a restart must not tell the user to install it again.""" + import pm + from agent.bedrock_adapter import _require_boto3 + from pm.package import InstallError + + restart = InstallError("venv", "bedrock installed; restart Hermes to activate the new dependency environment") + + def ensure_import(extra): + raise restart + + monkeypatch.setattr(pm, "ensure_import", ensure_import) + with patch.dict("sys.modules", {"boto3": None}): + with pytest.raises(ImportError) as excinfo: + _require_boto3() + assert str(restart) in str(excinfo.value) + assert pm.install_hint("bedrock") not in str(excinfo.value) + class TestImageBase64Decoding: