From 8abf95335bc18501bd48aa34d415c3552c229f40 Mon Sep 17 00:00:00 2001 From: Mike McHargue Date: Mon, 14 Sep 2026 18:42:38 -0700 Subject: [PATCH] test+docs: exercise moved-name call sites; document the agent_compat contract Review follow-up for the Hermes Agent moved-name fix. Tests: replace the AST source guard with behavior tests that run each affected WebUI entry point against two fake Agent shapes, pre_split (name on the original module) and pointer_removed (name only in its new home): approval session identity bind/reset, MCP runtime status, /reload-mcp, Claude Code credential read, LM Studio reasoning options, and kanban connections (closed after use), board counts, SSE event feed, and dispatch. All 8 pointer_removed cases fail against the pre-fix call sites; the pre_split cases pass on both. Docs: ARCHITECTURE.md gains the api/agent_compat.py inventory entry and section 4.9 covering ownership (Agent's compat_manifest.json), resolution order, call-site rules, and the removal policy. Co-Authored-By: Claude Opus 5 --- ARCHITECTURE.md | 34 ++++++ tests/test_agent_compat.py | 226 ++++++++++++++++++++++++++++++++----- 2 files changed, 233 insertions(+), 27 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index d465c724..72be8bc8 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -56,6 +56,7 @@ actions. The topbar remains focused on conversation context and the workspace/fi .dockerignore Excludes .git, tests/, .env* from Docker builds api/ __init__.py Package marker + agent_compat.py Resolver for Hermes Agent names moved to sibling modules (compatibility-only) auth.py Optional password authentication, signed cookies, passkeys/WebAuthn config.py Discovery, globals, model detection, reloadable config helpers.py HTTP helpers: j(), bad(), require(), safe_resolve(), security headers @@ -418,6 +419,39 @@ whose default `SessionDB()` path remains frozen at module import. Keep this fall compatibility-only: new goal semantics belong in Hermes Agent's native manager rather than a second WebUI implementation. +### 4.9 Hermes Agent Moved-Name Compatibility + +Hermes Agent owns its module layout. Its September 2026 decomposition moved names the +WebUI uses (for example `tools.approval.set_current_session_key` to +`tools.approval_context`) into `_` sibling modules. The old paths resolve +only through temporary PLUGIN-COMPAT `__getattr__` pointers that emit +`HermesPluginCompatWarning` and are removed on schedule. The Agent's +`compat_manifest.json` is the authoritative map of what moved where. + +WebUI code reaches a moved name only through +`api.agent_compat.agent_attr(owner, name, home, default=...)`, which resolves in this +order: + +1. the owner 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 `home` module: split Agents, with or without the old-path pointer (no warning); +3. plain attribute access on the owner: non-module test doubles. + +It raises like the import it replaces (or returns `default`), so each call site keeps its +existing fallback. Current users: approval session identity and MCP discovery +(`streaming.py`), `/reload-mcp` (`commands.py`), MCP runtime status (`routes.py`), Claude +Code credential linking (`oauth.py`), LM Studio reasoning options (`config.py`), and +kanban connections and dispatch (`kanban_bridge.py`). + +Import names that are still native to their module directly. Never +`from import `, and never feature-detect a moved name with +`hasattr`/`getattr` on the old module: once the pointers are removed those silently turn +"moved" into "missing" behind the call sites' broad `except` blocks. When a later Agent +split moves another name, route it through `agent_attr` and add pre-split and +pointer-removed cases to `tests/test_agent_compat.py`. The resolver is +compatibility-only: delete it, and import directly from the new homes, once the WebUI +stops supporting Agents that predate the split. + --- ## 5. Frontend Architecture: Current State diff --git a/tests/test_agent_compat.py b/tests/test_agent_compat.py index e489624b..883f8eb2 100644 --- a/tests/test_agent_compat.py +++ b/tests/test_agent_compat.py @@ -1,4 +1,4 @@ -"""api.agent_compat.agent_attr resolves Hermes Agent names that moved modules. +"""Hermes Agent names that moved modules keep working at every WebUI call site. The Agent's September 2026 decomposition moved names such as ``tools.approval.set_current_session_key`` into sibling modules. The old paths @@ -6,16 +6,27 @@ 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. + +The call-site tests run each affected WebUI entry point against two fake Agent +shapes: ``pre_split`` (the name still lives on its original module) and +``pointer_removed`` (the name lives only in its new home module, and the +original module no longer resolves it). """ +import contextlib +import sqlite3 import sys +import threading import types import warnings +from types import SimpleNamespace import pytest from api.agent_compat import agent_attr +SHAPES = ("pre_split", "pointer_removed") + class _CompatWarning(FutureWarning): pass @@ -33,6 +44,25 @@ def _install(monkeypatch, name, **attrs): return mod +def _install_agent(monkeypatch, shape, facade, moved, native=None): + """Install a fake Agent module ``facade``. + + ``moved`` maps each new home module to the ``{name: object}`` it now owns. + ``pre_split`` puts those names on ``facade``; ``pointer_removed`` installs + them only in their home modules. ``native`` names always stay on ``facade``. + """ + names = dict(native or {}) + for home, attrs in moved.items(): + if shape == "pre_split": + names.update(attrs) + else: + _install(monkeypatch, home, **attrs) + return _install(monkeypatch, facade, **names) + + +# ── resolver ───────────────────────────────────────────────────────────────── + + 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).""" @@ -98,30 +128,172 @@ def test_missing_agent_raises_import_error_or_returns_default(): 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 +# ── WebUI call sites ───────────────────────────────────────────────────────── - 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}" + +@pytest.mark.parametrize("shape", SHAPES) +def test_turn_session_identity_binds_and_resets_agent_approval_key(monkeypatch, shape): + import api.streaming as streaming + + calls = [] + + def set_current_session_key(sid): + calls.append(("set", sid)) + return f"token-{sid}" + + def reset_current_session_key(token): + calls.append(("reset", token)) + + _install_agent(monkeypatch, shape, "tools.approval", {"tools.approval_context": { + "set_current_session_key": set_current_session_key, + "reset_current_session_key": reset_current_session_key, + }}) + + tokens = streaming._set_turn_session_identity("sess-1") + assert tokens.get("approval") == "token-sess-1" + streaming._reset_turn_session_identity(tokens) + assert calls == [("set", "sess-1"), ("reset", "token-sess-1")] + + +@pytest.mark.parametrize("shape", SHAPES) +def test_mcp_runtime_status_reads_agent_registry(monkeypatch, shape): + import api.routes as routes + + statuses = [{"name": "alpha", "connected": True}, {"name": "beta", "connected": False}] + _install_agent(monkeypatch, shape, "tools.mcp_tool", {"tools.mcp_tool_discovery": { + "get_mcp_status": lambda: statuses, + }}) + + assert routes._mcp_runtime_status_by_name() == {"alpha": statuses[0], "beta": statuses[1]} + + +@pytest.mark.parametrize("shape", SHAPES) +def test_reload_mcp_command_shuts_down_and_rediscovers(monkeypatch, shape): + import api.commands as commands + + servers = {"old": object()} + calls = [] + + def shutdown_mcp_servers(): + calls.append("shutdown") + servers.clear() + + def discover_mcp_tools(): + calls.append("discover") + servers.update(old=object(), new=object()) + return ["t1", "t2", "t3"] + + _install_agent( + monkeypatch, shape, "tools.mcp_tool", + { + "tools.mcp_tool_lifecycle": {"shutdown_mcp_servers": shutdown_mcp_servers}, + "tools.mcp_tool_discovery": {"discover_mcp_tools": discover_mcp_tools}, + }, + native={"_servers": servers, "_lock": threading.Lock()}, + ) + + out = commands._run_reload_mcp_command() + assert calls == ["shutdown", "discover"] + assert "Reconnected: old" in out + assert "Added: new" in out + assert "3 tool(s) available across 2 server(s)" in out + + +@pytest.mark.parametrize("shape", SHAPES) +def test_claude_code_credentials_read_through_agent(monkeypatch, shape): + import api.oauth as oauth + + creds = {"accessToken": "cc-access", "refreshToken": "cc-refresh"} + _install_agent(monkeypatch, shape, "agent.anthropic_adapter", {"agent.anthropic_credentials": { + "read_claude_code_credentials": lambda: creds, + "is_claude_code_token_valid": lambda value: value is creds, + }}) + + assert oauth._read_claude_code_credentials() is creds + + +@pytest.mark.parametrize("shape", SHAPES) +def test_lmstudio_reasoning_options_use_agent_probe(monkeypatch, shape): + import api.config as config + + seen = [] + + def lmstudio_model_reasoning_options(model, base_url, api_key=None, timeout=5.0): + seen.append((model, base_url, timeout)) + return ["low", "high"] + + monkeypatch.setattr(config, "_lmstudio_reasoning_probe_options_fallback", lambda *a, **k: ["fallback"]) + _install_agent(monkeypatch, shape, "hermes_cli.models", {"hermes_cli.models_local": { + "lmstudio_model_reasoning_options": lmstudio_model_reasoning_options, + }}) + + assert config._lmstudio_model_reasoning_options("qwen", "http://127.0.0.1:1234", timeout=2.0) == ["low", "high"] + assert seen == [("qwen", "http://127.0.0.1:1234", 2.0)] + + +_KANBAN_SQL = """ +CREATE TABLE tasks (id INTEGER PRIMARY KEY, status TEXT); +INSERT INTO tasks (status) VALUES ('todo'), ('todo'), ('done'), ('archived'); +CREATE TABLE task_events ( + id INTEGER PRIMARY KEY, task_id TEXT, run_id TEXT, kind TEXT, payload TEXT, created_at INTEGER +); +INSERT INTO task_events VALUES (1, 't1', NULL, 'created', '{"a": 1}', 100), (2, 't2', 'r1', 'moved', NULL, 101); +""" + + +def _install_kanban_agent(monkeypatch, shape): + import api.kanban_bridge as bridge + + def connect(*, board=None): + conn = sqlite3.connect(":memory:") + conn.row_factory = sqlite3.Row + conn.executescript(_KANBAN_SQL) + return conn + + def connect_closing(*, board=None): + return contextlib.closing(connect(board=board)) + + def dispatch_once(conn, dry_run=False, max_spawn=8): + tasks = conn.execute("SELECT COUNT(*) FROM tasks").fetchone()[0] + return {"dry_run": dry_run, "max_spawn": max_spawn, "tasks_seen": tasks} + + kb = _install_agent( + monkeypatch, shape, "hermes_cli.kanban_db", + { + "hermes_cli.kanban_db_connect": {"connect": connect, "connect_closing": connect_closing}, + "hermes_cli.kanban_db_dispatch": {"dispatch_once": dispatch_once}, + }, + native={"init_db": lambda *, board=None: None, "board_exists": lambda slug: True, "DEFAULT_BOARD": "default"}, + ) + pkg = types.ModuleType("hermes_cli") + pkg.kanban_db = kb + monkeypatch.setitem(sys.modules, "hermes_cli", pkg) + return bridge + + +@pytest.mark.parametrize("shape", SHAPES) +def test_kanban_connection_is_closed_after_use(monkeypatch, shape): + bridge = _install_kanban_agent(monkeypatch, shape) + + with bridge._conn(board=None) as conn: + assert conn.execute("SELECT COUNT(*) FROM tasks").fetchone()[0] == 4 + with pytest.raises(sqlite3.ProgrammingError): + conn.execute("SELECT 1") # connect_closing, not a leaked raw connection + + +@pytest.mark.parametrize("shape", SHAPES) +def test_kanban_counts_and_event_feed_read_agent_db(monkeypatch, shape): + bridge = _install_kanban_agent(monkeypatch, shape) + + assert bridge._board_counts_for_slug("default") == {"todo": 2, "done": 1} + cursor, events = bridge._kanban_sse_fetch_new(None, 0) + assert cursor == 2 + assert [(e["id"], e["kind"], e["payload"]) for e in events] == [(1, "created", {"a": 1}), (2, "moved", None)] + + +@pytest.mark.parametrize("shape", SHAPES) +def test_kanban_dispatch_runs_agent_dispatcher(monkeypatch, shape): + bridge = _install_kanban_agent(monkeypatch, shape) + + parsed = SimpleNamespace(path="/api/kanban/dispatch", query="dry_run=true&max=3") + assert bridge._dispatch_payload(parsed) == {"dry_run": True, "max_spawn": 3, "tasks_seen": 4}