Merge branch 'main' of github.com:NousResearch/hermes-agent into feat/ink-refactor

This commit is contained in:
Brooklyn Nicholson
2026-04-14 19:33:03 -05:00
28 changed files with 2378 additions and 541 deletions

View File

@@ -31,18 +31,25 @@ def _clear_browser_caches():
class TestSanePath:
"""Verify _SANE_PATH includes Homebrew directories."""
"""Verify _SANE_PATH includes fallback directories used by browser_tool."""
def test_includes_termux_bin(self):
assert "/data/data/com.termux/files/usr/bin" in _SANE_PATH.split(os.pathsep)
def test_includes_termux_sbin(self):
assert "/data/data/com.termux/files/usr/sbin" in _SANE_PATH.split(os.pathsep)
def test_includes_homebrew_bin(self):
assert "/opt/homebrew/bin" in _SANE_PATH
assert "/opt/homebrew/bin" in _SANE_PATH.split(os.pathsep)
def test_includes_homebrew_sbin(self):
assert "/opt/homebrew/sbin" in _SANE_PATH
assert "/opt/homebrew/sbin" in _SANE_PATH.split(os.pathsep)
def test_includes_standard_dirs(self):
assert "/usr/local/bin" in _SANE_PATH
assert "/usr/bin" in _SANE_PATH
assert "/bin" in _SANE_PATH
path_parts = _SANE_PATH.split(os.pathsep)
assert "/usr/local/bin" in path_parts
assert "/usr/bin" in path_parts
assert "/bin" in path_parts
class TestDiscoverHomebrewNodeDirs:
@@ -143,6 +150,44 @@ class TestFindAgentBrowser:
result = _find_agent_browser()
assert result == "npx agent-browser"
def test_finds_npx_in_termux_fallback_path(self):
"""Should find npx when only Termux fallback dirs are available."""
def mock_which(cmd, path=None):
if cmd == "agent-browser":
return None
if cmd == "npx":
if path and "/data/data/com.termux/files/usr/bin" in path:
return "/data/data/com.termux/files/usr/bin/npx"
return None
return None
original_path_exists = Path.exists
def mock_path_exists(self):
if "node_modules" in str(self) and "agent-browser" in str(self):
return False
return original_path_exists(self)
real_isdir = os.path.isdir
def selective_isdir(path):
if path in (
"/data/data/com.termux/files/usr/bin",
"/data/data/com.termux/files/usr/sbin",
):
return True
return real_isdir(path)
with patch("shutil.which", side_effect=mock_which), \
patch("os.path.isdir", side_effect=selective_isdir), \
patch.object(Path, "exists", mock_path_exists), \
patch(
"tools.browser_tool._discover_homebrew_node_dirs",
return_value=[],
):
result = _find_agent_browser()
assert result == "npx agent-browser"
def test_raises_when_not_found(self):
"""Should raise FileNotFoundError when nothing works."""
original_path_exists = Path.exists
@@ -399,3 +444,51 @@ class TestRunBrowserCommandPathConstruction:
result_path = captured_env.get("PATH", "")
assert "/opt/homebrew/bin" in result_path
assert "/opt/homebrew/sbin" in result_path
def test_subprocess_path_includes_termux_fallback_dirs(self, tmp_path):
"""Termux fallback dirs should survive browser PATH rebuilding."""
captured_env = {}
mock_proc = MagicMock()
mock_proc.returncode = 0
mock_proc.wait.return_value = 0
def capture_popen(cmd, **kwargs):
captured_env.update(kwargs.get("env", {}))
return mock_proc
fake_session = {
"session_name": "test-session",
"session_id": "test-id",
"cdp_url": None,
}
fake_json = json.dumps({"success": True})
real_isdir = os.path.isdir
def selective_isdir(path):
if path in (
"/data/data/com.termux/files/usr/bin",
"/data/data/com.termux/files/usr/sbin",
):
return True
if path.startswith(str(tmp_path)):
return True
return real_isdir(path)
with patch("tools.browser_tool._find_agent_browser", return_value="/usr/local/bin/agent-browser"), \
patch("tools.browser_tool._get_session_info", return_value=fake_session), \
patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \
patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \
patch("os.path.isdir", side_effect=selective_isdir), \
patch("subprocess.Popen", side_effect=capture_popen), \
patch("os.open", return_value=99), \
patch("os.close"), \
patch("tools.interrupt.is_interrupted", return_value=False), \
patch.dict(os.environ, {"PATH": "/usr/bin:/bin", "HOME": "/home/test"}, clear=True):
with patch("builtins.open", mock_open(read_data=fake_json)):
_run_browser_command("test-task", "navigate", ["https://example.com"])
result_path = captured_env.get("PATH", "")
assert "/data/data/com.termux/files/usr/bin" in result_path
assert "/data/data/com.termux/files/usr/sbin" in result_path

View File

@@ -21,34 +21,19 @@ class TestRegisterServerTools:
def mock_registry(self):
return ToolRegistry()
@pytest.fixture
def mock_toolsets(self):
return {
"hermes-cli": {"tools": ["terminal"], "description": "CLI", "includes": []},
"hermes-telegram": {"tools": ["terminal"], "description": "TG", "includes": []},
"custom-toolset": {"tools": [], "description": "Other", "includes": []},
}
def test_injects_hermes_toolsets(self, mock_registry, mock_toolsets):
"""Tools are injected into hermes-* toolsets but not custom ones."""
def test_exposes_live_server_aliases(self, mock_registry):
"""Registered MCP tools are reachable via live raw-server aliases."""
server = MCPServerTask("my_srv")
server._tools = [_make_mcp_tool("my_tool", "desc")]
server.session = MagicMock()
from toolsets import resolve_toolset, validate_toolset
with patch("tools.registry.registry", mock_registry), \
patch("toolsets.create_custom_toolset"), \
patch.dict("toolsets.TOOLSETS", mock_toolsets, clear=True):
with patch("tools.registry.registry", mock_registry):
registered = _register_server_tools("my_srv", server, {})
assert "mcp_my_srv_my_tool" in registered
assert "mcp_my_srv_my_tool" in mock_registry.get_all_tool_names()
# Injected into hermes-* toolsets
assert "mcp_my_srv_my_tool" in mock_toolsets["hermes-cli"]["tools"]
assert "mcp_my_srv_my_tool" in mock_toolsets["hermes-telegram"]["tools"]
# NOT into non-hermes toolsets
assert "mcp_my_srv_my_tool" not in mock_toolsets["custom-toolset"]["tools"]
assert "mcp_my_srv_my_tool" in registered
assert "mcp_my_srv_my_tool" in mock_registry.get_all_tool_names()
assert validate_toolset("my_srv") is True
assert "mcp_my_srv_my_tool" in resolve_toolset("my_srv")
class TestRefreshTools:
@@ -58,19 +43,13 @@ class TestRefreshTools:
def mock_registry(self):
return ToolRegistry()
@pytest.fixture
def mock_toolsets(self):
return {
"hermes-cli": {"tools": ["terminal"], "description": "CLI", "includes": []},
"hermes-telegram": {"tools": ["terminal"], "description": "TG", "includes": []},
}
@pytest.mark.asyncio
async def test_nuke_and_repave(self, mock_registry, mock_toolsets):
async def test_nuke_and_repave(self, mock_registry):
"""Old tools are removed and new tools registered on refresh."""
server = MCPServerTask("live_srv")
server._refresh_lock = asyncio.Lock()
server._config = {}
from toolsets import resolve_toolset
# Seed initial state: one old tool registered
mock_registry.register(
@@ -79,7 +58,6 @@ class TestRefreshTools:
description="", emoji="",
)
server._registered_tool_names = ["mcp_live_srv_old_tool"]
mock_toolsets["hermes-cli"]["tools"].append("mcp_live_srv_old_tool")
# New tool list from server
new_tool = _make_mcp_tool("new_tool", "new behavior")
@@ -89,20 +67,13 @@ class TestRefreshTools:
)
)
with patch("tools.registry.registry", mock_registry), \
patch("toolsets.create_custom_toolset"), \
patch.dict("toolsets.TOOLSETS", mock_toolsets, clear=True):
with patch("tools.registry.registry", mock_registry):
await server._refresh_tools()
# Old tool completely gone
assert "mcp_live_srv_old_tool" not in mock_registry.get_all_tool_names()
assert "mcp_live_srv_old_tool" not in mock_toolsets["hermes-cli"]["tools"]
# New tool registered
assert "mcp_live_srv_new_tool" in mock_registry.get_all_tool_names()
assert "mcp_live_srv_new_tool" in mock_toolsets["hermes-cli"]["tools"]
assert server._registered_tool_names == ["mcp_live_srv_new_tool"]
assert "mcp_live_srv_old_tool" not in mock_registry.get_all_tool_names()
assert "mcp_live_srv_old_tool" not in resolve_toolset("live_srv")
assert "mcp_live_srv_new_tool" in mock_registry.get_all_tool_names()
assert "mcp_live_srv_new_tool" in resolve_toolset("live_srv")
assert server._registered_tool_names == ["mcp_live_srv_new_tool"]
class TestMessageHandler:
@@ -165,6 +136,25 @@ class TestDeregister:
# bar still in ts1, so check should remain
assert "ts1" in reg._toolset_checks
def test_removes_toolset_alias_when_last_tool_is_removed(self):
reg = ToolRegistry()
reg.register(name="foo", toolset="mcp-srv", schema={}, handler=lambda x: x)
reg.register_toolset_alias("srv", "mcp-srv")
reg.deregister("foo")
assert reg.get_toolset_alias_target("srv") is None
def test_preserves_toolset_alias_while_toolset_still_exists(self):
reg = ToolRegistry()
reg.register(name="foo", toolset="mcp-srv", schema={}, handler=lambda x: x)
reg.register(name="bar", toolset="mcp-srv", schema={}, handler=lambda x: x)
reg.register_toolset_alias("srv", "mcp-srv")
reg.deregister("foo")
assert reg.get_toolset_alias_target("srv") == "mcp-srv"
def test_noop_for_unknown_tool(self):
reg = ToolRegistry()
reg.deregister("nonexistent") # Should not raise

View File

@@ -184,11 +184,7 @@ class TestToolHandler:
def _patch_mcp_loop(self, coro_side_effect=None):
"""Return a patch for _run_on_mcp_loop that runs the coroutine directly."""
def fake_run(coro, timeout=30):
loop = asyncio.new_event_loop()
try:
return loop.run_until_complete(coro)
finally:
loop.close()
return asyncio.run(coro)
if coro_side_effect:
return patch("tools.mcp_tool._run_on_mcp_loop", side_effect=coro_side_effect)
return patch("tools.mcp_tool._run_on_mcp_loop", side_effect=fake_run)
@@ -365,10 +361,13 @@ class TestDiscoverAndRegister:
_servers.pop("fs", None)
def test_toolset_created(self):
"""A custom toolset is created for the MCP server."""
def test_toolset_resolves_live_from_registry(self):
"""MCP toolsets resolve through the live registry without TOOLSETS mutation."""
from tools.registry import ToolRegistry
from tools.mcp_tool import _discover_and_register_server, _servers, MCPServerTask
from toolsets import resolve_toolset, validate_toolset
mock_registry = ToolRegistry()
mock_tools = [_make_mcp_tool("ping", "Ping")]
mock_session = MagicMock()
@@ -378,16 +377,16 @@ class TestDiscoverAndRegister:
server._tools = mock_tools
return server
mock_create = MagicMock()
with patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \
patch("toolsets.create_custom_toolset", mock_create):
patch("tools.registry.registry", mock_registry):
asyncio.run(
_discover_and_register_server("myserver", {"command": "test"})
)
mock_create.assert_called_once()
call_kwargs = mock_create.call_args
assert call_kwargs[1]["name"] == "mcp-myserver" or call_kwargs[0][0] == "mcp-myserver"
assert validate_toolset("myserver") is True
assert validate_toolset("mcp-myserver") is True
assert "mcp_myserver_ping" in resolve_toolset("myserver")
assert "mcp_myserver_ping" in resolve_toolset("mcp-myserver")
_servers.pop("myserver", None)
@@ -550,12 +549,15 @@ class TestMCPServerTask:
# ---------------------------------------------------------------------------
class TestToolsetInjection:
def test_mcp_tools_added_to_all_hermes_toolsets(self):
"""Discovered MCP tools are dynamically injected into all hermes-* toolsets."""
def test_mcp_tools_resolve_through_server_aliases(self):
"""Discovered MCP tools resolve through raw server-name aliases."""
from tools.mcp_tool import MCPServerTask
from tools.registry import ToolRegistry
from toolsets import resolve_toolset, validate_toolset
mock_tools = [_make_mcp_tool("list_files", "List files")]
mock_session = MagicMock()
mock_registry = ToolRegistry()
fresh_servers = {}
@@ -565,43 +567,32 @@ class TestToolsetInjection:
server._tools = mock_tools
return server
fake_toolsets = {
"hermes-cli": {"tools": ["terminal"], "description": "CLI", "includes": []},
"hermes-telegram": {"tools": ["terminal"], "description": "TG", "includes": []},
"hermes-gateway": {"tools": [], "description": "GW", "includes": []},
"non-hermes": {"tools": [], "description": "other", "includes": []},
}
fake_config = {"fs": {"command": "npx", "args": []}}
with patch("tools.mcp_tool._MCP_AVAILABLE", True), \
patch("tools.mcp_tool._servers", fresh_servers), \
patch("tools.mcp_tool._load_mcp_config", return_value=fake_config), \
patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \
patch("toolsets.TOOLSETS", fake_toolsets):
patch("tools.registry.registry", mock_registry):
from tools.mcp_tool import discover_mcp_tools
result = discover_mcp_tools()
assert "mcp_fs_list_files" in result
# All hermes-* toolsets get injection
assert "mcp_fs_list_files" in fake_toolsets["hermes-cli"]["tools"]
assert "mcp_fs_list_files" in fake_toolsets["hermes-telegram"]["tools"]
assert "mcp_fs_list_files" in fake_toolsets["hermes-gateway"]["tools"]
# Non-hermes toolset should NOT get injection
assert "mcp_fs_list_files" not in fake_toolsets["non-hermes"]["tools"]
# Original tools preserved
assert "terminal" in fake_toolsets["hermes-cli"]["tools"]
# Server name becomes a standalone toolset
assert "fs" in fake_toolsets
assert "mcp_fs_list_files" in fake_toolsets["fs"]["tools"]
assert fake_toolsets["fs"]["description"].startswith("MCP server '")
assert "mcp_fs_list_files" in result
assert validate_toolset("fs") is True
assert validate_toolset("mcp-fs") is True
assert "mcp_fs_list_files" in resolve_toolset("fs")
assert "mcp_fs_list_files" in resolve_toolset("mcp-fs")
def test_server_toolset_skips_builtin_collision(self):
"""MCP server named after a built-in toolset shouldn't overwrite it."""
"""MCP raw aliases never overwrite a built-in toolset name."""
from tools.mcp_tool import MCPServerTask
from tools.registry import ToolRegistry
from toolsets import resolve_toolset, validate_toolset
mock_tools = [_make_mcp_tool("run", "Run command")]
mock_session = MagicMock()
fresh_servers = {}
mock_registry = ToolRegistry()
async def fake_connect(name, config):
server = MCPServerTask(name)
@@ -620,12 +611,15 @@ class TestToolsetInjection:
patch("tools.mcp_tool._servers", fresh_servers), \
patch("tools.mcp_tool._load_mcp_config", return_value=fake_config), \
patch("tools.mcp_tool._connect_server", side_effect=fake_connect), \
patch("tools.registry.registry", mock_registry), \
patch("toolsets.TOOLSETS", fake_toolsets):
from tools.mcp_tool import discover_mcp_tools
discover_mcp_tools()
# Built-in toolset preserved — description unchanged
assert fake_toolsets["terminal"]["description"] == "Terminal tools"
assert fake_toolsets["terminal"]["description"] == "Terminal tools"
assert "mcp_terminal_run" not in resolve_toolset("terminal")
assert validate_toolset("mcp-terminal") is True
assert "mcp_terminal_run" in resolve_toolset("mcp-terminal")
def test_server_connection_failure_skipped(self):
"""If one server fails to connect, others still proceed."""
@@ -776,6 +770,42 @@ class TestShutdown:
assert len(_servers) == 0
mock_server.shutdown.assert_called_once()
def test_shutdown_deregisters_registered_tools(self):
"""shutdown_mcp_servers removes MCP tools and their raw alias."""
import tools.mcp_tool as mcp_mod
from tools.mcp_tool import MCPServerTask, shutdown_mcp_servers, _servers
from tools.registry import registry
from toolsets import resolve_toolset, validate_toolset
_servers.clear()
registry.register(
name="mcp_test_ping",
toolset="mcp-test",
schema={
"name": "mcp_test_ping",
"description": "Ping",
"parameters": {"type": "object", "properties": {}},
},
handler=lambda *_args, **_kwargs: "{}",
)
registry.register_toolset_alias("test", "mcp-test")
server = MCPServerTask("test")
server._registered_tool_names = ["mcp_test_ping"]
_servers["test"] = server
mcp_mod._ensure_mcp_loop()
try:
assert validate_toolset("test") is True
assert "mcp_test_ping" in resolve_toolset("test")
shutdown_mcp_servers()
finally:
mcp_mod._mcp_loop = None
mcp_mod._mcp_thread = None
assert "mcp_test_ping" not in registry.get_all_tool_names()
assert validate_toolset("test") is False
def test_shutdown_handles_errors(self):
"""shutdown_mcp_servers handles errors during close gracefully."""
import tools.mcp_tool as mcp_mod
@@ -1179,7 +1209,11 @@ class TestConfigurableTimeouts:
try:
handler = _make_tool_handler("test_srv", "my_tool", 180)
with patch("tools.mcp_tool._run_on_mcp_loop") as mock_run:
mock_run.return_value = json.dumps({"result": "ok"})
def fake_run(coro, timeout=30):
coro.close()
return json.dumps({"result": "ok"})
mock_run.side_effect = fake_run
handler({})
# Verify timeout=180 was passed
call_kwargs = mock_run.call_args
@@ -1279,11 +1313,7 @@ class TestUtilityHandlers:
def _patch_mcp_loop(self):
"""Return a patch for _run_on_mcp_loop that runs the coroutine directly."""
def fake_run(coro, timeout=30):
loop = asyncio.new_event_loop()
try:
return loop.run_until_complete(coro)
finally:
loop.close()
return asyncio.run(coro)
return patch("tools.mcp_tool._run_on_mcp_loop", side_effect=fake_run)
# -- list_resources --
@@ -3038,14 +3068,23 @@ class TestSanitizeMcpNameComponent:
assert "/" not in name
assert "." not in name
def test_slash_in_sync_mcp_toolsets(self):
"""_sync_mcp_toolsets uses sanitize consistently with _convert_mcp_schema."""
from tools.mcp_tool import sanitize_mcp_name_component
def test_slash_in_server_alias_resolution(self):
"""Server names with slashes resolve through their live MCP alias."""
from tools.registry import ToolRegistry
from toolsets import resolve_toolset, validate_toolset
# Verify the prefix generation matches what _convert_mcp_schema produces
server_name = "ai.exa/exa"
safe_prefix = f"mcp_{sanitize_mcp_name_component(server_name)}_"
assert safe_prefix == "mcp_ai_exa_exa_"
reg = ToolRegistry()
reg.register(
name="mcp_ai_exa_exa_search",
toolset="mcp-ai.exa/exa",
schema={"name": "mcp_ai_exa_exa_search", "description": "Search", "parameters": {"type": "object", "properties": {}}},
handler=lambda *_args, **_kwargs: "{}",
)
reg.register_toolset_alias("ai.exa/exa", "mcp-ai.exa/exa")
with patch("tools.registry.registry", reg):
assert validate_toolset("ai.exa/exa") is True
assert "mcp_ai_exa_exa_search" in resolve_toolset("ai.exa/exa")
# ---------------------------------------------------------------------------