tancouandClaude Opus 5
90b814f673
Two profiles configuring a same-named MCP server shared one connection and
the MCP settings panel showed another profile's runtime ("Active, N tools"
next to an empty inventory); /reload-mcp stopped every profile's servers and
rediscovered from the process-default config.
- api.profiles._set_hermes_home() is the single writer of the process-profile
home: it records get_process_profile_home() and hands it to Hermes Agent's
hermes_constants.pin_process_hermes_home when available, at startup and on
switch_profile(process_wide=True), so the per-turn HERMES_HOME mirror no
longer makes every turn look like the process profile (bare-name,
cross-profile MCP keys) and the anchor cannot drift from HERMES_HOME.
- Run get_mcp_status/registry reads and /reload-mcp inside
api.mcp_runtime.mcp_runtime_scope(): request profile's Hermes-home override
and secret scope (root included), no os.environ mutation, restored on exit.
- One ledger predicate, ledger_key_serves_view() (owned via
_server_scope_keys/key scope, or adopted via _server_tool_scopes), is shared
by status, inventory and the reload summary. The Agent's launch-profile view
is process-wide and name-indexed, so filter_runtime_status_to_view()
rebuilds connected/tools/status/error from the entries serving the profile:
the root profile never shows a routed profile's connection (nor lets its own
parked failed-spawn task borrow a routed profile's live same-named one).
- Inventory lists only tools in the profile's own registry slot for servers it
configures; responses carry runtime_scope.
- Fail closed when the agent's routing does not match the resolved profile:
hide runtime data (runtime_scope "unavailable", shown as a notice in the MCP
panel, 14 locales) and refuse /reload-mcp.
- /reload-mcp shuts down only the profile's own ledger names (parked
included), drops its own connect cooldown/error entries so a failed spawn is
retried (the wildcard cleared every cooldown; a scoped shutdown only clears
the live keys it tears down), rediscovers under the profile's home and
counts live connections only. The bare wildcard remains for agents without
profile-scoped MCP.
- Signature introspection is centralized (accepts_keywords) and falls back to
the bare call instead of blanking every status; ledger key helpers resolve
to the Agent's tools.mcp_tool_scope when present; tools.mcp_tool is
resolved by full name so stubbed agents still work.
- ARCHITECTURE.md §4.10 and docs/troubleshooting.md document the boundary.
- Tests: agent-free contract (models adoption, parked tasks, cooldowns and the
process-wide root view) and real-Agent integration (fake stdio MCP server,
skips without hermes-agent + mcp SDK).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
445 lines
18 KiB
Python
445 lines
18 KiB
Python
"""Tests for issue #538 — MCP server management API."""
|
|
import json, pytest
|
|
from unittest.mock import patch, MagicMock, call
|
|
import yaml
|
|
from api.routes import (
|
|
_handle_mcp_servers_list,
|
|
_handle_mcp_server_update,
|
|
_handle_mcp_server_delete,
|
|
_handle_mcp_server_toggle,
|
|
_mask_secrets,
|
|
_parse_mcp_enabled,
|
|
_server_summary,
|
|
_strip_masked_values,
|
|
)
|
|
|
|
|
|
def _make_handler():
|
|
h = MagicMock()
|
|
h.path = '/api/mcp/servers'
|
|
h.command = 'GET'
|
|
return h
|
|
|
|
|
|
def _json_payload(handler):
|
|
body = handler.wfile.write.call_args[0][0]
|
|
return json.loads(body.decode('utf-8'))
|
|
|
|
|
|
SAMPLE_MCP = {
|
|
"searxng": {
|
|
"command": "mcp-searxng",
|
|
"args": ["--port", "8888"],
|
|
"timeout": 120
|
|
},
|
|
"web-reader": {
|
|
"url": "http://localhost:3001/mcp",
|
|
"timeout": 60,
|
|
"headers": {"Authorization": "Bearer secret123"}
|
|
}
|
|
}
|
|
|
|
|
|
class TestMcpList:
|
|
"""GET /api/mcp/servers — list with masked secrets."""
|
|
|
|
@patch('api.routes.get_config_for_profile_home')
|
|
@patch('api.routes.get_active_hermes_home')
|
|
def test_returns_servers_list(self, mock_home, mock_cfg):
|
|
mock_home.return_value = sentinel_home = object()
|
|
mock_cfg.return_value = {'mcp_servers': SAMPLE_MCP}
|
|
h = _make_handler()
|
|
_handle_mcp_servers_list(h)
|
|
assert h.send_response.called
|
|
status = h.send_response.call_args[0][0]
|
|
assert status == 200
|
|
mock_cfg.assert_called_once_with(sentinel_home)
|
|
|
|
@patch('api.routes.get_config_for_profile_home')
|
|
@patch('api.routes.get_active_hermes_home')
|
|
def test_reads_active_profile_home_for_servers(self, mock_home, mock_cfg):
|
|
mock_home.return_value = sentinel_home = object()
|
|
mock_cfg.return_value = {'mcp_servers': {'active': SAMPLE_MCP['searxng']}}
|
|
h = _make_handler()
|
|
_handle_mcp_servers_list(h)
|
|
payload = _json_payload(h)
|
|
assert [srv['name'] for srv in payload['servers']] == ['active']
|
|
mock_cfg.assert_called_once_with(sentinel_home)
|
|
|
|
@patch('api.routes.get_config_for_profile_home')
|
|
@patch('api.routes.get_active_hermes_home')
|
|
def test_empty_config(self, mock_home, mock_cfg):
|
|
mock_home.return_value = object()
|
|
mock_cfg.return_value = {}
|
|
h = _make_handler()
|
|
_handle_mcp_servers_list(h)
|
|
assert h.send_response.called
|
|
status = h.send_response.call_args[0][0]
|
|
assert status == 200
|
|
payload = _json_payload(h)
|
|
assert payload['servers'] == []
|
|
assert payload['toggle_supported'] is True
|
|
assert payload['reload_required'] is True
|
|
|
|
@patch('api.routes._mcp_runtime_status_by_name')
|
|
@patch('api.routes.get_config_for_profile_home')
|
|
@patch('api.routes.get_active_hermes_home')
|
|
def test_list_payload_includes_status_tool_counts_and_safe_invalid_config(self, mock_home, mock_cfg, mock_runtime):
|
|
mock_home.return_value = object()
|
|
mock_cfg.return_value = {
|
|
'mcp_servers': {
|
|
'searxng': {'command': 'mcp-searxng', 'args': ['--port', '8888']},
|
|
'web-reader': {
|
|
'url': 'http://localhost:3001/mcp',
|
|
'headers': {'Authorization': 'Bearer secret123'},
|
|
},
|
|
'disabled': {'command': 'disabled-cmd', 'enabled': 0},
|
|
'broken': 'not-a-dict',
|
|
}
|
|
}
|
|
mock_runtime.return_value = {
|
|
'searxng': {'connected': True, 'tools': 3},
|
|
'web-reader': {'connected': False, 'tools': 0},
|
|
}
|
|
h = _make_handler()
|
|
_handle_mcp_servers_list(h)
|
|
payload = _json_payload(h)
|
|
by_name = {s['name']: s for s in payload['servers']}
|
|
assert by_name['searxng']['status'] == 'active'
|
|
assert by_name['searxng']['active'] is True
|
|
assert by_name['searxng']['tool_count'] == 3
|
|
assert by_name['web-reader']['status'] == 'configured'
|
|
assert '••••' in by_name['web-reader']['headers']['Authorization']
|
|
assert by_name['disabled']['enabled'] is False
|
|
assert by_name['disabled']['active'] is False
|
|
assert by_name['disabled']['status'] == 'disabled'
|
|
assert by_name['broken']['transport'] == 'invalid'
|
|
assert by_name['broken']['status'] == 'invalid_config'
|
|
|
|
def test_secrets_are_masked(self):
|
|
"""_mask_secrets hides API keys in headers and env."""
|
|
masked = _mask_secrets(SAMPLE_MCP['web-reader']['headers'])
|
|
assert masked['Authorization'] != 'Bearer secret123'
|
|
assert '••••' in masked['Authorization']
|
|
|
|
def test_server_summary_stdio(self):
|
|
summary = _server_summary('searxng', SAMPLE_MCP['searxng'])
|
|
assert summary['transport'] == 'stdio'
|
|
assert summary['command'] == 'mcp-searxng'
|
|
assert summary['args'] == ['--port', '8888']
|
|
|
|
def test_server_summary_http(self):
|
|
summary = _server_summary('web-reader', SAMPLE_MCP['web-reader'])
|
|
assert summary['transport'] == 'http'
|
|
assert summary['url'] == 'http://localhost:3001/mcp'
|
|
assert '••••' in summary['headers']['Authorization']
|
|
|
|
def test_server_summary_default_timeout(self):
|
|
summary = _server_summary('minimal', {'command': 'x'})
|
|
assert summary['timeout'] == 120
|
|
|
|
def test_numeric_zero_enabled_flag_is_disabled(self):
|
|
"""YAML numeric false-y values should not show a disabled server as enabled."""
|
|
assert _parse_mcp_enabled(0) is False
|
|
|
|
def test_active_home_list_reads_external_config_override_used_by_writes(self, monkeypatch, tmp_path):
|
|
"""HERMES_CONFIG_PATH outside the active home remains the read/write authority."""
|
|
from api import config, profiles, routes
|
|
|
|
active_home = tmp_path / 'active-home'
|
|
override_path = tmp_path / 'override-dir' / 'config.yaml'
|
|
active_home.mkdir()
|
|
override_path.parent.mkdir()
|
|
active_home.joinpath('config.yaml').write_text(
|
|
yaml.safe_dump({'mcp_servers': {'wrong-home': {'command': 'wrong'}}}, sort_keys=False),
|
|
encoding='utf-8',
|
|
)
|
|
override_path.write_text(
|
|
yaml.safe_dump({'mcp_servers': {'override-srv': {'command': 'override'}}}, sort_keys=False),
|
|
encoding='utf-8',
|
|
)
|
|
monkeypatch.setenv('HERMES_CONFIG_PATH', str(override_path))
|
|
monkeypatch.setattr(profiles, 'get_active_hermes_home', lambda: active_home)
|
|
monkeypatch.setattr(routes, 'get_active_hermes_home', lambda: active_home)
|
|
monkeypatch.setattr(routes, '_mcp_runtime_status_by_name', lambda *_args, **_kwargs: {})
|
|
config.reload_config()
|
|
|
|
h = _make_handler()
|
|
_handle_mcp_servers_list(h)
|
|
payload = _json_payload(h)
|
|
assert [srv['name'] for srv in payload['servers']] == ['override-srv']
|
|
|
|
h = _make_handler()
|
|
h.command = 'PUT'
|
|
_handle_mcp_server_update(h, 'new-srv', {'command': 'new-command'})
|
|
saved = yaml.safe_load(override_path.read_text(encoding='utf-8'))
|
|
active_home_saved = yaml.safe_load(active_home.joinpath('config.yaml').read_text(encoding='utf-8'))
|
|
assert 'new-srv' in saved['mcp_servers']
|
|
assert 'new-srv' not in active_home_saved['mcp_servers']
|
|
|
|
|
|
class TestMcpSave:
|
|
"""PUT /api/mcp/servers/<name> — add or update."""
|
|
|
|
@patch('api.routes.reload_config')
|
|
@patch('api.routes._save_yaml_config_file')
|
|
@patch('api.routes._get_config_path', return_value='/tmp/test.yaml')
|
|
@patch('api.routes.get_config')
|
|
def test_add_new_stdio_server(self, mock_cfg, mock_path, mock_save, mock_reload):
|
|
mock_cfg.return_value = {}
|
|
h = _make_handler()
|
|
h.command = 'PUT'
|
|
body = {"command": "test-cmd", "timeout": 30}
|
|
_handle_mcp_server_update(h, 'test-server', body)
|
|
assert mock_save.called
|
|
saved = mock_save.call_args[0][1]
|
|
assert 'test-server' in saved['mcp_servers']
|
|
assert saved['mcp_servers']['test-server']['command'] == 'test-cmd'
|
|
|
|
@patch('api.routes.reload_config')
|
|
@patch('api.routes._save_yaml_config_file')
|
|
@patch('api.routes._get_config_path', return_value='/tmp/test.yaml')
|
|
@patch('api.routes.get_config')
|
|
def test_add_new_http_server(self, mock_cfg, mock_path, mock_save, mock_reload):
|
|
mock_cfg.return_value = {}
|
|
h = _make_handler()
|
|
h.command = 'PUT'
|
|
body = {"url": "http://localhost:4000", "timeout": 60}
|
|
_handle_mcp_server_update(h, 'http-srv', body)
|
|
saved = mock_save.call_args[0][1]
|
|
assert saved['mcp_servers']['http-srv']['url'] == 'http://localhost:4000'
|
|
|
|
@patch('api.routes.reload_config')
|
|
@patch('api.routes._save_yaml_config_file')
|
|
@patch('api.routes._get_config_path', return_value='/tmp/test.yaml')
|
|
@patch('api.routes.get_config')
|
|
def test_update_existing(self, mock_cfg, mock_path, mock_save, mock_reload):
|
|
mock_cfg.return_value = {'mcp_servers': {'existing': {'command': 'old'}}}
|
|
h = _make_handler()
|
|
h.command = 'PUT'
|
|
body = {"command": "new-cmd"}
|
|
_handle_mcp_server_update(h, 'existing', body)
|
|
saved = mock_save.call_args[0][1]
|
|
assert saved['mcp_servers']['existing']['command'] == 'new-cmd'
|
|
|
|
@patch('api.routes.reload_config')
|
|
@patch('api.routes._save_yaml_config_file')
|
|
@patch('api.routes._get_config_path', return_value='/tmp/test.yaml')
|
|
@patch('api.routes.get_config')
|
|
def test_preserves_other_servers(self, mock_cfg, mock_path, mock_save, mock_reload):
|
|
mock_cfg.return_value = {'mcp_servers': {'keep': {'command': 'stay'}}}
|
|
h = _make_handler()
|
|
h.command = 'PUT'
|
|
body = {"command": "new"}
|
|
_handle_mcp_server_update(h, 'add-me', body)
|
|
saved = mock_save.call_args[0][1]
|
|
assert 'keep' in saved['mcp_servers']
|
|
assert 'add-me' in saved['mcp_servers']
|
|
|
|
def test_empty_name_rejected(self):
|
|
h = _make_handler()
|
|
h.command = 'PUT'
|
|
_handle_mcp_server_update(h, '', {"command": "test"})
|
|
assert h.send_response.called
|
|
status = h.send_response.call_args[0][0]
|
|
assert status == 400
|
|
|
|
def test_missing_command_and_url_rejected(self):
|
|
h = _make_handler()
|
|
h.command = 'PUT'
|
|
_handle_mcp_server_update(h, 'test', {"timeout": 30})
|
|
assert h.send_response.called
|
|
status = h.send_response.call_args[0][0]
|
|
assert status == 400
|
|
|
|
|
|
class TestMcpDelete:
|
|
"""DELETE /api/mcp/servers/<name>."""
|
|
|
|
@patch('api.routes.reload_config')
|
|
@patch('api.routes._save_yaml_config_file')
|
|
@patch('api.routes._get_config_path', return_value='/tmp/test.yaml')
|
|
@patch('api.routes.get_config')
|
|
def test_delete_existing(self, mock_cfg, mock_path, mock_save, mock_reload):
|
|
mock_cfg.return_value = {'mcp_servers': {'target': {'command': 'rm'}}}
|
|
h = _make_handler()
|
|
h.command = 'DELETE'
|
|
_handle_mcp_server_delete(h, 'target')
|
|
assert mock_save.called
|
|
saved = mock_save.call_args[0][1]
|
|
assert 'target' not in saved.get('mcp_servers', {})
|
|
|
|
@patch('api.routes.get_config')
|
|
def test_delete_nonexistent(self, mock_cfg):
|
|
mock_cfg.return_value = {'mcp_servers': {}}
|
|
h = _make_handler()
|
|
h.command = 'DELETE'
|
|
_handle_mcp_server_delete(h, 'ghost')
|
|
status = h.send_response.call_args[0][0]
|
|
assert status == 404
|
|
|
|
@patch('api.routes.reload_config')
|
|
@patch('api.routes._save_yaml_config_file')
|
|
@patch('api.routes._get_config_path', return_value='/tmp/test.yaml')
|
|
@patch('api.routes.get_config')
|
|
def test_preserves_others(self, mock_cfg, mock_path, mock_save, mock_reload):
|
|
mock_cfg.return_value = {'mcp_servers': {'a': {'c': '1'}, 'b': {'c': '2'}}}
|
|
h = _make_handler()
|
|
h.command = 'DELETE'
|
|
_handle_mcp_server_delete(h, 'a')
|
|
saved = mock_save.call_args[0][1]
|
|
assert 'a' not in saved['mcp_servers']
|
|
assert 'b' in saved['mcp_servers']
|
|
|
|
def test_empty_name_rejected(self):
|
|
h = _make_handler()
|
|
h.command = 'DELETE'
|
|
_handle_mcp_server_delete(h, '')
|
|
status = h.send_response.call_args[0][0]
|
|
assert status == 400
|
|
|
|
|
|
class TestMaskSecrets:
|
|
"""Unit tests for _mask_secrets helper."""
|
|
|
|
def test_masks_env_values(self):
|
|
obj = {"env": {"API_KEY": "***", "PUBLIC_VAR": "visible"}}
|
|
result = _mask_secrets(obj)
|
|
assert result["env"]["API_KEY"] == "••••••"
|
|
assert result["env"]["PUBLIC_VAR"] == "visible"
|
|
|
|
def test_masks_headers(self):
|
|
obj = {"headers": {"Authorization": "Bearer token", "Accept": "application/json"}}
|
|
result = _mask_secrets(obj)
|
|
assert "••••" in result["headers"]["Authorization"]
|
|
assert result["headers"]["Accept"] == "application/json"
|
|
|
|
def test_passes_non_dict(self):
|
|
assert _mask_secrets("hello") == "hello"
|
|
assert _mask_secrets(42) == 42
|
|
assert _mask_secrets(None) is None
|
|
|
|
def test_handles_empty_dict(self):
|
|
assert _mask_secrets({}) == {}
|
|
|
|
def test_masks_password_key(self):
|
|
obj = {"password": "hunter2"}
|
|
result = _mask_secrets(obj)
|
|
assert result["password"] == "••••••"
|
|
|
|
|
|
class TestStripMaskedValues:
|
|
"""Unit tests for _strip_masked_values helper (secret round-trip protection)."""
|
|
|
|
def test_masked_env_preserves_original(self):
|
|
"""Submitting masked env value should keep the original stored value."""
|
|
existing = {"API_KEY": "real-secret-123", "PUBLIC": "visible"}
|
|
submitted = {"API_KEY": "••••••", "PUBLIC": "updated"}
|
|
result = _strip_masked_values(submitted, existing)
|
|
assert result["API_KEY"] == "real-secret-123"
|
|
assert result["PUBLIC"] == "updated"
|
|
|
|
def test_masked_headers_preserves_original(self):
|
|
"""Submitting masked header value should keep the original stored value."""
|
|
existing = {"Authorization": "Bearer token123", "Accept": "application/json"}
|
|
submitted = {"Authorization": "••••••", "Accept": "text/html"}
|
|
result = _strip_masked_values(submitted, existing)
|
|
assert result["Authorization"] == "Bearer token123"
|
|
assert result["Accept"] == "text/html"
|
|
|
|
def test_new_key_still_saved(self):
|
|
"""New keys (not in existing) should be saved even if they look sensitive."""
|
|
existing = {"OLD_KEY": "old"}
|
|
submitted = {"NEW_KEY": "new-value", "OLD_KEY": "••••••"}
|
|
result = _strip_masked_values(submitted, existing)
|
|
assert result["OLD_KEY"] == "old"
|
|
assert result["NEW_KEY"] == "new-value"
|
|
|
|
def test_non_dict_passthrough(self):
|
|
assert _strip_masked_values("hello", {}) == "hello"
|
|
assert _strip_masked_values(42, {}) == 42
|
|
|
|
def test_empty_dicts(self):
|
|
assert _strip_masked_values({}, {}) == {}
|
|
assert _strip_masked_values({"k": "v"}, {}) == {"k": "v"}
|
|
|
|
|
|
class TestMcpToggle:
|
|
"""PATCH /api/mcp/servers/<name> — enable/disable."""
|
|
|
|
@patch('api.routes.reload_config')
|
|
@patch('api.routes._save_yaml_config_file')
|
|
@patch('api.routes._get_config_path', return_value='/tmp/test.yaml')
|
|
@patch('api.routes.get_config')
|
|
def test_disable_server(self, mock_cfg, mock_path, mock_save, mock_reload):
|
|
mock_cfg.return_value = {'mcp_servers': {'myserver': {'command': 'run'}}}
|
|
h = _make_handler()
|
|
h.command = 'PATCH'
|
|
_handle_mcp_server_toggle(h, 'myserver', {'enabled': False})
|
|
assert mock_save.called
|
|
saved = mock_save.call_args[0][1]
|
|
assert saved['mcp_servers']['myserver']['enabled'] is False
|
|
assert mock_reload.called
|
|
|
|
@patch('api.routes.reload_config')
|
|
@patch('api.routes._save_yaml_config_file')
|
|
@patch('api.routes._get_config_path', return_value='/tmp/test.yaml')
|
|
@patch('api.routes.get_config')
|
|
def test_enable_server(self, mock_cfg, mock_path, mock_save, mock_reload):
|
|
mock_cfg.return_value = {'mcp_servers': {'myserver': {'command': 'run', 'enabled': False}}}
|
|
h = _make_handler()
|
|
h.command = 'PATCH'
|
|
_handle_mcp_server_toggle(h, 'myserver', {'enabled': True})
|
|
saved = mock_save.call_args[0][1]
|
|
assert saved['mcp_servers']['myserver']['enabled'] is True
|
|
|
|
@patch('api.routes.get_config')
|
|
def test_nonexistent_server_returns_404(self, mock_cfg):
|
|
mock_cfg.return_value = {'mcp_servers': {}}
|
|
h = _make_handler()
|
|
h.command = 'PATCH'
|
|
_handle_mcp_server_toggle(h, 'ghost', {'enabled': True})
|
|
status = h.send_response.call_args[0][0]
|
|
assert status == 404
|
|
|
|
def test_empty_name_rejected(self):
|
|
h = _make_handler()
|
|
h.command = 'PATCH'
|
|
_handle_mcp_server_toggle(h, '', {'enabled': True})
|
|
status = h.send_response.call_args[0][0]
|
|
assert status == 400
|
|
|
|
def test_missing_enabled_field_rejected(self):
|
|
h = _make_handler()
|
|
h.command = 'PATCH'
|
|
_handle_mcp_server_toggle(h, 'myserver', {})
|
|
status = h.send_response.call_args[0][0]
|
|
assert status == 400
|
|
|
|
@patch('api.routes.reload_config')
|
|
@patch('api.routes._save_yaml_config_file')
|
|
@patch('api.routes._get_config_path', return_value='/tmp/test.yaml')
|
|
@patch('api.routes.get_config')
|
|
def test_response_payload(self, mock_cfg, mock_path, mock_save, mock_reload):
|
|
mock_cfg.return_value = {'mcp_servers': {'srv': {'url': 'http://localhost'}}}
|
|
h = _make_handler()
|
|
h.command = 'PATCH'
|
|
_handle_mcp_server_toggle(h, 'srv', {'enabled': False})
|
|
body = h.wfile.write.call_args[0][0]
|
|
payload = json.loads(body.decode('utf-8'))
|
|
assert payload == {'ok': True, 'name': 'srv', 'enabled': False}
|
|
|
|
@patch('api.routes.reload_config')
|
|
@patch('api.routes._save_yaml_config_file')
|
|
@patch('api.routes._get_config_path', return_value='/tmp/test.yaml')
|
|
@patch('api.routes.get_config')
|
|
def test_url_encoded_name(self, mock_cfg, mock_path, mock_save, mock_reload):
|
|
"""Names with special characters must be URL-decoded."""
|
|
mock_cfg.return_value = {'mcp_servers': {'my server': {'command': 'x'}}}
|
|
h = _make_handler()
|
|
h.command = 'PATCH'
|
|
_handle_mcp_server_toggle(h, 'my%20server', {'enabled': False})
|
|
saved = mock_save.call_args[0][1]
|
|
assert 'my server' in saved['mcp_servers']
|
|
assert saved['mcp_servers']['my server']['enabled'] is False
|