-
Notifications
You must be signed in to change notification settings - Fork 2
feat: concurrent multi-tool daemon support (#12) #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
86f2b05
df0344b
8556fda
d15609d
d5dc54e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |||||||||||||||||||
| import shutil | ||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||
| from datetime import datetime | ||||||||||||||||||||
| from typing import Optional | ||||||||||||||||||||
| from typing import List, Optional | ||||||||||||||||||||
| import click | ||||||||||||||||||||
| from rich.console import Console | ||||||||||||||||||||
| from rich.live import Live | ||||||||||||||||||||
|
|
@@ -20,6 +20,7 @@ | |||||||||||||||||||
| from context_scribe.evaluator import get_evaluator, EVALUATOR_REGISTRY | ||||||||||||||||||||
| from context_scribe.bridge.mcp_client import MemoryBankClient | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| logger = logging.getLogger("context_scribe") | ||||||||||||||||||||
| console: Console = Console() | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -163,6 +164,31 @@ def bootstrap_claude_config() -> None: | |||||||||||||||||||
| f.write(f"\n{MASTER_RETRIEVAL_RULE}\n") | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| TOOL_REGISTRY = { | ||||||||||||||||||||
| "gemini-cli": (GeminiCliProvider, bootstrap_global_config), | ||||||||||||||||||||
| "copilot": (CopilotProvider, bootstrap_copilot_config), | ||||||||||||||||||||
| "claude": (ClaudeProvider, bootstrap_claude_config), | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| def _create_providers(tools: List[str]): | ||||||||||||||||||||
| """Create and bootstrap providers for the given tool names. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Raises ValueError for unknown tool names. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| providers = [] | ||||||||||||||||||||
| for tool in tools: | ||||||||||||||||||||
| entry = TOOL_REGISTRY.get(tool) | ||||||||||||||||||||
| if entry is None: | ||||||||||||||||||||
| raise ValueError( | ||||||||||||||||||||
| f"Unknown tool '{tool}'. Available: {', '.join(sorted(TOOL_REGISTRY))}" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| provider_cls, bootstrap_fn = entry | ||||||||||||||||||||
| bootstrap_fn() | ||||||||||||||||||||
| providers.append((tool, provider_cls())) | ||||||||||||||||||||
| return providers | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| def _detect_evaluator(preferred_tool: Optional[str] = None) -> str: | ||||||||||||||||||||
| """Auto-detect which evaluator CLI is available, prioritizing the preferred tool.""" | ||||||||||||||||||||
| # Map tool names to their corresponding CLI commands | ||||||||||||||||||||
|
|
@@ -209,22 +235,20 @@ def _status(msg: str, db, live, debug: bool): | |||||||||||||||||||
| live.update(db.generate_layout()) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| async def run_daemon(tool: str, bank_path: str, debug: bool = False, evaluator_name: str = "auto") -> bool: | ||||||||||||||||||||
| if tool == "gemini-cli": | ||||||||||||||||||||
| bootstrap_global_config() | ||||||||||||||||||||
| provider = GeminiCliProvider() | ||||||||||||||||||||
| elif tool == "copilot": | ||||||||||||||||||||
| bootstrap_copilot_config() | ||||||||||||||||||||
| provider = CopilotProvider() | ||||||||||||||||||||
| elif tool == "claude": | ||||||||||||||||||||
| bootstrap_claude_config() | ||||||||||||||||||||
| provider = ClaudeProvider() | ||||||||||||||||||||
| async def run_daemon(tool: str, bank_path: str, debug: bool = False, evaluator_name: str = "auto", tools: Optional[List[str]] = None) -> bool: | ||||||||||||||||||||
| # Build provider list: --tools takes precedence over --tool | ||||||||||||||||||||
| if tools is not None: | ||||||||||||||||||||
| if not tools: | ||||||||||||||||||||
| raise ValueError("--tools was provided but resolved to an empty list.") | ||||||||||||||||||||
| tool_names = tools | ||||||||||||||||||||
| else: | ||||||||||||||||||||
| provider = None | ||||||||||||||||||||
| if not provider: return False | ||||||||||||||||||||
| tool_names = [tool] | ||||||||||||||||||||
| providers = _create_providers(tool_names) | ||||||||||||||||||||
| if not providers: | ||||||||||||||||||||
| return False | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if evaluator_name == "auto": | ||||||||||||||||||||
| evaluator_name = _detect_evaluator(tool) | ||||||||||||||||||||
| evaluator_name = _detect_evaluator(tool_names[0]) | ||||||||||||||||||||
| evaluator = get_evaluator(evaluator_name) | ||||||||||||||||||||
| mcp_client = MemoryBankClient(bank_path=bank_path) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -234,29 +258,54 @@ async def run_daemon(tool: str, bank_path: str, debug: bool = False, evaluator_n | |||||||||||||||||||
| console.print("[bold red]Fatal Error: Could not connect to the Memory Bank MCP server.[/bold red]") | ||||||||||||||||||||
| raise SystemExit(1) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| db = Dashboard(tool, bank_path) | ||||||||||||||||||||
| display_name = ",".join(tool_names) | ||||||||||||||||||||
| db = Dashboard(display_name, bank_path) | ||||||||||||||||||||
| queue: asyncio.Queue = asyncio.Queue(maxsize=1000) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| async def _watch_provider(tool_name: str, provider): | ||||||||||||||||||||
| """Run a provider's watch() in a thread and feed interactions into the shared queue.""" | ||||||||||||||||||||
| loop = asyncio.get_event_loop() | ||||||||||||||||||||
| watch_iter = provider.watch() | ||||||||||||||||||||
| try: | ||||||||||||||||||||
| while True: | ||||||||||||||||||||
| interaction = await loop.run_in_executor(None, next, watch_iter) | ||||||||||||||||||||
| if interaction is not None: | ||||||||||||||||||||
| await queue.put((tool_name, interaction)) | ||||||||||||||||||||
| except (StopIteration, asyncio.CancelledError, KeyboardInterrupt): | ||||||||||||||||||||
| pass | ||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||
| logger.error("Watcher for %s failed: %s", tool_name, e) | ||||||||||||||||||||
|
||||||||||||||||||||
| logger.error("Watcher for %s failed: %s", tool_name, e) | |
| logger.error("Watcher for %s failed: %s", tool_name, e) | |
| finally: | |
| close = getattr(watch_iter, "close", None) | |
| if callable(close): | |
| try: | |
| close() | |
| except Exception as e: | |
| logger.warning("Failed to close watcher for %s: %s", tool_name, e) |
Copilot
AI
Apr 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Watcher tasks are cancelled in finally, but they are never awaited. This can leave pending tasks and produce "Task was destroyed but it is pending" / unhandled-exception warnings. After cancelling, await them (e.g., await asyncio.gather(*watcher_tasks, return_exceptions=True)) before closing the MCP client.
| task.cancel() | |
| task.cancel() | |
| if watcher_tasks: | |
| await asyncio.gather(*watcher_tasks, return_exceptions=True) |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,141 @@ | ||||||||||||||||||||||||
| """Tests for concurrent multi-tool daemon support.""" | ||||||||||||||||||||||||
| import asyncio | ||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||
| from unittest.mock import patch, MagicMock | ||||||||||||||||||||||||
| import pytest | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @pytest.fixture(autouse=True) | ||||||||||||||||||||||||
| def mock_heavy_deps(): | ||||||||||||||||||||||||
| """Mock heavy imports so we can import main without mcp/rich.""" | ||||||||||||||||||||||||
| mocks = {} | ||||||||||||||||||||||||
| for mod in ["mcp", "mcp.client", "mcp.client.stdio", | ||||||||||||||||||||||||
| "rich", "rich.console", "rich.live", "rich.panel", | ||||||||||||||||||||||||
| "rich.text", "rich.layout", "rich.table", "rich.spinner"]: | ||||||||||||||||||||||||
| if mod not in sys.modules or not hasattr(sys.modules.get(mod), '__file__'): | ||||||||||||||||||||||||
| mocks[mod] = MagicMock() | ||||||||||||||||||||||||
| with patch.dict(sys.modules, mocks): | ||||||||||||||||||||||||
| # Clear cached imports so they re-resolve with mocks | ||||||||||||||||||||||||
| for key in list(sys.modules.keys()): | ||||||||||||||||||||||||
| if key.startswith("context_scribe.main") or key.startswith("context_scribe.bridge"): | ||||||||||||||||||||||||
| del sys.modules[key] | ||||||||||||||||||||||||
| yield | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def test_tool_registry_populated(): | ||||||||||||||||||||||||
| from context_scribe.main import TOOL_REGISTRY | ||||||||||||||||||||||||
| assert "gemini-cli" in TOOL_REGISTRY | ||||||||||||||||||||||||
| assert "copilot" in TOOL_REGISTRY | ||||||||||||||||||||||||
| assert "claude" in TOOL_REGISTRY | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def test_create_providers_single(): | ||||||||||||||||||||||||
| from context_scribe.main import _create_providers | ||||||||||||||||||||||||
| with patch("context_scribe.main.bootstrap_global_config"): | ||||||||||||||||||||||||
| with patch("context_scribe.main.GeminiCliProvider") as mock_cls: | ||||||||||||||||||||||||
| mock_cls.return_value = MagicMock() | ||||||||||||||||||||||||
| providers = _create_providers(["gemini-cli"]) | ||||||||||||||||||||||||
| assert len(providers) == 1 | ||||||||||||||||||||||||
| assert providers[0][0] == "gemini-cli" | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def test_create_providers_multiple(): | ||||||||||||||||||||||||
| from context_scribe.main import _create_providers | ||||||||||||||||||||||||
| with patch("context_scribe.main.bootstrap_global_config"): | ||||||||||||||||||||||||
| with patch("context_scribe.main.bootstrap_claude_config"): | ||||||||||||||||||||||||
| with patch("context_scribe.main.GeminiCliProvider", return_value=MagicMock()): | ||||||||||||||||||||||||
| with patch("context_scribe.main.ClaudeProvider", return_value=MagicMock()): | ||||||||||||||||||||||||
| providers = _create_providers(["gemini-cli", "claude"]) | ||||||||||||||||||||||||
| assert len(providers) == 2 | ||||||||||||||||||||||||
| names = [p[0] for p in providers] | ||||||||||||||||||||||||
| assert "gemini-cli" in names | ||||||||||||||||||||||||
| assert "claude" in names | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def test_create_providers_unknown_raises(): | ||||||||||||||||||||||||
| from context_scribe.main import _create_providers | ||||||||||||||||||||||||
| with pytest.raises(ValueError, match="Unknown tool"): | ||||||||||||||||||||||||
| _create_providers(["nonexistent"]) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def test_create_providers_calls_bootstrap(): | ||||||||||||||||||||||||
| from context_scribe.main import _create_providers, TOOL_REGISTRY | ||||||||||||||||||||||||
| mock_boot = MagicMock() | ||||||||||||||||||||||||
| original = TOOL_REGISTRY["gemini-cli"] | ||||||||||||||||||||||||
| TOOL_REGISTRY["gemini-cli"] = (original[0], mock_boot) | ||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||
| with patch("context_scribe.main.GeminiCliProvider", return_value=MagicMock()): | ||||||||||||||||||||||||
| _create_providers(["gemini-cli"]) | ||||||||||||||||||||||||
| mock_boot.assert_called_once() | ||||||||||||||||||||||||
| finally: | ||||||||||||||||||||||||
| TOOL_REGISTRY["gemini-cli"] = original | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def test_create_providers_all_three(): | ||||||||||||||||||||||||
| from context_scribe.main import _create_providers | ||||||||||||||||||||||||
| with patch("context_scribe.main.bootstrap_global_config"): | ||||||||||||||||||||||||
| with patch("context_scribe.main.bootstrap_copilot_config"): | ||||||||||||||||||||||||
| with patch("context_scribe.main.bootstrap_claude_config"): | ||||||||||||||||||||||||
| with patch("context_scribe.main.GeminiCliProvider", return_value=MagicMock()): | ||||||||||||||||||||||||
| with patch("context_scribe.main.CopilotProvider", return_value=MagicMock()): | ||||||||||||||||||||||||
| with patch("context_scribe.main.ClaudeProvider", return_value=MagicMock()): | ||||||||||||||||||||||||
| providers = _create_providers(["gemini-cli", "copilot", "claude"]) | ||||||||||||||||||||||||
| assert len(providers) == 3 | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # --- CLI tests for --tools flag --- | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def test_cli_tools_deduplication(): | ||||||||||||||||||||||||
| """--tools with duplicate entries should deduplicate preserving order.""" | ||||||||||||||||||||||||
| from click.testing import CliRunner | ||||||||||||||||||||||||
| from context_scribe.main import cli | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| runner = CliRunner() | ||||||||||||||||||||||||
| captured_tools = {} | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| original_run_daemon = None | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| async def fake_run_daemon(tool, bank_path, debug=False, evaluator_name="auto", tools=None): | ||||||||||||||||||||||||
| captured_tools["tools"] = tools | ||||||||||||||||||||||||
| return True | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| with patch("context_scribe.main.run_daemon", side_effect=fake_run_daemon) as mock_rd: | ||||||||||||||||||||||||
| loop = asyncio.new_event_loop() | ||||||||||||||||||||||||
| with patch("asyncio.run", side_effect=lambda coro: loop.run_until_complete(coro)): | ||||||||||||||||||||||||
| result = runner.invoke(cli, ["--tools", "gemini-cli,gemini-cli,claude"]) | ||||||||||||||||||||||||
|
Comment on lines
+102
to
+105
|
||||||||||||||||||||||||
| with patch("context_scribe.main.run_daemon", side_effect=fake_run_daemon) as mock_rd: | |
| loop = asyncio.new_event_loop() | |
| with patch("asyncio.run", side_effect=lambda coro: loop.run_until_complete(coro)): | |
| result = runner.invoke(cli, ["--tools", "gemini-cli,gemini-cli,claude"]) | |
| loop = asyncio.new_event_loop() | |
| try: | |
| with patch("context_scribe.main.run_daemon", side_effect=fake_run_daemon) as mock_rd: | |
| with patch("asyncio.run", side_effect=lambda coro: loop.run_until_complete(coro)): | |
| result = runner.invoke(cli, ["--tools", "gemini-cli,gemini-cli,claude"]) | |
| finally: | |
| loop.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In async contexts,
asyncio.get_event_loop()is deprecated on recent Python versions and can emit DeprecationWarnings; useasyncio.get_running_loop()here instead (and reuse that loop variable inside the coroutine).