From 1f7e5e78da690ca75183b650bd4da182045e5eda Mon Sep 17 00:00:00 2001 From: Mike McHargue Date: Mon, 14 Sep 2026 17:41:53 -0700 Subject: [PATCH] fix: resolve Hermes Agent names moved by the Sep 2026 decomposition Hermes Agent moved several names the WebUI uses into sibling modules (tools.approval_context, tools.mcp_tool_discovery/_lifecycle, hermes_cli.kanban_db_connect/_dispatch, hermes_cli.models_local, agent.anthropic_credentials). The old paths only resolved through PLUGIN-COMPAT __getattr__ pointers that warn on every use and are removed on schedule by reverting one commit. With the pointers removed, every affected WebUI call site is wrapped in a broad except, so nothing crashes; features silently degrade instead: per-turn approval session-key binding is skipped, MCP discovery/status return nothing, /reload-mcp reports "MCP runtime unavailable", Claude Code credential linking returns None, every kanban connection raises AttributeError, and the dispatcher reports "unavailable". Add api.agent_compat.agent_attr, which resolves a moved name from the original module's own namespace first (pre-split Agents and existing test stubs), then from its new home (split Agents, with or without pointers), then plain attribute access (non-module doubles). Importing only the new modules would have broken pre-split Agents and bypassed the sys.modules stubs the test suite installs on the original modules. Names still native to their original module (_servers, _lock, has_blocking_approval, is_session_yolo_enabled, register/unregister gateway notify, the rest of kanban_db) are unchanged. Co-Authored-By: Claude Opus 5 --- api/agent_compat.py | 54 ++++++++++++++++ api/commands.py | 10 ++- api/config.py | 6 +- api/kanban_bridge.py | 19 ++++-- api/oauth.py | 10 ++- api/routes.py | 3 +- api/streaming.py | 13 +++- tests/test_agent_compat.py | 127 +++++++++++++++++++++++++++++++++++++ 8 files changed, 226 insertions(+), 16 deletions(-) create mode 100644 api/agent_compat.py create mode 100644 tests/test_agent_compat.py diff --git a/api/agent_compat.py b/api/agent_compat.py new file mode 100644 index 00000000..74d02a27 --- /dev/null +++ b/api/agent_compat.py @@ -0,0 +1,54 @@ +"""Resolve Hermes Agent names that moved to a sibling module. + +Hermes Agent's September 2026 decomposition moved many names out of their +original modules (``tools.approval``, ``tools.mcp_tool``, +``hermes_cli.kanban_db``, ...) into ``_`` siblings. The old paths +kept resolving for a while only through PEP 562 ``__getattr__`` pointers that +emit ``HermesPluginCompatWarning`` and are removed on schedule, so WebUI code +must not rely on them. Importing only the new module would instead break Agent +installs that predate the split. + +``agent_attr`` is compatibility-only and resolves a moved name in this order: + +1. the original module's own namespace -- pre-split Agents, and tests that stub + the original module in ``sys.modules`` or patch the name onto it; +2. the new home module -- split Agents, with or without the old-path pointers; +3. plain attribute access on the original object -- non-module test doubles, + or an Agent whose new home is not importable. +""" + +from __future__ import annotations + +import importlib +from types import ModuleType +from typing import Any + +_MISSING = object() + + +def agent_attr(owner: Any, name: str, home: str, default: Any = _MISSING) -> Any: + """Return ``owner.name`` without going through a removed/deprecated pointer. + + ``owner`` is the original module (or its dotted name); ``home`` is the + dotted name of the module the Agent moved ``name`` to. Raises + ``ImportError``/``AttributeError`` like the plain import it replaces, unless + ``default`` is given. + """ + if isinstance(owner, str): + try: + owner = importlib.import_module(owner) + except ImportError: + if default is _MISSING: + return getattr(importlib.import_module(home), name) + try: + return getattr(importlib.import_module(home), name, default) + except ImportError: + return default + if isinstance(owner, ModuleType) and name not in vars(owner): + try: + return getattr(importlib.import_module(home), name) + except (ImportError, AttributeError): + pass + if default is _MISSING: + return getattr(owner, name) + return getattr(owner, name, default) diff --git a/api/commands.py b/api/commands.py index 36d85920..31f72315 100644 --- a/api/commands.py +++ b/api/commands.py @@ -260,7 +260,15 @@ def _run_reload_mcp_command() -> str: """Execute the MCP reconnect path and return a short user-facing summary.""" with _RELOAD_MCP_LOCK: try: - from tools.mcp_tool import shutdown_mcp_servers, discover_mcp_tools, _servers, _lock + from tools.mcp_tool import _servers, _lock + from api.agent_compat import agent_attr + + shutdown_mcp_servers = agent_attr( + "tools.mcp_tool", "shutdown_mcp_servers", "tools.mcp_tool_lifecycle" + ) + discover_mcp_tools = agent_attr( + "tools.mcp_tool", "discover_mcp_tools", "tools.mcp_tool_discovery" + ) except Exception as exc: logger.warning("Failed to import MCP runtime for /reload-mcp", exc_info=True) raise RuntimeError("MCP runtime unavailable") from exc diff --git a/api/config.py b/api/config.py index 0eb26136..1eb04bc0 100644 --- a/api/config.py +++ b/api/config.py @@ -5562,8 +5562,10 @@ def _lmstudio_model_reasoning_options( ) try: - from hermes_cli.models import ( - lmstudio_model_reasoning_options as _cli_lmstudio_model_reasoning_options, + from api.agent_compat import agent_attr + + _cli_lmstudio_model_reasoning_options = agent_attr( + "hermes_cli.models", "lmstudio_model_reasoning_options", "hermes_cli.models_local" ) except Exception: return _lmstudio_reasoning_probe_options_fallback( diff --git a/api/kanban_bridge.py b/api/kanban_bridge.py index bd8dfaa9..4f1488e0 100644 --- a/api/kanban_bridge.py +++ b/api/kanban_bridge.py @@ -19,6 +19,7 @@ import time from dataclasses import asdict, is_dataclass from urllib.parse import parse_qs, unquote +from api.agent_compat import agent_attr from api.helpers import bad, j from api.workspace import resolve_trusted_workspace @@ -33,6 +34,11 @@ def _kb(): return kb +def _kb_connect(kb, board=None): + """Raw ``kb.connect`` (moved to ``hermes_cli.kanban_db_connect`` by the Agent split).""" + return agent_attr(kb, "connect", "hermes_cli.kanban_db_connect")(board=board) + + def _resolve_board(parsed): """Validate and normalise a ?board= query param. @@ -93,13 +99,13 @@ def _conn(board=None): """ kb = _kb() kb.init_db(board=board) - closing = getattr(kb, "connect_closing", None) + closing = agent_attr(kb, "connect_closing", "hermes_cli.kanban_db_connect", None) if closing is not None: return closing(board=board) # Older kanban_db builds (and lightweight test doubles) without # connect_closing: fall back to the raw connection; sqlite3's own # context manager at least scopes the transaction. - return kb.connect(board=board) + return _kb_connect(kb, board=board) def _obj_dict(value): @@ -720,10 +726,11 @@ def _dispatch_payload(parsed): kb = _kb() dry_run = _bool_query(parsed, "dry_run", False) max_spawn = _int_query(parsed, "max", 8, minimum=1, maximum=100) - if not hasattr(kb, "dispatch_once"): + dispatch_once = agent_attr(kb, "dispatch_once", "hermes_cli.kanban_db_dispatch", None) + if dispatch_once is None: raise ValueError("dispatcher is unavailable") with _conn(board=board) as conn: - result = kb.dispatch_once(conn, dry_run=dry_run, max_spawn=max_spawn) + result = dispatch_once(conn, dry_run=dry_run, max_spawn=max_spawn) if isinstance(result, dict): return result try: @@ -787,7 +794,7 @@ def _board_counts_for_slug(slug): if not kb.board_exists(slug): return {} try: - conn = kb.connect(board=slug) + conn = _kb_connect(kb, board=slug) except Exception: return {} try: @@ -1021,7 +1028,7 @@ def _kanban_sse_fetch_new(board, cursor): if board != default_slug and not kb.board_exists(board): return cursor, [] try: - conn = kb.connect(board=board) + conn = _kb_connect(kb, board=board) except Exception: return cursor, [] try: diff --git a/api/oauth.py b/api/oauth.py index 13d9a693..97924c5b 100644 --- a/api/oauth.py +++ b/api/oauth.py @@ -314,9 +314,13 @@ def _read_claude_code_credentials() -> dict[str, Any] | None: and macOS Keychain. Returns the credential dict or None. """ try: - from agent.anthropic_adapter import ( - is_claude_code_token_valid, - read_claude_code_credentials, + from api.agent_compat import agent_attr + + is_claude_code_token_valid = agent_attr( + "agent.anthropic_adapter", "is_claude_code_token_valid", "agent.anthropic_credentials" + ) + read_claude_code_credentials = agent_attr( + "agent.anthropic_adapter", "read_claude_code_credentials", "agent.anthropic_credentials" ) creds = read_claude_code_credentials() diff --git a/api/routes.py b/api/routes.py index 4cdb91c5..7f42ed9d 100644 --- a/api/routes.py +++ b/api/routes.py @@ -28836,7 +28836,8 @@ def _mcp_runtime_status_by_name() -> dict[str, dict]: is unavailable, fall back to an empty map so the API remains safe. """ try: - from tools.mcp_tool import get_mcp_status + from api.agent_compat import agent_attr + get_mcp_status = agent_attr("tools.mcp_tool", "get_mcp_status", "tools.mcp_tool_discovery") statuses = get_mcp_status() except Exception: return {} diff --git a/api/streaming.py b/api/streaming.py index beab649b..71cf8208 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -2981,7 +2981,10 @@ def _set_turn_session_identity(session_id: str, workspace: str = ""): sid = str(session_id or "") tokens: dict = {} try: - from tools.approval import set_current_session_key + from api.agent_compat import agent_attr + set_current_session_key = agent_attr( + "tools.approval", "set_current_session_key", "tools.approval_context" + ) tokens["approval"] = set_current_session_key(sid) except Exception: logger.debug("per-turn approval session-key bind failed", exc_info=True) @@ -3043,7 +3046,10 @@ def _reset_turn_session_identity(tokens) -> None: tok = tokens.get("approval") if tok is not None: try: - from tools.approval import reset_current_session_key + from api.agent_compat import agent_attr + reset_current_session_key = agent_attr( + "tools.approval", "reset_current_session_key", "tools.approval_context" + ) reset_current_session_key(tok) except Exception: logger.debug("per-turn approval session-key reset failed", exc_info=True) @@ -9857,7 +9863,8 @@ def _run_agent_streaming( # lives outside this WebUI repo. This change fixes the headline bug # for users who run a single non-default profile per WebUI process. try: - from tools.mcp_tool import discover_mcp_tools + from api.agent_compat import agent_attr + discover_mcp_tools = agent_attr("tools.mcp_tool", "discover_mcp_tools", "tools.mcp_tool_discovery") discover_mcp_tools() except Exception: pass # MCP not available or not configured — non-fatal diff --git a/tests/test_agent_compat.py b/tests/test_agent_compat.py new file mode 100644 index 00000000..e489624b --- /dev/null +++ b/tests/test_agent_compat.py @@ -0,0 +1,127 @@ +"""api.agent_compat.agent_attr resolves Hermes Agent names that moved modules. + +The Agent's September 2026 decomposition moved names such as +``tools.approval.set_current_session_key`` into sibling modules. The old paths +resolved only through warning ``__getattr__`` pointers that are removed on +schedule; with the pointers gone, the WebUI's broad ``except Exception`` call +sites silently dropped approval session binding, MCP discovery/status, Claude +Code credential linking, and every kanban connection. +""" + +import sys +import types +import warnings + +import pytest + +from api.agent_compat import agent_attr + + +class _CompatWarning(FutureWarning): + pass + + +def _real(value): + return value + + +def _install(monkeypatch, name, **attrs): + mod = types.ModuleType(name) + for key, value in attrs.items(): + setattr(mod, key, value) + monkeypatch.setitem(sys.modules, name, mod) + return mod + + +def _split_facade(monkeypatch, *, with_pointer): + """An Agent after the split: the name lives in the home module; the facade + optionally keeps a warning PEP 562 pointer (the temporary compat layer).""" + _install(monkeypatch, "fakeagent_home", moved=_real) + facade = _install(monkeypatch, "fakeagent_facade", native=_real) + if with_pointer: + def __getattr__(name): + if name == "moved": + warnings.warn("`fakeagent_facade.moved` moved", _CompatWarning, stacklevel=2) + return _real + raise AttributeError(name) + facade.__getattr__ = __getattr__ + return facade + + +def test_pre_split_agent_uses_original_module(monkeypatch): + orig = _install(monkeypatch, "fakeagent_facade", moved=lambda: "pre-split") + assert agent_attr("fakeagent_facade", "moved", "fakeagent_missing_home")() == "pre-split" + assert agent_attr(orig, "moved", "fakeagent_missing_home")() == "pre-split" + + +def test_split_agent_with_pointer_uses_home_without_compat_warning(monkeypatch): + _split_facade(monkeypatch, with_pointer=True) + with warnings.catch_warnings(): + warnings.simplefilter("error", _CompatWarning) + assert agent_attr("fakeagent_facade", "moved", "fakeagent_home") is _real + + +def test_split_agent_after_pointer_removal_uses_home(monkeypatch): + facade = _split_facade(monkeypatch, with_pointer=False) + with pytest.raises(AttributeError): + _ = facade.moved # what `from facade import moved` hits after the removal + assert agent_attr(facade, "moved", "fakeagent_home") is _real + + +def test_stub_on_original_module_still_wins(monkeypatch): + _split_facade(monkeypatch, with_pointer=False) + stub = lambda: "stub" # noqa: E731 + monkeypatch.setattr(sys.modules["fakeagent_facade"], "moved", stub, raising=False) + assert agent_attr("fakeagent_facade", "moved", "fakeagent_home") is stub + + +def test_non_module_double_uses_plain_attribute(monkeypatch): + class FakeKanbanDB: + def connect(self, *, board=None): + return board + + double = FakeKanbanDB() + assert agent_attr(double, "connect", "fakeagent_home")(board="b") == "b" + assert agent_attr(double, "connect_closing", "fakeagent_home", None) is None + + +def test_default_when_name_is_absent_everywhere(monkeypatch): + _split_facade(monkeypatch, with_pointer=False) + assert agent_attr("fakeagent_facade", "nope", "fakeagent_home", None) is None + with pytest.raises(AttributeError): + agent_attr("fakeagent_facade", "nope", "fakeagent_home") + + +def test_missing_agent_raises_import_error_or_returns_default(): + with pytest.raises(ImportError): + agent_attr("fakeagent_not_installed", "moved", "fakeagent_not_installed_home") + assert agent_attr("fakeagent_not_installed", "moved", "fakeagent_not_installed_home", None) is None + + +def test_webui_does_not_use_removed_old_agent_paths(): + """Guard the old paths the Agent removed from being reintroduced in api/.""" + import ast + from pathlib import Path + + moved_imports = { + "tools.approval": {"set_current_session_key", "reset_current_session_key"}, + "tools.mcp_tool": {"discover_mcp_tools", "shutdown_mcp_servers", "get_mcp_status"}, + "agent.anthropic_adapter": {"read_claude_code_credentials", "is_claude_code_token_valid"}, + "hermes_cli.models": {"lmstudio_model_reasoning_options"}, + } + moved_kb = {"connect", "connect_closing", "dispatch_once"} + offenders = [] + for path in sorted((Path(__file__).resolve().parent.parent / "api").glob("*.py")): + for node in ast.walk(ast.parse(path.read_text(encoding="utf-8"))): + if isinstance(node, ast.ImportFrom) and node.module in moved_imports: + offenders += [(path.name, node.lineno, a.name) for a in node.names + if a.name in moved_imports[node.module]] + elif (isinstance(node, ast.Attribute) and node.attr in moved_kb + and isinstance(node.value, ast.Name) and node.value.id == "kb"): + offenders.append((path.name, node.lineno, f"kb.{node.attr}")) + elif (isinstance(node, ast.Call) and isinstance(node.func, ast.Name) + and node.func.id in {"getattr", "hasattr"} and len(node.args) >= 2 + and isinstance(node.args[0], ast.Name) and node.args[0].id == "kb" + and isinstance(node.args[1], ast.Constant) and node.args[1].value in moved_kb): + offenders.append((path.name, node.lineno, f"{node.func.id}(kb, {node.args[1].value!r})")) + assert not offenders, f"old agent paths (use api.agent_compat.agent_attr): {offenders}"