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 <noreply@anthropic.com>
This commit is contained in:
1 parent
1f7e5e78da
commit
8abf95335b
2 files changed
+233
-27
No files matched your search
@@ -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 `<stem>_<topic>` 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 <old module> import <moved name>`, 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
|
||||
|
||||
+199
-27
@@ -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}
|
||||
Reference in new issue
Block a user