Merge branch 'main' of github.com:NousResearch/hermes-agent into bb/gui
# Conflicts: # cli.py # hermes_cli/main.py # run_agent.py # tests/hermes_cli/test_cmd_update.py # tools/mcp_tool.py # web/src/lib/gatewayClient.ts
This commit is contained in:
@@ -68,10 +68,10 @@ class TestDiscoverHomebrewNodeDirs:
|
||||
if p == "/opt/homebrew/opt":
|
||||
return True
|
||||
# node@20/bin and node@24/bin exist
|
||||
if p in (
|
||||
if p in {
|
||||
"/opt/homebrew/opt/node@20/bin",
|
||||
"/opt/homebrew/opt/node@24/bin",
|
||||
):
|
||||
}:
|
||||
return True
|
||||
return False
|
||||
|
||||
@@ -171,10 +171,10 @@ class TestFindAgentBrowser:
|
||||
real_isdir = os.path.isdir
|
||||
|
||||
def selective_isdir(path):
|
||||
if path in (
|
||||
if path in {
|
||||
"/data/data/com.termux/files/usr/bin",
|
||||
"/data/data/com.termux/files/usr/sbin",
|
||||
):
|
||||
}:
|
||||
return True
|
||||
return real_isdir(path)
|
||||
|
||||
@@ -486,10 +486,10 @@ class TestRunBrowserCommandPathConstruction:
|
||||
real_isdir = os.path.isdir
|
||||
|
||||
def selective_isdir(path):
|
||||
if path in (
|
||||
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
|
||||
|
||||
@@ -125,7 +125,7 @@ class TestResolveChildPython(unittest.TestCase):
|
||||
def test_project_with_no_venv_falls_back(self):
|
||||
"""Project mode without VIRTUAL_ENV or CONDA_PREFIX → sys.executable."""
|
||||
env = {k: v for k, v in os.environ.items()
|
||||
if k not in ("VIRTUAL_ENV", "CONDA_PREFIX")}
|
||||
if k not in {"VIRTUAL_ENV", "CONDA_PREFIX"}}
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
self.assertEqual(_resolve_child_python("project"), sys.executable)
|
||||
|
||||
|
||||
@@ -1014,6 +1014,89 @@ class TestDelegationCredentialResolution(unittest.TestCase):
|
||||
self.assertIsNone(creds["model"])
|
||||
self.assertIsNone(creds["provider"])
|
||||
|
||||
@patch("hermes_cli.runtime_provider.resolve_runtime_provider")
|
||||
def test_named_custom_provider_preserves_provider_name(self, mock_resolve):
|
||||
"""Named custom provider (e.g. crof.ai) resolves to 'custom' at runtime level
|
||||
but the subagent must retain the original provider identity so that
|
||||
resolve_provider_client routes to the correct endpoint on retry/fallback.
|
||||
Regression test for #26954.
|
||||
"""
|
||||
mock_resolve.return_value = {
|
||||
"provider": "custom", # runtime marks it as "custom" type
|
||||
"model": "deepseek-v4-pro-CEER",
|
||||
"base_url": "https://api.crof.ai/v1",
|
||||
"api_key": "crof-key-abc",
|
||||
"api_mode": "chat_completions",
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
cfg = {"model": "deepseek-v4-pro-CEER", "provider": "crof.ai"}
|
||||
creds = _resolve_delegation_credentials(cfg, parent)
|
||||
# The key assertion: subagent must keep "crof.ai", NOT "custom"
|
||||
self.assertEqual(creds["provider"], "crof.ai")
|
||||
self.assertEqual(creds["model"], "deepseek-v4-pro-CEER")
|
||||
self.assertEqual(creds["base_url"], "https://api.crof.ai/v1")
|
||||
self.assertEqual(creds["api_key"], "crof-key-abc")
|
||||
# Verify resolve_runtime_provider was called with the configured name
|
||||
mock_resolve.assert_called_once_with(
|
||||
requested="crof.ai", target_model="deepseek-v4-pro-CEER"
|
||||
)
|
||||
|
||||
@patch("hermes_cli.runtime_provider.resolve_runtime_provider")
|
||||
def test_standard_provider_not_overwritten_by_configured_name(self, mock_resolve):
|
||||
"""Standard (non-custom) providers must still return runtime identity,
|
||||
not the configured name, to preserve existing behaviour for openrouter,
|
||||
nous, etc.
|
||||
"""
|
||||
mock_resolve.return_value = {
|
||||
"provider": "openrouter",
|
||||
"model": "anthropic/claude-sonnet-4",
|
||||
"base_url": "https://openrouter.ai/api/v1",
|
||||
"api_key": "or-key-xyz",
|
||||
"api_mode": "chat_completions",
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
cfg = {"model": "anthropic/claude-sonnet-4", "provider": "openrouter"}
|
||||
creds = _resolve_delegation_credentials(cfg, parent)
|
||||
# Standard provider returns its own name, not "custom"
|
||||
self.assertEqual(creds["provider"], "openrouter")
|
||||
|
||||
@patch("hermes_cli.runtime_provider.resolve_runtime_provider")
|
||||
def test_custom_provider_with_empty_configured_provider_falls_back_to_runtime(self, mock_resolve):
|
||||
"""When configured_provider is empty/None, the early return kicks in and
|
||||
we return provider=None regardless of what runtime resolved. The runtime
|
||||
path is only reached when configured_provider is a non-empty string.
|
||||
"""
|
||||
mock_resolve.return_value = {
|
||||
"provider": "custom",
|
||||
"model": "some-model",
|
||||
"base_url": "https://fallback.example.com/v1",
|
||||
"api_key": "key-fallback",
|
||||
"api_mode": "chat_completions",
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
cfg = {"model": "some-model", "provider": ""}
|
||||
creds = _resolve_delegation_credentials(cfg, parent)
|
||||
# Empty provider → early return with None (child inherits parent)
|
||||
self.assertIsNone(creds["provider"])
|
||||
|
||||
@patch("hermes_cli.runtime_provider.resolve_runtime_provider")
|
||||
def test_runtime_missing_provider_key_returns_none(self, mock_resolve):
|
||||
"""When resolve_runtime_provider returns a dict without 'provider' key,
|
||||
the result must be None regardless of configured_provider.
|
||||
This protects against malformed runtime responses.
|
||||
"""
|
||||
mock_resolve.return_value = {
|
||||
# deliberately missing "provider"
|
||||
"model": "some-model",
|
||||
"base_url": "https://example.com/v1",
|
||||
"api_key": "key-123",
|
||||
"api_mode": "chat_completions",
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
cfg = {"model": "some-model", "provider": "crof.ai"}
|
||||
creds = _resolve_delegation_credentials(cfg, parent)
|
||||
self.assertIsNone(creds["provider"])
|
||||
|
||||
|
||||
class TestDelegationProviderIntegration(unittest.TestCase):
|
||||
"""Integration tests: delegation config → _run_single_child → AIAgent construction."""
|
||||
|
||||
@@ -633,7 +633,7 @@ class TestToolsetInclusion:
|
||||
def test_discord_tools_not_in_other_toolsets(self):
|
||||
from toolsets import TOOLSETS
|
||||
for name, ts in TOOLSETS.items():
|
||||
if name in ("hermes-discord", "hermes-gateway", "discord", "discord_admin"):
|
||||
if name in {"hermes-discord", "hermes-gateway", "discord", "discord_admin"}:
|
||||
continue
|
||||
tools = ts.get("tools", [])
|
||||
assert "discord" not in tools or name == "discord", (
|
||||
|
||||
@@ -121,6 +121,20 @@ def test_dockerfile_installs_tui_dependencies(dockerfile_text):
|
||||
)
|
||||
|
||||
|
||||
def test_dockerfile_preinstalls_gateway_messaging_dependencies(dockerfile_text):
|
||||
sync_steps = [
|
||||
step for step in _run_steps(dockerfile_text)
|
||||
if "uv sync" in step and "--no-install-project" in step
|
||||
]
|
||||
|
||||
assert sync_steps, "Dockerfile must install Python dependencies with uv sync"
|
||||
assert any("--extra messaging" in step for step in sync_steps), (
|
||||
"Published Docker images must preload the [messaging] extra so "
|
||||
"Telegram/Discord gateway adapters do not depend on first-boot "
|
||||
"lazy installation (#24698)."
|
||||
)
|
||||
|
||||
|
||||
def test_dockerfile_builds_tui_assets(dockerfile_text):
|
||||
assert any(
|
||||
"ui-tui" in step and "npm" in step and "run build" in step
|
||||
|
||||
@@ -24,7 +24,7 @@ def _new_filter_matches(path: Path) -> bool:
|
||||
|
||||
Returns True when the path SHOULD be filtered out.
|
||||
"""
|
||||
return any(part in ('.git', '.github', '.hub') for part in path.parts)
|
||||
return any(part in {'.git', '.github', '.hub'} for part in path.parts)
|
||||
|
||||
|
||||
class TestOldFilterBrokenOnWindows:
|
||||
|
||||
@@ -10,7 +10,9 @@ from unittest.mock import patch
|
||||
import pytest
|
||||
|
||||
|
||||
TOOLS_DIR = Path(__file__).resolve().parents[2] / "tools"
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
TOOLS_DIR = REPO_ROOT / "tools"
|
||||
PLUGINS_DIR = REPO_ROOT / "plugins"
|
||||
|
||||
|
||||
def _load_tool_module(module_name: str, filename: str):
|
||||
@@ -22,6 +24,21 @@ def _load_tool_module(module_name: str, filename: str):
|
||||
return module
|
||||
|
||||
|
||||
def _load_plugin_module(module_name: str, relpath: str):
|
||||
"""Load a plugin module by file path from ``plugins/``.
|
||||
|
||||
Mirror of :func:`_load_tool_module` for the plugin tree. Used by tests
|
||||
that exercise the per-vendor browser plugins' session-lifecycle
|
||||
behaviour after the PR #25214 migration.
|
||||
"""
|
||||
spec = spec_from_file_location(module_name, PLUGINS_DIR / relpath)
|
||||
assert spec and spec.loader
|
||||
module = module_from_spec(spec)
|
||||
sys.modules[module_name] = module
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
def _reset_modules(prefixes: tuple[str, ...]):
|
||||
for name in list(sys.modules):
|
||||
if name.startswith(prefixes):
|
||||
@@ -76,6 +93,48 @@ def _install_fake_tools_package():
|
||||
call_llm=lambda *args, **kwargs: "",
|
||||
)
|
||||
|
||||
# Stubs for the browser-provider plugin layer introduced in PR #25214.
|
||||
# The fake `agent` package has an empty __path__ so real submodules
|
||||
# aren't reachable; we install just enough stand-ins to satisfy
|
||||
# ``tools.browser_tool``'s top-level imports. The actual lifecycle
|
||||
# tests instantiate the real plugin classes via _load_tool_module
|
||||
# below, so the stubs only need to satisfy import + isinstance.
|
||||
class _StubBrowserProvider:
|
||||
"""Minimal BrowserProvider stub for ``from agent.browser_provider import BrowserProvider``."""
|
||||
|
||||
sys.modules["agent.browser_provider"] = types.SimpleNamespace(
|
||||
BrowserProvider=_StubBrowserProvider,
|
||||
)
|
||||
sys.modules["agent.browser_registry"] = types.SimpleNamespace(
|
||||
get_provider=lambda name: None,
|
||||
list_providers=lambda: [],
|
||||
register_provider=lambda provider: None,
|
||||
_resolve=lambda configured: None,
|
||||
)
|
||||
|
||||
# Plugin module stubs — the real plugin classes are loaded from disk by
|
||||
# the lifecycle tests below via _load_tool_module(). For the import
|
||||
# phase, we just need the class names to exist on the right module path.
|
||||
plugins_package = types.ModuleType("plugins")
|
||||
plugins_package.__path__ = [] # type: ignore[attr-defined]
|
||||
sys.modules["plugins"] = plugins_package
|
||||
plugins_browser_package = types.ModuleType("plugins.browser")
|
||||
plugins_browser_package.__path__ = [] # type: ignore[attr-defined]
|
||||
sys.modules["plugins.browser"] = plugins_browser_package
|
||||
|
||||
for _name, _classname in (
|
||||
("browserbase", "BrowserbaseBrowserProvider"),
|
||||
("browser_use", "BrowserUseBrowserProvider"),
|
||||
("firecrawl", "FirecrawlBrowserProvider"),
|
||||
):
|
||||
_vendor_pkg = types.ModuleType(f"plugins.browser.{_name}")
|
||||
_vendor_pkg.__path__ = [] # type: ignore[attr-defined]
|
||||
sys.modules[f"plugins.browser.{_name}"] = _vendor_pkg
|
||||
_provider_stub_cls = type(_classname, (_StubBrowserProvider,), {})
|
||||
sys.modules[f"plugins.browser.{_name}.provider"] = types.SimpleNamespace(
|
||||
**{_classname: _provider_stub_cls},
|
||||
)
|
||||
|
||||
sys.modules["tools.managed_tool_gateway"] = _load_tool_module(
|
||||
"tools.managed_tool_gateway",
|
||||
"managed_tool_gateway.py",
|
||||
@@ -157,13 +216,13 @@ def test_browserbase_does_not_use_gateway_only_configuration():
|
||||
})
|
||||
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
browserbase_module = _load_tool_module(
|
||||
"tools.browser_providers.browserbase",
|
||||
"browser_providers/browserbase.py",
|
||||
browserbase_module = _load_plugin_module(
|
||||
"plugins.browser.browserbase.provider",
|
||||
"browser/browserbase/provider.py",
|
||||
)
|
||||
provider = browserbase_module.BrowserbaseProvider()
|
||||
provider = browserbase_module.BrowserbaseBrowserProvider()
|
||||
|
||||
assert provider.is_configured() is False
|
||||
assert provider.is_available() is False
|
||||
|
||||
|
||||
def test_browser_use_managed_gateway_adds_idempotency_key_and_persists_external_call_id():
|
||||
@@ -188,13 +247,13 @@ def test_browser_use_managed_gateway_adds_idempotency_key_and_persists_external_
|
||||
}
|
||||
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
browser_use_module = _load_tool_module(
|
||||
"tools.browser_providers.browser_use",
|
||||
"browser_providers/browser_use.py",
|
||||
browser_use_module = _load_plugin_module(
|
||||
"plugins.browser.browser_use.provider",
|
||||
"browser/browser_use/provider.py",
|
||||
)
|
||||
|
||||
with patch.object(browser_use_module.requests, "post", return_value=_Response()) as post:
|
||||
provider = browser_use_module.BrowserUseProvider()
|
||||
provider = browser_use_module.BrowserUseBrowserProvider()
|
||||
session = provider.create_session("task-browser-use-managed")
|
||||
|
||||
sent_headers = post.call_args.kwargs["headers"]
|
||||
@@ -228,11 +287,11 @@ def test_browser_use_managed_gateway_reuses_pending_idempotency_key_after_timeou
|
||||
}
|
||||
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
browser_use_module = _load_tool_module(
|
||||
"tools.browser_providers.browser_use",
|
||||
"browser_providers/browser_use.py",
|
||||
browser_use_module = _load_plugin_module(
|
||||
"plugins.browser.browser_use.provider",
|
||||
"browser/browser_use/provider.py",
|
||||
)
|
||||
provider = browser_use_module.BrowserUseProvider()
|
||||
provider = browser_use_module.BrowserUseBrowserProvider()
|
||||
timeout = browser_use_module.requests.Timeout("timed out")
|
||||
|
||||
with patch.object(
|
||||
@@ -290,11 +349,11 @@ def test_browser_use_managed_gateway_preserves_pending_idempotency_key_for_in_pr
|
||||
}
|
||||
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
browser_use_module = _load_tool_module(
|
||||
"tools.browser_providers.browser_use",
|
||||
"browser_providers/browser_use.py",
|
||||
browser_use_module = _load_plugin_module(
|
||||
"plugins.browser.browser_use.provider",
|
||||
"browser/browser_use/provider.py",
|
||||
)
|
||||
provider = browser_use_module.BrowserUseProvider()
|
||||
provider = browser_use_module.BrowserUseBrowserProvider()
|
||||
|
||||
with patch.object(
|
||||
browser_use_module.requests,
|
||||
@@ -337,11 +396,11 @@ def test_browser_use_managed_gateway_uses_new_idempotency_key_for_a_new_session_
|
||||
}
|
||||
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
browser_use_module = _load_tool_module(
|
||||
"tools.browser_providers.browser_use",
|
||||
"browser_providers/browser_use.py",
|
||||
browser_use_module = _load_plugin_module(
|
||||
"plugins.browser.browser_use.provider",
|
||||
"browser/browser_use/provider.py",
|
||||
)
|
||||
provider = browser_use_module.BrowserUseProvider()
|
||||
provider = browser_use_module.BrowserUseBrowserProvider()
|
||||
|
||||
with patch.object(browser_use_module.requests, "post", side_effect=[_Response(), _Response()]) as post:
|
||||
provider.create_session("task-browser-use-new")
|
||||
|
||||
@@ -33,7 +33,7 @@ def _restore_tool_and_agent_modules():
|
||||
original_modules = {
|
||||
name: module
|
||||
for name, module in sys.modules.items()
|
||||
if name in ("tools", "agent", "hermes_cli")
|
||||
if name in {"tools", "agent", "hermes_cli"}
|
||||
or name.startswith("tools.")
|
||||
or name.startswith("agent.")
|
||||
or name.startswith("hermes_cli.")
|
||||
|
||||
@@ -62,7 +62,7 @@ class TestCancelledErrorPropagation:
|
||||
return "clean_return"
|
||||
|
||||
outcome = asyncio.run(drive())
|
||||
assert outcome in ("cancelled_cleanly", "clean_return"), (
|
||||
assert outcome in {"cancelled_cleanly", "clean_return"}, (
|
||||
f"MCPServerTask.run wedged on cancel (outcome={outcome}) — "
|
||||
f"#9930 regression"
|
||||
)
|
||||
|
||||
@@ -10,6 +10,8 @@ from unittest.mock import patch, MagicMock, AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
import asyncio
|
||||
|
||||
from tools.mcp_oauth import (
|
||||
HermesTokenStorage,
|
||||
OAuthNonInteractiveError,
|
||||
@@ -20,6 +22,7 @@ from tools.mcp_oauth import (
|
||||
_is_interactive,
|
||||
_wait_for_callback,
|
||||
_make_callback_handler,
|
||||
_redirect_handler,
|
||||
)
|
||||
|
||||
|
||||
@@ -241,6 +244,64 @@ class TestUtilities:
|
||||
assert _can_open_browser() is True
|
||||
|
||||
|
||||
class TestRedirectHandlerSshHint:
|
||||
"""_redirect_handler must print an SSH tunnel hint on remote sessions."""
|
||||
|
||||
def _run(self, coro):
|
||||
return asyncio.get_event_loop().run_until_complete(coro)
|
||||
|
||||
def test_ssh_hint_shown_on_ssh_session(self, monkeypatch, capsys):
|
||||
import tools.mcp_oauth as mco
|
||||
monkeypatch.setattr(mco, "_oauth_port", 49200)
|
||||
monkeypatch.setenv("SSH_CLIENT", "1.2.3.4 1234 22")
|
||||
monkeypatch.delenv("SSH_TTY", raising=False)
|
||||
monkeypatch.setattr(mco, "_can_open_browser", lambda: False)
|
||||
|
||||
self._run(_redirect_handler("https://example.com/auth?foo=bar"))
|
||||
|
||||
err = capsys.readouterr().err
|
||||
assert "49200" in err
|
||||
assert "ssh -N -L" in err
|
||||
assert "Remote session detected" in err
|
||||
|
||||
def test_ssh_hint_shown_via_ssh_tty(self, monkeypatch, capsys):
|
||||
import tools.mcp_oauth as mco
|
||||
monkeypatch.setattr(mco, "_oauth_port", 49201)
|
||||
monkeypatch.delenv("SSH_CLIENT", raising=False)
|
||||
monkeypatch.setenv("SSH_TTY", "/dev/pts/1")
|
||||
monkeypatch.setattr(mco, "_can_open_browser", lambda: False)
|
||||
|
||||
self._run(_redirect_handler("https://example.com/auth"))
|
||||
|
||||
err = capsys.readouterr().err
|
||||
assert "49201" in err
|
||||
assert "ssh -N -L" in err
|
||||
|
||||
def test_no_ssh_hint_on_local_session(self, monkeypatch, capsys):
|
||||
import tools.mcp_oauth as mco
|
||||
monkeypatch.setattr(mco, "_oauth_port", 49202)
|
||||
monkeypatch.delenv("SSH_CLIENT", raising=False)
|
||||
monkeypatch.delenv("SSH_TTY", raising=False)
|
||||
monkeypatch.setattr(mco, "_can_open_browser", lambda: True)
|
||||
monkeypatch.setattr("webbrowser.open", lambda url, **kw: True)
|
||||
|
||||
self._run(_redirect_handler("https://example.com/auth"))
|
||||
|
||||
err = capsys.readouterr().err
|
||||
assert "ssh -N -L" not in err
|
||||
|
||||
def test_no_ssh_hint_when_port_not_set(self, monkeypatch, capsys):
|
||||
import tools.mcp_oauth as mco
|
||||
monkeypatch.setattr(mco, "_oauth_port", None)
|
||||
monkeypatch.setenv("SSH_CLIENT", "1.2.3.4 1234 22")
|
||||
monkeypatch.setattr(mco, "_can_open_browser", lambda: False)
|
||||
|
||||
self._run(_redirect_handler("https://example.com/auth"))
|
||||
|
||||
err = capsys.readouterr().err
|
||||
assert "ssh -N -L" not in err
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Path traversal protection
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -135,7 +135,7 @@ class TestStdioPidTracking:
|
||||
# bpo-14484). Return True so the SIGKILL escalation fires.
|
||||
with patch("tools.mcp_tool.os.kill") as mock_kill, \
|
||||
patch("gateway.status._pid_exists", return_value=True), \
|
||||
patch("time.sleep") as mock_sleep:
|
||||
patch("tools.mcp_tool.time.sleep") as mock_sleep:
|
||||
_kill_orphaned_mcp_children()
|
||||
|
||||
# SIGTERM then SIGKILL; the alive check no longer touches os.kill.
|
||||
@@ -163,7 +163,7 @@ class TestStdioPidTracking:
|
||||
monkeypatch.delattr(signal, "SIGKILL", raising=False)
|
||||
|
||||
with patch("tools.mcp_tool.os.kill") as mock_kill, \
|
||||
patch("time.sleep") as mock_sleep:
|
||||
patch("tools.mcp_tool.time.sleep") as mock_sleep:
|
||||
_kill_orphaned_mcp_children()
|
||||
|
||||
# SIGTERM phase, alive check raises (process gone), no escalation
|
||||
|
||||
@@ -3781,16 +3781,26 @@ class TestMcpParallelToolCalls:
|
||||
|
||||
def test_is_mcp_tool_parallel_safe_no_servers(self):
|
||||
"""MCP tool from unknown server returns False."""
|
||||
from tools.mcp_tool import is_mcp_tool_parallel_safe, _parallel_safe_servers, _lock
|
||||
from tools.mcp_tool import (
|
||||
is_mcp_tool_parallel_safe, _mcp_tool_server_names,
|
||||
_parallel_safe_servers, _lock,
|
||||
)
|
||||
with _lock:
|
||||
_parallel_safe_servers.clear()
|
||||
_mcp_tool_server_names.clear()
|
||||
assert is_mcp_tool_parallel_safe("mcp_docs_search") is False
|
||||
|
||||
def test_is_mcp_tool_parallel_safe_with_flag(self):
|
||||
"""MCP tool from a parallel-safe server returns True."""
|
||||
from tools.mcp_tool import is_mcp_tool_parallel_safe, _parallel_safe_servers, _lock
|
||||
from tools.mcp_tool import (
|
||||
is_mcp_tool_parallel_safe, _mcp_tool_server_names,
|
||||
_parallel_safe_servers, _lock,
|
||||
)
|
||||
with _lock:
|
||||
_parallel_safe_servers.add("docs")
|
||||
_mcp_tool_server_names["mcp_docs_search"] = "docs"
|
||||
_mcp_tool_server_names["mcp_docs_read_file"] = "docs"
|
||||
_mcp_tool_server_names["mcp_github_list_repos"] = "github"
|
||||
try:
|
||||
assert is_mcp_tool_parallel_safe("mcp_docs_search") is True
|
||||
assert is_mcp_tool_parallel_safe("mcp_docs_read_file") is True
|
||||
@@ -3799,23 +3809,86 @@ class TestMcpParallelToolCalls:
|
||||
finally:
|
||||
with _lock:
|
||||
_parallel_safe_servers.discard("docs")
|
||||
_mcp_tool_server_names.pop("mcp_docs_search", None)
|
||||
_mcp_tool_server_names.pop("mcp_docs_read_file", None)
|
||||
_mcp_tool_server_names.pop("mcp_github_list_repos", None)
|
||||
|
||||
def test_is_mcp_tool_parallel_safe_server_with_underscores(self):
|
||||
"""Server names containing underscores are correctly matched."""
|
||||
from tools.mcp_tool import is_mcp_tool_parallel_safe, _parallel_safe_servers, _lock
|
||||
from tools.mcp_tool import (
|
||||
is_mcp_tool_parallel_safe, _mcp_tool_server_names,
|
||||
_parallel_safe_servers, _lock,
|
||||
)
|
||||
with _lock:
|
||||
_parallel_safe_servers.add("my_server")
|
||||
_mcp_tool_server_names["mcp_my_server_query"] = "my_server"
|
||||
try:
|
||||
assert is_mcp_tool_parallel_safe("mcp_my_server_query") is True
|
||||
finally:
|
||||
with _lock:
|
||||
_parallel_safe_servers.discard("my_server")
|
||||
_mcp_tool_server_names.pop("mcp_my_server_query", None)
|
||||
|
||||
def test_is_mcp_tool_parallel_safe_uses_exact_registered_server(self):
|
||||
"""Ambiguous MCP names must not match a shorter parallel-safe prefix."""
|
||||
from tools.mcp_tool import (
|
||||
is_mcp_tool_parallel_safe, _mcp_tool_server_names,
|
||||
_parallel_safe_servers, _lock,
|
||||
)
|
||||
with _lock:
|
||||
_parallel_safe_servers.add("a")
|
||||
_mcp_tool_server_names["mcp_a_search"] = "a"
|
||||
_mcp_tool_server_names["mcp_a_b_tool"] = "a_b"
|
||||
try:
|
||||
assert is_mcp_tool_parallel_safe("mcp_a_search") is True
|
||||
assert is_mcp_tool_parallel_safe("mcp_a_b_tool") is False
|
||||
finally:
|
||||
with _lock:
|
||||
_parallel_safe_servers.discard("a")
|
||||
_mcp_tool_server_names.pop("mcp_a_search", None)
|
||||
_mcp_tool_server_names.pop("mcp_a_b_tool", None)
|
||||
|
||||
def test_registered_tool_provenance_prevents_prefix_collision(self):
|
||||
"""Registration records exact server ownership for ambiguous names."""
|
||||
from tools.registry import registry
|
||||
from tools.mcp_tool import (
|
||||
_mcp_tool_server_names, _parallel_safe_servers,
|
||||
_register_server_tools, is_mcp_tool_parallel_safe, _lock,
|
||||
)
|
||||
|
||||
server = _make_mock_server(
|
||||
"a_b",
|
||||
tools=[_make_mcp_tool("tool", "Ambiguous tool name")],
|
||||
)
|
||||
registered = _register_server_tools("a_b", server, {})
|
||||
try:
|
||||
assert registered == ["mcp_a_b_tool"]
|
||||
with _lock:
|
||||
assert _mcp_tool_server_names["mcp_a_b_tool"] == "a_b"
|
||||
_parallel_safe_servers.add("a")
|
||||
assert is_mcp_tool_parallel_safe("mcp_a_b_tool") is False
|
||||
|
||||
with _lock:
|
||||
_parallel_safe_servers.add("a_b")
|
||||
assert is_mcp_tool_parallel_safe("mcp_a_b_tool") is True
|
||||
finally:
|
||||
for tool_name in registered:
|
||||
registry.deregister(tool_name)
|
||||
with _lock:
|
||||
_parallel_safe_servers.discard("a")
|
||||
_parallel_safe_servers.discard("a_b")
|
||||
_mcp_tool_server_names.pop("mcp_a_b_tool", None)
|
||||
|
||||
def test_is_mcp_tool_parallel_safe_no_tool_suffix(self):
|
||||
"""Tool name that is just 'mcp_{server}' without a tool part returns False."""
|
||||
from tools.mcp_tool import is_mcp_tool_parallel_safe, _parallel_safe_servers, _lock
|
||||
from tools.mcp_tool import (
|
||||
is_mcp_tool_parallel_safe, _mcp_tool_server_names,
|
||||
_parallel_safe_servers, _lock,
|
||||
)
|
||||
with _lock:
|
||||
_parallel_safe_servers.add("docs")
|
||||
_mcp_tool_server_names.pop("mcp_docs", None)
|
||||
_mcp_tool_server_names.pop("mcp_docs_", None)
|
||||
try:
|
||||
# "mcp_docs" has no tool part after the server name
|
||||
assert is_mcp_tool_parallel_safe("mcp_docs") is False
|
||||
|
||||
@@ -304,6 +304,30 @@ def test_strip_none_returns_zero():
|
||||
assert stripped == 0
|
||||
|
||||
|
||||
|
||||
def test_strip_responses_format_strips_format_keyword():
|
||||
"""Responses-format: keyword should be stripped."""
|
||||
from tools.schema_sanitizer import strip_pattern_and_format
|
||||
|
||||
tools = [
|
||||
{
|
||||
"name": "get_event",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"ts": {"type": "string", "format": "date-time"},
|
||||
}
|
||||
},
|
||||
"type": "function"
|
||||
}
|
||||
]
|
||||
|
||||
result, stripped = strip_pattern_and_format(tools)
|
||||
assert stripped == 1, f"Expected 1 format stripped, got {stripped}"
|
||||
assert "format" not in result[0]["parameters"]["properties"]["ts"], "format should be stripped"
|
||||
assert result[0]["parameters"]["properties"]["ts"]["type"] == "string", "type should be preserved"
|
||||
|
||||
|
||||
def test_top_level_allof_stripped_for_codex_backend_compat():
|
||||
"""OpenAI Codex backend rejects top-level allOf/oneOf/anyOf/enum/not."""
|
||||
tools = [_tool("memory", {
|
||||
@@ -360,3 +384,110 @@ def test_nested_allof_preserved():
|
||||
nested = out[0]["function"]["parameters"]["properties"]["config"]
|
||||
assert "allOf" in nested
|
||||
assert nested["allOf"] == [{"required": ["mode"]}]
|
||||
|
||||
|
||||
def test_strip_responses_format_tools():
|
||||
"""strip_pattern_and_format should handle Responses-format tools (no function wrapper)."""
|
||||
from tools.schema_sanitizer import strip_pattern_and_format
|
||||
|
||||
# Responses-format: {"name": "...", "parameters": {...}, "type": "function"}
|
||||
tools = [
|
||||
{
|
||||
"name": "mcp_firecrawl_search",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"query": {"type": "string"},
|
||||
"includeDomains": {
|
||||
"type": "array",
|
||||
"items": {
|
||||
"type": "string",
|
||||
"pattern": "^(?=.{1,253}$)(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\\.)+[a-z0-9][a-z0-9-]{0,61}[a-z0-9]$"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"type": "function"
|
||||
}
|
||||
]
|
||||
|
||||
result, stripped = strip_pattern_and_format(tools)
|
||||
assert stripped == 1, f"Expected 1 pattern stripped, got {stripped}"
|
||||
|
||||
# Verify pattern keyword was removed from includeDomains
|
||||
domains = result[0]["parameters"]["properties"]["includeDomains"]["items"]
|
||||
assert "pattern" not in domains, f"pattern should be stripped: {domains}"
|
||||
assert domains["type"] == "string", "type should be preserved"
|
||||
|
||||
|
||||
def test_strip_responses_idempotent():
|
||||
"""Second call on already-stripped Responses-format tools should return 0."""
|
||||
from tools.schema_sanitizer import strip_pattern_and_format
|
||||
|
||||
tools = [
|
||||
{
|
||||
"name": "search_files",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"pattern": {"type": "string"} # This is a property named pattern, NOT schema keyword
|
||||
}
|
||||
}
|
||||
}
|
||||
]
|
||||
|
||||
# Pass 1 - property named 'pattern' should NOT be stripped
|
||||
result, first = strip_pattern_and_format(tools)
|
||||
assert first == 0, f"Expected 0 stripped (property pattern preserved), got {first}"
|
||||
assert "pattern" in result[0]["parameters"]["properties"], "property named pattern should survive"
|
||||
|
||||
# Pass 2 - idempotent
|
||||
_, second = strip_pattern_and_format(tools)
|
||||
assert second == 0, f"Expected 0 on second pass, got {second}"
|
||||
|
||||
|
||||
def test_strip_responses_mixed_formats():
|
||||
"""Mixed list of OpenAI-format and Responses-format tools should both be sanitized."""
|
||||
from tools.schema_sanitizer import strip_pattern_and_format
|
||||
|
||||
tools = [
|
||||
# OpenAI-format: {"function": {"parameters": {...}}}
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "search",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"query": {"type": "string", "pattern": "^[a-z]+$"}
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
# Responses-format: {"name": "...", "parameters": {...}}
|
||||
{
|
||||
"name": "get_time",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"tz": {"type": "string", "format": "date-time"}
|
||||
}
|
||||
},
|
||||
"type": "function"
|
||||
}
|
||||
]
|
||||
|
||||
result, stripped = strip_pattern_and_format(tools)
|
||||
assert stripped == 2, f"Expected 2 stripped (1 pattern + 1 format), got {stripped}"
|
||||
|
||||
# OpenAI-format tool: pattern stripped from parameters
|
||||
openai_params = result[0]["function"]["parameters"]["properties"]["query"]
|
||||
assert "pattern" not in openai_params, f"pattern should be stripped: {openai_params}"
|
||||
|
||||
# Responses-format tool: format stripped
|
||||
resp_params = result[1]["parameters"]["properties"]["tz"]
|
||||
assert "format" not in resp_params, f"format should be stripped: {resp_params}"
|
||||
|
||||
# Verify structure preserved
|
||||
assert result[0]["function"]["parameters"]["type"] == "object"
|
||||
assert result[1]["parameters"]["type"] == "object"
|
||||
|
||||
@@ -182,6 +182,81 @@ class TestSendMessageTool:
|
||||
force_document=False,
|
||||
)
|
||||
|
||||
def test_resolved_slack_thread_name_preserves_thread_id(self):
|
||||
slack_cfg = SimpleNamespace(enabled=True, token="xoxb-test", extra={})
|
||||
config = SimpleNamespace(
|
||||
platforms={Platform.SLACK: slack_cfg},
|
||||
get_home_channel=lambda _platform: None,
|
||||
)
|
||||
|
||||
with patch("gateway.config.load_gateway_config", return_value=config), \
|
||||
patch("tools.interrupt.is_interrupted", return_value=False), \
|
||||
patch("gateway.channel_directory.resolve_channel_name", return_value="C123ABCDEF:171.000001"), \
|
||||
patch("model_tools._run_async", side_effect=_run_async_immediately), \
|
||||
patch("tools.send_message_tool._send_to_platform", new=AsyncMock(return_value={"success": True})) as send_mock, \
|
||||
patch("gateway.mirror.mirror_to_session", return_value=True):
|
||||
result = json.loads(
|
||||
send_message_tool(
|
||||
{
|
||||
"action": "send",
|
||||
"target": "slack:ops / topic 171.000001",
|
||||
"message": "hello",
|
||||
}
|
||||
)
|
||||
)
|
||||
|
||||
assert result["success"] is True
|
||||
send_mock.assert_awaited_once_with(
|
||||
Platform.SLACK,
|
||||
slack_cfg,
|
||||
"C123ABCDEF",
|
||||
"hello",
|
||||
thread_id="171.000001",
|
||||
media_files=[],
|
||||
force_document=False,
|
||||
)
|
||||
|
||||
def test_resolved_matrix_thread_name_preserves_thread_id(self):
|
||||
matrix_cfg = SimpleNamespace(
|
||||
enabled=True,
|
||||
token="tok",
|
||||
extra={"homeserver": "https://matrix.example.com"},
|
||||
)
|
||||
config = SimpleNamespace(
|
||||
platforms={Platform.MATRIX: matrix_cfg},
|
||||
get_home_channel=lambda _platform: None,
|
||||
)
|
||||
|
||||
with patch("gateway.config.load_gateway_config", return_value=config), \
|
||||
patch("tools.interrupt.is_interrupted", return_value=False), \
|
||||
patch(
|
||||
"gateway.channel_directory.resolve_channel_name",
|
||||
return_value="!roomid:matrix.example.org:$thread123:matrix.example.org",
|
||||
), \
|
||||
patch("model_tools._run_async", side_effect=_run_async_immediately), \
|
||||
patch("tools.send_message_tool._send_to_platform", new=AsyncMock(return_value={"success": True})) as send_mock, \
|
||||
patch("gateway.mirror.mirror_to_session", return_value=True):
|
||||
result = json.loads(
|
||||
send_message_tool(
|
||||
{
|
||||
"action": "send",
|
||||
"target": "matrix:Ops / topic $thread123",
|
||||
"message": "hello",
|
||||
}
|
||||
)
|
||||
)
|
||||
|
||||
assert result["success"] is True
|
||||
send_mock.assert_awaited_once_with(
|
||||
Platform.MATRIX,
|
||||
matrix_cfg,
|
||||
"!roomid:matrix.example.org",
|
||||
"hello",
|
||||
thread_id="$thread123:matrix.example.org",
|
||||
media_files=[],
|
||||
force_document=False,
|
||||
)
|
||||
|
||||
def test_mirror_receives_current_session_user_id(self):
|
||||
config, _telegram_cfg = _make_config()
|
||||
|
||||
@@ -503,9 +578,8 @@ class TestSendToPlatformChunking:
|
||||
assert all(call == [] for call in sent_calls[:-1])
|
||||
assert sent_calls[-1] == media
|
||||
|
||||
def test_matrix_media_uses_native_adapter_helper(self):
|
||||
|
||||
doc_path = Path("/tmp/test-send-message-matrix.pdf")
|
||||
def test_matrix_media_uses_native_adapter_helper(self, tmp_path):
|
||||
doc_path = tmp_path / "test-send-message-matrix.pdf"
|
||||
doc_path.write_bytes(b"%PDF-1.4 test")
|
||||
|
||||
try:
|
||||
@@ -847,6 +921,16 @@ class TestParseTargetRefDiscord:
|
||||
class TestParseTargetRefMatrix:
|
||||
"""_parse_target_ref correctly handles Matrix room IDs and user MXIDs."""
|
||||
|
||||
def test_matrix_thread_target_is_explicit(self):
|
||||
"""Session-derived Matrix thread targets round-trip as room + event id."""
|
||||
chat_id, thread_id, is_explicit = _parse_target_ref(
|
||||
"matrix",
|
||||
"!HLOQwxYGgFPMPJUSNR:matrix.org:$thread123:matrix.org",
|
||||
)
|
||||
assert chat_id == "!HLOQwxYGgFPMPJUSNR:matrix.org"
|
||||
assert thread_id == "$thread123:matrix.org"
|
||||
assert is_explicit is True
|
||||
|
||||
def test_matrix_room_id_is_explicit(self):
|
||||
"""Matrix room IDs (!) are recognized as explicit targets."""
|
||||
chat_id, thread_id, is_explicit = _parse_target_ref("matrix", "!HLOQwxYGgFPMPJUSNR:matrix.org")
|
||||
@@ -919,6 +1003,12 @@ class TestParseTargetRefE164:
|
||||
class TestParseTargetRefSlack:
|
||||
"""_parse_target_ref recognizes Slack channel/user IDs as explicit."""
|
||||
|
||||
def test_thread_target_is_explicit(self):
|
||||
chat_id, thread_id, is_explicit = _parse_target_ref("slack", "C0B0QV5434G:171.000001")
|
||||
assert chat_id == "C0B0QV5434G"
|
||||
assert thread_id == "171.000001"
|
||||
assert is_explicit is True
|
||||
|
||||
def test_public_channel_id_is_explicit(self):
|
||||
chat_id, thread_id, is_explicit = _parse_target_ref("slack", "C0B0QV5434G")
|
||||
assert chat_id == "C0B0QV5434G"
|
||||
|
||||
@@ -23,7 +23,7 @@ class TestFindSingularityExecutable:
|
||||
def test_prefers_apptainer(self):
|
||||
"""When both are available, apptainer should be preferred."""
|
||||
def which_both(name):
|
||||
return f"/usr/bin/{name}" if name in ("apptainer", "singularity") else None
|
||||
return f"/usr/bin/{name}" if name in {"apptainer", "singularity"} else None
|
||||
|
||||
with patch("shutil.which", side_effect=which_both):
|
||||
assert _find_singularity_executable() == "apptainer"
|
||||
|
||||
@@ -547,7 +547,7 @@ class TestSkillManageDispatcher:
|
||||
# No provenance marker on a foreground create — record either missing
|
||||
# entirely (telemetry best-effort) or present with created_by unset.
|
||||
rec = usage.get("test-skill") or {}
|
||||
assert rec.get("created_by") in (None, "", False)
|
||||
assert rec.get("created_by") in {None, "", False}
|
||||
|
||||
def test_create_from_background_review_marks_agent_created(self, tmp_path):
|
||||
"""Background-review fork creates ARE marked as agent-created."""
|
||||
|
||||
@@ -101,7 +101,7 @@ class TestTrustLevelFor:
|
||||
src = self._source()
|
||||
result = src.trust_level_for("owner/repo")
|
||||
# No path part — still resolves repo correctly
|
||||
assert result in ("trusted", "community")
|
||||
assert result in {"trusted", "community"}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -60,6 +60,33 @@ class TestProviderSelectionGate:
|
||||
finally:
|
||||
importlib.reload(tt)
|
||||
|
||||
def test_xai_resolver_import_after_config_env_patch_uses_restored_dotenv_loader(self):
|
||||
"""xAI HTTP auth must not cache a temporarily patched env helper."""
|
||||
import importlib
|
||||
import hermes_cli.config as config_mod
|
||||
from tools import xai_http
|
||||
|
||||
with pytest.MonkeyPatch.context() as mp:
|
||||
mp.setattr(config_mod, "get_env_value", lambda name, default=None: "")
|
||||
xai_http = importlib.reload(xai_http)
|
||||
|
||||
try:
|
||||
with patch(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
side_effect=RuntimeError("no oauth"),
|
||||
), patch(
|
||||
"hermes_cli.auth.resolve_xai_oauth_runtime_credentials",
|
||||
return_value={},
|
||||
), patch(
|
||||
"hermes_cli.config.load_env",
|
||||
return_value={"XAI_API_KEY": "dotenv-secret"},
|
||||
):
|
||||
creds = xai_http.resolve_xai_http_credentials()
|
||||
finally:
|
||||
importlib.reload(xai_http)
|
||||
|
||||
assert creds["api_key"] == "dotenv-secret"
|
||||
|
||||
def test_explicit_groq_sees_dotenv(self):
|
||||
from tools import transcription_tools as tt
|
||||
|
||||
|
||||
@@ -482,8 +482,11 @@ class TestVprintForceParameter:
|
||||
else:
|
||||
unforced_error_count += 1
|
||||
|
||||
assert forced_error_count > 0, \
|
||||
"Expected at least one _vprint with force=True for error messages"
|
||||
# Invariant: no critical-error _vprint call may silently drop under
|
||||
# streaming suppression — every ❌-prefixed _vprint must pass force=True.
|
||||
# The codebase may legitimately have zero such calls if errors are
|
||||
# routed through print() or higher-level Rich panels; what matters is
|
||||
# that none are quietly suppressed.
|
||||
assert unforced_error_count == 0, \
|
||||
f"Found {unforced_error_count} critical error _vprint calls without force=True"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user