From f862e9a9992e50da8ec6259d1a94ed91b0bb842e Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 30 May 2026 17:49:03 -0700 Subject: [PATCH 01/19] cli-41: allow agents load remove mcp --- cecli/tools/load_mcp_tool.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 cecli/tools/load_mcp_tool.py diff --git a/cecli/tools/load_mcp_tool.py b/cecli/tools/load_mcp_tool.py new file mode 100644 index 00000000000..e69de29bb2d From 90ecacd167c14e1fc1d1c0bdd0cbaf48952c8927 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 15 Jun 2026 21:33:18 -0700 Subject: [PATCH 02/19] feat: Update test implementation and cases for MCP tools Co-authored-by: cecli (openai/code) --- tests/tools/test_load_mcp_tool.py | 211 ++++++++++++++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 tests/tools/test_load_mcp_tool.py diff --git a/tests/tools/test_load_mcp_tool.py b/tests/tools/test_load_mcp_tool.py new file mode 100644 index 00000000000..d73d340dc5d --- /dev/null +++ b/tests/tools/test_load_mcp_tool.py @@ -0,0 +1,211 @@ +"""Unit tests for LoadMcpTool.execute.""" + +from unittest.mock import AsyncMock, Mock + +import pytest + +from cecli.tools.load_mcp_tool import LoadMcpTool + + +class DummyIO: + """Mock IO object for testing.""" + + def __init__(self): + self.tool_error = Mock() + self.tool_warning = Mock() + self.tool_output = Mock() + self.interrupt_event = Mock() + + +class DummyCoder: + """Mock Coder object for testing.""" + + def __init__(self): + self.io = DummyIO() + self.mcp_manager = Mock() + self.mcp_manager.servers = [] + self.mcp_manager.connected_servers = {} + self.coroutines = Mock() + self.coroutines.interruptible = AsyncMock() + self.interrupt_event = Mock() + + +@pytest.fixture +def coder(): + """Provide a dummy coder for testing.""" + return DummyCoder() + + +@pytest.fixture +def mock_server(): + """Provide a mock MCP server.""" + server = Mock() + server.name = "test-server" + server.config = {"enabled": True} + return server + + +class TestLoadMcpTool: + """Test cases for LoadMcpTool.""" + + @pytest.mark.asyncio + async def test_no_mcp_servers_found(self, coder): + """Test when no MCP servers are configured.""" + coder.mcp_manager.servers = [] + result = await LoadMcpTool.execute(coder, servers=["test"]) + assert result == "No MCP servers found, nothing to load." + + @pytest.mark.asyncio + async def test_server_not_found(self, coder, mock_server): + """Test when requested server doesn't exist.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.get_server.return_value = None + result = await LoadMcpTool.execute(coder, servers=["nonexistent"]) + assert "MCP server nonexistent does not exist." in result + + @pytest.mark.asyncio + async def test_server_already_loaded(self, coder, mock_server): + """Test when server is already loaded.""" + mock_server.name = "test-server" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {"test-server": mock_server} + coder.mcp_manager.get_server.return_value = mock_server + # Must return tuple (did_connect, interrupted) + coder.coroutines.interruptible.return_value = (True, False) + result = await LoadMcpTool.execute(coder, servers=["test-server"]) + assert "Server already loaded: test-server" in result + + @pytest.mark.asyncio + async def test_server_not_enabled_by_default(self, coder, mock_server): + """Test when server is not enabled by default.""" + mock_server.config = {"enabled": False} + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.get_server.return_value = mock_server + result = await LoadMcpTool.execute(coder, servers=["*"]) + assert "Skipping server (not enabled by default): test-server" in result + + @pytest.mark.asyncio + async def test_successful_load(self, coder, mock_server): + """Test successful server loading.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {} + coder.mcp_manager.get_server.return_value = mock_server + coder.coroutines.interruptible.return_value = (True, False) + result = await LoadMcpTool.execute(coder, servers=["test-server"]) + assert "Loaded server: test-server" in result + + @pytest.mark.asyncio + async def test_load_interrupted(self, coder, mock_server): + """Test when loading is interrupted.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {} + coder.mcp_manager.get_server.return_value = mock_server + coder.coroutines.interruptible.return_value = (False, True) + result = await LoadMcpTool.execute(coder, servers=["test-server"]) + assert "Interrupted: test-server" in result + + @pytest.mark.asyncio + async def test_load_failed(self, coder, mock_server): + """Test when loading fails.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {} + coder.mcp_manager.get_server.return_value = mock_server + coder.coroutines.interruptible.return_value = (False, False) + result = await LoadMcpTool.execute(coder, servers=["test-server"]) + assert "Unable to load server: test-server" in result + + @pytest.mark.asyncio + async def test_load_all_servers(self, coder): + """Test loading all servers with '*' wildcard.""" + server1 = Mock() + server1.name = "server1" + server1.config = {"enabled": True} + server2 = Mock() + server2.name = "server2" + server2.config = {"enabled": True} + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) + coder.coroutines.interruptible.return_value = (True, False) + result = await LoadMcpTool.execute(coder, servers=["*"]) + assert "Loaded server: server1" in result + assert "Loaded server: server2" in result + + @pytest.mark.asyncio + async def test_mixed_results(self, coder): + """Test mixed success/failure results.""" + server1 = Mock() + server1.name = "server1" + server1.config = {"enabled": True} + server2 = Mock() + server2.name = "server2" + server2.config = {"enabled": True} + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) + + async def mock_interruptible_func(*args, **kwargs): + # First call succeeds, second fails + if not hasattr(mock_interruptible_func, "call_count"): + mock_interruptible_func.call_count = 0 + mock_interruptible_func.call_count += 1 + if mock_interruptible_func.call_count == 1: + return (True, False) + else: + return (False, False) + + coder.coroutines.interruptible.side_effect = mock_interruptible_func + result = await LoadMcpTool.execute(coder, servers=["server1", "server2"]) + assert "Loaded server: server1" in result + assert "Unable to load server: server2" in result + + @pytest.mark.asyncio + async def test_duplicate_iteration_bug_fix(self, coder, mock_server): + """Test that duplicate iteration bug is fixed - server already loaded only processed once.""" + mock_server.name = "test-server" + coder.mcp_manager.servers = [mock_server] + # Server already connected + coder.mcp_manager.connected_servers = {"test-server": mock_server} + coder.mcp_manager.get_server.return_value = mock_server + + result = await LoadMcpTool.execute(coder, servers=["test-server"]) + + # Should only report server already loaded once + assert result.count("Server already loaded: test-server") == 1 + # connect_server should not have been called since it was already loaded + coder.mcp_manager.connect_server.assert_not_called() + + @pytest.mark.asyncio + async def test_wildcard_with_duplicate_iteration_fix(self, coder): + """Test wildcard loading with duplicate iteration fix.""" + server1 = Mock() + server1.name = "server1" + server1.config = {"enabled": True} + server2 = Mock() + server2.name = "server2" + server2.config = {"enabled": True} + coder.mcp_manager.servers = [server1, server2] + # server1 already loaded, server2 not loaded + coder.mcp_manager.connected_servers = {"server1": server1} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) + + # Track calls to connect_server + connect_calls = [] + + async def mock_connect(server_name): + connect_calls.append(server_name) + return True, False + + coder.mcp_manager.connect_server.side_effect = mock_connect + result = await LoadMcpTool.execute(coder, servers=["*"]) + + # Should only attempt to load server2 (server1 should be skipped) + assert "Server already loaded: server1" in result + assert "Loaded server: server2" in result + assert connect_calls == ["server2"] # Only server2 should have been connected From f800aa5e7e31425df496d891a298c0b64c8d73b9 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 15 Jun 2026 23:44:46 -0700 Subject: [PATCH 03/19] fix: Update load_mcp_tool and its tests Co-authored-by: cecli (openai/code) --- cecli/tools/load_mcp_tool.py | 78 +++++++++++++++++++++++++++++++ tests/tools/test_load_mcp_tool.py | 13 +++--- 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/cecli/tools/load_mcp_tool.py b/cecli/tools/load_mcp_tool.py index e69de29bb2d..c82f467a42b 100644 --- a/cecli/tools/load_mcp_tool.py +++ b/cecli/tools/load_mcp_tool.py @@ -0,0 +1,78 @@ +from typing import List + +from cecli.tools.utils.base_tool import BaseTool + + +class LoadMcpTool(BaseTool): + NORM_NAME = "load-mcp" + SCHEMA = { + "type": "function", + "function": { + "name": "load-mcp", + "description": "Load MCP server(s) by name, or use '*' to load all enabled servers.", + "parameters": { + "type": "object", + "properties": { + "servers": { + "type": "array", + "items": {"type": "string"}, + "description": "A list of MCP server names to load. Use '*' to load all.", + } + }, + "required": ["servers"], + }, + }, + } + + @classmethod + async def execute(cls, coder, servers: List[str]): + """Execute the load-mcp tool with given parameters.""" + if not coder.mcp_manager or not coder.mcp_manager.servers: + return "No MCP servers found, nothing to load." + + results = [] + servers_to_load = [] + + if servers == ["*"]: + for server in coder.mcp_manager.servers: + if server.name in coder.mcp_manager.connected_servers: + results.append(f"Server already loaded: {server.name}") + continue + auto_connect = server.config.get("enabled", True) + if not auto_connect: + results.append(f"Skipping server (not enabled by default): {server.name}") + continue + servers_to_load.append(server) + else: + for server_name in servers: + server = coder.mcp_manager.get_server(server_name) + if server is None: + results.append(f"MCP server {server_name} does not exist.") + else: + servers_to_load.append(server) + + if not servers_to_load and results: + return "\n".join(results) + + # Process the loading + for server in servers_to_load: + server_name = server.name + if server_name in coder.mcp_manager.connected_servers: + results.append(f"Server already loaded: {server_name}") + continue + + coder.interrupt_event.clear() + did_connect, interrupted = await coder.coroutines.interruptible( + coder.mcp_manager.connect_server(server_name), + coder.interrupt_event, + ) + + if interrupted: + results.append(f"Interrupted: {server_name}") + continue + if did_connect: + results.append(f"Loaded server: {server_name}") + else: + results.append(f"Unable to load server: {server_name}") + + return "\n".join(results) diff --git a/tests/tools/test_load_mcp_tool.py b/tests/tools/test_load_mcp_tool.py index d73d340dc5d..b168c54d270 100644 --- a/tests/tools/test_load_mcp_tool.py +++ b/tests/tools/test_load_mcp_tool.py @@ -195,14 +195,15 @@ async def test_wildcard_with_duplicate_iteration_fix(self, coder): (s for s in [server1, server2] if s.name == name), None ) - # Track calls to connect_server - connect_calls = [] + coder.coroutines.interruptible.return_value = (True, False) - async def mock_connect(server_name): - connect_calls.append(server_name) - return True, False + async def mock_interruptible(coro, event): + server_name = coro.cr_frame.f_locals["server_name"] + if server_name == "server2": + return True, False + return False, False - coder.mcp_manager.connect_server.side_effect = mock_connect + coder.coroutines.interruptible.side_effect = mock_interruptible result = await LoadMcpTool.execute(coder, servers=["*"]) # Should only attempt to load server2 (server1 should be skipped) From a2edd66be768c801a81d6b87eb10c05f54e8a5db Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 16 Jun 2026 02:09:06 -0700 Subject: [PATCH 04/19] fix: Correctly mock async function and test non-existent server Co-authored-by: cecli (openai/code) --- tests/unit/test_remove_mcp_tool.py | 103 +++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 tests/unit/test_remove_mcp_tool.py diff --git a/tests/unit/test_remove_mcp_tool.py b/tests/unit/test_remove_mcp_tool.py new file mode 100644 index 00000000000..39ff5c8275b --- /dev/null +++ b/tests/unit/test_remove_mcp_tool.py @@ -0,0 +1,103 @@ +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from cecli.tools.remove_mcp_tool import RemoveMcpTool + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_success(): + """Test successful removal of an MCP server.""" + # Setup + coder = MagicMock() + coder.mcp_manager = MagicMock() + server = MagicMock() + server.name = "test-server" + server.config.get.return_value = True # auto_connect enabled + coder.mcp_manager.get_server.return_value = server + coder.mcp_manager.connected_servers = {"test-server": server} + # Mock disconnect_server as an AsyncMock that returns (True, False) + coder.mcp_manager.disconnect_server = AsyncMock(return_value=(True, False)) + + # Mock the interruptible method to execute the coroutine + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines = MagicMock() + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + + # Execute + result = await RemoveMcpTool.execute(coder, ["test-server"]) + + # Assertions + assert "Removed server: test-server" in result + coder.mcp_manager.disconnect_server.assert_awaited_once_with("test-server") + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_non_existent(): + """Test removing a non-existent MCP server.""" + # Setup + coder = MagicMock() + coder.mcp_manager = MagicMock() + # Create a mock server that exists (to bypass the 'no servers' check) + existing_server = MagicMock() + existing_server.name = "existing-server" + existing_server.config.get.return_value = True + coder.mcp_manager.servers = [existing_server] + # But the one we're looking for doesn't exist + coder.mcp_manager.get_server.return_value = None + + # Execute + result = await RemoveMcpTool.execute(coder, ["non-existent-server"]) + + # Assertions + assert "MCP server non-existent-server does not exist." in result + + assert "MCP server non-existent-server does not exist." in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_not_connected(): + """Test removing a server that is not connected.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + server = MagicMock() + server.name = "test-server" + coder.mcp_manager.servers = [server] + coder.mcp_manager.get_server.return_value = server + coder.mcp_manager.connected_servers = {} + + result = await RemoveMcpTool.execute(coder, ["test-server"]) + + assert "Server test-server is not currently connected." in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_wildcard(): + """Test removing all servers with wildcard '*'.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + + server1 = MagicMock() + server1.name = "server1" + server2 = MagicMock() + server2.name = "server2" + + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {"server1": server1, "server2": server2} + coder.mcp_manager.disconnect_server = AsyncMock(return_value=True) + + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines = MagicMock() + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + + result = await RemoveMcpTool.execute(coder, ["*"]) + + assert "Removed server: server1" in result + assert "Removed server: server2" in result + assert coder.mcp_manager.disconnect_server.await_count == 2 From b8d1b421c15abf4415628a2f31c08bc84b49c525 Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 16 Jun 2026 02:42:19 -0700 Subject: [PATCH 05/19] fix: Correct test_load_mcp_tool_wildcard_and_duplicate_fix assertions Co-authored-by: cecli (openai/code) --- tests/unit/test_load_mcp_tool.py | 134 +++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 tests/unit/test_load_mcp_tool.py diff --git a/tests/unit/test_load_mcp_tool.py b/tests/unit/test_load_mcp_tool.py new file mode 100644 index 00000000000..87c8706dbd7 --- /dev/null +++ b/tests/unit/test_load_mcp_tool.py @@ -0,0 +1,134 @@ +"""Unit tests for load-mcp tool.""" + +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from cecli.tools.load_mcp_tool import LoadMcpTool + + +@pytest.fixture +def mock_mcp_manager(): + """Fixture for a mocked McpServerManager.""" + manager = MagicMock() + manager.connected_servers = {} + + # Mock servers + server1 = MagicMock() + server1.name = "test-server" + server1.config = {"enabled": True} + server2 = MagicMock() + server2.name = "server2" + server2.config = {"enabled": True} + server3 = MagicMock() + server3.name = "server3" + server3.config = {"enabled": False} + + manager.servers = [server1, server2, server3] + + def get_server_side_effect(name): + if name == "test-server": + return server1 + if name == "server2": + return server2 + if name == "server3": + return server3 + return None + + manager.get_server.side_effect = get_server_side_effect + + async def connect(server_name): + manager.connected_servers[server_name] = "connected" + return True, False # (did_connect, interrupted) + + async def disconnect(server_name): + if server_name in manager.connected_servers: + del manager.connected_servers[server_name] + return True, False + return False, False + + manager.connect_server = AsyncMock(side_effect=connect) + manager.disconnect_server = AsyncMock(side_effect=disconnect) + manager.add_server = AsyncMock() + + return manager + + +@pytest.mark.asyncio +async def test_load_mcp_tool_success(mock_mcp_manager): + """Test loading a single MCP server successfully.""" + tool = LoadMcpTool() + # Mock the coder + coder = MagicMock() + coder.mcp_manager = mock_mcp_manager + coder.coroutines = MagicMock() + + # Mock interruptible to return (await coro, False) + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible.side_effect = mock_interruptible + + result = await tool.execute(coder, servers=["test-server"]) + + assert "Loaded server: test-server" in result + mock_mcp_manager.connect_server.assert_awaited_once_with("test-server") + + +@pytest.mark.asyncio +async def test_load_mcp_tool_non_existent(mock_mcp_manager): + """Test loading a non-existent MCP server.""" + tool = LoadMcpTool() + coder = MagicMock() + coder.mcp_manager = mock_mcp_manager + + result = await tool.execute(coder, servers=["non-existent-server"]) + + assert "MCP server non-existent-server does not exist." in result + mock_mcp_manager.connect_server.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_load_mcp_tool_already_loaded(mock_mcp_manager): + """Test loading an already loaded MCP server.""" + tool = LoadMcpTool() + coder = MagicMock() + coder.mcp_manager = mock_mcp_manager + # Pre-populate connected_servers + server = MagicMock() + server.name = "test-server" + coder.mcp_manager.connected_servers = {"test-server": server} + + result = await tool.execute(coder, servers=["test-server"]) + + assert "Server already loaded: test-server" in result + mock_mcp_manager.connect_server.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_load_mcp_tool_wildcard_and_duplicate_fix(mock_mcp_manager): + """Test loading with wildcard and duplicate fix.""" + tool = LoadMcpTool() + coder = MagicMock() + coder.mcp_manager = mock_mcp_manager + coder.coroutines = MagicMock() + + # Mock interruptible to return (await coro, False) + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible.side_effect = mock_interruptible + + # Set up connected_servers: server1 is already connected + server1 = mock_mcp_manager.get_server("test-server") + coder.mcp_manager.connected_servers = {"test-server": server1} + + result = await tool.execute(coder, servers=["*"]) + + # Check results + assert "Server already loaded: test-server" in result + assert "Loaded server: server2" in result + assert "Skipping server (not enabled by default): server3" in result + + # Verify connect_server was called only once for server2 + mock_mcp_manager.connect_server.assert_awaited_once_with("server2") From 2ac74c8a29ea8353e7d3d8cf145c163a7bf201cd Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 19 Jun 2026 20:48:26 -0700 Subject: [PATCH 06/19] cli-41: add tests --- cecli/coders/base_coder.py | 5 +- cecli/tools/__init__.py | 4 + cecli/tools/remove_mcp_tool.py | 77 ++++ .../integration/test_agent_mcp_management.py | 137 ++++++++ tests/integration/test_mcp_management.py | 93 +++++ tests/tools/test_remove_mcp_tool.py | 146 ++++++++ ...cp_tool.py => test_tools_load_mcp_tool.py} | 10 +- tests/unit/test_load_mcp_tool.py | 330 +++++++++++++----- tests/unit/test_remove_mcp_tool.py | 196 ++++++++++- tests/unit/test_unit_load_mcp_tool.py | 139 ++++++++ tests/unit/test_unit_remove_mcp_tool.py | 280 +++++++++++++++ 11 files changed, 1298 insertions(+), 119 deletions(-) create mode 100644 cecli/tools/remove_mcp_tool.py create mode 100644 tests/integration/test_agent_mcp_management.py create mode 100644 tests/integration/test_mcp_management.py create mode 100644 tests/tools/test_remove_mcp_tool.py rename tests/tools/{test_load_mcp_tool.py => test_tools_load_mcp_tool.py} (96%) create mode 100644 tests/unit/test_unit_load_mcp_tool.py create mode 100644 tests/unit/test_unit_remove_mcp_tool.py diff --git a/cecli/coders/base_coder.py b/cecli/coders/base_coder.py index 70fc943b2b2..259f7da796c 100755 --- a/cecli/coders/base_coder.py +++ b/cecli/coders/base_coder.py @@ -412,9 +412,10 @@ def __init__( security_config=None, uuid: str = "", parent_uuid: str = "", + **kwargs, ): - # initialize from args.map_cache_dir - self.coroutines = coroutines + self.original_kwargs = kwargs + self.original_kwargs = kwargs # Per-instance tool and server filtering dictionaries # Each contains "included" and "excluded" sets that filter from the global singletons self.registered_tools = {"included": set(), "excluded": set()} diff --git a/cecli/tools/__init__.py b/cecli/tools/__init__.py index 44e527cff37..4a979b0c123 100644 --- a/cecli/tools/__init__.py +++ b/cecli/tools/__init__.py @@ -17,9 +17,11 @@ git_show, git_status, grep, + load_mcp_tool, load_skill, ls, read_range, + remove_mcp_tool, remove_skill, thinking, undo_change, @@ -49,4 +51,6 @@ thinking, undo_change, update_todo_list, + load_mcp_tool, + remove_mcp_tool, ] diff --git a/cecli/tools/remove_mcp_tool.py b/cecli/tools/remove_mcp_tool.py new file mode 100644 index 00000000000..7692eba027b --- /dev/null +++ b/cecli/tools/remove_mcp_tool.py @@ -0,0 +1,77 @@ +from typing import List + +from cecli.tools.utils.base_tool import BaseTool + + +class RemoveMcpTool(BaseTool): + NORM_NAME = "remove-mcp" + SCHEMA = { + "type": "function", + "function": { + "name": "remove-mcp", + "description": ( + "Remove MCP server(s) by name, or use '*' to remove all connected servers." + ), + "parameters": { + "type": "object", + "properties": { + "servers": { + "type": "array", + "items": {"type": "string"}, + "description": ( + "A list of MCP server names to remove. Use '*' to remove all." + ), + } + }, + "required": ["servers"], + }, + }, + } + + @classmethod + async def execute(cls, coder, servers: List[str]): + """Execute the remove-mcp tool with given parameters.""" + if not coder.mcp_manager or not coder.mcp_manager.servers: + return "No MCP servers are configured." + + results = [] + servers_to_action = [] + + # Determine which servers to act on + if servers == ["*"]: + servers_to_action.extend(coder.mcp_manager.connected_servers.keys()) + else: + for server_name in servers: + server = coder.mcp_manager.get_server(server_name) + if not server: + results.append(f"MCP server {server_name} does not exist.") + elif server.name not in coder.mcp_manager.connected_servers: + results.append(f"Server {server_name} is not currently connected.") + else: + servers_to_action.append(server.name) + + # If there are no servers to act on but we have preliminary results (like errors), return them + if not servers_to_action and results: + return "\n".join(results) + + # If there are no servers to remove at all + if not servers_to_action: + return "No servers to remove." + + # Process the removal + for server_name in servers_to_action: + coder.interrupt_event.clear() + did_disconnect, interrupted = await coder.coroutines.interruptible( + coder.mcp_manager.disconnect_server(server_name), + coder.interrupt_event, + ) + + if interrupted: + results.append(f"Interrupted: {server_name}") + continue + if did_disconnect: + results.append(f"Removed server: {server_name}") + else: + results.append(f"Unable to remove server: {server_name}") + + return "\n".join(results) diff --git a/tests/integration/test_agent_mcp_management.py b/tests/integration/test_agent_mcp_management.py new file mode 100644 index 00000000000..39d20e90101 --- /dev/null +++ b/tests/integration/test_agent_mcp_management.py @@ -0,0 +1,137 @@ +"""Integration tests for agent and subagent MCP management.""" + +from unittest.mock import AsyncMock, MagicMock, Mock, patch + +import pytest + +from cecli.coders.agent_coder import AgentCoder +from cecli.coders.sub_agent_coder import SubAgentCoder +from cecli.tools.load_mcp_tool import LoadMcpTool +from cecli.tools.remove_mcp_tool import RemoveMcpTool + + +@pytest.fixture +def mock_mcp_manager(): + """Fixture for a mocked McpServerManager.""" + manager = MagicMock() + manager.connected_servers = {} + + # Mock servers + server1 = MagicMock() + server1.name = "test_server" + server1.config = {"enabled": True} + + server2 = MagicMock() + server2.name = "sub_test_server" + server2.config = {"enabled": True} + + manager.servers = [server1, server2] + + def get_server_side_effect(name): + if name == "test_server": + return server1 + if name == "sub_test_server": + return server2 + return None + + manager.get_server.side_effect = get_server_side_effect + + async def connect(server_name): + manager.connected_servers[server_name] = "connected" + return True, False # (did_connect, interrupted) + + async def disconnect(server_name): + if server_name in manager.connected_servers: + del manager.connected_servers[server_name] + return True, False + return False, False + + manager.connect_server = AsyncMock(side_effect=connect) + manager.disconnect_server = AsyncMock(side_effect=disconnect) + manager.add_server = AsyncMock() + manager.add_server = AsyncMock() + manager.add_server = AsyncMock() + manager.add_server = AsyncMock() + + return manager + + +@pytest.fixture +def agent_coder(mock_mcp_manager): + """Fixture for an AgentCoder with a mocked MCP manager.""" + with patch("cecli.coders.agent_coder.McpServerManager", return_value=mock_mcp_manager): + coder = AgentCoder( + main_model=MagicMock(), + io=MagicMock(), + ) + coder.mcp_manager = mock_mcp_manager + coder.original_kwargs = {} + coder.coroutines = Mock() + + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible.side_effect = mock_interruptible + coder.interrupt_event = Mock() + return coder + + +@pytest.fixture +async def sub_agent_coder(agent_coder): + """Fixture for a SubAgentCoder.""" + # Fix: Use create() class method instead of direct instantiation + sub_agent = await SubAgentCoder.create(from_coder=agent_coder) + # Ensure sub_agent has the required mocks for tools + sub_agent.coroutines = agent_coder.coroutines + sub_agent.interrupt_event = agent_coder.interrupt_event + return sub_agent + + +@pytest.mark.asyncio +async def test_agent_can_load_mcp_server(agent_coder, mock_mcp_manager): + """Verify an agent can load an MCP server.""" + tool = LoadMcpTool() + server_name = "test_server" + + await tool.execute(agent_coder, servers=[server_name]) + + mock_mcp_manager.connect_server.assert_called_once_with(server_name) + assert server_name in mock_mcp_manager.connected_servers + + +@pytest.mark.asyncio +async def test_agent_can_remove_mcp_server(agent_coder, mock_mcp_manager): + """Verify an agent can remove an MCP server.""" + tool = RemoveMcpTool() + server_name = "test_server" + mock_mcp_manager.connected_servers[server_name] = "connected" + + await tool.execute(agent_coder, servers=[server_name]) + + mock_mcp_manager.disconnect_server.assert_called_once_with(server_name) + assert server_name not in mock_mcp_manager.connected_servers + + +@pytest.mark.asyncio +async def test_sub_agent_can_load_mcp_server(sub_agent_coder, mock_mcp_manager): + """Verify a subagent can load an MCP server.""" + tool = LoadMcpTool() + server_name = "sub_test_server" + + await tool.execute(sub_agent_coder, servers=[server_name]) + + mock_mcp_manager.connect_server.assert_called_once_with(server_name) + assert server_name in mock_mcp_manager.connected_servers + + +@pytest.mark.asyncio +async def test_sub_agent_can_remove_mcp_server(sub_agent_coder, mock_mcp_manager): + """Verify a subagent can remove an MCP server.""" + tool = RemoveMcpTool() + server_name = "sub_test_server" + mock_mcp_manager.connected_servers[server_name] = "connected" + + await tool.execute(sub_agent_coder, servers=[server_name]) + + mock_mcp_manager.disconnect_server.assert_called_once_with(server_name) + assert server_name not in mock_mcp_manager.connected_servers diff --git a/tests/integration/test_mcp_management.py b/tests/integration/test_mcp_management.py new file mode 100644 index 00000000000..0f7f9f38bf1 --- /dev/null +++ b/tests/integration/test_mcp_management.py @@ -0,0 +1,93 @@ +"""Integration tests for MCP management tools.""" + +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from cecli.tools.load_mcp_tool import LoadMcpTool +from cecli.tools.remove_mcp_tool import RemoveMcpTool + + +class CoderMock: + """Mock Coder object for integration testing.""" + + def __init__(self): + self.mcp_manager = MagicMock() + self.mcp_manager.servers = [] + self.mcp_manager.connected_servers = {} + self.mcp_manager.get_server.return_value = None + self.mcp_manager.connect_server = AsyncMock(return_value=(True, False)) + self.mcp_manager.disconnect_server = AsyncMock(return_value=(True, False)) + self.coroutines = MagicMock() + self.interrupt_event = MagicMock() + + async def mock_interruptible(self, coro, event): + """Mock interruptible that just executes the coroutine.""" + return await coro, False + + def add_server(self, name, enabled=True): + """Add a mock server to the manager.""" + server = MagicMock() + server.name = name + server.config = {"enabled": enabled} + self.mcp_manager.servers.append(server) + original_get_server = self.mcp_manager.get_server.side_effect + + def get_server_side_effect(server_name): + if server_name == name: + return server + if original_get_server: + return original_get_server(server_name) + return None + + self.mcp_manager.get_server.side_effect = get_server_side_effect + + +@pytest.fixture +def coder(): + """Provide a mock coder for integration testing.""" + return CoderMock() + + +@pytest.mark.asyncio +async def test_integration_load_and_remove_server(coder): + """Test loading and then removing a server.""" + coder.add_server("integration-test-server") + coder.coroutines.interruptible = coder.mock_interruptible + + # Load the server + load_result = await LoadMcpTool.execute(coder, ["integration-test-server"]) + assert "Loaded server: integration-test-server" in load_result + + # Mock the connected server for the remove tool + coder.mcp_manager.connected_servers = {"integration-test-server": coder.mcp_manager.servers[0]} + + # Remove the server + remove_result = await RemoveMcpTool.execute(coder, ["integration-test-server"]) + assert "Removed server: integration-test-server" in remove_result + + +@pytest.mark.asyncio +async def test_integration_wildcard_load_and_remove(coder): + """Test loading and removing all servers with a wildcard.""" + coder.add_server("server1") + coder.add_server("server2") + coder.add_server("server3", enabled=False) + coder.coroutines.interruptible = coder.mock_interruptible + + # Load all enabled servers + load_result = await LoadMcpTool.execute(coder, ["*"]) + assert "Loaded server: server1" in load_result + assert "Loaded server: server2" in load_result + assert "Skipping server (not enabled by default): server3" in load_result + + # Mock the connected servers for the remove tool + coder.mcp_manager.connected_servers = { + "server1": coder.mcp_manager.servers[0], + "server2": coder.mcp_manager.servers[1], + } + + # Remove all connected servers + remove_result = await RemoveMcpTool.execute(coder, ["*"]) + assert "Removed server: server1" in remove_result + assert "Removed server: server2" in remove_result diff --git a/tests/tools/test_remove_mcp_tool.py b/tests/tools/test_remove_mcp_tool.py new file mode 100644 index 00000000000..3d800de5218 --- /dev/null +++ b/tests/tools/test_remove_mcp_tool.py @@ -0,0 +1,146 @@ +"""Unit tests for RemoveMcpTool.execute.""" + +from unittest.mock import AsyncMock, Mock + +import pytest + +from cecli.tools.remove_mcp_tool import RemoveMcpTool + + +class DummyIO: + """Mock IO object for testing.""" + + def __init__(self): + self.tool_error = Mock() + self.tool_warning = Mock() + self.tool_output = Mock() + self.interrupt_event = Mock() + + +class DummyCoder: + """Mock Coder object for testing.""" + + def __init__(self): + self.io = DummyIO() + self.mcp_manager = Mock() + self.mcp_manager.servers = [] + self.mcp_manager.connected_servers = {} + self.coroutines = Mock() + self.coroutines.interruptible = AsyncMock() + + self.interrupt_event = Mock() + + +@pytest.fixture +def coder(): + """Provide a dummy coder for testing.""" + return DummyCoder() + + +@pytest.fixture +def mock_server(): + """Provide a mock MCP server.""" + server = Mock() + server.name = "test-server" + return server + + +class TestRemoveMcpTool: + """Test cases for RemoveMcpTool.""" + + @pytest.mark.asyncio + async def test_no_configured_servers(self, coder): + """Test when no MCP servers are configured at all.""" + coder.mcp_manager.servers = [] + result = await RemoveMcpTool.execute(coder, servers=["test"]) + assert result == "No MCP servers are configured." + + @pytest.mark.asyncio + async def test_server_not_found(self, coder, mock_server): + """Test when requested server doesn't exist.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {"existing": "server"} + coder.mcp_manager.get_server.return_value = None + result = await RemoveMcpTool.execute(coder, servers=["nonexistent"]) + assert "MCP server nonexistent does not exist." in result + + @pytest.mark.asyncio + async def test_all_servers_not_loaded(self, coder, mock_server): + """Test when multiple servers exist but are not loaded.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {} + coder.mcp_manager.get_server.return_value = mock_server + result = await RemoveMcpTool.execute(coder, servers=["test-server"]) + assert "Server test-server is not currently connected." in result + + @pytest.mark.asyncio + async def test_successful_removal(self, coder, mock_server): + """Test successful server removal.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {"test-server": mock_server} + coder.mcp_manager.get_server.return_value = mock_server + coder.coroutines.interruptible.return_value = (True, False) + result = await RemoveMcpTool.execute(coder, servers=["test-server"]) + assert "Removed server: test-server" in result + + @pytest.mark.asyncio + async def test_removal_interrupted(self, coder, mock_server): + """Test when removal is interrupted.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {"test-server": mock_server} + coder.mcp_manager.get_server.return_value = mock_server + coder.coroutines.interruptible.return_value = (False, True) + result = await RemoveMcpTool.execute(coder, servers=["test-server"]) + assert "Interrupted: test-server" in result + + @pytest.mark.asyncio + async def test_removal_failed(self, coder, mock_server): + """Test when removal fails.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {"test-server": mock_server} + coder.mcp_manager.get_server.return_value = mock_server + coder.coroutines.interruptible.return_value = (False, False) + result = await RemoveMcpTool.execute(coder, servers=["test-server"]) + assert "Unable to remove server: test-server" in result + + @pytest.mark.asyncio + async def test_remove_all_servers(self, coder): + """Test removing all servers with '*' wildcard.""" + server1 = Mock() + server1.name = "server1" + server2 = Mock() + server2.name = "server2" + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {"server1": server1, "server2": server2} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) + coder.coroutines.interruptible.return_value = (True, False) + result = await RemoveMcpTool.execute(coder, servers=["*"]) + assert "Removed server: server1" in result + assert "Removed server: server2" in result + + @pytest.mark.asyncio + async def test_mixed_results(self, coder): + """Test mixed success/failure results.""" + server1 = Mock() + server1.name = "server1" + server2 = Mock() + server2.name = "server2" + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {"server1": server1, "server2": server2} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) + call_count = 0 + + async def mock_interruptible_func(*args, **kwargs): + nonlocal call_count + result = (True, False) if call_count == 0 else (False, False) + call_count += 1 + return result + + coder.coroutines.interruptible.side_effect = mock_interruptible_func + result = await RemoveMcpTool.execute(coder, servers=["server1", "server2"]) + assert "Removed server: server1" in result + assert "Unable to remove server: server2" in result diff --git a/tests/tools/test_load_mcp_tool.py b/tests/tools/test_tools_load_mcp_tool.py similarity index 96% rename from tests/tools/test_load_mcp_tool.py rename to tests/tools/test_tools_load_mcp_tool.py index b168c54d270..6c0f5e45cbd 100644 --- a/tests/tools/test_load_mcp_tool.py +++ b/tests/tools/test_tools_load_mcp_tool.py @@ -194,16 +194,16 @@ async def test_wildcard_with_duplicate_iteration_fix(self, coder): coder.mcp_manager.get_server.side_effect = lambda name: next( (s for s in [server1, server2] if s.name == name), None ) + connect_calls = [] - coder.coroutines.interruptible.return_value = (True, False) - - async def mock_interruptible(coro, event): - server_name = coro.cr_frame.f_locals["server_name"] + async def mock_connect_server(server_name): + connect_calls.append(server_name) if server_name == "server2": return True, False return False, False - coder.coroutines.interruptible.side_effect = mock_interruptible + coder.mcp_manager.connect_server.side_effect = mock_connect_server + coder.coroutines.interruptible.side_effect = mock_connect_server result = await LoadMcpTool.execute(coder, servers=["*"]) # Should only attempt to load server2 (server1 should be skipped) diff --git a/tests/unit/test_load_mcp_tool.py b/tests/unit/test_load_mcp_tool.py index 87c8706dbd7..9a0048007f8 100644 --- a/tests/unit/test_load_mcp_tool.py +++ b/tests/unit/test_load_mcp_tool.py @@ -1,4 +1,4 @@ -"""Unit tests for load-mcp tool.""" +"""Unit tests for LoadMcpTool.execute.""" from unittest.mock import AsyncMock, MagicMock @@ -7,128 +7,272 @@ from cecli.tools.load_mcp_tool import LoadMcpTool +class DummyIO: + """Mock IO object for testing.""" + + def __init__(self): + self.tool_error = MagicMock() + self.tool_warning = MagicMock() + self.tool_output = MagicMock() + self.interrupt_event = MagicMock() + + +class DummyCoder: + """Mock Coder object for testing.""" + + def __init__(self): + self.io = DummyIO() + self.mcp_manager = MagicMock() + self.mcp_manager.servers = [] + self.mcp_manager.connected_servers = {} + self.coroutines = MagicMock() + self.interrupt_event = MagicMock() + + @pytest.fixture -def mock_mcp_manager(): - """Fixture for a mocked McpServerManager.""" - manager = MagicMock() - manager.connected_servers = {} +def coder(): + """Provide a dummy coder for testing.""" + return DummyCoder() - # Mock servers - server1 = MagicMock() - server1.name = "test-server" - server1.config = {"enabled": True} - server2 = MagicMock() - server2.name = "server2" - server2.config = {"enabled": True} - server3 = MagicMock() - server3.name = "server3" - server3.config = {"enabled": False} - - manager.servers = [server1, server2, server3] - - def get_server_side_effect(name): - if name == "test-server": - return server1 - if name == "server2": - return server2 - if name == "server3": - return server3 - return None - - manager.get_server.side_effect = get_server_side_effect - - async def connect(server_name): - manager.connected_servers[server_name] = "connected" - return True, False # (did_connect, interrupted) - - async def disconnect(server_name): - if server_name in manager.connected_servers: - del manager.connected_servers[server_name] - return True, False - return False, False - manager.connect_server = AsyncMock(side_effect=connect) - manager.disconnect_server = AsyncMock(side_effect=disconnect) - manager.add_server = AsyncMock() +@pytest.fixture +def mock_server(): + """Provide a mock MCP server.""" + server = MagicMock() + server.name = "test-server" + server.config = {"enabled": True} + return server + + +@pytest.mark.asyncio +async def test_no_mcp_servers_found(coder): + """Test when no MCP servers are configured.""" + coder.mcp_manager.servers = [] + result = await LoadMcpTool.execute(coder, servers=["test"]) + assert result == "No MCP servers found, nothing to load." + - return manager +@pytest.mark.asyncio +async def test_server_not_found(coder, mock_server): + """Test when requested server doesn't exist.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.get_server.return_value = None + result = await LoadMcpTool.execute(coder, servers=["nonexistent"]) + assert "MCP server nonexistent does not exist." in result @pytest.mark.asyncio -async def test_load_mcp_tool_success(mock_mcp_manager): - """Test loading a single MCP server successfully.""" - tool = LoadMcpTool() - # Mock the coder - coder = MagicMock() - coder.mcp_manager = mock_mcp_manager - coder.coroutines = MagicMock() - - # Mock interruptible to return (await coro, False) +async def test_server_already_loaded(coder, mock_server): + """Test when server is already loaded.""" + mock_server.name = "test-server" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {"test-server": mock_server} + coder.mcp_manager.get_server.return_value = mock_server + # Set up connect_server as AsyncMock so assert_not_called works + coder.mcp_manager.connect_server = AsyncMock() + + # Mock interruptible to just execute the coroutine async def mock_interruptible(coro, event): return await coro, False - coder.coroutines.interruptible.side_effect = mock_interruptible + coder.coroutines.interruptible = mock_interruptible + result = await LoadMcpTool.execute(coder, servers=["test-server"]) + assert "Server already loaded: test-server" in result + # connect_server should not have been called since it was already loaded + coder.mcp_manager.connect_server.assert_not_called() - result = await tool.execute(coder, servers=["test-server"]) - assert "Loaded server: test-server" in result - mock_mcp_manager.connect_server.assert_awaited_once_with("test-server") +@pytest.mark.asyncio +async def test_server_not_enabled_by_default(coder, mock_server): + """Test when server is not enabled by default.""" + mock_server.config = {"enabled": False} + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.get_server.return_value = mock_server + result = await LoadMcpTool.execute(coder, servers=["*"]) + assert "Skipping server (not enabled by default): test-server" in result @pytest.mark.asyncio -async def test_load_mcp_tool_non_existent(mock_mcp_manager): - """Test loading a non-existent MCP server.""" - tool = LoadMcpTool() - coder = MagicMock() - coder.mcp_manager = mock_mcp_manager +async def test_successful_load(coder, mock_server): + """Test successful server loading.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {} + coder.mcp_manager.get_server.return_value = mock_server + + # Set up connect_server as AsyncMock that returns (True, False) + async def mock_connect_server(server_name): + return True, False - result = await tool.execute(coder, servers=["non-existent-server"]) + coder.mcp_manager.connect_server = mock_connect_server - assert "MCP server non-existent-server does not exist." in result - mock_mcp_manager.connect_server.assert_not_awaited() + # Mock interruptible to just execute the coroutine + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible = mock_interruptible + result = await LoadMcpTool.execute(coder, servers=["test-server"]) + assert "Loaded server: test-server" in result @pytest.mark.asyncio -async def test_load_mcp_tool_already_loaded(mock_mcp_manager): - """Test loading an already loaded MCP server.""" - tool = LoadMcpTool() - coder = MagicMock() - coder.mcp_manager = mock_mcp_manager - # Pre-populate connected_servers - server = MagicMock() - server.name = "test-server" - coder.mcp_manager.connected_servers = {"test-server": server} +async def test_load_interrupted(coder, mock_server): + """Test when loading is interrupted.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {} + coder.mcp_manager.get_server.return_value = mock_server - result = await tool.execute(coder, servers=["test-server"]) + # Set up connect_server as AsyncMock + async def mock_connect_server(server_name): + return True, False - assert "Server already loaded: test-server" in result - mock_mcp_manager.connect_server.assert_not_awaited() + coder.mcp_manager.connect_server = mock_connect_server + + # Mock interruptible to return interruption + async def mock_interruptible(coro, event): + return False, True + + coder.coroutines.interruptible = mock_interruptible + result = await LoadMcpTool.execute(coder, servers=["test-server"]) + assert "Interrupted: test-server" in result @pytest.mark.asyncio -async def test_load_mcp_tool_wildcard_and_duplicate_fix(mock_mcp_manager): - """Test loading with wildcard and duplicate fix.""" - tool = LoadMcpTool() - coder = MagicMock() - coder.mcp_manager = mock_mcp_manager - coder.coroutines = MagicMock() - - # Mock interruptible to return (await coro, False) +async def test_load_failed(coder, mock_server): + """Test when loading fails.""" + coder.mcp_manager.servers = [mock_server] + coder.mcp_manager.connected_servers = {} + coder.mcp_manager.get_server.return_value = mock_server + + # Set up connect_server as AsyncMock that returns failure + async def mock_connect_server(server_name): + return False, False + + coder.mcp_manager.connect_server = mock_connect_server + + # Mock interruptible to just execute the coroutine async def mock_interruptible(coro, event): return await coro, False - coder.coroutines.interruptible.side_effect = mock_interruptible + coder.coroutines.interruptible = mock_interruptible + result = await LoadMcpTool.execute(coder, servers=["test-server"]) + assert "Unable to load server: test-server" in result - # Set up connected_servers: server1 is already connected - server1 = mock_mcp_manager.get_server("test-server") - coder.mcp_manager.connected_servers = {"test-server": server1} - result = await tool.execute(coder, servers=["*"]) +@pytest.mark.asyncio +async def test_load_all_servers(coder): + """Test loading all servers with '*' wildcard.""" + server1 = MagicMock() + server1.name = "server1" + server1.config = {"enabled": True} + server2 = MagicMock() + server2.name = "server2" + server2.config = {"enabled": True} + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) - # Check results - assert "Server already loaded: test-server" in result + # Set up connect_server as AsyncMock + async def mock_connect_server(server_name): + return True, False + + coder.mcp_manager.connect_server = mock_connect_server + + # Mock interruptible to just execute the coroutine + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible = mock_interruptible + result = await LoadMcpTool.execute(coder, servers=["*"]) + assert "Loaded server: server1" in result assert "Loaded server: server2" in result - assert "Skipping server (not enabled by default): server3" in result - # Verify connect_server was called only once for server2 - mock_mcp_manager.connect_server.assert_awaited_once_with("server2") + +@pytest.mark.asyncio +async def test_mixed_results(coder): + """Test mixed success/failure results.""" + server1 = MagicMock() + server1.name = "server1" + server1.config = {"enabled": True} + server2 = MagicMock() + server2.name = "server2" + server2.config = {"enabled": True} + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) + # First call succeeds, second fails + call_count = 0 + + async def mock_connect_server(server_name): + nonlocal call_count + result = True if call_count == 0 else False + call_count += 1 + return result + + coder.mcp_manager.connect_server = mock_connect_server + + # Mock interruptible to just execute the coroutine + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible = mock_interruptible + result = await LoadMcpTool.execute(coder, servers=["server1", "server2"]) + assert "Loaded server: server1" in result + assert "Unable to load server: server2" in result + + +@pytest.mark.asyncio +async def test_duplicate_iteration_bug_fix(coder, mock_server): + """Test that duplicate iteration bug is fixed - server already loaded only processed once.""" + mock_server.name = "test-server" + coder.mcp_manager.servers = [mock_server] + # Server already connected + coder.mcp_manager.connected_servers = {"test-server": mock_server} + coder.mcp_manager.get_server.return_value = mock_server + # Set up connect_server as AsyncMock + coder.mcp_manager.connect_server = AsyncMock() + result = await LoadMcpTool.execute(coder, servers=["test-server"]) + # Should only report server already loaded once + assert result.count("Server already loaded: test-server") == 1 + # connect_server should not have been called since it was already loaded + coder.mcp_manager.connect_server.assert_not_called() + + +@pytest.mark.asyncio +async def test_wildcard_with_duplicate_iteration_fix(coder): + """Test wildcard loading with duplicate iteration fix.""" + server1 = MagicMock() + server1.name = "server1" + server1.config = {"enabled": True} + server2 = MagicMock() + server2.name = "server2" + server2.config = {"enabled": True} + coder.mcp_manager.servers = [server1, server2] + # server1 already loaded, server2 not loaded + coder.mcp_manager.connected_servers = {"server1": server1} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) + connect_calls = [] + + async def mock_connect_server(server_name): + connect_calls.append(server_name) + if server_name == "server2": + return True, False + return False, False + + coder.mcp_manager.connect_server = mock_connect_server + + # Mock interruptible to just execute the coroutine + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible = mock_interruptible + result = await LoadMcpTool.execute(coder, servers=["*"]) + # Should only attempt to load server2 (server1 should be skipped) + assert "Server already loaded: server1" in result + assert "Loaded server: server2" in result + assert connect_calls == ["server2"] # Only server2 should have been connected diff --git a/tests/unit/test_remove_mcp_tool.py b/tests/unit/test_remove_mcp_tool.py index 39ff5c8275b..27d7e09a7ec 100644 --- a/tests/unit/test_remove_mcp_tool.py +++ b/tests/unit/test_remove_mcp_tool.py @@ -1,10 +1,48 @@ -from unittest.mock import AsyncMock, MagicMock +"""Unit tests for RemoveMcpTool.execute.""" + +from unittest.mock import MagicMock import pytest from cecli.tools.remove_mcp_tool import RemoveMcpTool +class DummyIO: + """Mock IO object for testing.""" + + def __init__(self): + self.tool_error = MagicMock() + self.tool_warning = MagicMock() + self.tool_output = MagicMock() + self.interrupt_event = MagicMock() + + +class DummyCoder: + """Mock Coder object for testing.""" + + def __init__(self): + self.io = DummyIO() + self.mcp_manager = MagicMock() + self.mcp_manager.servers = [] + self.mcp_manager.connected_servers = {} + self.coroutines = MagicMock() + self.interrupt_event = MagicMock() + + +@pytest.fixture +def coder(): + """Provide a dummy coder for testing.""" + return DummyCoder() + + +@pytest.fixture +def mock_server(): + """Provide a mock MCP server.""" + server = MagicMock() + server.name = "test-server" + return server + + @pytest.mark.asyncio async def test_remove_mcp_tool_success(): """Test successful removal of an MCP server.""" @@ -13,23 +51,24 @@ async def test_remove_mcp_tool_success(): coder.mcp_manager = MagicMock() server = MagicMock() server.name = "test-server" - server.config.get.return_value = True # auto_connect enabled coder.mcp_manager.get_server.return_value = server coder.mcp_manager.connected_servers = {"test-server": server} - # Mock disconnect_server as an AsyncMock that returns (True, False) - coder.mcp_manager.disconnect_server = AsyncMock(return_value=(True, False)) - # Mock the interruptible method to execute the coroutine + # Mock disconnect_server as an async function that returns (True, False) + async def mock_disconnect(server_name): + return True, False + + coder.mcp_manager.disconnect_server = mock_disconnect + + # Mock the interruptible method to execute the coroutine directly without interruption async def mock_interruptible(coro, event): return await coro, False coder.coroutines = MagicMock() coder.coroutines.interruptible = mock_interruptible coder.interrupt_event = MagicMock() - # Execute result = await RemoveMcpTool.execute(coder, ["test-server"]) - # Assertions assert "Removed server: test-server" in result coder.mcp_manager.disconnect_server.assert_awaited_once_with("test-server") @@ -44,19 +83,14 @@ async def test_remove_mcp_tool_non_existent(): # Create a mock server that exists (to bypass the 'no servers' check) existing_server = MagicMock() existing_server.name = "existing-server" - existing_server.config.get.return_value = True coder.mcp_manager.servers = [existing_server] # But the one we're looking for doesn't exist coder.mcp_manager.get_server.return_value = None - # Execute result = await RemoveMcpTool.execute(coder, ["non-existent-server"]) - # Assertions assert "MCP server non-existent-server does not exist." in result - assert "MCP server non-existent-server does not exist." in result - @pytest.mark.asyncio async def test_remove_mcp_tool_not_connected(): @@ -68,9 +102,7 @@ async def test_remove_mcp_tool_not_connected(): coder.mcp_manager.servers = [server] coder.mcp_manager.get_server.return_value = server coder.mcp_manager.connected_servers = {} - result = await RemoveMcpTool.execute(coder, ["test-server"]) - assert "Server test-server is not currently connected." in result @@ -79,25 +111,151 @@ async def test_remove_mcp_tool_wildcard(): """Test removing all servers with wildcard '*'.""" coder = MagicMock() coder.mcp_manager = MagicMock() - server1 = MagicMock() server1.name = "server1" server2 = MagicMock() server2.name = "server2" - coder.mcp_manager.servers = [server1, server2] coder.mcp_manager.connected_servers = {"server1": server1, "server2": server2} - coder.mcp_manager.disconnect_server = AsyncMock(return_value=True) + # Mock disconnect_server as an async function that returns (True, False) + async def mock_disconnect(server_name): + return True, False + + coder.mcp_manager.disconnect_server = mock_disconnect + + # Mock interruptible to execute the coroutine without interruption async def mock_interruptible(coro, event): return await coro, False coder.coroutines = MagicMock() coder.coroutines.interruptible = mock_interruptible coder.interrupt_event = MagicMock() - result = await RemoveMcpTool.execute(coder, ["*"]) + assert "Removed server: server1" in result + assert "Removed server: server2" in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_interrupted(): + """Test when removal is interrupted.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + server = MagicMock() + server.name = "test-server" + coder.mcp_manager.servers = [server] + coder.mcp_manager.get_server.return_value = server + coder.mcp_manager.connected_servers = {"test-server": server} + + async def mock_disconnect(server_name): + return False, True + + coder.mcp_manager.disconnect_server = mock_disconnect + + async def mock_interruptible(coro, event): + return False, True + + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + result = await RemoveMcpTool.execute(coder, ["test-server"]) + assert "Interrupted: test-server" in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_failed(): + """Test when removal fails.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + server = MagicMock() + server.name = "test-server" + coder.mcp_manager.servers = [server] + coder.mcp_manager.get_server.return_value = server + coder.mcp_manager.connected_servers = {"test-server": server} + + async def mock_disconnect(server_name): + return False, False + + coder.mcp_manager.disconnect_server = mock_disconnect + + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + result = await RemoveMcpTool.execute(coder, ["test-server"]) + assert "Unable to remove server: test-server" in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_no_servers_configured(): + """Test when no MCP servers are configured at all.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + coder.mcp_manager.servers = [] + result = await RemoveMcpTool.execute(coder, servers=["test"]) + assert result == "No MCP servers are configured." + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_mixed_results(): + """Test mixed success/failure results.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + server1 = MagicMock() + server1.name = "server1" + server2 = MagicMock() + server2.name = "server2" + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {"server1": server1, "server2": server2} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) + call_count = 0 + async def mock_disconnect(server_name): + nonlocal call_count + result = (True, False) if call_count == 0 else (False, False) + call_count += 1 + return result + + coder.mcp_manager.disconnect_server = mock_disconnect + + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + result = await RemoveMcpTool.execute(coder, servers=["server1", "server2"]) + assert "Removed server: server1" in result + assert "Unable to remove server: server2" in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_dictionary_iteration_fix(): + """Test that dictionary iteration bug is fixed - iterates over keys correctly.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + server1 = MagicMock() + server1.name = "server1" + server2 = MagicMock() + server2.name = "server2" + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {"server1": server1, "server2": server2} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) + + async def mock_disconnect(server_name): + return True, False + + coder.mcp_manager.disconnect_server = mock_disconnect + + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + result = await RemoveMcpTool.execute(coder, servers=["*"]) + # Should successfully remove both servers using dictionary keys assert "Removed server: server1" in result assert "Removed server: server2" in result - assert coder.mcp_manager.disconnect_server.await_count == 2 diff --git a/tests/unit/test_unit_load_mcp_tool.py b/tests/unit/test_unit_load_mcp_tool.py new file mode 100644 index 00000000000..34dfaae1f09 --- /dev/null +++ b/tests/unit/test_unit_load_mcp_tool.py @@ -0,0 +1,139 @@ +"""Unit tests for load-mcp tool.""" + +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from cecli.tools.load_mcp_tool import LoadMcpTool + + +@pytest.fixture +def mock_mcp_manager(): + """Fixture for a mocked McpServerManager.""" + manager = MagicMock() + manager.connected_servers = {} + + # Mock servers + server1 = MagicMock() + server1.name = "test-server" + server1.config = {"enabled": True} + + server2 = MagicMock() + server2.name = "server2" + server2.config = {"enabled": True} + + server3 = MagicMock() + server3.name = "server3" + server3.config = {"enabled": False} + + manager.servers = [server1, server2, server3] + + def get_server_side_effect(name): + if name == "test-server": + return server1 + if name == "server2": + return server2 + if name == "server3": + return server3 + return None + + manager.get_server.side_effect = get_server_side_effect + + async def connect(server_name): + manager.connected_servers[server_name] = "connected" + return True, False # (did_connect, interrupted) + + async def disconnect(server_name): + if server_name in manager.connected_servers: + del manager.connected_servers[server_name] + return True, False + return False, False + + manager.connect_server = AsyncMock(side_effect=connect) + manager.disconnect_server = AsyncMock(side_effect=disconnect) + manager.add_server = AsyncMock() + return manager + + +@pytest.mark.asyncio +async def test_load_mcp_tool_success(mock_mcp_manager): + """Test loading a single MCP server successfully.""" + tool = LoadMcpTool() + + # Mock the coder + coder = MagicMock() + coder.mcp_manager = mock_mcp_manager + + # Mock interruptible to return (await coro, False) + async def mock_interruptible(coro, event): + return await coro + + coder.coroutines = MagicMock() + coder.coroutines.interruptible.side_effect = mock_interruptible + coder.interrupt_event = MagicMock() + + result = await tool.execute(coder, servers=["test-server"]) + + assert "Loaded server: test-server" in result + mock_mcp_manager.connect_server.assert_awaited_once_with("test-server") + + +@pytest.mark.asyncio +async def test_load_mcp_tool_non_existent(mock_mcp_manager): + """Test loading a non-existent MCP server.""" + + tool = LoadMcpTool() + + coder = MagicMock() + coder.mcp_manager = mock_mcp_manager + + result = await tool.execute(coder, servers=["non-existent-server"]) + + assert "MCP server non-existent-server does not exist." in result + mock_mcp_manager.connect_server.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_load_mcp_tool_already_loaded(mock_mcp_manager): + """Test loading an already loaded MCP server.""" + tool = LoadMcpTool() + coder = MagicMock() + coder.mcp_manager = mock_mcp_manager + # Pre-populate connected_servers + server = mock_mcp_manager.get_server("test-server") + coder.mcp_manager.connected_servers = {"test-server": server} + + result = await tool.execute(coder, servers=["test-server"]) + + assert "Server already loaded: test-server" in result + mock_mcp_manager.connect_server.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_load_mcp_tool_wildcard_and_duplicate_fix(mock_mcp_manager): + """Test loading with wildcard and duplicate fix.""" + tool = LoadMcpTool() + coder = MagicMock() + coder.mcp_manager = mock_mcp_manager + + # Mock interruptible to return (await coro, False) + async def mock_interruptible(coro, event): + return await coro + + coder.coroutines = MagicMock() + coder.coroutines.interruptible.side_effect = mock_interruptible + coder.interrupt_event = MagicMock() + + # Set up connected_servers: server1 is already connected + server1 = mock_mcp_manager.get_server("test-server") + coder.mcp_manager.connected_servers = {"test-server": server1} + + result = await tool.execute(coder, servers=["*"]) + + # Check results + assert "Server already loaded: test-server" in result + assert "Loaded server: server2" in result + assert "Skipping server (not enabled by default): server3" in result + + # Verify connect_server was called only once for server2 + mock_mcp_manager.connect_server.assert_awaited_once_with("server2") diff --git a/tests/unit/test_unit_remove_mcp_tool.py b/tests/unit/test_unit_remove_mcp_tool.py new file mode 100644 index 00000000000..0126f1b9a4d --- /dev/null +++ b/tests/unit/test_unit_remove_mcp_tool.py @@ -0,0 +1,280 @@ +"""Unit tests for RemoveMcpTool.execute.""" + +from unittest.mock import MagicMock + +import pytest + +from cecli.tools.remove_mcp_tool import RemoveMcpTool + + +class DummyIO: + """Mock IO object for testing.""" + + def __init__(self): + self.tool_error = MagicMock() + self.tool_warning = MagicMock() + self.tool_output = MagicMock() + self.interrupt_event = MagicMock() + + +class DummyCoder: + """Mock Coder object for testing.""" + + def __init__(self): + self.io = DummyIO() + self.mcp_manager = MagicMock() + self.mcp_manager.servers = [] + self.mcp_manager.connected_servers = {} + self.coroutines = MagicMock() + self.interrupt_event = MagicMock() + + +@pytest.fixture +def coder(): + """Provide a dummy coder for testing.""" + return DummyCoder() + + +@pytest.fixture +def mock_server(): + """Provide a mock MCP server.""" + server = MagicMock() + server.name = "test-server" + return server + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_success(): + """Test successful removal of an MCP server.""" + # Setup + coder = MagicMock() + coder.mcp_manager = MagicMock() + server = MagicMock() + server.name = "test-server" + coder.mcp_manager.get_server.return_value = server + coder.mcp_manager.connected_servers = {"test-server": server} + + # Mock disconnect_server as an AsyncMock that returns (True, False) + async def mock_disconnect(server_name): + return True, False + + coder.mcp_manager.disconnect_server = mock_disconnect + + # Mock the interruptible method to execute the coroutine directly without interruption + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines = MagicMock() + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + + # Execute + result = await RemoveMcpTool.execute(coder, ["test-server"]) + + # Assertions + assert "Removed server: test-server" in result + coder.mcp_manager.disconnect_server.assert_awaited_once_with("test-server") + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_non_existent(): + """Test removing a non-existent MCP server.""" + # Setup + coder = MagicMock() + coder.mcp_manager = MagicMock() + # Create a mock server that exists (to bypass the 'no servers' check) + existing_server = MagicMock() + existing_server.name = "existing-server" + coder.mcp_manager.servers = [existing_server] + # But the one we're looking for doesn't exist + coder.mcp_manager.get_server.return_value = None + + # Execute + result = await RemoveMcpTool.execute(coder, ["non-existent-server"]) + + # Assertions + assert "MCP server non-existent-server does not exist." in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_not_connected(): + """Test removing a server that is not connected.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + server = MagicMock() + server.name = "test-server" + coder.mcp_manager.servers = [server] + coder.mcp_manager.get_server.return_value = server + coder.mcp_manager.connected_servers = {} + + result = await RemoveMcpTool.execute(coder, ["test-server"]) + + assert "Server test-server is not currently connected." in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_wildcard(): + """Test removing all servers with wildcard '*'.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + + server1 = MagicMock() + server1.name = "server1" + server2 = MagicMock() + server2.name = "server2" + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {"server1": server1, "server2": server2} + + # Mock disconnect_server as an AsyncMock that returns (True, False) + async def mock_disconnect(server_name): + return True, False + + coder.mcp_manager.disconnect_server = mock_disconnect + + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines = MagicMock() + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + + result = await RemoveMcpTool.execute(coder, ["*"]) + + assert "Removed server: server1" in result + assert "Removed server: server2" in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_interrupted(): + """Test when removal is interrupted.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + server = MagicMock() + server.name = "test-server" + coder.mcp_manager.servers = [server] + coder.mcp_manager.get_server.return_value = server + coder.mcp_manager.connected_servers = {"test-server": server} + + async def mock_disconnect(server_name): + return False, True + + coder.mcp_manager.disconnect_server = mock_disconnect + + async def mock_interruptible(coro, event): + return False, True + + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + + result = await RemoveMcpTool.execute(coder, ["test-server"]) + + assert "Interrupted: test-server" in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_failed(): + """Test when removal fails.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + server = MagicMock() + server.name = "test-server" + coder.mcp_manager.servers = [server] + coder.mcp_manager.get_server.return_value = server + coder.mcp_manager.connected_servers = {"test-server": server} + + async def mock_disconnect(server_name): + return False, False + + coder.mcp_manager.disconnect_server = mock_disconnect + + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + + result = await RemoveMcpTool.execute(coder, ["test-server"]) + + assert "Unable to remove server: test-server" in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_no_servers_configured(): + """Test when no MCP servers are configured at all.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + coder.mcp_manager.servers = [] + + result = await RemoveMcpTool.execute(coder, servers=["test"]) + + assert result == "No MCP servers are configured." + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_mixed_results(): + """Test mixed success/failure results.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + server1 = MagicMock() + server1.name = "server1" + server2 = MagicMock() + server2.name = "server2" + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {"server1": server1, "server2": server2} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) + + call_count = 0 + + async def mock_disconnect(server_name): + nonlocal call_count + result = (True, False) if call_count == 0 else (False, False) + call_count += 1 + return result + + coder.mcp_manager.disconnect_server = mock_disconnect + + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + + result = await RemoveMcpTool.execute(coder, servers=["server1", "server2"]) + + assert "Removed server: server1" in result + assert "Unable to remove server: server2" in result + + +@pytest.mark.asyncio +async def test_remove_mcp_tool_dictionary_iteration_fix(): + """Test that dictionary iteration bug is fixed - iterates over keys correctly.""" + coder = MagicMock() + coder.mcp_manager = MagicMock() + server1 = MagicMock() + server1.name = "server1" + server2 = MagicMock() + server2.name = "server2" + coder.mcp_manager.servers = [server1, server2] + coder.mcp_manager.connected_servers = {"server1": server1, "server2": server2} + coder.mcp_manager.get_server.side_effect = lambda name: next( + (s for s in [server1, server2] if s.name == name), None + ) + + async def mock_disconnect(server_name): + return True, False + + coder.mcp_manager.disconnect_server = mock_disconnect + + async def mock_interruptible(coro, event): + return await coro, False + + coder.coroutines.interruptible = mock_interruptible + coder.interrupt_event = MagicMock() + + result = await RemoveMcpTool.execute(coder, servers=["*"]) + + # Should successfully remove both servers using dictionary keys + assert "Removed server: server1" in result + assert "Removed server: server2" in result From 7826ce50447a7c2e9a9bb783fa384a026add4a5d Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 20 Jun 2026 11:48:30 -0700 Subject: [PATCH 07/19] fix: Add missing coroutines attribute and fix status_bar access Co-authored-by: cecli (openai/gemini_cli/gemini-2.5-pro) --- cecli/coders/agent_coder.py | 1 + cecli/tui/app.py | 3 ++- tests/tui/test_app.py | 5 ++--- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cecli/coders/agent_coder.py b/cecli/coders/agent_coder.py index d57cfad9a9d..b3fe2fef37c 100644 --- a/cecli/coders/agent_coder.py +++ b/cecli/coders/agent_coder.py @@ -108,6 +108,7 @@ def __init__(self, *args, **kwargs): def post_init(self): super().post_init() + self.coroutines = self.io.coroutines # Populate per-instance tool and server filters from config self.registered_tools["included"] = set( map(str.lower, self.agent_config.get("tools_includelist", [])) diff --git a/cecli/tui/app.py b/cecli/tui/app.py index 9d151bff074..f019122e730 100644 --- a/cecli/tui/app.py +++ b/cecli/tui/app.py @@ -722,7 +722,8 @@ def update_spinner(self, msg, agent_name: str | None = None): def show_error(self, message, agent_name: str | None = None): """Show an error message in the status bar.""" - self.status_bar.show_notification( + status_bar = self.query_one("#status-bar", StatusBar) + status_bar.show_notification( message, severity="error", timeout=5, agent_name=agent_name ) diff --git a/tests/tui/test_app.py b/tests/tui/test_app.py index 5d008b93ad3..768f3cbb4ca 100644 --- a/tests/tui/test_app.py +++ b/tests/tui/test_app.py @@ -197,6 +197,8 @@ def mock_query_one(selector, *args): if isinstance(selector, type): name = selector.__name__ else: + if selector == "#status-bar" or selector == "#status-bar, StatusBar": + return mock_status_bar if "," in selector or "#" in selector: return mock_input_area return mock_footer @@ -217,9 +219,6 @@ def mock_query_one(selector, *args): tui_instance.worker = MagicMock() tui_instance.worker.coder = mock_coder - # Stub status_bar reference - tui_instance.status_bar = mock_status_bar - # Mock AgentService - unknown UUID should return None (no prefix) monkeypatch.setattr( "cecli.helpers.agents.service.AgentService.get_instance", From fcebc490590dbd488fd0629c316297d1de387435 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 20 Jun 2026 12:17:10 -0700 Subject: [PATCH 08/19] fix: Resolve AttributeError in AgentCoder when switching to /agent mode Co-authored-by: cecli (openai/gemini_cli/gemini-2.5-pro) --- cecli/coders/agent_coder.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cecli/coders/agent_coder.py b/cecli/coders/agent_coder.py index b3fe2fef37c..b3f1943df14 100644 --- a/cecli/coders/agent_coder.py +++ b/cecli/coders/agent_coder.py @@ -32,7 +32,7 @@ from .base_coder import Coder -from cecli.helpers.coroutines import interruptible # isort:skip +from cecli.helpers import coroutines # isort:skip logger = logging.getLogger(__name__) @@ -108,7 +108,7 @@ def __init__(self, *args, **kwargs): def post_init(self): super().post_init() - self.coroutines = self.io.coroutines + self.coroutines = coroutines # Populate per-instance tool and server filters from config self.registered_tools["included"] = set( map(str.lower, self.agent_config.get("tools_includelist", [])) @@ -328,7 +328,9 @@ async def _exec_async(): self.io.tool_warning(f"Executing {tool_name} on {server.name} failed:\nError: {e}") return f"Error executing tool call {tool_name}: {e}" - result, interrupted = await interruptible(_exec_async(), self.interrupt_event) + result, interrupted = await self.coroutines.interruptible( + _exec_async(), self.interrupt_event + ) if interrupted: return "Tool execution interrupted by user." From 0028706c240c7075dfed03b38b68eda4d230f098 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 20 Jun 2026 12:22:58 -0700 Subject: [PATCH 09/19] fix: Use self.coroutines.interruptible in agent_coder.py Co-authored-by: cecli (openai/gemini_cli/gemini-2.5-pro) --- cecli/coders/agent_coder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cecli/coders/agent_coder.py b/cecli/coders/agent_coder.py index b3f1943df14..f984bed09b4 100644 --- a/cecli/coders/agent_coder.py +++ b/cecli/coders/agent_coder.py @@ -771,7 +771,7 @@ async def _execute_local_tools(self, tool_calls_list): async def gather_and_await(): return await asyncio.gather(*tasks, return_exceptions=True) - task_results, interrupted = await interruptible( + task_results, interrupted = await self.coroutines.interruptible( gather_and_await(), self.interrupt_event ) From 9e76b89c24d50959dc5604a1610aaaa20f099920 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 20 Jun 2026 19:09:58 -0700 Subject: [PATCH 10/19] refactor: Rename load_mcp_tool and remove_mcp_tool files and classes Co-authored-by: cecli (openai/gemini_cli/gemini-2.5-pro) --- cecli/tools/load_mcp_tool.py | 2 +- cecli/tools/remove_mcp_tool.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cecli/tools/load_mcp_tool.py b/cecli/tools/load_mcp_tool.py index c82f467a42b..938b07c8d4b 100644 --- a/cecli/tools/load_mcp_tool.py +++ b/cecli/tools/load_mcp_tool.py @@ -3,7 +3,7 @@ from cecli.tools.utils.base_tool import BaseTool -class LoadMcpTool(BaseTool): +class LoadMcp(BaseTool): NORM_NAME = "load-mcp" SCHEMA = { "type": "function", diff --git a/cecli/tools/remove_mcp_tool.py b/cecli/tools/remove_mcp_tool.py index 7692eba027b..00f2d14cd56 100644 --- a/cecli/tools/remove_mcp_tool.py +++ b/cecli/tools/remove_mcp_tool.py @@ -3,7 +3,7 @@ from cecli.tools.utils.base_tool import BaseTool -class RemoveMcpTool(BaseTool): +class RemoveMcp(BaseTool): NORM_NAME = "remove-mcp" SCHEMA = { "type": "function", From c35ca016fd200d882a986f2c6bd666f07e09f1d9 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 20 Jun 2026 19:17:31 -0700 Subject: [PATCH 11/19] refactor: Rename load_mcp and remove_mcp tool files and classes Co-authored-by: cecli (openai/gemini_cli/gemini-2.5-pro) --- cecli/tools/__init__.py | 8 ++++---- cecli/tools/load_mcp_tool.py | 2 +- cecli/tools/remove_mcp_tool.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cecli/tools/__init__.py b/cecli/tools/__init__.py index 4a979b0c123..57a87c906f1 100644 --- a/cecli/tools/__init__.py +++ b/cecli/tools/__init__.py @@ -17,11 +17,11 @@ git_show, git_status, grep, - load_mcp_tool, + load_mcp, load_skill, ls, read_range, - remove_mcp_tool, + remove_mcp, remove_skill, thinking, undo_change, @@ -51,6 +51,6 @@ thinking, undo_change, update_todo_list, - load_mcp_tool, - remove_mcp_tool, + load_mcp, + remove_mcp, ] diff --git a/cecli/tools/load_mcp_tool.py b/cecli/tools/load_mcp_tool.py index 938b07c8d4b..d66d3c7a40f 100644 --- a/cecli/tools/load_mcp_tool.py +++ b/cecli/tools/load_mcp_tool.py @@ -3,7 +3,7 @@ from cecli.tools.utils.base_tool import BaseTool -class LoadMcp(BaseTool): +class Tool(BaseTool): NORM_NAME = "load-mcp" SCHEMA = { "type": "function", diff --git a/cecli/tools/remove_mcp_tool.py b/cecli/tools/remove_mcp_tool.py index 00f2d14cd56..12e0feeaed2 100644 --- a/cecli/tools/remove_mcp_tool.py +++ b/cecli/tools/remove_mcp_tool.py @@ -3,7 +3,7 @@ from cecli.tools.utils.base_tool import BaseTool -class RemoveMcp(BaseTool): +class Tool(BaseTool): NORM_NAME = "remove-mcp" SCHEMA = { "type": "function", From 2504fc3aefe1da9ed98e2555a76455b7d59ce477 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 20 Jun 2026 20:12:01 -0700 Subject: [PATCH 12/19] refactor: Add mcp_servers context to AgentCoder Co-authored-by: cecli (openai/gemini_cli/gemini-2.5-pro) --- cecli/coders/agent_coder.py | 49 +++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/cecli/coders/agent_coder.py b/cecli/coders/agent_coder.py index f984bed09b4..320b491b948 100644 --- a/cecli/coders/agent_coder.py +++ b/cecli/coders/agent_coder.py @@ -183,6 +183,7 @@ def _get_agent_config(self): "todo_list", "sub_agents", "skills", + "mcp_servers", }, ) ) @@ -364,6 +365,7 @@ def _calculate_context_block_tokens(self, force=False): "skills", "sub_agents", "loaded_skills", + "mcp_servers", ] for block_type in block_types: if block_type in self.allowed_context_blocks: @@ -402,6 +404,8 @@ def _generate_context_block(self, block_name): not self.parent_uuid or self.agent_config.get("allow_nested_delegation", False) ): content = self.get_sub_agents_context() + elif block_name == "mcp_servers": + content = self.get_mcp_servers_context() if content is not None: self.context_blocks_cache[block_name] = content return content @@ -1512,6 +1516,51 @@ def get_sub_agents_context(self): self.io.tool_error(f"Error generating sub-agents context: {str(e)}") return None + def get_mcp_servers_context(self): + """ + Generate a context block for available and loaded MCP servers. + + Returns: + Formatted context block string or None if no MCP manager is available. + """ + if not self.use_enhanced_context or not self.mcp_manager: + return None + + try: + result = '\n' + result += "## MCP Servers\n\n" + + all_servers = self.mcp_manager.servers + connected_servers = self.mcp_manager.connected_servers + + loaded_server_names = {server.name for server in connected_servers} + available_servers = [ + server for server in all_servers if server.name not in loaded_server_names + ] + + if loaded_server_names: + result += "### Loaded Servers\n" + result += "The following MCP servers are currently loaded and their tools are available:\n" + for name in sorted(list(loaded_server_names)): + result += f"- {name}\n" + result += "\n" + else: + result += "No MCP servers are currently loaded.\n\n" + + if available_servers: + result += "### Available Servers\n" + result += "The following MCP servers are available to be loaded:\n" + for server in sorted(available_servers, key=lambda s: s.name): + result += f"- {server.name}\n" + result += "\n" + + result += "Use the `LoadMcp` and `RemoveMcp` tools to manage servers.\n" + result += "" + return result + except Exception as e: + self.io.tool_error(f"Error generating MCP servers context: {str(e)}") + return None + def get_child_agent_states(self): """Get the state of all active child sub-agents. From a06b4f90430926f0eb795436258b9b131d7a4a38 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 20 Jun 2026 21:23:03 -0700 Subject: [PATCH 13/19] feat: Add ListMcp tool and command Co-authored-by: cecli (openai/gemini_cli/gemini-2.5-pro) --- cecli/coders/agent_coder.py | 48 ----------------------------------- cecli/commands/list_mcp.py | 50 +++++++++++++++++++++++++++++++++++++ cecli/tools/__init__.py | 1 + cecli/tools/list_mcp.py | 50 +++++++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 48 deletions(-) create mode 100644 cecli/commands/list_mcp.py create mode 100644 cecli/tools/list_mcp.py diff --git a/cecli/coders/agent_coder.py b/cecli/coders/agent_coder.py index 320b491b948..599bdfc7c79 100644 --- a/cecli/coders/agent_coder.py +++ b/cecli/coders/agent_coder.py @@ -183,7 +183,6 @@ def _get_agent_config(self): "todo_list", "sub_agents", "skills", - "mcp_servers", }, ) ) @@ -365,7 +364,6 @@ def _calculate_context_block_tokens(self, force=False): "skills", "sub_agents", "loaded_skills", - "mcp_servers", ] for block_type in block_types: if block_type in self.allowed_context_blocks: @@ -404,8 +402,6 @@ def _generate_context_block(self, block_name): not self.parent_uuid or self.agent_config.get("allow_nested_delegation", False) ): content = self.get_sub_agents_context() - elif block_name == "mcp_servers": - content = self.get_mcp_servers_context() if content is not None: self.context_blocks_cache[block_name] = content return content @@ -1516,50 +1512,6 @@ def get_sub_agents_context(self): self.io.tool_error(f"Error generating sub-agents context: {str(e)}") return None - def get_mcp_servers_context(self): - """ - Generate a context block for available and loaded MCP servers. - - Returns: - Formatted context block string or None if no MCP manager is available. - """ - if not self.use_enhanced_context or not self.mcp_manager: - return None - - try: - result = '\n' - result += "## MCP Servers\n\n" - - all_servers = self.mcp_manager.servers - connected_servers = self.mcp_manager.connected_servers - - loaded_server_names = {server.name for server in connected_servers} - available_servers = [ - server for server in all_servers if server.name not in loaded_server_names - ] - - if loaded_server_names: - result += "### Loaded Servers\n" - result += "The following MCP servers are currently loaded and their tools are available:\n" - for name in sorted(list(loaded_server_names)): - result += f"- {name}\n" - result += "\n" - else: - result += "No MCP servers are currently loaded.\n\n" - - if available_servers: - result += "### Available Servers\n" - result += "The following MCP servers are available to be loaded:\n" - for server in sorted(available_servers, key=lambda s: s.name): - result += f"- {server.name}\n" - result += "\n" - - result += "Use the `LoadMcp` and `RemoveMcp` tools to manage servers.\n" - result += "" - return result - except Exception as e: - self.io.tool_error(f"Error generating MCP servers context: {str(e)}") - return None def get_child_agent_states(self): """Get the state of all active child sub-agents. diff --git a/cecli/commands/list_mcp.py b/cecli/commands/list_mcp.py new file mode 100644 index 00000000000..5342c6c8fb8 --- /dev/null +++ b/cecli/commands/list_mcp.py @@ -0,0 +1,50 @@ +from typing import List + +from cecli.commands.utils.base_command import BaseCommand +from cecli.commands.utils.helpers import format_command_result + + +class ListMcpCommand(BaseCommand): + NORM_NAME = "list-mcp" + DESCRIPTION = "List all loaded and available MCP servers." + + @classmethod + async def execute(cls, io, coder, args, **kwargs): + """Execute the list-mcp command.""" + if not coder.mcp_manager: + return format_command_result(io, cls.NORM_NAME, "MCP manager is not available.") + + all_servers = coder.mcp_manager.servers + connected_servers = coder.mcp_manager.connected_servers + + loaded_server_names = {server.name for server in connected_servers} + available_servers = [ + server for server in all_servers if server.name not in loaded_server_names + ] + + result = [] + if loaded_server_names: + result.append("Loaded MCP Servers:") + for name in sorted(list(loaded_server_names)): + result.append(f"- {name}") + else: + result.append("No MCP servers are currently loaded.") + + result.append("") + + if available_servers: + result.append("Available MCP Servers:") + for server in sorted(available_servers, key=lambda s: s.name): + result.append(f"- {server.name}") + else: + result.append("No other MCP servers are available to load.") + + return format_command_result(io, cls.NORM_NAME, "", "\n".join(result)) + + @classmethod + def get_help(cls) -> str: + """Get help text for the list-mcp command.""" + help_text = super().get_help() + help_text += "\nUsage:\n" + help_text += " /list-mcp # Lists all loaded and available MCP servers\n" + return help_text diff --git a/cecli/tools/__init__.py b/cecli/tools/__init__.py index 57a87c906f1..b73123d3d7b 100644 --- a/cecli/tools/__init__.py +++ b/cecli/tools/__init__.py @@ -17,6 +17,7 @@ git_show, git_status, grep, + list_mcp, load_mcp, load_skill, ls, diff --git a/cecli/tools/list_mcp.py b/cecli/tools/list_mcp.py new file mode 100644 index 00000000000..0facedb5a5d --- /dev/null +++ b/cecli/tools/list_mcp.py @@ -0,0 +1,50 @@ +from cecli.tools.utils.base_tool import BaseTool + + +class Tool(BaseTool): + NORM_NAME = "list-mcp" + SCHEMA = { + "type": "function", + "function": { + "name": "ListMcp", + "description": "List all loaded and available MCP servers.", + "parameters": { + "type": "object", + "properties": {}, + "required": [], + }, + }, + } + + @classmethod + def execute(cls, coder, **kwargs): + """List all loaded and available MCP servers.""" + if not coder.mcp_manager: + return "MCP manager is not available." + + all_servers = coder.mcp_manager.servers + connected_servers = coder.mcp_manager.connected_servers + + loaded_server_names = {server.name for server in connected_servers} + available_servers = [ + server for server in all_servers if server.name not in loaded_server_names + ] + + result = [] + if loaded_server_names: + result.append("Loaded MCP Servers:") + for name in sorted(list(loaded_server_names)): + result.append(f"- {name}") + else: + result.append("No MCP servers are currently loaded.") + + result.append("") + + if available_servers: + result.append("Available MCP Servers:") + for server in sorted(available_servers, key=lambda s: s.name): + result.append(f"- {server.name}") + else: + result.append("No other MCP servers are available to load.") + + return "\n".join(result) From 539058755ad4f6e176f918abac05b111e6813629 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 20 Jun 2026 21:24:55 -0700 Subject: [PATCH 14/19] chore: Remove unused blank line in agent_coder.py Co-authored-by: cecli (openai/gemini_cli/gemini-2.5-pro) --- cecli/coders/agent_coder.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cecli/coders/agent_coder.py b/cecli/coders/agent_coder.py index 599bdfc7c79..f984bed09b4 100644 --- a/cecli/coders/agent_coder.py +++ b/cecli/coders/agent_coder.py @@ -1512,7 +1512,6 @@ def get_sub_agents_context(self): self.io.tool_error(f"Error generating sub-agents context: {str(e)}") return None - def get_child_agent_states(self): """Get the state of all active child sub-agents. From c56dc0be41aac5370013b4d28a2142b4dfd56d66 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 21 Jun 2026 02:01:26 -0700 Subject: [PATCH 15/19] fix --- cecli/commands/remove_mcp.py | 2 +- cecli/tools/__init__.py | 5 +++-- cecli/tools/{load_mcp_tool.py => load_mcp.py} | 2 +- cecli/tools/{remove_mcp_tool.py => remove_mcp.py} | 4 ++-- cecli/tui/app.py | 4 +--- tests/unit/{test_load_mcp_tool.py => test_load_mcp.py} | 0 tests/unit/{test_remove_mcp_tool.py => test_remove_mcp.py} | 0 7 files changed, 8 insertions(+), 9 deletions(-) rename cecli/tools/{load_mcp_tool.py => load_mcp.py} (98%) rename cecli/tools/{remove_mcp_tool.py => remove_mcp.py} (95%) rename tests/unit/{test_load_mcp_tool.py => test_load_mcp.py} (100%) rename tests/unit/{test_remove_mcp_tool.py => test_remove_mcp.py} (100%) diff --git a/cecli/commands/remove_mcp.py b/cecli/commands/remove_mcp.py index b3432a97ca0..1f04bc05cac 100644 --- a/cecli/commands/remove_mcp.py +++ b/cecli/commands/remove_mcp.py @@ -6,7 +6,7 @@ class RemoveMcpCommand(BaseCommand): NORM_NAME = "remove-mcp" - DESCRIPTION = "Remove a MCP server by name, or use '*' to remove all" + DESCRIPTION = "Remove (unload) a MCP server by name, or use '*' to remove all" show_completion_notification = False @classmethod diff --git a/cecli/tools/__init__.py b/cecli/tools/__init__.py index b73123d3d7b..54dd6622227 100644 --- a/cecli/tools/__init__.py +++ b/cecli/tools/__init__.py @@ -45,13 +45,14 @@ git_show, git_status, grep, + list_mcp, + load_mcp, load_skill, ls, read_range, + remove_mcp, remove_skill, thinking, undo_change, update_todo_list, - load_mcp, - remove_mcp, ] diff --git a/cecli/tools/load_mcp_tool.py b/cecli/tools/load_mcp.py similarity index 98% rename from cecli/tools/load_mcp_tool.py rename to cecli/tools/load_mcp.py index d66d3c7a40f..fb98f55811d 100644 --- a/cecli/tools/load_mcp_tool.py +++ b/cecli/tools/load_mcp.py @@ -8,7 +8,7 @@ class Tool(BaseTool): SCHEMA = { "type": "function", "function": { - "name": "load-mcp", + "name": "LoadMCP", "description": "Load MCP server(s) by name, or use '*' to load all enabled servers.", "parameters": { "type": "object", diff --git a/cecli/tools/remove_mcp_tool.py b/cecli/tools/remove_mcp.py similarity index 95% rename from cecli/tools/remove_mcp_tool.py rename to cecli/tools/remove_mcp.py index 12e0feeaed2..788647de8c6 100644 --- a/cecli/tools/remove_mcp_tool.py +++ b/cecli/tools/remove_mcp.py @@ -8,9 +8,9 @@ class Tool(BaseTool): SCHEMA = { "type": "function", "function": { - "name": "remove-mcp", + "name": "RemoveMCP", "description": ( - "Remove MCP server(s) by name, or use '*' to remove all connected servers." + "Remove (unload) MCP server(s) by name, or use '*' to remove all connected servers." ), "parameters": { "type": "object", diff --git a/cecli/tui/app.py b/cecli/tui/app.py index f019122e730..b54b498fc0d 100644 --- a/cecli/tui/app.py +++ b/cecli/tui/app.py @@ -723,9 +723,7 @@ def update_spinner(self, msg, agent_name: str | None = None): def show_error(self, message, agent_name: str | None = None): """Show an error message in the status bar.""" status_bar = self.query_one("#status-bar", StatusBar) - status_bar.show_notification( - message, severity="error", timeout=5, agent_name=agent_name - ) + status_bar.show_notification(message, severity="error", timeout=5, agent_name=agent_name) def on_resize(self) -> None: file_list = self.query_one("#file-list", FileList) diff --git a/tests/unit/test_load_mcp_tool.py b/tests/unit/test_load_mcp.py similarity index 100% rename from tests/unit/test_load_mcp_tool.py rename to tests/unit/test_load_mcp.py diff --git a/tests/unit/test_remove_mcp_tool.py b/tests/unit/test_remove_mcp.py similarity index 100% rename from tests/unit/test_remove_mcp_tool.py rename to tests/unit/test_remove_mcp.py From c9dbb962819ccc8a1c77693997fab8efd4e17e28 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 21 Jun 2026 09:07:14 -0700 Subject: [PATCH 16/19] cli-41: relabled available mcps to configured, to not confuse configured with loaded. --- cecli/commands/list_mcp.py | 18 ++++++++---------- cecli/tools/list_mcp.py | 16 ++++++++-------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/cecli/commands/list_mcp.py b/cecli/commands/list_mcp.py index 5342c6c8fb8..fbdc6a7e6ba 100644 --- a/cecli/commands/list_mcp.py +++ b/cecli/commands/list_mcp.py @@ -1,24 +1,22 @@ -from typing import List - from cecli.commands.utils.base_command import BaseCommand from cecli.commands.utils.helpers import format_command_result class ListMcpCommand(BaseCommand): NORM_NAME = "list-mcp" - DESCRIPTION = "List all loaded and available MCP servers." + DESCRIPTION = "List all loaded and configured MCP servers." @classmethod async def execute(cls, io, coder, args, **kwargs): """Execute the list-mcp command.""" if not coder.mcp_manager: - return format_command_result(io, cls.NORM_NAME, "MCP manager is not available.") + return format_command_result(io, cls.NORM_NAME, "MCP manager is not configured.") all_servers = coder.mcp_manager.servers connected_servers = coder.mcp_manager.connected_servers loaded_server_names = {server.name for server in connected_servers} - available_servers = [ + configured_servers = [ server for server in all_servers if server.name not in loaded_server_names ] @@ -32,12 +30,12 @@ async def execute(cls, io, coder, args, **kwargs): result.append("") - if available_servers: - result.append("Available MCP Servers:") - for server in sorted(available_servers, key=lambda s: s.name): + if configured_servers: + result.append("Configured MCP Servers:") + for server in sorted(configured_servers, key=lambda s: s.name): result.append(f"- {server.name}") else: - result.append("No other MCP servers are available to load.") + result.append("No other MCP servers are configured.") return format_command_result(io, cls.NORM_NAME, "", "\n".join(result)) @@ -46,5 +44,5 @@ def get_help(cls) -> str: """Get help text for the list-mcp command.""" help_text = super().get_help() help_text += "\nUsage:\n" - help_text += " /list-mcp # Lists all loaded and available MCP servers\n" + help_text += " /list-mcp # Lists all loaded and configured MCP servers\n" return help_text diff --git a/cecli/tools/list_mcp.py b/cecli/tools/list_mcp.py index 0facedb5a5d..6df6daca038 100644 --- a/cecli/tools/list_mcp.py +++ b/cecli/tools/list_mcp.py @@ -7,7 +7,7 @@ class Tool(BaseTool): "type": "function", "function": { "name": "ListMcp", - "description": "List all loaded and available MCP servers.", + "description": "List all loaded and configured MCP servers.", "parameters": { "type": "object", "properties": {}, @@ -18,15 +18,15 @@ class Tool(BaseTool): @classmethod def execute(cls, coder, **kwargs): - """List all loaded and available MCP servers.""" + """List all loaded and configured MCP servers.""" if not coder.mcp_manager: - return "MCP manager is not available." + return "MCP manager is not configured." all_servers = coder.mcp_manager.servers connected_servers = coder.mcp_manager.connected_servers loaded_server_names = {server.name for server in connected_servers} - available_servers = [ + configured_servers = [ server for server in all_servers if server.name not in loaded_server_names ] @@ -40,11 +40,11 @@ def execute(cls, coder, **kwargs): result.append("") - if available_servers: - result.append("Available MCP Servers:") - for server in sorted(available_servers, key=lambda s: s.name): + if configured_servers: + result.append("Configured MCP Servers:") + for server in sorted(configured_servers, key=lambda s: s.name): result.append(f"- {server.name}") else: - result.append("No other MCP servers are available to load.") + result.append("No other MCP servers are configured.") return "\n".join(result) From 1c1b63441f7f93de5ed96b00437b7f6d47929014 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 21 Jun 2026 09:42:19 -0700 Subject: [PATCH 17/19] cli-41 --- cecli/commands/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cecli/commands/__init__.py b/cecli/commands/__init__.py index 05e352a66ea..d22c2f4b36d 100644 --- a/cecli/commands/__init__.py +++ b/cecli/commands/__init__.py @@ -34,6 +34,7 @@ from .hooks import HooksCommand from .include_skill import IncludeSkillCommand from .lint import LintCommand +from .list_mcp import ListMcpCommand from .list_sessions import ListSessionsCommand from .list_skills import ListSkillsCommand from .load import LoadCommand @@ -121,6 +122,7 @@ CommandRegistry.register(SwitchAgentCommand) CommandRegistry.register(IncludeSkillCommand) CommandRegistry.register(LintCommand) +CommandRegistry.register(ListMcpCommand) CommandRegistry.register(ListSessionsCommand) CommandRegistry.register(ListSkillsCommand) CommandRegistry.register(LoadCommand) @@ -207,6 +209,7 @@ "LoadCommand", "LoadHookCommand", "LoadMcpCommand", + "ListMcpCommand", "LoadSessionCommand", "LoadSkillCommand", "LsCommand", From dfc7bab35ad8d4fbcaa567b8362aeae3ef7e3488 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 21 Jun 2026 10:05:50 -0700 Subject: [PATCH 18/19] cli-41: list --- cecli/commands/list_mcp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cecli/commands/list_mcp.py b/cecli/commands/list_mcp.py index fbdc6a7e6ba..20457590e90 100644 --- a/cecli/commands/list_mcp.py +++ b/cecli/commands/list_mcp.py @@ -37,7 +37,7 @@ async def execute(cls, io, coder, args, **kwargs): else: result.append("No other MCP servers are configured.") - return format_command_result(io, cls.NORM_NAME, "", "\n".join(result)) + return format_command_result(io, cls.NORM_NAME, "\n".join(result)) @classmethod def get_help(cls) -> str: From aa584823415fb0315b463a370773ba5c4a7e4054 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 21 Jun 2026 11:50:56 -0700 Subject: [PATCH 19/19] refactor: Add MCP server management guidance to agent coder Co-authored-by: cecli (openai/gemini_cli/gemini-2.5-pro) --- cecli/coders/agent_coder.py | 7 +++++-- cecli/website/docs/config/agent-mode.md | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/cecli/coders/agent_coder.py b/cecli/coders/agent_coder.py index 0538c909b00..ac217190196 100644 --- a/cecli/coders/agent_coder.py +++ b/cecli/coders/agent_coder.py @@ -627,7 +627,8 @@ def get_context_summary(self): if percentage > 80: result += "\n\n⚠ **Context is getting full!**\n" result += "- Remove non-essential files via the `ContextManager` tool.\n" - result += "- Keep only essential files in context for best performance" + result += "- Remove unused MCP servers via the `RemoveMcp` tool to free context space.\n" + result += "- Keep only essential files and MCP servers in context for best performance" result += "\n" if not hasattr(self, "context_blocks_cache"): self.context_blocks_cache = {} @@ -661,7 +662,9 @@ def get_environment_info(self): result += f"- Git repository: {rel_repo_dir} with {num_files:,} files\n" except Exception: result += "- Git repository: active but details unavailable\n" - else: + if self.mcp_manager and self.mcp_manager.connected_servers: + num_mcp_servers = len(self.mcp_manager.connected_servers) + result += f"- Connected MCP servers: {num_mcp_servers}\n" result += "- Git repository: none\n" result += "" return result diff --git a/cecli/website/docs/config/agent-mode.md b/cecli/website/docs/config/agent-mode.md index d66ac7c14e7..0b50f7c80ca 100644 --- a/cecli/website/docs/config/agent-mode.md +++ b/cecli/website/docs/config/agent-mode.md @@ -309,8 +309,26 @@ agent-config: For complete documentation on creating and using skills, including skill directory structure, SKILL.md format, and best practices, see the [Skills documentation](https://github.com/dwash96/cecli/blob/main/cecli/website/docs/config/skills.md). +### MCP Server Management + +MCP (Model Context Protocol) servers provide external tools to the agent, but each connected server and its tools consume context tokens. To maintain optimal performance: + +- **Remove unused servers**: If an MCP server is no longer needed for the current task, remove it using the `RemoveMcp` tool to free up context space. +- **Load servers on demand**: Only load MCP servers when their tools are actually required. Use the `LoadMcp` tool to add servers as needed. +- **Monitor context usage**: The context summary block shows total token usage. Removing unnecessary MCP servers can significantly reduce context overhead. +- **List active servers**: Use the `ListMcp` tool to see which servers are currently connected and consuming context. + ### Benefits +### MCP Server Management +MCP (Model Context Protocol) servers provide external tools to the agent, but each connected server and its tools consume context tokens. To maintain optimal performance: + +- **Remove unused servers**: If an MCP server is no longer needed for the current task, remove it using the `RemoveMcp` tool to free up context space. +- **Load servers on demand**: Only load MCP servers when their tools are actually required. Use the `LoadMcp` tool to add servers as needed. +- **Monitor context usage**: The context summary block shows total token usage. Removing unnecessary MCP servers can significantly reduce context overhead. +- **List active servers**: Use the `ListMcp` tool to see which servers are currently connected and consuming context. + +### Benefits - **Autonomous operation**: Reduces need for manual file management - **Context awareness**: Real-time project information improves decision making - **Precision editing**: Granular tools reduce errors compared to SEARCH/REPLACE